Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755916AbbGaU0D (ORCPT ); Fri, 31 Jul 2015 16:26:03 -0400 Received: from mga09.intel.com ([134.134.136.24]:20466 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932072AbbGaUZ7 (ORCPT ); Fri, 31 Jul 2015 16:25:59 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,586,1432623600"; d="scan'208";a="739777518" From: "Moore, Robert" To: Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" CC: "stable@vger.kernel.org" , "Zheng, Lv" , "Wysocki, Rafael J" Subject: RE: [PATCH 4.1 209/267] ACPICA: Tables: Enable both 32-bit and 64-bit FACS Thread-Topic: [PATCH 4.1 209/267] ACPICA: Tables: Enable both 32-bit and 64-bit FACS Thread-Index: AQHQy8tGAUvYneHNDE6G14MBUpRxD532Bkdw Date: Fri, 31 Jul 2015 20:25:26 +0000 Message-ID: <94F2FBAB4432B54E8AACC7DFDE6C92E37D32A0BC@ORSMSX112.amr.corp.intel.com> References: <20150731194001.933895871@linuxfoundation.org> <20150731194009.527223667@linuxfoundation.org> In-Reply-To: <20150731194009.527223667@linuxfoundation.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.22.254.138] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t6VKQ9Fc013783 Content-Length: 8683 Lines: 222 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@linuxfoundation.org] > Sent: Friday, July 31, 2015 12:41 PM > To: linux-kernel@vger.kernel.org > Cc: Greg Kroah-Hartman; stable@vger.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 > > 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 > Signed-off-by: Lv Zheng > Signed-off-by: Bob Moore > Signed-off-by: Rafael J. Wysocki > Signed-off-by: Greg Kroah-Hartman > > --- > 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?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?