ACPI: PRM: Check whether EFI runtime is available
Subsystems (3)
Ard Biesheuvel
maintainerPATCH 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.
--- drivers/acpi/prmt.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/acpi/prmt.c +5 -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 ("PRM: EFI runtime services unavailable\n" ); +return AE_NOT_IMPLEMENTED ; + } +/* * 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.
Signature · 1 line
2.39.0
Rafael J. Wysocki
maintainerQuoted 20 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.Cc <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Len Brown <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/prmt.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/acpi/prmt.c +0
-0
drivers/acpi/prmt.c static acpi_status acpi_platformrt_space_handler(u32 function, · line 243
Quoted 8 lines
drivers/acpi/prmt.c +5
-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 ("PRM: EFI runtime services unavailable\n" ); +return AE_NOT_IMPLEMENTED ; + } +
Does the check need to be made in the address space handler and if so, then why?
It looks like it would be better to prevent it from being installed if EFI runtime services are not enabled in the first place, in init_prmt().
Quoted 4 lines
drivers/acpi/prmt.c +0
-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. --
Ard Biesheuvel
maintainerQuoted 25 lines · Rafael J. Wysocki
Quoted 23 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.Cc <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Len Brown <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/prmt.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/acpi/prmt.c +0
-0@@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,efi_status_t status;struct prm_context_buffer context;
Quoted 8 lines
drivers/acpi/prmt.c static acpi_status acpi_platformrt_space_handler(u32 function, · line 243
Quoted 5 lines
drivers/acpi/prmt.c +5
-0+if (!efi_enabled (EFI_RUNTIME_SERVICES )) { +pr_err ("PRM: EFI runtime services unavailable\n" ); +return AE_NOT_IMPLEMENTED ; + } +Does the check need to be made in the address space handler and if so, then why?
Yes. efi_enabled(EFI_RUNTIME_SERVICES) will transition from true to false if an exception occurs while executing the firmware code.
Unlike the EFI variable runtime services, which are quite uniform, this PRM code will be vendor specific, and so the likelihood that it is buggy and only tested with Windows is much higher, and so I would like us to be more cautious here.
Quoted 4 lines
It looks like it would be better to prevent it from being installed if EFI runtime services are not enabled in the first place, in
init_prmt().
We could add another check there as well, yes. And perhaps the one in the handler should we pr_warn_once() to prevent it from firing repeatedly.
Quoted 4 lines
Quoted 4 lines
drivers/acpi/prmt.c +0
-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. --
Rafael J. Wysocki
maintainerQuoted 36 lines · Ard Biesheuvel
Quoted 25 lines · Rafael J. Wysocki
Quoted 23 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.Cc <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Len Brown <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/prmt.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/acpi/prmt.c +0
-0@@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,efi_status_t status;struct prm_context_buffer context;Quoted 8 lines
drivers/acpi/prmt.c static acpi_status acpi_platformrt_space_handler(u32 function, · line 243
Quoted 5 lines
drivers/acpi/prmt.c +5
-0+if (!efi_enabled (EFI_RUNTIME_SERVICES )) { +pr_err ("PRM: EFI runtime services unavailable\n" ); +return AE_NOT_IMPLEMENTED ; + } +Does the check need to be made in the address space handler and if so, then why?
Quoted 2 lines
Yes.
efi_enabled(EFI_RUNTIME_SERVICES)will transition from true to false if an exception occurs while executing the firmware code.
OK
Quoted 4 lines
Unlike the EFI variable runtime services, which are quite uniform, this PRM code will be vendor specific, and so the likelihood that it is buggy and only tested with Windows is much higher, and so I would like us to be more cautious here.
OK
Quoted 8 lines
Quoted 4 lines
It looks like it would be better to prevent it from being installed if EFI runtime services are not enabled in the first place, in
init_prmt().We could add another check there as well, yes. And perhaps the one in the handler should we
pr_warn_once()to prevent it from firing repeatedly.
Sounds good to me, so are you going to send an update of the patch? Or how would you like to proceed otherwise?
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/acpi/prmt.c +0
-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. --
Ard Biesheuvel
maintainerQuoted 55 lines · Rafael J. Wysocki
Quoted 36 lines · Ard Biesheuvel
Quoted 25 lines · Rafael J. Wysocki
Quoted 23 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.Cc <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Len Brown <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/prmt.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/acpi/prmt.c +0
-0@@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,efi_status_t status;struct prm_context_buffer context;Quoted 8 lines
drivers/acpi/prmt.c static acpi_status acpi_platformrt_space_handler(u32 function, · line 243
Quoted 5 lines
drivers/acpi/prmt.c +5
-0+if (!efi_enabled (EFI_RUNTIME_SERVICES )) { +pr_err ("PRM: EFI runtime services unavailable\n" ); +return AE_NOT_IMPLEMENTED ; + } +Does the check need to be made in the address space handler and if so, then why?
Quoted 2 lines
Yes.
efi_enabled(EFI_RUNTIME_SERVICES)will transition from true to false if an exception occurs while executing the firmware code.OK
Quoted 4 lines
Unlike the EFI variable runtime services, which are quite uniform, this PRM code will be vendor specific, and so the likelihood that it is buggy and only tested with Windows is much higher, and so I would like us to be more cautious here.
OK
Quoted 5 lines
Quoted 4 lines
It looks like it would be better to prevent it from being installed if EFI runtime services are not enabled in the first place, in
init_prmt().
Quoted 7 lines
Quoted 3 lines
We could add another check there as well, yes. And perhaps the one in the handler should we
pr_warn_once()to prevent it from firing repeatedly.Sounds good to me, so are you going to send an update of the patch? Or how would you like to proceed otherwise?
I'll respin it.