Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
legacy contexts. (v3)") has caused a build failure for me when I
actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
,----
| Kernel: arch/x86/boot/bzImage is ready (#1)
| Building modules, stage 2.
| MODPOST 290 modules
| ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
| scripts/Makefile.modpost:91: recipe for target '__modpost' failed
`----
Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
apply in 5.1.9.
Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
them yet.
Cheers,
Sven
On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
> Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
> legacy contexts. (v3)") has caused a build failure for me when I
> actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
>
> ,----
> | Kernel: arch/x86/boot/bzImage is ready (#1)
> | Building modules, stage 2.
> | MODPOST 290 modules
> | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
> | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> `----
>
> Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
> Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
> drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
> apply in 5.1.9.
>
> Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
> them yet.
They probably are.
Should I just revert this patch in the stable tree, or add some other
patch (like the one pointed out here, which seems an odd patch for
stable...)
thanks,
greg k-h
On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
<[email protected]> wrote:
> On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
> > Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
> > legacy contexts. (v3)") has caused a build failure for me when I
> > actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
> >
> > ,----
> > | Kernel: arch/x86/boot/bzImage is ready (#1)
> > | Building modules, stage 2.
> > | MODPOST 290 modules
> > | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
> > | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> > `----
Calling drm_legacy_mmap is definitely not a great idea. I think either
we need a custom patch to remove that out on older kernels, or maybe
even #ifdef if you want to be super paranoid about breaking stuff ...
> > Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
> > Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
> > drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
> > apply in 5.1.9.
> >
> > Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
> > them yet.
>
> They probably are.
>
> Should I just revert this patch in the stable tree, or add some other
> patch (like the one pointed out here, which seems an odd patch for
> stable...)
... or backport the above patch, that should be save to do too. Not
sure what stable folks prefer?
-Daniel
>
> thanks,
>
> greg k-h
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Jun 11, 2019 at 07:33:16PM +0200, Daniel Vetter wrote:
> On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
> <[email protected]> wrote:
> > On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
> > > Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
> > > legacy contexts. (v3)") has caused a build failure for me when I
> > > actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
> > >
> > > ,----
> > > | Kernel: arch/x86/boot/bzImage is ready (#1)
> > > | Building modules, stage 2.
> > > | MODPOST 290 modules
> > > | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
> > > | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> > > `----
>
> Calling drm_legacy_mmap is definitely not a great idea. I think either
> we need a custom patch to remove that out on older kernels, or maybe
> even #ifdef if you want to be super paranoid about breaking stuff ...
>
> > > Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
> > > Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
> > > drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
> > > apply in 5.1.9.
> > >
> > > Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
> > > them yet.
> >
> > They probably are.
> >
> > Should I just revert this patch in the stable tree, or add some other
> > patch (like the one pointed out here, which seems an odd patch for
> > stable...)
>
> ... or backport the above patch, that should be save to do too. Not
> sure what stable folks prefer?
The above patch does not apply to all of the stable branches, so how
about I just revert this? People can live with this option not able to
turn off for now, and if they really want it, they can use a newer
kernel, right?
thanks,
greg k-h
On 2019-06-11 19:33 +0200, Daniel Vetter wrote:
> On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
> <[email protected]> wrote:
>> On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
>> > Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
>> > legacy contexts. (v3)") has caused a build failure for me when I
>> > actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
>> >
>> > ,----
>> > | Kernel: arch/x86/boot/bzImage is ready (#1)
>> > | Building modules, stage 2.
>> > | MODPOST 290 modules
>> > | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
>> > | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>> > `----
>
> Calling drm_legacy_mmap is definitely not a great idea.
Certainly not, but it was done by Dave in commit 2036eaa7403 ("nouveau:
bring back legacy mmap handler") for compatibility with old
xf86-video-nouveau versions (older than 1.0.4) that call DRIOpenDRMMaster.
If that is really necessary, it probably has been broken in Linus' tree
by commit bed2dd8421 where the test has been moved to ttm_bo_mmap() and
returns -EINVAL on failure.
> I think either
> we need a custom patch to remove that out on older kernels, or maybe
> even #ifdef if you want to be super paranoid about breaking stuff ...
>
>> > Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
>> > Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
>> > drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
>> > apply in 5.1.9.
>> >
>> > Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
>> > them yet.
>>
>> They probably are.
>>
>> Should I just revert this patch in the stable tree, or add some other
>> patch (like the one pointed out here, which seems an odd patch for
>> stable...)
>
> ... or backport the above patch, that should be save to do too. Not
> sure what stable folks prefer?
> -Daniel
Cheers,
Sven
Den 11-06-2019 kl. 20:40, skrev Greg Kroah-Hartman:
> On Tue, Jun 11, 2019 at 07:33:16PM +0200, Daniel Vetter wrote:
>> On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
>> <[email protected]> wrote:
>>> On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
>>>> Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
>>>> legacy contexts. (v3)") has caused a build failure for me when I
>>>> actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
>>>>
>>>> ,----
>>>> | Kernel: arch/x86/boot/bzImage is ready (#1)
>>>> | Building modules, stage 2.
>>>> | MODPOST 290 modules
>>>> | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
>>>> | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>>>> `----
>>
>> Calling drm_legacy_mmap is definitely not a great idea. I think either
>> we need a custom patch to remove that out on older kernels, or maybe
>> even #ifdef if you want to be super paranoid about breaking stuff ...
>>
>>>> Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
>>>> Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
>>>> drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
>>>> apply in 5.1.9.
>>>>
>>>> Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
>>>> them yet.
>>>
>>> They probably are.
>>>
>>> Should I just revert this patch in the stable tree, or add some other
>>> patch (like the one pointed out here, which seems an odd patch for
>>> stable...)
>>
>> ... or backport the above patch, that should be save to do too. Not
>> sure what stable folks prefer?
>
> The above patch does not apply to all of the stable branches, so how
> about I just revert this? People can live with this option not able to
> turn off for now, and if they really want it, they can use a newer
> kernel, right?
>
Or add the simple fix suggested by Daniel (if I understand correctly):
From: Thomas Backlund <[email protected]>
Setting CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n (added by commit:
b30a43ac7132) causes the build to fail with:
ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
Fix that by adding check for CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT around
the code using drm_legacy_mmap()
Fixes: b30a43ac7132 drm/nouveau: add kconfig option to turn off nouveau
legacy contexts. (v3)
Signed-off-by: Thomas Backlund <[email protected]>
---
drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 ++
1 file changed, 2 insertions(+)
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -168,8 +168,10 @@ nouveau_ttm_mmap(struct file *filp, stru
struct drm_file *file_priv = filp->private_data;
struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
+#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
return drm_legacy_mmap(filp, vma);
+#endif
return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
}
On Tue, Jun 11, 2019 at 7:40 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Tue, Jun 11, 2019 at 07:33:16PM +0200, Daniel Vetter wrote:
> > On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
> > <[email protected]> wrote:
> > > On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
> > > > Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
> > > > legacy contexts. (v3)") has caused a build failure for me when I
> > > > actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
> > > >
> > > > ,----
> > > > | Kernel: arch/x86/boot/bzImage is ready (#1)
> > > > | Building modules, stage 2.
> > > > | MODPOST 290 modules
> > > > | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
> > > > | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> > > > `----
> >
> > Calling drm_legacy_mmap is definitely not a great idea. I think either
> > we need a custom patch to remove that out on older kernels, or maybe
> > even #ifdef if you want to be super paranoid about breaking stuff ...
> >
> > > > Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
> > > > Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
> > > > drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
> > > > apply in 5.1.9.
> > > >
> > > > Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
> > > > them yet.
> > >
> > > They probably are.
> > >
> > > Should I just revert this patch in the stable tree, or add some other
> > > patch (like the one pointed out here, which seems an odd patch for
> > > stable...)
> >
> > ... or backport the above patch, that should be save to do too. Not
> > sure what stable folks prefer?
>
> The above patch does not apply to all of the stable branches, so how
> about I just revert this? People can live with this option not able to
> turn off for now, and if they really want it, they can use a newer
> kernel, right?
Lots of people can live with root holes in their kernels, it's still
not a great idea :-) Plan B would be to fix all the legacy ioctls and
close all the exploits in there, which absolutely no one wants to
spend time on. This way the only people who'll suffer are the ones
with horribly outdated userspace (and at that point who cares about a
few more exploits).
We should maybe update the help text to tell people they really
shouldn't enable this, the default y is really just to avoid
regression reports :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Jun 11, 2019 at 8:53 PM Sven Joachim <[email protected]> wrote:
>
> On 2019-06-11 19:33 +0200, Daniel Vetter wrote:
>
> > On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
> > <[email protected]> wrote:
> >> On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
> >> > Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
> >> > legacy contexts. (v3)") has caused a build failure for me when I
> >> > actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
> >> >
> >> > ,----
> >> > | Kernel: arch/x86/boot/bzImage is ready (#1)
> >> > | Building modules, stage 2.
> >> > | MODPOST 290 modules
> >> > | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
> >> > | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> >> > `----
> >
> > Calling drm_legacy_mmap is definitely not a great idea.
>
> Certainly not, but it was done by Dave in commit 2036eaa7403 ("nouveau:
> bring back legacy mmap handler") for compatibility with old
> xf86-video-nouveau versions (older than 1.0.4) that call DRIOpenDRMMaster.
>
> If that is really necessary, it probably has been broken in Linus' tree
> by commit bed2dd8421 where the test has been moved to ttm_bo_mmap() and
> returns -EINVAL on failure.
Looking at the commit it's actually 1.0.1, which was release in 2012.
I'd say lets keep current upstream as-is, and hope no one cares
anymore ...
-Daniel
> > I think either
> > we need a custom patch to remove that out on older kernels, or maybe
> > even #ifdef if you want to be super paranoid about breaking stuff ...
> >
> >> > Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
> >> > Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
> >> > drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
> >> > apply in 5.1.9.
> >> >
> >> > Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
> >> > them yet.
> >>
> >> They probably are.
> >>
> >> Should I just revert this patch in the stable tree, or add some other
> >> patch (like the one pointed out here, which seems an odd patch for
> >> stable...)
> >
> > ... or backport the above patch, that should be save to do too. Not
> > sure what stable folks prefer?
> > -Daniel
>
> Cheers,
> Sven
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Jun 11, 2019 at 10:08:10PM +0300, Thomas Backlund wrote:
> Den 11-06-2019 kl. 20:40, skrev Greg Kroah-Hartman:
> > On Tue, Jun 11, 2019 at 07:33:16PM +0200, Daniel Vetter wrote:
> > > On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
> > > <[email protected]> wrote:
> > > > On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
> > > > > Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
> > > > > legacy contexts. (v3)") has caused a build failure for me when I
> > > > > actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
> > > > >
> > > > > ,----
> > > > > | Kernel: arch/x86/boot/bzImage is ready (#1)
> > > > > | Building modules, stage 2.
> > > > > | MODPOST 290 modules
> > > > > | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
> > > > > | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> > > > > `----
> > >
> > > Calling drm_legacy_mmap is definitely not a great idea. I think either
> > > we need a custom patch to remove that out on older kernels, or maybe
> > > even #ifdef if you want to be super paranoid about breaking stuff ...
> > >
> > > > > Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
> > > > > Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
> > > > > drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
> > > > > apply in 5.1.9.
> > > > >
> > > > > Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
> > > > > them yet.
> > > >
> > > > They probably are.
> > > >
> > > > Should I just revert this patch in the stable tree, or add some other
> > > > patch (like the one pointed out here, which seems an odd patch for
> > > > stable...)
> > >
> > > ... or backport the above patch, that should be save to do too. Not
> > > sure what stable folks prefer?
> >
> > The above patch does not apply to all of the stable branches, so how
> > about I just revert this? People can live with this option not able to
> > turn off for now, and if they really want it, they can use a newer
> > kernel, right?
> >
>
> Or add the simple fix suggested by Daniel (if I understand correctly):
>
>
> From: Thomas Backlund <[email protected]>
>
> Setting CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n (added by commit: b30a43ac7132)
> causes the build to fail with:
>
> ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
>
> Fix that by adding check for CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT around
> the code using drm_legacy_mmap()
>
> Fixes: b30a43ac7132 drm/nouveau: add kconfig option to turn off nouveau
> legacy contexts. (v3)
> Signed-off-by: Thomas Backlund <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Not-entirely-upstream-sha1-but-equivalent: bed2dd8421 ("drm/ttm:
Quick-test mmap offset in ttm_bo_mmap()")
Cheers, Daniel
>
> ---
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> @@ -168,8 +168,10 @@ nouveau_ttm_mmap(struct file *filp, stru
> struct drm_file *file_priv = filp->private_data;
> struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
>
> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
> return drm_legacy_mmap(filp, vma);
> +#endif
>
> return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
> }
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On 2019-06-11 22:08 +0300, Thomas Backlund wrote:
> Den 11-06-2019 kl. 20:40, skrev Greg Kroah-Hartman:
>> On Tue, Jun 11, 2019 at 07:33:16PM +0200, Daniel Vetter wrote:
>>> On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
>>> <[email protected]> wrote:
>>>> On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
>>>>> Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
>>>>> legacy contexts. (v3)") has caused a build failure for me when I
>>>>> actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
>>>>>
>>>>> ,----
>>>>> | Kernel: arch/x86/boot/bzImage is ready (#1)
>>>>> | Building modules, stage 2.
>>>>> | MODPOST 290 modules
>>>>> | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
>>>>> | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>>>>> `----
>>>
>>> Calling drm_legacy_mmap is definitely not a great idea. I think either
>>> we need a custom patch to remove that out on older kernels, or maybe
>>> even #ifdef if you want to be super paranoid about breaking stuff ...
>>>
>>>>> Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
>>>>> Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
>>>>> drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
>>>>> apply in 5.1.9.
>>>>>
>>>>> Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
>>>>> them yet.
>>>>
>>>> They probably are.
>>>>
>>>> Should I just revert this patch in the stable tree, or add some other
>>>> patch (like the one pointed out here, which seems an odd patch for
>>>> stable...)
>>>
>>> ... or backport the above patch, that should be save to do too. Not
>>> sure what stable folks prefer?
>>
>> The above patch does not apply to all of the stable branches, so how
>> about I just revert this? People can live with this option not able to
>> turn off for now, and if they really want it, they can use a newer
>> kernel, right?
>>
>
> Or add the simple fix suggested by Daniel (if I understand correctly):
>
>
> From: Thomas Backlund <[email protected]>
>
> Setting CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n (added by commit:
> b30a43ac7132) causes the build to fail with:
>
> ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
>
> Fix that by adding check for CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT around
> the code using drm_legacy_mmap()
>
> Fixes: b30a43ac7132 drm/nouveau: add kconfig option to turn off
> nouveau legacy contexts. (v3)
> Signed-off-by: Thomas Backlund <[email protected]>
>
> ---
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> @@ -168,8 +168,10 @@ nouveau_ttm_mmap(struct file *filp, stru
> struct drm_file *file_priv = filp->private_data;
> struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
>
> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
> return drm_legacy_mmap(filp, vma);
> +#endif
>
> return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
> }
That's not quite correct, I am afraid. If
CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT is not defined, you still need to do
the test, but return -EINVAL. Something along these lines:
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 1543c2f8d3d3..05d513d54555 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -169,7 +169,11 @@ nouveau_ttm_mmap(struct file *filp, struct vm_area_struct *vma)
struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
+#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
return drm_legacy_mmap(filp, vma);
+#else
+ return -EINVAL;
+#endif
return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
}
At least that builds for me, need to reboot to check whether it works.
Cheers,
Sven
Den 11-06-2019 kl. 22:43, skrev Sven Joachim:
> On 2019-06-11 22:08 +0300, Thomas Backlund wrote:
>
>> Den 11-06-2019 kl. 20:40, skrev Greg Kroah-Hartman:
>>> On Tue, Jun 11, 2019 at 07:33:16PM +0200, Daniel Vetter wrote:
>>>> On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
>>>> <[email protected]> wrote:
>>>>> On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
>>>>>> Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
>>>>>> legacy contexts. (v3)") has caused a build failure for me when I
>>>>>> actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
>>>>>>
>>>>>> ,----
>>>>>> | Kernel: arch/x86/boot/bzImage is ready (#1)
>>>>>> | Building modules, stage 2.
>>>>>> | MODPOST 290 modules
>>>>>> | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
>>>>>> | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>>>>>> `----
>>>> Calling drm_legacy_mmap is definitely not a great idea. I think either
>>>> we need a custom patch to remove that out on older kernels, or maybe
>>>> even #ifdef if you want to be super paranoid about breaking stuff ...
>>>>
>>>>>> Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
>>>>>> Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
>>>>>> drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
>>>>>> apply in 5.1.9.
>>>>>>
>>>>>> Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
>>>>>> them yet.
>>>>> They probably are.
>>>>>
>>>>> Should I just revert this patch in the stable tree, or add some other
>>>>> patch (like the one pointed out here, which seems an odd patch for
>>>>> stable...)
>>>> ... or backport the above patch, that should be save to do too. Not
>>>> sure what stable folks prefer?
>>> The above patch does not apply to all of the stable branches, so how
>>> about I just revert this? People can live with this option not able to
>>> turn off for now, and if they really want it, they can use a newer
>>> kernel, right?
>>>
>> Or add the simple fix suggested by Daniel (if I understand correctly):
>>
>>
>> From: Thomas Backlund <[email protected]>
>>
>> Setting CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n (added by commit:
>> b30a43ac7132) causes the build to fail with:
>>
>> ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
>>
>> Fix that by adding check for CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT around
>> the code using drm_legacy_mmap()
>>
>> Fixes: b30a43ac7132 drm/nouveau: add kconfig option to turn off
>> nouveau legacy contexts. (v3)
>> Signed-off-by: Thomas Backlund <[email protected]>
>>
>> ---
>> drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
>> @@ -168,8 +168,10 @@ nouveau_ttm_mmap(struct file *filp, stru
>> struct drm_file *file_priv = filp->private_data;
>> struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
>>
>> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
>> if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
>> return drm_legacy_mmap(filp, vma);
>> +#endif
>>
>> return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
>> }
> That's not quite correct, I am afraid. If
> CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT is not defined, you still need to do
> the test, but return -EINVAL. Something along these lines:
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> index 1543c2f8d3d3..05d513d54555 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> @@ -169,7 +169,11 @@ nouveau_ttm_mmap(struct file *filp, struct vm_area_struct *vma)
> struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
>
> if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> return drm_legacy_mmap(filp, vma);
> +#else
> + return -EINVAL;
> +#endif
>
> return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
> }
>
>
> At least that builds for me, need to reboot to check whether it works.
>
> Cheers,
> Sven
Ah, indeed. thats what basically all other drivers did before bed2dd8421
("drm/ttm:
Quick-test mmap offset in ttm_bo_mmap()"), and in that commit the same
check
was moved to drivers/gpu/drm/ttm/ttm_bo_vm.c
--
Thomas
On Tue, Jun 11, 2019 at 07:40:06PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 11, 2019 at 07:33:16PM +0200, Daniel Vetter wrote:
> > On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman
> > <[email protected]> wrote:
> > > On Tue, Jun 11, 2019 at 03:56:35PM +0200, Sven Joachim wrote:
> > > > Commit 1e07d63749 ("drm/nouveau: add kconfig option to turn off nouveau
> > > > legacy contexts. (v3)") has caused a build failure for me when I
> > > > actually tried that option (CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n):
> > > >
> > > > ,----
> > > > | Kernel: arch/x86/boot/bzImage is ready (#1)
> > > > | Building modules, stage 2.
> > > > | MODPOST 290 modules
> > > > | ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
> > > > | scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> > > > `----
> >
> > Calling drm_legacy_mmap is definitely not a great idea. I think either
> > we need a custom patch to remove that out on older kernels, or maybe
> > even #ifdef if you want to be super paranoid about breaking stuff ...
> >
> > > > Upstream does not have that problem, as commit bed2dd8421 ("drm/ttm:
> > > > Quick-test mmap offset in ttm_bo_mmap()") has removed the use of
> > > > drm_legacy_mmap from nouveau_ttm.c. Unfortunately that commit does not
> > > > apply in 5.1.9.
> > > >
> > > > Most likely 4.19.50 and 4.14.125 are also affected, I haven't tested
> > > > them yet.
> > >
> > > They probably are.
> > >
> > > Should I just revert this patch in the stable tree, or add some other
> > > patch (like the one pointed out here, which seems an odd patch for
> > > stable...)
> >
> > ... or backport the above patch, that should be save to do too. Not
> > sure what stable folks prefer?
>
> The above patch does not apply to all of the stable branches, so how
> about I just revert this? People can live with this option not able to
> turn off for now, and if they really want it, they can use a newer
> kernel, right?
I've just reverted it now.
If someone can send me a patch series of all of what needs to be
applied, in a format that I can actually apply them in, I will be glad
to do so. But for now, I'd like to get people's systems building again.
thanks,
greg k-h
Den 13-06-2019 kl. 10:42, skrev Greg Kroah-Hartman:
> I've just reverted it now.
>
> If someone can send me a patch series of all of what needs to be
> applied, in a format that I can actually apply them in, I will be glad
> to do so. But for now, I'd like to get people's systems building again.
>
That would be basically re-adding the b30a43ac7132 commit and adding the
following patch (also attached in case the inlined version gets mangled):
From 0d91b155a7f9c1f4a2b360bc2b79dc728aee8b48 Mon Sep 17 00:00:00 2001
From: Thomas Backlund <[email protected]>
Date: Sat, 15 Jun 2019 12:22:44 +0300
Subject: [PATCH] nouveau: Fix build with
CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT disabled
Not-entirely-upstream-sha1-but-equivalent: bed2dd8421
("drm/ttm: Quick-test mmap offset in ttm_bo_mmap()")
Setting CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n (added by commit: b30a43ac7132)
causes the build to fail with:
ERROR: "drm_legacy_mmap" [drivers/gpu/drm/nouveau/nouveau.ko] undefined!
This does not happend upstream as the offending code got removed in:
bed2dd8421 ("drm/ttm: Quick-test mmap offset in ttm_bo_mmap()")
Fix that by adding check for CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT around
the drm_legacy_mmap() call.
Also, as Sven Joachim pointed out, we need to make the check in
CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT=n case return -EINVAL as its done
for basically all other gpu drivers, especially in upstream kernels
drivers/gpu/drm/ttm/ttm_bo_vm.c as of the upstream commit bed2dd8421.
NOTE. This is a minimal stable-only fix for trees where b30a43ac7132 is
backported as the build error affects nouveau only.
Fixes: b30a43ac7132 ("drm/nouveau: add kconfig option to turn off nouveau
legacy contexts. (v3)")
Signed-off-by: Thomas Backlund <[email protected]>
Cc: [email protected]
Cc: Daniel Vetter <[email protected]>
Cc: Sven Joachim <[email protected]>
---
drivers/gpu/drm/nouveau/nouveau_ttm.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 1543c2f8d3d3..05d513d54555 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -169,7 +169,11 @@ nouveau_ttm_mmap(struct file *filp, struct
vm_area_struct *vma)
struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
+#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
return drm_legacy_mmap(filp, vma);
+#else
+ return -EINVAL;
+#endif
return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
}
--
2.21.0
On Sat, Jun 15, 2019 at 01:21:59PM +0300, Thomas Backlund wrote:
> Den 13-06-2019 kl. 10:42, skrev Greg Kroah-Hartman:
>
> > I've just reverted it now.
> >
> > If someone can send me a patch series of all of what needs to be
> > applied, in a format that I can actually apply them in, I will be glad
> > to do so. But for now, I'd like to get people's systems building again.
> >
>
>
> That would be basically re-adding the b30a43ac7132 commit and adding the
> following patch (also attached in case the inlined version gets mangled):
Thanks for this, I've queued it up now, let's try this all again :)
greg k-h