diff options
author | Laszlo Ersek <lersek@redhat.com> | 2017-02-01 15:33:57 +0100 |
---|---|---|
committer | Laszlo Ersek <lersek@redhat.com> | 2017-03-09 15:03:27 +0100 |
commit | 198a46d768fb90d2f9b16e26451b4814e7469eaf (patch) | |
tree | 2e786a371ccdf1e751535f9a678c6f594593f283 /MdeModulePkg/Universal | |
parent | 9886ebfe43d5dfadb97657b4df6ada40d46fb9e8 (diff) | |
download | edk2-platforms-198a46d768fb90d2f9b16e26451b4814e7469eaf.tar.xz |
MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT,X_DSDT} mutual exclusion
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 to be set to a nonzero value.
MdeModulePkg/AcpiTableDxe handles this mutual exclusion somewhat
inconsistently.
- If the caller of EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() installs the
tables in "DSDT, FADT" order, then we enforce the exclusion between the
DSDT and X_DSDT fields:
DSDT under 4GB FADT.DSDT FADT.X_DSDT [VARIANT B]
-------------- --------- -----------
yes set clear
no clear set
This behavior conforms to 5.1 Errata B. (And it's not required by
earlier versions of the spec.)
- If the caller passes in the tables in "FADT, DSDT" relative order, then
we do not enforce the exclusion:
DSDT under 4GB FADT.DSDT FADT.X_DSDT [VARIANT A]
-------------- --------- -----------
yes set set
no clear set
This satisfies 5.1 Errata A and earlier, but breaks 5.1 Errata B and
later.
Unify the handling of both relative orders. In particular, check the major
and minor version numbers in the FADT. If the FADT version is strictly
before 5.1, then implement [VARIANT A]. If the FADT version is equal to or
larger than 5.1, then implement [VARIANT B].
We make three observations:
- We can't check the FADT table version precisely against "5.1 Errata B";
erratum levels are not captured in the table. We err in the safe
direction, namely we enforce the exclusion for "5.1" and "5.1 Errata A".
- The same applies to "6.0" versus "6.0 Errata A". Because we cannot
distinguish these two, we consider "6.0" to be "equal to or larger than
5.1", and apply [VARIANT B], enforcing the exclusion.
- While a blanket [VARIANT B] would be simpler, there is a significant
benefit to [VARIANT A], under the spec versions that permit it:
compatibility with a wider range of OSPMs (typically, older ones).
For example, Igor reported about a "DELL R430 system with rev4 FADT
where DSDT and X_DSDT are pointing to the same address". Michael also
reported about several systems that exhibit the same.
Regression tested with the following KVM guests (QEMU built at
ata0def594286d, "Merge remote-tracking branch
'remotes/bonzini/tags/for-upstream' into staging", 2017-01-30):
- OVMF: boot and S3 suspend/resume
- Ia32, Q35, SMM
- Fedlet 20141209
- Ia32X64, Q35, SMM
- Fedora 22
- Windows 7
- Windows 8.1
- Windows 10
- Windows Server 2008 R2
- Windows Server 2012 R2
- Windows Server 2016 Tech Preview 4
- X64, I440FX, no SMM
- Fedora 24
- RHEL-6.7
- RHEL-7.2-ish
- ArmVirtQemu: boot test with virtio-gpu
- AARCH64
- Fedora 24
- RHELSA-7.3
- openSUSE Tumbleweed (4.8.4-based)
This change is connected to ASWG ticket
<https://mantis.uefi.org/mantis/view.php?id=1757>, which is now
closed/fixed.
Cc: Al Stone <ahs3@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Michael Tsirkin <mtsirkin@redhat.com>
Cc: Phil Dennis-Jordan <phil@philjordan.eu>
Cc: Star Zeng <star.zeng@intel.com>
Reported-by: Phil Dennis-Jordan <phil@philjordan.eu>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Diffstat (limited to 'MdeModulePkg/Universal')
-rw-r--r-- | MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 62 |
1 files changed, 59 insertions, 3 deletions
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c index 7795ff7269..4bb848df52 100644 --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c @@ -430,6 +430,51 @@ ReallocateAcpiTableBuffer ( mEfiAcpiMaxNumTables = NewMaxTableNumber;
return EFI_SUCCESS;
}
+
+/**
+ 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.
@@ -647,12 +692,16 @@ AddTableToList ( }
if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) {
AcpiTableInstance->Fadt3->Dsdt = (UINT32) (UINTN) AcpiTableInstance->Dsdt3;
- ZeroMem (&AcpiTableInstance->Fadt3->XDsdt, sizeof (UINT64));
+ if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) {
+ Buffer64 = 0;
+ } else {
+ Buffer64 = AcpiTableInstance->Fadt3->Dsdt;
+ }
} else {
AcpiTableInstance->Fadt3->Dsdt = 0;
Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
- CopyMem (&AcpiTableInstance->Fadt3->XDsdt, &Buffer64, sizeof (UINT64));
}
+ CopyMem (&AcpiTableInstance->Fadt3->XDsdt, &Buffer64, sizeof (UINT64));
//
// RSDP OEM information is updated to match the FADT OEM information
@@ -847,8 +896,15 @@ 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;
+ }
+ } else {
+ AcpiTableInstance->Fadt3->Dsdt = 0;
+ Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
}
- Buffer64 = (UINT64) (UINTN) AcpiTableInstance->Dsdt3;
CopyMem (&AcpiTableInstance->Fadt3->XDsdt, &Buffer64, sizeof (UINT64));
//
|