From 1466b76f938501269d1c95f068b6d4938acec22d Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Mon, 9 May 2016 21:04:47 +0200 Subject: OvmfPkg: determine PMBA value dependent on host bridge device ID In this patch, the AcpiTimerLib instances, ResetSystemLib, and PlatformPei are modified together in order to keep VMs functional across a bisection: they all must agree on the PMBA value used. ResetSystemLib must not use dynamic PCDs. With SOURCE_DEBUG_ENABLE, it gets linked into the debug agent, therefore the same restrictions apply to it as to BaseRomAcpiTimerLib. Luckily, AcpiPmControl() is only used for powering off the virtual machine, thus the extra cost of a PCI config space read, compared to a PcdGet16(), should be negligible. This is the patch that moves the PMBA to IO port 0x0600 on Q35 in practice. The ResetSystemLib change is easiest to verify with the "reset -s" command in the UEFI shell (which goes through gRT->ResetSystem() and, in OVMF, PcAtChipsetPkg/KbcResetDxe). Cc: Gabriel Somlo Cc: Jordan Justen Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1333238 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jordan Justen Tested-by: Gabriel Somlo --- OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c | 8 +++++++- OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c | 8 +++++++- OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c | 22 ++++++++++++++++++++-- OvmfPkg/PlatformPei/Platform.c | 8 +++++++- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c index 652545d864..a839495e67 100644 --- a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c +++ b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c @@ -37,6 +37,8 @@ AcpiTimerLibConstructor ( { UINT16 HostBridgeDevId; UINTN Pmba; + UINT32 PmbaAndVal; + UINT32 PmbaOrVal; UINTN AcpiCtlReg; UINT8 AcpiEnBit; @@ -47,11 +49,15 @@ AcpiTimerLibConstructor ( switch (HostBridgeDevId) { case INTEL_82441_DEVICE_ID: Pmba = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA); + PmbaAndVal = ~(UINT32)PIIX4_PMBA_MASK; + PmbaOrVal = PIIX4_PMBA_VALUE; AcpiCtlReg = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMREGMISC); AcpiEnBit = PIIX4_PMREGMISC_PMIOSE; break; case INTEL_Q35_MCH_DEVICE_ID: Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE); + PmbaAndVal = ~(UINT32)ICH9_PMBASE_MASK; + PmbaOrVal = ICH9_PMBASE_VALUE; AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL); AcpiEnBit = ICH9_ACPI_CNTL_ACPI_EN; break; @@ -70,7 +76,7 @@ AcpiTimerLibConstructor ( // If the Power Management Base Address is not programmed, // then program it now. // - PciAndThenOr32 (Pmba, ~(UINT32)PIIX4_PMBA_MASK, PIIX4_PMBA_VALUE); + PciAndThenOr32 (Pmba, PmbaAndVal, PmbaOrVal); // // Enable PMBA I/O port decodes diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c index 735dfd2e4b..dbbecc93c1 100644 --- a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c +++ b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c @@ -35,6 +35,8 @@ AcpiTimerLibConstructor ( { UINT16 HostBridgeDevId; UINTN Pmba; + UINT32 PmbaAndVal; + UINT32 PmbaOrVal; UINTN AcpiCtlReg; UINT8 AcpiEnBit; @@ -45,11 +47,15 @@ AcpiTimerLibConstructor ( switch (HostBridgeDevId) { case INTEL_82441_DEVICE_ID: Pmba = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA); + PmbaAndVal = ~(UINT32)PIIX4_PMBA_MASK; + PmbaOrVal = PIIX4_PMBA_VALUE; AcpiCtlReg = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMREGMISC); AcpiEnBit = PIIX4_PMREGMISC_PMIOSE; break; case INTEL_Q35_MCH_DEVICE_ID: Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE); + PmbaAndVal = ~(UINT32)ICH9_PMBASE_MASK; + PmbaOrVal = ICH9_PMBASE_VALUE; AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL); AcpiEnBit = ICH9_ACPI_CNTL_ACPI_EN; break; @@ -68,7 +74,7 @@ AcpiTimerLibConstructor ( // If the Power Management Base Address is not programmed, // then program it now. // - PciAndThenOr32 (Pmba, ~(UINT32)PIIX4_PMBA_MASK, PIIX4_PMBA_VALUE); + PciAndThenOr32 (Pmba, PmbaAndVal, PmbaOrVal); // // Enable PMBA I/O port decodes diff --git a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c b/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c index 308a600214..399f547d91 100644 --- a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c +++ b/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -26,10 +27,27 @@ AcpiPmControl ( UINTN SuspendType ) { + UINT16 AcpiPmBaseAddress; + UINT16 HostBridgeDevId; + ASSERT (SuspendType < 6); - IoBitFieldWrite16 (PIIX4_PMBA_VALUE + 4, 10, 13, (UINT16) SuspendType); - IoOr16 (PIIX4_PMBA_VALUE + 4, BIT13); + AcpiPmBaseAddress = 0; + HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID); + switch (HostBridgeDevId) { + case INTEL_82441_DEVICE_ID: + AcpiPmBaseAddress = PIIX4_PMBA_VALUE; + break; + case INTEL_Q35_MCH_DEVICE_ID: + AcpiPmBaseAddress = ICH9_PMBASE_VALUE; + break; + default: + ASSERT (FALSE); + CpuDeadLoop (); + } + + IoBitFieldWrite16 (AcpiPmBaseAddress + 4, 10, 13, (UINT16) SuspendType); + IoOr16 (AcpiPmBaseAddress + 4, BIT13); CpuDeadLoop (); } diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 65b3df401a..a5654a5118 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -363,6 +363,8 @@ MiscInitialization ( { UINTN PmCmd; UINTN Pmba; + UINT32 PmbaAndVal; + UINT32 PmbaOrVal; UINTN AcpiCtlReg; UINT8 AcpiEnBit; @@ -385,12 +387,16 @@ MiscInitialization ( case INTEL_82441_DEVICE_ID: PmCmd = POWER_MGMT_REGISTER_PIIX4 (PCI_COMMAND_OFFSET); Pmba = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA); + PmbaAndVal = ~(UINT32)PIIX4_PMBA_MASK; + PmbaOrVal = PIIX4_PMBA_VALUE; AcpiCtlReg = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMREGMISC); AcpiEnBit = PIIX4_PMREGMISC_PMIOSE; break; case INTEL_Q35_MCH_DEVICE_ID: PmCmd = POWER_MGMT_REGISTER_Q35 (PCI_COMMAND_OFFSET); Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE); + PmbaAndVal = ~(UINT32)ICH9_PMBASE_MASK; + PmbaOrVal = ICH9_PMBASE_VALUE; AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL); AcpiEnBit = ICH9_ACPI_CNTL_ACPI_EN; break; @@ -412,7 +418,7 @@ MiscInitialization ( // The PEI phase should be exited with fully accessibe ACPI PM IO space: // 1. set PMBA // - PciAndThenOr32 (Pmba, ~(UINT32)PIIX4_PMBA_MASK, PIIX4_PMBA_VALUE); + PciAndThenOr32 (Pmba, PmbaAndVal, PmbaOrVal); // // 2. set PCICMD/IOSE -- cgit v1.2.3