[PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Revert commit 0249ed2444d6 (ACPICA: Add option to favor 32-bit FADT
addresses.) that breaks resume from system suspend on the Intel DP45SG
board.

Fixes: 0249ed2444d6 (ACPICA: Add option to favor 32-bit FADT addresses.)
References: https://bugzilla.kernel.org/show_bug.cgi?id=74021
Reported-and-tested-by: Oswald Buddenhagen <ossi@kde.org>
Cc: 3.14+ <stable@vger.kernel.org> # 3.14+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpica/acglobal.h |  10 --
 drivers/acpi/acpica/tbfadt.c   | 335 +++++++++++++++++++----------------------
 include/acpi/acpixf.h          |   1 -
 3 files changed, 152 insertions(+), 194 deletions(-)

diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index 49bbc71fad54..72578a4b8212 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -136,16 +136,6 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_copy_dsdt_locally, FALSE);
 ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
 
 /*
- * Optionally use 32-bit FADT addresses if and when there is a conflict
- * (address mismatch) between the 32-bit and 64-bit versions of the
- * address. Although ACPICA adheres to the ACPI specification which
- * requires the use of the corresponding 64-bit address if it is non-zero,
- * some machines have been found to have a corrupted non-zero 64-bit
- * address. Default is FALSE, do not favor the 32-bit addresses.
- */
-ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, FALSE);
-
-/*
  * 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
diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
index ec14588254d4..6dd5c590e0bb 100644
--- a/drivers/acpi/acpica/tbfadt.c
+++ b/drivers/acpi/acpica/tbfadt.c
@@ -56,10 +56,9 @@ acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
 
 static void acpi_tb_convert_fadt(void);
 
-static void acpi_tb_setup_fadt_registers(void);
+static void acpi_tb_validate_fadt(void);
 
-static u64
-acpi_tb_select_address(char *register_name, u32 address32, u64 address64);
+static void acpi_tb_setup_fadt_registers(void);
 
 /* Table for conversion of FADT to common internal format and FADT validation */
 
@@ -176,7 +175,6 @@ static struct acpi_fadt_pm_info fadt_pm_info_table[] = {
  *              space_id            - ACPI Space ID for this register
  *              byte_width          - Width of this register
  *              address             - Address of the register
- *              register_name       - ASCII name of the ACPI register
  *
  * RETURN:      None
  *
@@ -222,68 +220,6 @@ acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_tb_select_address
- *
- * PARAMETERS:  register_name       - ASCII name of the ACPI register
- *              address32           - 32-bit address of the register
- *              address64           - 64-bit address of the register
- *
- * RETURN:      The resolved 64-bit address
- *
- * DESCRIPTION: Select between 32-bit and 64-bit versions of addresses within
- *              the FADT. Used for the FACS and DSDT addresses.
- *
- * NOTES:
- *
- * Check for FACS and DSDT address mismatches. An address mismatch between
- * the 32-bit and 64-bit address fields (FIRMWARE_CTRL/X_FIRMWARE_CTRL and
- * DSDT/X_DSDT) could be a corrupted address field or it might indicate
- * the presence of two FACS or two DSDT tables.
- *
- * November 2013:
- * By default, as per the ACPICA specification, a valid 64-bit address is
- * used regardless of the value of the 32-bit address. However, this
- * behavior can be overridden via the acpi_gbl_use32_bit_fadt_addresses flag.
- *
- ******************************************************************************/
-
-static u64
-acpi_tb_select_address(char *register_name, u32 address32, u64 address64)
-{
-
-	if (!address64) {
-
-		/* 64-bit address is zero, use 32-bit address */
-
-		return ((u64)address32);
-	}
-
-	if (address32 && (address64 != (u64)address32)) {
-
-		/* Address mismatch between 32-bit and 64-bit versions */
-
-		ACPI_BIOS_WARNING((AE_INFO,
-				   "32/64X %s address mismatch in FADT: "
-				   "0x%8.8X/0x%8.8X%8.8X, using %u-bit address",
-				   register_name, address32,
-				   ACPI_FORMAT_UINT64(address64),
-				   acpi_gbl_use32_bit_fadt_addresses ? 32 :
-				   64));
-
-		/* 32-bit address override */
-
-		if (acpi_gbl_use32_bit_fadt_addresses) {
-			return ((u64)address32);
-		}
-	}
-
-	/* Default is to use the 64-bit address */
-
-	return (address64);
-}
-
-/*******************************************************************************
- *
  * FUNCTION:    acpi_tb_parse_fadt
  *
  * PARAMETERS:  table_index         - Index for the FADT
@@ -395,6 +331,10 @@ void acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 length)
 
 	acpi_tb_convert_fadt();
 
+	/* Validate FADT values now, before we make any changes */
+
+	acpi_tb_validate_fadt();
+
 	/* Initialize the global ACPI register structures */
 
 	acpi_tb_setup_fadt_registers();
@@ -404,55 +344,66 @@ void acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 length)
  *
  * FUNCTION:    acpi_tb_convert_fadt
  *
- * PARAMETERS:  none - acpi_gbl_FADT is used.
+ * PARAMETERS:  None, uses acpi_gbl_FADT
  *
  * RETURN:      None
  *
  * DESCRIPTION: Converts all versions of the FADT to a common internal format.
- *              Expand 32-bit addresses to 64-bit as necessary. Also validate
- *              important fields within the FADT.
+ *              Expand 32-bit addresses to 64-bit as necessary.
  *
- * NOTE:        acpi_gbl_FADT must be of size (struct acpi_table_fadt), and must
- *              contain a copy of the actual BIOS-provided FADT.
+ * NOTE:        acpi_gbl_FADT must be of size (struct acpi_table_fadt),
+ *              and must contain a copy of the actual FADT.
  *
  * Notes on 64-bit register addresses:
  *
  * After this FADT conversion, later ACPICA code will only use the 64-bit "X"
  * fields of the FADT for all ACPI register addresses.
  *
- * The 64-bit X fields are optional extensions to the original 32-bit FADT
+ * The 64-bit "X" fields are optional extensions to the original 32-bit FADT
  * V1.0 fields. Even if they are present in the FADT, they are optional and
  * are unused if the BIOS sets them to zero. Therefore, we must copy/expand
- * 32-bit V1.0 fields to the 64-bit X fields if the the 64-bit X field is
- * originally zero.
+ * 32-bit V1.0 fields if the corresponding X field is zero.
  *
- * For ACPI 1.0 FADTs (that contain no 64-bit addresses), all 32-bit address
- * fields are expanded to the corresponding 64-bit X fields in the internal
- * common FADT.
+ * For ACPI 1.0 FADTs, all 32-bit address fields are expanded to the
+ * corresponding "X" fields in the internal FADT.
  *
  * For ACPI 2.0+ FADTs, all valid (non-zero) 32-bit address fields are expanded
- * to the corresponding 64-bit X fields, if the 64-bit field is originally
- * zero. Adhering to the ACPI specification, we completely ignore the 32-bit
- * field if the 64-bit field is valid, regardless of whether the host OS is
- * 32-bit or 64-bit.
- *
- * Possible additional checks:
- *  (acpi_gbl_FADT.pm1_event_length >= 4)
- *  (acpi_gbl_FADT.pm1_control_length >= 2)
- *  (acpi_gbl_FADT.pm_timer_length >= 4)
- *  Gpe block lengths must be multiple of 2
+ * to the corresponding 64-bit X fields. For compatibility with other ACPI
+ * implementations, we ignore the 64-bit field if the 32-bit field is valid,
+ * regardless of whether the host OS is 32-bit or 64-bit.
  *
  ******************************************************************************/
 
 static void acpi_tb_convert_fadt(void)
 {
-	char *name;
 	struct acpi_generic_address *address64;
 	u32 address32;
-	u8 length;
 	u32 i;
 
 	/*
+	 * Expand the 32-bit FACS and DSDT addresses to 64-bit as necessary.
+	 * Later code will always use the X 64-bit field. Also, check for an
+	 * address mismatch between the 32-bit and 64-bit address fields
+	 * (FIRMWARE_CTRL/X_FIRMWARE_CTRL, DSDT/X_DSDT) which would indicate
+	 * the presence of two FACS or two DSDT tables.
+	 */
+	if (!acpi_gbl_FADT.Xfacs) {
+		acpi_gbl_FADT.Xfacs = (u64) acpi_gbl_FADT.facs;
+	} else if (acpi_gbl_FADT.facs &&
+		   (acpi_gbl_FADT.Xfacs != (u64) acpi_gbl_FADT.facs)) {
+		ACPI_WARNING((AE_INFO,
+		    "32/64 FACS address mismatch in FADT - two FACS tables!"));
+	}
+
+	if (!acpi_gbl_FADT.Xdsdt) {
+		acpi_gbl_FADT.Xdsdt = (u64) acpi_gbl_FADT.dsdt;
+	} else if (acpi_gbl_FADT.dsdt &&
+		   (acpi_gbl_FADT.Xdsdt != (u64) acpi_gbl_FADT.dsdt)) {
+		ACPI_WARNING((AE_INFO,
+		    "32/64 DSDT address mismatch in FADT - two DSDT tables!"));
+	}
+
+	/*
 	 * For ACPI 1.0 FADTs (revision 1 or 2), ensure that reserved fields which
 	 * should be zero are indeed zero. This will workaround BIOSs that
 	 * inadvertently place values in these fields.
@@ -470,24 +421,119 @@ static void acpi_tb_convert_fadt(void)
 		acpi_gbl_FADT.boot_flags = 0;
 	}
 
+	/* Update the local FADT table header length */
+
+	acpi_gbl_FADT.header.length = sizeof(struct acpi_table_fadt);
+
 	/*
-	 * Now we can update the local FADT length to the length of the
-	 * current FADT version as defined by the ACPI specification.
-	 * Thus, we will have a common FADT internally.
+	 * Expand the ACPI 1.0 32-bit addresses to the ACPI 2.0 64-bit "X"
+	 * generic address structures as necessary. Later code will always use
+	 * the 64-bit address structures.
+	 *
+	 * March 2009:
+	 * We now always use the 32-bit address if it is valid (non-null). This
+	 * is not in accordance with the ACPI specification which states that
+	 * the 64-bit address supersedes the 32-bit version, but we do this for
+	 * compatibility with other ACPI implementations. Most notably, in the
+	 * case where both the 32 and 64 versions are non-null, we use the 32-bit
+	 * version. This is the only address that is guaranteed to have been
+	 * tested by the BIOS manufacturer.
 	 */
-	acpi_gbl_FADT.header.length = sizeof(struct acpi_table_fadt);
+	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
+		address32 = *ACPI_ADD_PTR(u32,
+					  &acpi_gbl_FADT,
+					  fadt_info_table[i].address32);
+
+		address64 = ACPI_ADD_PTR(struct acpi_generic_address,
+					 &acpi_gbl_FADT,
+					 fadt_info_table[i].address64);
+
+		/*
+		 * If both 32- and 64-bit addresses are valid (non-zero),
+		 * they must match.
+		 */
+		if (address64->address && address32 &&
+		    (address64->address != (u64)address32)) {
+			ACPI_BIOS_ERROR((AE_INFO,
+					 "32/64X address mismatch in FADT/%s: "
+					 "0x%8.8X/0x%8.8X%8.8X, using 32",
+					 fadt_info_table[i].name, address32,
+					 ACPI_FORMAT_UINT64(address64->
+							    address)));
+		}
+
+		/* Always use 32-bit address if it is valid (non-null) */
+
+		if (address32) {
+			/*
+			 * Copy the 32-bit address to the 64-bit GAS structure. The
+			 * Space ID is always I/O for 32-bit legacy address fields
+			*/
+			acpi_tb_init_generic_address(address64,
+						     ACPI_ADR_SPACE_SYSTEM_IO,
+						     *ACPI_ADD_PTR(u8,
+								   &acpi_gbl_FADT,
+								   fadt_info_table
+								   [i].length),
+						     (u64) address32,
+						     fadt_info_table[i].name);
+		}
+	}
+}
+
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_tb_validate_fadt
+ *
+ * PARAMETERS:  table           - Pointer to the FADT to be validated
+ *
+ * RETURN:      None
+ *
+ * DESCRIPTION: Validate various important fields within the FADT. If a problem
+ *              is found, issue a message, but no status is returned.
+ *              Used by both the table manager and the disassembler.
+ *
+ * Possible additional checks:
+ * (acpi_gbl_FADT.pm1_event_length >= 4)
+ * (acpi_gbl_FADT.pm1_control_length >= 2)
+ * (acpi_gbl_FADT.pm_timer_length >= 4)
+ * Gpe block lengths must be multiple of 2
+ *
+ ******************************************************************************/
+
+static void acpi_tb_validate_fadt(void)
+{
+	char *name;
+	struct acpi_generic_address *address64;
+	u8 length;
+	u32 i;
 
 	/*
-	 * Expand the 32-bit FACS and DSDT addresses to 64-bit as necessary.
-	 * Later ACPICA code will always use the X 64-bit field.
+	 * Check for FACS and DSDT address mismatches. An address mismatch between
+	 * the 32-bit and 64-bit address fields (FIRMWARE_CTRL/X_FIRMWARE_CTRL and
+	 * DSDT/X_DSDT) would indicate the presence of two FACS or two DSDT tables.
 	 */
-	acpi_gbl_FADT.Xfacs = acpi_tb_select_address("FACS",
-						     acpi_gbl_FADT.facs,
-						     acpi_gbl_FADT.Xfacs);
+	if (acpi_gbl_FADT.facs &&
+	    (acpi_gbl_FADT.Xfacs != (u64)acpi_gbl_FADT.facs)) {
+		ACPI_BIOS_WARNING((AE_INFO,
+				   "32/64X FACS address mismatch in FADT - "
+				   "0x%8.8X/0x%8.8X%8.8X, using 32",
+				   acpi_gbl_FADT.facs,
+				   ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xfacs)));
+
+		acpi_gbl_FADT.Xfacs = (u64)acpi_gbl_FADT.facs;
+	}
+
+	if (acpi_gbl_FADT.dsdt &&
+	    (acpi_gbl_FADT.Xdsdt != (u64)acpi_gbl_FADT.dsdt)) {
+		ACPI_BIOS_WARNING((AE_INFO,
+				   "32/64X DSDT address mismatch in FADT - "
+				   "0x%8.8X/0x%8.8X%8.8X, using 32",
+				   acpi_gbl_FADT.dsdt,
+				   ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xdsdt)));
 
-	acpi_gbl_FADT.Xdsdt = acpi_tb_select_address("DSDT",
-						     acpi_gbl_FADT.dsdt,
-						     acpi_gbl_FADT.Xdsdt);
+		acpi_gbl_FADT.Xdsdt = (u64)acpi_gbl_FADT.dsdt;
+	}
 
 	/* If Hardware Reduced flag is set, we are all done */
 
@@ -499,95 +545,18 @@ static void acpi_tb_convert_fadt(void)
 
 	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
 		/*
-		 * Get the 32-bit and 64-bit addresses, as well as the register
-		 * length and register name.
+		 * Generate pointer to the 64-bit address, get the register
+		 * length (width) and the register name
 		 */
-		address32 = *ACPI_ADD_PTR(u32,
-					  &acpi_gbl_FADT,
-					  fadt_info_table[i].address32);
-
 		address64 = ACPI_ADD_PTR(struct acpi_generic_address,
 					 &acpi_gbl_FADT,
 					 fadt_info_table[i].address64);
-
-		length = *ACPI_ADD_PTR(u8,
-				       &acpi_gbl_FADT,
-				       fadt_info_table[i].length);
-
+		length =
+		    *ACPI_ADD_PTR(u8, &acpi_gbl_FADT,
+				  fadt_info_table[i].length);
 		name = fadt_info_table[i].name;
 
 		/*
-		 * Expand the ACPI 1.0 32-bit addresses to the ACPI 2.0 64-bit "X"
-		 * generic address structures as necessary. Later code will always use
-		 * the 64-bit address structures.
-		 *
-		 * November 2013:
-		 * Now always use the 64-bit address if it is valid (non-zero), in
-		 * accordance with the ACPI specification which states that a 64-bit
-		 * address supersedes the 32-bit version. This behavior can be
-		 * overridden by the acpi_gbl_use32_bit_fadt_addresses flag.
-		 *
-		 * During 64-bit address construction and verification,
-		 * these cases are handled:
-		 *
-		 * Address32 zero, Address64 [don't care]   - Use Address64
-		 *
-		 * Address32 non-zero, Address64 zero       - Copy/use Address32
-		 * Address32 non-zero == Address64 non-zero - Use Address64
-		 * Address32 non-zero != Address64 non-zero - Warning, use Address64
-		 *
-		 * Override: if acpi_gbl_use32_bit_fadt_addresses is TRUE, and:
-		 * Address32 non-zero != Address64 non-zero - Warning, copy/use Address32
-		 *
-		 * Note: space_id is always I/O for 32-bit legacy address fields
-		 */
-		if (address32) {
-			if (!address64->address) {
-
-				/* 64-bit address is zero, use 32-bit address */
-
-				acpi_tb_init_generic_address(address64,
-							     ACPI_ADR_SPACE_SYSTEM_IO,
-							     *ACPI_ADD_PTR(u8,
-									   &acpi_gbl_FADT,
-									   fadt_info_table
-									   [i].
-									   length),
-							     (u64)address32,
-							     name);
-			} else if (address64->address != (u64)address32) {
-
-				/* Address mismatch */
-
-				ACPI_BIOS_WARNING((AE_INFO,
-						   "32/64X address mismatch in FADT/%s: "
-						   "0x%8.8X/0x%8.8X%8.8X, using %u-bit address",
-						   name, address32,
-						   ACPI_FORMAT_UINT64
-						   (address64->address),
-						   acpi_gbl_use32_bit_fadt_addresses
-						   ? 32 : 64));
-
-				if (acpi_gbl_use32_bit_fadt_addresses) {
-
-					/* 32-bit address override */
-
-					acpi_tb_init_generic_address(address64,
-								     ACPI_ADR_SPACE_SYSTEM_IO,
-								     *ACPI_ADD_PTR
-								     (u8,
-								      &acpi_gbl_FADT,
-								      fadt_info_table
-								      [i].
-								      length),
-								     (u64)
-								     address32,
-								     name);
-				}
-			}
-		}
-
-		/*
 		 * For each extended field, check for length mismatch between the
 		 * legacy length field and the corresponding 64-bit X length field.
 		 * Note: If the legacy length field is > 0xFF bits, ignore this
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 44f5e9749601..40d1bc88cc14 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -82,7 +82,6 @@ extern u8 acpi_gbl_enable_interpreter_slack;
 extern u32 acpi_gbl_trace_flags;
 extern acpi_name acpi_gbl_trace_method_name;
 extern u8 acpi_gbl_truncate_io_addresses;
-extern u8 acpi_gbl_use32_bit_fadt_addresses;
 extern u8 acpi_gbl_use_default_register_widths;
 
 /*
-- 
1.8.4.5

RE: [PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[Zheng, Lv]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 19129 bytes --]

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Monday, May 12, 2014 8:12 AM
> 
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Revert commit 0249ed2444d6 (ACPICA: Add option to favor 32-bit FADT
> addresses.) that breaks resume from system suspend on the Intel DP45SG
> board.

This commit doesn't look like the root cause.
[    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20131218/tbprint-214)
[    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xCF661F40/0x00000000CF667E40, using 64-bit address (20131218/tbfadt-271)
It seems before using the FADT, ACPICA have already detected that the XSDT is not correct.

Please revert it for now.  I'll take a look at this bug.

Thanks and best regards
-Lv

> 
> Fixes: 0249ed2444d6 (ACPICA: Add option to favor 32-bit FADT addresses.)
> References: https://bugzilla.kernel.org/show_bug.cgi?id=74021
> Reported-and-tested-by: Oswald Buddenhagen <ossi@kde.org>
> Cc: 3.14+ <stable@vger.kernel.org> # 3.14+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/acpica/acglobal.h |  10 --
>  drivers/acpi/acpica/tbfadt.c   | 335 +++++++++++++++++++----------------------
>  include/acpi/acpixf.h          |   1 -
>  3 files changed, 152 insertions(+), 194 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> index 49bbc71fad54..72578a4b8212 100644
> --- a/drivers/acpi/acpica/acglobal.h
> +++ b/drivers/acpi/acpica/acglobal.h
> @@ -136,16 +136,6 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_copy_dsdt_locally, FALSE);
>  ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
> 
>  /*
> - * Optionally use 32-bit FADT addresses if and when there is a conflict
> - * (address mismatch) between the 32-bit and 64-bit versions of the
> - * address. Although ACPICA adheres to the ACPI specification which
> - * requires the use of the corresponding 64-bit address if it is non-zero,
> - * some machines have been found to have a corrupted non-zero 64-bit
> - * address. Default is FALSE, do not favor the 32-bit addresses.
> - */
> -ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, FALSE);
> -
> -/*
>   * 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
> diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
> index ec14588254d4..6dd5c590e0bb 100644
> --- a/drivers/acpi/acpica/tbfadt.c
> +++ b/drivers/acpi/acpica/tbfadt.c
> @@ -56,10 +56,9 @@ acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
> 
>  static void acpi_tb_convert_fadt(void);
> 
> -static void acpi_tb_setup_fadt_registers(void);
> +static void acpi_tb_validate_fadt(void);
> 
> -static u64
> -acpi_tb_select_address(char *register_name, u32 address32, u64 address64);
> +static void acpi_tb_setup_fadt_registers(void);
> 
>  /* Table for conversion of FADT to common internal format and FADT validation */
> 
> @@ -176,7 +175,6 @@ static struct acpi_fadt_pm_info fadt_pm_info_table[] = {
>   *              space_id            - ACPI Space ID for this register
>   *              byte_width          - Width of this register
>   *              address             - Address of the register
> - *              register_name       - ASCII name of the ACPI register
>   *
>   * RETURN:      None
>   *
> @@ -222,68 +220,6 @@ acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
> 
>  /*******************************************************************************
>   *
> - * FUNCTION:    acpi_tb_select_address
> - *
> - * PARAMETERS:  register_name       - ASCII name of the ACPI register
> - *              address32           - 32-bit address of the register
> - *              address64           - 64-bit address of the register
> - *
> - * RETURN:      The resolved 64-bit address
> - *
> - * DESCRIPTION: Select between 32-bit and 64-bit versions of addresses within
> - *              the FADT. Used for the FACS and DSDT addresses.
> - *
> - * NOTES:
> - *
> - * Check for FACS and DSDT address mismatches. An address mismatch between
> - * the 32-bit and 64-bit address fields (FIRMWARE_CTRL/X_FIRMWARE_CTRL and
> - * DSDT/X_DSDT) could be a corrupted address field or it might indicate
> - * the presence of two FACS or two DSDT tables.
> - *
> - * November 2013:
> - * By default, as per the ACPICA specification, a valid 64-bit address is
> - * used regardless of the value of the 32-bit address. However, this
> - * behavior can be overridden via the acpi_gbl_use32_bit_fadt_addresses flag.
> - *
> - ******************************************************************************/
> -
> -static u64
> -acpi_tb_select_address(char *register_name, u32 address32, u64 address64)
> -{
> -
> -	if (!address64) {
> -
> -		/* 64-bit address is zero, use 32-bit address */
> -
> -		return ((u64)address32);
> -	}
> -
> -	if (address32 && (address64 != (u64)address32)) {
> -
> -		/* Address mismatch between 32-bit and 64-bit versions */
> -
> -		ACPI_BIOS_WARNING((AE_INFO,
> -				   "32/64X %s address mismatch in FADT: "
> -				   "0x%8.8X/0x%8.8X%8.8X, using %u-bit address",
> -				   register_name, address32,
> -				   ACPI_FORMAT_UINT64(address64),
> -				   acpi_gbl_use32_bit_fadt_addresses ? 32 :
> -				   64));
> -
> -		/* 32-bit address override */
> -
> -		if (acpi_gbl_use32_bit_fadt_addresses) {
> -			return ((u64)address32);
> -		}
> -	}
> -
> -	/* Default is to use the 64-bit address */
> -
> -	return (address64);
> -}
> -
> -/*******************************************************************************
> - *
>   * FUNCTION:    acpi_tb_parse_fadt
>   *
>   * PARAMETERS:  table_index         - Index for the FADT
> @@ -395,6 +331,10 @@ void acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 length)
> 
>  	acpi_tb_convert_fadt();
> 
> +	/* Validate FADT values now, before we make any changes */
> +
> +	acpi_tb_validate_fadt();
> +
>  	/* Initialize the global ACPI register structures */
> 
>  	acpi_tb_setup_fadt_registers();
> @@ -404,55 +344,66 @@ void acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 length)
>   *
>   * FUNCTION:    acpi_tb_convert_fadt
>   *
> - * PARAMETERS:  none - acpi_gbl_FADT is used.
> + * PARAMETERS:  None, uses acpi_gbl_FADT
>   *
>   * RETURN:      None
>   *
>   * DESCRIPTION: Converts all versions of the FADT to a common internal format.
> - *              Expand 32-bit addresses to 64-bit as necessary. Also validate
> - *              important fields within the FADT.
> + *              Expand 32-bit addresses to 64-bit as necessary.
>   *
> - * NOTE:        acpi_gbl_FADT must be of size (struct acpi_table_fadt), and must
> - *              contain a copy of the actual BIOS-provided FADT.
> + * NOTE:        acpi_gbl_FADT must be of size (struct acpi_table_fadt),
> + *              and must contain a copy of the actual FADT.
>   *
>   * Notes on 64-bit register addresses:
>   *
>   * After this FADT conversion, later ACPICA code will only use the 64-bit "X"
>   * fields of the FADT for all ACPI register addresses.
>   *
> - * The 64-bit X fields are optional extensions to the original 32-bit FADT
> + * The 64-bit "X" fields are optional extensions to the original 32-bit FADT
>   * V1.0 fields. Even if they are present in the FADT, they are optional and
>   * are unused if the BIOS sets them to zero. Therefore, we must copy/expand
> - * 32-bit V1.0 fields to the 64-bit X fields if the the 64-bit X field is
> - * originally zero.
> + * 32-bit V1.0 fields if the corresponding X field is zero.
>   *
> - * For ACPI 1.0 FADTs (that contain no 64-bit addresses), all 32-bit address
> - * fields are expanded to the corresponding 64-bit X fields in the internal
> - * common FADT.
> + * For ACPI 1.0 FADTs, all 32-bit address fields are expanded to the
> + * corresponding "X" fields in the internal FADT.
>   *
>   * For ACPI 2.0+ FADTs, all valid (non-zero) 32-bit address fields are expanded
> - * to the corresponding 64-bit X fields, if the 64-bit field is originally
> - * zero. Adhering to the ACPI specification, we completely ignore the 32-bit
> - * field if the 64-bit field is valid, regardless of whether the host OS is
> - * 32-bit or 64-bit.
> - *
> - * Possible additional checks:
> - *  (acpi_gbl_FADT.pm1_event_length >= 4)
> - *  (acpi_gbl_FADT.pm1_control_length >= 2)
> - *  (acpi_gbl_FADT.pm_timer_length >= 4)
> - *  Gpe block lengths must be multiple of 2
> + * to the corresponding 64-bit X fields. For compatibility with other ACPI
> + * implementations, we ignore the 64-bit field if the 32-bit field is valid,
> + * regardless of whether the host OS is 32-bit or 64-bit.
>   *
>   ******************************************************************************/
> 
>  static void acpi_tb_convert_fadt(void)
>  {
> -	char *name;
>  	struct acpi_generic_address *address64;
>  	u32 address32;
> -	u8 length;
>  	u32 i;
> 
>  	/*
> +	 * Expand the 32-bit FACS and DSDT addresses to 64-bit as necessary.
> +	 * Later code will always use the X 64-bit field. Also, check for an
> +	 * address mismatch between the 32-bit and 64-bit address fields
> +	 * (FIRMWARE_CTRL/X_FIRMWARE_CTRL, DSDT/X_DSDT) which would indicate
> +	 * the presence of two FACS or two DSDT tables.
> +	 */
> +	if (!acpi_gbl_FADT.Xfacs) {
> +		acpi_gbl_FADT.Xfacs = (u64) acpi_gbl_FADT.facs;
> +	} else if (acpi_gbl_FADT.facs &&
> +		   (acpi_gbl_FADT.Xfacs != (u64) acpi_gbl_FADT.facs)) {
> +		ACPI_WARNING((AE_INFO,
> +		    "32/64 FACS address mismatch in FADT - two FACS tables!"));
> +	}
> +
> +	if (!acpi_gbl_FADT.Xdsdt) {
> +		acpi_gbl_FADT.Xdsdt = (u64) acpi_gbl_FADT.dsdt;
> +	} else if (acpi_gbl_FADT.dsdt &&
> +		   (acpi_gbl_FADT.Xdsdt != (u64) acpi_gbl_FADT.dsdt)) {
> +		ACPI_WARNING((AE_INFO,
> +		    "32/64 DSDT address mismatch in FADT - two DSDT tables!"));
> +	}
> +
> +	/*
>  	 * For ACPI 1.0 FADTs (revision 1 or 2), ensure that reserved fields which
>  	 * should be zero are indeed zero. This will workaround BIOSs that
>  	 * inadvertently place values in these fields.
> @@ -470,24 +421,119 @@ static void acpi_tb_convert_fadt(void)
>  		acpi_gbl_FADT.boot_flags = 0;
>  	}
> 
> +	/* Update the local FADT table header length */
> +
> +	acpi_gbl_FADT.header.length = sizeof(struct acpi_table_fadt);
> +
>  	/*
> -	 * Now we can update the local FADT length to the length of the
> -	 * current FADT version as defined by the ACPI specification.
> -	 * Thus, we will have a common FADT internally.
> +	 * Expand the ACPI 1.0 32-bit addresses to the ACPI 2.0 64-bit "X"
> +	 * generic address structures as necessary. Later code will always use
> +	 * the 64-bit address structures.
> +	 *
> +	 * March 2009:
> +	 * We now always use the 32-bit address if it is valid (non-null). This
> +	 * is not in accordance with the ACPI specification which states that
> +	 * the 64-bit address supersedes the 32-bit version, but we do this for
> +	 * compatibility with other ACPI implementations. Most notably, in the
> +	 * case where both the 32 and 64 versions are non-null, we use the 32-bit
> +	 * version. This is the only address that is guaranteed to have been
> +	 * tested by the BIOS manufacturer.
>  	 */
> -	acpi_gbl_FADT.header.length = sizeof(struct acpi_table_fadt);
> +	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
> +		address32 = *ACPI_ADD_PTR(u32,
> +					  &acpi_gbl_FADT,
> +					  fadt_info_table[i].address32);
> +
> +		address64 = ACPI_ADD_PTR(struct acpi_generic_address,
> +					 &acpi_gbl_FADT,
> +					 fadt_info_table[i].address64);
> +
> +		/*
> +		 * If both 32- and 64-bit addresses are valid (non-zero),
> +		 * they must match.
> +		 */
> +		if (address64->address && address32 &&
> +		    (address64->address != (u64)address32)) {
> +			ACPI_BIOS_ERROR((AE_INFO,
> +					 "32/64X address mismatch in FADT/%s: "
> +					 "0x%8.8X/0x%8.8X%8.8X, using 32",
> +					 fadt_info_table[i].name, address32,
> +					 ACPI_FORMAT_UINT64(address64->
> +							    address)));
> +		}
> +
> +		/* Always use 32-bit address if it is valid (non-null) */
> +
> +		if (address32) {
> +			/*
> +			 * Copy the 32-bit address to the 64-bit GAS structure. The
> +			 * Space ID is always I/O for 32-bit legacy address fields
> +			*/
> +			acpi_tb_init_generic_address(address64,
> +						     ACPI_ADR_SPACE_SYSTEM_IO,
> +						     *ACPI_ADD_PTR(u8,
> +								   &acpi_gbl_FADT,
> +								   fadt_info_table
> +								   [i].length),
> +						     (u64) address32,
> +						     fadt_info_table[i].name);
> +		}
> +	}
> +}
> +
> +/*******************************************************************************
> + *
> + * FUNCTION:    acpi_tb_validate_fadt
> + *
> + * PARAMETERS:  table           - Pointer to the FADT to be validated
> + *
> + * RETURN:      None
> + *
> + * DESCRIPTION: Validate various important fields within the FADT. If a problem
> + *              is found, issue a message, but no status is returned.
> + *              Used by both the table manager and the disassembler.
> + *
> + * Possible additional checks:
> + * (acpi_gbl_FADT.pm1_event_length >= 4)
> + * (acpi_gbl_FADT.pm1_control_length >= 2)
> + * (acpi_gbl_FADT.pm_timer_length >= 4)
> + * Gpe block lengths must be multiple of 2
> + *
> + ******************************************************************************/
> +
> +static void acpi_tb_validate_fadt(void)
> +{
> +	char *name;
> +	struct acpi_generic_address *address64;
> +	u8 length;
> +	u32 i;
> 
>  	/*
> -	 * Expand the 32-bit FACS and DSDT addresses to 64-bit as necessary.
> -	 * Later ACPICA code will always use the X 64-bit field.
> +	 * Check for FACS and DSDT address mismatches. An address mismatch between
> +	 * the 32-bit and 64-bit address fields (FIRMWARE_CTRL/X_FIRMWARE_CTRL and
> +	 * DSDT/X_DSDT) would indicate the presence of two FACS or two DSDT tables.
>  	 */
> -	acpi_gbl_FADT.Xfacs = acpi_tb_select_address("FACS",
> -						     acpi_gbl_FADT.facs,
> -						     acpi_gbl_FADT.Xfacs);
> +	if (acpi_gbl_FADT.facs &&
> +	    (acpi_gbl_FADT.Xfacs != (u64)acpi_gbl_FADT.facs)) {
> +		ACPI_BIOS_WARNING((AE_INFO,
> +				   "32/64X FACS address mismatch in FADT - "
> +				   "0x%8.8X/0x%8.8X%8.8X, using 32",
> +				   acpi_gbl_FADT.facs,
> +				   ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xfacs)));
> +
> +		acpi_gbl_FADT.Xfacs = (u64)acpi_gbl_FADT.facs;
> +	}
> +
> +	if (acpi_gbl_FADT.dsdt &&
> +	    (acpi_gbl_FADT.Xdsdt != (u64)acpi_gbl_FADT.dsdt)) {
> +		ACPI_BIOS_WARNING((AE_INFO,
> +				   "32/64X DSDT address mismatch in FADT - "
> +				   "0x%8.8X/0x%8.8X%8.8X, using 32",
> +				   acpi_gbl_FADT.dsdt,
> +				   ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xdsdt)));
> 
> -	acpi_gbl_FADT.Xdsdt = acpi_tb_select_address("DSDT",
> -						     acpi_gbl_FADT.dsdt,
> -						     acpi_gbl_FADT.Xdsdt);
> +		acpi_gbl_FADT.Xdsdt = (u64)acpi_gbl_FADT.dsdt;
> +	}
> 
>  	/* If Hardware Reduced flag is set, we are all done */
> 
> @@ -499,95 +545,18 @@ static void acpi_tb_convert_fadt(void)
> 
>  	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
>  		/*
> -		 * Get the 32-bit and 64-bit addresses, as well as the register
> -		 * length and register name.
> +		 * Generate pointer to the 64-bit address, get the register
> +		 * length (width) and the register name
>  		 */
> -		address32 = *ACPI_ADD_PTR(u32,
> -					  &acpi_gbl_FADT,
> -					  fadt_info_table[i].address32);
> -
>  		address64 = ACPI_ADD_PTR(struct acpi_generic_address,
>  					 &acpi_gbl_FADT,
>  					 fadt_info_table[i].address64);
> -
> -		length = *ACPI_ADD_PTR(u8,
> -				       &acpi_gbl_FADT,
> -				       fadt_info_table[i].length);
> -
> +		length =
> +		    *ACPI_ADD_PTR(u8, &acpi_gbl_FADT,
> +				  fadt_info_table[i].length);
>  		name = fadt_info_table[i].name;
> 
>  		/*
> -		 * Expand the ACPI 1.0 32-bit addresses to the ACPI 2.0 64-bit "X"
> -		 * generic address structures as necessary. Later code will always use
> -		 * the 64-bit address structures.
> -		 *
> -		 * November 2013:
> -		 * Now always use the 64-bit address if it is valid (non-zero), in
> -		 * accordance with the ACPI specification which states that a 64-bit
> -		 * address supersedes the 32-bit version. This behavior can be
> -		 * overridden by the acpi_gbl_use32_bit_fadt_addresses flag.
> -		 *
> -		 * During 64-bit address construction and verification,
> -		 * these cases are handled:
> -		 *
> -		 * Address32 zero, Address64 [don't care]   - Use Address64
> -		 *
> -		 * Address32 non-zero, Address64 zero       - Copy/use Address32
> -		 * Address32 non-zero == Address64 non-zero - Use Address64
> -		 * Address32 non-zero != Address64 non-zero - Warning, use Address64
> -		 *
> -		 * Override: if acpi_gbl_use32_bit_fadt_addresses is TRUE, and:
> -		 * Address32 non-zero != Address64 non-zero - Warning, copy/use Address32
> -		 *
> -		 * Note: space_id is always I/O for 32-bit legacy address fields
> -		 */
> -		if (address32) {
> -			if (!address64->address) {
> -
> -				/* 64-bit address is zero, use 32-bit address */
> -
> -				acpi_tb_init_generic_address(address64,
> -							     ACPI_ADR_SPACE_SYSTEM_IO,
> -							     *ACPI_ADD_PTR(u8,
> -									   &acpi_gbl_FADT,
> -									   fadt_info_table
> -									   [i].
> -									   length),
> -							     (u64)address32,
> -							     name);
> -			} else if (address64->address != (u64)address32) {
> -
> -				/* Address mismatch */
> -
> -				ACPI_BIOS_WARNING((AE_INFO,
> -						   "32/64X address mismatch in FADT/%s: "
> -						   "0x%8.8X/0x%8.8X%8.8X, using %u-bit address",
> -						   name, address32,
> -						   ACPI_FORMAT_UINT64
> -						   (address64->address),
> -						   acpi_gbl_use32_bit_fadt_addresses
> -						   ? 32 : 64));
> -
> -				if (acpi_gbl_use32_bit_fadt_addresses) {
> -
> -					/* 32-bit address override */
> -
> -					acpi_tb_init_generic_address(address64,
> -								     ACPI_ADR_SPACE_SYSTEM_IO,
> -								     *ACPI_ADD_PTR
> -								     (u8,
> -								      &acpi_gbl_FADT,
> -								      fadt_info_table
> -								      [i].
> -								      length),
> -								     (u64)
> -								     address32,
> -								     name);
> -				}
> -			}
> -		}
> -
> -		/*
>  		 * For each extended field, check for length mismatch between the
>  		 * legacy length field and the corresponding 64-bit X length field.
>  		 * Note: If the legacy length field is > 0xFF bits, ignore this
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index 44f5e9749601..40d1bc88cc14 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -82,7 +82,6 @@ extern u8 acpi_gbl_enable_interpreter_slack;
>  extern u32 acpi_gbl_trace_flags;
>  extern acpi_name acpi_gbl_trace_method_name;
>  extern u8 acpi_gbl_truncate_io_addresses;
> -extern u8 acpi_gbl_use32_bit_fadt_addresses;
>  extern u8 acpi_gbl_use_default_register_widths;
> 
>  /*
> --
> 1.8.4.5
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[Zheng, Lv]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 22000 bytes --]

Hi, Rafael

I checked the bug.

The dmesg of the kernel without the bisected commit:
[    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20140214/tbprint-218)
[    0.000000] ACPI Warning: 32/64 FACS address mismatch in FADT - two FACS tables! (20140214/tbfadt-395)
[    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT - 0xCF661F40/0x00000000CF667E40, using 32 (20140214/tbfadt-522)

The dmesg of the kernel with the bisected commit:
[    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20131218/tbprint-214)
[    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xCF661F40/0x00000000CF667E40, using 64-bit address (20131218/tbfadt-271)

This is the purpose of the bisected commit.
According to the link below:
http://bugs.acpica.org/show_bug.cgi?id=885
And Windows documentation:
http://download.microsoft.com/download/5/b/9/5b97017b-e28a-4bae-ba48-174cf47d23cd/CPA002_WH06.ppt
We believe 64-bit addresses should be used by default so that new features can be enabled according to the public knowledge of Windows Vista+ behavior.
For old Windows, it's hard for us to guess, we should wait for the reports and add quirks for them.

Thus this commit is not wrong, it shouldn't be reverted.
Though this platform is newer than vista, we still should offer a quirk mechanism for it as a quick fix:
	Release Date: 01/21/2010

What's your opinion?

Thanks and best regards
-Lv

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Zheng, Lv
> Sent: Monday, May 12, 2014 9:10 AM
> 
> > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > Sent: Monday, May 12, 2014 8:12 AM
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Revert commit 0249ed2444d6 (ACPICA: Add option to favor 32-bit FADT
> > addresses.) that breaks resume from system suspend on the Intel DP45SG
> > board.
> 
> This commit doesn't look like the root cause.
> [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20131218/tbprint-214)
> [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xCF661F40/0x00000000CF667E40, using 64-bit address
> (20131218/tbfadt-271)
> It seems before using the FADT, ACPICA have already detected that the XSDT is not correct.
> 
> Please revert it for now.  I'll take a look at this bug.
> 
> Thanks and best regards
> -Lv
> 
> >
> > Fixes: 0249ed2444d6 (ACPICA: Add option to favor 32-bit FADT addresses.)
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=74021
> > Reported-and-tested-by: Oswald Buddenhagen <ossi@kde.org>
> > Cc: 3.14+ <stable@vger.kernel.org> # 3.14+
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/acpica/acglobal.h |  10 --
> >  drivers/acpi/acpica/tbfadt.c   | 335 +++++++++++++++++++----------------------
> >  include/acpi/acpixf.h          |   1 -
> >  3 files changed, 152 insertions(+), 194 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> > index 49bbc71fad54..72578a4b8212 100644
> > --- a/drivers/acpi/acpica/acglobal.h
> > +++ b/drivers/acpi/acpica/acglobal.h
> > @@ -136,16 +136,6 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_copy_dsdt_locally, FALSE);
> >  ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
> >
> >  /*
> > - * Optionally use 32-bit FADT addresses if and when there is a conflict
> > - * (address mismatch) between the 32-bit and 64-bit versions of the
> > - * address. Although ACPICA adheres to the ACPI specification which
> > - * requires the use of the corresponding 64-bit address if it is non-zero,
> > - * some machines have been found to have a corrupted non-zero 64-bit
> > - * address. Default is FALSE, do not favor the 32-bit addresses.
> > - */
> > -ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, FALSE);
> > -
> > -/*
> >   * 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
> > diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
> > index ec14588254d4..6dd5c590e0bb 100644
> > --- a/drivers/acpi/acpica/tbfadt.c
> > +++ b/drivers/acpi/acpica/tbfadt.c
> > @@ -56,10 +56,9 @@ acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
> >
> >  static void acpi_tb_convert_fadt(void);
> >
> > -static void acpi_tb_setup_fadt_registers(void);
> > +static void acpi_tb_validate_fadt(void);
> >
> > -static u64
> > -acpi_tb_select_address(char *register_name, u32 address32, u64 address64);
> > +static void acpi_tb_setup_fadt_registers(void);
> >
> >  /* Table for conversion of FADT to common internal format and FADT validation */
> >
> > @@ -176,7 +175,6 @@ static struct acpi_fadt_pm_info fadt_pm_info_table[] = {
> >   *              space_id            - ACPI Space ID for this register
> >   *              byte_width          - Width of this register
> >   *              address             - Address of the register
> > - *              register_name       - ASCII name of the ACPI register
> >   *
> >   * RETURN:      None
> >   *
> > @@ -222,68 +220,6 @@ acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
> >
> >  /*******************************************************************************
> >   *
> > - * FUNCTION:    acpi_tb_select_address
> > - *
> > - * PARAMETERS:  register_name       - ASCII name of the ACPI register
> > - *              address32           - 32-bit address of the register
> > - *              address64           - 64-bit address of the register
> > - *
> > - * RETURN:      The resolved 64-bit address
> > - *
> > - * DESCRIPTION: Select between 32-bit and 64-bit versions of addresses within
> > - *              the FADT. Used for the FACS and DSDT addresses.
> > - *
> > - * NOTES:
> > - *
> > - * Check for FACS and DSDT address mismatches. An address mismatch between
> > - * the 32-bit and 64-bit address fields (FIRMWARE_CTRL/X_FIRMWARE_CTRL and
> > - * DSDT/X_DSDT) could be a corrupted address field or it might indicate
> > - * the presence of two FACS or two DSDT tables.
> > - *
> > - * November 2013:
> > - * By default, as per the ACPICA specification, a valid 64-bit address is
> > - * used regardless of the value of the 32-bit address. However, this
> > - * behavior can be overridden via the acpi_gbl_use32_bit_fadt_addresses flag.
> > - *
> > - ******************************************************************************/
> > -
> > -static u64
> > -acpi_tb_select_address(char *register_name, u32 address32, u64 address64)
> > -{
> > -
> > -	if (!address64) {
> > -
> > -		/* 64-bit address is zero, use 32-bit address */
> > -
> > -		return ((u64)address32);
> > -	}
> > -
> > -	if (address32 && (address64 != (u64)address32)) {
> > -
> > -		/* Address mismatch between 32-bit and 64-bit versions */
> > -
> > -		ACPI_BIOS_WARNING((AE_INFO,
> > -				   "32/64X %s address mismatch in FADT: "
> > -				   "0x%8.8X/0x%8.8X%8.8X, using %u-bit address",
> > -				   register_name, address32,
> > -				   ACPI_FORMAT_UINT64(address64),
> > -				   acpi_gbl_use32_bit_fadt_addresses ? 32 :
> > -				   64));
> > -
> > -		/* 32-bit address override */
> > -
> > -		if (acpi_gbl_use32_bit_fadt_addresses) {
> > -			return ((u64)address32);
> > -		}
> > -	}
> > -
> > -	/* Default is to use the 64-bit address */
> > -
> > -	return (address64);
> > -}
> > -
> > -/*******************************************************************************
> > - *
> >   * FUNCTION:    acpi_tb_parse_fadt
> >   *
> >   * PARAMETERS:  table_index         - Index for the FADT
> > @@ -395,6 +331,10 @@ void acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 length)
> >
> >  	acpi_tb_convert_fadt();
> >
> > +	/* Validate FADT values now, before we make any changes */
> > +
> > +	acpi_tb_validate_fadt();
> > +
> >  	/* Initialize the global ACPI register structures */
> >
> >  	acpi_tb_setup_fadt_registers();
> > @@ -404,55 +344,66 @@ void acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 length)
> >   *
> >   * FUNCTION:    acpi_tb_convert_fadt
> >   *
> > - * PARAMETERS:  none - acpi_gbl_FADT is used.
> > + * PARAMETERS:  None, uses acpi_gbl_FADT
> >   *
> >   * RETURN:      None
> >   *
> >   * DESCRIPTION: Converts all versions of the FADT to a common internal format.
> > - *              Expand 32-bit addresses to 64-bit as necessary. Also validate
> > - *              important fields within the FADT.
> > + *              Expand 32-bit addresses to 64-bit as necessary.
> >   *
> > - * NOTE:        acpi_gbl_FADT must be of size (struct acpi_table_fadt), and must
> > - *              contain a copy of the actual BIOS-provided FADT.
> > + * NOTE:        acpi_gbl_FADT must be of size (struct acpi_table_fadt),
> > + *              and must contain a copy of the actual FADT.
> >   *
> >   * Notes on 64-bit register addresses:
> >   *
> >   * After this FADT conversion, later ACPICA code will only use the 64-bit "X"
> >   * fields of the FADT for all ACPI register addresses.
> >   *
> > - * The 64-bit X fields are optional extensions to the original 32-bit FADT
> > + * The 64-bit "X" fields are optional extensions to the original 32-bit FADT
> >   * V1.0 fields. Even if they are present in the FADT, they are optional and
> >   * are unused if the BIOS sets them to zero. Therefore, we must copy/expand
> > - * 32-bit V1.0 fields to the 64-bit X fields if the the 64-bit X field is
> > - * originally zero.
> > + * 32-bit V1.0 fields if the corresponding X field is zero.
> >   *
> > - * For ACPI 1.0 FADTs (that contain no 64-bit addresses), all 32-bit address
> > - * fields are expanded to the corresponding 64-bit X fields in the internal
> > - * common FADT.
> > + * For ACPI 1.0 FADTs, all 32-bit address fields are expanded to the
> > + * corresponding "X" fields in the internal FADT.
> >   *
> >   * For ACPI 2.0+ FADTs, all valid (non-zero) 32-bit address fields are expanded
> > - * to the corresponding 64-bit X fields, if the 64-bit field is originally
> > - * zero. Adhering to the ACPI specification, we completely ignore the 32-bit
> > - * field if the 64-bit field is valid, regardless of whether the host OS is
> > - * 32-bit or 64-bit.
> > - *
> > - * Possible additional checks:
> > - *  (acpi_gbl_FADT.pm1_event_length >= 4)
> > - *  (acpi_gbl_FADT.pm1_control_length >= 2)
> > - *  (acpi_gbl_FADT.pm_timer_length >= 4)
> > - *  Gpe block lengths must be multiple of 2
> > + * to the corresponding 64-bit X fields. For compatibility with other ACPI
> > + * implementations, we ignore the 64-bit field if the 32-bit field is valid,
> > + * regardless of whether the host OS is 32-bit or 64-bit.
> >   *
> >   ******************************************************************************/
> >
> >  static void acpi_tb_convert_fadt(void)
> >  {
> > -	char *name;
> >  	struct acpi_generic_address *address64;
> >  	u32 address32;
> > -	u8 length;
> >  	u32 i;
> >
> >  	/*
> > +	 * Expand the 32-bit FACS and DSDT addresses to 64-bit as necessary.
> > +	 * Later code will always use the X 64-bit field. Also, check for an
> > +	 * address mismatch between the 32-bit and 64-bit address fields
> > +	 * (FIRMWARE_CTRL/X_FIRMWARE_CTRL, DSDT/X_DSDT) which would indicate
> > +	 * the presence of two FACS or two DSDT tables.
> > +	 */
> > +	if (!acpi_gbl_FADT.Xfacs) {
> > +		acpi_gbl_FADT.Xfacs = (u64) acpi_gbl_FADT.facs;
> > +	} else if (acpi_gbl_FADT.facs &&
> > +		   (acpi_gbl_FADT.Xfacs != (u64) acpi_gbl_FADT.facs)) {
> > +		ACPI_WARNING((AE_INFO,
> > +		    "32/64 FACS address mismatch in FADT - two FACS tables!"));
> > +	}
> > +
> > +	if (!acpi_gbl_FADT.Xdsdt) {
> > +		acpi_gbl_FADT.Xdsdt = (u64) acpi_gbl_FADT.dsdt;
> > +	} else if (acpi_gbl_FADT.dsdt &&
> > +		   (acpi_gbl_FADT.Xdsdt != (u64) acpi_gbl_FADT.dsdt)) {
> > +		ACPI_WARNING((AE_INFO,
> > +		    "32/64 DSDT address mismatch in FADT - two DSDT tables!"));
> > +	}
> > +
> > +	/*
> >  	 * For ACPI 1.0 FADTs (revision 1 or 2), ensure that reserved fields which
> >  	 * should be zero are indeed zero. This will workaround BIOSs that
> >  	 * inadvertently place values in these fields.
> > @@ -470,24 +421,119 @@ static void acpi_tb_convert_fadt(void)
> >  		acpi_gbl_FADT.boot_flags = 0;
> >  	}
> >
> > +	/* Update the local FADT table header length */
> > +
> > +	acpi_gbl_FADT.header.length = sizeof(struct acpi_table_fadt);
> > +
> >  	/*
> > -	 * Now we can update the local FADT length to the length of the
> > -	 * current FADT version as defined by the ACPI specification.
> > -	 * Thus, we will have a common FADT internally.
> > +	 * Expand the ACPI 1.0 32-bit addresses to the ACPI 2.0 64-bit "X"
> > +	 * generic address structures as necessary. Later code will always use
> > +	 * the 64-bit address structures.
> > +	 *
> > +	 * March 2009:
> > +	 * We now always use the 32-bit address if it is valid (non-null). This
> > +	 * is not in accordance with the ACPI specification which states that
> > +	 * the 64-bit address supersedes the 32-bit version, but we do this for
> > +	 * compatibility with other ACPI implementations. Most notably, in the
> > +	 * case where both the 32 and 64 versions are non-null, we use the 32-bit
> > +	 * version. This is the only address that is guaranteed to have been
> > +	 * tested by the BIOS manufacturer.
> >  	 */
> > -	acpi_gbl_FADT.header.length = sizeof(struct acpi_table_fadt);
> > +	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
> > +		address32 = *ACPI_ADD_PTR(u32,
> > +					  &acpi_gbl_FADT,
> > +					  fadt_info_table[i].address32);
> > +
> > +		address64 = ACPI_ADD_PTR(struct acpi_generic_address,
> > +					 &acpi_gbl_FADT,
> > +					 fadt_info_table[i].address64);
> > +
> > +		/*
> > +		 * If both 32- and 64-bit addresses are valid (non-zero),
> > +		 * they must match.
> > +		 */
> > +		if (address64->address && address32 &&
> > +		    (address64->address != (u64)address32)) {
> > +			ACPI_BIOS_ERROR((AE_INFO,
> > +					 "32/64X address mismatch in FADT/%s: "
> > +					 "0x%8.8X/0x%8.8X%8.8X, using 32",
> > +					 fadt_info_table[i].name, address32,
> > +					 ACPI_FORMAT_UINT64(address64->
> > +							    address)));
> > +		}
> > +
> > +		/* Always use 32-bit address if it is valid (non-null) */
> > +
> > +		if (address32) {
> > +			/*
> > +			 * Copy the 32-bit address to the 64-bit GAS structure. The
> > +			 * Space ID is always I/O for 32-bit legacy address fields
> > +			*/
> > +			acpi_tb_init_generic_address(address64,
> > +						     ACPI_ADR_SPACE_SYSTEM_IO,
> > +						     *ACPI_ADD_PTR(u8,
> > +								   &acpi_gbl_FADT,
> > +								   fadt_info_table
> > +								   [i].length),
> > +						     (u64) address32,
> > +						     fadt_info_table[i].name);
> > +		}
> > +	}
> > +}
> > +
> > +/*******************************************************************************
> > + *
> > + * FUNCTION:    acpi_tb_validate_fadt
> > + *
> > + * PARAMETERS:  table           - Pointer to the FADT to be validated
> > + *
> > + * RETURN:      None
> > + *
> > + * DESCRIPTION: Validate various important fields within the FADT. If a problem
> > + *              is found, issue a message, but no status is returned.
> > + *              Used by both the table manager and the disassembler.
> > + *
> > + * Possible additional checks:
> > + * (acpi_gbl_FADT.pm1_event_length >= 4)
> > + * (acpi_gbl_FADT.pm1_control_length >= 2)
> > + * (acpi_gbl_FADT.pm_timer_length >= 4)
> > + * Gpe block lengths must be multiple of 2
> > + *
> > + ******************************************************************************/
> > +
> > +static void acpi_tb_validate_fadt(void)
> > +{
> > +	char *name;
> > +	struct acpi_generic_address *address64;
> > +	u8 length;
> > +	u32 i;
> >
> >  	/*
> > -	 * Expand the 32-bit FACS and DSDT addresses to 64-bit as necessary.
> > -	 * Later ACPICA code will always use the X 64-bit field.
> > +	 * Check for FACS and DSDT address mismatches. An address mismatch between
> > +	 * the 32-bit and 64-bit address fields (FIRMWARE_CTRL/X_FIRMWARE_CTRL and
> > +	 * DSDT/X_DSDT) would indicate the presence of two FACS or two DSDT tables.
> >  	 */
> > -	acpi_gbl_FADT.Xfacs = acpi_tb_select_address("FACS",
> > -						     acpi_gbl_FADT.facs,
> > -						     acpi_gbl_FADT.Xfacs);
> > +	if (acpi_gbl_FADT.facs &&
> > +	    (acpi_gbl_FADT.Xfacs != (u64)acpi_gbl_FADT.facs)) {
> > +		ACPI_BIOS_WARNING((AE_INFO,
> > +				   "32/64X FACS address mismatch in FADT - "
> > +				   "0x%8.8X/0x%8.8X%8.8X, using 32",
> > +				   acpi_gbl_FADT.facs,
> > +				   ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xfacs)));
> > +
> > +		acpi_gbl_FADT.Xfacs = (u64)acpi_gbl_FADT.facs;
> > +	}
> > +
> > +	if (acpi_gbl_FADT.dsdt &&
> > +	    (acpi_gbl_FADT.Xdsdt != (u64)acpi_gbl_FADT.dsdt)) {
> > +		ACPI_BIOS_WARNING((AE_INFO,
> > +				   "32/64X DSDT address mismatch in FADT - "
> > +				   "0x%8.8X/0x%8.8X%8.8X, using 32",
> > +				   acpi_gbl_FADT.dsdt,
> > +				   ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xdsdt)));
> >
> > -	acpi_gbl_FADT.Xdsdt = acpi_tb_select_address("DSDT",
> > -						     acpi_gbl_FADT.dsdt,
> > -						     acpi_gbl_FADT.Xdsdt);
> > +		acpi_gbl_FADT.Xdsdt = (u64)acpi_gbl_FADT.dsdt;
> > +	}
> >
> >  	/* If Hardware Reduced flag is set, we are all done */
> >
> > @@ -499,95 +545,18 @@ static void acpi_tb_convert_fadt(void)
> >
> >  	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
> >  		/*
> > -		 * Get the 32-bit and 64-bit addresses, as well as the register
> > -		 * length and register name.
> > +		 * Generate pointer to the 64-bit address, get the register
> > +		 * length (width) and the register name
> >  		 */
> > -		address32 = *ACPI_ADD_PTR(u32,
> > -					  &acpi_gbl_FADT,
> > -					  fadt_info_table[i].address32);
> > -
> >  		address64 = ACPI_ADD_PTR(struct acpi_generic_address,
> >  					 &acpi_gbl_FADT,
> >  					 fadt_info_table[i].address64);
> > -
> > -		length = *ACPI_ADD_PTR(u8,
> > -				       &acpi_gbl_FADT,
> > -				       fadt_info_table[i].length);
> > -
> > +		length =
> > +		    *ACPI_ADD_PTR(u8, &acpi_gbl_FADT,
> > +				  fadt_info_table[i].length);
> >  		name = fadt_info_table[i].name;
> >
> >  		/*
> > -		 * Expand the ACPI 1.0 32-bit addresses to the ACPI 2.0 64-bit "X"
> > -		 * generic address structures as necessary. Later code will always use
> > -		 * the 64-bit address structures.
> > -		 *
> > -		 * November 2013:
> > -		 * Now always use the 64-bit address if it is valid (non-zero), in
> > -		 * accordance with the ACPI specification which states that a 64-bit
> > -		 * address supersedes the 32-bit version. This behavior can be
> > -		 * overridden by the acpi_gbl_use32_bit_fadt_addresses flag.
> > -		 *
> > -		 * During 64-bit address construction and verification,
> > -		 * these cases are handled:
> > -		 *
> > -		 * Address32 zero, Address64 [don't care]   - Use Address64
> > -		 *
> > -		 * Address32 non-zero, Address64 zero       - Copy/use Address32
> > -		 * Address32 non-zero == Address64 non-zero - Use Address64
> > -		 * Address32 non-zero != Address64 non-zero - Warning, use Address64
> > -		 *
> > -		 * Override: if acpi_gbl_use32_bit_fadt_addresses is TRUE, and:
> > -		 * Address32 non-zero != Address64 non-zero - Warning, copy/use Address32
> > -		 *
> > -		 * Note: space_id is always I/O for 32-bit legacy address fields
> > -		 */
> > -		if (address32) {
> > -			if (!address64->address) {
> > -
> > -				/* 64-bit address is zero, use 32-bit address */
> > -
> > -				acpi_tb_init_generic_address(address64,
> > -							     ACPI_ADR_SPACE_SYSTEM_IO,
> > -							     *ACPI_ADD_PTR(u8,
> > -									   &acpi_gbl_FADT,
> > -									   fadt_info_table
> > -									   [i].
> > -									   length),
> > -							     (u64)address32,
> > -							     name);
> > -			} else if (address64->address != (u64)address32) {
> > -
> > -				/* Address mismatch */
> > -
> > -				ACPI_BIOS_WARNING((AE_INFO,
> > -						   "32/64X address mismatch in FADT/%s: "
> > -						   "0x%8.8X/0x%8.8X%8.8X, using %u-bit address",
> > -						   name, address32,
> > -						   ACPI_FORMAT_UINT64
> > -						   (address64->address),
> > -						   acpi_gbl_use32_bit_fadt_addresses
> > -						   ? 32 : 64));
> > -
> > -				if (acpi_gbl_use32_bit_fadt_addresses) {
> > -
> > -					/* 32-bit address override */
> > -
> > -					acpi_tb_init_generic_address(address64,
> > -								     ACPI_ADR_SPACE_SYSTEM_IO,
> > -								     *ACPI_ADD_PTR
> > -								     (u8,
> > -								      &acpi_gbl_FADT,
> > -								      fadt_info_table
> > -								      [i].
> > -								      length),
> > -								     (u64)
> > -								     address32,
> > -								     name);
> > -				}
> > -			}
> > -		}
> > -
> > -		/*
> >  		 * For each extended field, check for length mismatch between the
> >  		 * legacy length field and the corresponding 64-bit X length field.
> >  		 * Note: If the legacy length field is > 0xFF bits, ignore this
> > diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> > index 44f5e9749601..40d1bc88cc14 100644
> > --- a/include/acpi/acpixf.h
> > +++ b/include/acpi/acpixf.h
> > @@ -82,7 +82,6 @@ extern u8 acpi_gbl_enable_interpreter_slack;
> >  extern u32 acpi_gbl_trace_flags;
> >  extern acpi_name acpi_gbl_trace_method_name;
> >  extern u8 acpi_gbl_truncate_io_addresses;
> > -extern u8 acpi_gbl_use32_bit_fadt_addresses;
> >  extern u8 acpi_gbl_use_default_register_widths;
> >
> >  /*
> > --
> > 1.8.4.5
> >
> 
> \x04�{.n�+�������+%��lzwm��b�맲��r��zX��\x16��(��\x17��ܨ}���Ơz�&j:+v���\r����zZ+��+zf���h���~����i���z�\x1e�w���?����&�)
> ߢ^[f
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[Rafael J. Wysocki]

On Monday, May 12, 2014 08:51:36 AM Zheng, Lv wrote:
> Hi, Rafael
> 
> I checked the bug.
> 
> The dmesg of the kernel without the bisected commit:
> [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20140214/tbprint-218)
> [    0.000000] ACPI Warning: 32/64 FACS address mismatch in FADT - two FACS tables! (20140214/tbfadt-395)
> [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT - 0xCF661F40/0x00000000CF667E40, using 32 (20140214/tbfadt-522)
> 
> The dmesg of the kernel with the bisected commit:
> [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20131218/tbprint-214)
> [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xCF661F40/0x00000000CF667E40, using 64-bit address (20131218/tbfadt-271)
> 
> This is the purpose of the bisected commit.
> According to the link below:
> http://bugs.acpica.org/show_bug.cgi?id=885
> And Windows documentation:
> http://download.microsoft.com/download/5/b/9/5b97017b-e28a-4bae-ba48-174cf47d23cd/CPA002_WH06.ppt
> We believe 64-bit addresses should be used by default so that new features can be enabled according to the public knowledge of Windows Vista+ behavior.
> For old Windows, it's hard for us to guess, we should wait for the reports and add quirks for them.
> 
> Thus this commit is not wrong, it shouldn't be reverted.

It is wrong, because it breaks a system that worked without it.

It's *that* simple.

And either you have a fix for that (which is not a quirk, because there may be
more machines like that), or we have to revert it.

> Though this platform is newer than vista, we still should offer a quirk mechanism
> for it as a quick fix:

We didn't need a quirk for it before, though.

So really, I'm reverting it.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[Zheng, Lv]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2806 bytes --]

Hi,

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Tuesday, May 13, 2014 8:09 AM
> 
> On Monday, May 12, 2014 08:51:36 AM Zheng, Lv wrote:
> > Hi, Rafael
> >
> > I checked the bug.
> >
> > The dmesg of the kernel without the bisected commit:
> > [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20140214/tbprint-218)
> > [    0.000000] ACPI Warning: 32/64 FACS address mismatch in FADT - two FACS tables! (20140214/tbfadt-395)
> > [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT - 0xCF661F40/0x00000000CF667E40, using 32
> (20140214/tbfadt-522)
> >
> > The dmesg of the kernel with the bisected commit:
> > [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20131218/tbprint-214)
> > [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xCF661F40/0x00000000CF667E40, using 64-bit
> address (20131218/tbfadt-271)
> >
> > This is the purpose of the bisected commit.
> > According to the link below:
> > http://bugs.acpica.org/show_bug.cgi?id=885
> > And Windows documentation:
> > http://download.microsoft.com/download/5/b/9/5b97017b-e28a-4bae-ba48-174cf47d23cd/CPA002_WH06.ppt
> > We believe 64-bit addresses should be used by default so that new features can be enabled according to the public knowledge of
> Windows Vista+ behavior.
> > For old Windows, it's hard for us to guess, we should wait for the reports and add quirks for them.
> >
> > Thus this commit is not wrong, it shouldn't be reverted.
> 
> It is wrong, because it breaks a system that worked without it.
> 
> It's *that* simple.

For this commit, we knew there would be systems broken.
And was prepared to add quirks for them.
The quirks are not there just because we rely on end users to report.

> 
> And either you have a fix for that (which is not a quirk, because there may be
> more machines like that), or we have to revert it.
> 
> > Though this platform is newer than vista, we still should offer a quirk mechanism
> > for it as a quick fix:
> 
> We didn't need a quirk for it before, though.

But according to BZ885, we need more quirks for other machines before.
For example, ThinkPad 40e and ThinkPad 51e reported in the BZ885.

> 
> So really, I'm reverting it.

OK.
I'll first try to figure out the cause of the issue that is happening to Intel DP45SG.
And then try this approach again in a smarter way that is more tolerant torward the possible regressions.

Thanks and best regards
-Lv

> 
> Thanks!
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[Rafael J. Wysocki]

On Tuesday, May 13, 2014 01:05:59 AM Zheng, Lv wrote:
> Hi,
> 
> > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > Sent: Tuesday, May 13, 2014 8:09 AM
> > 
> > On Monday, May 12, 2014 08:51:36 AM Zheng, Lv wrote:
> > > Hi, Rafael
> > >
> > > I checked the bug.
> > >
> > > The dmesg of the kernel without the bisected commit:
> > > [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20140214/tbprint-218)
> > > [    0.000000] ACPI Warning: 32/64 FACS address mismatch in FADT - two FACS tables! (20140214/tbfadt-395)
> > > [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT - 0xCF661F40/0x00000000CF667E40, using 32
> > (20140214/tbfadt-522)
> > >
> > > The dmesg of the kernel with the bisected commit:
> > > [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20131218/tbprint-214)
> > > [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xCF661F40/0x00000000CF667E40, using 64-bit
> > address (20131218/tbfadt-271)
> > >
> > > This is the purpose of the bisected commit.
> > > According to the link below:
> > > http://bugs.acpica.org/show_bug.cgi?id=885
> > > And Windows documentation:
> > > http://download.microsoft.com/download/5/b/9/5b97017b-e28a-4bae-ba48-174cf47d23cd/CPA002_WH06.ppt
> > > We believe 64-bit addresses should be used by default so that new features can be enabled according to the public knowledge of
> > Windows Vista+ behavior.
> > > For old Windows, it's hard for us to guess, we should wait for the reports and add quirks for them.
> > >
> > > Thus this commit is not wrong, it shouldn't be reverted.
> > 
> > It is wrong, because it breaks a system that worked without it.
> > 
> > It's *that* simple.
> 
> For this commit, we knew there would be systems broken.
> And was prepared to add quirks for them.
> The quirks are not there just because we rely on end users to report.
> 
> > 
> > And either you have a fix for that (which is not a quirk, because there may be
> > more machines like that), or we have to revert it.
> > 
> > > Though this platform is newer than vista, we still should offer a quirk mechanism
> > > for it as a quick fix:
> > 
> > We didn't need a quirk for it before, though.
> 
> But according to BZ885, we need more quirks for other machines before.
> For example, ThinkPad 40e and ThinkPad 51e reported in the BZ885.
> 
> > 
> > So really, I'm reverting it.
> 
> OK.
> I'll first try to figure out the cause of the issue that is happening to Intel DP45SG.
> And then try this approach again in a smarter way that is more tolerant torward the possible regressions.

Great, thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."

[Zheng, Lv]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3707 bytes --]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Tuesday, May 13, 2014 9:31 AM
> To: Zheng, Lv
> Cc: RobertBMoore@compuserve.com; Thomas Renninger (trenn@suse.de); Oswald Buddenhagen; ACPI Devel Maling List; Linux
> Kernel Mailing List
> Subject: Re: [PATCH] ACPICA: Revert "ACPICA: Add option to favor 32-bit FADT addresses."
> 
> On Tuesday, May 13, 2014 01:05:59 AM Zheng, Lv wrote:
> > Hi,
> >
> > > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > > Sent: Tuesday, May 13, 2014 8:09 AM
> > >
> > > On Monday, May 12, 2014 08:51:36 AM Zheng, Lv wrote:
> > > > Hi, Rafael
> > > >
> > > > I checked the bug.
> > > >
> > > > The dmesg of the kernel without the bisected commit:
> > > > [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20140214/tbprint-218)
> > > > [    0.000000] ACPI Warning: 32/64 FACS address mismatch in FADT - two FACS tables! (20140214/tbfadt-395)
> > > > [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT - 0xCF661F40/0x00000000CF667E40, using 32
> > > (20140214/tbfadt-522)
> > > >
> > > > The dmesg of the kernel with the bisected commit:
> > > > [    0.000000] ACPI BIOS Warning (bug): Incorrect checksum in table [XSDT] - 0xA0, should be 0xC9 (20131218/tbprint-214)
> > > > [    0.000000] ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xCF661F40/0x00000000CF667E40, using 64-bit
> > > address (20131218/tbfadt-271)
> > > >
> > > > This is the purpose of the bisected commit.
> > > > According to the link below:
> > > > http://bugs.acpica.org/show_bug.cgi?id=885
> > > > And Windows documentation:
> > > > http://download.microsoft.com/download/5/b/9/5b97017b-e28a-4bae-ba48-174cf47d23cd/CPA002_WH06.ppt
> > > > We believe 64-bit addresses should be used by default so that new features can be enabled according to the public knowledge
> of
> > > Windows Vista+ behavior.
> > > > For old Windows, it's hard for us to guess, we should wait for the reports and add quirks for them.
> > > >
> > > > Thus this commit is not wrong, it shouldn't be reverted.
> > >
> > > It is wrong, because it breaks a system that worked without it.
> > >
> > > It's *that* simple.
> >
> > For this commit, we knew there would be systems broken.
> > And was prepared to add quirks for them.
> > The quirks are not there just because we rely on end users to report.
> >
> > >
> > > And either you have a fix for that (which is not a quirk, because there may be
> > > more machines like that), or we have to revert it.
> > >
> > > > Though this platform is newer than vista, we still should offer a quirk mechanism
> > > > for it as a quick fix:
> > >
> > > We didn't need a quirk for it before, though.
> >
> > But according to BZ885, we need more quirks for other machines before.
> > For example, ThinkPad 40e and ThinkPad 51e reported in the BZ885.
> >
> > >
> > > So really, I'm reverting it.
> >
> > OK.
> > I'll first try to figure out the cause of the issue that is happening to Intel DP45SG.
> > And then try this approach again in a smarter way that is more tolerant torward the possible regressions.
> 
> Great, thanks!

If you have troubles in reverting things.
I can offer one line patch to revert the functionality of the bisected commit back to the original behavior.
In acpixf.h, make acpi_gbl_use32_bit_fadt_address to TRUE can be a fix.
I'll ask for the test on the bugzilla.

Thanks and best regards
-Lv

> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH] Tables: Restore old behavor to favor 32-bit FADT addresses.

[Lv Zheng]

We need to find a smarter way to switch to 64-bit FADT addresses according
to the bug report.  This patch reverts Linux to the original behavior.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=74021
Reported-by: Oswald Buddenhagen <ossi@kde.org>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Oswald Buddenhagen <ossi@kde.org>
Cc: <stable@vger.kernel.org> # 3.14.x: 0249ed24: ACPICA: Add option to favor 32-bit FADT addresses
---
 include/acpi/acpixf.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 4e3044c..b66b6a8 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -190,7 +190,7 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
  * some machines have been found to have a corrupted non-zero 64-bit
  * address. Default is FALSE, do not favor the 32-bit addresses.
  */
-ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, FALSE);
+ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, TRUE);
 
 /*
  * Optionally truncate I/O addresses to 16 bits. Provides compatibility
-- 
1.7.10

Re: [PATCH] Tables: Restore old behavor to favor 32-bit FADT addresses.

[Rafael J. Wysocki]

Hi Lv,

There was a merge conflict with the new ACPICA material queued up for 3.16
related to this.

Can you please have a look at the linux-next branch in my tree and see if
the ACPICA material in there is in a good shape?

Rafael


On Tuesday, May 13, 2014 04:50:30 PM Lv Zheng wrote:
> We need to find a smarter way to switch to 64-bit FADT addresses according
> to the bug report.  This patch reverts Linux to the original behavior.
> 
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=74021
> Reported-by: Oswald Buddenhagen <ossi@kde.org>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Tested-by: Oswald Buddenhagen <ossi@kde.org>
> Cc: <stable@vger.kernel.org> # 3.14.x: 0249ed24: ACPICA: Add option to favor 32-bit FADT addresses
> ---
>  include/acpi/acpixf.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index 4e3044c..b66b6a8 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -190,7 +190,7 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
>   * some machines have been found to have a corrupted non-zero 64-bit
>   * address. Default is FALSE, do not favor the 32-bit addresses.
>   */
> -ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, FALSE);
> +ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, TRUE);
>  
>  /*
>   * Optionally truncate I/O addresses to 16 bits. Provides compatibility
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[RFC PATCH 0/6] ACPICA: 64bit FADT addresses enabling.

[Lv Zheng]

From: Lv Zheng <lv.zheng@intel.com>

This series enable 64-bit addresses on top of linux-pm.git/linux-next.

Commit 0249ed2444d6 (ACPICA: Add option to favor 32-bit FADT addresses.)
breaks resuming from system-suspend on the Intel DP45SG board.

This is because the commit has changed the default behavior of original
Linux.  And the root cause is:
1. 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" or
2. BIOS may favor the 64-bit FACS waking vector address when the version of
   the FACS is greater than 0 while Linux currently only supports resuming
   from the real mode, so the 64-bit firmware waking vector is not tested
   and set to 0.

This patchset fixes this issue by excluding the above 2 cases and
re-enables 64-bit FADT addresses again using the following smarter way:
1. Restore old behavior of 32-bit favor for FADT;
2. To exclude cause 1:
   Add 64-bit firmware waking vector setting support for selected FACS,
   the 64-bit firmware waking vector favor is also controlled by the new
   acpi_set_firmware_waking_vector() parameter. OSPM that supports 32-bit
   waking environment can set a valid physical address here. For Linux
   which only supports real mode waking environment, this should be 0.
3. Fix 32-bit FACS favor by loading both 32-bit and 64-bit FACS, the
   selection of FACS is controlled by a new global option -
   acpi_gbl_use32_bit_facs_addresses, which is set to TRUE to select the
   FACS reported by "FIRMWARE_CTRL" field. Add firmware waking vector
   setting support for both 32-bit and 64-bit FACS. This is used to work
   around the reported platform where 2 FACS is reported in the FADT;
4. Restore new behavior of 64-bit favor for FADT.
So that it is ensured that the 32-bit firmware waking vector address of the
FACS reported by "FIRMWARE_CTRL" filed is still set after enabling 64-bit
FADT addresses favor.
To enable X_FIRMWARE_CTRL favor and 64-bit firmware waking vector favor,
OSPMs are required to set acpi_gbl_use32_bit_facs_addresses to FALSE and
pass a valid 64-bit firmware waking vector address for
acpi_set_firmware_waking_vector() invocations, thus now the FACS favor and
the 64-bit firmware waking vector support can be completely controlled by
OSPMs and ACPICA does not make any decision silently.

Lv Zheng (6):
  ACPICA: Hardware: Reduce divergences for sleep functions.
  ACPICA: Hardware: Enable 64-bit firmware waking vector for selected
    FACS.
  ACPI: sleep: Update acpi_set_firmware_waking_vector() invocations to
    favor 32-bit firmware waking vector.
  ACPICA: Tables: Enable both 32-bit and 64-bit FACS.
  ACPICA: Hardware: Enable firmware waking vector for both 32-bit and
    64-bit FACS.
  ACPICA: Tables: Enable default 64-bit FADT addresses favor.

 arch/ia64/include/asm/acpi.h    |    1 +
 arch/x86/include/asm/acpi.h     |    3 +-
 drivers/acpi/acpica/acglobal.h  |    2 +
 drivers/acpi/acpica/aclocal.h   |    1 +
 drivers/acpi/acpica/hwxfsleep.c |   83 +++++++++++++++++++++++++--------------
 drivers/acpi/acpica/tbfadt.c    |   21 ++++++----
 drivers/acpi/acpica/tbutils.c   |   37 +++++++++++------
 drivers/acpi/acpica/tbxfload.c  |    3 +-
 drivers/acpi/sleep.c            |    8 +++-
 include/acpi/acpixf.h           |   24 ++++++-----
 10 files changed, 121 insertions(+), 62 deletions(-)

-- 
1.7.10

[RFC PATCH 1/6] ACPICA: Hardware: Reduce divergences for sleep functions.

[Lv Zheng]

From: Lv Zheng <lv.zheng@intel.com>

This patch is generated to reduce source code differences between Linux and
ACPICA so that further sleep related modifications can be applied directly
without human interventions. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Cc: Oswald Buddenhagen <ossi@kde.org>
---
 drivers/acpi/acpica/hwxfsleep.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 6921c7f..103d9a7 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -138,7 +138,6 @@ acpi_status acpi_set_firmware_waking_vector64(u64 physical_address)
 {
 	ACPI_FUNCTION_TRACE(acpi_set_firmware_waking_vector64);
 
-
 	/* Determine if the 64-bit vector actually exists */
 
 	if ((acpi_gbl_FACS->length <= 32) || (acpi_gbl_FACS->version < 1)) {
@@ -154,7 +153,6 @@ acpi_status acpi_set_firmware_waking_vector64(u64 physical_address)
 
 ACPI_EXPORT_SYMBOL(acpi_set_firmware_waking_vector64)
 #endif
-
 /*******************************************************************************
  *
  * FUNCTION:    acpi_enter_sleep_state_s4bios
-- 
1.7.10

[RFC PATCH 2/6] ACPICA: Hardware: Enable 64-bit firmware waking vector for selected FACS.

[Lv Zheng]

From: Lv Zheng <lv.zheng@intel.com>

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 1.

ACPI specification says:
A. 32-bit FACS address (FIRMWARE_CTRL field in FADT):
   Physical memory address of the FACS, where OSPM and firmware exchange
   control information.
   If the X_FIRMWARE_CTRL field contains a non zero value then this field
   must be zero.
   A zero value indicates that no FACS is specified by this field.
B. 64-bit FACS address (X_FIRMWARE_CTRL field in FADT):
   64bit physical memory address of the FACS.
   This field is used when the physical address of the FACS is above 4GB.
   If the FIRMWARE_CTRL field contains a non zero value then this field
   must be zero.
   A zero value indicates that no FACS is specified by this field.
Thus the 32bit and 64bit firmware waking vector should indicate completely
different resuming environment - real mode (1MB addressable) and non real
mode (4GB+ addressable) and currently Linux only supports resuming from
real mode.

This patch enables 64-bit firmware waking vector for selected FACS via
acpi_set_firmware_waking_vector() so that it's up to OSPMs to determine which
resuming mode should be used by BIOS and ACPICA changes won't trigger the
bugs caused by the root cause 1. For example, Linux can pass
physical_address64=0 as the parameter of acpi_set_firmware_waking_vector() to
indicate no 64bit waking vector support. Lv Zheng.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=74021
Reported-by: Oswald Buddenhagen <ossi@kde.org>
Cc: Oswald Buddenhagen <ossi@kde.org>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/hwxfsleep.c |   61 ++++++++++++---------------------------
 include/acpi/acpixf.h           |   11 +++----
 2 files changed, 23 insertions(+), 49 deletions(-)

diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 103d9a7..2b988d5 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -73,7 +73,6 @@ static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
 /*
  * These functions are removed for the ACPI_REDUCED_HARDWARE case:
  *      acpi_set_firmware_waking_vector
- *      acpi_set_firmware_waking_vector64
  *      acpi_enter_sleep_state_s4bios
  */
 
@@ -83,15 +82,19 @@ static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
  * FUNCTION:    acpi_set_firmware_waking_vector
  *
  * PARAMETERS:  physical_address    - 32-bit physical address of ACPI real mode
- *                                    entry point.
+ *                                    entry point
+ *              physical_address64  - 64-bit physical address of ACPI protected
+ *                                    entry point
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Sets the 32-bit firmware_waking_vector field of the FACS
+ * DESCRIPTION: Sets the firmware_waking_vector fields of the FACS
  *
  ******************************************************************************/
 
-acpi_status acpi_set_firmware_waking_vector(u32 physical_address)
+acpi_status
+acpi_set_firmware_waking_vector(acpi_physical_address physical_address,
+				acpi_physical_address physical_address64)
 {
 	ACPI_FUNCTION_TRACE(acpi_set_firmware_waking_vector);
 
@@ -106,53 +109,27 @@ acpi_status acpi_set_firmware_waking_vector(u32 physical_address)
 
 	/* Set the 32-bit vector */
 
-	acpi_gbl_FACS->firmware_waking_vector = physical_address;
+	acpi_gbl_FACS->firmware_waking_vector = (u32)physical_address;
 
-	/* Clear the 64-bit vector if it exists */
+	if (acpi_gbl_FACS->length > 32) {
+		if (acpi_gbl_FACS->version >= 1) {
 
-	if ((acpi_gbl_FACS->length > 32) && (acpi_gbl_FACS->version >= 1)) {
-		acpi_gbl_FACS->xfirmware_waking_vector = 0;
-	}
-
-	return_ACPI_STATUS(AE_OK);
-}
-
-ACPI_EXPORT_SYMBOL(acpi_set_firmware_waking_vector)
-
-#if ACPI_MACHINE_WIDTH == 64
-/*******************************************************************************
- *
- * FUNCTION:    acpi_set_firmware_waking_vector64
- *
- * PARAMETERS:  physical_address    - 64-bit physical address of ACPI protected
- *                                    mode entry point.
- *
- * RETURN:      Status
- *
- * DESCRIPTION: Sets the 64-bit X_firmware_waking_vector field of the FACS, if
- *              it exists in the table. This function is intended for use with
- *              64-bit host operating systems.
- *
- ******************************************************************************/
-acpi_status acpi_set_firmware_waking_vector64(u64 physical_address)
-{
-	ACPI_FUNCTION_TRACE(acpi_set_firmware_waking_vector64);
+			/* Set the 64-bit vector */
 
-	/* Determine if the 64-bit vector actually exists */
+			acpi_gbl_FACS->xfirmware_waking_vector =
+			    physical_address64;
+		} else {
+			/* Clear the 64-bit vector if it exists */
 
-	if ((acpi_gbl_FACS->length <= 32) || (acpi_gbl_FACS->version < 1)) {
-		return_ACPI_STATUS(AE_NOT_EXIST);
+			acpi_gbl_FACS->xfirmware_waking_vector = 0;
+		}
 	}
 
-	/* Clear 32-bit vector, set the 64-bit X_ vector */
-
-	acpi_gbl_FACS->firmware_waking_vector = 0;
-	acpi_gbl_FACS->xfirmware_waking_vector = physical_address;
 	return_ACPI_STATUS(AE_OK);
 }
 
-ACPI_EXPORT_SYMBOL(acpi_set_firmware_waking_vector64)
-#endif
+ACPI_EXPORT_SYMBOL(acpi_set_firmware_waking_vector)
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_enter_sleep_state_s4bios
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index b5f7132..cfa8782 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -777,13 +777,10 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_leave_sleep_state(u8 sleep_state))
 
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
-				acpi_set_firmware_waking_vector(u32
-								physical_address))
-#if ACPI_MACHINE_WIDTH == 64
-ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
-				acpi_set_firmware_waking_vector64(u64
-								  physical_address))
-#endif
+				acpi_set_firmware_waking_vector
+				(acpi_physical_address physical_address,
+				 acpi_physical_address physical_address64))
+
 /*
  * ACPI Timer interfaces
  */
