[PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf

[Brian Starkey]

On Fri, Aug 25, 2017 at 03:34:33PM +0200, Daniel Vetter wrote:
>On Thu, Aug 24, 2017 at 11:04:01AM +0100, Brian Starkey wrote:
>> Hi,
>>
>> Thanks for the CC.
>>
>> On Fri, Aug 18, 2017 at 06:13:14PM +0200, Noralf Tr??nnes wrote:
>> > (cc affected parties)
>> >
>> >
>> > Den 18.08.2017 09.46, skrev Daniel Vetter:
>> > > On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Tr??nnes wrote:
>> > > > Reject mapping an imported dma-buf since is's an invalid use-case.
>> > > >
>> > > > Cc: Philipp Zabel <p.zabel at pengutronix.de>
>> > > > Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> > > > Cc: Sean Paul <seanpaul at chromium.org>
>> > > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> > > > Signed-off-by: Noralf Tr??nnes <noralf at tronnes.org>
>> > > I think acks from someone using mali would be good too. amdgpu already has
>> > > such checks, so I think on the desktop side we're ok.
>> > >
>>
>> This looks like it would break anyone running the Mali-4xx series GPUs
>> with the Arm graphics stack (e.g. Hikey board).
>>
>> I don't know where that sits in terms of policy.
>
>Why would this break this use-case? Is the mali blob somehow using dumb
>mmap for it's own buffers it shares with display? That would be rather
>backwards ...

If it's running with a DRM display driver (which may or may not be
Mali-DP, they are separate IPs), then its window system code always
uses dumb mmap if it needs a pointer. That might be a native DRM
buffer that the display driver allocated, or a dma-buf which it PRIME
imported (which would now fail).

-Brian

>
>Either way, blob -> meh, not a concern really.
>-Daniel
>
>>
>> Cheers,
>> -Brian
>>
>> > > Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>> > >
>> > > But I think this one here definitely needs a few more acks. I could break
>> > > uabi if we're unlucky, so let's not rush it.
>> >
>> > Ok, I've CC'ed the affected parties to increase the odds that they look
>> > at this. These are the drivers using drm_gem_dumb_map_offset()
>> > (hopefully I got the list right):
>> >
>> > arc
>> > atmel-hlcdc
>> > cirrus
>> > exynos
>> > fsl-dcu
>> > gma500
>> > hdlcd
>> > imx
>> > kirin
>> > mali-dp
>> > mediatek
>> > meson
>> > mxsfb
>> > pl111
>> > rcar-du
>> > rockchip
>> > shmobile
>> > sti
>> > stm
>> > sun4i
>> > tegra
>> > tilcd
>> > vc4
>> > zte
>> >
>> >
>> > Noralf.
>> >
>> > > -Daniel
>> > >
>> > > > ---
>> > > >  drivers/gpu/drm/drm_gem.c | 6 ++++++
>> > > >  1 file changed, 6 insertions(+)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> > > > index ad4e9cf..8da5801 100644
>> > > > --- a/drivers/gpu/drm/drm_gem.c
>> > > > +++ b/drivers/gpu/drm/drm_gem.c
>> > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
>> > > >  	if (!obj)
>> > > >  		return -ENOENT;
>> > > > +	/* Don't allow imported objects to be mapped */
>> > > > +	if (obj->import_attach) {
>> > > > +		ret = -EINVAL;
>> > > > +		goto out;
>> > > > +	}
>> > > > +
>> > > >  	ret = drm_gem_create_mmap_offset(obj);
>> > > >  	if (ret)
>> > > >  		goto out;
>> > > > --
>> > > > 2.7.4
>> > > >
>> >
>
>-- 
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch