RE: [PATCH] acpica: clear global_lock bits at FACS initialization

[Kaneda, Erik]

> -----Original Message-----
> From: linux-acpi-owner@xxxxxxxxxxxxxxx <linux-acpi-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Rafael J. Wysocki
> Sent: Wednesday, April 1, 2020 2:12 AM
> To: Jan Engelhardt <jengelh@xxxxxxx>; Kaneda, Erik
> <erik.kaneda@xxxxxxxxx>; Moore, Robert <robert.moore@xxxxxxxxx>
> Cc: Wysocki, Rafael J <rafael.j.wysocki@xxxxxxxxx>; ACPI Devel Maling List
> <linux-acpi@xxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-
> kernel@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH] acpica: clear global_lock bits at FACS initialization
> 
> On Mon, Mar 30, 2020 at 10:58 AM Jan Engelhardt <jengelh@xxxxxxx> wrote:
> >
Hi Jan,

I've been reading the ACPI spec and there's nothing stated about what the
initial state of the lock should be... This patch is assuming that the lock should
be free when the FACS is being initialized and I don't think this is a safe
assumption to make.

What if this is a legitimate acquisition by an SMI handler very early in OS boot?

> > When the firmware ROM supplies a FACS table with garbage, and the
> > firmware code does not clear the global_lock field before booting to a
> > loader/OS, the garbage bytes in that field (like 0xffffffff) can
> > indicate that the lock is taken when it is not, thereby preventing
> > obtaining said lock even though it is otherwise perfectly usable if
> > the field were not prepopulated with garbage.

How do we know that the lock is taken when it is not?

Erik
> >
> > Reset the lock to a known good state upon ACPI initialization.
> >
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=206553
> > Signed-off-by: Jan Engelhardt <jengelh@xxxxxxx>
> 
> Bob, Erik, please let me know if you want me to apply this directly or you
> prefer to route it through the upstream.
> 
> > ---
> >
> >  drivers/acpi/acpica/tbutils.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/acpi/acpica/tbutils.c
> > b/drivers/acpi/acpica/tbutils.c index c5f0b8ec70cc..26bdbc585d7e
> > 100644
> > --- a/drivers/acpi/acpica/tbutils.c
> > +++ b/drivers/acpi/acpica/tbutils.c
> > @@ -56,6 +56,9 @@ acpi_status acpi_tb_initialize_facs(void)
> >                                                                      &facs));
> >                 acpi_gbl_FACS = facs;
> >         }
> > +       /* Clear potential garbage from the initial FACS table. */
> > +       if (facs != NULL)
> > +               facs->global_lock &= ~0x3;
> >
> >         /* If there is no FACS, just continue. There was already an
> > error msg */
> >
> > --
> > 2.26.0
> >