-- 
1.7.10

[RFC PATCH 3/6] ACPI: sleep: Update acpi_set_firmware_waking_vector() invocations to favor 32-bit firmware waking vector.

[Lv Zheng]

From: Lv Zheng <lv.zheng@intel.com>

This patch updates acpi_set_firmware_waking_vector() invocations in order
to keep 32-bit firmware waking vector favor for Linux.

64-bit firmware waking vector has never been enabled by Linux.  The
(acpi_physical_address)0 for 64-bit address can be used to force ACPICA to
set only 32-bit firmware waking vector for Linux.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Cc: Oswald Buddenhagen <ossi@kde.org>
---
 arch/ia64/include/asm/acpi.h |    1 +
 arch/x86/include/asm/acpi.h  |    3 ++-
 drivers/acpi/sleep.c         |    8 ++++++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h
index 75dc59a..45e45ec 100644
--- a/arch/ia64/include/asm/acpi.h
+++ b/arch/ia64/include/asm/acpi.h
@@ -75,6 +75,7 @@ int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
 extern int acpi_suspend_lowlevel(void);
 
 extern unsigned long acpi_wakeup_address;
+#define acpi_wakeup_address64	((acpi_physical_address)0)
 
 /*
  * Record the cpei override flag and current logical cpu. This is
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index e06225e..46a7dd8 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -71,7 +71,8 @@ static inline void acpi_disable_pci(void)
 extern int (*acpi_suspend_lowlevel)(void);
 
 /* Physical address to resume after wakeup */
-#define acpi_wakeup_address ((unsigned long)(real_mode_header->wakeup_start))
+#define acpi_wakeup_address	((acpi_physical_address)(real_mode_header->wakeup_start))
+#define acpi_wakeup_address64	((acpi_physical_address)(0))
 
 /*
  * Check if the CPU can handle C2 and deeper
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index c11e379..5ca7ab0 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -23,6 +23,8 @@
 #include "internal.h"
 #include "sleep.h"
 
+#define ACPI_NO_WAKING_VECTOR		((acpi_physical_address)0)
+
 static u8 sleep_states[ACPI_S_STATE_COUNT];
 
 static void acpi_sleep_tts_switch(u32 acpi_state)
@@ -59,7 +61,8 @@ static int acpi_sleep_prepare(u32 acpi_state)
 	if (acpi_state == ACPI_STATE_S3) {
 		if (!acpi_wakeup_address)
 			return -EFAULT;
-		acpi_set_firmware_waking_vector(acpi_wakeup_address);
+		acpi_set_firmware_waking_vector(acpi_wakeup_address,
+						acpi_wakeup_address64);
 
 	}
 	ACPI_FLUSH_CPU_CACHE();
@@ -403,7 +406,8 @@ static void acpi_pm_finish(void)
 	acpi_leave_sleep_state(acpi_state);
 
 	/* reset firmware waking vector */
-	acpi_set_firmware_waking_vector((acpi_physical_address) 0);
+	acpi_set_firmware_waking_vector(ACPI_NO_WAKING_VECTOR,
+					ACPI_NO_WAKING_VECTOR);
 
 	acpi_target_sleep_state = ACPI_STATE_S0;
 
-- 
1.7.10

[RFC PATCH 4/6] ACPICA: Tables: Enable both 32-bit and 64-bit FACS.

[Lv Zheng]

From: Lv Zheng <lv.zheng@intel.com>

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.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=74021
Reported-by: Oswald Buddenhagen <ossi@kde.org>
Cc: Oswald Buddenhagen <ossi@kde.org>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/aclocal.h  |    1 +
 drivers/acpi/acpica/tbfadt.c   |   21 +++++++++++++--------
 drivers/acpi/acpica/tbutils.c  |   37 ++++++++++++++++++++++++++-----------
 drivers/acpi/acpica/tbxfload.c |    3 ++-
 include/acpi/acpixf.h          |    9 +++++++++
 5 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h
index 91f801a..c78761d 100644
--- 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;
diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
index 41519a9..dfa3f36 100644
--- 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);
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index e37a103..d4552e8 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -68,7 +68,8 @@ acpi_tb_get_root_table_entry(u8 *table_entry, u32 table_entry_size);
 
 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,25 @@ 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 (!facs32 && !facs64) {
+		return (AE_NO_MEMORY);
+	}
+
+	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 +116,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 +372,11 @@ acpi_status __init acpi_tb_parse_root_table(acpi_physical_address rsdp_address)
 	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 */
 
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index ab5308b..435f716 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -166,7 +166,8 @@ static acpi_status acpi_tb_load_namespace(void)
 
 	(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)
 		     &&
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index cfa8782..0f79659 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -193,6 +193,15 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
 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
