[edk2-devel] [PATCH v2 0/2] Move NestedInterruptTplLib to MdeModulePkg

[edk2-devel] [PATCH v2 0/2] Move NestedInterruptTplLib to MdeModulePkg

[Michael Brown]

NestedInterruptTplLib provides a way for timer interrupt handlers
(which must support nested interrupts) to prevent unbounded stack
consumption.

The underlying issue was first observed in OvmfPkg, since interrupt
storms can arise more easily in virtual machines due to CPU
starvation.  However, careful investigation shows that the unbounded
stack consumption can also occur in physical machines.

Move NestedInterruptTplLib from OvmfPkg to MdeModulePkg so that it can
more easily be consumed by drivers outside of OvmfPkg.

Changes since v1:
  - Add missing Iret.h to NestedInterruptTplLib sources list

Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>

Michael Brown (2):
  MdeModulePkg: Move NestedInterruptTplLib to MdeModulePkg
  MdeModulePkg: Add missing Iret.h to NestedInterruptTplLib sources list

 MdeModulePkg/MdeModulePkg.dec                                 | 4 ++++
 OvmfPkg/OvmfPkg.dec                                           | 4 ----
 OvmfPkg/AmdSev/AmdSevX64.dsc                                  | 2 +-
 OvmfPkg/CloudHv/CloudHvX64.dsc                                | 2 +-
 OvmfPkg/IntelTdx/IntelTdxX64.dsc                              | 2 +-
 OvmfPkg/Microvm/MicrovmX64.dsc                                | 2 +-
 OvmfPkg/OvmfPkgIa32.dsc                                       | 2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                                    | 2 +-
 OvmfPkg/OvmfPkgX64.dsc                                        | 2 +-
 OvmfPkg/OvmfXen.dsc                                           | 2 +-
 UefiPayloadPkg/UefiPayloadPkg.dsc                             | 2 +-
 .../Library/NestedInterruptTplLib/NestedInterruptTplLib.inf   | 3 ++-
 OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf               | 1 +
 .../Include/Library/NestedInterruptTplLib.h                   | 0
 .../Library/NestedInterruptTplLib/Iret.h                      | 0
 .../Library/NestedInterruptTplLib/Iret.c                      | 0
 {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c | 0
 17 files changed, 16 insertions(+), 14 deletions(-)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf (90%)
 rename {OvmfPkg => MdeModulePkg}/Include/Library/NestedInterruptTplLib.h (100%)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.h (100%)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.c (100%)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c (100%)

-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114119): https://edk2.groups.io/g/devel/message/114119
Mute This Topic: https://groups.io/mt/103852899/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/2] MdeModulePkg: Move NestedInterruptTplLib to MdeModulePkg

[Michael Brown]

NestedInterruptTplLib provides a way for timer interrupt handlers
(which must support nested interrupts) to prevent unbounded stack
consumption.

The underlying issue was first observed in OvmfPkg, since interrupt
storms can arise more easily in virtual machines due to CPU
starvation.  However, careful investigation shows that the unbounded
stack consumption can also occur in physical machines.

Move NestedInterruptTplLib from OvmfPkg to MdeModulePkg so that it can
more easily be consumed by drivers outside of OvmfPkg.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 MdeModulePkg/MdeModulePkg.dec                                 | 4 ++++
 OvmfPkg/OvmfPkg.dec                                           | 4 ----
 OvmfPkg/AmdSev/AmdSevX64.dsc                                  | 2 +-
 OvmfPkg/CloudHv/CloudHvX64.dsc                                | 2 +-
 OvmfPkg/IntelTdx/IntelTdxX64.dsc                              | 2 +-
 OvmfPkg/Microvm/MicrovmX64.dsc                                | 2 +-
 OvmfPkg/OvmfPkgIa32.dsc                                       | 2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                                    | 2 +-
 OvmfPkg/OvmfPkgX64.dsc                                        | 2 +-
 OvmfPkg/OvmfXen.dsc                                           | 2 +-
 UefiPayloadPkg/UefiPayloadPkg.dsc                             | 2 +-
 .../Library/NestedInterruptTplLib/NestedInterruptTplLib.inf   | 2 +-
 OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf               | 1 +
 .../Include/Library/NestedInterruptTplLib.h                   | 0
 .../Library/NestedInterruptTplLib/Iret.h                      | 0
 .../Library/NestedInterruptTplLib/Iret.c                      | 0
 {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c | 0
 17 files changed, 15 insertions(+), 14 deletions(-)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf (91%)
 rename {OvmfPkg => MdeModulePkg}/Include/Library/NestedInterruptTplLib.h (100%)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.h (100%)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.c (100%)
 rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c (100%)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a2cd83345f5b..d6fb729af5a7 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -169,6 +169,10 @@ [LibraryClasses]
   #
   ImagePropertiesRecordLib|Include/Library/ImagePropertiesRecordLib.h
 
+  ##  @libraryclass  Handle TPL changes within nested interrupt handlers
+  #
+  NestedInterruptTplLib|Include/Library/NestedInterruptTplLib.h
+
 [Guids]
   ## MdeModule package token space guid
   # Include/Guid/MdeModulePkgTokenSpace.h
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index b44fa039f76c..05d43d5a6861 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -41,10 +41,6 @@ [LibraryClasses]
   #
   MemEncryptTdxLib|Include/Library/MemEncryptTdxLib.h
 
-  ##  @libraryclass  Handle TPL changes within nested interrupt handlers
-  #
-  NestedInterruptTplLib|Include/Library/NestedInterruptTplLib.h
-
   ##  @libraryclass  Save and restore variables using a file
   #
   NvVarsFileLib|Include/Library/NvVarsFileLib.h
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index a31a89344a60..80456f878a22 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -354,7 +354,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
 
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index b522fa10594d..9c6c68ae2c35 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -394,7 +394,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 82e3e41cfc57..5270c59e1279 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -310,7 +310,7 @@ [LibraryClasses.common.DXE_DRIVER]
   LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 063324cd0572..8ec00eaf2015 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -393,7 +393,7 @@ [LibraryClasses.common.DXE_DRIVER]
   PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 28379961a78e..154afbbaf45d 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -401,7 +401,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 5e9eee628aea..0f6173607759 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -407,7 +407,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index bf4c7906c460..93e7e9a18cd0 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -427,7 +427,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 976b795d41c9..c2472aca38aa 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -340,7 +340,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
-  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_APPLICATION]
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 4f195c1e5212..51f18b5c0c2f 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -687,7 +687,7 @@ [Components.X64]
 !elseif $(TIMER_SUPPORT) == "LAPIC"
   OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf {
     <LibraryClasses>
-      NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+      NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   }
 !else
   !error "Invalid TIMER_SUPPORT"
diff --git a/OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
similarity index 91%
rename from OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
rename to MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
index 5eafb4197842..1e03e1364e0f 100644
--- a/OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+++ b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
@@ -24,7 +24,7 @@ [Sources]
 
 [Packages]
   MdePkg/MdePkg.dec
-  OvmfPkg/OvmfPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
   BaseLib
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
index b85965c75ea3..8a992764fb98 100644
--- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
@@ -20,6 +20,7 @@ [Defines]
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
   OvmfPkg/OvmfPkg.dec
 
diff --git a/OvmfPkg/Include/Library/NestedInterruptTplLib.h b/MdeModulePkg/Include/Library/NestedInterruptTplLib.h
similarity index 100%
rename from OvmfPkg/Include/Library/NestedInterruptTplLib.h
rename to MdeModulePkg/Include/Library/NestedInterruptTplLib.h
diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Iret.h b/MdeModulePkg/Library/NestedInterruptTplLib/Iret.h
similarity index 100%
rename from OvmfPkg/Library/NestedInterruptTplLib/Iret.h
rename to MdeModulePkg/Library/NestedInterruptTplLib/Iret.h
diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Iret.c b/MdeModulePkg/Library/NestedInterruptTplLib/Iret.c
similarity index 100%
rename from OvmfPkg/Library/NestedInterruptTplLib/Iret.c
rename to MdeModulePkg/Library/NestedInterruptTplLib/Iret.c
diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c b/MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c
similarity index 100%
rename from OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
rename to MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114120): https://edk2.groups.io/g/devel/message/114120
Mute This Topic: https://groups.io/mt/103852900/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 2/2] MdeModulePkg: Add missing Iret.h to NestedInterruptTplLib sources list

[Michael Brown]

Suggested-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 .../Library/NestedInterruptTplLib/NestedInterruptTplLib.inf      | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
index 1e03e1364e0f..f130d6dcd213 100644
--- a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+++ b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
@@ -20,6 +20,7 @@ [Defines]
 
 [Sources]
   Tpl.c
+  Iret.h
   Iret.c
 
 [Packages]
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114121): https://edk2.groups.io/g/devel/message/114121
Mute This Topic: https://groups.io/mt/103852903/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: Move NestedInterruptTplLib to MdeModulePkg

[Laszlo Ersek]

Two comments:

On 1/20/24 18:08, Michael Brown wrote:
> NestedInterruptTplLib provides a way for timer interrupt handlers
> (which must support nested interrupts) to prevent unbounded stack
> consumption.
>
> The underlying issue was first observed in OvmfPkg, since interrupt
> storms can arise more easily in virtual machines due to CPU
> starvation.  However, careful investigation shows that the unbounded
> stack consumption can also occur in physical machines.
>
> Move NestedInterruptTplLib from OvmfPkg to MdeModulePkg so that it can
> more easily be consumed by drivers outside of OvmfPkg.
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Michael Brown <mcb30@ipxe.org>
> ---
>  MdeModulePkg/MdeModulePkg.dec                                 | 4 ++++
>  OvmfPkg/OvmfPkg.dec                                           | 4 ----
>  OvmfPkg/AmdSev/AmdSevX64.dsc                                  | 2 +-
>  OvmfPkg/CloudHv/CloudHvX64.dsc                                | 2 +-
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc                              | 2 +-
>  OvmfPkg/Microvm/MicrovmX64.dsc                                | 2 +-
>  OvmfPkg/OvmfPkgIa32.dsc                                       | 2 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                                    | 2 +-
>  OvmfPkg/OvmfPkgX64.dsc                                        | 2 +-
>  OvmfPkg/OvmfXen.dsc                                           | 2 +-
>  UefiPayloadPkg/UefiPayloadPkg.dsc                             | 2 +-
>  .../Library/NestedInterruptTplLib/NestedInterruptTplLib.inf   | 2 +-
>  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf               | 1 +
>  .../Include/Library/NestedInterruptTplLib.h                   | 0
>  .../Library/NestedInterruptTplLib/Iret.h                      | 0
>  .../Library/NestedInterruptTplLib/Iret.c                      | 0
>  {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c | 0
>  17 files changed, 15 insertions(+), 14 deletions(-)
>  rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf (91%)
>  rename {OvmfPkg => MdeModulePkg}/Include/Library/NestedInterruptTplLib.h (100%)
>  rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.h (100%)
>  rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.c (100%)
>  rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c (100%)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index a2cd83345f5b..d6fb729af5a7 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -169,6 +169,10 @@ [LibraryClasses]
>    #
>    ImagePropertiesRecordLib|Include/Library/ImagePropertiesRecordLib.h
>
> +  ##  @libraryclass  Handle TPL changes within nested interrupt handlers
> +  #
> +  NestedInterruptTplLib|Include/Library/NestedInterruptTplLib.h
> +
>  [Guids]
>    ## MdeModule package token space guid
>    # Include/Guid/MdeModulePkgTokenSpace.h
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index b44fa039f76c..05d43d5a6861 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -41,10 +41,6 @@ [LibraryClasses]
>    #
>    MemEncryptTdxLib|Include/Library/MemEncryptTdxLib.h
>
> -  ##  @libraryclass  Handle TPL changes within nested interrupt handlers
> -  #
> -  NestedInterruptTplLib|Include/Library/NestedInterruptTplLib.h
> -
>    ##  @libraryclass  Save and restore variables using a file
>    #
>    NvVarsFileLib|Include/Library/NvVarsFileLib.h
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index a31a89344a60..80456f878a22 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -354,7 +354,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>    QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
> index b522fa10594d..9c6c68ae2c35 100644
> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
> @@ -394,7 +394,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>    QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> index 82e3e41cfc57..5270c59e1279 100644
> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> @@ -310,7 +310,7 @@ [LibraryClasses.common.DXE_DRIVER]
>    LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>    QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 063324cd0572..8ec00eaf2015 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
> @@ -393,7 +393,7 @@ [LibraryClasses.common.DXE_DRIVER]
>    PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>    PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>    QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 28379961a78e..154afbbaf45d 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -401,7 +401,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>    QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 5e9eee628aea..0f6173607759 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -407,7 +407,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>    QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index bf4c7906c460..93e7e9a18cd0 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -427,7 +427,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>    QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 976b795d41c9..c2472aca38aa 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -340,7 +340,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>
>  [LibraryClasses.common.UEFI_APPLICATION]
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 4f195c1e5212..51f18b5c0c2f 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -687,7 +687,7 @@ [Components.X64]
>  !elseif $(TIMER_SUPPORT) == "LAPIC"
>    OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf {
>      <LibraryClasses>
> -      NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +      NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>    }
>  !else
>    !error "Invalid TIMER_SUPPORT"
> diff --git a/OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> similarity index 91%
> rename from OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> rename to MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> index 5eafb4197842..1e03e1364e0f 100644
> --- a/OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +++ b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> @@ -24,7 +24,7 @@ [Sources]
>
>  [Packages]
>    MdePkg/MdePkg.dec
> -  OvmfPkg/OvmfPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
>
>  [LibraryClasses]
>    BaseLib
> diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> index b85965c75ea3..8a992764fb98 100644
> --- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> +++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
> @@ -20,6 +20,7 @@ [Defines]
>
>  [Packages]
>    MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
>    UefiCpuPkg/UefiCpuPkg.dec
>    OvmfPkg/OvmfPkg.dec
>

(1) Does LocalApicTimerDxe have any other dependencies on OvmfPkg than
the library class header?

In other words, can't we remove "OvmfPkg/OvmfPkg.dec" from this
[Packages] section, too?

I can't see anything else, so I'd suggest to remove
"OvmfPkg/OvmfPkg.dec", but I could be missing something.


> diff --git a/OvmfPkg/Include/Library/NestedInterruptTplLib.h b/MdeModulePkg/Include/Library/NestedInterruptTplLib.h
> similarity index 100%
> rename from OvmfPkg/Include/Library/NestedInterruptTplLib.h
> rename to MdeModulePkg/Include/Library/NestedInterruptTplLib.h
> diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Iret.h b/MdeModulePkg/Library/NestedInterruptTplLib/Iret.h
> similarity index 100%
> rename from OvmfPkg/Library/NestedInterruptTplLib/Iret.h
> rename to MdeModulePkg/Library/NestedInterruptTplLib/Iret.h
> diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Iret.c b/MdeModulePkg/Library/NestedInterruptTplLib/Iret.c
> similarity index 100%
> rename from OvmfPkg/Library/NestedInterruptTplLib/Iret.c
> rename to MdeModulePkg/Library/NestedInterruptTplLib/Iret.c
> diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c b/MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c
> similarity index 100%
> rename from OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
> rename to MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c

(2) Core packages tend to differ from "leaf" packages in some ways; eg.:

- they don't build actual firmware platforms (so they don't have FDF
files),

- their DSC files list all possible components just for ensuring build
coverage -- not only driver and application binaries (executables), but
library instances too. (Well, for executables, it may not be *only*
"build coverage", as some of those *.efi binaries may well be executable
from a UEFI shell; but the basic idea is build coverage.)

Therefore, I suggest adding "NestedInterruptTplLib.inf" to
"MdeModulePkg.dsc". AIUI, right now the library instance is x86-only
(with a proposal on-list for the AARCH64 extension). Therefore, the
section in "MdeModulePkg.dsc" to add the INF file to would be:

[Components.IA32, Components.X64]

Within that section, library instances and drivers are apparently not
listed in lexicographical order, so maybe just tack the new line to the
bottom.

For build-testing, run the command

  build \
    -a IA32 -a X64 \
    -b NOOPT \
    -p MdeModulePkg/MdeModulePkg.dsc \
    -t GCC5 \
    -m MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf

(Feel free to use DEBUG or RELEASE rather than NOOPT; NOOPT should be
the fastest build. Also, change GCC5 to something else if necessary.)

With (1) and (2) considered / updated:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114140): https://edk2.groups.io/g/devel/message/114140
Mute This Topic: https://groups.io/mt/103852900/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 2/2] MdeModulePkg: Add missing Iret.h to NestedInterruptTplLib sources list

