public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe
@ 2021-10-29  6:50 Min Xu
  2021-10-29  6:50 ` [PATCH 1/2] OvmfPkg: " Min Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Min Xu @ 2021-10-29  6:50 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

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                              | 3 +--
 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                                   | 6 +++++-
 OvmfPkg/OvmfPkgIa32.fdf                                   | 8 ++++++--
 OvmfPkg/OvmfPkgIa32X64.dsc                                | 6 +++++-
 OvmfPkg/OvmfPkgIa32X64.fdf                                | 8 ++++++--
 OvmfPkg/OvmfPkgX64.dsc                                    | 6 +++++-
 OvmfPkg/OvmfPkgX64.fdf                                    | 8 ++++++--
 OvmfPkg/OvmfXen.dsc                                       | 2 +-
 OvmfPkg/OvmfXen.fdf                                       | 2 +-
 15 files changed, 47 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] 6+ messages in thread

* [PATCH 1/2] OvmfPkg: Rename XenTimerDxe to LocalApicTimerDxe
  2021-10-29  6:50 [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
@ 2021-10-29  6:50 ` Min Xu
  2021-10-29  6:50 ` [PATCH 2/2] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
  2021-10-29  9:19 ` [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe Ard Biesheuvel
  2 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2021-10-29  6:50 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.

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] 6+ messages in thread

* [PATCH 2/2] OvmfPkg: Switch timer in build time for OvmfPkg
  2021-10-29  6:50 [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
  2021-10-29  6:50 ` [PATCH 1/2] OvmfPkg: " Min Xu
@ 2021-10-29  6:50 ` Min Xu
  2021-10-29 11:37   ` Gerd Hoffmann
  2021-10-29  9:19 ` [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe Ard Biesheuvel
  2 siblings, 1 reply; 6+ messages in thread
From: Min Xu @ 2021-10-29  6:50 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 LocalApicTimerDxe 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                  | 3 +--
 OvmfPkg/AmdSev/AmdSevX64.fdf                  | 3 +--
 OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c | 4 ++--
 OvmfPkg/OvmfPkgIa32.dsc                       | 6 +++++-
 OvmfPkg/OvmfPkgIa32.fdf                       | 8 ++++++--
 OvmfPkg/OvmfPkgIa32X64.dsc                    | 6 +++++-
 OvmfPkg/OvmfPkgIa32X64.fdf                    | 8 ++++++--
 OvmfPkg/OvmfPkgX64.dsc                        | 6 +++++-
 OvmfPkg/OvmfPkgX64.fdf                        | 8 ++++++--
 9 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 5ee54451169b..89e37cfdd72b 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -670,10 +670,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..3e25a60ebf02 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -753,10 +753,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..36efa7bc98fe 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -767,10 +767,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..49774d683359 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -765,10 +765,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] 6+ messages in thread

* Re: [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe
  2021-10-29  6:50 [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
  2021-10-29  6:50 ` [PATCH 1/2] OvmfPkg: " Min Xu
  2021-10-29  6:50 ` [PATCH 2/2] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
@ 2021-10-29  9:19 ` Ard Biesheuvel
  2 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2021-10-29  9:19 UTC (permalink / raw)
  To: Min Xu
  Cc: edk2-devel-groups-io, Ard Biesheuvel, Jordan Justen,
	Brijesh Singh, Erdem Aktas, James Bottomley, Jiewen Yao,
	Tom Lendacky, Gerd Hoffmann

On Fri, 29 Oct 2021 at 08:51, Min Xu <min.m.xu@intel.com> 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.
>

For the series,

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> Code at: https://github.com/mxu9/edk2/tree/ovmf_lapic_timer
>
> 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                              | 3 +--
>  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                                   | 6 +++++-
>  OvmfPkg/OvmfPkgIa32.fdf                                   | 8 ++++++--
>  OvmfPkg/OvmfPkgIa32X64.dsc                                | 6 +++++-
>  OvmfPkg/OvmfPkgIa32X64.fdf                                | 8 ++++++--
>  OvmfPkg/OvmfPkgX64.dsc                                    | 6 +++++-
>  OvmfPkg/OvmfPkgX64.fdf                                    | 8 ++++++--
>  OvmfPkg/OvmfXen.dsc                                       | 2 +-
>  OvmfPkg/OvmfXen.fdf                                       | 2 +-
>  15 files changed, 47 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] 6+ messages in thread

* Re: [PATCH 2/2] OvmfPkg: Switch timer in build time for OvmfPkg
  2021-10-29  6:50 ` [PATCH 2/2] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
@ 2021-10-29 11:37   ` Gerd Hoffmann
  2021-10-30  7:24     ` [edk2-devel] " Min Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2021-10-29 11:37 UTC (permalink / raw)
  To: Min Xu
  Cc: devel, Ard Biesheuvel, Jordan Justen, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky

> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -670,10 +670,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 {

Also needs "gEfiMdePkgTokenSpaceGuid.PcdFSBClock|1000000000"

(same for the other OvmfPkg files).

> --- 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;

Why is this cast needed?

take care,
  Gerd


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

* Re: [edk2-devel] [PATCH 2/2] OvmfPkg: Switch timer in build time for OvmfPkg
  2021-10-29 11:37   ` Gerd Hoffmann
@ 2021-10-30  7:24     ` Min Xu
  0 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2021-10-30  7:24 UTC (permalink / raw)
  To: devel@edk2.groups.io, kraxel@redhat.com
  Cc: Ard Biesheuvel, Justen, Jordan L, Brijesh Singh, Erdem Aktas,
	James Bottomley, Yao, Jiewen, Tom Lendacky

On October 29, 2021 7:37 PM, Gerd Hoffmann wrote:
> > --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> > +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> > @@ -670,10 +670,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.i
> nf
> >    OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> >    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
> 
> Also needs "gEfiMdePkgTokenSpaceGuid.PcdFSBClock|1000000000"
> 
> (same for the other OvmfPkg files).
Ah, thanks for reminder. The default value is 200000000. It will be fixed in the next version.

> 
> > --- 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;
> 
> Why is this cast needed?
> 
That is because of below error when build OvmfPkgX64.dsc
OvmfPkg\LocalApicTimerDxe\LocalApicTimerDxe.c(176): error C2220: warning treated as error - no 'object' file generated
OvmfPkg\LocalApicTimerDxe\LocalApicTimerDxe.c(176): warning C4244: '=': conversion from 'UINTN' to 'UINT32', possible loss of data

Thanks.
Min

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

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

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-29  6:50 [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe Min Xu
2021-10-29  6:50 ` [PATCH 1/2] OvmfPkg: " Min Xu
2021-10-29  6:50 ` [PATCH 2/2] OvmfPkg: Switch timer in build time for OvmfPkg Min Xu
2021-10-29 11:37   ` Gerd Hoffmann
2021-10-30  7:24     ` [edk2-devel] " Min Xu
2021-10-29  9:19 ` [PATCH 0/2] Rename XenTimerDxe to LocalApicTimerDxe Ard Biesheuvel

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