On 11/7/23 03:43, Wu, Jiaxin wrote:
> Processor extended information is filled when
> CPU_V2_EXTENDED_TOPOLOGY is set in parameter ProcessorNumber
> from GetProcessorInfo() (See commit: 1fadd18d).
>
> This filed value is retrieved from CPUID leaf 1FH, which is
> a preferred superset to leaf 0BH.
>
> Since Intel recommends first use the CPUID leaf 1FH instead of
> leaf 0BH, this patch change to use the processor extended
> information, which can reflect the value from CPUID leaf 1FH.
>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Star Zeng <star.z...@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin...@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c | 10 ++++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 6 +++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> index 391b64e9f2..c0485b0519 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> @@ -169,10 +169,20 @@ SmmAddProcessor (
> &gSmmCpuPrivate->ProcessorInfo[Index].Location.Package,
> &gSmmCpuPrivate->ProcessorInfo[Index].Location.Core,
> &gSmmCpuPrivate->ProcessorInfo[Index].Location.Thread
> );
>
> + GetProcessorLocation2ByApicId (
> + (UINT32)ProcessorId,
> +
> &gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Package,
> +
> &gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Die,
> +
> &gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Tile,
> +
> &gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Module,
> +
> &gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Core,
> +
> &gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Thread
> + );
> +
> *ProcessorNumber = Index;
> gSmmCpuPrivate->Operation[Index] = SmmCpuAdd;
> return EFI_SUCCESS;
> }
> }
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 25d058c5b9..c61562c867 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -177,11 +177,11 @@ IsPackageFirstThread (
> IN UINTN CpuIndex
> )
> {
> UINT32 PackageIndex;
>
> - PackageIndex = gSmmCpuPrivate->ProcessorInfo[CpuIndex].Location.Package;
> + PackageIndex =
> gSmmCpuPrivate->ProcessorInfo[CpuIndex].ExtendedInformation.Location2.Package;
>
> ASSERT (mPackageFirstThreadIndex != NULL);
>
> //
> // Set the value of mPackageFirstThreadIndex[PackageIndex].
> @@ -1834,12 +1834,12 @@ InitPackageFirstThreadIndexInfo (
>
> //
> // Count the number of package, set to max PackageId + 1
> //
> for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (PackageId < gSmmCpuPrivate->ProcessorInfo[Index].Location.Package) {
> - PackageId = gSmmCpuPrivate->ProcessorInfo[Index].Location.Package;
> + if (PackageId <
> gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Package) {
> + PackageId =
> gSmmCpuPrivate->ProcessorInfo[Index].ExtendedInformation.Location2.Package;
> }
> }
>
> PackageCount = PackageId + 1;
>
The patch looks OK to me, but:
- I would like to test it with CPU hotplug (later, likely under v2), and
- I think this should be two patches.
First, the SmmAddProcessor() function should be extended just to
complete commit 1fadd18d. (BTW I highly appreciate the reference to
commit 1fadd18d; otherwise I couldn't find where the *coldplugged* CPUs'
locations were retrieved!)
Then the Package calculations should be updated separately -- mostly
because I would appreciate a concrete description in that separate
commit message why the difference matters. Clearly you have a use case
where the v1 and v2 package numbers differ, and recording that in the
commit history would be great.
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110872): https://edk2.groups.io/g/devel/message/110872
Mute This Topic: https://groups.io/mt/102436095/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
