linux-kernel - Re: [PATCH v3] ACPI: NFIT: core: Convert the driver to a platform one

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <69408df0c974d_10c0751008f@iweiny-mobl.notmuch>
Date: Mon, 15 Dec 2025 16:38:40 -0600
From: Ira Weiny <ira.weiny@...el.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>, Linux ACPI
	<linux-acpi@...r.kernel.org>, Ira Weiny <ira.weiny@...el.com>
CC: LKML <linux-kernel@...r.kernel.org>, Linux PM <linux-pm@...r.kernel.org>,
	Armin Wolf <w_armin@....de>, Hans de Goede <hansg@...nel.org>, Dan Williams
	<dan.j.williams@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, "Dave
 Jiang" <dave.jiang@...el.com>, Ira Weiny <ira.weiny@...el.com>,
	<nvdimm@...ts.linux.dev>
Subject: Re: [PATCH v3] ACPI: NFIT: core: Convert the driver to a platform one

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 <rafael.j.wysocki@...el.com>
> > > > 
> > > > 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/20231011083334.3987477-6-michal.wilczynski@intel.com/ [1]
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > > ---
> > > >  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 <rafael.j.wysocki@...el.com>
> 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/20231011083334.3987477-6-michal.wilczynski@intel.com/ [1]
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

Thanks!  This version passes the tests!

Acked-by: Ira Weiny <ira.weiny@...el.com>
Tested-by: Ira Weiny <ira.weiny@...el.com>

[snip]