On 2014-11-08 14:02:00, Gabriel L. Somlo wrote:
> Since in OVMF both PEI_CORE and PEIM run from RAM, and thus may
> utilize global variables, use the "Base" AcpiTimerLib instance
> (instead of BaseRom) to take advantage of the improved efficiency
> of storing the timer register IO address in a global variable.
>
> This leaves only SEC using the BaseRomAcpiTimerLib instance.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Gabriel Somlo <so...@cmu.edu>
> ---
> OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c | 35
> +++++++++++++++-------
> OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf | 5 +++-
> .../Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf | 2 +-
> OvmfPkg/OvmfPkgIa32.dsc | 2 --
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 --
> OvmfPkg/OvmfPkgX64.dsc | 2 --
> 6 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> index 8612ace..2faf3be 100644
> --- a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> +++ b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> @@ -15,12 +15,14 @@
> #include <Library/DebugLib.h>
> #include <Library/IoLib.h>
> #include <Library/PciLib.h>
> +#include <Library/PcdLib.h>
> #include <OvmfPlatforms.h>
>
> //
> // Power Management PCI Configuration Register fields
> //
> #define PMBA_RTE BIT0
> +#define PMIOSE BIT0
>
> //
> // Offset in the Power Management Base Address to the ACPI Timer
> @@ -33,14 +35,8 @@
> STATIC UINT32 mAcpiTimerIoAddr;
>
> /**
> - The constructor function caches the ACPI tick counter address
> -
> - At the time this constructor runs (DXE_CORE or later), ACPI IO space
> - has already been enabled by either PlatformPei or by the "Base"
> - instance of this library.
> - In order to avoid querying the underlying platform type during each
> - tick counter read operation, we cache the counter address during
> - initialization of this instance of the Timer Library.
> + The constructor function caches the ACPI tick counter address, and,
> + if necessary, enables ACPI IO space.
>
> @retval EFI_SUCCESS The constructor always returns RETURN_SUCCESS.
>
> @@ -53,6 +49,7 @@ AcpiTimerLibConstructor (
> {
> UINT16 HostBridgeDevId;
> UINTN Pmba;
> + UINTN PmRegMisc;
>
> //
> // Query Host Bridge DID to determine platform type
> @@ -60,10 +57,12 @@ AcpiTimerLibConstructor (
> HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
> switch (HostBridgeDevId) {
> case INTEL_82441_DEVICE_ID:
> - Pmba = POWER_MGMT_REGISTER_PIIX4 (0x40);
> + Pmba = POWER_MGMT_REGISTER_PIIX4 (0x40);
> + PmRegMisc = POWER_MGMT_REGISTER_PIIX4 (0x80);
> break;
> case INTEL_Q35_MCH_DEVICE_ID:
> - Pmba = POWER_MGMT_REGISTER_Q35 (0x40);
> + 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",
> @@ -73,6 +72,22 @@ AcpiTimerLibConstructor (
>
> mAcpiTimerIoAddr = (PciRead32 (Pmba) & ~PMBA_RTE) + ACPI_TIMER_OFFSET;
>
> + //
> + // Check to see if the Power Management Base Address is already enabled
> + //
> + if ((PciRead8 (PmRegMisc) & PMIOSE) == 0) {
> + //
> + // 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);
Wouldn't the 'Rom' version program this during SEC?
I don't think we should access the PCD in BaseAcpiTimerLib.
-Jordan
> + }
> +
> return RETURN_SUCCESS;
> }
>
> diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> index 35c6edc..43ffc8a 100644
> --- a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +++ b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> @@ -20,7 +20,7 @@
> FILE_GUID = FB648CF5-91BE-4737-9023-FD807AC6D96D
> MODULE_TYPE = BASE
> VERSION_STRING = 1.0
> - LIBRARY_CLASS = TimerLib|DXE_CORE DXE_SMM_DRIVER UEFI_DRIVER
> UEFI_APPLICATION SMM_CORE
> + LIBRARY_CLASS = TimerLib|PEI_CORE PEIM DXE_CORE DXE_SMM_DRIVER
> UEFI_DRIVER UEFI_APPLICATION SMM_CORE
> CONSTRUCTOR = AcpiTimerLibConstructor
>
> [Sources]
> @@ -31,6 +31,9 @@
> MdePkg/MdePkg.dec
> OvmfPkg/OvmfPkg.dec
>
> +[Pcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdAcpiPmBaseAddress
> +
> [LibraryClasses]
> BaseLib
> PciLib
> diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> index 2a3016f..d5e50ae 100644
> --- a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> +++ b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> @@ -19,7 +19,7 @@
> FILE_GUID = CDD9D74F-213E-4c28-98F7-8B4A167DB936
> MODULE_TYPE = BASE
> VERSION_STRING = 1.0
> - LIBRARY_CLASS = TimerLib|SEC PEI_CORE PEIM
> + LIBRARY_CLASS = TimerLib|SEC
> CONSTRUCTOR = AcpiTimerLibConstructor
>
> [Sources]
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 5baec1c..cd7e252 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -150,7 +150,6 @@
>
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>
> [LibraryClasses.common.PEI_CORE]
> - TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>
> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
> PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> @@ -167,7 +166,6 @@
> PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
>
> [LibraryClasses.common.PEIM]
> - TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>
> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
> PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index c7a18d0..16f988b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -155,7 +155,6 @@
>
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>
> [LibraryClasses.common.PEI_CORE]
> - TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>
> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
> PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> @@ -172,7 +171,6 @@
> PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
>
> [LibraryClasses.common.PEIM]
> - TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>
> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
> PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 7cafcac..b70985b 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -155,7 +155,6 @@
>
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>
> [LibraryClasses.common.PEI_CORE]
> - TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>
> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
> PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> @@ -172,7 +171,6 @@
> PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
>
> [LibraryClasses.common.PEIM]
> - TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>
> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
> PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> --
> 1.9.3
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
