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

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.

Since LocalApicTimerDxe doesn't depend on OvmfPkg, so it is moved to
UefiCpuPkg.

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.

Patch #3:
Move LocalApicTimerDxe from OvmfPkg to UefiCpuPkg.

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

v3 changes:
 - Move LocalApicTimerDxe to UefiCpuPkg
 - Fix the errors in v2 patch-1 that OvmfXen.dsc / .fdf and
   MicrovmX64.dsc / .fdf are not updated for new name.
 - Add XenTimerDxe reviewers.

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>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

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

 OvmfPkg/AmdSev/AmdSevX64.dsc                          |  5 +++--
 OvmfPkg/AmdSev/AmdSevX64.fdf                          |  3 +--
 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 +-
 .../LocalApicTimerDxe/LocalApicTimerDxe.c             |  7 +++----
 .../LocalApicTimerDxe/LocalApicTimerDxe.h             |  4 ++--
 .../LocalApicTimerDxe/LocalApicTimerDxe.inf           | 11 +++++++----
 15 files changed, 65 insertions(+), 27 deletions(-)
 rename OvmfPkg/XenTimerDxe/XenTimerDxe.c => UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c (95%)
 rename OvmfPkg/XenTimerDxe/XenTimerDxe.h => UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.h (96%)
 rename OvmfPkg/XenTimerDxe/XenTimerDxe.inf => UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf (72%)

-- 
2.29.2.windows.2


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH V3 1/3] OvmfPkg: Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-08  6:07 [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
@ 2021-11-08  6:07 ` Min Xu
  2021-11-11 14:46   ` Anthony PERARD
  2021-11-08  6:07 ` [PATCH V3 2/3] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Min Xu @ 2021-11-08  6:07 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann,
	Anthony Perard, Julien Grall

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.

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>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
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..1a0e848f8356 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/LocalApicTimerDxe.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..ac9efba26811 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/LocalApicTimerDxe.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..7c4c5412d102 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/LocalApicTimerDxe.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..196853740753 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/LocalApicTimerDxe.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] 12+ messages in thread
* [PATCH V3 2/3] OvmfPkg: Switch timer in build time for OvmfPkg
  2021-11-08  6:07 [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
  2021-11-08  6:07 ` [PATCH V3 1/3] OvmfPkg: " Min Xu
@ 2021-11-08  6:07 ` Min Xu
  2021-11-08  6:08 ` [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg Min Xu
  2021-11-09  6:59 ` [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Gerd Hoffmann
  3 siblings, 0 replies; 12+ messages in thread
From: Min Xu @ 2021-11-08  6:07 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/LocalApicTimerDxe/LocalApicTimerDxe.inf |  1 +
 OvmfPkg/OvmfPkgIa32.dsc                         | 10 +++++++++-
 OvmfPkg/OvmfPkgIa32.fdf                         |  8 ++++++--
 OvmfPkg/OvmfPkgIa32X64.dsc                      | 10 +++++++++-
 OvmfPkg/OvmfPkgIa32X64.fdf                      |  8 ++++++--
 OvmfPkg/OvmfPkgX64.dsc                          | 10 +++++++++-
 OvmfPkg/OvmfPkgX64.fdf                          |  8 ++++++--
 10 files changed, 52 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/LocalApicTimerDxe/LocalApicTimerDxe.inf b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
index 63b75b75c921..3ad28a148c5b 100644
--- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
@@ -1,5 +1,6 @@
 ## @file
 # Local APIC timer driver that provides Timer Arch protocol.
+# PcdFSBClock is defined in MdePkg and it should be set by the consumer.
 #
 # Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2019, Citrix Systems, Inc.
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] 12+ messages in thread
* [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
  2021-11-08  6:07 [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
  2021-11-08  6:07 ` [PATCH V3 1/3] OvmfPkg: " Min Xu
  2021-11-08  6:07 ` [PATCH V3 2/3] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
@ 2021-11-08  6:08 ` Min Xu
  2021-11-11 14:47   ` Anthony PERARD
  2021-11-22  3:04   ` Ni, Ray
  2021-11-09  6:59 ` [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Gerd Hoffmann
  3 siblings, 2 replies; 12+ messages in thread
From: Min Xu @ 2021-11-08  6:08 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Jiewen Yao, Gerd Hoffmann, Anthony Perard, Julien Grall,
	Eric Dong, Ray Ni

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

OvmfPkg.dec is removed from [Packages] because it doesn't depend
on OvmfPkg. LocalApicTimerDxe is moved to UefiCpuPkg.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc                                | 2 +-
 OvmfPkg/AmdSev/AmdSevX64.fdf                                | 2 +-
 OvmfPkg/Microvm/MicrovmX64.dsc                              | 2 +-
 OvmfPkg/Microvm/MicrovmX64.fdf                              | 2 +-
 OvmfPkg/OvmfPkgIa32.dsc                                     | 2 +-
 OvmfPkg/OvmfPkgIa32.fdf                                     | 2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                                  | 2 +-
 OvmfPkg/OvmfPkgIa32X64.fdf                                  | 2 +-
 OvmfPkg/OvmfPkgX64.dsc                                      | 2 +-
 OvmfPkg/OvmfPkgX64.fdf                                      | 2 +-
 OvmfPkg/OvmfXen.dsc                                         | 2 +-
 OvmfPkg/OvmfXen.fdf                                         | 2 +-
 .../LocalApicTimerDxe/LocalApicTimerDxe.c                   | 0
 .../LocalApicTimerDxe/LocalApicTimerDxe.h                   | 0
 .../LocalApicTimerDxe/LocalApicTimerDxe.inf                 | 6 ++++--
 15 files changed, 16 insertions(+), 14 deletions(-)
 rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.c (100%)
 rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.h (100%)
 rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.inf (79%)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 88c51dfe8337..888fc24f1b58 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -674,7 +674,7 @@
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/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 7489b04198fe..659810f96bec 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -208,7 +208,7 @@ INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
-INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 1a0e848f8356..fcb8b571a041 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.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 ac9efba26811..d02e88e2a48e 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
+INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index d0e9f3ca05f6..f8c8ef1e58be 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -763,7 +763,7 @@
   OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index b7b35a3a490a..321d4a871afa 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -218,7 +218,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
   INF OvmfPkg/8259InterruptControllerDxe/8259.inf
   INF OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  INF UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index d3531d388e24..4de4ed21a5ca 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -777,7 +777,7 @@
   OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 986228a44c78..10e97c35001f 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -222,7 +222,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
   INF OvmfPkg/8259InterruptControllerDxe/8259.inf
   INF OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  INF UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index c6ee624fc738..57b0c3c10826 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -775,7 +775,7 @@
   OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 99339e73bb51..b52c43127845 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -238,7 +238,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
   INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
   INF  OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 7c4c5412d102..d4a9e8f87def 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -551,7 +551,7 @@
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf
index 196853740753..76934d354fa6 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
+INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
similarity index 100%
rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
similarity index 100%
rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
similarity index 79%
rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
index 3ad28a148c5b..4f2b4db9e5dc 100644
--- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+++ b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
@@ -1,6 +1,9 @@
 ## @file
 # Local APIC timer driver that provides Timer Arch protocol.
-# PcdFSBClock is defined in MdePkg and it should be set by the consumer.
+#
+# This driver is to support fixed frequency. If a real platform happens
+# to have fixed frequency, then it can be used. In this case the consumer
+# should set PcdFSBClock which is defined in MdePkg.
 #
 # Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2019, Citrix Systems, Inc.
@@ -21,7 +24,6 @@
 [Packages]
   MdePkg/MdePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
-  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   UefiBootServicesTableLib
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-08  6:07 [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
                   ` (2 preceding siblings ...)
  2021-11-08  6:08 ` [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg Min Xu
@ 2021-11-09  6:59 ` Gerd Hoffmann
  2021-11-09  7:03   ` Yao, Jiewen
  3 siblings, 1 reply; 12+ messages in thread
From: Gerd Hoffmann @ 2021-11-09  6:59 UTC (permalink / raw)
  To: Min Xu
  Cc: devel, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Eric Dong, Ray Ni

On Mon, Nov 08, 2021 at 02:07:57PM +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.
> 
> Since LocalApicTimerDxe doesn't depend on OvmfPkg, so it is moved to
> UefiCpuPkg.
> 
> 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.
> 
> Patch #3:
> Move LocalApicTimerDxe from OvmfPkg to UefiCpuPkg.
> 
> Code at: https://github.com/mxu9/edk2/tree/ovmf_lapic_timer.v3
> 
> v3 changes:
>  - Move LocalApicTimerDxe to UefiCpuPkg
>  - Fix the errors in v2 patch-1 that OvmfXen.dsc / .fdf and
>    MicrovmX64.dsc / .fdf are not updated for new name.
>  - Add XenTimerDxe reviewers.
> 
> 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>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>

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


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-09  6:59 ` [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Gerd Hoffmann
@ 2021-11-09  7:03   ` Yao, Jiewen
  0 siblings, 0 replies; 12+ messages in thread
From: Yao, Jiewen @ 2021-11-09  7:03 UTC (permalink / raw)
  To: Gerd Hoffmann, Xu, Min M
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Justen, Jordan L,
	Brijesh Singh, Erdem Aktas, James Bottomley, Tom Lendacky,
	Dong, Eric, Ni, Ray

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, November 9, 2021 3:00 PM
> To: Xu, Min M <min.m.xu@intel.com>
> Cc: devel@edk2.groups.io; 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>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: Re: [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe
> 
> On Mon, Nov 08, 2021 at 02:07:57PM +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.
> >
> > Since LocalApicTimerDxe doesn't depend on OvmfPkg, so it is moved to
> > UefiCpuPkg.
> >
> > 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.
> >
> > Patch #3:
> > Move LocalApicTimerDxe from OvmfPkg to UefiCpuPkg.
> >
> > Code at: https://github.com/mxu9/edk2/tree/ovmf_lapic_timer.v3
> >
> > v3 changes:
> >  - Move LocalApicTimerDxe to UefiCpuPkg
> >  - Fix the errors in v2 patch-1 that OvmfXen.dsc / .fdf and
> >    MicrovmX64.dsc / .fdf are not updated for new name.
> >  - Add XenTimerDxe reviewers.
> >
> > 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>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 1/3] OvmfPkg: Rename XenTimerDxe to LocalApicTimerDxe
  2021-11-08  6:07 ` [PATCH V3 1/3] OvmfPkg: " Min Xu
@ 2021-11-11 14:46   ` Anthony PERARD
  0 siblings, 0 replies; 12+ messages in thread
From: Anthony PERARD @ 2021-11-11 14:46 UTC (permalink / raw)
  To: Min Xu
  Cc: devel, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann,
	Julien Grall

On Mon, Nov 08, 2021 at 02:07:58PM +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.
> 
> 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>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Signed-off-by: Min Xu <min.m.xu@intel.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
  2021-11-08  6:08 ` [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg Min Xu
@ 2021-11-11 14:47   ` Anthony PERARD
  2021-11-22  3:04   ` Ni, Ray
  1 sibling, 0 replies; 12+ messages in thread
From: Anthony PERARD @ 2021-11-11 14:47 UTC (permalink / raw)
  To: Min Xu; +Cc: devel, Jiewen Yao, Gerd Hoffmann, Julien Grall, Eric Dong, Ray Ni

On Mon, Nov 08, 2021 at 02:08:00PM +0800, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711
> 
> OvmfPkg.dec is removed from [Packages] because it doesn't depend
> on OvmfPkg. LocalApicTimerDxe is moved to UefiCpuPkg.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
  2021-11-08  6:08 ` [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg Min Xu
  2021-11-11 14:47   ` Anthony PERARD
@ 2021-11-22  3:04   ` Ni, Ray
  2021-11-22  3:34     ` Yao, Jiewen
  2021-11-23  8:24     ` Min Xu
  1 sibling, 2 replies; 12+ messages in thread
From: Ni, Ray @ 2021-11-22  3:04 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Yao, Jiewen, Gerd Hoffmann, Anthony Perard, Julien Grall,
	Dong, Eric

Min,
What's the reason of moving this driver to UefiCpuPkg?

When the LocalApicTimerDxe is in OvmfPkg, it's clear that this driver is only used by OVMF/QEMU platform.
Now since the patch moves the driver to UefiCpuPkg, it's possible that other platforms may choose this driver as the timer driver in DXE phase.

So, we need to make sure the quality of this driver is good enough for a broad scope of platforms.

1. What's the issue when this driver still stays in OvmfPkg? If it's a must that this driver stays in UefiCpuPkg, please help to address following questions:

2. Can SourceLevelDebug (rely on Local APIC timer) work if this driver is chosen as DXE timer driver?
3. Can detailed comments be added for " @bug : This does not handle missed timer interrupts" in TimerInterruptHandler()?
4. Can detailed comments be added for " DisableInterrupts ();" in TimerInterruptHandler()?
5. In general what kinds of platforms are capable of using this driver as Timer driver?

Thanks,
Ray

-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com> 
Sent: Monday, November 8, 2021 2:08 PM
To: devel@edk2.groups.io
Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Anthony Perard <anthony.perard@citrix.com>; Julien Grall <julien@xen.org>; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg

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

OvmfPkg.dec is removed from [Packages] because it doesn't depend
on OvmfPkg. LocalApicTimerDxe is moved to UefiCpuPkg.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc                                | 2 +-
 OvmfPkg/AmdSev/AmdSevX64.fdf                                | 2 +-
 OvmfPkg/Microvm/MicrovmX64.dsc                              | 2 +-
 OvmfPkg/Microvm/MicrovmX64.fdf                              | 2 +-
 OvmfPkg/OvmfPkgIa32.dsc                                     | 2 +-
 OvmfPkg/OvmfPkgIa32.fdf                                     | 2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                                  | 2 +-
 OvmfPkg/OvmfPkgIa32X64.fdf                                  | 2 +-
 OvmfPkg/OvmfPkgX64.dsc                                      | 2 +-
 OvmfPkg/OvmfPkgX64.fdf                                      | 2 +-
 OvmfPkg/OvmfXen.dsc                                         | 2 +-
 OvmfPkg/OvmfXen.fdf                                         | 2 +-
 .../LocalApicTimerDxe/LocalApicTimerDxe.c                   | 0
 .../LocalApicTimerDxe/LocalApicTimerDxe.h                   | 0
 .../LocalApicTimerDxe/LocalApicTimerDxe.inf                 | 6 ++++--
 15 files changed, 16 insertions(+), 14 deletions(-)
 rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.c (100%)
 rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.h (100%)
 rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.inf (79%)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 88c51dfe8337..888fc24f1b58 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -674,7 +674,7 @@
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/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 7489b04198fe..659810f96bec 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -208,7 +208,7 @@ INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
-INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 1a0e848f8356..fcb8b571a041 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.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 ac9efba26811..d02e88e2a48e 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
+INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index d0e9f3ca05f6..f8c8ef1e58be 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -763,7 +763,7 @@
   OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index b7b35a3a490a..321d4a871afa 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -218,7 +218,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
   INF OvmfPkg/8259InterruptControllerDxe/8259.inf
   INF OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  INF UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index d3531d388e24..4de4ed21a5ca 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -777,7 +777,7 @@
   OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 986228a44c78..10e97c35001f 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -222,7 +222,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
   INF OvmfPkg/8259InterruptControllerDxe/8259.inf
   INF OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  INF UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index c6ee624fc738..57b0c3c10826 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -775,7 +775,7 @@
   OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 99339e73bb51..b52c43127845 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -238,7 +238,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
   INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
   INF  OvmfPkg/8254TimerDxe/8254Timer.inf
 !else
-  INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 !endif
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 7c4c5412d102..d4a9e8f87def 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -551,7 +551,7 @@
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf
index 196853740753..76934d354fa6 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
+INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
similarity index 100%
rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
similarity index 100%
rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
similarity index 79%
rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
index 3ad28a148c5b..4f2b4db9e5dc 100644
--- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+++ b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
@@ -1,6 +1,9 @@
 ## @file
 # Local APIC timer driver that provides Timer Arch protocol.
-# PcdFSBClock is defined in MdePkg and it should be set by the consumer.
+#
+# This driver is to support fixed frequency. If a real platform happens
+# to have fixed frequency, then it can be used. In this case the consumer
+# should set PcdFSBClock which is defined in MdePkg.
 #
 # Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2019, Citrix Systems, Inc.
@@ -21,7 +24,6 @@
 [Packages]
   MdePkg/MdePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
-  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   UefiBootServicesTableLib
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
  2021-11-22  3:04   ` Ni, Ray
@ 2021-11-22  3:34     ` Yao, Jiewen
  2021-11-23  8:24     ` Min Xu
  1 sibling, 0 replies; 12+ messages in thread
From: Yao, Jiewen @ 2021-11-22  3:34 UTC (permalink / raw)
  To: Ni, Ray, Xu, Min M, devel@edk2.groups.io
  Cc: Gerd Hoffmann, Anthony Perard, Julien Grall, Dong, Eric

Thanks Ray.

I prefer to put the LocalApicTimerDxe to UefiCpuPkg, because it is hardware feature.
There is no specific OVMF or QEMU related thing there. It does not make sense to keep it to OvmfPkg.

Your quality concern is valid. I think the quality expectation for LocalApicTimerDxe should be same, no matter it is in OvmfPkg or UefiCpuPkg.
I don't think the expectation is that the quality can be lower if it is in OvmfPkg and it must be higher if it is in UefiCpuPkg.

All quality issues of LocalApicTimerDxe should be resolved, no matter where it is finally located.

Thank you
Yao Jiewen


> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Monday, November 22, 2021 11:05 AM
> To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Anthony Perard <anthony.perard@citrix.com>; Julien
> Grall <julien@xen.org>; Dong, Eric <eric.dong@intel.com>
> Subject: RE: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
> 
> Min,
> What's the reason of moving this driver to UefiCpuPkg?
> 
> When the LocalApicTimerDxe is in OvmfPkg, it's clear that this driver is only used
> by OVMF/QEMU platform.
> Now since the patch moves the driver to UefiCpuPkg, it's possible that other
> platforms may choose this driver as the timer driver in DXE phase.
> 
> So, we need to make sure the quality of this driver is good enough for a broad
> scope of platforms.
> 
> 1. What's the issue when this driver still stays in OvmfPkg? If it's a must that this
> driver stays in UefiCpuPkg, please help to address following questions:
> 
> 2. Can SourceLevelDebug (rely on Local APIC timer) work if this driver is chosen
> as DXE timer driver?
> 3. Can detailed comments be added for " @bug : This does not handle missed
> timer interrupts" in TimerInterruptHandler()?
> 4. Can detailed comments be added for " DisableInterrupts ();" in
> TimerInterruptHandler()?
> 5. In general what kinds of platforms are capable of using this driver as Timer
> driver?
> 
> Thanks,
> Ray
> 
> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Monday, November 8, 2021 2:08 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gerd Hoffmann <kraxel@redhat.com>; Anthony Perard
> <anthony.perard@citrix.com>; Julien Grall <julien@xen.org>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711
> 
> OvmfPkg.dec is removed from [Packages] because it doesn't depend
> on OvmfPkg. LocalApicTimerDxe is moved to UefiCpuPkg.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc                                | 2 +-
>  OvmfPkg/AmdSev/AmdSevX64.fdf                                | 2 +-
>  OvmfPkg/Microvm/MicrovmX64.dsc                              | 2 +-
>  OvmfPkg/Microvm/MicrovmX64.fdf                              | 2 +-
>  OvmfPkg/OvmfPkgIa32.dsc                                     | 2 +-
>  OvmfPkg/OvmfPkgIa32.fdf                                     | 2 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                                  | 2 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf                                  | 2 +-
>  OvmfPkg/OvmfPkgX64.dsc                                      | 2 +-
>  OvmfPkg/OvmfPkgX64.fdf                                      | 2 +-
>  OvmfPkg/OvmfXen.dsc                                         | 2 +-
>  OvmfPkg/OvmfXen.fdf                                         | 2 +-
>  .../LocalApicTimerDxe/LocalApicTimerDxe.c                   | 0
>  .../LocalApicTimerDxe/LocalApicTimerDxe.h                   | 0
>  .../LocalApicTimerDxe/LocalApicTimerDxe.inf                 | 6 ++++--
>  15 files changed, 16 insertions(+), 14 deletions(-)
>  rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.c
> (100%)
>  rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.h
> (100%)
>  rename {OvmfPkg => UefiCpuPkg}/LocalApicTimerDxe/LocalApicTimerDxe.inf
> (79%)
> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc
> b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index 88c51dfe8337..888fc24f1b58 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -674,7 +674,7 @@
>    MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>    UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>    UefiCpuPkg/CpuDxe/CpuDxe.inf
> -  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  UefiCpuPkg/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 7489b04198fe..659810f96bec 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.fdf
> +++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
> @@ -208,7 +208,7 @@ INF
> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>  INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>  INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>  INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
> -INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  INF
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
>  INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
>  INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc
> b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 1a0e848f8356..fcb8b571a041 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.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 ac9efba26811..d02e88e2a48e 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>  INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
>  INF
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index d0e9f3ca05f6..f8c8ef1e58be 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -763,7 +763,7 @@
>    OvmfPkg/8259InterruptControllerDxe/8259.inf
>    OvmfPkg/8254TimerDxe/8254Timer.inf
>  !else
> -  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  !endif
> 
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
>    OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index b7b35a3a490a..321d4a871afa 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -218,7 +218,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
>    INF OvmfPkg/8259InterruptControllerDxe/8259.inf
>    INF OvmfPkg/8254TimerDxe/8254Timer.inf
>  !else
> -  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  INF UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  !endif
>  INF
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
>  INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index d3531d388e24..4de4ed21a5ca 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -777,7 +777,7 @@
>    OvmfPkg/8259InterruptControllerDxe/8259.inf
>    OvmfPkg/8254TimerDxe/8254Timer.inf
>  !else
> -  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  !endif
> 
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
>    OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 986228a44c78..10e97c35001f 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -222,7 +222,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
>    INF OvmfPkg/8259InterruptControllerDxe/8259.inf
>    INF OvmfPkg/8254TimerDxe/8254Timer.inf
>  !else
> -  INF OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  INF UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  !endif
>  INF
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
>  INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index c6ee624fc738..57b0c3c10826 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -775,7 +775,7 @@
>    OvmfPkg/8259InterruptControllerDxe/8259.inf
>    OvmfPkg/8254TimerDxe/8254Timer.inf
>  !else
> -  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  !endif
> 
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
>    OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 99339e73bb51..b52c43127845 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -238,7 +238,7 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
>    INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
>    INF  OvmfPkg/8254TimerDxe/8254Timer.inf
>  !else
> -  INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  !endif
>  INF
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
>  INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 7c4c5412d102..d4a9e8f87def 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -551,7 +551,7 @@
>    MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> 
>    MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
> -  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>    UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>    UefiCpuPkg/CpuDxe/CpuDxe.inf
>    OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf
> index 196853740753..76934d354fa6 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/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +INF  UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
>  INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>  INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
>  INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
> b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
> similarity index 100%
> rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
> rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
> diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
> b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
> similarity index 100%
> rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
> rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.h
> diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> similarity index 79%
> rename from OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> rename to UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> index 3ad28a148c5b..4f2b4db9e5dc 100644
> --- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +++ b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> @@ -1,6 +1,9 @@
>  ## @file
>  # Local APIC timer driver that provides Timer Arch protocol.
> -# PcdFSBClock is defined in MdePkg and it should be set by the consumer.
> +#
> +# This driver is to support fixed frequency. If a real platform happens
> +# to have fixed frequency, then it can be used. In this case the consumer
> +# should set PcdFSBClock which is defined in MdePkg.
>  #
>  # Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved.<BR>
>  # Copyright (c) 2019, Citrix Systems, Inc.
> @@ -21,7 +24,6 @@
>  [Packages]
>    MdePkg/MdePkg.dec
>    UefiCpuPkg/UefiCpuPkg.dec
> -  OvmfPkg/OvmfPkg.dec
> 
>  [LibraryClasses]
>    UefiBootServicesTableLib
> --
> 2.29.2.windows.2


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
  2021-11-22  3:04   ` Ni, Ray
  2021-11-22  3:34     ` Yao, Jiewen
@ 2021-11-23  8:24     ` Min Xu
  2021-11-23  9:45       ` Gerd Hoffmann
  1 sibling, 1 reply; 12+ messages in thread
From: Min Xu @ 2021-11-23  8:24 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Gerd Hoffmann, Rebecca Cran,
	Peter Grehan, Tom Lendacky, Anthony Perard
  Cc: Yao, Jiewen, Dong, Eric

On November 22, 2021 11:05 AM, Ni Ray wrote:
> Min,
> What's the reason of moving this driver to UefiCpuPkg?
Please refer to the discussion in https://edk2.groups.io/g/devel/topic/86735078#83126
Because localApicTimerDxe is hardware feature. There is no specific OVMF or QEMU related thing there. It does not make sense to keep it to OvmfPkg.
> 
> When the LocalApicTimerDxe is in OvmfPkg, it's clear that this driver is only
> used by OVMF/QEMU platform.
> Now since the patch moves the driver to UefiCpuPkg, it's possible that other
> platforms may choose this driver as the timer driver in DXE phase.
> 
> So, we need to make sure the quality of this driver is good enough for a
> broad scope of platforms.
> 
> 1. What's the issue when this driver still stays in OvmfPkg? If it's a must that
> this driver stays in UefiCpuPkg, please help to address following questions:
> 
> 2. Can SourceLevelDebug (rely on Local APIC timer) work if this driver is
> chosen as DXE timer driver?
After checking the code and running the PoC (enable SourceLevelDebug and LocalApicTimerDxe),  I find this driver doesn't work anymore. 
Actually it asserts in https://github.com/mxu9/edk2/blob/ovmf_lapic_timer.v3/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c#L334

For TDVF we assume SourceLevelDebug is not supported. So LocalApicTimerDxe is still working for TDVF.

For other VM guests in OvmfPkg, can we assume SourceLevelDebug is NOT supported after LocalApicTimer is used (the older timer is 8254)?
@Gerd Hoffmann (OvmfPkg)
@ Rebecca Cran, @ Peter Grehan  (Bhyve)
@ Tom Lendacky, @Brijesh Singh (Sev)
@ Anthony Perard , Julien Grall (Xen) 

This limitation will be documented in LocalApicTimerDxe.inf so that the user of this timer driver is aware of it.

> 3. Can detailed comments be added for " @bug : This does not handle
> missed timer interrupts" in TimerInterruptHandler()?
Sure. More comments will be added.
> 4. Can detailed comments be added for " DisableInterrupts ();" in
> TimerInterruptHandler()?
Sure. More comments will be added.
> 5. In general what kinds of platforms are capable of using this driver as Timer
> driver?
Platforms that support local Apic are capable of using this driver.
> 

Thanks
Min

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg
  2021-11-23  8:24     ` Min Xu
@ 2021-11-23  9:45       ` Gerd Hoffmann
  0 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2021-11-23  9:45 UTC (permalink / raw)
  To: Xu, Min M
  Cc: Ni, Ray, devel@edk2.groups.io, Rebecca Cran, Peter Grehan,
	Tom Lendacky, Anthony Perard, Yao, Jiewen, Dong, Eric

  Hi,

> > 2. Can SourceLevelDebug (rely on Local APIC timer) work if this driver is
> > chosen as DXE timer driver?
> After checking the code and running the PoC (enable SourceLevelDebug and LocalApicTimerDxe),  I find this driver doesn't work anymore. 
> Actually it asserts in https://github.com/mxu9/edk2/blob/ovmf_lapic_timer.v3/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c#L334

Hmm, I guess this makes LocalApicTimerDxe alot less useful on physical
hardware.  So keep it in OvmdPkg?

For virtual machines there are other debugging options, using qemu's
builtin gdb server for example, so SourceLevelDebug not being available
isn't a big issue IMHO.

> For TDVF we assume SourceLevelDebug is not supported. So LocalApicTimerDxe is still working for TDVF.
> 
> For other VM guests in OvmfPkg, can we assume SourceLevelDebug is NOT supported after LocalApicTimer is used (the older timer is 8254)?
> @ Gerd Hoffmann (OvmfPkg)

Fine with me.

> @ Anthony Perard , Julien Grall (Xen) 

Xen already uses the apic timer today, so that shouldn't be a problem
either ;)

take care,
  Gerd


^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-11-23  9:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-08  6:07 [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
2021-11-08  6:07 ` [PATCH V3 1/3] OvmfPkg: " Min Xu
2021-11-11 14:46   ` Anthony PERARD
2021-11-08  6:07 ` [PATCH V3 2/3] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
2021-11-08  6:08 ` [PATCH V3 3/3] OvmfPkg: Move LocalApicTimerDxe to UefiCpuPkg Min Xu
2021-11-11 14:47   ` Anthony PERARD
2021-11-22  3:04   ` Ni, Ray
2021-11-22  3:34     ` Yao, Jiewen
2021-11-23  8:24     ` Min Xu
2021-11-23  9:45       ` Gerd Hoffmann
2021-11-09  6:59 ` [PATCH V3 0/3] Rename XenTimerDxe to LocalApicTimerDxe Gerd Hoffmann
2021-11-09  7:03   ` Yao, Jiewen

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