Re: [PATCH v12 27/32] UefiCpuPkg/MpInitLib: use BSP to do extended topology check

["Ni, Ray" <ray.ni@intel.com>]

3 comments:

> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -224,6 +224,7 @@ typedef struct {
>    BOOLEAN               SevEsIsEnabled;
>    BOOLEAN               SevSnpIsEnabled;
>    UINTN                 GhcbBase;
> +  BOOLEAN               ExtTopoAvail;
>  } MP_CPU_EXCHANGE_INFO;

1. Can you please group all SEV-ES fields together?
Those might include followings:
  BOOLEAN               SevEsIsEnabled;
  BOOLEAN               SevSnpIsEnabled;
  UINTN                 GhcbBase;
  BOOLEAN               ExtTopoAvail;


> 
> +  if (ExchangeInfo->SevSnpIsEnabled) {
> +    AsmCpuid (CPUID_SIGNATURE, &StdRangeMax, NULL, NULL, NULL);
> +    if (StdRangeMax >= CPUID_EXTENDED_TOPOLOGY) {
> +      CPUID_EXTENDED_TOPOLOGY_EBX ExtTopoEbx;
> +
> +      AsmCpuid (CPUID_EXTENDED_TOPOLOGY, NULL, &ExtTopoEbx.Uint32, NULL, NULL);
> +      ExchangeInfo->ExtTopoAvail = !!ExtTopoEbx.Bits.LogicalProcessors;
> +    }
> +  }
> +

2. With the grouping, can you please create a routine such as FillExchangeInfoDataSevEs (...) in AmdSev.c?


>    //
>    // Get the BSP's data of GDT and IDT
>    //
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> index 01668638f245..aba53f57201c 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> +++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> @@ -94,6 +94,7 @@ struc MP_CPU_EXCHANGE_INFO
>    .SevEsIsEnabled:               CTYPE_BOOLEAN 1
>    .SevSnpIsEnabled               CTYPE_BOOLEAN 1
>    .GhcbBase:                     CTYPE_UINTN 1
> +  .ExtTopoAvail:                 CTYPE_BOOLEAN 1
>  endstruc

3. can you please do same group in NASM struct?