[PATCH] acpi: Use 32-bit FADT values on X86

[PATCH] acpi: Use 32-bit FADT values on X86

[Matthew Garrett]

The ACPI specification says that we should use the 64-bit address 
offsets contained within the FADT if they exist. However, Windows uses 
the legacy address. Various vendors have left incorrect values in the 
64-bit field which then causes problems later. Since the vast majority 
of machines have never been tested with an OS that uses the 64-bit value 
by default, we should behave like Windows and ignore the spec by only 
using the 64-bit address if it contains something that can't be 
represented in the legacy field. Since system io space is only 16 bits 
on x86, this should be entirely safe.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

Some question remains as to whether we should be using the 32-bit values 
from the FADT provided by the XSDT or whether we should just be using 
the values from the FADT provided by the RSDT. So far every acpidump 
I've looked at has contained the same values in both, even when the 
64-bit values are broken. We know that there's a large number of 
machines out there that are broken in this respect. We have no evidence 
whatsoever to believe that there are any machines that this breaks. Can 
we just apply it and worry about further corner cases later?

diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
index 3636e4f..ad0e858 100644
--- a/drivers/acpi/acpica/tbfadt.c
+++ b/drivers/acpi/acpica/tbfadt.c
@@ -361,9 +361,28 @@ static void acpi_tb_convert_fadt(void)
 		    ACPI_ADD_PTR(struct acpi_generic_address, &acpi_gbl_FADT,
 				 fadt_info_table[i].address64);
 
-		/* Expand only if the 64-bit X target is null */
+		/*
+		 * The ACPI specification says that we should use the
+		 * 64-bit address offsets if they exists. However,
+		 * Windows uses the legacy address. Various vendors
+		 * have left incorrect values in the 64-bit field,
+		 * which then causes problems later. Since the vast
+		 * majority of machines have never been tested with an
+		 * OS that uses the 64-bit value by default, we should
+		 * behave like Windows and ignore the spec by only
+		 * using the 64-bit address if it contains something
+		 * that can't be represented in the legacy
+		 * field. Since system io space is only 16 bits on
+		 * x86, this should be entirely safe. We also extend
+		 * the 32-bit value into the 64-bit one if no 64-bit
+		 * address is provided.
+		 */
 
-		if (!target64->address) {
+		if (!target64->address
+#ifdef CONFIG_X86
+		    || (target64->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+#endif
+			) {
 
 			/* The space_id is always I/O for the 32-bit legacy address fields */
 
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] acpi: Use 32-bit FADT values on X86

[Len Brown]

This was already fixed in ACPICA (granted, the patch hasn't gone over the 
list yet)

http://git.kernel.org/?p=linux/kernel/git/lenb/linux-acpi-2.6.git;a=commitdiff;h=31fbc073a35a017e34840deb9e865a701e986002
--
Len Brown, Intel Open Source Technology Center

On Fri, 27 Mar 2009, Matthew Garrett wrote:

> The ACPI specification says that we should use the 64-bit address 
> offsets contained within the FADT if they exist. However, Windows uses 
> the legacy address. Various vendors have left incorrect values in the 
> 64-bit field which then causes problems later. Since the vast majority 
> of machines have never been tested with an OS that uses the 64-bit value 
> by default, we should behave like Windows and ignore the spec by only 
> using the 64-bit address if it contains something that can't be 
> represented in the legacy field. Since system io space is only 16 bits 
> on x86, this should be entirely safe.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> ---
> 
> Some question remains as to whether we should be using the 32-bit values 
> from the FADT provided by the XSDT or whether we should just be using 
> the values from the FADT provided by the RSDT. So far every acpidump 
> I've looked at has contained the same values in both, even when the 
> 64-bit values are broken. We know that there's a large number of 
> machines out there that are broken in this respect. We have no evidence 
> whatsoever to believe that there are any machines that this breaks. Can 
> we just apply it and worry about further corner cases later?
> 
> diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
> index 3636e4f..ad0e858 100644
> --- a/drivers/acpi/acpica/tbfadt.c
> +++ b/drivers/acpi/acpica/tbfadt.c
> @@ -361,9 +361,28 @@ static void acpi_tb_convert_fadt(void)
>  		    ACPI_ADD_PTR(struct acpi_generic_address, &acpi_gbl_FADT,
>  				 fadt_info_table[i].address64);
>  
> -		/* Expand only if the 64-bit X target is null */
> +		/*
> +		 * The ACPI specification says that we should use the
> +		 * 64-bit address offsets if they exists. However,
> +		 * Windows uses the legacy address. Various vendors
> +		 * have left incorrect values in the 64-bit field,
> +		 * which then causes problems later. Since the vast
> +		 * majority of machines have never been tested with an
> +		 * OS that uses the 64-bit value by default, we should
> +		 * behave like Windows and ignore the spec by only
> +		 * using the 64-bit address if it contains something
> +		 * that can't be represented in the legacy
> +		 * field. Since system io space is only 16 bits on
> +		 * x86, this should be entirely safe. We also extend
> +		 * the 32-bit value into the 64-bit one if no 64-bit
> +		 * address is provided.
> +		 */
>  
> -		if (!target64->address) {
> +		if (!target64->address
> +#ifdef CONFIG_X86
> +		    || (target64->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> +#endif
> +			) {
>  
>  			/* The space_id is always I/O for the 32-bit legacy address fields */
>  
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org
> 

Re: [PATCH] acpi: Use 32-bit FADT values on X86

[Matthew Garrett]

On Fri, Mar 27, 2009 at 08:47:55PM -0400, Len Brown wrote:
> This was already fixed in ACPICA (granted, the patch hasn't gone over the 
> list yet)

Works for me. Thanks!

-- 
Matthew Garrett | mjg59@srcf.ucam.org