Brian Norris briannorris@chromium.org writes:
Hello Brian,
(Tweaking subject; this indeed isn't related to the regression at all)
Hi,
On Mon, Sep 09, 2024 at 10:02:00AM +0200, Borislav Petkov wrote:
Looking at your log, the first warn is in framebuffer_coreboot. Some mess in the sysfs platform devices registration.
Adding the relevant people for that:
Aug 20 20:29:36 luna kernel: sysfs: cannot create duplicate filename '/bus/platform/devices/simple-framebuffer.0' Aug 20 20:29:36 luna kernel: CPU: 5 PID: 571 Comm: (udev-worker) Tainted: G OE 6.10.6-arch1-1 #1 703d152c24f1971e36f16e505405e456fc9e23f8 Aug 20 20:29:36 luna kernel: Hardware name: Purism Librem 14/Librem 14, BIOS 4.14-Purism-1 06/18/2021 Aug 20 20:29:36 luna kernel: Call Trace: Aug 20 20:29:36 luna kernel: <TASK> Aug 20 20:29:36 luna kernel: dump_stack_lvl+0x5d/0x80 Aug 20 20:29:36 luna kernel: sysfs_warn_dup.cold+0x17/0x23 Aug 20 20:29:36 luna kernel: sysfs_do_create_link_sd+0xcf/0xe0 Aug 20 20:29:36 luna kernel: bus_add_device+0x6b/0x130 Aug 20 20:29:36 luna kernel: device_add+0x3b3/0x870 Aug 20 20:29:36 luna kernel: platform_device_add+0xed/0x250 Aug 20 20:29:36 luna kernel: platform_device_register_full+0xbb/0x140 Aug 20 20:29:36 luna kernel: platform_device_register_resndata.constprop.0+0x54/0x80 [framebuffer_coreboot a587d2fc243ebaa0205c3badd33442a004d284e0] Aug 20 20:29:36 luna kernel: framebuffer_probe+0x165/0x1b0 [framebuffer_coreboot a587d2fc243ebaa0205c3badd33442a004d284e0] Aug 20 20:29:36 luna kernel: really_probe+0xdb/0x340 Aug 20 20:29:36 luna kernel: ? pm_runtime_barrier+0x54/0x90 Aug 20 20:29:36 luna kernel: ? __pfx___driver_attach+0x10/0x10 Aug 20 20:29:36 luna kernel: __driver_probe_device+0x78/0x110 Aug 20 20:29:36 luna kernel: driver_probe_device+0x1f/0xa0 Aug 20 20:29:36 luna kernel: __driver_attach+0xba/0x1c0 Aug 20 20:29:36 luna kernel: bus_for_each_dev+0x8c/0xe0 Aug 20 20:29:36 luna kernel: bus_add_driver+0x112/0x1f0 Aug 20 20:29:36 luna kernel: driver_register+0x72/0xd0 Aug 20 20:29:36 luna kernel: ? __pfx_framebuffer_driver_init+0x10/0x10 [framebuffer_coreboot a587d2fc243ebaa0205c3badd33442a004d284e0] Aug 20 20:29:36 luna kernel: do_one_initcall+0x58/0x310 Aug 20 20:29:36 luna kernel: do_init_module+0x60/0x220 Aug 20 20:29:36 luna kernel: init_module_from_file+0x89/0xe0 Aug 20 20:29:36 luna kernel: idempotent_init_module+0x121/0x320 Aug 20 20:29:36 luna kernel: __x64_sys_finit_module+0x5e/0xb0 Aug 20 20:29:36 luna kernel: do_syscall_64+0x82/0x190 Aug 20 20:29:36 luna kernel: ? __do_sys_newfstatat+0x3c/0x80 Aug 20 20:29:36 luna kernel: ? syscall_exit_to_user_mode+0x72/0x200 Aug 20 20:29:36 luna kernel: ? do_syscall_64+0x8e/0x190 Aug 20 20:29:36 luna kernel: ? do_sys_openat2+0x9c/0xe0 Aug 20 20:29:36 luna kernel: ? syscall_exit_to_user_mode+0x72/0x200 Aug 20 20:29:36 luna kernel: ? do_syscall_64+0x8e/0x190 Aug 20 20:29:36 luna kernel: ? clear_bhb_loop+0x25/0x80 Aug 20 20:29:36 luna kernel: ? clear_bhb_loop+0x25/0x80 Aug 20 20:29:36 luna kernel: ? clear_bhb_loop+0x25/0x80 Aug 20 20:29:36 luna kernel: entry_SYSCALL_64_after_hwframe+0x76/0x7e Aug 20 20:29:36 luna kernel: RIP: 0033:0x7b1bee2f81fd
Looks like it might be a conflict with drivers/firmware/sysfb_simplefb.c, which also uses the "simple-framebuffer" name with a constant ID of 0. It's possible both drivers should be switched to use PLATFORM_DEVID_AUTO? Or at least one of them. Or they should use different base names.
I'm unsure about PLATFORM_DEVID_AUTO because I don't know if there are user-space programs that assume this to always be "simple-framebuffer.0".
I'm not really sure what the best option is (does anyone rely on or care about the device naming?), and I don't actually use this driver. But here's an untested diff to try if you'd really like. If you test it, feel free to submit as a proper patch with my:
I've discussed this with Thomas Zimmermann (simpledrm maintainer) and he suggests that the problem is the system framebuffer information to be provided in both Coreboot table entry (AFAIU is LB_TAG_FRAMEBUFFER) and in the boot_params, which leads to struct screen_info to be filled.
We had the same problem for EFI systems that passed DTB to the kernel instead of ACPI, in those cases both a "simple-framebuffer" DT node and an EFI-GOP table could be provided.
Commit 3310288f6135 "(of/platform: Disable sysfb if a simple-framebuffer node is found") solved that issue. I've typed the same for Coreboot to handle in the same way. Please let me know what you think:
From 6955149fb13af1c0cba2e5c1fbb1ac9367a09ae2 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas javierm@redhat.com Date: Thu, 12 Sep 2024 12:55:29 +0200 Subject: [RFC PATCH] firmware: coreboot: Disable sysfb if Coreboot already provides a FB
On Coreboot platforms, a system framebuffer may be provided to the Linux kernel by filling a LB_TAG_FRAMEBUFFER entry in the Coreboot table. But it seems SeaBIOS payload can also provide a VGA mode in the boot params.
If that the case, early arch x86 boot code will fill the global struct screen_info data.
The data is used by the Generic System Framebuffers (sysfb) framework to add a platform device with platform data about the system framebuffer.
But if there is information about the system framebuffer in the Coreboot table as well, the framebuffer_coreboot driver will also try to do the same and add another device for the system framebuffer. This will fail though because there's already a simple-framebuffer.0 device registered:
sysfs: cannot create duplicate filename '/bus/platform/devices/simple-framebuffer.0' ... coreboot: could not register framebuffer framebuffer coreboot8: probe with driver framebuffer failed with error -17
To prevent the issue, make the framebuffer_core driver to disable sysfb if there is system framebuffer data in the Coreboot table. That way only this driver will register a device and sysfb would not attempt to do it (or remove its registered device if was already executed before).
Reported-by: Brian Norris briannorris@chromium.org Link: https://lore.kernel.org/all/ZuCG-DggNThuF4pj@b20ea791c01f/T/#ma7fb65acbc1a56... Suggested-by: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Javier Martinez Canillas javierm@redhat.com --- drivers/firmware/google/framebuffer-coreboot.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c index daadd71d8ddd..0a28aa5b17dc 100644 --- a/drivers/firmware/google/framebuffer-coreboot.c +++ b/drivers/firmware/google/framebuffer-coreboot.c @@ -61,6 +61,19 @@ static int framebuffer_probe(struct coreboot_device *dev) if (res.end <= res.start) return -EINVAL;
+ /* + * Since a "simple-framebuffer" device is already added + * here, disable the Generic System Framebuffers (sysfb) + * to prevent it from registering another device for the + * system framebuffer later (e.g: using the screen_info + * data that may had been filled as well). + * + * This can happen for example on Coreboot systems, that + * advertise a LB_TAG_FRAMEBUFFER entry in their Coreboot + * table and also a VESA mode by the BIOS used as payload. + */ + sysfb_disable(); + pdev = platform_device_register_resndata(&dev->dev, "simple-framebuffer", 0, &res, 1, &pdata,