Rafael J. Wysocki wrote: > On Sunday, December 14, 2025 3:25:04 PM CET Rafael J. Wysocki wrote: > > On Saturday, December 13, 2025 12:04:02 AM CET Ira Weiny wrote: > > > Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki <[email protected]> > > > > > > > > While binding drivers directly to struct acpi_device objects allows > > > > basic functionality to be provided, at least in the majority of cases, > > > > there are some problems with it, related to general consistency, sysfs > > > > layout, power management operation ordering, and code cleanliness. > > > > > > > > Overall, it is better to bind drivers to platform devices than to their > > > > ACPI companions, so convert the ACPI NFIT core driver to a platform one. > > > > > > > > While this is not expected to alter functionality, it changes sysfs > > > > layout and so it will be visible to user space. > > > > > > I'm not sure right off why but when I run the libndctl test with this > > > patch I > > > get the following panic. > > > > > > [ 17.483472] BUG: kernel NULL pointer dereference, address: > > > 0000000000000008 > > > [ 17.484116] #PF: supervisor read access in kernel mode > > > [ 17.484593] #PF: error_code(0x0000) - not-present page > > > [ 17.485056] PGD 9def067 P4D 9def067 PUD 9df3067 PMD 0 > > > [ 17.485516] Oops: Oops: 0000 [#1] SMP NOPTI > > > [ 17.485886] CPU: 2 UID: 0 PID: 1191 Comm: libndctl Tainted: G > > > O 6.18.0ira+ #34 PREEMPT(voluntary) > > > [ 17.486804] Tainted: [O]=OOT_MODULE > > > [ 17.487125] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > > > edk2-20250812-18.fc42 08/12/2025 > > > [ 17.487749] RIP: 0010:acpi_nfit_ctl+0x40b/0xa00 [nfit] > > > [ 17.488222] Code: 48 48 c7 44 24 28 28 f1 8c a1 48 8b 83 c8 01 00 00 > > > 44 89 e7 48 89 44 24 50 e8 01 83 fd ff 48 c7 44 24 40 10 58 8c a1 48 89 > > > c3 <49> 8b 47 08 48 c7 44 24 30 30 f1 8c a1 48 89 44 24 18 e9 24 fd > > > ff > > > [ 17.491668] RSP: 0018:ffffc9000f11ba28 EFLAGS: 00010286 > > > [ 17.492422] RAX: ffffffffa18679f0 RBX: ffffffffa18679f0 RCX: > > > ffffc9000f11bb40 > > > [ 17.492903] RDX: 000000000000041e RSI: ffffffffa18cf116 RDI: > > > 0000000000000003 > > > [ 17.493408] RBP: ffffc9000f11bb40 R08: 0000000000000008 R09: > > > ffffc9000f11bafc > > > [ 17.493888] R10: ffffc9000f11bae0 R11: 0000000000004019 R12: > > > 0000000000000003 > > > [ 17.494396] R13: 0000000000000000 R14: 0000000000000000 R15: > > > 0000000000000000 > > > [ 17.494878] FS: 00007f432f5fd7c0(0000) GS:ffff8880f9fdd000(0000) > > > knlGS:0000000000000000 > > > [ 17.495436] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > [ 17.495826] CR2: 0000000000000008 CR3: 0000000009e0c005 CR4: > > > 0000000000770ef0 > > > [ 17.496324] PKRU: 55555554 > > > [ 17.496516] Call Trace: > > > [ 17.496691] <TASK> > > > [ 17.496844] ? __kmalloc_noprof+0x410/0x650 > > > [ 17.497138] ? setup_result+0x1b/0xa0 [nfit_test] > > > [ 17.497474] nfit_ctl_test+0x21a/0x780 [nfit_test] > > > [ 17.497803] ? preempt_count_add+0x51/0xd0 > > > [ 17.498086] ? up_write+0x13/0x60 > > > [ 17.498333] ? up_write+0x35/0x60 > > > [ 17.498565] ? preempt_count_add+0x51/0xd0 > > > [ 17.498846] ? kernfs_next_descendant_post+0x1b/0xe0 > > > [ 17.499196] nfit_test_probe+0x350/0x4d0 [nfit_test] > > > [ 17.499535] platform_probe+0x38/0x70 > > > [ 17.499791] really_probe+0xde/0x380 > > > [ 17.500039] ? _raw_spin_unlock_irq+0x18/0x40 > > > [ 17.500354] __driver_probe_device+0xc0/0x150 > > > [ 17.500656] driver_probe_device+0x1f/0xa0 > > > > > > [ > > > 17.500939] ? __pfx___driver_attach+0x10/0x10 > > > [ 17.501263] __driver_attach+0xc7/0x200 > > > [ 17.501529] bus_for_each_dev+0x63/0xa0 > > > [ 17.501794] bus_add_driver+0x114/0x200 > > > [ 17.502059] driver_register+0x71/0xe0 > > > [ 17.502480] nfit_test_init+0x24e/0xff0 [nfit_test] > > > [ 17.502956] ? __pfx_nfit_test_init+0x10/0x10 [nfit_test] > > > [ 17.503483] do_one_initcall+0x42/0x210 > > > [ 17.503891] do_init_module+0x62/0x230 > > > [ 17.504296] init_module_from_file+0xb1/0xe0 > > > [ 17.504732] idempotent_init_module+0xed/0x2d0 > > > [ 17.505184] __x64_sys_finit_module+0x6d/0xe0 > > > [ 17.505626] do_syscall_64+0x62/0x390 > > > [ 17.506016] entry_SYSCALL_64_after_hwframe+0x76/0x7e > > > [ 17.506563] RIP: 0033:0x7f432f8920cd > > > [ 17.506946] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa > > > 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f > > > 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 03 4d 0f 00 f7 d8 64 89 01 > > > 48 > > > [ 17.508548] RSP: 002b:00007fff0a6ccd98 EFLAGS: 00000246 ORIG_RAX: > > > 0000000000000139 > > > [ 17.509209] RAX: ffffffffffffffda RBX: 000000001f5def50 RCX: > > > 00007f432f8920cd > > > [ 17.509831] RDX: 0000000000000000 RSI: 00007f432f9aa965 RDI: > > > 0000000000000003 > > > > > > [ 17.510472] RBP: 00007fff0a6cce50 R08: 0000000000000000 R09: > > > 00007fff0a6cce00 > > > [ 17.511091] R10: 0000000000000000 R11: 0000000000000246 R12: > > > 0000000000020000 > > > [ 17.511727] R13: 000000001f5deb60 R14: 00007f432f9aa965 R15: > > > 0000000000000000 > > > [ 17.512353] </TASK> > > > [ 17.512638] Modules linked in: nfit_test(O+) device_dax(O) nd_pmem(O) > > > dax_pmem(O) kmem nd_btt(O) nfit(O) dax_cxl cxl_pci nd_e820(O) > > > cxl_mock_mem(O) cxl_test(O) cxl_mem(O) cxl_pmem(O) cxl_acpi(O) > > > cxl_port(O) cx > > > l_mock(O) libnvdimm(O) nfit_test_iomap(O) cxl_core(O) fwctl > > > [ 17.514512] CR2: 0000000000000008 > > > [ 17.514878] ---[ end trace 0000000000000000 ]--- > > > > > > > > > I'll try and find some time to dig into it but perhaps yall have a quick > > > idea of what it could be? > > > > > > Ira > > > > > > > > > > > This change was mostly developed by Michal Wilczynski [1]. > > > > > > > > Linu: > > > > https://lore.kernel.org/linux-acpi/[email protected]/ > > > > [1] > > > > Signed-off-by: Rafael J. Wysocki <[email protected]> > > > > --- > > > > drivers/acpi/nfit/core.c | 34 ++++++++++++++++++---------------- > > > > 1 file changed, 18 insertions(+), 16 deletions(-) > > > > > > > > --- a/drivers/acpi/nfit/core.c > > > > +++ b/drivers/acpi/nfit/core.c > > > > @@ -2,6 +2,7 @@ > > > > /* > > > > * Copyright(c) 2013-2015 Intel Corporation. All rights reserved. > > > > */ > > > > +#include <linux/platform_device.h> > > > > #include <linux/list_sort.h> > > > > #include <linux/libnvdimm.h> > > > > #include <linux/module.h> > > > > @@ -98,7 +99,7 @@ static struct acpi_device *to_acpi_dev(s > > > > || strcmp(nd_desc->provider_name, "ACPI.NFIT") > > > > != 0) > > > > return NULL; > > > > > > > > - return to_acpi_device(acpi_desc->dev); > > > > + return ACPI_COMPANION(acpi_desc->dev); > > > > } > > > > It's likely this change and it is not even necessary. > > > > If possible, please check the v2 below. > > Well, scratch this, it was a mistake. > > The original patch was almost there AFAICS, but it overlooked the fact that > nfit_ctl_test() could create an acpi_desc with dev pointing to an artificial > struct acpi_device. > > So to_acpi_dev() needs to check if the ACPI_COMPANION() is there and fall back > to acpi_desc->dev if that is not the case. > > --- > From: Rafael J. Wysocki <[email protected]> > Subject: [PATCH v3] ACPI: NFIT: core: Convert the driver to a platform one > > While binding drivers directly to struct acpi_device objects allows > basic functionality to be provided, at least in the majority of cases, > there are some problems with it, related to general consistency, sysfs > layout, power management operation ordering, and code cleanliness. > > Overall, it is better to bind drivers to platform devices than to their > ACPI companions, so convert the ACPI NFIT core driver to a platform one. > > While this is not expected to alter functionality, it changes sysfs > layout and so it will be visible to user space. > > This change was mostly developed by Michal Wilczynski [1]. > > Linu: > https://lore.kernel.org/linux-acpi/[email protected]/ > [1] > Signed-off-by: Rafael J. Wysocki <[email protected]>
Thanks! This version passes the tests! Acked-by: Ira Weiny <[email protected]> Tested-by: Ira Weiny <[email protected]> [snip]
