From: "Ni, Ray" <ray.ni@intel.com>
To: Michael Brown <mcb30@ipxe.org>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>,
Gerd Hoffmann <kraxel@redhat.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Move NestedInterruptTplLib to MdeModulePkg
Date: Sat, 20 Jan 2024 07:03:27 +0000 [thread overview]
Message-ID: <MN6PR11MB824421D73C80FEC6374B392E8C772@MN6PR11MB8244.namprd11.prod.outlook.com> (raw)
In-Reply-To: <0102018d240b0ba0-b3b75cd0-d7bc-4505-bea7-db233fdcd82a-000000@eu-west-1.amazonses.com>
Michael,
Can you submit another patch to add Iret.h to the INF file [Sources] section?
Without that, incremental build might not work if changes are made in Iret.h.
It doesn't support non-x86 CPU. Will ARM have similar problems?
+Ard,
Thanks,
Ray
> -----Original Message-----
> From: Michael Brown <mcb30@ipxe.org>
> Sent: Saturday, January 20, 2024 7:25 AM
> To: devel@edk2.groups.io
> Cc: Michael Brown <mcb30@ipxe.org>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH 1/1] MdeModulePkg: Move NestedInterruptTplLib to
> MdeModulePkg
>
> 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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQe
> muLoadImageLib.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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoa
> dImageLib.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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoa
> dImageLib.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/BaseCachingPciExp
> ressLib.inf
> MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -
> NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoa
> dImageLib.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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoa
> dImageLib.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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoa
> dImageLib.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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoa
> dImageLib.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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.inf
>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF
> wCfg.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/NestedInterr
> uptTplLib.inf
> +
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/Neste
> dInterruptTplLib.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 (#114114): https://edk2.groups.io/g/devel/message/114114
Mute This Topic: https://groups.io/mt/103841406/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-01-20 7:03 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-19 23:25 [edk2-devel] [PATCH 1/1] MdeModulePkg: Move NestedInterruptTplLib to MdeModulePkg Michael Brown
2024-01-20 7:03 ` Ni, Ray [this message]
2024-01-20 17:02 ` Michael Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN6PR11MB824421D73C80FEC6374B392E8C772@MN6PR11MB8244.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox