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 <somlo@cmu.edu>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1333238
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Tested-by: Gabriel Somlo <somlo@cmu.edu>

diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
index 652545d8647e5e8d8c0fd702a910a51f5575f302..a839495e673dbd7f124fed24fd2dbda590d7bd80 100644 (file)
--- a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
+++ b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
@@ -37,6 +37,8 @@ AcpiTimerLibConstructor (
 {\r
   UINT16 HostBridgeDevId;\r
   UINTN Pmba;\r
+  UINT32 PmbaAndVal;\r
+  UINT32 PmbaOrVal;\r
   UINTN AcpiCtlReg;\r
   UINT8 AcpiEnBit;\r
 \r
@@ -47,11 +49,15 @@ AcpiTimerLibConstructor (
   switch (HostBridgeDevId) {\r
     case INTEL_82441_DEVICE_ID:\r
       Pmba       = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA);\r
+      PmbaAndVal = ~(UINT32)PIIX4_PMBA_MASK;\r
+      PmbaOrVal  = PIIX4_PMBA_VALUE;\r
       AcpiCtlReg = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMREGMISC);\r
       AcpiEnBit  = PIIX4_PMREGMISC_PMIOSE;\r
       break;\r
     case INTEL_Q35_MCH_DEVICE_ID:\r
       Pmba       = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);\r
+      PmbaAndVal = ~(UINT32)ICH9_PMBASE_MASK;\r
+      PmbaOrVal  = ICH9_PMBASE_VALUE;\r
       AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL);\r
       AcpiEnBit  = ICH9_ACPI_CNTL_ACPI_EN;\r
       break;\r
@@ -70,7 +76,7 @@ AcpiTimerLibConstructor (
     // If the Power Management Base Address is not programmed,\r
     // then program it now.\r
     //\r
-    PciAndThenOr32 (Pmba, ~(UINT32)PIIX4_PMBA_MASK, PIIX4_PMBA_VALUE);\r
+    PciAndThenOr32 (Pmba, PmbaAndVal, PmbaOrVal);\r
 \r
     //\r
     // Enable PMBA I/O port decodes\r

diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
index 735dfd2e4bf90ff9e84e89002e9b99ed69ed48b5..dbbecc93c116dd3d47badb060dd5013e59c961f5 100644 (file)
--- a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
+++ b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
@@ -35,6 +35,8 @@ AcpiTimerLibConstructor (
 {\r
   UINT16 HostBridgeDevId;\r
   UINTN Pmba;\r
+  UINT32 PmbaAndVal;\r
+  UINT32 PmbaOrVal;\r
   UINTN AcpiCtlReg;\r
   UINT8 AcpiEnBit;\r
 \r
@@ -45,11 +47,15 @@ AcpiTimerLibConstructor (
   switch (HostBridgeDevId) {\r
     case INTEL_82441_DEVICE_ID:\r
       Pmba       = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA);\r
+      PmbaAndVal = ~(UINT32)PIIX4_PMBA_MASK;\r
+      PmbaOrVal  = PIIX4_PMBA_VALUE;\r
       AcpiCtlReg = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMREGMISC);\r
       AcpiEnBit  = PIIX4_PMREGMISC_PMIOSE;\r
       break;\r
     case INTEL_Q35_MCH_DEVICE_ID:\r
       Pmba       = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);\r
+      PmbaAndVal = ~(UINT32)ICH9_PMBASE_MASK;\r
+      PmbaOrVal  = ICH9_PMBASE_VALUE;\r
       AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL);\r
       AcpiEnBit  = ICH9_ACPI_CNTL_ACPI_EN;\r
       break;\r
@@ -68,7 +74,7 @@ AcpiTimerLibConstructor (
     // If the Power Management Base Address is not programmed,\r
     // then program it now.\r
     //\r
-    PciAndThenOr32 (Pmba, ~(UINT32)PIIX4_PMBA_MASK, PIIX4_PMBA_VALUE);\r
+    PciAndThenOr32 (Pmba, PmbaAndVal, PmbaOrVal);\r
 \r
     //\r
     // Enable PMBA I/O port decodes\r

diff --git a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c b/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c
index 308a6002149dcb05884f46890c46c102ce47d972..399f547d91854ef6e7ecb64d19cb4de83c88dfd4 100644 (file)
--- a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c
+++ b/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c
@@ -18,6 +18,7 @@
 #include <Library/DebugLib.h>\r
 #include <Library/IoLib.h>\r
 #include <Library/TimerLib.h>\r
+#include <OvmfPlatforms.h>\r
 \r
 #include <OvmfPlatforms.h>\r
 \r
@@ -26,10 +27,27 @@ AcpiPmControl (
   UINTN SuspendType\r
   )\r
 {\r
+  UINT16 AcpiPmBaseAddress;\r
+  UINT16 HostBridgeDevId;\r
+\r
   ASSERT (SuspendType < 6);\r
 \r
-  IoBitFieldWrite16 (PIIX4_PMBA_VALUE + 4, 10, 13, (UINT16) SuspendType);\r
-  IoOr16 (PIIX4_PMBA_VALUE + 4, BIT13);\r
+  AcpiPmBaseAddress = 0;\r
+  HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);\r
+  switch (HostBridgeDevId) {\r
+  case INTEL_82441_DEVICE_ID:\r
+    AcpiPmBaseAddress = PIIX4_PMBA_VALUE;\r
+    break;\r
+  case INTEL_Q35_MCH_DEVICE_ID:\r
+    AcpiPmBaseAddress = ICH9_PMBASE_VALUE;\r
+    break;\r
+  default:\r
+    ASSERT (FALSE);\r
+    CpuDeadLoop ();\r
+  }\r
+\r
+  IoBitFieldWrite16 (AcpiPmBaseAddress + 4, 10, 13, (UINT16) SuspendType);\r
+  IoOr16 (AcpiPmBaseAddress + 4, BIT13);\r
   CpuDeadLoop ();\r
 }\r
 \r

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 65b3df401abbf9d33269f9e77c08acaeeed01f98..a5654a51183bf36c2a8d68e78b0f91c23ad77e6d 100644 (file)
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -363,6 +363,8 @@ MiscInitialization (
 {\r
   UINTN  PmCmd;\r
   UINTN  Pmba;\r
+  UINT32 PmbaAndVal;\r
+  UINT32 PmbaOrVal;\r
   UINTN  AcpiCtlReg;\r
   UINT8  AcpiEnBit;\r
 \r
@@ -385,12 +387,16 @@ MiscInitialization (
     case INTEL_82441_DEVICE_ID:\r
       PmCmd      = POWER_MGMT_REGISTER_PIIX4 (PCI_COMMAND_OFFSET);\r
       Pmba       = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA);\r
+      PmbaAndVal = ~(UINT32)PIIX4_PMBA_MASK;\r
+      PmbaOrVal  = PIIX4_PMBA_VALUE;\r
       AcpiCtlReg = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMREGMISC);\r
       AcpiEnBit  = PIIX4_PMREGMISC_PMIOSE;\r
       break;\r
     case INTEL_Q35_MCH_DEVICE_ID:\r
       PmCmd      = POWER_MGMT_REGISTER_Q35 (PCI_COMMAND_OFFSET);\r
       Pmba       = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);\r
+      PmbaAndVal = ~(UINT32)ICH9_PMBASE_MASK;\r
+      PmbaOrVal  = ICH9_PMBASE_VALUE;\r
       AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL);\r
       AcpiEnBit  = ICH9_ACPI_CNTL_ACPI_EN;\r
       break;\r
@@ -412,7 +418,7 @@ MiscInitialization (
     // The PEI phase should be exited with fully accessibe ACPI PM IO space:\r
     // 1. set PMBA\r
     //\r
-    PciAndThenOr32 (Pmba, ~(UINT32)PIIX4_PMBA_MASK, PIIX4_PMBA_VALUE);\r
+    PciAndThenOr32 (Pmba, PmbaAndVal, PmbaOrVal);\r
 \r
     //\r
     // 2. set PCICMD/IOSE\r