* [PATCH 0/5] sysfb: Fix memory-region management @ 2022-01-25 9:12 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, Thomas Zimmermann, dri-devel Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug operation when removing fbdev firmware drivers. After being unloaded by a hardware driver, simplefb leaves behind the firmware framebuffer's platform device. This prevents other drivers from acquiring the memory as reported at [1]. Patch 1 changes the removal code of remove_conflicting_framebuffers() to remove the underlying device and the rsp memory region. Patches 2 to 4 update sysfb and its drivers. The sysfb code does no longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the device drivers acquire the memory when they probe the device. Patch 5 adds a todo item to acquire memory regions in all DRM drivers. Tested with simpledrm and simplefb. [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ v2: * fix possible NULL deref in simpledrm (Jocelyn) * various style fixes (Javier) Javier Martinez Canillas (1): drivers/firmware: Don't mark as busy the simple-framebuffer IO resource Thomas Zimmermann (4): fbdev: Hot-unplug firmware fb devices on forced removal drm/simpledrm: Request memory region in driver fbdev/simplefb: Request memory region in driver drm: Add TODO item for requesting memory regions Documentation/gpu/todo.rst | 15 +++++++ drivers/firmware/sysfb_simplefb.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 22 ++++++++--- drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++-- drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++---------- include/linux/fb.h | 1 + 6 files changed, 105 insertions(+), 29 deletions(-) base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 prerequisite-patch-id: 8e52143a6cd7b8fb789e656208f6edde71d0f499 -- 2.34.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 0/5] sysfb: Fix memory-region management @ 2022-01-25 9:12 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug operation when removing fbdev firmware drivers. After being unloaded by a hardware driver, simplefb leaves behind the firmware framebuffer's platform device. This prevents other drivers from acquiring the memory as reported at [1]. Patch 1 changes the removal code of remove_conflicting_framebuffers() to remove the underlying device and the rsp memory region. Patches 2 to 4 update sysfb and its drivers. The sysfb code does no longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the device drivers acquire the memory when they probe the device. Patch 5 adds a todo item to acquire memory regions in all DRM drivers. Tested with simpledrm and simplefb. [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ v2: * fix possible NULL deref in simpledrm (Jocelyn) * various style fixes (Javier) Javier Martinez Canillas (1): drivers/firmware: Don't mark as busy the simple-framebuffer IO resource Thomas Zimmermann (4): fbdev: Hot-unplug firmware fb devices on forced removal drm/simpledrm: Request memory region in driver fbdev/simplefb: Request memory region in driver drm: Add TODO item for requesting memory regions Documentation/gpu/todo.rst | 15 +++++++ drivers/firmware/sysfb_simplefb.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 22 ++++++++--- drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++-- drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++---------- include/linux/fb.h | 1 + 6 files changed, 105 insertions(+), 29 deletions(-) base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 prerequisite-patch-id: 8e52143a6cd7b8fb789e656208f6edde71d0f499 -- 2.34.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 9:12 ` Thomas Zimmermann -1 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: stable, linux-fbdev, Thomas Zimmermann, dri-devel Hot-unplug all firmware-framebuffer devices as part of removing them via remove_conflicting_framebuffers() et al. Releases all memory regions to be acquired by native drivers. Firmware, such as EFI, install a framebuffer while posting the computer. After removing the firmware-framebuffer device from fbdev, a native driver takes over the hardware and the firmware framebuffer becomes invalid. Firmware-framebuffer drivers, specifically simplefb, don't release their device from Linux' device hierarchy. It still owns the firmware framebuffer and blocks the native drivers from loading. This has been observed in the vmwgfx driver. [1] Initiating a device removal (i.e., hot unplug) as part of remove_conflicting_framebuffers() removes the underlying device and returns the memory range to the system. [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ v2: * rename variable 'dev' to 'device' (Javier) Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reported-by: Zack Rusin <zackr@vmware.com> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Zack Rusin <zackr@vmware.com> CC: stable@vger.kernel.org # v5.11+ --- drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- include/linux/fb.h | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..b585339509b0 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -25,6 +25,7 @@ #include <linux/init.h> #include <linux/linux_logo.h> #include <linux/proc_fs.h> +#include <linux/platform_device.h> #include <linux/seq_file.h> #include <linux/console.h> #include <linux/kmod.h> @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, /* check all firmware fbs and kick off if the base addr overlaps */ for_each_registered_fb(i) { struct apertures_struct *gen_aper; + struct device *device; if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) continue; gen_aper = registered_fb[i]->apertures; + device = registered_fb[i]->device; if (fb_do_apertures_overlap(gen_aper, a) || (primary && gen_aper && gen_aper->count && gen_aper->ranges[0].base == VGA_FB_PHYS)) { printk(KERN_INFO "fb%d: switching to %s from %s\n", i, name, registered_fb[i]->fix.id); - do_unregister_framebuffer(registered_fb[i]); + + /* + * If we kick-out a firmware driver, we also want to remove + * the underlying platform device, such as simple-framebuffer, + * VESA, EFI, etc. A native driver will then be able to + * allocate the memory range. + * + * 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)) { + registered_fb[i]->forced_out = true; + platform_device_unregister(to_platform_device(device)); + } else { + pr_warn("fb%d: cannot remove device\n", i); + do_unregister_framebuffer(registered_fb[i]); + } } } } @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); void unregister_framebuffer(struct fb_info *fb_info) { - mutex_lock(®istration_lock); + bool forced_out = fb_info->forced_out; + + if (!forced_out) + mutex_lock(®istration_lock); do_unregister_framebuffer(fb_info); - mutex_unlock(®istration_lock); + if (!forced_out) + mutex_unlock(®istration_lock); } EXPORT_SYMBOL(unregister_framebuffer); diff --git a/include/linux/fb.h b/include/linux/fb.h index 3da95842b207..9a14f3f8a329 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -502,6 +502,7 @@ struct fb_info { } *apertures; bool skip_vt_switch; /* no VT switch on suspend/resume required */ + bool forced_out; /* set when being removed by another driver */ }; static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal @ 2022-01-25 9:12 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann, stable Hot-unplug all firmware-framebuffer devices as part of removing them via remove_conflicting_framebuffers() et al. Releases all memory regions to be acquired by native drivers. Firmware, such as EFI, install a framebuffer while posting the computer. After removing the firmware-framebuffer device from fbdev, a native driver takes over the hardware and the firmware framebuffer becomes invalid. Firmware-framebuffer drivers, specifically simplefb, don't release their device from Linux' device hierarchy. It still owns the firmware framebuffer and blocks the native drivers from loading. This has been observed in the vmwgfx driver. [1] Initiating a device removal (i.e., hot unplug) as part of remove_conflicting_framebuffers() removes the underlying device and returns the memory range to the system. [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ v2: * rename variable 'dev' to 'device' (Javier) Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reported-by: Zack Rusin <zackr@vmware.com> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Zack Rusin <zackr@vmware.com> CC: stable@vger.kernel.org # v5.11+ --- drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- include/linux/fb.h | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..b585339509b0 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -25,6 +25,7 @@ #include <linux/init.h> #include <linux/linux_logo.h> #include <linux/proc_fs.h> +#include <linux/platform_device.h> #include <linux/seq_file.h> #include <linux/console.h> #include <linux/kmod.h> @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, /* check all firmware fbs and kick off if the base addr overlaps */ for_each_registered_fb(i) { struct apertures_struct *gen_aper; + struct device *device; if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) continue; gen_aper = registered_fb[i]->apertures; + device = registered_fb[i]->device; if (fb_do_apertures_overlap(gen_aper, a) || (primary && gen_aper && gen_aper->count && gen_aper->ranges[0].base == VGA_FB_PHYS)) { printk(KERN_INFO "fb%d: switching to %s from %s\n", i, name, registered_fb[i]->fix.id); - do_unregister_framebuffer(registered_fb[i]); + + /* + * If we kick-out a firmware driver, we also want to remove + * the underlying platform device, such as simple-framebuffer, + * VESA, EFI, etc. A native driver will then be able to + * allocate the memory range. + * + * 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)) { + registered_fb[i]->forced_out = true; + platform_device_unregister(to_platform_device(device)); + } else { + pr_warn("fb%d: cannot remove device\n", i); + do_unregister_framebuffer(registered_fb[i]); + } } } } @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); void unregister_framebuffer(struct fb_info *fb_info) { - mutex_lock(®istration_lock); + bool forced_out = fb_info->forced_out; + + if (!forced_out) + mutex_lock(®istration_lock); do_unregister_framebuffer(fb_info); - mutex_unlock(®istration_lock); + if (!forced_out) + mutex_unlock(®istration_lock); } EXPORT_SYMBOL(unregister_framebuffer); diff --git a/include/linux/fb.h b/include/linux/fb.h index 3da95842b207..9a14f3f8a329 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -502,6 +502,7 @@ struct fb_info { } *apertures; bool skip_vt_switch; /* no VT switch on suspend/resume required */ + bool forced_out; /* set when being removed by another driver */ }; static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/5] drivers/firmware: Don't mark as busy the simple-framebuffer IO resource 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 9:12 ` Thomas Zimmermann -1 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, Thomas Zimmermann, dri-devel From: Javier Martinez Canillas <javierm@redhat.com> The sysfb_create_simplefb() function requests a IO memory resource for the simple-framebuffer platform device, but it also marks it as busy which can lead to drivers requesting the same memory resource to fail. Let's drop the IORESOURCE_BUSY flag and let drivers to request it as busy instead. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Zack Rusin <zackr@vmware.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/firmware/sysfb_simplefb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c index 303a491e520d..76c4abc42a30 100644 --- a/drivers/firmware/sysfb_simplefb.c +++ b/drivers/firmware/sysfb_simplefb.c @@ -99,7 +99,7 @@ __init int sysfb_create_simplefb(const struct screen_info *si, /* setup IORESOURCE_MEM as framebuffer memory */ memset(&res, 0, sizeof(res)); - res.flags = IORESOURCE_MEM | IORESOURCE_BUSY; + res.flags = IORESOURCE_MEM; res.name = simplefb_resname; res.start = base; res.end = res.start + length - 1; -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/5] drivers/firmware: Don't mark as busy the simple-framebuffer IO resource @ 2022-01-25 9:12 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann From: Javier Martinez Canillas <javierm@redhat.com> The sysfb_create_simplefb() function requests a IO memory resource for the simple-framebuffer platform device, but it also marks it as busy which can lead to drivers requesting the same memory resource to fail. Let's drop the IORESOURCE_BUSY flag and let drivers to request it as busy instead. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Zack Rusin <zackr@vmware.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/firmware/sysfb_simplefb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c index 303a491e520d..76c4abc42a30 100644 --- a/drivers/firmware/sysfb_simplefb.c +++ b/drivers/firmware/sysfb_simplefb.c @@ -99,7 +99,7 @@ __init int sysfb_create_simplefb(const struct screen_info *si, /* setup IORESOURCE_MEM as framebuffer memory */ memset(&res, 0, sizeof(res)); - res.flags = IORESOURCE_MEM | IORESOURCE_BUSY; + res.flags = IORESOURCE_MEM; res.name = simplefb_resname; res.start = base; res.end = res.start + length - 1; -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 3/5] drm/simpledrm: Request memory region in driver 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 9:12 ` Thomas Zimmermann -1 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, Thomas Zimmermann, dri-devel Requesting the framebuffer memory in simpledrm marks the memory range as busy. This used to be done by the firmware sysfb code, but the driver is the correct place. v2: * use I/O memory if request_mem_region() fails (Jocelyn) Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> --- drivers/gpu/drm/tiny/simpledrm.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 04146da2d1d8..d191e87a37b5 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -526,21 +526,33 @@ static int simpledrm_device_init_mm(struct simpledrm_device *sdev) { struct drm_device *dev = &sdev->dev; struct platform_device *pdev = sdev->pdev; - struct resource *mem; + struct resource *res, *mem; void __iomem *screen_base; int ret; - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!mem) + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) return -EINVAL; - ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem)); + ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res)); if (ret) { drm_err(dev, "could not acquire memory range %pr: error %d\n", - mem, ret); + res, ret); return ret; } + mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), + sdev->dev.driver->name); + if (!mem) { + /* + * We cannot make this fatal. Sometimes this comes from magic + * spaces our resource handlers simply don't know about. Use + * the I/O-memory resource as-is and try to map that instead. + */ + drm_warn(dev, "could not acquire memory region %pr\n", res); + mem = res; + } + screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem)); if (!screen_base) -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 3/5] drm/simpledrm: Request memory region in driver @ 2022-01-25 9:12 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann Requesting the framebuffer memory in simpledrm marks the memory range as busy. This used to be done by the firmware sysfb code, but the driver is the correct place. v2: * use I/O memory if request_mem_region() fails (Jocelyn) Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> --- drivers/gpu/drm/tiny/simpledrm.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 04146da2d1d8..d191e87a37b5 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -526,21 +526,33 @@ static int simpledrm_device_init_mm(struct simpledrm_device *sdev) { struct drm_device *dev = &sdev->dev; struct platform_device *pdev = sdev->pdev; - struct resource *mem; + struct resource *res, *mem; void __iomem *screen_base; int ret; - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!mem) + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) return -EINVAL; - ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem)); + ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res)); if (ret) { drm_err(dev, "could not acquire memory range %pr: error %d\n", - mem, ret); + res, ret); return ret; } + mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), + sdev->dev.driver->name); + if (!mem) { + /* + * We cannot make this fatal. Sometimes this comes from magic + * spaces our resource handlers simply don't know about. Use + * the I/O-memory resource as-is and try to map that instead. + */ + drm_warn(dev, "could not acquire memory region %pr\n", res); + mem = res; + } + screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem)); if (!screen_base) -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 3/5] drm/simpledrm: Request memory region in driver 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 13:41 ` Jocelyn Falempe -1 siblings, 0 replies; 32+ messages in thread From: Jocelyn Falempe @ 2022-01-25 13:41 UTC (permalink / raw) To: Thomas Zimmermann, zackr, javierm, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, dri-devel Hi, Thanks for the fix. On 25/01/2022 10:12, Thomas Zimmermann wrote: > Requesting the framebuffer memory in simpledrm marks the memory > range as busy. This used to be done by the firmware sysfb code, > but the driver is the correct place. > > v2: > * use I/O memory if request_mem_region() fails (Jocelyn) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com> -- Jocelyn Falempe ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/5] drm/simpledrm: Request memory region in driver @ 2022-01-25 13:41 ` Jocelyn Falempe 0 siblings, 0 replies; 32+ messages in thread From: Jocelyn Falempe @ 2022-01-25 13:41 UTC (permalink / raw) To: Thomas Zimmermann, zackr, javierm, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev Hi, Thanks for the fix. On 25/01/2022 10:12, Thomas Zimmermann wrote: > Requesting the framebuffer memory in simpledrm marks the memory > range as busy. This used to be done by the firmware sysfb code, > but the driver is the correct place. > > v2: > * use I/O memory if request_mem_region() fails (Jocelyn) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com> -- Jocelyn Falempe ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 4/5] fbdev/simplefb: Request memory region in driver 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 9:12 ` Thomas Zimmermann -1 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, Thomas Zimmermann, dri-devel Requesting the framebuffer memory in simpledrm marks the memory range as busy. This used to be done by the firmware sysfb code, but the driver is the correct place. v2: * store memory region in struct for later cleanup (Javier) Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index 57541887188b..94fc9c6d0411 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -66,16 +66,36 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue, return 0; } -struct simplefb_par; +struct simplefb_par { + u32 palette[PSEUDO_PALETTE_SIZE]; + struct resource *mem; +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK + bool clks_enabled; + unsigned int clk_count; + struct clk **clks; +#endif +#if defined CONFIG_OF && defined CONFIG_REGULATOR + bool regulators_enabled; + u32 regulator_count; + struct regulator **regulators; +#endif +}; + static void simplefb_clocks_destroy(struct simplefb_par *par); static void simplefb_regulators_destroy(struct simplefb_par *par); static void simplefb_destroy(struct fb_info *info) { + struct simplefb_par *par = info->par; + struct resource *mem = par->mem; + simplefb_regulators_destroy(info->par); simplefb_clocks_destroy(info->par); if (info->screen_base) iounmap(info->screen_base); + + if (mem) + release_mem_region(mem->start, resource_size(mem)); } static const struct fb_ops simplefb_ops = { @@ -169,20 +189,6 @@ static int simplefb_parse_pd(struct platform_device *pdev, return 0; } -struct simplefb_par { - u32 palette[PSEUDO_PALETTE_SIZE]; -#if defined CONFIG_OF && defined CONFIG_COMMON_CLK - bool clks_enabled; - unsigned int clk_count; - struct clk **clks; -#endif -#if defined CONFIG_OF && defined CONFIG_REGULATOR - bool regulators_enabled; - u32 regulator_count; - struct regulator **regulators; -#endif -}; - #if defined CONFIG_OF && defined CONFIG_COMMON_CLK /* * Clock handling code. @@ -405,7 +411,7 @@ static int simplefb_probe(struct platform_device *pdev) struct simplefb_params params; struct fb_info *info; struct simplefb_par *par; - struct resource *mem; + struct resource *res, *mem; /* * Generic drivers must not be registered if a framebuffer exists. @@ -430,15 +436,28 @@ static int simplefb_probe(struct platform_device *pdev) if (ret) return ret; - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!mem) { + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { dev_err(&pdev->dev, "No memory resource\n"); return -EINVAL; } + mem = request_mem_region(res->start, resource_size(res), "simplefb"); + if (!mem) { + /* + * We cannot make this fatal. Sometimes this comes from magic + * spaces our resource handlers simply don't know about. Use + * the I/O-memory resource as-is and try to map that instead. + */ + dev_warn(&pdev->dev, "simplefb: cannot reserve video memory at %pR\n", res); + mem = res; + } + info = framebuffer_alloc(sizeof(struct simplefb_par), &pdev->dev); - if (!info) - return -ENOMEM; + if (!info) { + ret = -ENOMEM; + goto error_release_mem_region; + } platform_set_drvdata(pdev, info); par = info->par; @@ -495,6 +514,9 @@ static int simplefb_probe(struct platform_device *pdev) info->var.xres, info->var.yres, info->var.bits_per_pixel, info->fix.line_length); + if (mem != res) + par->mem = mem; /* release in clean-up handler */ + ret = register_framebuffer(info); if (ret < 0) { dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret); @@ -513,6 +535,9 @@ static int simplefb_probe(struct platform_device *pdev) iounmap(info->screen_base); error_fb_release: framebuffer_release(info); +error_release_mem_region: + if (mem != res) + release_mem_region(mem->start, resource_size(mem)); return ret; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 4/5] fbdev/simplefb: Request memory region in driver @ 2022-01-25 9:12 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann Requesting the framebuffer memory in simpledrm marks the memory range as busy. This used to be done by the firmware sysfb code, but the driver is the correct place. v2: * store memory region in struct for later cleanup (Javier) Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index 57541887188b..94fc9c6d0411 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -66,16 +66,36 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue, return 0; } -struct simplefb_par; +struct simplefb_par { + u32 palette[PSEUDO_PALETTE_SIZE]; + struct resource *mem; +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK + bool clks_enabled; + unsigned int clk_count; + struct clk **clks; +#endif +#if defined CONFIG_OF && defined CONFIG_REGULATOR + bool regulators_enabled; + u32 regulator_count; + struct regulator **regulators; +#endif +}; + static void simplefb_clocks_destroy(struct simplefb_par *par); static void simplefb_regulators_destroy(struct simplefb_par *par); static void simplefb_destroy(struct fb_info *info) { + struct simplefb_par *par = info->par; + struct resource *mem = par->mem; + simplefb_regulators_destroy(info->par); simplefb_clocks_destroy(info->par); if (info->screen_base) iounmap(info->screen_base); + + if (mem) + release_mem_region(mem->start, resource_size(mem)); } static const struct fb_ops simplefb_ops = { @@ -169,20 +189,6 @@ static int simplefb_parse_pd(struct platform_device *pdev, return 0; } -struct simplefb_par { - u32 palette[PSEUDO_PALETTE_SIZE]; -#if defined CONFIG_OF && defined CONFIG_COMMON_CLK - bool clks_enabled; - unsigned int clk_count; - struct clk **clks; -#endif -#if defined CONFIG_OF && defined CONFIG_REGULATOR - bool regulators_enabled; - u32 regulator_count; - struct regulator **regulators; -#endif -}; - #if defined CONFIG_OF && defined CONFIG_COMMON_CLK /* * Clock handling code. @@ -405,7 +411,7 @@ static int simplefb_probe(struct platform_device *pdev) struct simplefb_params params; struct fb_info *info; struct simplefb_par *par; - struct resource *mem; + struct resource *res, *mem; /* * Generic drivers must not be registered if a framebuffer exists. @@ -430,15 +436,28 @@ static int simplefb_probe(struct platform_device *pdev) if (ret) return ret; - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!mem) { + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { dev_err(&pdev->dev, "No memory resource\n"); return -EINVAL; } + mem = request_mem_region(res->start, resource_size(res), "simplefb"); + if (!mem) { + /* + * We cannot make this fatal. Sometimes this comes from magic + * spaces our resource handlers simply don't know about. Use + * the I/O-memory resource as-is and try to map that instead. + */ + dev_warn(&pdev->dev, "simplefb: cannot reserve video memory at %pR\n", res); + mem = res; + } + info = framebuffer_alloc(sizeof(struct simplefb_par), &pdev->dev); - if (!info) - return -ENOMEM; + if (!info) { + ret = -ENOMEM; + goto error_release_mem_region; + } platform_set_drvdata(pdev, info); par = info->par; @@ -495,6 +514,9 @@ static int simplefb_probe(struct platform_device *pdev) info->var.xres, info->var.yres, info->var.bits_per_pixel, info->fix.line_length); + if (mem != res) + par->mem = mem; /* release in clean-up handler */ + ret = register_framebuffer(info); if (ret < 0) { dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret); @@ -513,6 +535,9 @@ static int simplefb_probe(struct platform_device *pdev) iounmap(info->screen_base); error_fb_release: framebuffer_release(info); +error_release_mem_region: + if (mem != res) + release_mem_region(mem->start, resource_size(mem)); return ret; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 4/5] fbdev/simplefb: Request memory region in driver 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 9:33 ` Javier Martinez Canillas -1 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-25 9:33 UTC (permalink / raw) To: Thomas Zimmermann, zackr, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, dri-devel Hello Thomas, On 1/25/22 10:12, Thomas Zimmermann wrote: > Requesting the framebuffer memory in simpledrm marks the memory > range as busy. This used to be done by the firmware sysfb code, > but the driver is the correct place. > > v2: > * store memory region in struct for later cleanup (Javier) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Thanks a lot for updating the patch. Now this logic is also more similar to the changes done for the simpledrm driver in PATCH 3/5. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 4/5] fbdev/simplefb: Request memory region in driver @ 2022-01-25 9:33 ` Javier Martinez Canillas 0 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-25 9:33 UTC (permalink / raw) To: Thomas Zimmermann, zackr, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev Hello Thomas, On 1/25/22 10:12, Thomas Zimmermann wrote: > Requesting the framebuffer memory in simpledrm marks the memory > range as busy. This used to be done by the firmware sysfb code, > but the driver is the correct place. > > v2: > * store memory region in struct for later cleanup (Javier) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Thanks a lot for updating the patch. Now this logic is also more similar to the changes done for the simpledrm driver in PATCH 3/5. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 5/5] drm: Add TODO item for requesting memory regions 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 9:12 ` Thomas Zimmermann -1 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, Thomas Zimmermann, dri-devel Add a TODO item about requesting memory regions for each driver. The current DRM drivers don't do this consistently. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> --- Documentation/gpu/todo.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index da138dd39883..1b2372ef4131 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -467,6 +467,21 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de> Level: Intermediate +Request memory regions in all drivers +------------------------------------- + +Go through all drivers and add code to request the memory regions that the +driver uses. This requires adding calls to request_mem_region(), +pci_request_region() or similar functions. Use helpers for managed cleanup +where possible. + +Drivers are pretty bad at doing this and there used to be conflicts among +DRM and fbdev drivers. Still, it's the correct thing to do. + +Contact: Thomas Zimmermann <tzimmermann@suse.de> + +Level: Starter + Core refactorings ================= -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 5/5] drm: Add TODO item for requesting memory regions @ 2022-01-25 9:12 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:12 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann Add a TODO item about requesting memory regions for each driver. The current DRM drivers don't do this consistently. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> --- Documentation/gpu/todo.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index da138dd39883..1b2372ef4131 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -467,6 +467,21 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de> Level: Intermediate +Request memory regions in all drivers +------------------------------------- + +Go through all drivers and add code to request the memory regions that the +driver uses. This requires adding calls to request_mem_region(), +pci_request_region() or similar functions. Use helpers for managed cleanup +where possible. + +Drivers are pretty bad at doing this and there used to be conflicts among +DRM and fbdev drivers. Still, it's the correct thing to do. + +Contact: Thomas Zimmermann <tzimmermann@suse.de> + +Level: Starter + Core refactorings ================= -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 0/5] sysfb: Fix memory-region management 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 9:13 ` Thomas Zimmermann -1 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:13 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 2319 bytes --] This v2 of the patchset. I forgot to adapt the subject line :/ Am 25.01.22 um 10:12 schrieb Thomas Zimmermann: > Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug > operation when removing fbdev firmware drivers. > > After being unloaded by a hardware driver, simplefb leaves behind the > firmware framebuffer's platform device. This prevents other drivers > from acquiring the memory as reported at [1]. > > Patch 1 changes the removal code of remove_conflicting_framebuffers() > to remove the underlying device and the rsp memory region. > > Patches 2 to 4 update sysfb and its drivers. The sysfb code does no > longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the > device drivers acquire the memory when they probe the device. > > Patch 5 adds a todo item to acquire memory regions in all DRM drivers. > > Tested with simpledrm and simplefb. > > [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > > v2: > * fix possible NULL deref in simpledrm (Jocelyn) > * various style fixes (Javier) > > Javier Martinez Canillas (1): > drivers/firmware: Don't mark as busy the simple-framebuffer IO > resource > > Thomas Zimmermann (4): > fbdev: Hot-unplug firmware fb devices on forced removal > drm/simpledrm: Request memory region in driver > fbdev/simplefb: Request memory region in driver > drm: Add TODO item for requesting memory regions > > Documentation/gpu/todo.rst | 15 +++++++ > drivers/firmware/sysfb_simplefb.c | 2 +- > drivers/gpu/drm/tiny/simpledrm.c | 22 ++++++++--- > drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++-- > drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++---------- > include/linux/fb.h | 1 + > 6 files changed, 105 insertions(+), 29 deletions(-) > > > base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f > prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d > prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 > prerequisite-patch-id: 8e52143a6cd7b8fb789e656208f6edde71d0f499 -- 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] 32+ messages in thread
* Re: [PATCH 0/5] sysfb: Fix memory-region management @ 2022-01-25 9:13 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-25 9:13 UTC (permalink / raw) To: zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev [-- Attachment #1.1: Type: text/plain, Size: 2319 bytes --] This v2 of the patchset. I forgot to adapt the subject line :/ Am 25.01.22 um 10:12 schrieb Thomas Zimmermann: > Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug > operation when removing fbdev firmware drivers. > > After being unloaded by a hardware driver, simplefb leaves behind the > firmware framebuffer's platform device. This prevents other drivers > from acquiring the memory as reported at [1]. > > Patch 1 changes the removal code of remove_conflicting_framebuffers() > to remove the underlying device and the rsp memory region. > > Patches 2 to 4 update sysfb and its drivers. The sysfb code does no > longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the > device drivers acquire the memory when they probe the device. > > Patch 5 adds a todo item to acquire memory regions in all DRM drivers. > > Tested with simpledrm and simplefb. > > [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > > v2: > * fix possible NULL deref in simpledrm (Jocelyn) > * various style fixes (Javier) > > Javier Martinez Canillas (1): > drivers/firmware: Don't mark as busy the simple-framebuffer IO > resource > > Thomas Zimmermann (4): > fbdev: Hot-unplug firmware fb devices on forced removal > drm/simpledrm: Request memory region in driver > fbdev/simplefb: Request memory region in driver > drm: Add TODO item for requesting memory regions > > Documentation/gpu/todo.rst | 15 +++++++ > drivers/firmware/sysfb_simplefb.c | 2 +- > drivers/gpu/drm/tiny/simpledrm.c | 22 ++++++++--- > drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++-- > drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++---------- > include/linux/fb.h | 1 + > 6 files changed, 105 insertions(+), 29 deletions(-) > > > base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f > prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d > prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 > prerequisite-patch-id: 8e52143a6cd7b8fb789e656208f6edde71d0f499 -- 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] 32+ messages in thread
* Re: [PATCH 0/5] sysfb: Fix memory-region management 2022-01-25 9:12 ` Thomas Zimmermann @ 2022-01-25 15:34 ` Hans de Goede -1 siblings, 0 replies; 32+ messages in thread From: Hans de Goede @ 2022-01-25 15:34 UTC (permalink / raw) To: Thomas Zimmermann, zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller Cc: linux-fbdev, dri-devel Hi Thomas, On 1/25/22 10:12, Thomas Zimmermann wrote: > Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug > operation when removing fbdev firmware drivers. > > After being unloaded by a hardware driver, simplefb leaves behind the > firmware framebuffer's platform device. This prevents other drivers > from acquiring the memory as reported at [1]. > > Patch 1 changes the removal code of remove_conflicting_framebuffers() > to remove the underlying device and the rsp memory region. > > Patches 2 to 4 update sysfb and its drivers. The sysfb code does no > longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the > device drivers acquire the memory when they probe the device. > > Patch 5 adds a todo item to acquire memory regions in all DRM drivers. > > Tested with simpledrm and simplefb. > > [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > > v2: > * fix possible NULL deref in simpledrm (Jocelyn) > * various style fixes (Javier) The entire series looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> for the series. Regards, Hans > > Javier Martinez Canillas (1): > drivers/firmware: Don't mark as busy the simple-framebuffer IO > resource > > Thomas Zimmermann (4): > fbdev: Hot-unplug firmware fb devices on forced removal > drm/simpledrm: Request memory region in driver > fbdev/simplefb: Request memory region in driver > drm: Add TODO item for requesting memory regions > > Documentation/gpu/todo.rst | 15 +++++++ > drivers/firmware/sysfb_simplefb.c | 2 +- > drivers/gpu/drm/tiny/simpledrm.c | 22 ++++++++--- > drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++-- > drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++---------- > include/linux/fb.h | 1 + > 6 files changed, 105 insertions(+), 29 deletions(-) > > > base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f > prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d > prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 > prerequisite-patch-id: 8e52143a6cd7b8fb789e656208f6edde71d0f499 > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 0/5] sysfb: Fix memory-region management @ 2022-01-25 15:34 ` Hans de Goede 0 siblings, 0 replies; 32+ messages in thread From: Hans de Goede @ 2022-01-25 15:34 UTC (permalink / raw) To: Thomas Zimmermann, zackr, javierm, jfalempe, maarten.lankhorst, mripard, airlied, daniel, deller Cc: dri-devel, linux-fbdev Hi Thomas, On 1/25/22 10:12, Thomas Zimmermann wrote: > Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug > operation when removing fbdev firmware drivers. > > After being unloaded by a hardware driver, simplefb leaves behind the > firmware framebuffer's platform device. This prevents other drivers > from acquiring the memory as reported at [1]. > > Patch 1 changes the removal code of remove_conflicting_framebuffers() > to remove the underlying device and the rsp memory region. > > Patches 2 to 4 update sysfb and its drivers. The sysfb code does no > longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the > device drivers acquire the memory when they probe the device. > > Patch 5 adds a todo item to acquire memory regions in all DRM drivers. > > Tested with simpledrm and simplefb. > > [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > > v2: > * fix possible NULL deref in simpledrm (Jocelyn) > * various style fixes (Javier) The entire series looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> for the series. Regards, Hans > > Javier Martinez Canillas (1): > drivers/firmware: Don't mark as busy the simple-framebuffer IO > resource > > Thomas Zimmermann (4): > fbdev: Hot-unplug firmware fb devices on forced removal > drm/simpledrm: Request memory region in driver > fbdev/simplefb: Request memory region in driver > drm: Add TODO item for requesting memory regions > > Documentation/gpu/todo.rst | 15 +++++++ > drivers/firmware/sysfb_simplefb.c | 2 +- > drivers/gpu/drm/tiny/simpledrm.c | 22 ++++++++--- > drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++-- > drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++---------- > include/linux/fb.h | 1 + > 6 files changed, 105 insertions(+), 29 deletions(-) > > > base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f > prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d > prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 > prerequisite-patch-id: 8e52143a6cd7b8fb789e656208f6edde71d0f499 > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 0/5] sysfb: Fix memory-region management @ 2022-01-24 12:36 Thomas Zimmermann 2022-01-24 12:36 ` Thomas Zimmermann 0 siblings, 1 reply; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-24 12:36 UTC (permalink / raw) To: zackr, javierm, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann Request framebuffer memory in simpledrm and simplefb. Do a hot-unplug operation when removing fbdev firmware drivers. After being unloaded by a hardware driver, simplefb leaves behind the firmware framebuffer's platform device. This prevents other drivers from acquiring the memory as reported at [1]. Patch 1 changes the removal code of remove_conflicting_framebuffers() to remove the underlying device and the rsp memory region. Patches 2 to 4 update sysfb and its drivers. The sysfb code does no longer mark the framebuffer memory with IORESOURCE_BUSY. Instead, the device drivers acquire the memory when they probe the device. Patch 5 adds a todo item to acquire memory regions in all DRM drivers. Tested with simpledrm and simplefb. [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ Javier Martinez Canillas (1): drivers/firmware: Don't mark as busy the simple-framebuffer IO resource Thomas Zimmermann (4): fbdev: Hot-unplug firmware fb devices on forced removal drm/simpledrm: Request memory region in driver fbdev/simplefb: Request memory region in driver drm: Add TODO item for requesting memory regions Documentation/gpu/todo.rst | 15 ++++++++ drivers/firmware/sysfb_simplefb.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 20 ++++++++--- drivers/video/fbdev/core/fbmem.c | 29 +++++++++++++-- drivers/video/fbdev/simplefb.c | 59 ++++++++++++++++++++++--------- include/linux/fb.h | 1 + 6 files changed, 100 insertions(+), 26 deletions(-) base-commit: 0bb81b5d6db5f689b67f9d8b35323235c45e890f -- 2.34.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal 2022-01-24 12:36 Thomas Zimmermann @ 2022-01-24 12:36 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-24 12:36 UTC (permalink / raw) To: zackr, javierm, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, Thomas Zimmermann, stable Hot-unplug all firmware-framebuffer devices as part of removing them via remove_conflicting_framebuffers() et al. Releases all memory regions to be acquired by native drivers. Firmware, such as EFI, install a framebuffer while posting the computer. After removing the firmware-framebuffer device from fbdev, a native driver takes over the hardware and the firmware framebuffer becomes invalid. Firmware-framebuffer drivers, specifically simplefb, don't release their device from Linux' device hierarchy. It still owns the firmware framebuffer and blocks the native drivers from loading. This has been observed in the vmwgfx driver. [1] Initiating a device removal (i.e., hot unplug) as part of remove_conflicting_framebuffers() removes the underlying device and returns the memory range to the system. [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> CC: stable@vger.kernel.org # v5.11+ --- drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- include/linux/fb.h | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..f73f8415b8cb 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -25,6 +25,7 @@ #include <linux/init.h> #include <linux/linux_logo.h> #include <linux/proc_fs.h> +#include <linux/platform_device.h> #include <linux/seq_file.h> #include <linux/console.h> #include <linux/kmod.h> @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, /* check all firmware fbs and kick off if the base addr overlaps */ for_each_registered_fb(i) { struct apertures_struct *gen_aper; + struct device *dev; if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) continue; gen_aper = registered_fb[i]->apertures; + dev = registered_fb[i]->device; if (fb_do_apertures_overlap(gen_aper, a) || (primary && gen_aper && gen_aper->count && gen_aper->ranges[0].base == VGA_FB_PHYS)) { printk(KERN_INFO "fb%d: switching to %s from %s\n", i, name, registered_fb[i]->fix.id); - do_unregister_framebuffer(registered_fb[i]); + + /* + * If we kick-out a firmware driver, we also want to remove + * the underlying platform device, such as simple-framebuffer, + * VESA, EFI, etc. A native driver will then be able to + * allocate the memory range. + * + * 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(dev)) { + registered_fb[i]->forced_out = true; + platform_device_unregister(to_platform_device(dev)); + } else { + pr_warn("fb%d: cannot remove device\n", i); + do_unregister_framebuffer(registered_fb[i]); + } } } } @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); void unregister_framebuffer(struct fb_info *fb_info) { - mutex_lock(®istration_lock); + bool forced_out = fb_info->forced_out; + + if (!forced_out) + mutex_lock(®istration_lock); do_unregister_framebuffer(fb_info); - mutex_unlock(®istration_lock); + if (!forced_out) + mutex_unlock(®istration_lock); } EXPORT_SYMBOL(unregister_framebuffer); diff --git a/include/linux/fb.h b/include/linux/fb.h index 3da95842b207..9a14f3f8a329 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -502,6 +502,7 @@ struct fb_info { } *apertures; bool skip_vt_switch; /* no VT switch on suspend/resume required */ + bool forced_out; /* set when being removed by another driver */ }; static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal @ 2022-01-24 12:36 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-24 12:36 UTC (permalink / raw) To: zackr, javierm, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: stable, linux-fbdev, Thomas Zimmermann, dri-devel Hot-unplug all firmware-framebuffer devices as part of removing them via remove_conflicting_framebuffers() et al. Releases all memory regions to be acquired by native drivers. Firmware, such as EFI, install a framebuffer while posting the computer. After removing the firmware-framebuffer device from fbdev, a native driver takes over the hardware and the firmware framebuffer becomes invalid. Firmware-framebuffer drivers, specifically simplefb, don't release their device from Linux' device hierarchy. It still owns the firmware framebuffer and blocks the native drivers from loading. This has been observed in the vmwgfx driver. [1] Initiating a device removal (i.e., hot unplug) as part of remove_conflicting_framebuffers() removes the underlying device and returns the memory range to the system. [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> CC: stable@vger.kernel.org # v5.11+ --- drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- include/linux/fb.h | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..f73f8415b8cb 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -25,6 +25,7 @@ #include <linux/init.h> #include <linux/linux_logo.h> #include <linux/proc_fs.h> +#include <linux/platform_device.h> #include <linux/seq_file.h> #include <linux/console.h> #include <linux/kmod.h> @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, /* check all firmware fbs and kick off if the base addr overlaps */ for_each_registered_fb(i) { struct apertures_struct *gen_aper; + struct device *dev; if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) continue; gen_aper = registered_fb[i]->apertures; + dev = registered_fb[i]->device; if (fb_do_apertures_overlap(gen_aper, a) || (primary && gen_aper && gen_aper->count && gen_aper->ranges[0].base == VGA_FB_PHYS)) { printk(KERN_INFO "fb%d: switching to %s from %s\n", i, name, registered_fb[i]->fix.id); - do_unregister_framebuffer(registered_fb[i]); + + /* + * If we kick-out a firmware driver, we also want to remove + * the underlying platform device, such as simple-framebuffer, + * VESA, EFI, etc. A native driver will then be able to + * allocate the memory range. + * + * 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(dev)) { + registered_fb[i]->forced_out = true; + platform_device_unregister(to_platform_device(dev)); + } else { + pr_warn("fb%d: cannot remove device\n", i); + do_unregister_framebuffer(registered_fb[i]); + } } } } @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); void unregister_framebuffer(struct fb_info *fb_info) { - mutex_lock(®istration_lock); + bool forced_out = fb_info->forced_out; + + if (!forced_out) + mutex_lock(®istration_lock); do_unregister_framebuffer(fb_info); - mutex_unlock(®istration_lock); + if (!forced_out) + mutex_unlock(®istration_lock); } EXPORT_SYMBOL(unregister_framebuffer); diff --git a/include/linux/fb.h b/include/linux/fb.h index 3da95842b207..9a14f3f8a329 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -502,6 +502,7 @@ struct fb_info { } *apertures; bool skip_vt_switch; /* no VT switch on suspend/resume required */ + bool forced_out; /* set when being removed by another driver */ }; static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal 2022-01-24 12:36 ` Thomas Zimmermann @ 2022-01-24 13:52 ` Javier Martinez Canillas -1 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-24 13:52 UTC (permalink / raw) To: Thomas Zimmermann, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, stable Hello Thomas, Thanks for the patch. On 1/24/22 13:36, Thomas Zimmermann wrote: > Hot-unplug all firmware-framebuffer devices as part of removing > them via remove_conflicting_framebuffers() et al. Releases all > memory regions to be acquired by native drivers. > > Firmware, such as EFI, install a framebuffer while posting the > computer. After removing the firmware-framebuffer device from fbdev, > a native driver takes over the hardware and the firmware framebuffer > becomes invalid. > > Firmware-framebuffer drivers, specifically simplefb, don't release > their device from Linux' device hierarchy. It still owns the firmware > framebuffer and blocks the native drivers from loading. This has been > observed in the vmwgfx driver. [1] > > Initiating a device removal (i.e., hot unplug) as part of > remove_conflicting_framebuffers() removes the underlying device and > returns the memory range to the system. > > [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > I would add a Reported-by tag here for Zack. > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > CC: stable@vger.kernel.org # v5.11+ > --- > drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- > include/linux/fb.h | 1 + > 2 files changed, 27 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index 0fa7ede94fa6..f73f8415b8cb 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -25,6 +25,7 @@ > #include <linux/init.h> > #include <linux/linux_logo.h> > #include <linux/proc_fs.h> > +#include <linux/platform_device.h> > #include <linux/seq_file.h> > #include <linux/console.h> > #include <linux/kmod.h> > @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, > /* check all firmware fbs and kick off if the base addr overlaps */ > for_each_registered_fb(i) { > struct apertures_struct *gen_aper; > + struct device *dev; > > if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) > continue; > > gen_aper = registered_fb[i]->apertures; > + dev = registered_fb[i]->device; > if (fb_do_apertures_overlap(gen_aper, a) || > (primary && gen_aper && gen_aper->count && > gen_aper->ranges[0].base == VGA_FB_PHYS)) { > > printk(KERN_INFO "fb%d: switching to %s from %s\n", > i, name, registered_fb[i]->fix.id); > - do_unregister_framebuffer(registered_fb[i]); > + > + /* > + * If we kick-out a firmware driver, we also want to remove > + * the underlying platform device, such as simple-framebuffer, > + * VESA, EFI, etc. A native driver will then be able to > + * allocate the memory range. > + * > + * 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(dev)) { In do_register_framebuffer() creating the fb%d is not a fatal error. It would be safer to do if (!IS_ERR_OR_NULL(dev) && dev_is_platform(dev)) instead here. https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L1605 > + registered_fb[i]->forced_out = true; > + platform_device_unregister(to_platform_device(dev)); > + } else { > + pr_warn("fb%d: cannot remove device\n", i); > + do_unregister_framebuffer(registered_fb[i]); > + } > } > } > } > @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); > void > unregister_framebuffer(struct fb_info *fb_info) > { > - mutex_lock(®istration_lock); > + bool forced_out = fb_info->forced_out; > + > + if (!forced_out) > + mutex_lock(®istration_lock); > do_unregister_framebuffer(fb_info); > - mutex_unlock(®istration_lock); > + if (!forced_out) > + mutex_unlock(®istration_lock); > } I'm not sure to follow the logic here. The forced_out bool is set when the platform device is unregistered in do_remove_conflicting_framebuffers(), but shouldn't the struct platform_driver .remove callback be executed even in this case ? That is, the platform_device_unregister() will trigger the call to the .remove callback that in turn will call unregister_framebuffer(). Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal @ 2022-01-24 13:52 ` Javier Martinez Canillas 0 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-24 13:52 UTC (permalink / raw) To: Thomas Zimmermann, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, stable, dri-devel Hello Thomas, Thanks for the patch. On 1/24/22 13:36, Thomas Zimmermann wrote: > Hot-unplug all firmware-framebuffer devices as part of removing > them via remove_conflicting_framebuffers() et al. Releases all > memory regions to be acquired by native drivers. > > Firmware, such as EFI, install a framebuffer while posting the > computer. After removing the firmware-framebuffer device from fbdev, > a native driver takes over the hardware and the firmware framebuffer > becomes invalid. > > Firmware-framebuffer drivers, specifically simplefb, don't release > their device from Linux' device hierarchy. It still owns the firmware > framebuffer and blocks the native drivers from loading. This has been > observed in the vmwgfx driver. [1] > > Initiating a device removal (i.e., hot unplug) as part of > remove_conflicting_framebuffers() removes the underlying device and > returns the memory range to the system. > > [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > I would add a Reported-by tag here for Zack. > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > CC: stable@vger.kernel.org # v5.11+ > --- > drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- > include/linux/fb.h | 1 + > 2 files changed, 27 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index 0fa7ede94fa6..f73f8415b8cb 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -25,6 +25,7 @@ > #include <linux/init.h> > #include <linux/linux_logo.h> > #include <linux/proc_fs.h> > +#include <linux/platform_device.h> > #include <linux/seq_file.h> > #include <linux/console.h> > #include <linux/kmod.h> > @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, > /* check all firmware fbs and kick off if the base addr overlaps */ > for_each_registered_fb(i) { > struct apertures_struct *gen_aper; > + struct device *dev; > > if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) > continue; > > gen_aper = registered_fb[i]->apertures; > + dev = registered_fb[i]->device; > if (fb_do_apertures_overlap(gen_aper, a) || > (primary && gen_aper && gen_aper->count && > gen_aper->ranges[0].base == VGA_FB_PHYS)) { > > printk(KERN_INFO "fb%d: switching to %s from %s\n", > i, name, registered_fb[i]->fix.id); > - do_unregister_framebuffer(registered_fb[i]); > + > + /* > + * If we kick-out a firmware driver, we also want to remove > + * the underlying platform device, such as simple-framebuffer, > + * VESA, EFI, etc. A native driver will then be able to > + * allocate the memory range. > + * > + * 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(dev)) { In do_register_framebuffer() creating the fb%d is not a fatal error. It would be safer to do if (!IS_ERR_OR_NULL(dev) && dev_is_platform(dev)) instead here. https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L1605 > + registered_fb[i]->forced_out = true; > + platform_device_unregister(to_platform_device(dev)); > + } else { > + pr_warn("fb%d: cannot remove device\n", i); > + do_unregister_framebuffer(registered_fb[i]); > + } > } > } > } > @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); > void > unregister_framebuffer(struct fb_info *fb_info) > { > - mutex_lock(®istration_lock); > + bool forced_out = fb_info->forced_out; > + > + if (!forced_out) > + mutex_lock(®istration_lock); > do_unregister_framebuffer(fb_info); > - mutex_unlock(®istration_lock); > + if (!forced_out) > + mutex_unlock(®istration_lock); > } I'm not sure to follow the logic here. The forced_out bool is set when the platform device is unregistered in do_remove_conflicting_framebuffers(), but shouldn't the struct platform_driver .remove callback be executed even in this case ? That is, the platform_device_unregister() will trigger the call to the .remove callback that in turn will call unregister_framebuffer(). Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal 2022-01-24 13:52 ` Javier Martinez Canillas @ 2022-01-24 13:56 ` Javier Martinez Canillas -1 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-24 13:56 UTC (permalink / raw) To: Thomas Zimmermann, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: dri-devel, linux-fbdev, stable On 1/24/22 14:52, Javier Martinez Canillas wrote: [snip] >> @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); >> void >> unregister_framebuffer(struct fb_info *fb_info) >> { >> - mutex_lock(®istration_lock); >> + bool forced_out = fb_info->forced_out; >> + >> + if (!forced_out) >> + mutex_lock(®istration_lock); >> do_unregister_framebuffer(fb_info); >> - mutex_unlock(®istration_lock); >> + if (!forced_out) >> + mutex_unlock(®istration_lock); >> } > > I'm not sure to follow the logic here. The forced_out bool is set when the > platform device is unregistered in do_remove_conflicting_framebuffers(), > but shouldn't the struct platform_driver .remove callback be executed even > in this case ? > > That is, the platform_device_unregister() will trigger the call to the > .remove callback that in turn will call unregister_framebuffer(). > > Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? > Scratch that, I got it now. That's exactly the reason why you skip the mutext_lock(). After adding the check for dev, feel free to add: Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal @ 2022-01-24 13:56 ` Javier Martinez Canillas 0 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-24 13:56 UTC (permalink / raw) To: Thomas Zimmermann, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, stable, dri-devel On 1/24/22 14:52, Javier Martinez Canillas wrote: [snip] >> @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); >> void >> unregister_framebuffer(struct fb_info *fb_info) >> { >> - mutex_lock(®istration_lock); >> + bool forced_out = fb_info->forced_out; >> + >> + if (!forced_out) >> + mutex_lock(®istration_lock); >> do_unregister_framebuffer(fb_info); >> - mutex_unlock(®istration_lock); >> + if (!forced_out) >> + mutex_unlock(®istration_lock); >> } > > I'm not sure to follow the logic here. The forced_out bool is set when the > platform device is unregistered in do_remove_conflicting_framebuffers(), > but shouldn't the struct platform_driver .remove callback be executed even > in this case ? > > That is, the platform_device_unregister() will trigger the call to the > .remove callback that in turn will call unregister_framebuffer(). > > Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? > Scratch that, I got it now. That's exactly the reason why you skip the mutext_lock(). After adding the check for dev, feel free to add: Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal 2022-01-24 13:52 ` Javier Martinez Canillas @ 2022-01-24 14:19 ` Thomas Zimmermann -1 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-24 14:19 UTC (permalink / raw) To: Javier Martinez Canillas, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, stable, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 5210 bytes --] Hi Am 24.01.22 um 14:52 schrieb Javier Martinez Canillas: > Hello Thomas, > > Thanks for the patch. > > On 1/24/22 13:36, Thomas Zimmermann wrote: >> Hot-unplug all firmware-framebuffer devices as part of removing >> them via remove_conflicting_framebuffers() et al. Releases all >> memory regions to be acquired by native drivers. >> >> Firmware, such as EFI, install a framebuffer while posting the >> computer. After removing the firmware-framebuffer device from fbdev, >> a native driver takes over the hardware and the firmware framebuffer >> becomes invalid. >> >> Firmware-framebuffer drivers, specifically simplefb, don't release >> their device from Linux' device hierarchy. It still owns the firmware >> framebuffer and blocks the native drivers from loading. This has been >> observed in the vmwgfx driver. [1] >> >> Initiating a device removal (i.e., hot unplug) as part of >> remove_conflicting_framebuffers() removes the underlying device and >> returns the memory range to the system. >> >> [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ >> > > I would add a Reported-by tag here for Zack. Indeed, I simply forgot about it. > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> CC: stable@vger.kernel.org # v5.11+ >> --- >> drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- >> include/linux/fb.h | 1 + >> 2 files changed, 27 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c >> index 0fa7ede94fa6..f73f8415b8cb 100644 >> --- a/drivers/video/fbdev/core/fbmem.c >> +++ b/drivers/video/fbdev/core/fbmem.c >> @@ -25,6 +25,7 @@ >> #include <linux/init.h> >> #include <linux/linux_logo.h> >> #include <linux/proc_fs.h> >> +#include <linux/platform_device.h> >> #include <linux/seq_file.h> >> #include <linux/console.h> >> #include <linux/kmod.h> >> @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, >> /* check all firmware fbs and kick off if the base addr overlaps */ >> for_each_registered_fb(i) { >> struct apertures_struct *gen_aper; >> + struct device *dev; >> >> if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) >> continue; >> >> gen_aper = registered_fb[i]->apertures; >> + dev = registered_fb[i]->device; >> if (fb_do_apertures_overlap(gen_aper, a) || >> (primary && gen_aper && gen_aper->count && >> gen_aper->ranges[0].base == VGA_FB_PHYS)) { >> >> printk(KERN_INFO "fb%d: switching to %s from %s\n", >> i, name, registered_fb[i]->fix.id); >> - do_unregister_framebuffer(registered_fb[i]); >> + >> + /* >> + * If we kick-out a firmware driver, we also want to remove >> + * the underlying platform device, such as simple-framebuffer, >> + * VESA, EFI, etc. A native driver will then be able to >> + * allocate the memory range. >> + * >> + * 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(dev)) { > > In do_register_framebuffer() creating the fb%d is not a fatal error. It would > be safer to do if (!IS_ERR_OR_NULL(dev) && dev_is_platform(dev)) instead here. > > https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L1605 'dev' here refers to 'fb_info->device', which is the underlying device created by the sysfb code. fb_info->dev is something different. > >> + registered_fb[i]->forced_out = true; >> + platform_device_unregister(to_platform_device(dev)); >> + } else { >> + pr_warn("fb%d: cannot remove device\n", i); >> + do_unregister_framebuffer(registered_fb[i]); >> + } >> } >> } >> } >> @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); >> void >> unregister_framebuffer(struct fb_info *fb_info) >> { >> - mutex_lock(®istration_lock); >> + bool forced_out = fb_info->forced_out; >> + >> + if (!forced_out) >> + mutex_lock(®istration_lock); >> do_unregister_framebuffer(fb_info); >> - mutex_unlock(®istration_lock); >> + if (!forced_out) >> + mutex_unlock(®istration_lock); >> } > > I'm not sure to follow the logic here. The forced_out bool is set when the > platform device is unregistered in do_remove_conflicting_framebuffers(), > but shouldn't the struct platform_driver .remove callback be executed even > in this case ? > > That is, the platform_device_unregister() will trigger the call to the > .remove callback that in turn will call unregister_framebuffer(). > > Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? Doing the hot-unplug will end up in unregister_framebuffer(), but we already hold the lock from the do_remove_conflicting_framebuffer() code. Best regards Thomas > > Best regards, -- 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] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal @ 2022-01-24 14:19 ` Thomas Zimmermann 0 siblings, 0 replies; 32+ messages in thread From: Thomas Zimmermann @ 2022-01-24 14:19 UTC (permalink / raw) To: Javier Martinez Canillas, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, dri-devel, stable [-- Attachment #1.1: Type: text/plain, Size: 5210 bytes --] Hi Am 24.01.22 um 14:52 schrieb Javier Martinez Canillas: > Hello Thomas, > > Thanks for the patch. > > On 1/24/22 13:36, Thomas Zimmermann wrote: >> Hot-unplug all firmware-framebuffer devices as part of removing >> them via remove_conflicting_framebuffers() et al. Releases all >> memory regions to be acquired by native drivers. >> >> Firmware, such as EFI, install a framebuffer while posting the >> computer. After removing the firmware-framebuffer device from fbdev, >> a native driver takes over the hardware and the firmware framebuffer >> becomes invalid. >> >> Firmware-framebuffer drivers, specifically simplefb, don't release >> their device from Linux' device hierarchy. It still owns the firmware >> framebuffer and blocks the native drivers from loading. This has been >> observed in the vmwgfx driver. [1] >> >> Initiating a device removal (i.e., hot unplug) as part of >> remove_conflicting_framebuffers() removes the underlying device and >> returns the memory range to the system. >> >> [1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ >> > > I would add a Reported-by tag here for Zack. Indeed, I simply forgot about it. > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> CC: stable@vger.kernel.org # v5.11+ >> --- >> drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- >> include/linux/fb.h | 1 + >> 2 files changed, 27 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c >> index 0fa7ede94fa6..f73f8415b8cb 100644 >> --- a/drivers/video/fbdev/core/fbmem.c >> +++ b/drivers/video/fbdev/core/fbmem.c >> @@ -25,6 +25,7 @@ >> #include <linux/init.h> >> #include <linux/linux_logo.h> >> #include <linux/proc_fs.h> >> +#include <linux/platform_device.h> >> #include <linux/seq_file.h> >> #include <linux/console.h> >> #include <linux/kmod.h> >> @@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, >> /* check all firmware fbs and kick off if the base addr overlaps */ >> for_each_registered_fb(i) { >> struct apertures_struct *gen_aper; >> + struct device *dev; >> >> if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) >> continue; >> >> gen_aper = registered_fb[i]->apertures; >> + dev = registered_fb[i]->device; >> if (fb_do_apertures_overlap(gen_aper, a) || >> (primary && gen_aper && gen_aper->count && >> gen_aper->ranges[0].base == VGA_FB_PHYS)) { >> >> printk(KERN_INFO "fb%d: switching to %s from %s\n", >> i, name, registered_fb[i]->fix.id); >> - do_unregister_framebuffer(registered_fb[i]); >> + >> + /* >> + * If we kick-out a firmware driver, we also want to remove >> + * the underlying platform device, such as simple-framebuffer, >> + * VESA, EFI, etc. A native driver will then be able to >> + * allocate the memory range. >> + * >> + * 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(dev)) { > > In do_register_framebuffer() creating the fb%d is not a fatal error. It would > be safer to do if (!IS_ERR_OR_NULL(dev) && dev_is_platform(dev)) instead here. > > https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L1605 'dev' here refers to 'fb_info->device', which is the underlying device created by the sysfb code. fb_info->dev is something different. > >> + registered_fb[i]->forced_out = true; >> + platform_device_unregister(to_platform_device(dev)); >> + } else { >> + pr_warn("fb%d: cannot remove device\n", i); >> + do_unregister_framebuffer(registered_fb[i]); >> + } >> } >> } >> } >> @@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); >> void >> unregister_framebuffer(struct fb_info *fb_info) >> { >> - mutex_lock(®istration_lock); >> + bool forced_out = fb_info->forced_out; >> + >> + if (!forced_out) >> + mutex_lock(®istration_lock); >> do_unregister_framebuffer(fb_info); >> - mutex_unlock(®istration_lock); >> + if (!forced_out) >> + mutex_unlock(®istration_lock); >> } > > I'm not sure to follow the logic here. The forced_out bool is set when the > platform device is unregistered in do_remove_conflicting_framebuffers(), > but shouldn't the struct platform_driver .remove callback be executed even > in this case ? > > That is, the platform_device_unregister() will trigger the call to the > .remove callback that in turn will call unregister_framebuffer(). > > Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? Doing the hot-unplug will end up in unregister_framebuffer(), but we already hold the lock from the do_remove_conflicting_framebuffer() code. Best regards Thomas > > Best regards, -- 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] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal 2022-01-24 14:19 ` Thomas Zimmermann @ 2022-01-24 14:31 ` Javier Martinez Canillas -1 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-24 14:31 UTC (permalink / raw) To: Thomas Zimmermann, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, stable, dri-devel On 1/24/22 15:19, Thomas Zimmermann wrote: [snip] >>> + if (dev_is_platform(dev)) { >> >> In do_register_framebuffer() creating the fb%d is not a fatal error. It would >> be safer to do if (!IS_ERR_OR_NULL(dev) && dev_is_platform(dev)) instead here. >> >> https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L1605 > > 'dev' here refers to 'fb_info->device', which is the underlying device > created by the sysfb code. fb_info->dev is something different. > oh, indeed. I conflated the two. Maybe the local variable could be renamed to 'device' just to avoid confusion ? [snip] >> I'm not sure to follow the logic here. The forced_out bool is set when the >> platform device is unregistered in do_remove_conflicting_framebuffers(), >> but shouldn't the struct platform_driver .remove callback be executed even >> in this case ? >> >> That is, the platform_device_unregister() will trigger the call to the >> .remove callback that in turn will call unregister_framebuffer(). >> >> Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? > > Doing the hot-unplug will end up in unregister_framebuffer(), but we > already hold the lock from the do_remove_conflicting_framebuffer() code. > Yes, I realized that just after sending the first email. Sorry for the noise. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal @ 2022-01-24 14:31 ` Javier Martinez Canillas 0 siblings, 0 replies; 32+ messages in thread From: Javier Martinez Canillas @ 2022-01-24 14:31 UTC (permalink / raw) To: Thomas Zimmermann, zackr, maarten.lankhorst, mripard, airlied, daniel, deller, hdegoede Cc: linux-fbdev, dri-devel, stable On 1/24/22 15:19, Thomas Zimmermann wrote: [snip] >>> + if (dev_is_platform(dev)) { >> >> In do_register_framebuffer() creating the fb%d is not a fatal error. It would >> be safer to do if (!IS_ERR_OR_NULL(dev) && dev_is_platform(dev)) instead here. >> >> https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L1605 > > 'dev' here refers to 'fb_info->device', which is the underlying device > created by the sysfb code. fb_info->dev is something different. > oh, indeed. I conflated the two. Maybe the local variable could be renamed to 'device' just to avoid confusion ? [snip] >> I'm not sure to follow the logic here. The forced_out bool is set when the >> platform device is unregistered in do_remove_conflicting_framebuffers(), >> but shouldn't the struct platform_driver .remove callback be executed even >> in this case ? >> >> That is, the platform_device_unregister() will trigger the call to the >> .remove callback that in turn will call unregister_framebuffer(). >> >> Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ? > > Doing the hot-unplug will end up in unregister_framebuffer(), but we > already hold the lock from the do_remove_conflicting_framebuffer() code. > Yes, I realized that just after sending the first email. Sorry for the noise. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal 2022-01-24 12:36 ` Thomas Zimmermann @ 2022-01-24 15:59 ` Zack Rusin -1 siblings, 0 replies; 32+ messages in thread From: Zack Rusin @ 2022-01-24 15:59 UTC (permalink / raw) To: airlied, tzimmermann, javierm, hdegoede, maarten.lankhorst, deller, daniel, mripard Cc: dri-devel, stable, linux-fbdev On Mon, 2022-01-24 at 13:36 +0100, Thomas Zimmermann wrote: > Hot-unplug all firmware-framebuffer devices as part of removing > them via remove_conflicting_framebuffers() et al. Releases all > memory regions to be acquired by native drivers. > > Firmware, such as EFI, install a framebuffer while posting the > computer. After removing the firmware-framebuffer device from fbdev, > a native driver takes over the hardware and the firmware framebuffer > becomes invalid. > > Firmware-framebuffer drivers, specifically simplefb, don't release > their device from Linux' device hierarchy. It still owns the firmware > framebuffer and blocks the native drivers from loading. This has been > observed in the vmwgfx driver. [1] > > Initiating a device removal (i.e., hot unplug) as part of > remove_conflicting_framebuffers() removes the underlying device and > returns the memory range to the system. > > [1] > https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > CC: stable@vger.kernel.org # v5.11+ Looks great, thanks! Reviewed-by: Zack Rusin <zackr@vmware.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal @ 2022-01-24 15:59 ` Zack Rusin 0 siblings, 0 replies; 32+ messages in thread From: Zack Rusin @ 2022-01-24 15:59 UTC (permalink / raw) To: airlied, tzimmermann, javierm, hdegoede, maarten.lankhorst, deller, daniel, mripard Cc: linux-fbdev, stable, dri-devel On Mon, 2022-01-24 at 13:36 +0100, Thomas Zimmermann wrote: > Hot-unplug all firmware-framebuffer devices as part of removing > them via remove_conflicting_framebuffers() et al. Releases all > memory regions to be acquired by native drivers. > > Firmware, such as EFI, install a framebuffer while posting the > computer. After removing the firmware-framebuffer device from fbdev, > a native driver takes over the hardware and the firmware framebuffer > becomes invalid. > > Firmware-framebuffer drivers, specifically simplefb, don't release > their device from Linux' device hierarchy. It still owns the firmware > framebuffer and blocks the native drivers from loading. This has been > observed in the vmwgfx driver. [1] > > Initiating a device removal (i.e., hot unplug) as part of > remove_conflicting_framebuffers() removes the underlying device and > returns the memory range to the system. > > [1] > https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > CC: stable@vger.kernel.org # v5.11+ Looks great, thanks! Reviewed-by: Zack Rusin <zackr@vmware.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2022-01-25 15:36 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-25 9:12 [PATCH 0/5] sysfb: Fix memory-region management Thomas Zimmermann 2022-01-25 9:12 ` Thomas Zimmermann 2022-01-25 9:12 ` [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal Thomas Zimmermann 2022-01-25 9:12 ` Thomas Zimmermann 2022-01-25 9:12 ` [PATCH 2/5] drivers/firmware: Don't mark as busy the simple-framebuffer IO resource Thomas Zimmermann 2022-01-25 9:12 ` Thomas Zimmermann 2022-01-25 9:12 ` [PATCH 3/5] drm/simpledrm: Request memory region in driver Thomas Zimmermann 2022-01-25 9:12 ` Thomas Zimmermann 2022-01-25 13:41 ` Jocelyn Falempe 2022-01-25 13:41 ` Jocelyn Falempe 2022-01-25 9:12 ` [PATCH 4/5] fbdev/simplefb: " Thomas Zimmermann 2022-01-25 9:12 ` Thomas Zimmermann 2022-01-25 9:33 ` Javier Martinez Canillas 2022-01-25 9:33 ` Javier Martinez Canillas 2022-01-25 9:12 ` [PATCH 5/5] drm: Add TODO item for requesting memory regions Thomas Zimmermann 2022-01-25 9:12 ` Thomas Zimmermann 2022-01-25 9:13 ` [PATCH 0/5] sysfb: Fix memory-region management Thomas Zimmermann 2022-01-25 9:13 ` Thomas Zimmermann 2022-01-25 15:34 ` Hans de Goede 2022-01-25 15:34 ` Hans de Goede -- strict thread matches above, loose matches on Subject: below -- 2022-01-24 12:36 Thomas Zimmermann 2022-01-24 12:36 ` [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal Thomas Zimmermann 2022-01-24 12:36 ` Thomas Zimmermann 2022-01-24 13:52 ` Javier Martinez Canillas 2022-01-24 13:52 ` Javier Martinez Canillas 2022-01-24 13:56 ` Javier Martinez Canillas 2022-01-24 13:56 ` Javier Martinez Canillas 2022-01-24 14:19 ` Thomas Zimmermann 2022-01-24 14:19 ` Thomas Zimmermann 2022-01-24 14:31 ` Javier Martinez Canillas 2022-01-24 14:31 ` Javier Martinez Canillas 2022-01-24 15:59 ` Zack Rusin 2022-01-24 15:59 ` Zack Rusin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.