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 gregkh@linuxfoundation.org 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
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 gregkh@linuxfoundation.org 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
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 gregkh@linuxfoundation.org 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 tmb@mageia.org
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 tmb@mageia.org
--- 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 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 gregkh@linuxfoundation.org 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 tmb@mageia.org
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 tmb@mageia.org
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
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); }
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 gregkh@linuxfoundation.org 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 tmb@mageia.org
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 tmb@mageia.org
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 gregkh@linuxfoundation.org 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 tmb@mageia.org
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 tmb@mageia.org
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 7:40 PM Greg Kroah-Hartman gregkh@linuxfoundation.org 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 gregkh@linuxfoundation.org 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
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 gregkh@linuxfoundation.org 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 tmb@mageia.org 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 tmb@mageia.org Cc: stable@vger.kernel.org Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Sven Joachim svenjoac@gmx.de --- 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); }
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
On 2019-06-11 19:33 +0200, Daniel Vetter wrote:
On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman gregkh@linuxfoundation.org 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
On Tue, Jun 11, 2019 at 8:53 PM Sven Joachim svenjoac@gmx.de wrote:
On 2019-06-11 19:33 +0200, Daniel Vetter wrote:
On Tue, Jun 11, 2019 at 5:37 PM Greg Kroah-Hartman gregkh@linuxfoundation.org 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
linux-stable-mirror@lists.linaro.org
-
Daniel Vetter -
Daniel Vetter -
Greg Kroah-Hartman -
Sven Joachim -
Thomas Backlund