[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <94F2FBAB4432B54E8AACC7DFDE6C92E37D32A0BC@ORSMSX112.amr.corp.intel.com>
Date: Fri, 31 Jul 2015 20:25:26 +0000
From: "Moore, Robert" <robert.moore@...el.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "stable@...r.kernel.org" <stable@...r.kernel.org>,
"Zheng, Lv" <lv.zheng@...el.com>,
"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>
Subject: RE: [PATCH 4.1 209/267] ACPICA: Tables: Enable both 32-bit and
64-bit FACS
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@...uxfoundation.org]
> Sent: Friday, July 31, 2015 12:41 PM
> To: linux-kernel@...r.kernel.org
> Cc: Greg Kroah-Hartman; stable@...r.kernel.org; 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@...el.com>
>
> 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@....org>
> Signed-off-by: Lv Zheng <lv.zheng@...el.com>
> Signed-off-by: Bob Moore <robert.moore@...el.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>
> ---
> 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
>
Powered by blists - more mailing lists