.../Acpi/AcpiTableDxe/AcpiTableProtocol.c | 88 +++++++++------------- 1 file changed, 34 insertions(+), 54 deletions(-)
198a46d768fb90d2f9b16e26451b4814e7469eaf improved the DSDT and X_DSDT
fields mutual exclusion by checking FADT revision, but that breaks
some OS that has assumption to only consume X_DSDT field even the
DSDT address is < 4G.
To have better compatibility, this patch is to update the code to not
make FADT.{DSDT,X_DSDT} mutual exclusion, but always set both DSDT and
X_DSDT fields in the FADT when the DSDT address is < 4G.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
NOTE: This patch comes out from the discussion at
https://lists.01.org/pipermail/edk2-devel/2017-March/008580.html.
---
.../Acpi/AcpiTableDxe/AcpiTableProtocol.c | 88 +++++++++-------------
1 file changed, 34 insertions(+), 54 deletions(-)
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index 4bb848df5203..a4fd9aff845d 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -432,50 +432,6 @@ ReallocateAcpiTableBuffer (
}
/**
- Determine whether the FADT table passed in as parameter requires mutual
- exclusion between the DSDT and X_DSDT fields. (That is, whether there exists
- an explicit requirement that at most one of those fields is permitted to be
- nonzero.)
-
- @param[in] Fadt The EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE object to
- check.
-
- @retval TRUE Fadt requires mutual exclusion between DSDT and X_DSDT.
- @retval FALSE Otherwise.
-**/
-BOOLEAN
-RequireDsdtXDsdtExclusion (
- IN EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
- )
-{
- //
- // Mantis ticket #1393 was addressed in ACPI 5.1 Errata B. Unfortunately, we
- // can't tell apart 5.1 Errata A and 5.1 Errata B just from looking at the
- // FADT table. Therefore let's require exclusion for table versions >= 5.1.
- //
- // While this needlessly covers 5.1 and 5.1A too, it is safer to require
- // DSDT<->X_DSDT exclusion for lax (5.1, 5.1A) versions of the spec than to
- // permit DSDT<->X_DSDT duplication for strict (5.1B) versions of the spec.
- //
- // The same applies to 6.0 vs. 6.0A. While 6.0 does not require the
- // exclusion, 6.0A and 6.1 do. Since we cannot distinguish 6.0 from 6.0A
- // based on just the FADT, we lump 6.0 in with the rest of >= 5.1.
- //
- if ((Fadt->Header.Revision < 5) ||
- ((Fadt->Header.Revision == 5) &&
- (((EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *)Fadt)->MinorVersion == 0))) {
- //
- // version <= 5.0
- //
- return FALSE;
- }
- //
- // version >= 5.1
- //
- return TRUE;
-}
-
-/**
This function adds an ACPI table to the table list. It will detect FACS and
allocate the correct type of memory and properly align the table.
@@ -692,11 +648,23 @@ AddTableToList (
}
if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) {
AcpiTableInstance->Fadt3->Dsdt = (UINT32) (UINTN) AcpiTableInstance->Dsdt3;
- if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) {
- Buffer64 = 0;
- } else {
- Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
- }
+ //
+ // Comment block "the caller installs the tables in "DSDT, FADT" order"
+ // The below comments are also in "the caller installs the tables in "FADT, DSDT" order" comment block.
+ //
+ // The ACPI specification, up to and including revision 5.1 Errata A,
+ // allows the DSDT and X_DSDT fields to be both set in the FADT.
+ // (Obviously, this only makes sense if the DSDT address is representable in 4 bytes.)
+ // Starting with 5.1 Errata B, specifically for Mantis 1393 <https://mantis.uefi.org/mantis/view.php?id=1393>,
+ // the spec requires at most one of DSDT and X_DSDT fields to be set to a nonzero value,
+ // but strangely an exception is 6.0 that has no this requirement.
+ //
+ // Here we do not make the DSDT and X_DSDT fields mutual exclusion conditionally
+ // by checking FADT revision, but always set both DSDT and X_DSDT fields in the FADT
+ // to have better compatibility as some OS may have assumption to only consume X_DSDT
+ // field even the DSDT address is < 4G.
+ //
+ Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
} else {
AcpiTableInstance->Fadt3->Dsdt = 0;
Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
@@ -896,11 +864,23 @@ AddTableToList (
if (AcpiTableInstance->Fadt3 != NULL) {
if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) {
AcpiTableInstance->Fadt3->Dsdt = (UINT32) (UINTN) AcpiTableInstance->Dsdt3;
- if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) {
- Buffer64 = 0;
- } else {
- Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
- }
+ //
+ // Comment block "the caller installs the tables in "FADT, DSDT" order"
+ // The below comments are also in "the caller installs the tables in "DSDT, FADT" order" comment block.
+ //
+ // The ACPI specification, up to and including revision 5.1 Errata A,
+ // allows the DSDT and X_DSDT fields to be both set in the FADT.
+ // (Obviously, this only makes sense if the DSDT address is representable in 4 bytes.)
+ // Starting with 5.1 Errata B, specifically for Mantis 1393 <https://mantis.uefi.org/mantis/view.php?id=1393>,
+ // the spec requires at most one of DSDT and X_DSDT fields to be set to a nonzero value,
+ // but strangely an exception is 6.0 that has no this requirement.
+ //
+ // Here we do not make the DSDT and X_DSDT fields mutual exclusion conditionally
+ // by checking FADT revision, but always set both DSDT and X_DSDT fields in the FADT
+ // to have better compatibility as some OS may have assumption to only consume X_DSDT
+ // field even the DSDT address is < 4G.
+ //
+ Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
} else {
AcpiTableInstance->Fadt3->Dsdt = 0;
Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
--
2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/16/17 09:17, Star Zeng wrote:
> 198a46d768fb90d2f9b16e26451b4814e7469eaf improved the DSDT and X_DSDT
> fields mutual exclusion by checking FADT revision, but that breaks
> some OS that has assumption to only consume X_DSDT field even the
> DSDT address is < 4G.
>
> To have better compatibility, this patch is to update the code to not
> make FADT.{DSDT,X_DSDT} mutual exclusion, but always set both DSDT and
> X_DSDT fields in the FADT when the DSDT address is < 4G.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
>
> NOTE: This patch comes out from the discussion at
> https://lists.01.org/pipermail/edk2-devel/2017-March/008580.html.
> ---
> .../Acpi/AcpiTableDxe/AcpiTableProtocol.c | 88 +++++++++-------------
> 1 file changed, 34 insertions(+), 54 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thank you!
Laszlo
> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> index 4bb848df5203..a4fd9aff845d 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> @@ -432,50 +432,6 @@ ReallocateAcpiTableBuffer (
> }
>
> /**
> - Determine whether the FADT table passed in as parameter requires mutual
> - exclusion between the DSDT and X_DSDT fields. (That is, whether there exists
> - an explicit requirement that at most one of those fields is permitted to be
> - nonzero.)
> -
> - @param[in] Fadt The EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE object to
> - check.
> -
> - @retval TRUE Fadt requires mutual exclusion between DSDT and X_DSDT.
> - @retval FALSE Otherwise.
> -**/
> -BOOLEAN
> -RequireDsdtXDsdtExclusion (
> - IN EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
> - )
> -{
> - //
> - // Mantis ticket #1393 was addressed in ACPI 5.1 Errata B. Unfortunately, we
> - // can't tell apart 5.1 Errata A and 5.1 Errata B just from looking at the
> - // FADT table. Therefore let's require exclusion for table versions >= 5.1.
> - //
> - // While this needlessly covers 5.1 and 5.1A too, it is safer to require
> - // DSDT<->X_DSDT exclusion for lax (5.1, 5.1A) versions of the spec than to
> - // permit DSDT<->X_DSDT duplication for strict (5.1B) versions of the spec.
> - //
> - // The same applies to 6.0 vs. 6.0A. While 6.0 does not require the
> - // exclusion, 6.0A and 6.1 do. Since we cannot distinguish 6.0 from 6.0A
> - // based on just the FADT, we lump 6.0 in with the rest of >= 5.1.
> - //
> - if ((Fadt->Header.Revision < 5) ||
> - ((Fadt->Header.Revision == 5) &&
> - (((EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *)Fadt)->MinorVersion == 0))) {
> - //
> - // version <= 5.0
> - //
> - return FALSE;
> - }
> - //
> - // version >= 5.1
> - //
> - return TRUE;
> -}
> -
> -/**
> This function adds an ACPI table to the table list. It will detect FACS and
> allocate the correct type of memory and properly align the table.
>
> @@ -692,11 +648,23 @@ AddTableToList (
> }
> if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) {
> AcpiTableInstance->Fadt3->Dsdt = (UINT32) (UINTN) AcpiTableInstance->Dsdt3;
> - if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) {
> - Buffer64 = 0;
> - } else {
> - Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> - }
> + //
> + // Comment block "the caller installs the tables in "DSDT, FADT" order"
> + // The below comments are also in "the caller installs the tables in "FADT, DSDT" order" comment block.
> + //
> + // The ACPI specification, up to and including revision 5.1 Errata A,
> + // allows the DSDT and X_DSDT fields to be both set in the FADT.
> + // (Obviously, this only makes sense if the DSDT address is representable in 4 bytes.)
> + // Starting with 5.1 Errata B, specifically for Mantis 1393 <https://mantis.uefi.org/mantis/view.php?id=1393>,
> + // the spec requires at most one of DSDT and X_DSDT fields to be set to a nonzero value,
> + // but strangely an exception is 6.0 that has no this requirement.
> + //
> + // Here we do not make the DSDT and X_DSDT fields mutual exclusion conditionally
> + // by checking FADT revision, but always set both DSDT and X_DSDT fields in the FADT
> + // to have better compatibility as some OS may have assumption to only consume X_DSDT
> + // field even the DSDT address is < 4G.
> + //
> + Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> } else {
> AcpiTableInstance->Fadt3->Dsdt = 0;
> Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
> @@ -896,11 +864,23 @@ AddTableToList (
> if (AcpiTableInstance->Fadt3 != NULL) {
> if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) {
> AcpiTableInstance->Fadt3->Dsdt = (UINT32) (UINTN) AcpiTableInstance->Dsdt3;
> - if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) {
> - Buffer64 = 0;
> - } else {
> - Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> - }
> + //
> + // Comment block "the caller installs the tables in "FADT, DSDT" order"
> + // The below comments are also in "the caller installs the tables in "DSDT, FADT" order" comment block.
> + //
> + // The ACPI specification, up to and including revision 5.1 Errata A,
> + // allows the DSDT and X_DSDT fields to be both set in the FADT.
> + // (Obviously, this only makes sense if the DSDT address is representable in 4 bytes.)
> + // Starting with 5.1 Errata B, specifically for Mantis 1393 <https://mantis.uefi.org/mantis/view.php?id=1393>,
> + // the spec requires at most one of DSDT and X_DSDT fields to be set to a nonzero value,
> + // but strangely an exception is 6.0 that has no this requirement.
> + //
> + // Here we do not make the DSDT and X_DSDT fields mutual exclusion conditionally
> + // by checking FADT revision, but always set both DSDT and X_DSDT fields in the FADT
> + // to have better compatibility as some OS may have assumption to only consume X_DSDT
> + // field even the DSDT address is < 4G.
> + //
> + Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> } else {
> AcpiTableInstance->Fadt3->Dsdt = 0;
> Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Tested-by: Jeff Fan <jeff.fan@intel.com>
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, March 16, 2017 7:59 PM
To: Zeng, Star; edk2-devel@lists.01.org
Cc: Fan, Jeff; Yao, Jiewen
Subject: Re: [PATCH] MdeModulePkg/AcpiTableDxe: Not make FADT.{DSDT,X_DSDT} mutual exclusion
On 03/16/17 09:17, Star Zeng wrote:
> 198a46d768fb90d2f9b16e26451b4814e7469eaf improved the DSDT and X_DSDT
> fields mutual exclusion by checking FADT revision, but that breaks
> some OS that has assumption to only consume X_DSDT field even the DSDT
> address is < 4G.
>
> To have better compatibility, this patch is to update the code to not
> make FADT.{DSDT,X_DSDT} mutual exclusion, but always set both DSDT and
> X_DSDT fields in the FADT when the DSDT address is < 4G.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
>
> NOTE: This patch comes out from the discussion at
> https://lists.01.org/pipermail/edk2-devel/2017-March/008580.html.
> ---
> .../Acpi/AcpiTableDxe/AcpiTableProtocol.c | 88 +++++++++-------------
> 1 file changed, 34 insertions(+), 54 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thank you!
Laszlo
> diff --git
> a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> index 4bb848df5203..a4fd9aff845d 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> @@ -432,50 +432,6 @@ ReallocateAcpiTableBuffer ( }
>
> /**
> - Determine whether the FADT table passed in as parameter requires
> mutual
> - exclusion between the DSDT and X_DSDT fields. (That is, whether
> there exists
> - an explicit requirement that at most one of those fields is
> permitted to be
> - nonzero.)
> -
> - @param[in] Fadt The EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE object to
> - check.
> -
> - @retval TRUE Fadt requires mutual exclusion between DSDT and X_DSDT.
> - @retval FALSE Otherwise.
> -**/
> -BOOLEAN
> -RequireDsdtXDsdtExclusion (
> - IN EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
> - )
> -{
> - //
> - // Mantis ticket #1393 was addressed in ACPI 5.1 Errata B.
> Unfortunately, we
> - // can't tell apart 5.1 Errata A and 5.1 Errata B just from looking
> at the
> - // FADT table. Therefore let's require exclusion for table versions >= 5.1.
> - //
> - // While this needlessly covers 5.1 and 5.1A too, it is safer to
> require
> - // DSDT<->X_DSDT exclusion for lax (5.1, 5.1A) versions of the spec
> than to
> - // permit DSDT<->X_DSDT duplication for strict (5.1B) versions of the spec.
> - //
> - // The same applies to 6.0 vs. 6.0A. While 6.0 does not require the
> - // exclusion, 6.0A and 6.1 do. Since we cannot distinguish 6.0 from
> 6.0A
> - // based on just the FADT, we lump 6.0 in with the rest of >= 5.1.
> - //
> - if ((Fadt->Header.Revision < 5) ||
> - ((Fadt->Header.Revision == 5) &&
> - (((EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *)Fadt)->MinorVersion == 0))) {
> - //
> - // version <= 5.0
> - //
> - return FALSE;
> - }
> - //
> - // version >= 5.1
> - //
> - return TRUE;
> -}
> -
> -/**
> This function adds an ACPI table to the table list. It will detect FACS and
> allocate the correct type of memory and properly align the table.
>
> @@ -692,11 +648,23 @@ AddTableToList (
> }
> if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) {
> AcpiTableInstance->Fadt3->Dsdt = (UINT32) (UINTN) AcpiTableInstance->Dsdt3;
> - if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) {
> - Buffer64 = 0;
> - } else {
> - Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> - }
> + //
> + // Comment block "the caller installs the tables in "DSDT, FADT" order"
> + // The below comments are also in "the caller installs the tables in "FADT, DSDT" order" comment block.
> + //
> + // The ACPI specification, up to and including revision 5.1 Errata A,
> + // allows the DSDT and X_DSDT fields to be both set in the FADT.
> + // (Obviously, this only makes sense if the DSDT address is representable in 4 bytes.)
> + // Starting with 5.1 Errata B, specifically for Mantis 1393 <https://mantis.uefi.org/mantis/view.php?id=1393>,
> + // the spec requires at most one of DSDT and X_DSDT fields to be set to a nonzero value,
> + // but strangely an exception is 6.0 that has no this requirement.
> + //
> + // Here we do not make the DSDT and X_DSDT fields mutual exclusion conditionally
> + // by checking FADT revision, but always set both DSDT and X_DSDT fields in the FADT
> + // to have better compatibility as some OS may have assumption to only consume X_DSDT
> + // field even the DSDT address is < 4G.
> + //
> + Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> } else {
> AcpiTableInstance->Fadt3->Dsdt = 0;
> Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3; @@
> -896,11 +864,23 @@ AddTableToList (
> if (AcpiTableInstance->Fadt3 != NULL) {
> if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) {
> AcpiTableInstance->Fadt3->Dsdt = (UINT32) (UINTN) AcpiTableInstance->Dsdt3;
> - if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) {
> - Buffer64 = 0;
> - } else {
> - Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> - }
> + //
> + // Comment block "the caller installs the tables in "FADT, DSDT" order"
> + // The below comments are also in "the caller installs the tables in "DSDT, FADT" order" comment block.
> + //
> + // The ACPI specification, up to and including revision 5.1 Errata A,
> + // allows the DSDT and X_DSDT fields to be both set in the FADT.
> + // (Obviously, this only makes sense if the DSDT address is representable in 4 bytes.)
> + // Starting with 5.1 Errata B, specifically for Mantis 1393 <https://mantis.uefi.org/mantis/view.php?id=1393>,
> + // the spec requires at most one of DSDT and X_DSDT fields to be set to a nonzero value,
> + // but strangely an exception is 6.0 that has no this requirement.
> + //
> + // Here we do not make the DSDT and X_DSDT fields mutual exclusion conditionally
> + // by checking FADT revision, but always set both DSDT and X_DSDT fields in the FADT
> + // to have better compatibility as some OS may have assumption to only consume X_DSDT
> + // field even the DSDT address is < 4G.
> + //
> + Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
> } else {
> AcpiTableInstance->Fadt3->Dsdt = 0;
> Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.