| Message ID | 20210325154713.670104-7-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | OvmfXen: Set PcdFSBClock at runtime | expand |
On 03/25/21 16:47, Anthony PERARD via groups.io wrote: > Calculate the frequency of the APIC timer that Xen provides. > > Even though the frequency is currently hard-coded, it isn't part of > the public ABI that Xen provides and thus may change at any time. OVMF > needs to determine the frequency by an other mean. > > Fortunately, Xen provides a way to determines the frequency of the > TSC, so we can use TSC to calibrate the frequency of the APIC timer. > That information is found in the shared_info page which we map and > unmap once done (XenBusDxe is going to map the page somewhere else). > > The shared_info page is map at the highest physical address allowed as (1) s/map/mapped/ > it doesn't need to be in the RAM, thus there's a call to update the > page table. > > The calculated frequency is only logged in this patch, it will be used > in a following patch. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > CC: Roger Pau Monné <roger.pau@citrix.com> > --- > > Notes: > v2: > - fix CamelCases > - Use U64 multiplication and division helpers > - don't read TscShift from the SharedInfo page again > - change the location of the shared info page to be outside of the ram > - check for overflow in XenDelay > - check for overflow when calculating the calculating APIC frequency > > OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 2 + > OvmfPkg/XenPlatformPei/Platform.h | 5 + > OvmfPkg/XenPlatformPei/Platform.c | 1 + > OvmfPkg/XenPlatformPei/Xen.c | 177 ++++++++++++++++++++++ > 4 files changed, 185 insertions(+) > > diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > index 8790d907d3ec..5732d2188871 100644 > --- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > @@ -52,6 +52,7 @@ [LibraryClasses] > DebugLib > HobLib > IoLib > + LocalApicLib > PciLib > ResourcePublicationLib > PeiServicesLib > @@ -59,6 +60,7 @@ [LibraryClasses] > MtrrLib > MemEncryptSevLib > PcdLib > + SafeIntLib > XenHypercallLib > > [Pcd] > diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h > index e70ca58078eb..77d88fc159d7 100644 > --- a/OvmfPkg/XenPlatformPei/Platform.h > +++ b/OvmfPkg/XenPlatformPei/Platform.h > @@ -132,6 +132,11 @@ PhysicalAddressIdentityMapping ( > IN EFI_PHYSICAL_ADDRESS AddressToMap > ); > > +VOID > +CalibrateLapicTimer ( > + VOID > + ); > + > extern EFI_BOOT_MODE mBootMode; > > extern UINT8 mPhysMemAddressWidth; > diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c > index 717fd0ab1a45..e9511eb40c62 100644 > --- a/OvmfPkg/XenPlatformPei/Platform.c > +++ b/OvmfPkg/XenPlatformPei/Platform.c > @@ -448,6 +448,7 @@ InitializeXenPlatform ( > InitializeRamRegions (); > > InitializeXen (); > + CalibrateLapicTimer (); > > if (mBootMode != BOOT_ON_S3_RESUME) { > ReserveEmuVariableNvStore (); > diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c > index b2a7d1c21dac..7524aaa11a29 100644 > --- a/OvmfPkg/XenPlatformPei/Xen.c > +++ b/OvmfPkg/XenPlatformPei/Xen.c > @@ -21,8 +21,10 @@ > #include <Library/CpuLib.h> > #include <Library/DebugLib.h> > #include <Library/HobLib.h> > +#include <Library/LocalApicLib.h> > #include <Library/MemoryAllocationLib.h> > #include <Library/PcdLib.h> > +#include <Library/SafeIntLib.h> > #include <Guid/XenInfo.h> > #include <IndustryStandard/E820.h> > #include <Library/ResourcePublicationLib.h> > @@ -457,3 +459,178 @@ PhysicalAddressIdentityMapping ( > > return EFI_SUCCESS; > } > + > +STATIC > +EFI_STATUS > +MapSharedInfoPage ( > + IN VOID *PagePtr > + ) > +{ > + xen_add_to_physmap_t Parameters; > + INTN ReturnCode; > + > + Parameters.domid = DOMID_SELF; > + Parameters.space = XENMAPSPACE_shared_info; > + Parameters.idx = 0; > + Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT; > + ReturnCode = XenHypercallMemoryOp (XENMEM_add_to_physmap, &Parameters); > + if (ReturnCode != 0) { > + return EFI_NO_MAPPING; > + } > + return EFI_SUCCESS; > +} > + > +STATIC > +VOID > +UnmapXenPage ( > + IN VOID *PagePtr > + ) > +{ > + xen_remove_from_physmap_t Parameters; > + INTN ReturnCode; > + > + Parameters.domid = DOMID_SELF; > + Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT; > + ReturnCode = XenHypercallMemoryOp (XENMEM_remove_from_physmap, &Parameters); > + ASSERT (ReturnCode == 0); > +} > + > + > +STATIC > +UINT64 > +GetCpuFreq ( > + IN XEN_VCPU_TIME_INFO *VcpuTime > + ) > +{ > + UINT32 Version; > + UINT32 TscToSystemMultiplier; > + INT8 TscShift; > + UINT64 CpuFreq; > + > + do { > + Version = VcpuTime->Version; > + MemoryFence (); > + TscToSystemMultiplier = VcpuTime->TscToSystemMultiplier; > + TscShift = VcpuTime->TscShift; > + MemoryFence (); > + } while (((Version & 1) != 0) && (Version != VcpuTime->Version)); > + > + CpuFreq = DivU64x32 (LShiftU64 (1000000000ULL, 32), TscToSystemMultiplier); > + if (TscShift >= 0) { > + CpuFreq = RShiftU64 (CpuFreq, TscShift); > + } else { > + CpuFreq = LShiftU64 (CpuFreq, -TscShift); > + } > + return CpuFreq; > +} > + > +STATIC > +VOID > +XenDelay ( > + IN XEN_VCPU_TIME_INFO *VcpuTimeInfo, > + IN UINT64 DelayNs > + ) > +{ > + UINT64 Tick; > + UINT64 CpuFreq; > + UINT64 Delay; > + UINT64 DelayTick; > + UINT64 NewTick; > + RETURN_STATUS Status; > + > + Tick = AsmReadTsc (); > + > + CpuFreq = GetCpuFreq (VcpuTimeInfo); > + Status = SafeUint64Mult (DelayNs, CpuFreq, &Delay); > + ASSERT_EFI_ERROR (Status); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, > + "XenDelay (%ld ns): delay too big in relation to CPU freq %ld Hz\n", > + DelayNs, CpuFreq)); > + CpuDeadLoop (); > + } (2) I suggest moving the ASSERT_EFI_ERROR() into the EFI_ERROR() branch, namely between the DEBUG message and the CpuDeadLoop() call. Applies to the rest of the ASSERT_EFI_ERROR() invocations in this patch. (3) DelayNs and CpuFreq are of type UINT64, they should be logged with %lu, not %ld. (4) The indentation of the DEBUG macro arguments is incorrect; should be 2 spaces rather than 4. Applies to the rest of the DEBUG() invocations in this patch. > + > + DelayTick = DivU64x32 (Delay, 1000000000UL); (5) The UL suffix on the constant is wasteful / misleading IMO; DivU64x32 clearly takes a UINT32 as second parameter ("Divisor"). > + > + NewTick = Tick + DelayTick; > + > + // > + // Check for overflow > + // > + if (NewTick < Tick) { > + // > + // Overflow, wait for TSC to also overflow > + // > + while (AsmReadTsc () >= Tick) { > + CpuPause (); > + } > + } > + > + while (AsmReadTsc () <= NewTick) { > + CpuPause (); > + } > +} > + > + > +/** > + Calculate the frequency of the Local Apic Timer > +**/ > +VOID > +CalibrateLapicTimer ( > + VOID > + ) > +{ > + XEN_SHARED_INFO *SharedInfo; > + XEN_VCPU_TIME_INFO *VcpuTimeInfo; > + UINT32 TimerTick, TimerTick2, DiffTimer; > + UINT64 TscTick, TscTick2; > + UINT64 Freq; > + UINT64 Dividend; > + EFI_STATUS Status; > + > + > + SharedInfo = (VOID*)((1ULL << mPhysMemAddressWidth) - EFI_PAGE_SIZE); (I'm keeping in mind that this code is X64 only.) > + Status = PhysicalAddressIdentityMapping ((EFI_PHYSICAL_ADDRESS)SharedInfo); > + ASSERT_EFI_ERROR (Status); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, > + "Failed to add page table entry for Xen shared info page: %r\n", > + Status)); > + return; > + } > + > + Status = MapSharedInfoPage (SharedInfo); > + ASSERT_EFI_ERROR (Status); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failed to map Xen's shared info page: %r\n", > + Status)); > + return; > + } > + > + VcpuTimeInfo = &SharedInfo->VcpuInfo[0].Time; > + > + InitializeApicTimer (1, MAX_UINT32, TRUE, 0); > + DisableApicTimerInterrupt (); > + > + TimerTick = GetApicTimerCurrentCount (); > + TscTick = AsmReadTsc (); > + XenDelay (VcpuTimeInfo, 1000000ULL); > + TimerTick2 = GetApicTimerCurrentCount (); > + TscTick2 = AsmReadTsc (); > + > + > + DiffTimer = TimerTick - TimerTick2; > + Status = SafeUint64Mult (GetCpuFreq (VcpuTimeInfo), DiffTimer, &Dividend); > + ASSERT_EFI_ERROR (Status); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "overflow while calculating APIC frequency\n")); > + DEBUG ((DEBUG_ERROR, "CPU freq: %ld Hz; APIC timer tick count for 1 ms: %d\n", > + GetCpuFreq (VcpuTimeInfo), DiffTimer)); (6) Please use %lu and %u for formatting the UINT64 retval of GetCpuFreq(), and the UINT32 variable DiffTimer, respectively. All trivial feedback of course, so: Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo > + CpuDeadLoop (); > + } > + > + Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL); > + DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq)); > + > + UnmapXenPage (SharedInfo); > +} >
diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf index 8790d907d3ec..5732d2188871 100644 --- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf @@ -52,6 +52,7 @@ [LibraryClasses] DebugLib HobLib IoLib + LocalApicLib PciLib ResourcePublicationLib PeiServicesLib @@ -59,6 +60,7 @@ [LibraryClasses] MtrrLib MemEncryptSevLib PcdLib + SafeIntLib XenHypercallLib [Pcd] diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h index e70ca58078eb..77d88fc159d7 100644 --- a/OvmfPkg/XenPlatformPei/Platform.h +++ b/OvmfPkg/XenPlatformPei/Platform.h @@ -132,6 +132,11 @@ PhysicalAddressIdentityMapping ( IN EFI_PHYSICAL_ADDRESS AddressToMap ); +VOID +CalibrateLapicTimer ( + VOID + ); + extern EFI_BOOT_MODE mBootMode; extern UINT8 mPhysMemAddressWidth; diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c index 717fd0ab1a45..e9511eb40c62 100644 --- a/OvmfPkg/XenPlatformPei/Platform.c +++ b/OvmfPkg/XenPlatformPei/Platform.c @@ -448,6 +448,7 @@ InitializeXenPlatform ( InitializeRamRegions (); InitializeXen (); + CalibrateLapicTimer (); if (mBootMode != BOOT_ON_S3_RESUME) { ReserveEmuVariableNvStore (); diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c index b2a7d1c21dac..7524aaa11a29 100644 --- a/OvmfPkg/XenPlatformPei/Xen.c +++ b/OvmfPkg/XenPlatformPei/Xen.c @@ -21,8 +21,10 @@ #include <Library/CpuLib.h> #include <Library/DebugLib.h> #include <Library/HobLib.h> +#include <Library/LocalApicLib.h> #include <Library/MemoryAllocationLib.h> #include <Library/PcdLib.h> +#include <Library/SafeIntLib.h> #include <Guid/XenInfo.h> #include <IndustryStandard/E820.h> #include <Library/ResourcePublicationLib.h> @@ -457,3 +459,178 @@ PhysicalAddressIdentityMapping ( return EFI_SUCCESS; } + +STATIC +EFI_STATUS +MapSharedInfoPage ( + IN VOID *PagePtr + ) +{ + xen_add_to_physmap_t Parameters; + INTN ReturnCode; + + Parameters.domid = DOMID_SELF; + Parameters.space = XENMAPSPACE_shared_info; + Parameters.idx = 0; + Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT; + ReturnCode = XenHypercallMemoryOp (XENMEM_add_to_physmap, &Parameters); + if (ReturnCode != 0) { + return EFI_NO_MAPPING; + } + return EFI_SUCCESS; +} + +STATIC +VOID +UnmapXenPage ( + IN VOID *PagePtr + ) +{ + xen_remove_from_physmap_t Parameters; + INTN ReturnCode; + + Parameters.domid = DOMID_SELF; + Parameters.gpfn = (UINTN)PagePtr >> EFI_PAGE_SHIFT; + ReturnCode = XenHypercallMemoryOp (XENMEM_remove_from_physmap, &Parameters); + ASSERT (ReturnCode == 0); +} + + +STATIC +UINT64 +GetCpuFreq ( + IN XEN_VCPU_TIME_INFO *VcpuTime + ) +{ + UINT32 Version; + UINT32 TscToSystemMultiplier; + INT8 TscShift; + UINT64 CpuFreq; + + do { + Version = VcpuTime->Version; + MemoryFence (); + TscToSystemMultiplier = VcpuTime->TscToSystemMultiplier; + TscShift = VcpuTime->TscShift; + MemoryFence (); + } while (((Version & 1) != 0) && (Version != VcpuTime->Version)); + + CpuFreq = DivU64x32 (LShiftU64 (1000000000ULL, 32), TscToSystemMultiplier); + if (TscShift >= 0) { + CpuFreq = RShiftU64 (CpuFreq, TscShift); + } else { + CpuFreq = LShiftU64 (CpuFreq, -TscShift); + } + return CpuFreq; +} + +STATIC +VOID +XenDelay ( + IN XEN_VCPU_TIME_INFO *VcpuTimeInfo, + IN UINT64 DelayNs + ) +{ + UINT64 Tick; + UINT64 CpuFreq; + UINT64 Delay; + UINT64 DelayTick; + UINT64 NewTick; + RETURN_STATUS Status; + + Tick = AsmReadTsc (); + + CpuFreq = GetCpuFreq (VcpuTimeInfo); + Status = SafeUint64Mult (DelayNs, CpuFreq, &Delay); + ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, + "XenDelay (%ld ns): delay too big in relation to CPU freq %ld Hz\n", + DelayNs, CpuFreq)); + CpuDeadLoop (); + } + + DelayTick = DivU64x32 (Delay, 1000000000UL); + + NewTick = Tick + DelayTick; + + // + // Check for overflow + // + if (NewTick < Tick) { + // + // Overflow, wait for TSC to also overflow + // + while (AsmReadTsc () >= Tick) { + CpuPause (); + } + } + + while (AsmReadTsc () <= NewTick) { + CpuPause (); + } +} + + +/** + Calculate the frequency of the Local Apic Timer +**/ +VOID +CalibrateLapicTimer ( + VOID + ) +{ + XEN_SHARED_INFO *SharedInfo; + XEN_VCPU_TIME_INFO *VcpuTimeInfo; + UINT32 TimerTick, TimerTick2, DiffTimer; + UINT64 TscTick, TscTick2; + UINT64 Freq; + UINT64 Dividend; + EFI_STATUS Status; + + + SharedInfo = (VOID*)((1ULL << mPhysMemAddressWidth) - EFI_PAGE_SIZE); + Status = PhysicalAddressIdentityMapping ((EFI_PHYSICAL_ADDRESS)SharedInfo); + ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, + "Failed to add page table entry for Xen shared info page: %r\n", + Status)); + return; + } + + Status = MapSharedInfoPage (SharedInfo); + ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Failed to map Xen's shared info page: %r\n", + Status)); + return; + } + + VcpuTimeInfo = &SharedInfo->VcpuInfo[0].Time; + + InitializeApicTimer (1, MAX_UINT32, TRUE, 0); + DisableApicTimerInterrupt (); + + TimerTick = GetApicTimerCurrentCount (); + TscTick = AsmReadTsc (); + XenDelay (VcpuTimeInfo, 1000000ULL); + TimerTick2 = GetApicTimerCurrentCount (); + TscTick2 = AsmReadTsc (); + + + DiffTimer = TimerTick - TimerTick2; + Status = SafeUint64Mult (GetCpuFreq (VcpuTimeInfo), DiffTimer, &Dividend); + ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "overflow while calculating APIC frequency\n")); + DEBUG ((DEBUG_ERROR, "CPU freq: %ld Hz; APIC timer tick count for 1 ms: %d\n", + GetCpuFreq (VcpuTimeInfo), DiffTimer)); + CpuDeadLoop (); + } + + Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL); + DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq)); + + UnmapXenPage (SharedInfo); +}
Calculate the frequency of the APIC timer that Xen provides. Even though the frequency is currently hard-coded, it isn't part of the public ABI that Xen provides and thus may change at any time. OVMF needs to determine the frequency by an other mean. Fortunately, Xen provides a way to determines the frequency of the TSC, so we can use TSC to calibrate the frequency of the APIC timer. That information is found in the shared_info page which we map and unmap once done (XenBusDxe is going to map the page somewhere else). The shared_info page is map at the highest physical address allowed as it doesn't need to be in the RAM, thus there's a call to update the page table. The calculated frequency is only logged in this patch, it will be used in a following patch. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490 Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- CC: Roger Pau Monné <roger.pau@citrix.com> --- Notes: v2: - fix CamelCases - Use U64 multiplication and division helpers - don't read TscShift from the SharedInfo page again - change the location of the shared info page to be outside of the ram - check for overflow in XenDelay - check for overflow when calculating the calculating APIC frequency OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 2 + OvmfPkg/XenPlatformPei/Platform.h | 5 + OvmfPkg/XenPlatformPei/Platform.c | 1 + OvmfPkg/XenPlatformPei/Xen.c | 177 ++++++++++++++++++++++ 4 files changed, 185 insertions(+)