RE: [PATCH 3.4 52/65] ACPICA: Tables: Fix an issue that FACS initialization is performed twice

[Zheng, Lv]

Hi,

> From: Moore, Robert
> Sent: Tuesday, October 20, 2015 9:35 PM
> 
> 
> 
> > -----Original Message-----
> > From: lizf@xxxxxxxxxx [mailto:lizf@xxxxxxxxxx]
> > Sent: Monday, October 19, 2015 5:48 PM
> > To: stable@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx; Zheng, Lv; Moore, Robert; Wysocki,
> > Rafael J; Zefan Li
> > Subject: [PATCH 3.4 52/65] ACPICA: Tables: Fix an issue that FACS
> > initialization is performed twice
> >
> > From: Lv Zheng <lv.zheng@xxxxxxxxx>
> >
> > 3.4.110-rc1 review patch.  If anyone has any objections, please let me
> > know.
> >
> > ------------------
> >
> >
> > commit c04be18448355441a0c424362df65b6422e27bda upstream.
> >
> > ACPICA commit 90f5332a15e9d9ba83831ca700b2b9f708274658
> >
> > This patch adds a new FACS initialization flag for acpi_tb_initialize().
> > acpi_enable_subsystem() might be invoked several times in OS bootup
> > process, and we don't want FACS initialization to be invoked twice. Lv
> > Zheng.
> >
> 
> Isn't calling acpi_enable_subsystem more than once essentially an OS bug?
> 
> 

I'm not sure if this is an OS bug.
And don't know the history of this function.

IMO, acpi_enable_subsystem() contains several initialization steps.
And the calling order of such steps might be OSPM specific, that's why so many flags are needed for acpi_enable_subsystem().
Then invoking acpi_enable_subsystem() twice at different OS boot stage (early or late) might not be a problem.

The patch is a different story, it just fixes an issue that one of the step is called twice in Linux because ACPICA hasn't a flag around acpi_tb_initialize_facs().

Thanks and best regards
-Lv


> 
> 
> 
> 
> > Link: https://github.com/acpica/acpica/commit/90f5332a
> > Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx>
> > Signed-off-by: Bob Moore <robert.moore@xxxxxxxxx>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > [lizf: Backported to 3.4: adjust filename]
> > Signed-off-by: Zefan Li <lizefan@xxxxxxxxxx>
> > ---
> >  drivers/acpi/acpica/utxface.c | 10 ++++++----
> >  include/acpi/actypes.h        |  1 +
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/utxface.c b/drivers/acpi/acpica/utxface.c
> > index afa94f5..0985ab7 100644
> > --- a/drivers/acpi/acpica/utxface.c
> > +++ b/drivers/acpi/acpica/utxface.c
> > @@ -166,10 +166,12 @@ acpi_status acpi_enable_subsystem(u32 flags)
> >  	 * Obtain a permanent mapping for the FACS. This is required for the
> >  	 * Global Lock and the Firmware Waking Vector
> >  	 */
> > -	status = acpi_tb_initialize_facs();
> > -	if (ACPI_FAILURE(status)) {
> > -		ACPI_WARNING((AE_INFO, "Could not map the FACS table"));
> > -		return_ACPI_STATUS(status);
> > +	if (!(flags & ACPI_NO_FACS_INIT)) {
> > +		status = acpi_tb_initialize_facs();
> > +		if (ACPI_FAILURE(status)) {
> > +			ACPI_WARNING((AE_INFO, "Could not map the FACS table"));
> > +			return_ACPI_STATUS(status);
> > +		}
> >  	}
> >  #endif				/* !ACPI_REDUCED_HARDWARE */
> >
> > diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index
> > 6d52429..0460073 100644
> > --- a/include/acpi/actypes.h
> > +++ b/include/acpi/actypes.h
> > @@ -495,6 +495,7 @@ typedef u64 acpi_integer;
> >  #define ACPI_NO_ACPI_ENABLE             0x10
> >  #define ACPI_NO_DEVICE_INIT             0x20
> >  #define ACPI_NO_OBJECT_INIT             0x40
> > +#define ACPI_NO_FACS_INIT               0x80
> >
> >  /*
> >   * Initialization state
> > --
> > 1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/