[SRU][Kinetic][PATCH 1/1] UBUNTU: SAUCE: Revert "video/aperture: Disable and unregister sysfb devices via aperture helpers"
Kai-Heng Feng
kai.heng.feng at canonical.com
Mon Apr 24 12:15:47 UTC 2023
Hi Matthew,
On Mon, Apr 24, 2023 at 12:38 PM Matthew Ruffell
<matthew.ruffell at canonical.com> wrote:
>
> Hi Kai-Heng,
>
> The whole series would be at minimum:
>
> commit 729d6872097ffc53216430e33bc61e91c421f52b
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:12 2022 +0200
> Subject: fbdev: Remove trailing whitespaces
> Link: https://github.com/torvalds/linux/commit/729d6872097ffc53216430e33bc61e91c421f52b
>
> commit 0db5b61e0dc07cee121e5a04932ca7f5d0b62f6a
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:13 2022 +0200
> Subject: fbdev/vga16fb: Create EGA/VGA devices in sysfb code
> Link: https://github.com/torvalds/linux/commit/0db5b61e0dc07cee121e5a04932ca7f5d0b62f6a
>
> commit 8a611e08257afb9ab5814d5c19239bf40e5030f4
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:14 2022 +0200
> Subject: fbdev/vga16fb: Auto-generate module init/exit code
> Link: https://github.com/torvalds/linux/commit/8a611e08257afb9ab5814d5c19239bf40e5030f4
>
> commit 9d69ef1838150c7d87afc1a87aa658c637217585
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:15 2022 +0200
> Subject: fbdev/core: Remove remove_conflicting_pci_framebuffers()
> Link: https://github.com/torvalds/linux/commit/9d69ef1838150c7d87afc1a87aa658c637217585
>
> commit 8d69d008f44cb96050c35e64fe940a22dd6b0113
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:16 2022 +0200
> Subject: fbdev: Convert drivers to aperture helpers
> Link: https://github.com/torvalds/linux/commit/8d69d008f44cb96050c35e64fe940a22dd6b0113
>
> commit 145eed48de278007f646b908fd70ac59d24ed81a
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:17 2022 +0200
> Subject: fbdev: Remove conflicting devices on PCI bus
> Link: https://github.com/torvalds/linux/commit/145eed48de278007f646b908fd70ac59d24ed81a
>
> commit 5e01376124309b4dbd30d413f43c0d9c2f60edea
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:18 2022 +0200
> Subject: video/aperture: Disable and unregister sysfb devices via
> aperture helpers
> Link: https://github.com/torvalds/linux/commit/5e01376124309b4dbd30d413f43c0d9c2f60edea
>
> commit 4652905f4e30ab7b4827faa38343fdafa90f2956
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:19 2022 +0200
> Subject: video: Provide constants for VGA I/O range
> Link: https://github.com/torvalds/linux/commit/4652905f4e30ab7b4827faa38343fdafa90f2956
>
> commit 482b1c7d478875508ac112c36dcd65739f664b0e
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:20 2022 +0200
> Subject: video/aperture: Remove conflicting VGA devices, if any
> Link: https://github.com/torvalds/linux/commit/482b1c7d478875508ac112c36dcd65739f664b0e
>
> commit 72a6a3e03bdc957996a74c1053eab1b2c073db8e
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:21 2022 +0200
> Subject: fbdev: Acquire framebuffer apertures for firmware devices
> Link: https://github.com/torvalds/linux/commit/72a6a3e03bdc957996a74c1053eab1b2c073db8e
>
> commit 15fced5b051e6e22c228a521a5894b12c2ba0892
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Mon Jul 18 09:23:22 2022 +0200
> Subject: fbdev: Remove conflict-handling code
> Link: https://github.com/torvalds/linux/commit/15fced5b051e6e22c228a521a5894b12c2ba0892
>
> And then the fix-ups:
>
> commit e0ba1a39b8dfe4f005bebdd85daa89e7382e26b7
> Author: Michał Mirosław <mirq-linux at rere.qmqm.pl>
> Date: Thu Oct 27 02:06:16 2022 +0200
> Subject: fbdev/core: Avoid uninitialized read in
> aperture_remove_conflicting_pci_device()
> Link: https://github.com/torvalds/linux/commit/e0ba1a39b8dfe4f005bebdd85daa89e7382e26b7
>
> commit ca5f13a21404f5496bcc006d9416c8bef21b227d
> Author: Thomas Zimmermann <tzimmermann at suse.de>
> Date: Thu Jul 21 10:16:55 2022 +0200
> Subject: fbdev: Fix order of arguments to aperture_remove_conflicting_devices()
> Link: https://github.com/torvalds/linux/commit/ca5f13a21404f5496bcc006d9416c8bef21b227d
>
> commit 04119ab1a49fc41cb70f0472be5455af268fa260
> Author: Dave Airlie <airlied at redhat.com>
> Date: Mon Feb 6 07:05:28 2023 +1000
> Subject: nvidiafb: detect the hardware support before removing console.
> Link: https://github.com/torvalds/linux/commit/04119ab1a49fc41cb70f0472be5455af268fa260
>
> commit 77bc762451c2dc72bdbea07b857c916c9e7f4952
> Author: Dan Carpenter <error27 at gmail.com>
> Date: Mon Feb 27 13:33:30 2023 +0300
> Subject: fbdev: chipsfb: Fix error codes in chipsfb_pci_init()
> Link: https://github.com/torvalds/linux/commit/77bc762451c2dc72bdbea07b857c916c9e7f4952
>
> Please read all of these commits, and then weigh up the decision to simply
> revert "video/aperture: Disable and unregister sysfb devices via
> aperture helpers"
> versus backporting and applying all of these to the 5.19 stable kernel.
Isn't use-after-free considered exploit?
Also, the following fixups are rather minor.
>
> I think the risk of regression is just too high for this ever to be an option
> versus reverting one commit and going back to what the 5.19 kernel was on
> release. The above patches don't even fix anything, they simply refactor the
> fbdev subsystem.
The firmware fbdev and PCI fbdev don't play nice together for quite some time.
I haven't tried the series yet, but gracefully handover resources from
firmware fbdev to DRM fbdev indeed fixes things.
>
> Please reconsider. I think reverting "video/aperture: Disable and unregister
> sysfb devices via aperture helpers" is the best option for the kinetic 5.19
> kernel.
I am not objecting. Please proceed if this is the better approach.
It's just that last time I made a simple change on efifb and it
inadvertently exposed issues in other DRM drivers.
So this fbdev series should actually make things easier for us.
Kai-Heng
>
> Thanks,
> Matthew
>
> On Fri, 21 Apr 2023 at 12:45, Kai-Heng Feng <kai.heng.feng at canonical.com> wrote:
> >
> > On Thu, Apr 20, 2023 at 8:22 AM Matthew Ruffell
> > <matthew.ruffell at canonical.com> wrote:
> > >
> > > BugLink: https://bugs.launchpad.net/bugs/2007001
> > >
> > > This reverts commit 89314ff239e1933357419fa91b20190150f114a8 (ubuntu-kinetic).
> > >
> > > Numerous VMWare users have reported that vmwgfx cannot reserve the memory
> > > region for the graphics framebuffer, leading their VMs to have blank screens.
> > >
> > > They see the following in dmesg:
> > >
> > > [ 11.135360] vmwgfx 0000:00:0f.0: BAR 2: can't reserve [mem 0x70000000-0x77ffffff 64bit pref]
> > > [ 11.135366] vmwgfx: probe of 0000:00:0f.0 failed with error -16
> > >
> > > And a cat /proc/iomem shows this:
> > >
> > > 50000000-7fffffff : PCI Bus 0000:00
> > > 70000000-77ffffff : 0000:00:0f.0
> > > 70000000-702fffff : BOOTFB
> > >
> > > The kernel has failed to release this memory region for vmwgfx to occupy.
> > >
> > > "video/aperture: Disable and unregister sysfb devices via aperture helpers" is
> > > a part of a much larger series that refactors device ownership, and this commit
> > > requires the whole series to be applied to work correctly. The whole series is
> > > large, has numerous fixups, and is not suitable for a stable kernel in the first
> > > place, as it does not fix any specific issue.
> > >
> > > Hence, reverting is the best way to fix this.
> >
> > The commit in question fixes "drm/aperture: Run fbdev removal before
> > internal helpers", which is also reported and tested by VMware.
> > Maybe backporting the whole series is more reasonable?
> >
> > Kai-Heng
> >
> >
> >
> > >
> > > Signed-off-by: Matthew Ruffell <matthew.ruffell at canonical.com>
> > > ---
> > > drivers/gpu/drm/drm_aperture.c | 14 --------------
> > > drivers/video/fbdev/core/fbmem.c | 12 ++++++++++++
> > > 2 files changed, 12 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
> > > index 528e2544661c..059fd71424f6 100644
> > > --- a/drivers/gpu/drm/drm_aperture.c
> > > +++ b/drivers/gpu/drm/drm_aperture.c
> > > @@ -7,7 +7,6 @@
> > > #include <linux/pci.h>
> > > #include <linux/platform_device.h> /* for firmware helpers */
> > > #include <linux/slab.h>
> > > -#include <linux/sysfb.h>
> > > #include <linux/types.h>
> > > #include <linux/vgaarb.h>
> > >
> > > @@ -293,20 +292,7 @@ int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_
> > > #if IS_REACHABLE(CONFIG_FB)
> > > struct apertures_struct *a;
> > > int ret;
> > > -#endif
> > > -
> > > - /*
> > > - * If a driver asked to unregister a platform device registered by
> > > - * sysfb, then can be assumed that this is a driver for a display
> > > - * that is set up by the system firmware and has a generic driver.
> > > - *
> > > - * Drivers for devices that don't have a generic driver will never
> > > - * ask for this, so let's assume that a real driver for the display
> > > - * was already probed and prevent sysfb to register devices later.
> > > - */
> > > - sysfb_disable();
> > >
> > > -#if IS_REACHABLE(CONFIG_FB)
> > > a = alloc_apertures(1);
> > > if (!a)
> > > return -ENOMEM;
> > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > > index 959fa1948f29..bc5a9154783d 100644
> > > --- a/drivers/video/fbdev/core/fbmem.c
> > > +++ b/drivers/video/fbdev/core/fbmem.c
> > > @@ -19,6 +19,7 @@
> > > #include <linux/kernel.h>
> > > #include <linux/major.h>
> > > #include <linux/slab.h>
> > > +#include <linux/sysfb.h>
> > > #include <linux/mm.h>
> > > #include <linux/mman.h>
> > > #include <linux/vt.h>
> > > @@ -1768,6 +1769,17 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
> > > do_free = true;
> > > }
> > >
> > > + /*
> > > + * If a driver asked to unregister a platform device registered by
> > > + * sysfb, then can be assumed that this is a driver for a display
> > > + * that is set up by the system firmware and has a generic driver.
> > > + *
> > > + * Drivers for devices that don't have a generic driver will never
> > > + * ask for this, so let's assume that a real driver for the display
> > > + * was already probed and prevent sysfb to register devices later.
> > > + */
> > > + sysfb_disable();
> > > +
> > > mutex_lock(®istration_lock);
> > > do_remove_conflicting_framebuffers(a, name, primary);
> > > mutex_unlock(®istration_lock);
> > > --
> > > 2.39.2
> > >
> > >
> > > --
> > > kernel-team mailing list
> > > kernel-team at lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list