public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
@ 2021-11-01  8:46 Min Xu
  2021-11-01  8:46 ` [PATCH V2 1/2] OvmfPkg: " Min Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Min Xu @ 2021-11-01  8:46 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711

XenTimerDxe is a local Apic timer driver and it has nothing to do
with Xen. So rename it to LocalApicTimerDxe.

After renaming, LocalApicTimerDxe is used in OvmfPkg if CSM_ENABLE=FALSE.
Otherwise 8254 timer is used.

Patch #1:
Rename XenTimerDxe to LocalApicTimerDxe

Patch #2:
Switch timer in build time for OvmfPkg. If CSM_ENABLE=TRUE, 8254 timer
is used, otherwise the timer is LocalApicTimerDxe.

Code at: https://github.com/mxu9/edk2/tree/ovmf_lapic_timer.v2

v2 changes:
 - Add gEfiMdePkgTokenSpaceGuid.PcdFSBClock in *.dsc

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

Min Xu (2):
  OvmfPkg: Rename XenTimerDxe to LocalApicTimerDxe
  OvmfPkg: Switch timer in build time for OvmfPkg

 OvmfPkg/AmdSev/AmdSevX64.dsc                           |  5 +++--
 OvmfPkg/AmdSev/AmdSevX64.fdf                           |  3 +--
 .../LocalApicTimerDxe.c}                               |  7 +++----
 .../LocalApicTimerDxe.h}                               |  4 ++--
 .../LocalApicTimerDxe.inf}                             |  6 +++---
 OvmfPkg/Microvm/MicrovmX64.dsc                         |  2 +-
 OvmfPkg/Microvm/MicrovmX64.fdf                         |  2 +-
 OvmfPkg/OvmfPkgIa32.dsc                                | 10 +++++++++-
 OvmfPkg/OvmfPkgIa32.fdf                                |  8 ++++++--
 OvmfPkg/OvmfPkgIa32X64.dsc                             | 10 +++++++++-
 OvmfPkg/OvmfPkgIa32X64.fdf                             |  8 ++++++--
 OvmfPkg/OvmfPkgX64.dsc                                 | 10 +++++++++-
 OvmfPkg/OvmfPkgX64.fdf                                 |  8 ++++++--
 OvmfPkg/OvmfXen.dsc                                    |  2 +-
 OvmfPkg/OvmfXen.fdf                                    |  2 +-
 15 files changed, 61 insertions(+), 26 deletions(-)
 rename OvmfPkg/{XenTimerDxe/XenTimerDxe.c => LocalApicTimerDxe/LocalApicTimerDxe.c} (95%)
 rename OvmfPkg/{XenTimerDxe/XenTimerDxe.h => LocalApicTimerDxe/LocalApicTimerDxe.h} (96%)
 rename OvmfPkg/{XenTimerDxe/XenTimerDxe.inf => LocalApicTimerDxe/LocalApicTimerDxe.inf} (86%)

-- 
2.29.2.windows.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V2 1/2] OvmfPkg: Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-01  8:46 [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
@ 2021-11-01  8:46 ` Min Xu
  2021-11-01  8:46 ` [PATCH V2 2/2] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Min Xu @ 2021-11-01  8:46 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3711

XenTimerDxe is a local Apic timer driver and it has nothing to do
with Xen. So rename it to LocalApicTimerDxe.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 .../XenTimerDxe.c => LocalApicTimerDxe/LocalApicTimerDxe.c} | 3 +--
 .../XenTimerDxe.h => LocalApicTimerDxe/LocalApicTimerDxe.h} | 4 ++--
 .../LocalApicTimerDxe.inf}                                  | 6 +++---
 OvmfPkg/Microvm/MicrovmX64.dsc                              | 2 +-
 OvmfPkg/Microvm/MicrovmX64.fdf                              | 2 +-
 OvmfPkg/OvmfXen.dsc                                         | 2 +-
 OvmfPkg/OvmfXen.fdf                                         | 2 +-
 7 files changed, 10 insertions(+), 11 deletions(-)
 rename OvmfPkg/{XenTimerDxe/XenTimerDxe.c => LocalApicTimerDxe/LocalApicTimerDxe.c} (96%)
 rename OvmfPkg/{XenTimerDxe/XenTimerDxe.h => LocalApicTimerDxe/LocalApicTimerDxe.h} (96%)
 rename OvmfPkg/{XenTimerDxe/XenTimerDxe.inf => LocalApicTimerDxe/LocalApicTimerDxe.inf} (86%)

diff --git a/OvmfPkg/XenTimerDxe/XenTimerDxe.c b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
similarity index 96%
rename from OvmfPkg/XenTimerDxe/XenTimerDxe.c
rename to OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
index 0bec59382b0a..5f57fd69d4fb 100644
--- a/OvmfPkg/XenTimerDxe/XenTimerDxe.c
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
@@ -8,7 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include "XenTimerDxe.h"
+#include "LocalApicTimerDxe.h"
 
 //
 // The handle onto which the Timer Architectural Protocol will be installed
@@ -353,4 +353,3 @@ TimerDriverInitialize (
 
   return Status;
 }
-
diff --git a/OvmfPkg/XenTimerDxe/XenTimerDxe.h b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
similarity index 96%
rename from OvmfPkg/XenTimerDxe/XenTimerDxe.h
rename to OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
index e0a3d95fd063..7d4802f8050f 100644
--- a/OvmfPkg/XenTimerDxe/XenTimerDxe.h
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
@@ -7,8 +7,8 @@ Copyright (c) 2019, Citrix Systems, Inc.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
-#ifndef _TIMER_H_
-#define _TIMER_H_
+#ifndef LOCAL_APIC_TIMER_H_
+#define LOCAL_APIC_TIMER_H_
 
 #include <PiDxe.h>
 
diff --git a/OvmfPkg/XenTimerDxe/XenTimerDxe.inf b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
similarity index 86%
rename from OvmfPkg/XenTimerDxe/XenTimerDxe.inf
rename to OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
index add1d01bbf9c..63b75b75c921 100644
--- a/OvmfPkg/XenTimerDxe/XenTimerDxe.inf
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
@@ -10,7 +10,7 @@
 
 [Defines]
   INF_VERSION                    = 0x00010005
-  BASE_NAME                      = XenTimerDxe
+  BASE_NAME                      = LocalApicTimerDxe
   FILE_GUID                      = 52fe8196-f9de-4d07-b22f-51f77a0e7c41
   MODULE_TYPE                    = DXE_DRIVER
   VERSION_STRING                 = 1.0
@@ -30,8 +30,8 @@
   LocalApicLib
 
 [Sources]
-  XenTimerDxe.h
-  XenTimerDxe.c
+  LocalApicTimerDxe.h
+  LocalApicTimerDxe.c
 
 [Protocols]
   gEfiCpuArchProtocolGuid       ## CONSUMES
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 617f92539518..13e04f650a8c 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -656,7 +656,7 @@
 
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/XenTimerDxe/XenTimerDxe.inf
+  OvmfPkg/LocalApicTimerDxe/XenTimerDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.fdf b/OvmfPkg/Microvm/MicrovmX64.fdf
index 6314014f3de7..2e3ad147e384 100644
--- a/OvmfPkg/Microvm/MicrovmX64.fdf
+++ b/OvmfPkg/Microvm/MicrovmX64.fdf
@@ -215,7 +215,7 @@ INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-INF  OvmfPkg/XenTimerDxe/XenTimerDxe.inf
+INF  OvmfPkg/LocalApicTimerDxe/XenTimerDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index a31519e356b7..5e6e68a5d21d 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -551,7 +551,7 @@
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/XenTimerDxe/XenTimerDxe.inf
+  OvmfPkg/LocalApicTimerDxe/XenTimerDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf
index 8b5823555937..effbaa463a2a 100644
--- a/OvmfPkg/OvmfXen.fdf
+++ b/OvmfPkg/OvmfXen.fdf
@@ -298,7 +298,7 @@ INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-INF  OvmfPkg/XenTimerDxe/XenTimerDxe.inf
+INF  OvmfPkg/LocalApicTimerDxe/XenTimerDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V2 2/2] OvmfPkg: Switch timer in build time for OvmfPkg
  2021-11-01  8:46 [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
  2021-11-01  8:46 ` [PATCH V2 1/2] OvmfPkg: " Min Xu
@ 2021-11-01  8:46 ` Min Xu
  2021-11-02  9:32 ` [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Gerd Hoffmann
  2021-11-02  9:38 ` Yao, Jiewen
  3 siblings, 0 replies; 9+ messages in thread
From: Min Xu @ 2021-11-01  8:46 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711

Discussion in https://bugzilla.tianocore.org/show_bug.cgi?id=1496 shows
that 8254TimerDxe was not written for OVMF. It was moved over from
PcAtChipsetPkg to OvmfPkg in 2019.  Probably because OVMF was the only
user left.

Most likely the reason OVMF used 8254TimerDxe initially was that it could
just use the existing driver in PcAtChipsetPkg.  And it simply hasn't
been changed ever.

CSM support was moved in 2019 too. (CSM support depends on 8254/8259
drivers). So 8254TimerDxe will be used when CSM_ENABLE=TRUE.

There are 4 .dsc which include the 8254Timer.
 - OvmfPkg/AmdSev/AmdSevX64.dsc
 - OvmfPkg/OvmfPkgIa32.dsc
 - OvmfPkg/OvmfPkgIa32X64.dsc
 - OvmfPkg/OvmfPkgX64.dsc

For the three OvmfPkg* configs using 8254TimerDxe with CSM_ENABLE=TRUE
and LapicTimerDxe otherwise.

For the AmdSev config it doesn't make sense to support a CSM. So use
the lapic timer unconditionally.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Suggested-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc                  |  5 +++--
 OvmfPkg/AmdSev/AmdSevX64.fdf                  |  3 +--
 OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c |  4 ++--
 OvmfPkg/OvmfPkgIa32.dsc                       | 10 +++++++++-
 OvmfPkg/OvmfPkgIa32.fdf                       |  8 ++++++--
 OvmfPkg/OvmfPkgIa32X64.dsc                    | 10 +++++++++-
 OvmfPkg/OvmfPkgIa32X64.fdf                    |  8 ++++++--
 OvmfPkg/OvmfPkgX64.dsc                        | 10 +++++++++-
 OvmfPkg/OvmfPkgX64.fdf                        |  8 ++++++--
 9 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 5ee54451169b..88c51dfe8337 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -579,6 +579,8 @@
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
 !endif
 
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
+
 [PcdsDynamicHii]
 !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
@@ -670,10 +672,9 @@
   }
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/8259InterruptControllerDxe/8259.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
-  OvmfPkg/8254TimerDxe/8254Timer.inf
+  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 56626098862c..7489b04198fe 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -206,10 +206,9 @@ INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
-INF  OvmfPkg/8254TimerDxe/8254Timer.inf
+INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
index 5f57fd69d4fb..fa91a4b2ad90 100644
--- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
@@ -173,7 +173,7 @@ TimerDriverSetTimerPeriod (
     //
     DisableApicTimerInterrupt();
   } else {
-    TimerFrequency = PcdGet32(PcdFSBClock) / DivideValue;
+    TimerFrequency = PcdGet32(PcdFSBClock) / (UINT32)DivideValue;
 
     //
     // Convert TimerPeriod into local APIC counts
@@ -193,7 +193,7 @@ TimerDriverSetTimerPeriod (
     //
     // Program the timer with the new count value
     //
-    InitializeApicTimer(DivideValue, TimerCount, TRUE, LOCAL_APIC_TIMER_VECTOR);
+    InitializeApicTimer(DivideValue, (UINT32)TimerCount, TRUE, LOCAL_APIC_TIMER_VECTOR);
 
     //
     // Enable timer interrupt
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 6a5be97c059d..d0e9f3ca05f6 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -650,6 +650,10 @@
   gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
   gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
 
+!if $(CSM_ENABLE) == FALSE
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
+!endif
+
 [PcdsDynamicHii]
 !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
@@ -753,10 +757,14 @@
   }
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/8259InterruptControllerDxe/8259.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
+!ifdef $(CSM_ENABLE)
+  OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
+!else
+  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+!endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 775ea2d71098..b7b35a3a490a 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -212,10 +212,14 @@ INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
-INF  OvmfPkg/8254TimerDxe/8254Timer.inf
+!ifdef $(CSM_ENABLE)
+  INF OvmfPkg/8259InterruptControllerDxe/8259.inf
+  INF OvmfPkg/8254TimerDxe/8254Timer.inf
+!else
+  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+!endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 71227d1b709a..d3531d388e24 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -658,6 +658,10 @@
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
 !endif
 
+!if $(CSM_ENABLE) == FALSE
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
+!endif
+
 [PcdsDynamicDefault.X64]
   # IPv4 and IPv6 PXE Boot support.
   gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
@@ -767,10 +771,14 @@
   }
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/8259InterruptControllerDxe/8259.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
+!ifdef $(CSM_ENABLE)
+  OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
+!else
+  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+!endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 9d8695922f97..986228a44c78 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -216,10 +216,14 @@ INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
-INF  OvmfPkg/8254TimerDxe/8254Timer.inf
+!ifdef $(CSM_ENABLE)
+  INF OvmfPkg/8259InterruptControllerDxe/8259.inf
+  INF OvmfPkg/8254TimerDxe/8254Timer.inf
+!else
+  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+!endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 52f7598cf1c7..c6ee624fc738 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -662,6 +662,10 @@
   gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
   gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
 
+!if $(CSM_ENABLE) == FALSE
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
+!endif
+
 [PcdsDynamicHii]
 !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
@@ -765,10 +769,14 @@
   }
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/8259InterruptControllerDxe/8259.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
+!ifdef $(CSM_ENABLE)
+  OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
+!else
+  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+!endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index b6cc3cabdd69..99339e73bb51 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -232,10 +232,14 @@ INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
-INF  OvmfPkg/8254TimerDxe/8254Timer.inf
+!ifdef $(CSM_ENABLE)
+  INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
+  INF  OvmfPkg/8254TimerDxe/8254Timer.inf
+!else
+  INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+!endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-01  8:46 [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
  2021-11-01  8:46 ` [PATCH V2 1/2] OvmfPkg: " Min Xu
  2021-11-01  8:46 ` [PATCH V2 2/2] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
@ 2021-11-02  9:32 ` Gerd Hoffmann
  2021-11-02  9:38 ` Yao, Jiewen
  3 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2021-11-02  9:32 UTC (permalink / raw)
  To: Min Xu
  Cc: devel, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky

On Mon, Nov 01, 2021 at 04:46:03PM +0800, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711
> 
> XenTimerDxe is a local Apic timer driver and it has nothing to do
> with Xen. So rename it to LocalApicTimerDxe.
> 
> After renaming, LocalApicTimerDxe is used in OvmfPkg if CSM_ENABLE=FALSE.
> Otherwise 8254 timer is used.
> 
> Patch #1:
> Rename XenTimerDxe to LocalApicTimerDxe
> 
> Patch #2:
> Switch timer in build time for OvmfPkg. If CSM_ENABLE=TRUE, 8254 timer
> is used, otherwise the timer is LocalApicTimerDxe.
> 
> Code at: https://github.com/mxu9/edk2/tree/ovmf_lapic_timer.v2
> 
> v2 changes:
>  - Add gEfiMdePkgTokenSpaceGuid.PcdFSBClock in *.dsc
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min Xu (2):
>   OvmfPkg: Rename XenTimerDxe to LocalApicTimerDxe
>   OvmfPkg: Switch timer in build time for OvmfPkg

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-01  8:46 [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
                   ` (2 preceding siblings ...)
  2021-11-02  9:32 ` [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Gerd Hoffmann
@ 2021-11-02  9:38 ` Yao, Jiewen
  2021-11-02  9:58   ` [edk2-devel] " Gerd Hoffmann
  3 siblings, 1 reply; 9+ messages in thread
From: Yao, Jiewen @ 2021-11-02  9:38 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Ard Biesheuvel, Justen, Jordan L, Brijesh Singh, Erdem Aktas,
	James Bottomley, Tom Lendacky, Gerd Hoffmann

Hi Min
I think it is good idea to rename to LocalApicTimeDxe.

I double checked dependency in INF file. But I did not know why it depends upon OvmfPkg.

=======================
[LibraryClasses]
  UefiBootServicesTableLib
  BaseLib
  DebugLib
  UefiDriverEntryPoint
  LocalApicLib

...

[Protocols]
  gEfiCpuArchProtocolGuid       ## CONSUMES
  gEfiTimerArchProtocolGuid     ## PRODUCES
[Pcd]
  gEfiMdePkgTokenSpaceGuid.PcdFSBClock  ## CONSUMES
[Depex]
  gEfiCpuArchProtocolGuid
=======================


Do you think it should so generic that it can remove OvmfPkg dependency and be moved to UefiCpuPkg?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Monday, November 1, 2021 4:46 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Erdem Aktas
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711
> 
> XenTimerDxe is a local Apic timer driver and it has nothing to do
> with Xen. So rename it to LocalApicTimerDxe.
> 
> After renaming, LocalApicTimerDxe is used in OvmfPkg if CSM_ENABLE=FALSE.
> Otherwise 8254 timer is used.
> 
> Patch #1:
> Rename XenTimerDxe to LocalApicTimerDxe
> 
> Patch #2:
> Switch timer in build time for OvmfPkg. If CSM_ENABLE=TRUE, 8254 timer
> is used, otherwise the timer is LocalApicTimerDxe.
> 
> Code at: https://github.com/mxu9/edk2/tree/ovmf_lapic_timer.v2
> 
> v2 changes:
>  - Add gEfiMdePkgTokenSpaceGuid.PcdFSBClock in *.dsc
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min Xu (2):
>   OvmfPkg: Rename XenTimerDxe to LocalApicTimerDxe
>   OvmfPkg: Switch timer in build time for OvmfPkg
> 
>  OvmfPkg/AmdSev/AmdSevX64.dsc                           |  5 +++--
>  OvmfPkg/AmdSev/AmdSevX64.fdf                           |  3 +--
>  .../LocalApicTimerDxe.c}                               |  7 +++----
>  .../LocalApicTimerDxe.h}                               |  4 ++--
>  .../LocalApicTimerDxe.inf}                             |  6 +++---
>  OvmfPkg/Microvm/MicrovmX64.dsc                         |  2 +-
>  OvmfPkg/Microvm/MicrovmX64.fdf                         |  2 +-
>  OvmfPkg/OvmfPkgIa32.dsc                                | 10 +++++++++-
>  OvmfPkg/OvmfPkgIa32.fdf                                |  8 ++++++--
>  OvmfPkg/OvmfPkgIa32X64.dsc                             | 10 +++++++++-
>  OvmfPkg/OvmfPkgIa32X64.fdf                             |  8 ++++++--
>  OvmfPkg/OvmfPkgX64.dsc                                 | 10 +++++++++-
>  OvmfPkg/OvmfPkgX64.fdf                                 |  8 ++++++--
>  OvmfPkg/OvmfXen.dsc                                    |  2 +-
>  OvmfPkg/OvmfXen.fdf                                    |  2 +-
>  15 files changed, 61 insertions(+), 26 deletions(-)
>  rename OvmfPkg/{XenTimerDxe/XenTimerDxe.c =>
> LocalApicTimerDxe/LocalApicTimerDxe.c} (95%)
>  rename OvmfPkg/{XenTimerDxe/XenTimerDxe.h =>
> LocalApicTimerDxe/LocalApicTimerDxe.h} (96%)
>  rename OvmfPkg/{XenTimerDxe/XenTimerDxe.inf =>
> LocalApicTimerDxe/LocalApicTimerDxe.inf} (86%)
> 
> --
> 2.29.2.windows.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-02  9:38 ` Yao, Jiewen
@ 2021-11-02  9:58   ` Gerd Hoffmann
  2021-11-02 10:05     ` Yao, Jiewen
  0 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2021-11-02  9:58 UTC (permalink / raw)
  To: devel, jiewen.yao
  Cc: Xu, Min M, Ard Biesheuvel, Justen, Jordan L, Brijesh Singh,
	Erdem Aktas, James Bottomley, Tom Lendacky

  Hi,

> Do you think it should so generic that it can remove OvmfPkg dependency and be moved to UefiCpuPkg?

It's not fully standalone, the driver needs to know the lapic frequency
(that is the reason why PcdFSBClock exists).

For KVM this is easy, the lapic uses a fixed frequency so it can simply
be set in the .dsc file.

For Xen the lapic frequency is the same as the tsc frequency, so the xen
code (platform init I think) goes figure the tsc freq and sets
PcdFSBClock accordingly.

So, when you want use the driver elsewhere you need to fill that gap,
and there is little reason to go that extra mile because on physical
hardware you have other options like using the hpet timer.

[ qemu supports hpet emulation but it is disabled by default for
  performance reasons, other timers have less virtualization
  overhead. ]

I'd suggest to keep it in OvmfPkg.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-02  9:58   ` [edk2-devel] " Gerd Hoffmann
@ 2021-11-02 10:05     ` Yao, Jiewen
  2021-11-02 10:21       ` Gerd Hoffmann
  0 siblings, 1 reply; 9+ messages in thread
From: Yao, Jiewen @ 2021-11-02 10:05 UTC (permalink / raw)
  To: Gerd Hoffmann, devel@edk2.groups.io
  Cc: Xu, Min M, Ard Biesheuvel, Justen, Jordan L, Brijesh Singh,
	Erdem Aktas, James Bottomley, Tom Lendacky

We can state, this driver is to support fixed frequency.
If a real platform happens to have fixed frequency, then it can be used.

gEfiMdePkgTokenSpaceGuid.PcdFSBClock  is defined in MdePkg.
The consumer need set PcdFSBClock. 

I don't see a need to bind to OVMF.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, November 2, 2021 5:59 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Xu, Min M <min.m.xu@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Erdem Aktas
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Tom
> Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH V2 0/2] Rename XenTimerDxe to
> LocalApicTimerDxe
> 
>   Hi,
> 
> > Do you think it should so generic that it can remove OvmfPkg dependency and
> be moved to UefiCpuPkg?
> 
> It's not fully standalone, the driver needs to know the lapic frequency
> (that is the reason why PcdFSBClock exists).
> 
> For KVM this is easy, the lapic uses a fixed frequency so it can simply
> be set in the .dsc file.
> 
> For Xen the lapic frequency is the same as the tsc frequency, so the xen
> code (platform init I think) goes figure the tsc freq and sets
> PcdFSBClock accordingly.
> 
> So, when you want use the driver elsewhere you need to fill that gap,
> and there is little reason to go that extra mile because on physical
> hardware you have other options like using the hpet timer.
> 
> [ qemu supports hpet emulation but it is disabled by default for
>   performance reasons, other timers have less virtualization
>   overhead. ]
> 
> I'd suggest to keep it in OvmfPkg.
> 
> take care,
>   Gerd


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-02 10:05     ` Yao, Jiewen
@ 2021-11-02 10:21       ` Gerd Hoffmann
  2021-11-02 10:24         ` Yao, Jiewen
  0 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2021-11-02 10:21 UTC (permalink / raw)
  To: devel, jiewen.yao
  Cc: Xu, Min M, Ard Biesheuvel, Justen, Jordan L, Brijesh Singh,
	Erdem Aktas, James Bottomley, Tom Lendacky

On Tue, Nov 02, 2021 at 10:05:56AM +0000, Yao, Jiewen wrote:
> We can state, this driver is to support fixed frequency.
> If a real platform happens to have fixed frequency, then it can be used.
> 
> gEfiMdePkgTokenSpaceGuid.PcdFSBClock  is defined in MdePkg.
> The consumer need set PcdFSBClock. 
> 
> I don't see a need to bind to OVMF.

Not needed indeed, but I doubt it'll actually be used outside
OvmfPkg for the reasons outlined in the last reply.

When we move it out we should clearly document the PcdFSBClock
so any potential users are aware of it.  Maybe drop a short
README into the directory then?

take care,
  Gerd


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-02 10:21       ` Gerd Hoffmann
@ 2021-11-02 10:24         ` Yao, Jiewen
  0 siblings, 0 replies; 9+ messages in thread
From: Yao, Jiewen @ 2021-11-02 10:24 UTC (permalink / raw)
  To: Gerd Hoffmann, devel@edk2.groups.io
  Cc: Xu, Min M, Ard Biesheuvel, Justen, Jordan L, Brijesh Singh,
	Erdem Aktas, James Bottomley, Tom Lendacky

Usually, we put those information in the header of module INF file.


> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, November 2, 2021 6:21 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Xu, Min M <min.m.xu@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Erdem Aktas
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Tom
> Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH V2 0/2] Rename XenTimerDxe to
> LocalApicTimerDxe
> 
> On Tue, Nov 02, 2021 at 10:05:56AM +0000, Yao, Jiewen wrote:
> > We can state, this driver is to support fixed frequency.
> > If a real platform happens to have fixed frequency, then it can be used.
> >
> > gEfiMdePkgTokenSpaceGuid.PcdFSBClock  is defined in MdePkg.
> > The consumer need set PcdFSBClock.
> >
> > I don't see a need to bind to OVMF.
> 
> Not needed indeed, but I doubt it'll actually be used outside
> OvmfPkg for the reasons outlined in the last reply.
> 
> When we move it out we should clearly document the PcdFSBClock
> so any potential users are aware of it.  Maybe drop a short
> README into the directory then?
> 
> take care,
>   Gerd


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-11-02 10:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-01  8:46 [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
2021-11-01  8:46 ` [PATCH V2 1/2] OvmfPkg: " Min Xu
2021-11-01  8:46 ` [PATCH V2 2/2] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
2021-11-02  9:32 ` [PATCH V2 0/2] Rename XenTimerDxe to LocalApicTimerDxe Gerd Hoffmann
2021-11-02  9:38 ` Yao, Jiewen
2021-11-02  9:58   ` [edk2-devel] " Gerd Hoffmann
2021-11-02 10:05     ` Yao, Jiewen
2021-11-02 10:21       ` Gerd Hoffmann
2021-11-02 10:24         ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox