[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5/5] OvmfPkg/OvmfXen: Set PcdFSBClock
On 01/29/20 13:12, Anthony PERARD wrote: > Update gEfiMdePkgTokenSpaceGuid.PcdFSBClock so it can have the correct > value when SecPeiDxeTimerLibCpu start to use it for the APIC timer. > > Currently, nothing appear to use the value in PcdFSBClock before > XenPlatformPei had a chance to set it even though TimerLib is included > in modules runned before XenPlatformPei. > > XenPlatformPei doesn't use any of the functions that would use that > value. No other modules in the PEI phase seems to use the TimerLib > before PcdFSBClock is set. There are currently two other modules in > the PEI phase that needs the TimerLib: > - S3Resume2Pei, but only because LocalApicLib needs it, but nothing is > using the value from PcdFSBClock. > - CpuMpPei, but I believe it only runs after XenPlatformPei > > Before the PEI phase, there's the SEC phase, and SecMain needs > TimerLib because of LocalApicLib. And it initialise the APIC timers > for the debug agent. But I don't think any of the DebugLib that > OvmfXen could use are actually using the *Delay functions in TimerLib, > and so would not use the value from PcdFSBClock which would be > uninitialised. > > A simple runtime test showed that TimerLib doesn't use PcdFSBClock > value before it is set. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490 > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx> > --- > OvmfPkg/OvmfXen.dsc | 4 +--- > OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 1 + > OvmfPkg/XenPlatformPei/Xen.c | 4 ++++ > 3 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc > index 8c11efe9b709..190d7400c148 100644 > --- a/OvmfPkg/OvmfXen.dsc > +++ b/OvmfPkg/OvmfXen.dsc > @@ -442,9 +442,6 @@ [PcdsFixedAtBuild] > # Point to the MdeModulePkg/Application/UiApp/UiApp.inf > gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, > 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } > > - ## Xen vlapic's frequence is 100 MHz > - gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000 > - > > ################################################################################ > # > # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this > Platform > @@ -468,6 +465,7 @@ [PcdsDynamicDefault] > gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0 > gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x800000000 > > + gEfiMdePkgTokenSpaceGuid.PcdFSBClock (1) This syntax looks strange; I thought it was mandatory to provide a default value too. https://edk2-docs.gitbooks.io/edk-ii-dsc-specification/content/2_dsc_overview/28_pcd_sections.html --------- 2.8.3.1 PcdsDynamicDefault [...] The format for a boolean or numeric datum type PCD entry in this section is: PcdTokenSpaceGuidCName.PcdCName|Value --------- I'm not sure if the "build" utility accepts this intentionally, or by mistake. Can you simply keep the "|100000000" part too? Otherwise, I'm OK with the argument laid out in the commit message. (Thank you for the detailed commit message!) With (1) fixed: Reviewed-by: Laszlo Ersek <lersek@xxxxxxxxxx> Thanks! Laszlo > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0 > > # Set video resolution for text setup. > diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > index 335a442538c2..177200f3b7e5 100644 > --- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > @@ -83,6 +83,7 @@ [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode > gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > + gEfiMdePkgTokenSpaceGuid.PcdFSBClock > gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy > gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress > > diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c > index d6cdc9a8e31c..fc990462dccc 100644 > --- a/OvmfPkg/XenPlatformPei/Xen.c > +++ b/OvmfPkg/XenPlatformPei/Xen.c > @@ -504,6 +504,10 @@ CalibrateLapicTimer ( > / (TscTick2 - TscTick); > DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq)); > > + ASSERT (Freq <= MAX_UINT32); > + Status = PcdSet32S (PcdFSBClock, Freq); > + ASSERT_RETURN_ERROR (Status); > + > UnmapXenPage (SharedInfo); > > Exit: > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |