Re: [PATCH v1] efi/runtime-wrappers: Avoid crashing on early PRM code invocations

Rafael J. Wysocki posted 1 patch 6 days, 8 hours ago
drivers/acpi/bus.c         |    8 +++++++-
drivers/firmware/efi/efi.c |   16 +++++++++++++++-
include/acpi/acpi_bus.h    |    4 ++++
3 files changed, 26 insertions(+), 2 deletions(-)
Re: [PATCH v1] efi/runtime-wrappers: Avoid crashing on early PRM code invocations
Posted by Rafael J. Wysocki 6 days, 8 hours ago
On Friday, May 15, 2026 7:29:03 PM CEST Ard Biesheuvel wrote:
> Hi Rafael,
> 
> On Fri, 15 May 2026, at 19:10, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >
> > There is a dependency between EFI and ACPI PRM that the latter cannot
> > run until the former is ready and PRM can be invoked from AML early
> > through acpi_platformrt_space_handler().  If that happens before
> > initializing efi_rts_wq, it leads to a NULL pointer dereference.
> >
> > Avoid that by adding an efi_rts_wq check against NULL to
> > efi_call_acpi_prm_handler().
> >
> > Fixes: 5894cf571e14 ("acpi/prmt: Use EFI runtime sandbox to invoke PRM 
> > handlers")
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: 6.6+ <stable@vger.kernel.org> # 6.6+
> > ---
> >
> > An alternative would be to somehow ensure that efisubsys_init() will always
> > run before acpi_init(), but moving any of them to another initcall level is
> > not an option AFAICS.
> >
> 
> Given that they both run as subsys_initcall() currently, changing acpi_init()
> to subsys_initcall_sync() is probably fine (famous last words :-))

Well, not quite because there is stuff depending on ACPI in that initcall level
(MWI and CXL, probably among other things).

> But if the PRM code can deal with EFI_NOT_READY than this is also fine,
> modulo the comment below.

It can, but that is not ideal because if EFI_NOT_READY is returned, so AML will
be aborted and that may be something like a GPE handler method, so it would be
better to ensure the working order.

Something like the patch below can be done.  It works AFAICS, but the initcall
error handling is a bit awkward (it is for debug though, so not a big deal I
guess).  If this is acceptable, I can send it as a v2.

---
 drivers/acpi/bus.c         |    8 +++++++-
 drivers/firmware/efi/efi.c |   16 +++++++++++++++-
 include/acpi/acpi_bus.h    |    4 ++++
 3 files changed, 26 insertions(+), 2 deletions(-)

--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1490,7 +1490,7 @@ EXPORT_SYMBOL_GPL(acpi_kobj);
 
 void __weak __init acpi_arch_init(void) { }
 
-static int __init acpi_init(void)
+int __init acpi_init(void)
 {
 	int result;
 
@@ -1531,4 +1531,10 @@ static int __init acpi_init(void)
 	return 0;
 }
 
+/*
+ * If EFI is enabled, it needs to initialize before ACPI due to a PRM
+ * dependency, so in that case acpi_init() will be called from there.
+ */
+#ifndef CONFIG_EFI
 subsys_initcall(acpi_init);
+#endif
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -500,7 +500,21 @@ err_destroy_wq:
 	return error;
 }
 
-subsys_initcall(efisubsys_init);
+static int __init efisubsys_and_acpi_init(void)
+{
+	int efi_ret, acpi_ret;
+
+	efi_ret = efisubsys_init();
+
+	/* Initialize ACPI after EFI to satisfy a PRM dependency. */
+	acpi_ret = acpi_init();
+
+	if (efi_ret)
+		return efi_ret;
+
+	return acpi_ret;
+}
+subsys_initcall(efisubsys_and_acpi_init);
 
 void __init efi_find_mirror(void)
 {
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -92,6 +92,8 @@ bool acpi_reduced_hardware(void);
 
 #ifdef CONFIG_ACPI
 
+int __init acpi_init(void);
+
 struct proc_dir_entry;
 
 #define ACPI_BUS_FILE_ROOT	"acpi"
@@ -994,6 +996,8 @@ int acpi_scan_add_dep(acpi_handle handle
 u32 arch_acpi_add_auto_dep(acpi_handle handle);
 #else	/* CONFIG_ACPI */
 
+static inline int acpi_init(void) { return 0; }
+
 static inline int register_acpi_bus_type(void *bus) { return 0; }
 static inline int unregister_acpi_bus_type(void *bus) { return 0; }
Re: [PATCH v1] efi/runtime-wrappers: Avoid crashing on early PRM code invocations
Posted by Ard Biesheuvel 6 days, 7 hours ago
On Mon, 18 May 2026, at 21:29, Rafael J. Wysocki wrote:
> On Friday, May 15, 2026 7:29:03 PM CEST Ard Biesheuvel wrote:
>> Hi Rafael,
>> 
>> On Fri, 15 May 2026, at 19:10, Rafael J. Wysocki wrote:
>> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>> >
>> > There is a dependency between EFI and ACPI PRM that the latter cannot
>> > run until the former is ready and PRM can be invoked from AML early
>> > through acpi_platformrt_space_handler().  If that happens before
>> > initializing efi_rts_wq, it leads to a NULL pointer dereference.
>> >
>> > Avoid that by adding an efi_rts_wq check against NULL to
>> > efi_call_acpi_prm_handler().
>> >
>> > Fixes: 5894cf571e14 ("acpi/prmt: Use EFI runtime sandbox to invoke PRM 
>> > handlers")
>> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> > Cc: 6.6+ <stable@vger.kernel.org> # 6.6+
>> > ---
>> >
>> > An alternative would be to somehow ensure that efisubsys_init() will always
>> > run before acpi_init(), but moving any of them to another initcall level is
>> > not an option AFAICS.
>> >
>> 
>> Given that they both run as subsys_initcall() currently, changing acpi_init()
>> to subsys_initcall_sync() is probably fine (famous last words :-))
>
> Well, not quite because there is stuff depending on ACPI in that initcall level
> (MWI and CXL, probably among other things).
>

Oh right.

>> But if the PRM code can deal with EFI_NOT_READY than this is also fine,
>> modulo the comment below.
>
> It can, but that is not ideal because if EFI_NOT_READY is returned, so AML will
> be aborted and that may be something like a GPE handler method, so it would be
> better to ensure the working order.
>
> Something like the patch below can be done.  It works AFAICS, but the initcall
> error handling is a bit awkward (it is for debug though, so not a big deal I
> guess).  If this is acceptable, I can send it as a v2.
>

I'd rather fix this more comprehensively if we can, by either
- moving the workqueue allocation into a separate initcall that executes
before subsys, or
- omitting the workqueue for early calls, and dispatching the PRM calls
directly from the calling thread, similar to how ResetSystem() is routed.

As far as I can tell, option #1 (which I prefer) is feasible because the
actual setup of the EFI runtime environment occurs way earlier, and only
the allocation of the workqueue is missing. Could you please verify if
that would fix this?