Hi Ray and Rahul,
Any comment on this patch ? If you are okay with the approach then can I
get Ack or R-b ?
-Brijesh
On 10/22/21 11:13 PM, Brijesh Singh via groups.io wrote:
> From: Michael Roth <michael.r...@amd.com>
>
> During AP bringup, just after switching to long mode, APs will do some
> cpuid calls to verify that the extended topology leaf (0xB) is available
> so they can fetch their x2 APIC IDs from it. In the case of SEV-ES,
> these cpuid instructions must be handled by direct use of the GHCB MSR
> protocol to fetch the values from the hypervisor, since a #VC handler
> is not yet available due to the AP's stack not being set up yet.
>
> For SEV-SNP, rather than relying on the GHCB MSR protocol, it is
> expected that these values would be obtained from the SEV-SNP CPUID
> table instead. The actual x2 APIC ID (and 8-bit APIC IDs) would still
> be fetched from hypervisor using the GHCB MSR protocol however, so
> introducing support for the SEV-SNP CPUID table in that part of the AP
> bring-up code would only be to handle the checks/validation of the
> extended topology leaf.
>
> Rather than introducing all the added complexity needed to handle these
> checks via the CPUID table, instead let the BSP do the check in advance,
> since it can make use of the #VC handler to avoid the need to scan the
> SNP CPUID table directly, and add a flag in ExchangeInfo to communicate
> the result of this check to APs.
>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Cc: James Bottomley <j...@linux.ibm.com>
> Cc: Min Xu <min.m...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> Cc: Erdem Aktas <erdemak...@google.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Acked-by: Gerd Hoffmann <kra...@redhat.com>
> Suggested-by: Brijesh Singh <brijesh.si...@amd.com>
> Signed-off-by: Michael Roth <michael.r...@amd.com>
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 +
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 11 ++++++++
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
> UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 27 ++++++++++++++++++++
> 4 files changed, 40 insertions(+)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 45bc1de23e3c..c52b6157429b 100644
> --- 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;
>
> #pragma pack()
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index ab838cbc0ff6..d8372ffa9d5a 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -853,6 +853,7 @@ FillExchangeInfoData (
> UINTN Size;
> IA32_SEGMENT_DESCRIPTOR *Selector;
> IA32_CR4 Cr4;
> + UINT32 StdRangeMax;
>
> ExchangeInfo = CpuMpData->MpCpuExchangeInfo;
> ExchangeInfo->StackStart = CpuMpData->Buffer;
> @@ -892,6 +893,16 @@ FillExchangeInfoData (
> ExchangeInfo->SevSnpIsEnabled = CpuMpData->SevSnpIsEnabled;
> ExchangeInfo->GhcbBase = (UINTN) CpuMpData->GhcbBase;
>
> + 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;
> + }
> + }
> +
> //
> // 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
>
> MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd -
> RendezvousFunnelProcStart)
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> index 0034920b2f6b..8bb1161fa0f7 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> @@ -118,6 +118,32 @@ SevEsGetApicId:
> or rax, rdx
> mov rdi, rax ; RDI now holds the original GHCB GPA
>
> + ;
> + ; For SEV-SNP, the recommended handling for getting the x2APIC ID
> + ; would be to use the SNP CPUID table to fetch CPUID.00H:EAX and
> + ; CPUID:0BH:EBX[15:0] instead of the GHCB MSR protocol vmgexits
> + ; below.
> + ;
> + ; To avoid the unecessary ugliness to accomplish that here, the BSP
> + ; has performed these checks in advance (where #VC handler handles
> + ; the CPUID table lookups automatically) and cached them in a flag
> + ; so those checks can be skipped here.
> + ;
> + mov eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevSnpIsEnabled)]
> + cmp al, 1
> + jne CheckExtTopoAvail
> +
> + ;
> + ; Even with SEV-SNP, the actual x2APIC ID in CPUID.0BH:EDX
> + ; fetched from the hypervisor the same way SEV-ES does it.
> + ;
> + mov eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ExtTopoAvail)]
> + cmp al, 1
> + je GetApicIdSevEs
> + ; The 8-bit APIC ID fallback is also the same as with SEV-ES
> + jmp NoX2ApicSevEs
> +
> +CheckExtTopoAvail:
> mov rdx, 0 ; CPUID function 0
> mov rax, 0 ; RAX register requested
> or rax, 4
> @@ -136,6 +162,7 @@ SevEsGetApicId:
> test edx, 0ffffh
> jz NoX2ApicSevEs ; CPUID.0BH:EBX[15:0] is zero
>
> +GetApicIdSevEs:
> mov rdx, 0bh ; CPUID function 0x0b
> mov rax, 0c0000000h ; RDX register requested
> or rax, 4
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82610): https://edk2.groups.io/g/devel/message/82610
Mute This Topic: https://groups.io/mt/86566133/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
