[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 7/7] OvmfPkg/OvmfXen: Set PcdFSBClock


  • To: <devel@xxxxxxxxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Mon, 12 Apr 2021 14:30:03 +0100
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Laszlo Ersek <lersek@xxxxxxxxxx>, Jordan Justen <jordan.l.justen@xxxxxxxxx>, Ard Biesheuvel <ardb+tianocore@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>
  • Delivery-date: Mon, 12 Apr 2021 13:30:26 +0000
  • Ironport-hdrordr: A9a23:OPVOCq/Rb18ChCH/RUFuk+BbI+orLtY04lQ7vn1ZYzY9SKOlvu qpm+kW0gKxtS0YX2sulcvFFK6LR37d8pAd2/h3AZ6JWg76tGy0aLxz9IeK+UyYJwTS/vNQvJ 0AT4FQE9v1ZGIWse/b502CH88k0J279smT9IXj5lNMaS0vVK169Qd+DW+gYylLbS1LH4AwGp bZxucvnVqdUE8aZMi6GXUJNtKrz7agqLvcbRELHBIh4gWV5AnYjYLSKBSEwgwYFwpG3LZKyx miryXC+q6hv/unoyWz60bv6f1t9OfJ+59mDMyIhtN9EESOtjqV
  • Ironport-sdr: +JtXh35ICEfOoM9HQ7D6d6F4xzfBG+HzgkPT3sOXbJaWOLu/5N6GCQ84C1mMXkgTOmT/AoJ9OT qj0jSDe3XseUcu7AKBHoldczyHggH/5dX1WmsVxU9h5XcBoBHLD39bswWlDsKK2WG0iiivDckh OqKZKN8XumVlimIUOPUb13wmfDeKpBNyQWyyMugj4nOXld+4dkHswlreW2WdyCyPlumJBW4GZe znX+sfpjaj+Vq1wIzkeicsRE2pngvbMsk5JP+Jy009JIZot7dyzgkMaFCoQQyqMH5CvgCCqnz9 P8Q=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 run 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>
Reviewed-by: Laszlo Ersek <lersek@xxxxxxxxxx>
---

Notes:
    v3:
    - cast Freq in assert
    - fix typos
    - use correct assert to check Status
    v2:
    - keep the default value of PcdFSBClock because that is part of the syntax.

 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 86abe277c349..e535503e385d 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -447,9 +447,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
-
   # We populate DXE IPL tables with 1G pages preferably on Xen
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE
 
@@ -476,6 +473,7 @@ [PcdsDynamicDefault]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x800000000
 
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
 
   # Set video resolution for text setup.
diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf 
b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 5732d2188871..87dd4b24679a 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -85,6 +85,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 8b06bebd7731..2dc9f9ff8f3c 100644
--- a/OvmfPkg/XenPlatformPei/Xen.c
+++ b/OvmfPkg/XenPlatformPei/Xen.c
@@ -632,5 +632,9 @@ CalibrateLapicTimer (
   Freq = DivU64x64Remainder (Dividend, TscTick2 - TscTick, NULL);
   DEBUG ((DEBUG_INFO, "APIC Freq % 8lu Hz\n", Freq));
 
+  ASSERT ((UINT32)Freq <= MAX_UINT32);
+  Status = PcdSet32S (PcdFSBClock, Freq);
+  ASSERT_EFI_ERROR (Status);
+
   UnmapXenPage (SharedInfo);
 }
-- 
Anthony PERARD


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.