[PATCH] drm/prime: Ensure mmap offset is initialized

[PATCH] drm/prime: Ensure mmap offset is initialized

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

If a GEM object is allocated, and then exported as a dma-buf fd which is
mmap'd before or without the GEM buffer being directly mmap'd, the
vma_node could be unitialized.  This leads to a situation where the CPU
mapping is not correctly torn down in drm_vma_node_unmap().

Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
Note, it's possible the issue existed in some related form prior to the
commit tagged with Fixes.

 drivers/gpu/drm/drm_prime.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index e3f09f18110c..849eea154dfc 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
 	struct file *fil;
 	int ret;
 
+	/* Ensure that the vma_node is initialized: */
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		return ret;
+
 	/* Add the fake offset */
 	vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
 
-- 
2.35.3

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2179 bytes --]

Hi

Am 29.05.22 um 18:29 schrieb Rob Clark:
> From: Rob Clark <robdclark@chromium.org>
> 
> If a GEM object is allocated, and then exported as a dma-buf fd which is
> mmap'd before or without the GEM buffer being directly mmap'd, the
> vma_node could be unitialized.  This leads to a situation where the CPU
> mapping is not correctly torn down in drm_vma_node_unmap().

Which drivers are affected by this problem?

I checked several drivers and most appear to be initializing the offset 
during object construction, such as GEM SHMEM. [1] TTM-based drivers 
also seem unaffected. [2]

 From a quick grep, only etnaviv, msm and omapdrm appear to be affected? 
They only seem to run drm_gem_create_mmap_offset() from their 
ioctl-handling code.

If so, I'd say it's preferable to fix these drivers and put a 
drm_WARN_ONCE() into drm_gem_prime_mmap().

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
[2] 
https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002

> 
> Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> Note, it's possible the issue existed in some related form prior to the
> commit tagged with Fixes.
> 
>   drivers/gpu/drm/drm_prime.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index e3f09f18110c..849eea154dfc 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>   	struct file *fil;
>   	int ret;
>   
> +	/* Ensure that the vma_node is initialized: */
> +	ret = drm_gem_create_mmap_offset(obj);
> +	if (ret)
> +		return ret;
> +
>   	/* Add the fake offset */
>   	vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
>   

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Rob Clark]

On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 29.05.22 um 18:29 schrieb Rob Clark:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > If a GEM object is allocated, and then exported as a dma-buf fd which is
> > mmap'd before or without the GEM buffer being directly mmap'd, the
> > vma_node could be unitialized.  This leads to a situation where the CPU
> > mapping is not correctly torn down in drm_vma_node_unmap().
>
> Which drivers are affected by this problem?
>
> I checked several drivers and most appear to be initializing the offset
> during object construction, such as GEM SHMEM. [1] TTM-based drivers
> also seem unaffected. [2]
>
>  From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
> They only seem to run drm_gem_create_mmap_offset() from their
> ioctl-handling code.
>
> If so, I'd say it's preferable to fix these drivers and put a
> drm_WARN_ONCE() into drm_gem_prime_mmap().

That is good if fewer drivers are affected, however I disagree with
your proposal.  At least for freedreno userspace, a lot of bo's never
get mmap'd (either directly of via dmabuf), so we should not be
allocating a mmap offset unnecessarily.

BR,
-R

> Best regards
> Thomas
>
> [1]
> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
> [2]
> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
>
> >
> > Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> > Note, it's possible the issue existed in some related form prior to the
> > commit tagged with Fixes.
> >
> >   drivers/gpu/drm/drm_prime.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > index e3f09f18110c..849eea154dfc 100644
> > --- a/drivers/gpu/drm/drm_prime.c
> > +++ b/drivers/gpu/drm/drm_prime.c
> > @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> >       struct file *fil;
> >       int ret;
> >
> > +     /* Ensure that the vma_node is initialized: */
> > +     ret = drm_gem_create_mmap_offset(obj);
> > +     if (ret)
> > +             return ret;
> > +
> >       /* Add the fake offset */
> >       vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3956 bytes --]

Hi

Am 30.05.22 um 15:47 schrieb Rob Clark:
> On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 29.05.22 um 18:29 schrieb Rob Clark:
>>> From: Rob Clark <robdclark@chromium.org>
>>>
>>> If a GEM object is allocated, and then exported as a dma-buf fd which is
>>> mmap'd before or without the GEM buffer being directly mmap'd, the
>>> vma_node could be unitialized.  This leads to a situation where the CPU
>>> mapping is not correctly torn down in drm_vma_node_unmap().
>>
>> Which drivers are affected by this problem?
>>
>> I checked several drivers and most appear to be initializing the offset
>> during object construction, such as GEM SHMEM. [1] TTM-based drivers
>> also seem unaffected. [2]
>>
>>   From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
>> They only seem to run drm_gem_create_mmap_offset() from their
>> ioctl-handling code.
>>
>> If so, I'd say it's preferable to fix these drivers and put a
>> drm_WARN_ONCE() into drm_gem_prime_mmap().
> 
> That is good if fewer drivers are affected, however I disagree with
> your proposal.  At least for freedreno userspace, a lot of bo's never
> get mmap'd (either directly of via dmabuf), so we should not be
> allocating a mmap offset unnecessarily.

I see.

I the reason I'm arguing against the current patch is that the fix 
appears like a workaround and 6 months from now, few will remember why 
it's there. Especially since most drivers initialize the offset 
correctly. (Not too long ago, I refactored the handling of these mmap 
calls throughout DRM drivers and it was confusing at times.)

So here's another suggestion:  I further looked at the 3 drivers that I 
mentioned. etnaviv and msm can easily wrap the call to 
drm_gem_prime_mmap() and init the offset first. [1][2]  omapdrm doesn't 
actually use drm_gem_prime_mmap(). The offset can instead be initialized 
at the top of the driver's dmabuf mmap function. [3]

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/etnaviv/etnaviv_drv.c#L480
[2] 
https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/msm/msm_drv.c#L961
[3] 
https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c#L66

> 
> BR,
> -R
> 
>> Best regards
>> Thomas
>>
>> [1]
>> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
>> [2]
>> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
>>
>>>
>>> Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>>> ---
>>> Note, it's possible the issue existed in some related form prior to the
>>> commit tagged with Fixes.
>>>
>>>    drivers/gpu/drm/drm_prime.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>>> index e3f09f18110c..849eea154dfc 100644
>>> --- a/drivers/gpu/drm/drm_prime.c
>>> +++ b/drivers/gpu/drm/drm_prime.c
>>> @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>>>        struct file *fil;
>>>        int ret;
>>>
>>> +     /* Ensure that the vma_node is initialized: */
>>> +     ret = drm_gem_create_mmap_offset(obj);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>>        /* Add the fake offset */
>>>        vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Rob Clark]

On Mon, May 30, 2022 at 7:16 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 30.05.22 um 15:47 schrieb Rob Clark:
> > On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi
> >>
> >> Am 29.05.22 um 18:29 schrieb Rob Clark:
> >>> From: Rob Clark <robdclark@chromium.org>
> >>>
> >>> If a GEM object is allocated, and then exported as a dma-buf fd which is
> >>> mmap'd before or without the GEM buffer being directly mmap'd, the
> >>> vma_node could be unitialized.  This leads to a situation where the CPU
> >>> mapping is not correctly torn down in drm_vma_node_unmap().
> >>
> >> Which drivers are affected by this problem?
> >>
> >> I checked several drivers and most appear to be initializing the offset
> >> during object construction, such as GEM SHMEM. [1] TTM-based drivers
> >> also seem unaffected. [2]
> >>
> >>   From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
> >> They only seem to run drm_gem_create_mmap_offset() from their
> >> ioctl-handling code.
> >>
> >> If so, I'd say it's preferable to fix these drivers and put a
> >> drm_WARN_ONCE() into drm_gem_prime_mmap().
> >
> > That is good if fewer drivers are affected, however I disagree with
> > your proposal.  At least for freedreno userspace, a lot of bo's never
> > get mmap'd (either directly of via dmabuf), so we should not be
> > allocating a mmap offset unnecessarily.
>
> I see.
>
> I the reason I'm arguing against the current patch is that the fix
> appears like a workaround and 6 months from now, few will remember why
> it's there. Especially since most drivers initialize the offset
> correctly. (Not too long ago, I refactored the handling of these mmap
> calls throughout DRM drivers and it was confusing at times.)

I dispute the "correctly" part.. and that this is a workaround ;-)

But I can send a v2 with the addition of a comment explaining the
reason, so git-blame archeology isn't required to understand the
reasoning

BR,
-R

> So here's another suggestion:  I further looked at the 3 drivers that I
> mentioned. etnaviv and msm can easily wrap the call to
> drm_gem_prime_mmap() and init the offset first. [1][2]  omapdrm doesn't
> actually use drm_gem_prime_mmap(). The offset can instead be initialized
> at the top of the driver's dmabuf mmap function. [3]
>
> Best regards
> Thomas
>
> [1]
> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/etnaviv/etnaviv_drv.c#L480
> [2]
> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/msm/msm_drv.c#L961
> [3]
> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c#L66
>
> >
> > BR,
> > -R
> >
> >> Best regards
> >> Thomas
> >>
> >> [1]
> >> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
> >> [2]
> >> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
> >>
> >>>
> >>> Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> >>> Signed-off-by: Rob Clark <robdclark@chromium.org>
> >>> ---
> >>> Note, it's possible the issue existed in some related form prior to the
> >>> commit tagged with Fixes.
> >>>
> >>>    drivers/gpu/drm/drm_prime.c | 5 +++++
> >>>    1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> >>> index e3f09f18110c..849eea154dfc 100644
> >>> --- a/drivers/gpu/drm/drm_prime.c
> >>> +++ b/drivers/gpu/drm/drm_prime.c
> >>> @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> >>>        struct file *fil;
> >>>        int ret;
> >>>
> >>> +     /* Ensure that the vma_node is initialized: */
> >>> +     ret = drm_gem_create_mmap_offset(obj);
> >>> +     if (ret)
> >>> +             return ret;
> >>> +
> >>>        /* Add the fake offset */
> >>>        vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> >>>
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> (HRB 36809, AG Nürnberg)
> >> Geschäftsführer: Ivo Totev
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Daniel Vetter]

On Mon, 30 May 2022 at 15:54, Rob Clark <robdclark@gmail.com> wrote:
>
> On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > Hi
> >
> > Am 29.05.22 um 18:29 schrieb Rob Clark:
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > If a GEM object is allocated, and then exported as a dma-buf fd which is
> > > mmap'd before or without the GEM buffer being directly mmap'd, the
> > > vma_node could be unitialized.  This leads to a situation where the CPU
> > > mapping is not correctly torn down in drm_vma_node_unmap().
> >
> > Which drivers are affected by this problem?
> >
> > I checked several drivers and most appear to be initializing the offset
> > during object construction, such as GEM SHMEM. [1] TTM-based drivers
> > also seem unaffected. [2]
> >
> >  From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
> > They only seem to run drm_gem_create_mmap_offset() from their
> > ioctl-handling code.
> >
> > If so, I'd say it's preferable to fix these drivers and put a
> > drm_WARN_ONCE() into drm_gem_prime_mmap().
>
> That is good if fewer drivers are affected, however I disagree with
> your proposal.  At least for freedreno userspace, a lot of bo's never
> get mmap'd (either directly of via dmabuf), so we should not be
> allocating a mmap offset unnecessarily.

Does this actually matter in the grand scheme of things? We originally
allocated mmap offset only on demand because userspace only had 32bit
loff_t support and so simply couldn't mmap anything if the offset
ended up above 32bit (even if there was still va space available).

But those days are long gone (about 10 years or so) and the allocation
overhead for an mmap offset is tiny. So I think unless you can
benchmark an impact allocating it at bo alloc seems like the simplest
design overall, and hence what we should be doing. And if the vma
offset allocation every gets too slow due to fragmentation we can lift
the hole tree from i915 into drm_mm and the job should be done. At
that point we could also allocate the offset unconditionally in the
gem_init function and be done with it.

Iow I concur with Thomas here, unless there's hard data contrary
simplicity imo trumps here.
-Daniel

>
> BR,
> -R
>
> > Best regards
> > Thomas
> >
> > [1]
> > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
> > [2]
> > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
> >
> > >
> > > Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > ---
> > > Note, it's possible the issue existed in some related form prior to the
> > > commit tagged with Fixes.
> > >
> > >   drivers/gpu/drm/drm_prime.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > index e3f09f18110c..849eea154dfc 100644
> > > --- a/drivers/gpu/drm/drm_prime.c
> > > +++ b/drivers/gpu/drm/drm_prime.c
> > > @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > >       struct file *fil;
> > >       int ret;
> > >
> > > +     /* Ensure that the vma_node is initialized: */
> > > +     ret = drm_gem_create_mmap_offset(obj);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > >       /* Add the fake offset */
> > >       vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> > >
> >
> > --
> > Thomas Zimmermann
> > Graphics Driver Developer
> > SUSE Software Solutions Germany GmbH
> > Maxfeldstr. 5, 90409 Nürnberg, Germany
> > (HRB 36809, AG Nürnberg)
> > Geschäftsführer: Ivo Totev



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Rob Clark]

On Mon, May 30, 2022 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, 30 May 2022 at 15:54, Rob Clark <robdclark@gmail.com> wrote:
> >
> > On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > >
> > > Hi
> > >
> > > Am 29.05.22 um 18:29 schrieb Rob Clark:
> > > > From: Rob Clark <robdclark@chromium.org>
> > > >
> > > > If a GEM object is allocated, and then exported as a dma-buf fd which is
> > > > mmap'd before or without the GEM buffer being directly mmap'd, the
> > > > vma_node could be unitialized.  This leads to a situation where the CPU
> > > > mapping is not correctly torn down in drm_vma_node_unmap().
> > >
> > > Which drivers are affected by this problem?
> > >
> > > I checked several drivers and most appear to be initializing the offset
> > > during object construction, such as GEM SHMEM. [1] TTM-based drivers
> > > also seem unaffected. [2]
> > >
> > >  From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
> > > They only seem to run drm_gem_create_mmap_offset() from their
> > > ioctl-handling code.
> > >
> > > If so, I'd say it's preferable to fix these drivers and put a
> > > drm_WARN_ONCE() into drm_gem_prime_mmap().
> >
> > That is good if fewer drivers are affected, however I disagree with
> > your proposal.  At least for freedreno userspace, a lot of bo's never
> > get mmap'd (either directly of via dmabuf), so we should not be
> > allocating a mmap offset unnecessarily.
>
> Does this actually matter in the grand scheme of things? We originally
> allocated mmap offset only on demand because userspace only had 32bit
> loff_t support and so simply couldn't mmap anything if the offset
> ended up above 32bit (even if there was still va space available).
>
> But those days are long gone (about 10 years or so) and the allocation
> overhead for an mmap offset is tiny. So I think unless you can
> benchmark an impact allocating it at bo alloc seems like the simplest
> design overall, and hence what we should be doing. And if the vma
> offset allocation every gets too slow due to fragmentation we can lift
> the hole tree from i915 into drm_mm and the job should be done. At
> that point we could also allocate the offset unconditionally in the
> gem_init function and be done with it.
>
> Iow I concur with Thomas here, unless there's hard data contrary
> simplicity imo trumps here.

32b userspace is still alive and well, at least on arm chromebooks ;-)

BR,
-R

> -Daniel
>
> >
> > BR,
> > -R
> >
> > > Best regards
> > > Thomas
> > >
> > > [1]
> > > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
> > > [2]
> > > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
> > >
> > > >
> > > > Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > ---
> > > > Note, it's possible the issue existed in some related form prior to the
> > > > commit tagged with Fixes.
> > > >
> > > >   drivers/gpu/drm/drm_prime.c | 5 +++++
> > > >   1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > > index e3f09f18110c..849eea154dfc 100644
> > > > --- a/drivers/gpu/drm/drm_prime.c
> > > > +++ b/drivers/gpu/drm/drm_prime.c
> > > > @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > > >       struct file *fil;
> > > >       int ret;
> > > >
> > > > +     /* Ensure that the vma_node is initialized: */
> > > > +     ret = drm_gem_create_mmap_offset(obj);
> > > > +     if (ret)
> > > > +             return ret;
> > > > +
> > > >       /* Add the fake offset */
> > > >       vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> > > >
> > >
> > > --
> > > Thomas Zimmermann
> > > Graphics Driver Developer
> > > SUSE Software Solutions Germany GmbH
> > > Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > (HRB 36809, AG Nürnberg)
> > > Geschäftsführer: Ivo Totev
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 4678 bytes --]

Hi

Am 30.05.22 um 17:41 schrieb Rob Clark:
> On Mon, May 30, 2022 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>>
>> On Mon, 30 May 2022 at 15:54, Rob Clark <robdclark@gmail.com> wrote:
>>>
>>> On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>>
>>>> Hi
>>>>
>>>> Am 29.05.22 um 18:29 schrieb Rob Clark:
>>>>> From: Rob Clark <robdclark@chromium.org>
>>>>>
>>>>> If a GEM object is allocated, and then exported as a dma-buf fd which is
>>>>> mmap'd before or without the GEM buffer being directly mmap'd, the
>>>>> vma_node could be unitialized.  This leads to a situation where the CPU
>>>>> mapping is not correctly torn down in drm_vma_node_unmap().
>>>>
>>>> Which drivers are affected by this problem?
>>>>
>>>> I checked several drivers and most appear to be initializing the offset
>>>> during object construction, such as GEM SHMEM. [1] TTM-based drivers
>>>> also seem unaffected. [2]
>>>>
>>>>   From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
>>>> They only seem to run drm_gem_create_mmap_offset() from their
>>>> ioctl-handling code.
>>>>
>>>> If so, I'd say it's preferable to fix these drivers and put a
>>>> drm_WARN_ONCE() into drm_gem_prime_mmap().
>>>
>>> That is good if fewer drivers are affected, however I disagree with
>>> your proposal.  At least for freedreno userspace, a lot of bo's never
>>> get mmap'd (either directly of via dmabuf), so we should not be
>>> allocating a mmap offset unnecessarily.
>>
>> Does this actually matter in the grand scheme of things? We originally
>> allocated mmap offset only on demand because userspace only had 32bit
>> loff_t support and so simply couldn't mmap anything if the offset
>> ended up above 32bit (even if there was still va space available).
>>
>> But those days are long gone (about 10 years or so) and the allocation
>> overhead for an mmap offset is tiny. So I think unless you can
>> benchmark an impact allocating it at bo alloc seems like the simplest
>> design overall, and hence what we should be doing. And if the vma
>> offset allocation every gets too slow due to fragmentation we can lift
>> the hole tree from i915 into drm_mm and the job should be done. At
>> that point we could also allocate the offset unconditionally in the
>> gem_init function and be done with it.
>>
>> Iow I concur with Thomas here, unless there's hard data contrary
>> simplicity imo trumps here.
> 
> 32b userspace is still alive and well, at least on arm chromebooks ;-)

I mostly dislike the inconsistency among drivers. If we want to create 
the offset on-demand in the DRM helpers, we should do so for all 
drivers. At least our generic GEM helpers and TTM should implement this 
pattern.

Best regards
Thomas

> 
> BR,
> -R
> 
>> -Daniel
>>
>>>
>>> BR,
>>> -R
>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>> [1]
>>>> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
>>>> [2]
>>>> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
>>>>
>>>>>
>>>>> Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
>>>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>>>>> ---
>>>>> Note, it's possible the issue existed in some related form prior to the
>>>>> commit tagged with Fixes.
>>>>>
>>>>>    drivers/gpu/drm/drm_prime.c | 5 +++++
>>>>>    1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>>>>> index e3f09f18110c..849eea154dfc 100644
>>>>> --- a/drivers/gpu/drm/drm_prime.c
>>>>> +++ b/drivers/gpu/drm/drm_prime.c
>>>>> @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>>>>>        struct file *fil;
>>>>>        int ret;
>>>>>
>>>>> +     /* Ensure that the vma_node is initialized: */
>>>>> +     ret = drm_gem_create_mmap_offset(obj);
>>>>> +     if (ret)
>>>>> +             return ret;
>>>>> +
>>>>>        /* Add the fake offset */
>>>>>        vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
>>>>>
>>>>
>>>> --
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>> (HRB 36809, AG Nürnberg)
>>>> Geschäftsführer: Ivo Totev
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Rob Clark]

On Mon, May 30, 2022 at 10:20 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 30.05.22 um 17:41 schrieb Rob Clark:
> > On Mon, May 30, 2022 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >>
> >> On Mon, 30 May 2022 at 15:54, Rob Clark <robdclark@gmail.com> wrote:
> >>>
> >>> On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>>>
> >>>> Hi
> >>>>
> >>>> Am 29.05.22 um 18:29 schrieb Rob Clark:
> >>>>> From: Rob Clark <robdclark@chromium.org>
> >>>>>
> >>>>> If a GEM object is allocated, and then exported as a dma-buf fd which is
> >>>>> mmap'd before or without the GEM buffer being directly mmap'd, the
> >>>>> vma_node could be unitialized.  This leads to a situation where the CPU
> >>>>> mapping is not correctly torn down in drm_vma_node_unmap().
> >>>>
> >>>> Which drivers are affected by this problem?
> >>>>
> >>>> I checked several drivers and most appear to be initializing the offset
> >>>> during object construction, such as GEM SHMEM. [1] TTM-based drivers
> >>>> also seem unaffected. [2]
> >>>>
> >>>>   From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
> >>>> They only seem to run drm_gem_create_mmap_offset() from their
> >>>> ioctl-handling code.
> >>>>
> >>>> If so, I'd say it's preferable to fix these drivers and put a
> >>>> drm_WARN_ONCE() into drm_gem_prime_mmap().
> >>>
> >>> That is good if fewer drivers are affected, however I disagree with
> >>> your proposal.  At least for freedreno userspace, a lot of bo's never
> >>> get mmap'd (either directly of via dmabuf), so we should not be
> >>> allocating a mmap offset unnecessarily.
> >>
> >> Does this actually matter in the grand scheme of things? We originally
> >> allocated mmap offset only on demand because userspace only had 32bit
> >> loff_t support and so simply couldn't mmap anything if the offset
> >> ended up above 32bit (even if there was still va space available).
> >>
> >> But those days are long gone (about 10 years or so) and the allocation
> >> overhead for an mmap offset is tiny. So I think unless you can
> >> benchmark an impact allocating it at bo alloc seems like the simplest
> >> design overall, and hence what we should be doing. And if the vma
> >> offset allocation every gets too slow due to fragmentation we can lift
> >> the hole tree from i915 into drm_mm and the job should be done. At
> >> that point we could also allocate the offset unconditionally in the
> >> gem_init function and be done with it.
> >>
> >> Iow I concur with Thomas here, unless there's hard data contrary
> >> simplicity imo trumps here.
> >
> > 32b userspace is still alive and well, at least on arm chromebooks ;-)
>
> I mostly dislike the inconsistency among drivers. If we want to create
> the offset on-demand in the DRM helpers, we should do so for all
> drivers. At least our generic GEM helpers and TTM should implement this
> pattern.

Possibly we should have drm_gem_get_mmap_offset() which combines
drm_gem_create_mmap_offset() and drm_vma_node_start() calls, and use
that everywhere.

But I think we should fix this issue first, and then refactor on top,
so that a fix can be backported to stable kernels ;-)

BR,
-R

> Best regards
> Thomas
>
> >
> > BR,
> > -R
> >
> >> -Daniel
> >>
> >>>
> >>> BR,
> >>> -R
> >>>
> >>>> Best regards
> >>>> Thomas
> >>>>
> >>>> [1]
> >>>> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
> >>>> [2]
> >>>> https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
> >>>>
> >>>>>
> >>>>> Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> >>>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
> >>>>> ---
> >>>>> Note, it's possible the issue existed in some related form prior to the
> >>>>> commit tagged with Fixes.
> >>>>>
> >>>>>    drivers/gpu/drm/drm_prime.c | 5 +++++
> >>>>>    1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> >>>>> index e3f09f18110c..849eea154dfc 100644
> >>>>> --- a/drivers/gpu/drm/drm_prime.c
> >>>>> +++ b/drivers/gpu/drm/drm_prime.c
> >>>>> @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> >>>>>        struct file *fil;
> >>>>>        int ret;
> >>>>>
> >>>>> +     /* Ensure that the vma_node is initialized: */
> >>>>> +     ret = drm_gem_create_mmap_offset(obj);
> >>>>> +     if (ret)
> >>>>> +             return ret;
> >>>>> +
> >>>>>        /* Add the fake offset */
> >>>>>        vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> >>>>>
> >>>>
> >>>> --
> >>>> Thomas Zimmermann
> >>>> Graphics Driver Developer
> >>>> SUSE Software Solutions Germany GmbH
> >>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >>>> (HRB 36809, AG Nürnberg)
> >>>> Geschäftsführer: Ivo Totev
> >>
> >>
> >>
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> http://blog.ffwll.ch
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Daniel Vetter]

On Mon, 30 May 2022 at 17:41, Rob Clark <robdclark@gmail.com> wrote:
>
> On Mon, May 30, 2022 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, 30 May 2022 at 15:54, Rob Clark <robdclark@gmail.com> wrote:
> > >
> > > On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > > >
> > > > Hi
> > > >
> > > > Am 29.05.22 um 18:29 schrieb Rob Clark:
> > > > > From: Rob Clark <robdclark@chromium.org>
> > > > >
> > > > > If a GEM object is allocated, and then exported as a dma-buf fd which is
> > > > > mmap'd before or without the GEM buffer being directly mmap'd, the
> > > > > vma_node could be unitialized.  This leads to a situation where the CPU
> > > > > mapping is not correctly torn down in drm_vma_node_unmap().
> > > >
> > > > Which drivers are affected by this problem?
> > > >
> > > > I checked several drivers and most appear to be initializing the offset
> > > > during object construction, such as GEM SHMEM. [1] TTM-based drivers
> > > > also seem unaffected. [2]
> > > >
> > > >  From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
> > > > They only seem to run drm_gem_create_mmap_offset() from their
> > > > ioctl-handling code.
> > > >
> > > > If so, I'd say it's preferable to fix these drivers and put a
> > > > drm_WARN_ONCE() into drm_gem_prime_mmap().
> > >
> > > That is good if fewer drivers are affected, however I disagree with
> > > your proposal.  At least for freedreno userspace, a lot of bo's never
> > > get mmap'd (either directly of via dmabuf), so we should not be
> > > allocating a mmap offset unnecessarily.
> >
> > Does this actually matter in the grand scheme of things? We originally
> > allocated mmap offset only on demand because userspace only had 32bit
> > loff_t support and so simply couldn't mmap anything if the offset
> > ended up above 32bit (even if there was still va space available).
> >
> > But those days are long gone (about 10 years or so) and the allocation
> > overhead for an mmap offset is tiny. So I think unless you can
> > benchmark an impact allocating it at bo alloc seems like the simplest
> > design overall, and hence what we should be doing. And if the vma
> > offset allocation every gets too slow due to fragmentation we can lift
> > the hole tree from i915 into drm_mm and the job should be done. At
> > that point we could also allocate the offset unconditionally in the
> > gem_init function and be done with it.
> >
> > Iow I concur with Thomas here, unless there's hard data contrary
> > simplicity imo trumps here.
>
> 32b userspace is still alive and well, at least on arm chromebooks ;-)

There's lots of different 32b userspace. The old thing was about
userspace which didn't use mmap64, but only mmap. Which could only
mmap the lower 4GB of a file, and so if you ended up with mmap_offset
above 4G then you'd blow up.

But mmap64 is a thing since forever, and if you compile with the right
glibc switch (loff_t is the magic thing iirc) it all works even with
default mmap. So I really don't think you should have this problem
anymore (except when cros is doing something really, really silly).
-Daniel

>
> BR,
> -R
>
> > -Daniel
> >
> > >
> > > BR,
> > > -R
> > >
> > > > Best regards
> > > > Thomas
> > > >
> > > > [1]
> > > > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
> > > > [2]
> > > > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
> > > >
> > > > >
> > > > > Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> > > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > > ---
> > > > > Note, it's possible the issue existed in some related form prior to the
> > > > > commit tagged with Fixes.
> > > > >
> > > > >   drivers/gpu/drm/drm_prime.c | 5 +++++
> > > > >   1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > > > index e3f09f18110c..849eea154dfc 100644
> > > > > --- a/drivers/gpu/drm/drm_prime.c
> > > > > +++ b/drivers/gpu/drm/drm_prime.c
> > > > > @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > > > >       struct file *fil;
> > > > >       int ret;
> > > > >
> > > > > +     /* Ensure that the vma_node is initialized: */
> > > > > +     ret = drm_gem_create_mmap_offset(obj);
> > > > > +     if (ret)
> > > > > +             return ret;
> > > > > +
> > > > >       /* Add the fake offset */
> > > > >       vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> > > > >
> > > >
> > > > --
> > > > Thomas Zimmermann
> > > > Graphics Driver Developer
> > > > SUSE Software Solutions Germany GmbH
> > > > Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > > (HRB 36809, AG Nürnberg)
> > > > Geschäftsführer: Ivo Totev
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/prime: Ensure mmap offset is initialized

[Rob Clark]

On Tue, May 31, 2022 at 5:32 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, 30 May 2022 at 17:41, Rob Clark <robdclark@gmail.com> wrote:
> >
> > On Mon, May 30, 2022 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > > On Mon, 30 May 2022 at 15:54, Rob Clark <robdclark@gmail.com> wrote:
> > > >
> > > > On Mon, May 30, 2022 at 12:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > > > >
> > > > > Hi
> > > > >
> > > > > Am 29.05.22 um 18:29 schrieb Rob Clark:
> > > > > > From: Rob Clark <robdclark@chromium.org>
> > > > > >
> > > > > > If a GEM object is allocated, and then exported as a dma-buf fd which is
> > > > > > mmap'd before or without the GEM buffer being directly mmap'd, the
> > > > > > vma_node could be unitialized.  This leads to a situation where the CPU
> > > > > > mapping is not correctly torn down in drm_vma_node_unmap().
> > > > >
> > > > > Which drivers are affected by this problem?
> > > > >
> > > > > I checked several drivers and most appear to be initializing the offset
> > > > > during object construction, such as GEM SHMEM. [1] TTM-based drivers
> > > > > also seem unaffected. [2]
> > > > >
> > > > >  From a quick grep, only etnaviv, msm and omapdrm appear to be affected?
> > > > > They only seem to run drm_gem_create_mmap_offset() from their
> > > > > ioctl-handling code.
> > > > >
> > > > > If so, I'd say it's preferable to fix these drivers and put a
> > > > > drm_WARN_ONCE() into drm_gem_prime_mmap().
> > > >
> > > > That is good if fewer drivers are affected, however I disagree with
> > > > your proposal.  At least for freedreno userspace, a lot of bo's never
> > > > get mmap'd (either directly of via dmabuf), so we should not be
> > > > allocating a mmap offset unnecessarily.
> > >
> > > Does this actually matter in the grand scheme of things? We originally
> > > allocated mmap offset only on demand because userspace only had 32bit
> > > loff_t support and so simply couldn't mmap anything if the offset
> > > ended up above 32bit (even if there was still va space available).
> > >
> > > But those days are long gone (about 10 years or so) and the allocation
> > > overhead for an mmap offset is tiny. So I think unless you can
> > > benchmark an impact allocating it at bo alloc seems like the simplest
> > > design overall, and hence what we should be doing. And if the vma
> > > offset allocation every gets too slow due to fragmentation we can lift
> > > the hole tree from i915 into drm_mm and the job should be done. At
> > > that point we could also allocate the offset unconditionally in the
> > > gem_init function and be done with it.
> > >
> > > Iow I concur with Thomas here, unless there's hard data contrary
> > > simplicity imo trumps here.
> >
> > 32b userspace is still alive and well, at least on arm chromebooks ;-)
>
> There's lots of different 32b userspace. The old thing was about
> userspace which didn't use mmap64, but only mmap. Which could only
> mmap the lower 4GB of a file, and so if you ended up with mmap_offset
> above 4G then you'd blow up.
>
> But mmap64 is a thing since forever, and if you compile with the right
> glibc switch (loff_t is the magic thing iirc) it all works even with
> default mmap. So I really don't think you should have this problem
> anymore (except when cros is doing something really, really silly).

The other thing, not sure quite how much it matters, is the
vma_offset_manager size is smaller with 32b kernels, which is still a
thing on some devices.

But at any rate, not allocating a mmap offset when it isn't needed
still seems like an eminently reasonable thing to do.  And IMO my fix
is quite reasonable too.  But if you disagree I can workaround the
core helpers in the driver.

BR,
-R

> -Daniel
>
> >
> > BR,
> > -R
> >
> > > -Daniel
> > >
> > > >
> > > > BR,
> > > > -R
> > > >
> > > > > Best regards
> > > > > Thomas
> > > > >
> > > > > [1]
> > > > > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L85
> > > > > [2]
> > > > > https://elixir.bootlin.com/linux/v5.18/source/drivers/gpu/drm/ttm/ttm_bo.c#L1002
> > > > >
> > > > > >
> > > > > > Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> > > > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > > > ---
> > > > > > Note, it's possible the issue existed in some related form prior to the
> > > > > > commit tagged with Fixes.
> > > > > >
> > > > > >   drivers/gpu/drm/drm_prime.c | 5 +++++
> > > > > >   1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > > > > index e3f09f18110c..849eea154dfc 100644
> > > > > > --- a/drivers/gpu/drm/drm_prime.c
> > > > > > +++ b/drivers/gpu/drm/drm_prime.c
> > > > > > @@ -716,6 +716,11 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > > > > >       struct file *fil;
> > > > > >       int ret;
> > > > > >
> > > > > > +     /* Ensure that the vma_node is initialized: */
> > > > > > +     ret = drm_gem_create_mmap_offset(obj);
> > > > > > +     if (ret)
> > > > > > +             return ret;
> > > > > > +
> > > > > >       /* Add the fake offset */
> > > > > >       vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> > > > > >
> > > > >
> > > > > --
> > > > > Thomas Zimmermann
> > > > > Graphics Driver Developer
> > > > > SUSE Software Solutions Germany GmbH
> > > > > Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > > > (HRB 36809, AG Nürnberg)
> > > > > Geschäftsführer: Ivo Totev
> > >
> > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch