On Wed, Mar 25, 2015 at 10:51:36PM -0400, Gabriel L. Somlo wrote:
> On PIIX4, availability of ACPI power management IO registers is controlled
> by bit 0 of the PMREGMISC (function 3, offset 0x80) register. OVMF checks
> the value of this bit to determine whether PIIX4 ACPI power management
> has already been configured elsewhere (e.g., by Xen).
>
> This patch ensures we don't attempt to use PMREGMISC when using the Q35
> architecture.
>
> ---
>
> Appears to work for me, at least for now. Reza: does this fix your
> reboot issue as well ?
>
> However, after a bit more RTFM-ing the ich data sheet, isn't the role
> of PMREGMISC/bit0 from piix4 now filled by ICH9_LPC_ACPI_CTRL/bit7
> (ICH9_LPC_ACPI_CTRL_ACPI_EN) ?
>
> If so, maybe two things need to happen:
>
> 1. This patch should switch Q35 to ICH9_LPC_ACPI_CTRL_ACPI_EN
> instead of simply not messing with (the unavailable) piix5 PMIOSE
>
> 2. I should patch qemu to actually reset ICH9_LPC_ACPI_CTRL :)
>
> Laszlo, Jordan, what do you guys think ?
>
> Thanks,
> Gabriel
>
> OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c | 21 ++++++++---------
> OvmfPkg/PlatformPei/Platform.c | 26
> ++++++++++++----------
> 2 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
> b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
> index 05b14d7..5ed422d 100644
> --- a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
> +++ b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
> @@ -48,19 +48,28 @@ AcpiTimerLibConstructor (
> UINT16 HostBridgeDevId;
> UINTN Pmba;
> UINTN PmRegMisc;
> + BOOLEAN ConfigurePmba;
>
> //
> // Query Host Bridge DID to determine platform type
> //
> + ConfigurePmba = TRUE;
> HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
> switch (HostBridgeDevId) {
> case INTEL_82441_DEVICE_ID:
> Pmba = POWER_MGMT_REGISTER_PIIX4 (0x40);
> PmRegMisc = POWER_MGMT_REGISTER_PIIX4 (0x80);
> + //
> + // Check to see if the Power Management Base Address is already enabled
> + //
> + if ((PciRead8 (PmRegMisc) & 0x01) == 0) {
> + PciOr8 (PmRegMisc, PMIOSE);
OK, so this would need to happen *after* reprogramming the PMBA,
so it's technically wrong to do it here even if it appears to work,
kinda :)
But if I'm right about ICH9_LPC_ACPI_CTRL, I can keep the old code
pattern, only use POWER_MGMT_REGISTER_Q35(0x44) for "PmRegMisc", and
a uint8_t for which bit (0 or 7) I'm supposed to look for, depending
on whether it's piix or q35...
> + } else {
> + ConfigurePmba = FALSE;
> + }
> break;
> case INTEL_Q35_MCH_DEVICE_ID:
> Pmba = POWER_MGMT_REGISTER_Q35 (0x40);
> - PmRegMisc = POWER_MGMT_REGISTER_Q35 (0x80);
> break;
> default:
> DEBUG ((EFI_D_ERROR, "%a: Unknown Host Bridge Device ID: 0x%04x\n",
> @@ -69,20 +78,12 @@ AcpiTimerLibConstructor (
> return RETURN_UNSUPPORTED;
> }
>
> - //
> - // Check to see if the Power Management Base Address is already enabled
> - //
> - if ((PciRead8 (PmRegMisc) & PMIOSE) == 0) {
> + if (ConfigurePmba) {
> //
> // If the Power Management Base Address is not programmed,
> // then program the Power Management Base Address from a PCD.
> //
> PciAndThenOr32 (Pmba, (UINT32) ~0xFFC0, PcdGet16 (PcdAcpiPmBaseAddress));
> -
> - //
> - // Enable PMBA I/O port decodes in PMREGMISC
> - //
> - PciOr8 (PmRegMisc, PMIOSE);
> }
>
> return RETURN_SUCCESS;
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 1940e01..51660af 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -233,6 +233,7 @@ MiscInitialization (
> UINTN PmCmd;
> UINTN Pmba;
> UINTN PmRegMisc;
> + BOOLEAN ConfigurePmba;
>
> //
> // Disable A20 Mask
> @@ -247,17 +248,28 @@ MiscInitialization (
> //
> // Query Host Bridge DID to determine platform type and save to PCD
> //
> + ConfigurePmba = TRUE;
> HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
> switch (HostBridgeDevId) {
> case INTEL_82441_DEVICE_ID:
> PmCmd = POWER_MGMT_REGISTER_PIIX4 (PCI_COMMAND_OFFSET);
> Pmba = POWER_MGMT_REGISTER_PIIX4 (0x40);
> PmRegMisc = POWER_MGMT_REGISTER_PIIX4 (0x80);
> + //
> + // If PMREGMISC/PMIOSE (only available on piix4, NOT q35) is set,
> + // assume the ACPI PMBA has been configured (for example by Xen)
> + // and skip the setup here. This matches the logic in
> + // AcpiTimerLibConstructor ().
> + //
> + if ((PciRead8 (PmRegMisc) & 0x01) == 0) {
> + PciOr8 (PmRegMisc, 0x01);
> + } else {
> + ConfigurePmba = FALSE;
> + }
> break;
> case INTEL_Q35_MCH_DEVICE_ID:
> PmCmd = POWER_MGMT_REGISTER_Q35 (PCI_COMMAND_OFFSET);
> Pmba = POWER_MGMT_REGISTER_Q35 (0x40);
> - PmRegMisc = POWER_MGMT_REGISTER_Q35 (0x80);
> break;
> default:
> DEBUG ((EFI_D_ERROR, "%a: Unknown Host Bridge Device ID: 0x%04x\n",
> @@ -267,12 +279,7 @@ MiscInitialization (
> }
> PcdSet16 (PcdOvmfHostBridgePciDevId, HostBridgeDevId);
>
> - //
> - // If PMREGMISC/PMIOSE is set, assume the ACPI PMBA has been configured
> (for
> - // example by Xen) and skip the setup here. This matches the logic in
> - // AcpiTimerLibConstructor ().
> - //
> - if ((PciRead8 (PmRegMisc) & 0x01) == 0) {
> + if (ConfigurePmba) {
> //
> // The PEI phase should be exited with fully accessibe PIIX4 IO space:
> // 1. set PMBA
> @@ -283,11 +290,6 @@ MiscInitialization (
> // 2. set PCICMD/IOSE
> //
> PciOr8 (PmCmd, EFI_PCI_COMMAND_IO_SPACE);
> -
> - //
> - // 3. set PMREGMISC/PMIOSE
> - //
> - PciOr8 (PmRegMisc, 0x01);
> }
> }
>
> --
> 2.1.0
>
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