-- 
1.7.10

[RFC PATCH 5/6] ACPICA: Hardware: Enable firmware waking vector for both 32-bit and 64-bit FACS.

[Lv Zheng]

From: Lv Zheng <lv.zheng@intel.com>

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 enables the firmware waking vectors for both 32bit/64bit FACS
tables in order to ensure we can exclude the cases that trigger the bugs
caused by the root cause 2. The exclusion is split into 2 commits so that
if it turns out not to be necessary, this single commit can be reverted
without affecting the useful one. Lv Zheng.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=74021
Reported-by: Oswald Buddenhagen <ossi@kde.org>
Cc: Oswald Buddenhagen <ossi@kde.org>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/acglobal.h  |    2 ++
 drivers/acpi/acpica/hwxfsleep.c |   72 ++++++++++++++++++++++++++++++++-------
 drivers/acpi/acpica/tbutils.c   |   14 ++++----
 3 files changed, 69 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index 115eedc..11b39d9 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -61,6 +61,8 @@ ACPI_GLOBAL(struct acpi_table_header, acpi_gbl_original_dsdt_header);
 
 #if (!ACPI_REDUCED_HARDWARE)
 ACPI_GLOBAL(struct acpi_table_facs *, acpi_gbl_FACS);
+ACPI_GLOBAL(struct acpi_table_facs *, acpi_gbl_facs32);
+ACPI_GLOBAL(struct acpi_table_facs *, acpi_gbl_facs64);
 
 #endif				/* !ACPI_REDUCED_HARDWARE */
 
diff --git a/drivers/acpi/acpica/hwxfsleep.c b/drivers/acpi/acpica/hwxfsleep.c
index 2b988d5..e29ee67 100644
--- a/drivers/acpi/acpica/hwxfsleep.c
+++ b/drivers/acpi/acpica/hwxfsleep.c
@@ -50,6 +50,11 @@
 ACPI_MODULE_NAME("hwxfsleep")
 
 /* Local prototypes */
+static acpi_status
+acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
+				   acpi_physical_address physical_address,
+				   acpi_physical_address physical_address64);
+
 static acpi_status acpi_hw_sleep_dispatch(u8 sleep_state, u32 function_id);
 
 /*
@@ -79,9 +84,10 @@ static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
 #if (!ACPI_REDUCED_HARDWARE)
 /*******************************************************************************
  *
- * FUNCTION:    acpi_set_firmware_waking_vector
+ * FUNCTION:    acpi_hw_set_firmware_waking_vector
  *
- * PARAMETERS:  physical_address    - 32-bit physical address of ACPI real mode
+ * PARAMETERS:  facs                - Pointer to FACS table
+ *              physical_address    - 32-bit physical address of ACPI real mode
  *                                    entry point
  *              physical_address64  - 64-bit physical address of ACPI protected
  *                                    entry point
@@ -92,11 +98,12 @@ static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
  *
  ******************************************************************************/
 
-acpi_status
-acpi_set_firmware_waking_vector(acpi_physical_address physical_address,
-				acpi_physical_address physical_address64)
+static acpi_status
+acpi_hw_set_firmware_waking_vector(struct acpi_table_facs *facs,
+				   acpi_physical_address physical_address,
+				   acpi_physical_address physical_address64)
 {
-	ACPI_FUNCTION_TRACE(acpi_set_firmware_waking_vector);
+	ACPI_FUNCTION_TRACE(acpi_hw_set_firmware_waking_vector);
 
 
 	/*
@@ -109,25 +116,66 @@ acpi_set_firmware_waking_vector(acpi_physical_address physical_address,
 
 	/* Set the 32-bit vector */
 
-	acpi_gbl_FACS->firmware_waking_vector = (u32)physical_address;
+	facs->firmware_waking_vector = (u32)physical_address;
 
-	if (acpi_gbl_FACS->length > 32) {
-		if (acpi_gbl_FACS->version >= 1) {
+	if (facs->length > 32) {
+		if (facs->version >= 1) {
 
 			/* Set the 64-bit vector */
 
-			acpi_gbl_FACS->xfirmware_waking_vector =
-			    physical_address64;
+			facs->xfirmware_waking_vector = physical_address64;
 		} else {
 			/* Clear the 64-bit vector if it exists */
 
-			acpi_gbl_FACS->xfirmware_waking_vector = 0;
+			facs->xfirmware_waking_vector = 0;
 		}
 	}
 
 	return_ACPI_STATUS(AE_OK);
 }
 
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_set_firmware_waking_vector
+ *
+ * PARAMETERS:  physical_address    - 32-bit physical address of ACPI real mode
+ *                                    entry point
+ *              physical_address64  - 64-bit physical address of ACPI protected
+ *                                    entry point
+ *
+ * RETURN:      Status
+ *
+ * DESCRIPTION: Sets the firmware_waking_vector fields of the FACS
+ *
+ ******************************************************************************/
+
+acpi_status
+acpi_set_firmware_waking_vector(acpi_physical_address physical_address,
+				acpi_physical_address physical_address64)
+{
+
+	ACPI_FUNCTION_TRACE(acpi_set_firmware_waking_vector);
+
+	/* If Hardware Reduced flag is set, there is no FACS */
+
+	if (acpi_gbl_reduced_hardware) {
+		return (AE_OK);
+	}
+
+	if (acpi_gbl_facs32) {
+		(void)acpi_hw_set_firmware_waking_vector(acpi_gbl_facs32,
+							 physical_address,
+							 physical_address64);
+	}
+	if (acpi_gbl_facs64) {
+		(void)acpi_hw_set_firmware_waking_vector(acpi_gbl_facs64,
+							 physical_address,
+							 physical_address64);
+	}
+
+	return_ACPI_STATUS(AE_OK);
+}
+
 ACPI_EXPORT_SYMBOL(acpi_set_firmware_waking_vector)
 
 /*******************************************************************************
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index d4552e8..5e8df70 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -68,8 +68,6 @@ acpi_tb_get_root_table_entry(u8 *table_entry, u32 table_entry_size);
 
 acpi_status acpi_tb_initialize_facs(void)
 {
-	struct acpi_table_facs *facs32;
-	struct acpi_table_facs *facs64;
 
 	/* If Hardware Reduced flag is set, there is no FACS */
 
@@ -81,19 +79,21 @@ acpi_status acpi_tb_initialize_facs(void)
 	(void)acpi_get_table_by_index(ACPI_TABLE_INDEX_FACS,
 				      ACPI_CAST_INDIRECT_PTR(struct
 							     acpi_table_header,
-							     &facs32));
+							     &acpi_gbl_facs32));
 	(void)acpi_get_table_by_index(ACPI_TABLE_INDEX_X_FACS,
 				      ACPI_CAST_INDIRECT_PTR(struct
 							     acpi_table_header,
-							     &facs64));
-	if (!facs32 && !facs64) {
+							     &acpi_gbl_facs64));
+	if (!acpi_gbl_facs32 && !acpi_gbl_facs64) {
 		return (AE_NO_MEMORY);
 	}
 
 	if (acpi_gbl_use32_bit_facs_addresses) {
-		acpi_gbl_FACS = facs32 ? facs32 : facs64;
+		acpi_gbl_FACS =
+		    acpi_gbl_facs32 ? acpi_gbl_facs32 : acpi_gbl_facs64;
 	} else {
-		acpi_gbl_FACS = facs64 ? facs64 : facs32;
+		acpi_gbl_FACS =
+		    acpi_gbl_facs64 ? acpi_gbl_facs64 : acpi_gbl_facs32;
 	}
 
 	return (AE_OK);
-- 
1.7.10

[RFC PATCH 6/6] ACPICA: Tables: Enable default 64-bit FADT addresses favor.

[Lv Zheng]

From: Lv Zheng <lv.zheng@intel.com>

With enough protections, this patch re-enables 64-bit FADT addresses by
default. If regressions are reported against such change, this patch should
be bisected and reverted.
Note that 64-bit FACS favor and 64-bit firmware waking vector favor are
excluded by this commit in order not to break OSPMs. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Cc: Oswald Buddenhagen <ossi@kde.org>
---
 include/acpi/acpixf.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 0f79659..6a9100a 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -188,9 +188,9 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
  * address. Although ACPICA adheres to the ACPI specification which
  * requires the use of the corresponding 64-bit address if it is non-zero,
  * some machines have been found to have a corrupted non-zero 64-bit
- * address. Default is TRUE, favor the 32-bit addresses.
+ * address. Default is FALSE, do not favor the 32-bit addresses.
  */
-ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, TRUE);
+ACPI_INIT_GLOBAL(u8, acpi_gbl_use32_bit_fadt_addresses, FALSE);
 
 /*
  * Optionally use 32-bit FACS table addresses.
-- 
1.7.10