[Laszlo Ersek]

On 1/20/24 18:08, Michael Brown wrote:
> Suggested-by: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael Brown <mcb30@ipxe.org>
> ---
>  .../Library/NestedInterruptTplLib/NestedInterruptTplLib.inf      | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> index 1e03e1364e0f..f130d6dcd213 100644
> --- a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +++ b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> @@ -20,6 +20,7 @@ [Defines]
>  
>  [Sources]
>    Tpl.c
> +  Iret.h
>    Iret.c
>  
>  [Packages]

(1) Please add a sentence to the commit message body: "This will allow
the build system to notice changes to Iret.h".

(The statement may be trivial :) I just don't like it when a commit
message only consists of a subject line.)

Thanks for considering!

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114141): https://edk2.groups.io/g/devel/message/114141
Mute This Topic: https://groups.io/mt/103852903/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: Move NestedInterruptTplLib to MdeModulePkg

[Michael Brown]

On 22/01/2024 10:36, Laszlo Ersek wrote:
> (1) Does LocalApicTimerDxe have any other dependencies on OvmfPkg than
> the library class header?
> 
> In other words, can't we remove "OvmfPkg/OvmfPkg.dec" from this
> [Packages] section, too?
> 
> I can't see anything else, so I'd suggest to remove
> "OvmfPkg/OvmfPkg.dec", but I could be missing something.

Good catch, thank you.  I have removed it and OvmfPkg still builds 
without errors, so I think it can go.

> Therefore, I suggest adding "NestedInterruptTplLib.inf" to
> "MdeModulePkg.dsc". AIUI, right now the library instance is x86-only
> (with a proposal on-list for the AARCH64 extension). Therefore, the
> section in "MdeModulePkg.dsc" to add the INF file to would be:
> 
> [Components.IA32, Components.X64]
> 
> Within that section, library instances and drivers are apparently not
> listed in lexicographical order, so maybe just tack the new line to the
> bottom.
> 
> For build-testing, run the command
> 
>    build \
>      -a IA32 -a X64 \
>      -b NOOPT \
>      -p MdeModulePkg/MdeModulePkg.dsc \
>      -t GCC5 \
>      -m MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf

Thank you very much!  I have made all of the changes that you suggested 
(in both emails).

Following Ray's concerns about the ability to test this code, I have 
also added a self-test capability.  This deliberately triggers a 
configurable number of delays within NestedInterruptRestoreTPL() to 
force the system to experience a nested interrupt.  The code verifies that:

a) the architecture's implementation of DisableInterruptsOnIret() is 
behaving as specified, and

b) the platform's timer interrupt handler is correctly rearming the 
timer interrupt before calling NestedInterruptRestoreTPL().

I've also added AArch64 and ARM implementations for 
DisableInterruptsOnIret(), which I shall test shortly.

Patch series v3 to follow.

Thanks,

Michael



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114198): https://edk2.groups.io/g/devel/message/114198
Mute This Topic: https://groups.io/mt/103852900/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-