dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fbdev: Fix unregistering of framebuffers without device
@ 2022-04-04 19:44 Thomas Zimmermann
  2022-04-05  9:01 ` Daniel Vetter
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2022-04-04 19:44 UTC (permalink / raw)
  To: daniel, deller, sudipm.mukherjee, sam, javierm, zackr, hdegoede
  Cc: linux-fbdev, Matthew Wilcox, Xiyu Yang, Tetsuo Handa,
	Daniel Vetter, Zheyu Ma, stable, dri-devel, Thomas Zimmermann,
	Zhen Lei, Alex Deucher, Guenter Roeck

OF framebuffers do not have an underlying device in the Linux
device hierarchy. Do a regular unregister call instead of hot
unplugging such a non-existing device. Fixes a NULL dereference.
An example error message on ppc64le is shown below.

  BUG: Kernel NULL pointer dereference on read at 0x00000060
  Faulting instruction address: 0xc00000000080dfa4
  Oops: Kernel access of bad area, sig: 11 [#1]
  LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
  [...]
  CPU: 2 PID: 139 Comm: systemd-udevd Not tainted 5.17.0-ae085d7f9365 #1
  NIP:  c00000000080dfa4 LR: c00000000080df9c CTR: c000000000797430
  REGS: c000000004132fe0 TRAP: 0300   Not tainted  (5.17.0-ae085d7f9365)
  MSR:  8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 28228282  XER: 20000000
  CFAR: c00000000000c80c DAR: 0000000000000060 DSISR: 40000000 IRQMASK: 0
  GPR00: c00000000080df9c c000000004133280 c00000000169d200 0000000000000029
  GPR04: 00000000ffffefff c000000004132f90 c000000004132f88 0000000000000000
  GPR08: c0000000015658f8 c0000000015cd200 c0000000014f57d0 0000000048228283
  GPR12: 0000000000000000 c00000003fffe300 0000000020000000 0000000000000000
  GPR16: 0000000000000000 0000000113fc4a40 0000000000000005 0000000113fcfb80
  GPR20: 000001000f7283b0 0000000000000000 c000000000e4a588 c000000000e4a5b0
  GPR24: 0000000000000001 00000000000a0000 c008000000db0168 c0000000021f6ec0
  GPR28: c0000000016d65a8 c000000004b36460 0000000000000000 c0000000016d64b0
  NIP [c00000000080dfa4] do_remove_conflicting_framebuffers+0x184/0x1d0
  [c000000004133280] [c00000000080df9c] do_remove_conflicting_framebuffers+0x17c/0x1d0 (unreliable)
  [c000000004133350] [c00000000080e4d0] remove_conflicting_framebuffers+0x60/0x150
  [c0000000041333a0] [c00000000080e6f4] remove_conflicting_pci_framebuffers+0x134/0x1b0
  [c000000004133450] [c008000000e70438] drm_aperture_remove_conflicting_pci_framebuffers+0x90/0x100 [drm]
  [c000000004133490] [c008000000da0ce4] bochs_pci_probe+0x6c/0xa64 [bochs]
  [...]
  [c000000004133db0] [c00000000002aaa0] system_call_exception+0x170/0x2d0
  [c000000004133e10] [c00000000000c3cc] system_call_common+0xec/0x250

The bug [1] was introduced by commit 27599aacbaef ("fbdev: Hot-unplug
firmware fb devices on forced removal"). Most firmware framebuffers
have an underlying platform device, which can be hot-unplugged
before loading the native graphics driver. OF framebuffers do not
(yet) have that device. Fix the code by unregistering the framebuffer
as before without a hot unplug.

Tested with 5.17 on qemu ppc64le emulation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal")
Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Zack Rusin <zackr@vmware.com>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: stable@vger.kernel.org # v5.11+
Cc: Helge Deller <deller@gmx.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Zheyu Ma <zheyuma97@gmail.com>
Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Link: https://lore.kernel.org/all/YkHXO6LGHAN0p1pq@debian/ # [1]
---
 drivers/video/fbdev/core/fbmem.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 34d6bb1bf82e..a6bb0e438216 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1579,7 +1579,14 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
 			 * If it's not a platform device, at least print a warning. A
 			 * fix would add code to remove the device from the system.
 			 */
-			if (dev_is_platform(device)) {
+			if (!device) {
+				/* TODO: Represent each OF framebuffer as its own
+				 * device in the device hierarchy. For now, offb
+				 * doesn't have such a device, so unregister the
+				 * framebuffer as before without warning.
+				 */
+				do_unregister_framebuffer(registered_fb[i]);
+			} else if (dev_is_platform(device)) {
 				registered_fb[i]->forced_out = true;
 				platform_device_unregister(to_platform_device(device));
 			} else {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] fbdev: Fix unregistering of framebuffers without device
  2022-04-04 19:44 [PATCH] fbdev: Fix unregistering of framebuffers without device Thomas Zimmermann
@ 2022-04-05  9:01 ` Daniel Vetter
  2022-04-05 18:24   ` Thomas Zimmermann
  2022-04-05 13:05 ` Sudip Mukherjee
  2022-04-05 14:08 ` [PATCH] " Javier Martinez Canillas
  2 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2022-04-05  9:01 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: linux-fbdev, Matthew Wilcox, Xiyu Yang, Guenter Roeck,
	Tetsuo Handa, Daniel Vetter, deller, Zheyu Ma, javierm, dri-devel,
	hdegoede, stable, Zhen Lei, Alex Deucher, sam, sudipm.mukherjee

On Mon, Apr 04, 2022 at 09:44:02PM +0200, Thomas Zimmermann wrote:
> OF framebuffers do not have an underlying device in the Linux
> device hierarchy. Do a regular unregister call instead of hot
> unplugging such a non-existing device. Fixes a NULL dereference.
> An example error message on ppc64le is shown below.
> 
>   BUG: Kernel NULL pointer dereference on read at 0x00000060
>   Faulting instruction address: 0xc00000000080dfa4
>   Oops: Kernel access of bad area, sig: 11 [#1]
>   LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
>   [...]
>   CPU: 2 PID: 139 Comm: systemd-udevd Not tainted 5.17.0-ae085d7f9365 #1
>   NIP:  c00000000080dfa4 LR: c00000000080df9c CTR: c000000000797430
>   REGS: c000000004132fe0 TRAP: 0300   Not tainted  (5.17.0-ae085d7f9365)
>   MSR:  8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 28228282  XER: 20000000
>   CFAR: c00000000000c80c DAR: 0000000000000060 DSISR: 40000000 IRQMASK: 0
>   GPR00: c00000000080df9c c000000004133280 c00000000169d200 0000000000000029
>   GPR04: 00000000ffffefff c000000004132f90 c000000004132f88 0000000000000000
>   GPR08: c0000000015658f8 c0000000015cd200 c0000000014f57d0 0000000048228283
>   GPR12: 0000000000000000 c00000003fffe300 0000000020000000 0000000000000000
>   GPR16: 0000000000000000 0000000113fc4a40 0000000000000005 0000000113fcfb80
>   GPR20: 000001000f7283b0 0000000000000000 c000000000e4a588 c000000000e4a5b0
>   GPR24: 0000000000000001 00000000000a0000 c008000000db0168 c0000000021f6ec0
>   GPR28: c0000000016d65a8 c000000004b36460 0000000000000000 c0000000016d64b0
>   NIP [c00000000080dfa4] do_remove_conflicting_framebuffers+0x184/0x1d0
>   [c000000004133280] [c00000000080df9c] do_remove_conflicting_framebuffers+0x17c/0x1d0 (unreliable)
>   [c000000004133350] [c00000000080e4d0] remove_conflicting_framebuffers+0x60/0x150
>   [c0000000041333a0] [c00000000080e6f4] remove_conflicting_pci_framebuffers+0x134/0x1b0
>   [c000000004133450] [c008000000e70438] drm_aperture_remove_conflicting_pci_framebuffers+0x90/0x100 [drm]
>   [c000000004133490] [c008000000da0ce4] bochs_pci_probe+0x6c/0xa64 [bochs]
>   [...]
>   [c000000004133db0] [c00000000002aaa0] system_call_exception+0x170/0x2d0
>   [c000000004133e10] [c00000000000c3cc] system_call_common+0xec/0x250
> 
> The bug [1] was introduced by commit 27599aacbaef ("fbdev: Hot-unplug
> firmware fb devices on forced removal"). Most firmware framebuffers
> have an underlying platform device, which can be hot-unplugged
> before loading the native graphics driver. OF framebuffers do not
> (yet) have that device. Fix the code by unregistering the framebuffer
> as before without a hot unplug.
> 
> Tested with 5.17 on qemu ppc64le emulation.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal")
> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Zack Rusin <zackr@vmware.com>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org # v5.11+
> Cc: Helge Deller <deller@gmx.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Zheyu Ma <zheyuma97@gmail.com>
> Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Cc: Zhen Lei <thunder.leizhen@huawei.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-fbdev@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Link: https://lore.kernel.org/all/YkHXO6LGHAN0p1pq@debian/ # [1]
> ---
>  drivers/video/fbdev/core/fbmem.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 34d6bb1bf82e..a6bb0e438216 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1579,7 +1579,14 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
>  			 * If it's not a platform device, at least print a warning. A
>  			 * fix would add code to remove the device from the system.
>  			 */
> -			if (dev_is_platform(device)) {
> +			if (!device) {
> +				/* TODO: Represent each OF framebuffer as its own
> +				 * device in the device hierarchy. For now, offb
> +				 * doesn't have such a device, so unregister the
> +				 * framebuffer as before without warning.
> +				 */
> +				do_unregister_framebuffer(registered_fb[i]);

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Might be good to have a fb_info flag for offb and then check in
register_framebuffer that everyone else does have a device? Just to make
sure we don't have more surprises here ...
-Daniel


> +			} else if (dev_is_platform(device)) {
>  				registered_fb[i]->forced_out = true;
>  				platform_device_unregister(to_platform_device(device));
>  			} else {
> -- 
> 2.35.1
> 

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: fbdev: Fix unregistering of framebuffers without device
  2022-04-04 19:44 [PATCH] fbdev: Fix unregistering of framebuffers without device Thomas Zimmermann
  2022-04-05  9:01 ` Daniel Vetter
