| Message ID | 20230112133319.3615177-1-ardb@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [v2] ACPI: PRM: Check whether EFI runtime is available | expand |
On Thu, Jan 12, 2023 at 2:33 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > 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: <stable@vger.kernel.org> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Len Brown <lenb@kernel.org> > Cc: linux-acpi@vger.kernel.org > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > v2: check both at registration and at invocation time > > drivers/acpi/prmt.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c > index 998101cf16e47145..3d4c4620f9f95309 100644 > --- a/drivers/acpi/prmt.c > +++ b/drivers/acpi/prmt.c > @@ -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? > + } > + > /* > * 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 >
On Tue, 17 Jan 2023 at 13:29, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Jan 12, 2023 at 2:33 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > 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: <stable@vger.kernel.org> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > > Cc: Len Brown <lenb@kernel.org> > > Cc: linux-acpi@vger.kernel.org > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > v2: check both at registration and at invocation time > > > > drivers/acpi/prmt.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c > > index 998101cf16e47145..3d4c4620f9f95309 100644 > > --- a/drivers/acpi/prmt.c > > +++ b/drivers/acpi/prmt.c > > @@ -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? > 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. > > > + } > > + > > /* > > * 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 > >
On Tue, Jan 17, 2023 at 4:51 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 17 Jan 2023 at 13:29, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Thu, Jan 12, 2023 at 2:33 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > 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: <stable@vger.kernel.org> > > > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > > > Cc: Len Brown <lenb@kernel.org> > > > Cc: linux-acpi@vger.kernel.org > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > --- > > > v2: check both at registration and at invocation time > > > > > > drivers/acpi/prmt.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c > > > index 998101cf16e47145..3d4c4620f9f95309 100644 > > > --- a/drivers/acpi/prmt.c > > > +++ b/drivers/acpi/prmt.c > > > @@ -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? > > > > 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!
diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c index 998101cf16e47145..3d4c4620f9f95309 100644 --- a/drivers/acpi/prmt.c +++ b/drivers/acpi/prmt.c @@ -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,
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: <stable@vger.kernel.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Cc: Len Brown <lenb@kernel.org> Cc: linux-acpi@vger.kernel.org Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- v2: check both at registration and at invocation time drivers/acpi/prmt.c | 10 ++++++++++ 1 file changed, 10 insertions(+)