On 03/12/21 07:26, Ankur Arora wrote:
> Init CPU_HOT_EJECT_DATA, which will be used to share CPU ejection
> state between SmmCpuFeaturesLib (via PiSmmCpuDxeSmm) and CpuHotPlugSmm.
>
> The init happens via SmmCpuFeaturesSmmRelocationComplete(), and so it
> will run as part of the PiSmmCpuDxeSmm entry point function,
> PiCpuSmmEntry(). Once inited, CPU_HOT_EJECT_DATA is exposed via
> PcdCpuHotEjectDataAddress.
>
> The CPU hot-eject handler (CPU_HOT_EJECT_DATA->Handler) is setup when
> there is an ejection request via CpuHotplugSmm.
>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Ard Biesheuvel <ard.biesheu...@arm.com>
> Cc: Igor Mammedov <imamm...@redhat.com>
> Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
> Cc: Aaron Young <aaron.yo...@oracle.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
> Signed-off-by: Ankur Arora <ankur.a.ar...@oracle.com>
> ---
>
> Notes:
> Addresses the following comments from v8:
>
> (1) Remove line before the "if (MaxNumberofCpus == 1)" check.
> (3) Fixup the space around "||".
> (2,6) Simplify the three SafeInt multiplication into the ones suggested
> by Laszlo.
> (4) Get rid of the mixed sizeof(mCpuHotEjectData->QemuSelectorMap[0]) and
> sizeof(UINT64) in favour of UINT64 everywhere. I was planning to use
> the first, but describing the alignment needed is easier in terms of
> the
> second.
> Also, as Laszlo's comments on v8-patch-9 mention, we don't really need
> this alignment for correctness reasons. This patch retains it, so we
> don't pay access penalty for unaligned access.
> (5) Change alignment from UINT64 to UINT64-1.
> (7) Use the more idiomatic ALIGN_POINTER instead of ALIGN_VALUE.
> (8) RETURN_ERROR -> ASSERT_RETURN_ERROR.
>
> .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 ++
> .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 77
> ++++++++++++++++++++++
> 2 files changed, 81 insertions(+)
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks!
Laszlo
>
> diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> index 97a10afb6e27..8a426a4c10fb 100644
> --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> @@ -30,9 +30,13 @@ [LibraryClasses]
> BaseMemoryLib
> DebugLib
> MemEncryptSevLib
> + MemoryAllocationLib
> PcdLib
> + SafeIntLib
> SmmServicesTableLib
> UefiBootServicesTableLib
>
> [Pcd]
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
> + gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress
> gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
> diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> index 7ef7ed98342e..5c025bc717c3 100644
> --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> @@ -11,10 +11,13 @@
> #include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> #include <Library/MemEncryptSevLib.h>
> +#include <Library/MemoryAllocationLib.h>
> #include <Library/PcdLib.h>
> +#include <Library/SafeIntLib.h>
> #include <Library/SmmCpuFeaturesLib.h>
> #include <Library/SmmServicesTableLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> +#include <Pcd/CpuHotEjectData.h>
> #include <PiSmm.h>
> #include <Register/Intel/SmramSaveStateMap.h>
> #include <Register/QemuSmramSaveStateMap.h>
> @@ -171,6 +174,77 @@ SmmCpuFeaturesHookReturnFromSmm (
> return OriginalInstructionPointer;
> }
>
> +STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData = NULL;
> +
> +/**
> + Initialize mCpuHotEjectData if PcdCpuMaxLogicalProcessorNumber > 1.
> +
> + Also setup the corresponding PcdCpuHotEjectDataAddress.
> +**/
> +STATIC
> +VOID
> +InitCpuHotEjectData (
> + VOID
> + )
> +{
> + UINTN Size;
> + UINT32 Idx;
> + UINT32 MaxNumberOfCpus;
> + RETURN_STATUS PcdStatus;
> +
> + MaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
> + if (MaxNumberOfCpus == 1) {
> + return;
> + }
> +
> + //
> + // We allocate CPU_HOT_EJECT_DATA and CPU_HOT_EJECT_DATA->QemuSelectorMap[]
> + // in a single allocation, and explicitly align the QemuSelectorMap[]
> (which
> + // is a UINT64 array) at its natural boundary.
> + // Accordingly, allocate:
> + // sizeof(*mCpuHotEjectData) + (MaxNumberOfCpus * sizeof(UINT64))
> + // and, add sizeof(UINT64) - 1 to use as padding if needed.
> + //
> +
> + if (RETURN_ERROR (SafeUintnMult (MaxNumberOfCpus, sizeof (UINT64), &Size))
> ||
> + RETURN_ERROR (SafeUintnAdd (Size, sizeof (*mCpuHotEjectData), &Size))
> ||
> + RETURN_ERROR (SafeUintnAdd (Size, sizeof (UINT64) - 1, &Size))) {
> + DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_EJECT_DATA\n", __FUNCTION__));
> + goto Fatal;
> + }
> +
> + mCpuHotEjectData = AllocatePool (Size);
> + if (mCpuHotEjectData == NULL) {
> + ASSERT (mCpuHotEjectData != NULL);
> + goto Fatal;
> + }
> +
> + mCpuHotEjectData->Handler = NULL;
> + mCpuHotEjectData->ArrayLength = MaxNumberOfCpus;
> +
> + mCpuHotEjectData->QemuSelectorMap = ALIGN_POINTER (mCpuHotEjectData + 1,
> + sizeof (UINT64));
> + //
> + // We use mCpuHotEjectData->QemuSelectorMap to map
> + // ProcessorNum -> QemuSelector. Initialize to invalid values.
> + //
> + for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) {
> + mCpuHotEjectData->QemuSelectorMap[Idx] = CPU_EJECT_QEMU_SELECTOR_INVALID;
> + }
> +
> + //
> + // Expose address of CPU Hot eject Data structure
> + //
> + PcdStatus = PcdSet64S (PcdCpuHotEjectDataAddress,
> + (UINTN)(VOID *)mCpuHotEjectData);
> + ASSERT_RETURN_ERROR (PcdStatus);
> +
> + return;
> +
> +Fatal:
> + CpuDeadLoop ();
> +}
> +
> /**
> Hook point in normal execution mode that allows the one CPU that was
> elected
> as monarch during System Management Mode initialization to perform
> additional
> @@ -188,6 +262,9 @@ SmmCpuFeaturesSmmRelocationComplete (
> UINTN MapPagesBase;
> UINTN MapPagesCount;
>
> +
> + InitCpuHotEjectData ();
> +
> if (!MemEncryptSevIsEnabled ()) {
> return;
> }
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72865): https://edk2.groups.io/g/devel/message/72865
Mute This Topic: https://groups.io/mt/81273607/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