@ 2022-04-05 13:05 ` Sudip Mukherjee
  2022-04-05 14:08 ` [PATCH] " Javier Martinez Canillas
  2 siblings, 0 replies; 5+ messages in thread
From: Sudip Mukherjee @ 2022-04-05 13:05 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: linux-fbdev, dri-devel, Xiyu Yang, Guenter Roeck, Tetsuo Handa,
	Daniel Vetter, deller, Zheyu Ma, javierm, Matthew Wilcox,
	hdegoede, stable, Zhen Lei, Alex Deucher, sam

On Mon, Apr 04, 2022 at 09:44:02PM +0200, Thomas Zimmermann wrote:
> OF framebuffers do not have an underlying device in the Linux
> device hierarchy. Do a regular unregister call instead of hot
> unplugging such a non-existing device. Fixes a NULL dereference.
> An example error message on ppc64le is shown below.
> 

<snip>

> 
> Tested with 5.17 on qemu ppc64le emulation.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal")
> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Zack Rusin <zackr@vmware.com>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org # v5.11+
> Cc: Helge Deller <deller@gmx.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Zheyu Ma <zheyuma97@gmail.com>
> Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Cc: Zhen Lei <thunder.leizhen@huawei.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-fbdev@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Link: https://lore.kernel.org/all/YkHXO6LGHAN0p1pq@debian/ # [1]
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---

Tested-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>

--
Regards
Sudip

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fbdev: Fix unregistering of framebuffers without device
  2022-04-04 19:44 [PATCH] fbdev: Fix unregistering of framebuffers without device Thomas Zimmermann
  2022-04-05  9:01 ` Daniel Vetter
  2022-04-05 13:05 ` Sudip Mukherjee
@ 2022-04-05 14:08 ` Javier Martinez Canillas
  2 siblings, 0 replies; 5+ messages in thread
From: Javier Martinez Canillas @ 2022-04-05 14:08 UTC (permalink / raw)
  To: Thomas Zimmermann, daniel, deller, sudipm.mukherjee, sam, zackr,
	hdegoede
  Cc: linux-fbdev, Matthew Wilcox, Xiyu Yang, Tetsuo Handa,
	Daniel Vetter, Zheyu Ma, stable, dri-devel, Zhen Lei,
	Alex Deucher, Guenter Roeck

Hello Thomas,

On 4/4/22 21:44, Thomas Zimmermann wrote:
> OF framebuffers do not have an underlying device in the Linux
> device hierarchy. Do a regular unregister call instead of hot
> unplugging such a non-existing device. Fixes a NULL dereference.
> An example error message on ppc64le is shown below.
> 
>   BUG: Kernel NULL pointer dereference on read at 0x00000060
>   Faulting instruction address: 0xc00000000080dfa4
>   Oops: Kernel access of bad area, sig: 11 [#1]
>   LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
>   [...]
>   CPU: 2 PID: 139 Comm: systemd-udevd Not tainted 5.17.0-ae085d7f9365 #1
>   NIP:  c00000000080dfa4 LR: c00000000080df9c CTR: c000000000797430
>   REGS: c000000004132fe0 TRAP: 0300   Not tainted  (5.17.0-ae085d7f9365)
>   MSR:  8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 28228282  XER: 20000000
>   CFAR: c00000000000c80c DAR: 0000000000000060 DSISR: 40000000 IRQMASK: 0
>   GPR00: c00000000080df9c c000000004133280 c00000000169d200 0000000000000029
>   GPR04: 00000000ffffefff c000000004132f90 c000000004132f88 0000000000000000
>   GPR08: c0000000015658f8 c0000000015cd200 c0000000014f57d0 0000000048228283
>   GPR12: 0000000000000000 c00000003fffe300 0000000020000000 0000000000000000
>   GPR16: 0000000000000000 0000000113fc4a40 0000000000000005 0000000113fcfb80
>   GPR20: 000001000f7283b0 0000000000000000 c000000000e4a588 c000000000e4a5b0
>   GPR24: 0000000000000001 00000000000a0000 c008000000db0168 c0000000021f6ec0
>   GPR28: c0000000016d65a8 c000000004b36460 0000000000000000 c0000000016d64b0
>   NIP [c00000000080dfa4] do_remove_conflicting_framebuffers+0x184/0x1d0
>   [c000000004133280] [c00000000080df9c] do_remove_conflicting_framebuffers+0x17c/0x1d0 (unreliable)
>   [c000000004133350] [c00000000080e4d0] remove_conflicting_framebuffers+0x60/0x150
>   [c0000000041333a0] [c00000000080e6f4] remove_conflicting_pci_framebuffers+0x134/0x1b0
>   [c000000004133450] [c008000000e70438] drm_aperture_remove_conflicting_pci_framebuffers+0x90/0x100 [drm]
>   [c000000004133490] [c008000000da0ce4] bochs_pci_probe+0x6c/0xa64 [bochs]
>   [...]
>   [c000000004133db0] [c00000000002aaa0] system_call_exception+0x170/0x2d0
>   [c000000004133e10] [c00000000000c3cc] system_call_common+0xec/0x250
> 
> The bug [1] was introduced by commit 27599aacbaef ("fbdev: Hot-unplug
> firmware fb devices on forced removal"). Most firmware framebuffers
> have an underlying platform device, which can be hot-unplugged
> before loading the native graphics driver. OF framebuffers do not
> (yet) have that device. Fix the code by unregistering the framebuffer
> as before without a hot unplug.
>

I believe the assumption that all firmware fb would have an underlying
device was a reasonable one and it's a pity that offb doesn't...

But that is how things are and your patch is the least intrusive fix.
 
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fbdev: Fix unregistering of framebuffers without device
  2022-04-05  9:01 ` Daniel Vetter
@ 2022-04-05 18:24   ` Thomas Zimmermann
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2022-04-05 18:24 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: linux-fbdev, Matthew Wilcox, Xiyu Yang, Guenter Roeck,
	Tetsuo Handa, Daniel Vetter, deller, Zheyu Ma, javierm, dri-devel,
	hdegoede, stable, Zhen Lei, Alex Deucher, sam, sudipm.mukherjee


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

Hi

Am 05.04.22 um 11:01 schrieb Daniel Vetter:
> On Mon, Apr 04, 2022 at 09:44:02PM +0200, Thomas Zimmermann wrote:
>> OF framebuffers do not have an underlying device in the Linux
>> device hierarchy. Do a regular unregister call instead of hot
>> unplugging such a non-existing device. Fixes a NULL dereference.
>> An example error message on ppc64le is shown below.
>>
>>    BUG: Kernel NULL pointer dereference on read at 0x00000060
>>    Faulting instruction address: 0xc00000000080dfa4
>>    Oops: Kernel access of bad area, sig: 11 [#1]
>>    LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
>>    [...]
>>    CPU: 2 PID: 139 Comm: systemd-udevd Not tainted 5.17.0-ae085d7f9365 #1
>>    NIP:  c00000000080dfa4 LR: c00000000080df9c CTR: c000000000797430
>>    REGS: c000000004132fe0 TRAP: 0300   Not tainted  (5.17.0-ae085d7f9365)
>>    MSR:  8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 28228282  XER: 20000000
>>    CFAR: c00000000000c80c DAR: 0000000000000060 DSISR: 40000000 IRQMASK: 0
>>    GPR00: c00000000080df9c c000000004133280 c00000000169d200 0000000000000029
>>    GPR04: 00000000ffffefff c000000004132f90 c000000004132f88 0000000000000000
>>    GPR08: c0000000015658f8 c0000000015cd200 c0000000014f57d0 0000000048228283
>>    GPR12: 0000000000000000 c00000003fffe300 0000000020000000 0000000000000000
>>    GPR16: 0000000000000000 0000000113fc4a40 0000000000000005 0000000113fcfb80
>>    GPR20: 000001000f7283b0 0000000000000000 c000000000e4a588 c000000000e4a5b0
>>    GPR24: 0000000000000001 00000000000a0000 c008000000db0168 c0000000021f6ec0
>>    GPR28: c0000000016d65a8 c000000004b36460 0000000000000000 c0000000016d64b0
>>    NIP [c00000000080dfa4] do_remove_conflicting_framebuffers+0x184/0x1d0
>>    [c000000004133280] [c00000000080df9c] do_remove_conflicting_framebuffers+0x17c/0x1d0 (unreliable)
>>    [c000000004133350] [c00000000080e4d0] remove_conflicting_framebuffers+0x60/0x150
>>    [c0000000041333a0] [c00000000080e6f4] remove_conflicting_pci_framebuffers+0x134/0x1b0
>>    [c000000004133450] [c008000000e70438] drm_aperture_remove_conflicting_pci_framebuffers+0x90/0x100 [drm]
>>    [c000000004133490] [c008000000da0ce4] bochs_pci_probe+0x6c/0xa64 [bochs]
>>    [...]
>>    [c000000004133db0] [c00000000002aaa0] system_call_exception+0x170/0x2d0
>>    [c000000004133e10] [c00000000000c3cc] system_call_common+0xec/0x250
>>
>> The bug [1] was introduced by commit 27599aacbaef ("fbdev: Hot-unplug
>> firmware fb devices on forced removal"). Most firmware framebuffers
>> have an underlying platform device, which can be hot-unplugged
>> before loading the native graphics driver. OF framebuffers do not
>> (yet) have that device. Fix the code by unregistering the framebuffer
>> as before without a hot unplug.
>>
>> Tested with 5.17 on qemu ppc64le emulation.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal")
>> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
>> Cc: Zack Rusin <zackr@vmware.com>
>> Cc: Javier Martinez Canillas <javierm@redhat.com>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: stable@vger.kernel.org # v5.11+
>> Cc: Helge Deller <deller@gmx.de>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Sam Ravnborg <sam@ravnborg.org>
>> Cc: Zheyu Ma <zheyuma97@gmail.com>
>> Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn>
>> Cc: Zhen Lei <thunder.leizhen@huawei.com>
>> Cc: Matthew Wilcox <willy@infradead.org>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Link: https://lore.kernel.org/all/YkHXO6LGHAN0p1pq@debian/ # [1]
>> ---
>>   drivers/video/fbdev/core/fbmem.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>> index 34d6bb1bf82e..a6bb0e438216 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1579,7 +1579,14 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
>>   			 * If it's not a platform device, at least print a warning. A
>>   			 * fix would add code to remove the device from the system.
>>   			 */
>> -			if (dev_is_platform(device)) {
>> +			if (!device) {
>> +				/* TODO: Represent each OF framebuffer as its own
>> +				 * device in the device hierarchy. For now, offb
>> +				 * doesn't have such a device, so unregister the
>> +				 * framebuffer as before without warning.
>> +				 */
>> +				do_unregister_framebuffer(registered_fb[i]);
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Might be good to have a fb_info flag for offb and then check in
> register_framebuffer that everyone else does have a device? Just to make
> sure we don't have more surprises here ...

That exists already: all generic/firmware drivers are flagged with 
FBINFO_MISC_FIRMWARE specifically for the purpose of unloading. 
HW-native fbdev drivers don't have the flags and will never be unloaded. 
I double-checked and of the generic drivers, (efifb, vesafb, offb, 
simplefb, vga16) only offb had no device set.

Best regards
Thomas

> -Daniel
> 
> 
>> +			} else if (dev_is_platform(device)) {
>>   				registered_fb[i]->forced_out = true;
>>   				platform_device_unregister(to_platform_device(device));
>>   			} else {
>> -- 
>> 2.35.1
>>
> 

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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-04-05 18:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-04 19:44 [PATCH] fbdev: Fix unregistering of framebuffers without device Thomas Zimmermann
2022-04-05  9:01 ` Daniel Vetter
2022-04-05 18:24   ` Thomas Zimmermann
2022-04-05 13:05 ` Sudip Mukherjee
2022-04-05 14:08 ` [PATCH] " Javier Martinez Canillas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox