* [PATCH] acpica: clear global_lock bits at FACS initialization @ 2020-03-30 8:58 Jan Engelhardt 2020-04-01 9:11 ` Rafael J. Wysocki 2020-04-02 11:28 ` Dan Carpenter 0 siblings, 2 replies; 5+ messages in thread From: Jan Engelhardt @ 2020-03-30 8:58 UTC (permalink / raw) To: rafael.j.wysocki; +Cc: linux-acpi, linux-kernel 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. 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@inai.de> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] acpica: clear global_lock bits at FACS initialization 2020-03-30 8:58 [PATCH] acpica: clear global_lock bits at FACS initialization Jan Engelhardt @ 2020-04-01 9:11 ` Rafael J. Wysocki 2020-04-01 21:55 ` Kaneda, Erik 2020-04-02 11:28 ` Dan Carpenter 1 sibling, 1 reply; 5+ messages in thread From: Rafael J. Wysocki @ 2020-04-01 9:11 UTC (permalink / raw) To: Jan Engelhardt, Erik Kaneda, Robert Moore Cc: Rafael Wysocki, ACPI Devel Maling List, Linux Kernel Mailing List On Mon, Mar 30, 2020 at 10:58 AM Jan Engelhardt <jengelh@inai.de> wrote: > > 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. > > 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@inai.de> 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] acpica: clear global_lock bits at FACS initialization 2020-04-01 9:11 ` Rafael J. Wysocki @ 2020-04-01 21:55 ` Kaneda, Erik 2020-04-02 0:13 ` Jan Engelhardt 0 siblings, 1 reply; 5+ messages in thread From: Kaneda, Erik @ 2020-04-01 21:55 UTC (permalink / raw) To: Rafael J. Wysocki, Jan Engelhardt, Moore, Robert Cc: Wysocki, Rafael J, ACPI Devel Maling List, Linux Kernel Mailing List > -----Original Message----- > From: linux-acpi-owner@vger.kernel.org <linux-acpi- > owner@vger.kernel.org> On Behalf Of Rafael J. Wysocki > Sent: Wednesday, April 1, 2020 2:12 AM > To: Jan Engelhardt <jengelh@inai.de>; Kaneda, Erik > <erik.kaneda@intel.com>; Moore, Robert <robert.moore@intel.com> > Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; ACPI Devel Maling List > <linux-acpi@vger.kernel.org>; Linux Kernel Mailing List <linux- > kernel@vger.kernel.org> > Subject: Re: [PATCH] acpica: clear global_lock bits at FACS initialization > > On Mon, Mar 30, 2020 at 10:58 AM Jan Engelhardt <jengelh@inai.de> 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@inai.de> > > 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] acpica: clear global_lock bits at FACS initialization 2020-04-01 21:55 ` Kaneda, Erik @ 2020-04-02 0:13 ` Jan Engelhardt 0 siblings, 0 replies; 5+ messages in thread From: Jan Engelhardt @ 2020-04-02 0:13 UTC (permalink / raw) To: Kaneda, Erik Cc: Rafael J. Wysocki, Moore, Robert, Wysocki, Rafael J, ACPI Devel Maling List, Linux Kernel Mailing List On Wednesday 2020-04-01 23:55, Kaneda, Erik wrote: > >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? Before the OS has initialized ACPI (which, to me, is best recognized by what action the power button will cause - either instant-off or ACPI event), I would imagine that there are no SMI handlers that try to make use of ACPI features like the FACS lock. Furthermore, if the OS has taken the FACS lock and an SMI happens, what would the SMI do if it cannot obtain the lock? It certainly can't busywait for the OS, because that's interrupted.. >> > 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? We don't. ACPI does not make itself look good in this instance I am afraid. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] acpica: clear global_lock bits at FACS initialization 2020-03-30 8:58 [PATCH] acpica: clear global_lock bits at FACS initialization Jan Engelhardt 2020-04-01 9:11 ` Rafael J. Wysocki @ 2020-04-02 11:28 ` Dan Carpenter 1 sibling, 0 replies; 5+ messages in thread From: Dan Carpenter @ 2020-04-02 11:28 UTC (permalink / raw) To: kbuild, Jan Engelhardt Cc: kbuild-all, rafael.j.wysocki, linux-acpi, linux-kernel Hi Jan, url: https://github.com/0day-ci/linux/commits/Jan-Engelhardt/acpica-clear-global_lock-bits-at-FACS-initialization/20200330-183705 base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/acpi/acpica/tbutils.c:60 acpi_tb_initialize_facs() error: uninitialized symbol 'facs'. # https://github.com/0day-ci/linux/commit/cc0fd9e263391ff230ac700aa76dbcf7195c8c42 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout cc0fd9e263391ff230ac700aa76dbcf7195c8c42 vim +/facs +60 drivers/acpi/acpica/tbutils.c 009c4cbe99bea2 drivers/acpi/tables/tbutils.c Bob Moore 2008-11-12 35 acpi_status acpi_tb_initialize_facs(void) 009c4cbe99bea2 drivers/acpi/tables/tbutils.c Bob Moore 2008-11-12 36 { 7484619bff495c drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 37 struct acpi_table_facs *facs; 009c4cbe99bea2 drivers/acpi/tables/tbutils.c Bob Moore 2008-11-12 38 22e5b40ab21fca drivers/acpi/acpica/tbutils.c Bob Moore 2011-11-16 39 /* If Hardware Reduced flag is set, there is no FACS */ 22e5b40ab21fca drivers/acpi/acpica/tbutils.c Bob Moore 2011-11-16 40 22e5b40ab21fca drivers/acpi/acpica/tbutils.c Bob Moore 2011-11-16 41 if (acpi_gbl_reduced_hardware) { 22e5b40ab21fca drivers/acpi/acpica/tbutils.c Bob Moore 2011-11-16 42 acpi_gbl_FACS = NULL; 22e5b40ab21fca drivers/acpi/acpica/tbutils.c Bob Moore 2011-11-16 43 return (AE_OK); 8ec3f459073e67 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 44 } else if (acpi_gbl_FADT.Xfacs && 8ec3f459073e67 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 45 (!acpi_gbl_FADT.facs 8ec3f459073e67 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 46 || !acpi_gbl_use32_bit_facs_addresses)) { 8ec3f459073e67 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 47 (void)acpi_get_table_by_index(acpi_gbl_xfacs_index, c04e1fb4396d27 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-07-01 48 ACPI_CAST_INDIRECT_PTR(struct c04e1fb4396d27 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-07-01 49 acpi_table_header, 7484619bff495c drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 50 &facs)); 7484619bff495c drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 51 acpi_gbl_FACS = facs; 8ec3f459073e67 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 52 } else if (acpi_gbl_FADT.facs) { ^^^^^^^ 8ec3f459073e67 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 53 (void)acpi_get_table_by_index(acpi_gbl_facs_index, 009c4cbe99bea2 drivers/acpi/tables/tbutils.c Bob Moore 2008-11-12 54 ACPI_CAST_INDIRECT_PTR(struct 009c4cbe99bea2 drivers/acpi/tables/tbutils.c Bob Moore 2008-11-12 55 acpi_table_header, 7484619bff495c drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 56 &facs)); 7484619bff495c drivers/acpi/acpica/tbutils.c Lv Zheng 2015-08-25 57 acpi_gbl_FACS = facs; c04e1fb4396d27 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-07-01 58 } There is no else path, only else if paths. cc0fd9e263391f drivers/acpi/acpica/tbutils.c Jan Engelhardt 2020-03-30 59 /* Clear potential garbage from the initial FACS table. */ cc0fd9e263391f drivers/acpi/acpica/tbutils.c Jan Engelhardt 2020-03-30 @60 if (facs != NULL) cc0fd9e263391f drivers/acpi/acpica/tbutils.c Jan Engelhardt 2020-03-30 61 facs->global_lock &= ~0x3; c04e1fb4396d27 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-07-01 62 f06147f9fbf134 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-07-01 63 /* If there is no FACS, just continue. There was already an error msg */ f06147f9fbf134 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-07-01 64 c04e1fb4396d27 drivers/acpi/acpica/tbutils.c Lv Zheng 2015-07-01 65 return (AE_OK); 009c4cbe99bea2 drivers/acpi/tables/tbutils.c Bob Moore 2008-11-12 66 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-02 11:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-30 8:58 [PATCH] acpica: clear global_lock bits at FACS initialization Jan Engelhardt 2020-04-01 9:11 ` Rafael J. Wysocki 2020-04-01 21:55 ` Kaneda, Erik 2020-04-02 0:13 ` Jan Engelhardt 2020-04-02 11:28 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox