ACPI: PRM: Check whether EFI runtime is available

PATCH In review Ard Biesheuvel 2 participants 4 messages Jan 12, 2023 – Jan 18, 2023

Subsystems (3)

Stable branch ACPI EFI

lore

Share & tools

lore

Ard Biesheuvel

author maintainer

2023-01-12 13:33 UTC #0 lore raw

PATCH v2 1/1 ACPI: PRM: Check whether EFI runtime is available

The ACPI PRM address space handler calls efi_call_virt_pointer() to execute PRM firmware code, but doing so is only permitted when the EFI runtime environment is available. Otherwise, such calls are guaranteed to result in a crash, and must therefore be avoided.

Given that the EFI runtime services may become unavailable after a crash occurring in the firmware, we need to check this each time the PRM address space handler is invoked. If the EFI runtime services were not available at registration time to being with, don't install the address space handler at all.

Cc <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Len Brown <[email protected]>

Cc [email protected]

Signed-off-by Ard Biesheuvel <[email protected]>

---
v2: check both at registration and at invocation time

 drivers/acpi/prmt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/acpi/prmt.c +10 -0

index 998101cf16e47145..3d4c4620f9f95309 100644
@@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,
 	efi_status_t status;
 	struct prm_context_buffer context;
 
+	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+		pr_err_ratelimited("PRM: EFI runtime services no longer available\n");
+		return AE_NO_HANDLER;
+	}
+
 	/*
 	 * The returned acpi_status will always be AE_OK. Error values will be
 	 * saved in the first byte of the PRM message buffer to be used by ASL.

@@ -325,6 +330,11 @@ void __init init_prmt(void)
 
 	pr_info("PRM: found %u modules\n", mc);
 
+	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+		pr_err("PRM: EFI runtime services unavailable\n");
+		return;
+	}
+
 	status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT,
 						    ACPI_ADR_SPACE_PLATFORM_RT,
 						    &acpi_platformrt_space_handler,

Signature · 1 line

2.39.0

Rafael J. Wysocki

maintainer

2023-01-17 12:29 UTC #1 lore raw

Quoted 28 lines · Ard Biesheuvel

The ACPI PRM address space handler calls efi_call_virt_pointer() to execute PRM firmware code, but doing so is only permitted when the EFI runtime environment is available. Otherwise, such calls are guaranteed to result in a crash, and must therefore be avoided.
Given that the EFI runtime services may become unavailable after a crash occurring in the firmware, we need to check this each time the PRM address space handler is invoked. If the EFI runtime services were not available at registration time to being with, don't install the address space handler at all.
Cc <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
Cc [email protected]
Signed-off-by Ard Biesheuvel <[email protected]>
---
v2: check both at registration and at invocation time
 drivers/acpi/prmt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/acpi/prmt.c +0 -0
index 998101cf16e47145..3d4c4620f9f95309 100644

drivers/acpi/prmt.c static acpi_status acpi_platformrt_space_handler(u32 function, · line 241

Quoted 6 lines

drivers/acpi/prmt.c +3 -0

@@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,
        efi_status_t status;
        struct prm_context_buffer context;
 
+       if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+               pr_err_ratelimited("PRM: EFI runtime services no longer available\n");
+               return AE_NO_HANDLER;

This error code is only used in GPE handling ATM.

The one that actually causes ACPICA to log a "no handler" error (in acpi_ex_access_region()) is AE_NOT_EXIST. Should it be used here?

Quoted 20 lines

drivers/acpi/prmt.c +7 -1

+       }
+
        /*
         * The returned acpi_status will always be AE_OK. Error values will be
         * saved in the first byte of the PRM message buffer to be used by ASL.

@@ -325,6 +330,11 @@ void __init init_prmt(void)
 
        pr_info("PRM: found %u modules\n", mc);
 
+       if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+               pr_err("PRM: EFI runtime services unavailable\n");
+               return;
+       }
+
        status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT,
                                                    ACPI_ADR_SPACE_PLATFORM_RT,
                                                    &acpi_platformrt_space_handler,
--

2.39.0

Ard Biesheuvel

author maintainer

2023-01-17 15:52 UTC #2 lore raw

Quoted 33 lines · Rafael J. Wysocki

Quoted 31 lines · Ard Biesheuvel
The ACPI PRM address space handler calls efi_call_virt_pointer() to execute PRM firmware code, but doing so is only permitted when the EFI runtime environment is available. Otherwise, such calls are guaranteed to result in a crash, and must therefore be avoided.
Given that the EFI runtime services may become unavailable after a crash occurring in the firmware, we need to check this each time the PRM address space handler is invoked. If the EFI runtime services were not available at registration time to being with, don't install the address space handler at all.
Cc <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
Cc [email protected]
Signed-off-by Ard Biesheuvel <[email protected]>
---
v2: check both at registration and at invocation time
 drivers/acpi/prmt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/acpi/prmt.c +0 -0
index 998101cf16e47145..3d4c4620f9f95309 100644
@@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,
        efi_status_t status;
        struct prm_context_buffer context;
 

Quoted 9 lines

drivers/acpi/prmt.c static acpi_status acpi_platformrt_space_handler(u32 function, · line 241
Quoted 3 lines
drivers/acpi/prmt.c +3 -0
+       if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+               pr_err_ratelimited("PRM: EFI runtime services no longer available\n");
+               return AE_NO_HANDLER;
This error code is only used in GPE handling ATM.
The one that actually causes ACPICA to log a "no handler" error (in acpi_ex_access_region()) is AE_NOT_EXIST. Should it be used here?

Not sure. Any error value is returned to the caller, the only difference is that AE_NOT_EXIST and AE_NOT_IMPLEMENTED trigger the non-ratelimited logging machinery.

Given that neither value seems appropriate (the region is implemented and it has a handler), and we already emit a rate limited error message, I think AE_NOT_EXIST is not the right choice.

Quoted 21 lines

drivers/acpi/prmt.c +7 -1

+       }
+
        /*
         * The returned acpi_status will always be AE_OK. Error values will be
         * saved in the first byte of the PRM message buffer to be used by ASL.

@@ -325,6 +330,11 @@ void __init init_prmt(void)
 
        pr_info("PRM: found %u modules\n", mc);
 
+       if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+               pr_err("PRM: EFI runtime services unavailable\n");
+               return;
+       }
+
        status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT,
                                                    ACPI_ADR_SPACE_PLATFORM_RT,
                                                    &acpi_platformrt_space_handler,
--

2.39.0

Rafael J. Wysocki

maintainer

2023-01-18 19:36 UTC #3 lore raw

Quoted 45 lines · Ard Biesheuvel

Quoted 33 lines · Rafael J. Wysocki
Quoted 31 lines · Ard Biesheuvel
The ACPI PRM address space handler calls efi_call_virt_pointer() to execute PRM firmware code, but doing so is only permitted when the EFI runtime environment is available. Otherwise, such calls are guaranteed to result in a crash, and must therefore be avoided.
Given that the EFI runtime services may become unavailable after a crash occurring in the firmware, we need to check this each time the PRM address space handler is invoked. If the EFI runtime services were not available at registration time to being with, don't install the address space handler at all.
Cc <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
Cc [email protected]
Signed-off-by Ard Biesheuvel <[email protected]>
---
v2: check both at registration and at invocation time
 drivers/acpi/prmt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/acpi/prmt.c +0 -0
index 998101cf16e47145..3d4c4620f9f95309 100644
@@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,
        efi_status_t status;
        struct prm_context_buffer context;
 
Quoted 9 lines
drivers/acpi/prmt.c static acpi_status acpi_platformrt_space_handler(u32 function, · line 241
Quoted 3 lines
drivers/acpi/prmt.c +3 -0
+       if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+               pr_err_ratelimited("PRM: EFI runtime services no longer available\n");
+               return AE_NO_HANDLER;
This error code is only used in GPE handling ATM.
The one that actually causes ACPICA to log a "no handler" error (in acpi_ex_access_region()) is AE_NOT_EXIST. Should it be used here?

Quoted 7 lines

Not sure. Any error value is returned to the caller, the only difference is that AE_NOT_EXIST and AE_NOT_IMPLEMENTED trigger the non-ratelimited logging machinery.
Given that neither value seems appropriate (the region is implemented and it has a handler), and we already emit a rate limited error message, I think AE_NOT_EXIST is not the right choice.

OK, applied as-is as 6.2-rc material, thanks!