On Tue, Jan 17, 2023 at 4:51 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> On Tue, 17 Jan 2023 at 13:29, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
> >
> > On Thu, Jan 12, 2023 at 2:33 PM Ard Biesheuvel <ardb@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> > > Cc: Len Brown <lenb@xxxxxxxxxx>
> > > Cc: linux-acpi@xxxxxxxxxxxxxxx
> > > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > ---
> > > 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!
