[SRU,Kinetic,1/1] UBUNTU: SAUCE: Revert "video/aperture: Disable and unregister sysfb devices via aperture helpers"

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.

Signed-off-by: Matthew Ruffell <matthew.ruffell@canonical.com>
---
 drivers/gpu/drm/drm_aperture.c   | 14 --------------
 drivers/video/fbdev/core/fbmem.c | 12 ++++++++++++
 2 files changed, 12 insertions(+), 14 deletions(-)

On Thu, Apr 20, 2023 at 8:22 AM Matthew Ruffell
<matthew.ruffell@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@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(&registration_lock);
>         do_remove_conflicting_framebuffers(a, name, primary);
>         mutex_unlock(&registration_lock);
> --
> 2.39.2
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Hi Kai-Heng,

The whole series would be at minimum:

commit 729d6872097ffc53216430e33bc61e91c421f52b
Author: Thomas Zimmermann <tzimmermann@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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.

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.

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.

Thanks,
Matthew

On Fri, 21 Apr 2023 at 12:45, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
>
> On Thu, Apr 20, 2023 at 8:22 AM Matthew Ruffell
> <matthew.ruffell@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@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(&registration_lock);
> >         do_remove_conflicting_framebuffers(a, name, primary);
> >         mutex_unlock(&registration_lock);
> > --
> > 2.39.2
> >
> >
> > --
> > kernel-team mailing list
> > kernel-team@lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team

Hi Matthew,

On Mon, Apr 24, 2023 at 12:38 PM Matthew Ruffell
<matthew.ruffell@canonical.com> wrote:
>
> Hi Kai-Heng,
>
> The whole series would be at minimum:
>
> commit 729d6872097ffc53216430e33bc61e91c421f52b
> Author: Thomas Zimmermann <tzimmermann@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@canonical.com> wrote:
> >
> > On Thu, Apr 20, 2023 at 8:22 AM Matthew Ruffell
> > <matthew.ruffell@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@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(&registration_lock);
> > >         do_remove_conflicting_framebuffers(a, name, primary);
> > >         mutex_unlock(&registration_lock);
> > > --
> > > 2.39.2
> > >
> > >
> > > --
> > > kernel-team mailing list
> > > kernel-team@lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team