Use 32-bit FADT values on X86
PATCH 1/1 Use 32-bit FADT values on X86
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.
---
Len, this is a clear case of the spec not matching real-life behaviour. I'd be amazed if anyone can find an x86 system that uses system-io space for these values and doesn't contain an accurate value in the 32-bit field. On the other hand, we've seen machines that assume the Windows-style behaviour and we keep finding more. A blacklist isn't the correct solution for this problem.
drivers/acpi/tables/tbfadt.c +24 -3
@@ -320,9 +320,30 @@ static void acpi_tb_convert_fadt(void) ACPI_ADD_PTR(struct acpi_generic_address, &acpi_gbl_FADT, fadt_info_table[i].target); -/* Expand only if the X target is null */ - - if (!target->address) { +/* +* 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 (!target->address +#ifdef CONFIG_X86 + || (target->space_id == ACPI_ADR_SPACE_SYSTEM_IO && + *ACPI_ADD_PTR(u32, &acpi_gbl_FADT, + fadt_info_table[i].source)) +#endif + ) { acpi_tb_init_generic_address(target, *ACPI_ADD_PTR(u8, &acpi_gbl_FADT,
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 3 lines · 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.
Including vista?
thanks,
rui
Quoted 57 lines
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 <[email protected]>---
Len, this is a clear case of the spec not matching real-life behaviour. I'd be amazed if anyone can find an x86 system that uses system-io space for these values and doesn't contain an accurate value in the 32-bit field. On the other hand, we've seen machines that assume the Windows-style behaviour and we keep finding more. A blacklist isn't the correct solution for this problem.
drivers/acpi/tables/tbfadt.c +24
-3@@ -320,9 +320,30 @@ static void acpi_tb_convert_fadt(void) ACPI_ADD_PTR(struct acpi_generic_address, &acpi_gbl_FADT, fadt_info_table[i].target); -/* Expand only if the X target is null */ - - if (!target->address) { +/* +* 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 (!target->address +#ifdef CONFIG_X86 + || (target->space_id == ACPI_ADR_SPACE_SYSTEM_IO && + *ACPI_ADD_PTR(u32, &acpi_gbl_FADT, + fadt_info_table[i].source)) +#endif + ) { acpi_tb_init_generic_address(target, *ACPI_ADD_PTR(u8, &acpi_gbl_FADT,
Quoted 5 lines · Zhang Rui
Quoted 3 lines · 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.
Including vista?
According to the information I have, yes.
Signature · 1 line
Matthew Garrett | [email protected]
Len Brown
reviewerIt seems extremely risky to make this blanket change for all registers without very detailed supporting proof that this is the right thing to do.
Signature · 2 lines
-Len Brown Intel Open Source Technology Center
Quoted 66 lines · 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 <[email protected]>---
Len, this is a clear case of the spec not matching real-life behaviour. I'd be amazed if anyone can find an x86 system that uses system-io space for these values and doesn't contain an accurate value in the 32-bit field. On the other hand, we've seen machines that assume the Windows-style behaviour and we keep finding more. A blacklist isn't the correct solution for this problem.
drivers/acpi/tables/tbfadt.c +24
-3@@ -320,9 +320,30 @@ static void acpi_tb_convert_fadt(void) ACPI_ADD_PTR(struct acpi_generic_address, &acpi_gbl_FADT, fadt_info_table[i].target); -/* Expand only if the X target is null */ - - if (!target->address) { +/* +* 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 (!target->address +#ifdef CONFIG_X86 + || (target->space_id == ACPI_ADR_SPACE_SYSTEM_IO && + *ACPI_ADD_PTR(u32, &acpi_gbl_FADT, + fadt_info_table[i].source)) +#endif + ) { acpi_tb_init_generic_address(target, *ACPI_ADD_PTR(u8, &acpi_gbl_FADT,Signature · 6 lines
-- Matthew Garrett | [email protected] -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html