RE: [PATCH 4.1 209/267] ACPICA: Tables: Enable both 32-bit and 64-bit FACS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This particular patch is not stable. Lv has fixed it and a new patch is forthcoming, at least a patch on top of this.


> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> Sent: Friday, July 31, 2015 12:41 PM
> To: linux-kernel@xxxxxxxxxxxxxxx
> Cc: Greg Kroah-Hartman; stable@xxxxxxxxxxxxxxx; Zheng, Lv; Moore, Robert;
> Wysocki, Rafael J
> Subject: [PATCH 4.1 209/267] ACPICA: Tables: Enable both 32-bit and 64-bit
> FACS
> 
> 4.1-stable review patch.  If anyone has any objections, please let me
> know.
> 
> ------------------
> 
> From: Lv Zheng <lv.zheng@xxxxxxxxx>
> 
> commit c04e1fb4396d27f18296db0f914760fa7fe8223a upstream.
> 
> ACPICA commit f7b86f35416e3d1f71c3d816ff5075ddd33ed486
> 
> The following commit is reported to have broken s2ram on some platforms:
>  Commit: 0249ed2444d65d65fc3f3f64f398f1ad0b7e54cd
>  ACPICA: Add option to favor 32-bit FADT addresses.
> The platform reports 2 FACS tables (which is not allowed by ACPI
> specification) and the new 32-bit address favor rule forces OSPMs to use
> the FACS table reported via FADT's X_FIRMWARE_CTRL field.
> 
> The root cause of the reported bug might be one of the followings:
> 1. BIOS may favor the 64-bit firmware waking vector address when the
>    version of the FACS is greater than 0 and Linux currently only supports
>    resuming from the real mode, so the 64-bit firmware waking vector has
>    never been set and might be invalid to BIOS while the commit enables
>    higher version FACS.
> 2. BIOS may favor the FACS reported via the "FIRMWARE_CTRL" field in the
>    FADT while the commit doesn't set the firmware waking vector address of
>    the FACS reported by "FIRMWARE_CTRL", it only sets the firware waking
>    vector address of the FACS reported by "X_FIRMWARE_CTRL".
> 
> This patch excludes the cases that can trigger the bugs caused by the root
> cause 2.
> 
> There is no handshaking mechanism can be used by OSPM to tell BIOS which
> FACS is currently used. Thus the FACS reported by "FIRMWARE_CTRL" may
> still be used by BIOS and the 0 value of the 32-bit firmware waking vector
> might trigger such failure.
> 
> This patch tries to favor 32bit FACS address in another way where both the
> FACS reported by "FIRMWARE_CTRL" and the FACS reported by
> "X_FIRMWARE_CTRL"
> are loaded so that further commit can set firmware waking vector in the
> both tables to ensure we can exclude the cases that trigger the bugs
> caused by the root cause 2. The exclusion is split into 2 commits as this
> commit is also useful for dumping more ACPI tables, it won't get reverted
> when such exclusion is no longer necessary. Lv Zheng.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=74021
> Link: https://github.com/acpica/acpica/commit/f7b86f35
> Reported-and-tested-by: Oswald Buddenhagen <ossi@xxxxxxx>
> 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>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> 
> ---
>  drivers/acpi/acpica/aclocal.h  |    1 +
>  drivers/acpi/acpica/tbfadt.c   |   21 +++++++++++++--------
>  drivers/acpi/acpica/tbutils.c  |   34 +++++++++++++++++++++++-----------
>  drivers/acpi/acpica/tbxfload.c |    3 ++-
>  include/acpi/acpixf.h          |    9 +++++++++
>  5 files changed, 48 insertions(+), 20 deletions(-)
> 
> --- a/drivers/acpi/acpica/aclocal.h
> +++ b/drivers/acpi/acpica/aclocal.h
> @@ -213,6 +213,7 @@ struct acpi_table_list {
> 
>  #define ACPI_TABLE_INDEX_DSDT           (0)
>  #define ACPI_TABLE_INDEX_FACS           (1)
> +#define ACPI_TABLE_INDEX_X_FACS         (2)
> 
>  struct acpi_find_context {
>  	char *search_for;
> --- a/drivers/acpi/acpica/tbfadt.c
> +++ b/drivers/acpi/acpica/tbfadt.c
> @@ -350,9 +350,18 @@ void acpi_tb_parse_fadt(u32 table_index)
>  	/* If Hardware Reduced flag is set, there is no FACS */
> 
>  	if (!acpi_gbl_reduced_hardware) {
> -		acpi_tb_install_fixed_table((acpi_physical_address)
> -					    acpi_gbl_FADT.Xfacs, ACPI_SIG_FACS,
> -					    ACPI_TABLE_INDEX_FACS);
> +		if (acpi_gbl_FADT.facs) {
> +			acpi_tb_install_fixed_table((acpi_physical_address)
> +						    acpi_gbl_FADT.facs,
> +						    ACPI_SIG_FACS,
> +						    ACPI_TABLE_INDEX_FACS);
> +		}
> +		if (acpi_gbl_FADT.Xfacs) {
> +			acpi_tb_install_fixed_table((acpi_physical_address)
> +						    acpi_gbl_FADT.Xfacs,
> +						    ACPI_SIG_FACS,
> +						    ACPI_TABLE_INDEX_X_FACS);
> +		}
>  	}
>  }
> 
> @@ -491,13 +500,9 @@ static void acpi_tb_convert_fadt(void)
>  	acpi_gbl_FADT.header.length = sizeof(struct acpi_table_fadt);
> 
>  	/*
> -	 * Expand the 32-bit FACS and DSDT addresses to 64-bit as necessary.
> +	 * Expand the 32-bit DSDT addresses to 64-bit as necessary.
>  	 * Later ACPICA code will always use the X 64-bit field.
>  	 */
> -	acpi_gbl_FADT.Xfacs = acpi_tb_select_address("FACS",
> -						     acpi_gbl_FADT.facs,
> -						     acpi_gbl_FADT.Xfacs);
> -
>  	acpi_gbl_FADT.Xdsdt = acpi_tb_select_address("DSDT",
>  						     acpi_gbl_FADT.dsdt,
>  						     acpi_gbl_FADT.Xdsdt);
> --- a/drivers/acpi/acpica/tbutils.c
> +++ b/drivers/acpi/acpica/tbutils.c
> @@ -68,7 +68,8 @@ acpi_tb_get_root_table_entry(u8 *table_e
> 
>  acpi_status acpi_tb_initialize_facs(void)  {
> -	acpi_status status;
> +	struct acpi_table_facs *facs32;
> +	struct acpi_table_facs *facs64;
> 
>  	/* If Hardware Reduced flag is set, there is no FACS */
> 
> @@ -77,11 +78,22 @@ acpi_status acpi_tb_initialize_facs(void
>  		return (AE_OK);
>  	}
> 
> -	status = acpi_get_table_by_index(ACPI_TABLE_INDEX_FACS,
> -					 ACPI_CAST_INDIRECT_PTR(struct
> -								acpi_table_header,
> -								&acpi_gbl_FACS));
> -	return (status);
> +	(void)acpi_get_table_by_index(ACPI_TABLE_INDEX_FACS,
> +				      ACPI_CAST_INDIRECT_PTR(struct
> +							     acpi_table_header,
> +							     &facs32));
> +	(void)acpi_get_table_by_index(ACPI_TABLE_INDEX_X_FACS,
> +				      ACPI_CAST_INDIRECT_PTR(struct
> +							     acpi_table_header,
> +							     &facs64));
> +
> +	if (acpi_gbl_use32_bit_facs_addresses) {
> +		acpi_gbl_FACS = facs32 ? facs32 : facs64;
> +	} else {
> +		acpi_gbl_FACS = facs64 ? facs64 : facs32;
> +	}
> +
> +	return (AE_OK);
>  }
>  #endif				/* !ACPI_REDUCED_HARDWARE */
> 
> @@ -101,7 +113,7 @@ acpi_status acpi_tb_initialize_facs(void
>  u8 acpi_tb_tables_loaded(void)
>  {
> 
> -	if (acpi_gbl_root_table_list.current_table_count >= 3) {
> +	if (acpi_gbl_root_table_list.current_table_count >= 4) {
>  		return (TRUE);
>  	}
> 
> @@ -357,11 +369,11 @@ acpi_status __init acpi_tb_parse_root_ta
>  	table_entry = ACPI_ADD_PTR(u8, table, sizeof(struct
> acpi_table_header));
> 
>  	/*
> -	 * First two entries in the table array are reserved for the DSDT
> -	 * and FACS, which are not actually present in the RSDT/XSDT - they
> -	 * come from the FADT
> +	 * First three entries in the table array are reserved for the DSDT
> +	 * and 32bit/64bit FACS, which are not actually present in the
> +	 * RSDT/XSDT - they come from the FADT
>  	 */
> -	acpi_gbl_root_table_list.current_table_count = 2;
> +	acpi_gbl_root_table_list.current_table_count = 3;
> 
>  	/* Initialize the root table array from the RSDT/XSDT */
> 
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -166,7 +166,8 @@ static acpi_status acpi_tb_load_namespac
> 
>  	(void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
>  	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
> -		if ((!ACPI_COMPARE_NAME
> +		if (!acpi_gbl_root_table_list.tables[i].address ||
> +		    (!ACPI_COMPARE_NAME
>  		     (&(acpi_gbl_root_table_list.tables[i].signature),
>  		      ACPI_SIG_SSDT)
>  		     &&
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -200,6 +200,15 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use
> ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, TRUE);
> 
>  /*
> + * Optionally use 32-bit FACS table addresses.
> + * It is reported that some platforms fail to resume from system
> +suspending
> + * if 64-bit FACS table address is selected:
> + * https://bugzilla.kernel.org/show_bug.cgi?id=74021
> + * Default is TRUE, favor the 32-bit addresses.
> + */
> +ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_facs_addresses, TRUE);
> +
> +/*
>   * Optionally truncate I/O addresses to 16 bits. Provides compatibility
>   * with other ACPI implementations. NOTE: During ACPICA initialization,
>   * this value is set to TRUE if any Windows OSI strings have been
> 

��.n��������+%������w��{.n�����������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]