Date   

Re: [PATCH v3] MdeModulePkg/Core/Dxe: Acquire a lock when iterating gHandleList

Dandan Bi
 

Reviewed-by: Dandan Bi <dandan.bi@intel.com>



Thanks,
Dandan

-----Original Message-----
From: Wang, Jian J <jian.j.wang@intel.com>
Sent: Wednesday, October 13, 2021 4:11 PM
To: Ma, Hua <hua.ma@intel.com>; devel@edk2.groups.io
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Bi, Dandan
<dandan.bi@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v3] MdeModulePkg/Core/Dxe: Acquire a lock when
iterating gHandleList

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

-----Original Message-----
From: Ma, Hua <hua.ma@intel.com>
Sent: Wednesday, October 13, 2021 3:45 PM
To: devel@edk2.groups.io
Cc: Ma, Hua <hua.ma@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Bi, Dandan
<dandan.bi@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v3] MdeModulePkg/Core/Dxe: Acquire a lock when
iterating gHandleList

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3680

This patch fixes the following issue:

The global variable gHandleList is a linked list.
This list is locked when a entry is added or removed from the list,
but there is no lock when iterating this list in function
CoreValidateHandle().
It can lead to "Handle.c (76): CR has Bad Signature" assertion if the
iterated entry in the list is just removed by other task during iterating.

Currently some caller functions of CoreValidateHandle() have
CoreAcquireProtocolLock(), but some caller functions of
CoreValidateHandle() do not CoreAcquireProtocolLock().
Add CoreAcquireProtocolLock() always when CoreValidateHandle() is
called, Also, A lock check is added in the CoreValidateHandle().

v3 changes:
- keep ASSERT_LOCKED(&gProtocolDatabaseLock) in CoreValidateHandle()
- Call CoreAcquireProtocolLock() before any calling of
CoreValidateHandle() and CoreReleaseProtocolLock() afterwards
- Update the commit message

v2 changes:
- Add lock check and comments in CoreGetProtocolInterface() before
calling CoreValidateHandle()
- Update the comments in CoreValidateHandle() header file

v1: https://edk2.groups.io/g/devel/topic/86233569

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hua Ma <hua.ma@intel.com>
---
MdeModulePkg/Core/Dxe/Hand/DriverSupport.c | 16 +++++
MdeModulePkg/Core/Dxe/Hand/Handle.c | 75 ++++++++++++----------
MdeModulePkg/Core/Dxe/Hand/Handle.h | 1 +
MdeModulePkg/Core/Dxe/Hand/Notify.c | 13 ++--
4 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
index feabf12faf..12a202417c 100644
--- a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
+++ b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
@@ -68,7 +68,12 @@ CoreConnectController (
//
// Make sure ControllerHandle is valid
//
+ CoreAcquireProtocolLock ();
+
Status = CoreValidateHandle (ControllerHandle);
+
+ CoreReleaseProtocolLock ();
+
if (EFI_ERROR (Status)) {
return Status;
}
@@ -268,7 +273,12 @@ AddSortedDriverBindingProtocol (
//
// Make sure the DriverBindingHandle is valid
//
+ CoreAcquireProtocolLock ();
+
Status = CoreValidateHandle (DriverBindingHandle);
+
+ CoreReleaseProtocolLock ();
+
if (EFI_ERROR (Status)) {
return;
}
@@ -746,8 +756,11 @@ CoreDisconnectController (
//
// Make sure ControllerHandle is valid
//
+ CoreAcquireProtocolLock ();
+
Status = CoreValidateHandle (ControllerHandle);
if (EFI_ERROR (Status)) {
+ CoreReleaseProtocolLock ();
return Status;
}

@@ -757,10 +770,13 @@ CoreDisconnectController (
if (ChildHandle != NULL) {
Status = CoreValidateHandle (ChildHandle);
if (EFI_ERROR (Status)) {
+ CoreReleaseProtocolLock ();
return Status;
}
}

+ CoreReleaseProtocolLock ();
+
Handle = ControllerHandle;

//
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c
b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 6eccb41ecb..92979281b7 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -53,6 +53,7 @@ CoreReleaseProtocolLock (

/**
Check whether a handle is a valid EFI_HANDLE
+ The gProtocolDatabaseLock must be owned

@param UserHandle The handle to check

@@ -72,6 +73,8 @@ CoreValidateHandle (
return EFI_INVALID_PARAMETER;
}

+ ASSERT_LOCKED(&gProtocolDatabaseLock);
+
for (Link = gHandleList.BackLink; Link != &gHandleList; Link = Link->BackLink)
{
Handle = CR (Link, IHANDLE, AllHandles, EFI_HANDLE_SIGNATURE);
if (Handle == (IHANDLE *) UserHandle) { @@ -720,19 +723,19 @@
CoreUninstallProtocolInterface (
return EFI_INVALID_PARAMETER;
}

+ //
+ // Lock the protocol database
+ //
+ CoreAcquireProtocolLock ();
+
//
// Check that UserHandle is a valid handle
//
Status = CoreValidateHandle (UserHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}

- //
- // Lock the protocol database
- //
- CoreAcquireProtocolLock ();
-
//
// Check that Protocol exists on UserHandle, and Interface matches
the interface in the database
//
@@ -1010,12 +1013,17 @@ CoreOpenProtocol (
return EFI_INVALID_PARAMETER;
}

+ //
+ // Lock the protocol database
+ //
+ CoreAcquireProtocolLock ();
+
//
// Check for invalid UserHandle
//
Status = CoreValidateHandle (UserHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}

//
@@ -1025,31 +1033,32 @@ CoreOpenProtocol (
case EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER :
Status = CoreValidateHandle (ImageHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
Status = CoreValidateHandle (ControllerHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
if (UserHandle == ControllerHandle) {
- return EFI_INVALID_PARAMETER;
+ Status = EFI_INVALID_PARAMETER;
+ goto Done;
}
break;
case EFI_OPEN_PROTOCOL_BY_DRIVER :
case EFI_OPEN_PROTOCOL_BY_DRIVER |
EFI_OPEN_PROTOCOL_EXCLUSIVE :
Status = CoreValidateHandle (ImageHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
Status = CoreValidateHandle (ControllerHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
break;
case EFI_OPEN_PROTOCOL_EXCLUSIVE :
Status = CoreValidateHandle (ImageHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
break;
case EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL :
@@ -1057,13 +1066,10 @@ CoreOpenProtocol (
case EFI_OPEN_PROTOCOL_TEST_PROTOCOL :
break;
default:
- return EFI_INVALID_PARAMETER;
+ Status = EFI_INVALID_PARAMETER;
+ goto Done;
}

- //
- // Lock the protocol database
- //
- CoreAcquireProtocolLock ();

//
// Look at each protocol interface for a match @@ -1246,32 +1252,33
@@ CoreCloseProtocol (
LIST_ENTRY *Link;
OPEN_PROTOCOL_DATA *OpenData;

+ //
+ // Lock the protocol database
+ //
+ CoreAcquireProtocolLock ();
+
//
// Check for invalid parameters
//
Status = CoreValidateHandle (UserHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
Status = CoreValidateHandle (AgentHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
if (ControllerHandle != NULL) {
Status = CoreValidateHandle (ControllerHandle);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Done;
}
}
if (Protocol == NULL) {
- return EFI_INVALID_PARAMETER;
+ Status = EFI_INVALID_PARAMETER;
+ goto Done;
}

- //
- // Lock the protocol database
- //
- CoreAcquireProtocolLock ();
-
//
// Look at each protocol interface for a match
//
@@ -1443,13 +1450,6 @@ CoreProtocolsPerHandle (
UINTN ProtocolCount;
EFI_GUID **Buffer;

- Status = CoreValidateHandle (UserHandle);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Handle = (IHANDLE *)UserHandle;
-
if (ProtocolBuffer == NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -1464,6 +1464,13 @@ CoreProtocolsPerHandle (

CoreAcquireProtocolLock ();

+ Status = CoreValidateHandle (UserHandle); if (EFI_ERROR (Status))
+ {
+ goto Done;
+ }
+
+ Handle = (IHANDLE *)UserHandle;
+
for (Link = Handle->Protocols.ForwardLink; Link !=
&Handle->Protocols; Link =
Link->ForwardLink) {
ProtocolCount++;
}
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.h
b/MdeModulePkg/Core/Dxe/Hand/Handle.h
index 83eb2b9f3a..3f83e3af15 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.h
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.h
@@ -242,6 +242,7 @@ CoreReleaseProtocolLock (

/**
Check whether a handle is a valid EFI_HANDLE
+ The gProtocolDatabaseLock must be owned

@param UserHandle The handle to check

diff --git a/MdeModulePkg/Core/Dxe/Hand/Notify.c
b/MdeModulePkg/Core/Dxe/Hand/Notify.c
index 553413a350..d05f95207f 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Notify.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Notify.c
@@ -188,22 +188,21 @@ CoreReinstallProtocolInterface (
PROTOCOL_INTERFACE *Prot;
PROTOCOL_ENTRY *ProtEntry;

- Status = CoreValidateHandle (UserHandle);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
if (Protocol == NULL) {
return EFI_INVALID_PARAMETER;
}

- Handle = (IHANDLE *) UserHandle;
-
//
// Lock the protocol database
//
CoreAcquireProtocolLock ();

+ Status = CoreValidateHandle (UserHandle); if (EFI_ERROR (Status))
+ {
+ goto Done;
+ }
+
+ Handle = (IHANDLE *) UserHandle;
//
// Check that Protocol exists on UserHandle, and Interface matches
the interface in the database
//
--
2.32.0.windows.2


Re: [PATCH V2 27/28] OvmfPkg: Update IoMmuDxe to support TDX

Min Xu
 

On October 12, 2021 8:16 PM, Gerd Hoffmann wrote:
Hi,

+#define IO_MMU_LEGACY 0x0
+#define IO_MMU_SEV 0x01
+#define IO_MMU_TDX 0x02
+
+UINTN mIoMmuType = IO_MMU_LEGACY;
Yet another place where you should be able to just use the
ConfidentialComputing PCD.
Thanks for reminder. It will be updated in next version.

Thanks
Min


Re: [PATCH V9 2/4] OvmfPkg: Clear WORK_AREA_GUEST_TYPE in Main.asm

Min Xu
 

On October 13, 2021 11:46 PM, Brijesh Singh wrote:
On 10/12/21 5:58 PM, Xu, Min M wrote:
On October 12, 2021 9:23 PM, Lendacky Thomas wrote:
Good point Tom. The WORK_AREA_GUEST_TYPE define should be moved
outside the ARCH_X86. I missed it mainly because we renamed the
ESWorkArea to Generic workarea but EsWorkArea was defined in ARCH_X86
only.

Min,

I can send a patch to move define outside the ARCH_X64 and then you can
align the TDX code accordingly.
Ok. I will update the TDX code based on your new patch.

Thanks.
Min


Re: [PATCH V2 14/28] OvmfPkg: Update SecEntry.nasm to support Tdx

Min Xu
 

On October 12, 2021 6:39 PM, Gerd Hoffmann wrote:
Hi,

- AcceptPages:
To mitigate the performance impact of accepting pages in SEC phase on
BSP, BSP will parse memory resources and assign each AP the task of
accepting a subset of pages. This command may be called several times
until all memory resources are processed. In accepting pages, PageLevel
may fall back to smaller one if SIZE_MISMATCH error is returned.
Hmm, I'm wondering whenever it is useful to have this in the first place with
the longer-term plan to implement lazy on-demand acceptance.
To mitigate the performance impact of accepting pages, there are 3 ways.
1. Big accept page size.
2. Accept the pages by BSP and APs together.
3. Lazy on-demand acceptance.
From the perspective of implementation complexity, 1 < 2 < 3.
Lazy on-demand acceptance need the update not only in TDVF, but also in Guest kernel. More investigation shows it also impacts the grub and memory management in EDK2.
From the perspective of performance improvement, 2M accept page size + BSP/AP together, can improve the performance a lot.
Based on above consideration, in current TDVF upstream, 1 + 2 are the ways to mitigate the performance impact.
Upstream of 3 is in the future plan.

Thanks!
Min


Re: [PATCH V2 13/28] UefiCpuPkg: Enable Tdx support in MpInitLib

Min Xu
 

On October 12, 2021 6:32 PM, Gerd Hoffman wrote:
Hi,

+ do {
+ AsmCpuid (0, &LargestEax, &Ebx, &Ecx, &Edx);
Again: this should use PCD.
ConfidentialComputing PCD is set in PlatformPei. So any check of this PCD should be after PlatformPei.
MpInitLib will be included in CpuMpPei. So if PCD is checked in MpInitLib, then we must assume CpuMpPei is called after PlatformPei.
In current OVMF, CpuMpPei is called after PlatforPei. But I am not sure if it is always the case. Can we have such assumption?
Based on above consideration, CPUID is used to probe if it is Tdx guest.

take care,
Gerd





Re: [PATCH v2 1/1] MdeModulePkg/Sd: Corrections for Extra.uni files

Wu, Hao A
 

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A
Sent: Tuesday, October 12, 2021 7:42 AM
To: Konstantin Aladyshev <aladyshev22@gmail.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; gaoliming@byosoft.com.cn
Subject: Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg/Sd: Corrections for
Extra.uni files

-----Original Message-----
From: Konstantin Aladyshev <aladyshev22@gmail.com>
Sent: Tuesday, October 12, 2021 1:01 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; gaoliming@byosoft.com.cn; Wu,
Hao A <hao.a.wu@intel.com>; Konstantin Aladyshev
<aladyshev22@gmail.com>
Subject: [PATCH v2 1/1] MdeModulePkg/Sd: Corrections for Extra.uni files

Add correct content to the 'SdDxeExtra.uni' file.
Include 'EmmcDxeExtra.uni' and 'SdDxeExtra.uni' files to their appropriate INF
files.

Thanks.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Will wait a couple of days before merging to see if any comment from others.

Pushed via:
PR - https://github.com/tianocore/edk2/pull/2060
Commit - https://github.com/tianocore/edk2/commit/43b38408732b6c11641300689e09df1ad7bdcc6a

Best Regards,
Hao Wu



Best Regards,
Hao Wu



Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf | 2 ++
MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf | 2 ++
MdeModulePkg/Bus/Sd/SdDxe/SdDxeExtra.uni | 9 ++++-----
3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
index 2afe4f42a985..ec7da2794b30 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
@@ -63,3 +63,5 @@ [Protocols]
## BY_START gEfiDevicePathProtocolGuid
+[UserExtensions.TianoCore."ExtraFiles"]+ EmmcDxeExtra.unidiff --git
a/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
b/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
index 69b856b665ea..d620e830046e 100644
--- a/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+++ b/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
@@ -62,3 +62,5 @@ [Protocols]
## BY_START gEfiDevicePathProtocolGuid
+[UserExtensions.TianoCore."ExtraFiles"]+ SdDxeExtra.unidiff --git
a/MdeModulePkg/Bus/Sd/SdDxe/SdDxeExtra.uni
b/MdeModulePkg/Bus/Sd/SdDxe/SdDxeExtra.uni
index eb8ebd5a3594..4651ac42036d 100644
--- a/MdeModulePkg/Bus/Sd/SdDxe/SdDxeExtra.uni
+++ b/MdeModulePkg/Bus/Sd/SdDxe/SdDxeExtra.uni
@@ -1,6 +1,5 @@
// /** @file-// SD memory card device driver to manage the SD memory card
device and provide interface for upper layer-// access.+// SdDxe Localized
Strings and Content // // Copyright (c) 2015, Intel Corporation. All rights
reserved.<BR> //@@ -8,8 +7,8 @@
// // **/ +#string STR_PROPERTIES_MODULE_NAME+#language en-US+"SD
Device Driver" -#string STR_MODULE_ABSTRACT #language en-US "SD
device driver to manage the SD memory card device and provide interface for
upper layer access"--#string STR_MODULE_DESCRIPTION #language en-
US
"This driver follows the UEFI driver model and layers on the SdMmcPassThru
protocol. It installs BlockIo and BlockIo2 protocols on the SD device." --
2.25.1




Re: Error when launching SEV-ES guest with OvmfPkg/AmdSev build

Dov Murik
 

Thanks Brijesh for looking into this.

On 13/10/2021 22:41, Brijesh Singh wrote:
Hi Dov,

On 10/13/21 2:35 AM, Dov Murik wrote:
Hello,

I encountered the following problem when trying to launch SEV-ES
(policy=0x5) guests with the OvmfPkg/AmdSev/AmdSevX64 package build:


$ sudo /home/dmurik/git/qemu/build/qemu-system-x86_64 -enable-kvm
-machine q35 -smp 1 -m 2G -machine confidential-guest-support=sev0
-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x5
-drive
if=pflash,format=raw,unit=0,file=/home/dmurik/git/edk2/Build/AmdSev/DEBUG_GCC5/FV/OVMF.fd,readonly=on
-nographic -global isa-debugcon.iobase=0x402 -debugcon file:ovmf-1.log
-monitor pty

char device redirected to /dev/pts/6 (label compat_monitor0)
error: kvm run failed Invalid argument
EAX=0000000a EBX=0000006f ECX=00000000 EDX=00000000
ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0000 00000000 00000000 00000000
CS =0000 00000000 00000000 00000000
SS =0000 00000000 00000000 00000000
DS =0000 00000000 00000000 00000000
FS =0000 00000000 00000000 00000000
GS =0000 00000000 00000000 00000000
LDT=0000 00000000 00000000 00000000
TR =0000 00000000 00000000 00000000
GDT= 00000000 00000000
IDT= 00000000 00000000
CR0=c0000033 CR2=00000000 CR3=00000000 CR4=00000660
DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000
DR3=0000000000000000
DR6=00000000ffff0ff0 DR7=0000000000000400
EFER=0000000000000100
Code=?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? <??> ??
?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??
?? ?? ?? ??


ovmf-1.log is empty (even though OVMF is compiled with debug flags).


Plain SEV (no -ES) guests work OK.


The error is "kvm run failed Invalid argument", so I first tried
switching kernels, but 5.11.0, 5.13.0, and 5.14.0 all gave the same result.

Then I tried an older OVMF release (edk2-stable202108) -- and it worked
OK. So I started a git bisect session and found this first bad commit:


commit ab77b6031b03733c28fa5f477d802fd67b3f3ee0
Author: Brijesh Singh <brijesh.singh@amd.com>
Date: Tue Aug 17 21:46:50 2021 +0800

OvmfPkg/ResetVector: update SEV support to use new work area format

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3429&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C161014bce6d140ebb89408d98e2cce3a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637697145350831431%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=26MCaioHLlTtw81C%2F3Mpf8kOG2LpQXFmvt3FvH%2FnjNk%3D&amp;reserved=0

Update the SEV support to switch to using the newer work area format.


I wonder if any change in this series should have also touched files in
OvmfPkg/AmdSev and missed them.
This is most likely because the patch repurposed the EsWorkArea to a
generic workarea but the AmdSevX64.fdf is still pointing the EsWorkArea
to be the start of the page. After we repurposed the EsWorkArea it got
assigned to a different value. See the OvmfPkgX64.fdf. I am having
trouble building the AmdSev package. I am getting this error

grub-mkimage: error: cannot open
`/usr/lib/grub/x86_64-efi/sevsecret.mod': No such file or directory.

Do you know what I am missing ? It seems like we have some dependency
with the grub but I cannot seem to find out what I need to do to get
those resolved. Any hint ?
A simple way to skip the grub embedding is to run

touch OvmfPkg/AmdSev/Grub/grub.efi

before running the build.

This will cause grub.sh to use the already existing (empty) grub.efi as
the embedded grub module; which is fine for now because I can't even
reach a single log line in OVMF.

-Dov

Any other ideas on how to debug this are welcome.

Let me know if this should be reported/discussed somewhere else.


Thanks,
-Dov


Re: Error when launching SEV-ES guest with OvmfPkg/AmdSev build

Brijesh Singh
 

Hi Dov,

On 10/13/21 2:35 AM, Dov Murik wrote:
Hello,

I encountered the following problem when trying to launch SEV-ES
(policy=0x5) guests with the OvmfPkg/AmdSev/AmdSevX64 package build:


$ sudo /home/dmurik/git/qemu/build/qemu-system-x86_64 -enable-kvm
-machine q35 -smp 1 -m 2G -machine confidential-guest-support=sev0
-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x5
-drive
if=pflash,format=raw,unit=0,file=/home/dmurik/git/edk2/Build/AmdSev/DEBUG_GCC5/FV/OVMF.fd,readonly=on
-nographic -global isa-debugcon.iobase=0x402 -debugcon file:ovmf-1.log
-monitor pty

char device redirected to /dev/pts/6 (label compat_monitor0)
error: kvm run failed Invalid argument
EAX=0000000a EBX=0000006f ECX=00000000 EDX=00000000
ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0000 00000000 00000000 00000000
CS =0000 00000000 00000000 00000000
SS =0000 00000000 00000000 00000000
DS =0000 00000000 00000000 00000000
FS =0000 00000000 00000000 00000000
GS =0000 00000000 00000000 00000000
LDT=0000 00000000 00000000 00000000
TR =0000 00000000 00000000 00000000
GDT= 00000000 00000000
IDT= 00000000 00000000
CR0=c0000033 CR2=00000000 CR3=00000000 CR4=00000660
DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000
DR3=0000000000000000
DR6=00000000ffff0ff0 DR7=0000000000000400
EFER=0000000000000100
Code=?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? <??> ??
?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??
?? ?? ?? ??


ovmf-1.log is empty (even though OVMF is compiled with debug flags).


Plain SEV (no -ES) guests work OK.


The error is "kvm run failed Invalid argument", so I first tried
switching kernels, but 5.11.0, 5.13.0, and 5.14.0 all gave the same result.

Then I tried an older OVMF release (edk2-stable202108) -- and it worked
OK. So I started a git bisect session and found this first bad commit:


commit ab77b6031b03733c28fa5f477d802fd67b3f3ee0
Author: Brijesh Singh <brijesh.singh@amd.com>
Date: Tue Aug 17 21:46:50 2021 +0800

OvmfPkg/ResetVector: update SEV support to use new work area format

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3429&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C161014bce6d140ebb89408d98e2cce3a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637697145350831431%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=26MCaioHLlTtw81C%2F3Mpf8kOG2LpQXFmvt3FvH%2FnjNk%3D&amp;reserved=0

Update the SEV support to switch to using the newer work area format.


I wonder if any change in this series should have also touched files in
OvmfPkg/AmdSev and missed them.
This is most likely because the patch repurposed the EsWorkArea to a
generic workarea but the AmdSevX64.fdf is still pointing the EsWorkArea
to be the start of the page. After we repurposed the EsWorkArea it got
assigned to a different value. See the OvmfPkgX64.fdf. I am having
trouble building the AmdSev package. I am getting this error

grub-mkimage: error: cannot open
`/usr/lib/grub/x86_64-efi/sevsecret.mod': No such file or directory.

Do you know what I am missing ? It seems like we have some dependency
with the grub but I cannot seem to find out what I need to do to get
those resolved. Any hint ?

Any other ideas on how to debug this are welcome.

Let me know if this should be reported/discussed somewhere else.


Thanks,
-Dov


Re: [PATCH v2 1/1] ArmPkg: Implement PlatformBootManagerLib for LinuxBoot

Samer El-Haj-Mahmoud
 

Ackd-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

Any update on getting this reviewed/merged? We have downstream platforms that depend on this and would like to avoid duplication of similar functionality in platform code.

Thanks!
--Samer

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
Biesheuvel via groups.io
Sent: Wednesday, September 22, 2021 7:49 AM
To: Nhi Pham <nhi@os.amperecomputing.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>;
patches@amperecomputing.com; Leif Lindholm <leif@nuviainc.com>; Ard
Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [edk2-devel] [PATCH v2 1/1] ArmPkg: Implement
PlatformBootManagerLib for LinuxBoot

On Tue, 7 Sept 2021 at 05:40, Nhi Pham <nhi@os.amperecomputing.com>
wrote:

LinuxBoot is a firmware that replaces specific firmware functionality
like the UEFI DXE phase with a Linux kernel and runtime. It is built-in
UEFI image like an application, which is executed at the end of DXE
phase.

To achieve the LinuxBoot boot flow "SEC->PEI->DXE->BDS->LinuxBoot",
today we use the common well-known GUID of UEFI Shell for LinuxBoot
payload, so LinuxBoot developers can effortlessly find the UEFI Shell
Application and replace it with the LinuxBoot payload without
recompiling platform EDK2 (There might be an issue with a few systems
that don't have a UEFI Shell). Also, we have a hard requirement to force
the BDS to boot into the LinuxBoot as it is essentially required that
only the LinuxBoot boot option is permissible and UEFI is an
intermediate bootstrap phase. Considering all the above, it is
reasonable to just have a new GUID for LinuxBoot and require a LinuxBoot
specific BDS implementation. In addition, with making the BDS
implementation simpler, we can reduce many DXE drivers which we think it
is not necessary for LinuxBoot booting.

This patch adds a new PlatformBootManagerLib implementation which
registers only the gArmTokenSpaceGuid.PcdLinuxBootFileGuid for
LinuxBoot
payload as an active boot option. It allows BDS to jump to the LinuxBoot
quickly by skipping the UiApp and UEFI Shell.

The PlatformBootManagerLib library derived from
ArmPkg/Library/PlatformBootManagerLib.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>

Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>

---
ArmPkg/ArmPkg.dec | 8 +
ArmPkg/ArmPkg.dsc | 2 +
ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
| 58 +++++++
ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c | 178
++++++++++++++++++++
4 files changed, 246 insertions(+)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 214b2f589217..f68e6ee00860 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -3,6 +3,7 @@
#
# Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.<BR>
# Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
+# Copyright (c) 2021, Ampere Computing LLC. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -382,3 +383,10 @@ [PcdsFixedAtBuild.common,
PcdsDynamic.common]
#
gArmTokenSpaceGuid.PcdPciBusMin|0x0|UINT32|0x00000059
gArmTokenSpaceGuid.PcdPciBusMax|0x0|UINT32|0x0000005A
+
+[PcdsDynamicEx]
+ #
+ # This dynamic PCD hold the GUID of a firmware FFS which contains
+ # the LinuxBoot payload.
+ #
+ gArmTokenSpaceGuid.PcdLinuxBootFileGuid|{0x0}|VOID*|0x0000005C
diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 926986cf7fbb..ffb1c261861e 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -5,6 +5,7 @@
# Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
# Copyright (c) Microsoft Corporation.<BR>
+# Copyright (c) 2021, Ampere Computing LLC. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -150,6 +151,7 @@ [Components.common]
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+
ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf

ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
diff --git
a/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
new file mode 100644
index 000000000000..139b6171990a
--- /dev/null
+++
b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
@@ -0,0 +1,58 @@
+## @file
+# Implementation for PlatformBootManagerLib library class interfaces.
+#
+# Copyright (C) 2015-2016, Red Hat, Inc.
+# Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
+# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
+# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights
reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = LinuxBootBootManagerLib
+ FILE_GUID = 1FA91547-DB23-4F6A-8AF8-3B9782A7F917
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PlatformBootManagerLib|DXE_DRIVER
+
+#
+# The following information is for reference only and not required by the
build tools.
+#
+# VALID_ARCHITECTURES = ARM AARCH64
+#
+
+[Sources]
+ LinuxBootBm.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ ShellPkg/ShellPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ PcdLib
+ PrintLib
+ UefiBootManagerLib
+ UefiBootServicesTableLib
+ UefiLib
+ UefiRuntimeServicesTableLib
+
+[Pcd]
+ gArmTokenSpaceGuid.PcdLinuxBootFileGuid
+
+[Guids]
+ gEfiEndOfDxeEventGroupGuid
+ gUefiShellFileGuid
+ gZeroGuid
+
+[Protocols]
+ gEfiLoadedImageProtocolGuid
diff --git a/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c
b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c
new file mode 100644
index 000000000000..f4941780efcd
--- /dev/null
+++ b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c
@@ -0,0 +1,178 @@
+/** @file
+ Implementation for PlatformBootManagerLib library class interfaces.
+
+ Copyright (C) 2015-2016, Red Hat, Inc.
+ Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights
reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+
+#include <Guid/EventGroup.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootManagerLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/LoadedImage.h>
+#include <Protocol/PlatformBootManager.h>
+
+STATIC
+VOID
+PlatformRegisterFvBootOption (
+ CONST EFI_GUID *FileGuid,
+ CHAR16 *Description,
+ UINT32 Attributes
+ )
+{
+ EFI_STATUS Status;
+ INTN OptionIndex;
+ EFI_BOOT_MANAGER_LOAD_OPTION NewOption;
+ EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
+ UINTN BootOptionCount;
+ MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;
+ EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+
+ Status = gBS->HandleProtocol (
+ gImageHandle,
+ &gEfiLoadedImageProtocolGuid,
+ (VOID **)&LoadedImage
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
+ DevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
+ ASSERT (DevicePath != NULL);
+ DevicePath = AppendDevicePathNode (
+ DevicePath,
+ (EFI_DEVICE_PATH_PROTOCOL *)&FileNode
+ );
+ ASSERT (DevicePath != NULL);
+
+ Status = EfiBootManagerInitializeLoadOption (
+ &NewOption,
+ LoadOptionNumberUnassigned,
+ LoadOptionTypeBoot,
+ Attributes,
+ Description,
+ DevicePath,
+ NULL,
+ 0
+ );
+ ASSERT_EFI_ERROR (Status);
+ FreePool (DevicePath);
+
+ BootOptions = EfiBootManagerGetLoadOptions (
+ &BootOptionCount,
+ LoadOptionTypeBoot
+ );
+
+ OptionIndex = EfiBootManagerFindLoadOption (
+ &NewOption,
+ BootOptions,
+ BootOptionCount
+ );
+
+ if (OptionIndex == -1) {
+ Status = EfiBootManagerAddLoadOptionVariable (&NewOption,
MAX_UINTN);
+ ASSERT_EFI_ERROR (Status);
+ }
+ EfiBootManagerFreeLoadOption (&NewOption);
+ EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
+}
+
+/**
+ Do the platform specific action before the console is connected.
+
+ Such as:
+ Update console variable;
+ Register new Driver#### or Boot####;
+ Signal ReadyToLock event.
+**/
+VOID
+EFIAPI
+PlatformBootManagerBeforeConsole (
+ VOID
+ )
+{
+ //
+ // Signal EndOfDxe PI Event
+ //
+ EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
+}
+
+/**
+ Do the platform specific action after the console is connected.
+
+ Such as:
+ Dynamically switch output mode;
+ Signal console ready platform customized event;
+ Run diagnostics like memory testing;
+ Connect certain devices;
+ Dispatch additional option roms.
+**/
+VOID
+EFIAPI
+PlatformBootManagerAfterConsole (
+ VOID
+ )
+{
+ EFI_GUID LinuxBootFileGuid;
+
+ CopyGuid (&LinuxBootFileGuid, PcdGetPtr (PcdLinuxBootFileGuid));
+
+ if (!CompareGuid (&LinuxBootFileGuid, &gZeroGuid)) {
+ //
+ // Register LinuxBoot
+ //
+ PlatformRegisterFvBootOption (
+ &LinuxBootFileGuid,
+ L"LinuxBoot",
+ LOAD_OPTION_ACTIVE
+ );
+ } else {
+ DEBUG ((DEBUG_ERROR, "%a: PcdLinuxBootFileGuid was not set!\n",
__FUNCTION__));
+ }
+}
+
+/**
+ This function is called each second during the boot manager waits the
+ timeout.
+
+ @param TimeoutRemain The remaining timeout.
+**/
+VOID
+EFIAPI
+PlatformBootManagerWaitCallback (
+ UINT16 TimeoutRemain
+ )
+{
+ return;
+}
+
+/**
+ The function is called when no boot option could be launched,
+ including platform recovery options and options pointing to applications
+ built into firmware volumes.
+
+ If this function returns, BDS attempts to enter an infinite loop.
+**/
+VOID
+EFIAPI
+PlatformBootManagerUnableToBoot (
+ VOID
+ )
+{
+ return;
+}
--
2.17.1


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[PATCH v2 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib

Bret Barkelew
 

Used to provision and maintain certain HW-defined NV spaces.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2994

Signed-off-by: Bret Barkelew <bret.barkelew@microsoft.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c | 122 +++++++++++++++++=
+++
SecurityPkg/Include/Library/Tpm2CommandLib.h | 22 ++++
2 files changed, 144 insertions(+)

diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c b/SecurityP=
kg/Library/Tpm2CommandLib/Tpm2NVStorage.c
index 87572de20164..275cb1683f51 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c
@@ -24,6 +24,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define RC_NV_UndefineSpace_authHandle (TPM_RC_H + TPM_RC_1)=0D
#define RC_NV_UndefineSpace_nvIndex (TPM_RC_H + TPM_RC_2)=0D
=0D
+#define RC_NV_UndefineSpaceSpecial_nvIndex (TPM_RC_H + TPM_RC_1)=0D
+=0D
#define RC_NV_Read_authHandle (TPM_RC_H + TPM_RC_1)=0D
#define RC_NV_Read_nvIndex (TPM_RC_H + TPM_RC_2)=0D
#define RC_NV_Read_size (TPM_RC_P + TPM_RC_1)=0D
@@ -74,6 +76,20 @@ typedef struct {
TPMS_AUTH_RESPONSE AuthSession;=0D
} TPM2_NV_UNDEFINESPACE_RESPONSE;=0D
=0D
+typedef struct {=0D
+ TPM2_COMMAND_HEADER Header;=0D
+ TPMI_RH_NV_INDEX NvIndex;=0D
+ TPMI_RH_PLATFORM Platform;=0D
+ UINT32 AuthSessionSize;=0D
+ TPMS_AUTH_COMMAND AuthSession;=0D
+} TPM2_NV_UNDEFINESPACESPECIAL_COMMAND;=0D
+=0D
+typedef struct {=0D
+ TPM2_RESPONSE_HEADER Header;=0D
+ UINT32 AuthSessionSize;=0D
+ TPMS_AUTH_RESPONSE AuthSession;=0D
+} TPM2_NV_UNDEFINESPACESPECIAL_RESPONSE;=0D
+=0D
typedef struct {=0D
TPM2_COMMAND_HEADER Header;=0D
TPMI_RH_NV_AUTH AuthHandle;=0D
@@ -506,6 +522,112 @@ Done:
return Status;=0D
}=0D
=0D
+/**=0D
+ This command allows removal of a platform-created NV Index that has TPMA=
_NV_POLICY_DELETE SET.=0D
+=0D
+ @param[in] NvIndex The NV Index.=0D
+ @param[in] IndexAuthSession Auth session context for the Index auth/=
policy=0D
+ @param[in] PlatAuthSession Auth session context for the Platform au=
th/policy=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_NOT_FOUND The command was returned successfully, b=
ut NvIndex is not found.=0D
+ @retval EFI_UNSUPPORTED Selected NvIndex does not support deleti=
on through this call.=0D
+ @retval EFI_SECURITY_VIOLATION Deletion is not authorized by current po=
licy session.=0D
+ @retval EFI_INVALID_PARAMETER The command was unsuccessful.=0D
+ @retval EFI_DEVICE_ERROR The command was unsuccessful.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+Tpm2NvUndefineSpaceSpecial (=0D
+ IN TPMI_RH_NV_INDEX NvIndex,=0D
+ IN TPMS_AUTH_COMMAND *IndexAuthSession OPTIONAL,=0D
+ IN TPMS_AUTH_COMMAND *PlatAuthSession OPTIONAL=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ TPM2_NV_UNDEFINESPACESPECIAL_COMMAND SendBuffer;=0D
+ TPM2_NV_UNDEFINESPACESPECIAL_RESPONSE RecvBuffer;=0D
+ UINT32 SendBufferSize;=0D
+ UINT32 RecvBufferSize;=0D
+ UINT8 *Buffer;=0D
+ UINT32 IndexAuthSize, PlatAuthSize;=0D
+ TPM_RC ResponseCode;=0D
+=0D
+ //=0D
+ // Construct command=0D
+ //=0D
+ SendBuffer.Header.tag =3D SwapBytes16(TPM_ST_SESSIONS);=0D
+ SendBuffer.Header.commandCode =3D SwapBytes32(TPM_CC_NV_UndefineSpaceSpe=
cial);=0D
+=0D
+ SendBuffer.NvIndex =3D SwapBytes32 (NvIndex);=0D
+ SendBuffer.Platform =3D SwapBytes32 (TPM_RH_PLATFORM);=0D
+=0D
+ //=0D
+ // Marshall the Auth Sessions for the two handles.=0D
+ Buffer =3D (UINT8 *)&SendBuffer.AuthSession;=0D
+ // IndexAuthSession=0D
+ IndexAuthSize =3D CopyAuthSessionCommand (IndexAuthSession, Buffer);=0D
+ Buffer +=3D IndexAuthSize;=0D
+ // PlatAuthSession=0D
+ PlatAuthSize =3D CopyAuthSessionCommand (PlatAuthSession, Buffer);=0D
+ Buffer +=3D PlatAuthSize;=0D
+ // AuthSessionSize=0D
+ SendBuffer.AuthSessionSize =3D SwapBytes32(IndexAuthSize + PlatAuthSize)=
;=0D
+=0D
+ // Update total command size.=0D
+ SendBufferSize =3D (UINT32)(Buffer - (UINT8 *)&SendBuffer);=0D
+ SendBuffer.Header.paramSize =3D SwapBytes32 (SendBufferSize);=0D
+=0D
+ //=0D
+ // send Tpm command=0D
+ //=0D
+ RecvBufferSize =3D sizeof (RecvBuffer);=0D
+ Status =3D Tpm2SubmitCommand (SendBufferSize, (UINT8 *)&SendBuffer, &Rec=
vBufferSize, (UINT8 *)&RecvBuffer);=0D
+ if (EFI_ERROR (Status)) {=0D
+ goto Done;=0D
+ }=0D
+=0D
+ if (RecvBufferSize < sizeof (TPM2_RESPONSE_HEADER)) {=0D
+ DEBUG ((EFI_D_ERROR, "Tpm2NvUndefineSpaceSpecial - RecvBufferSize Erro=
r - %x\n", RecvBufferSize));=0D
+ Status =3D EFI_DEVICE_ERROR;=0D
+ goto Done;=0D
+ }=0D
+=0D
+ ResponseCode =3D SwapBytes32(RecvBuffer.Header.responseCode);=0D
+ if (ResponseCode !=3D TPM_RC_SUCCESS) {=0D
+ DEBUG ((EFI_D_ERROR, "Tpm2NvUndefineSpaceSpecial - responseCode - %x\n=
", SwapBytes32(RecvBuffer.Header.responseCode)));=0D
+ }=0D
+ switch (ResponseCode) {=0D
+ case TPM_RC_SUCCESS:=0D
+ // return data=0D
+ break;=0D
+ case TPM_RC_ATTRIBUTES:=0D
+ case TPM_RC_ATTRIBUTES + RC_NV_UndefineSpaceSpecial_nvIndex:=0D
+ Status =3D EFI_UNSUPPORTED;=0D
+ break;=0D
+ case TPM_RC_NV_AUTHORIZATION:=0D
+ Status =3D EFI_SECURITY_VIOLATION;=0D
+ break;=0D
+ case TPM_RC_HANDLE + RC_NV_UndefineSpaceSpecial_nvIndex: // TPM_RC_NV_DE=
FINED:=0D
+ Status =3D EFI_NOT_FOUND;=0D
+ break;=0D
+ case TPM_RC_VALUE + RC_NV_UndefineSpace_nvIndex:=0D
+ Status =3D EFI_INVALID_PARAMETER;=0D
+ break;=0D
+ default:=0D
+ Status =3D EFI_DEVICE_ERROR;=0D
+ break;=0D
+ }=0D
+=0D
+Done:=0D
+ //=0D
+ // Clear AuthSession Content=0D
+ //=0D
+ ZeroMem (&SendBuffer, sizeof(SendBuffer));=0D
+ ZeroMem (&RecvBuffer, sizeof(RecvBuffer));=0D
+ return Status;=0D
+}=0D
+=0D
/**=0D
This command reads a value from an area in NV memory previously defined =
by TPM2_NV_DefineSpace().=0D
=0D
diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h b/SecurityPkg/Inc=
lude/Library/Tpm2CommandLib.h
index ee8eb622951c..92967662ce96 100644
--- a/SecurityPkg/Include/Library/Tpm2CommandLib.h
+++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h
@@ -364,6 +364,28 @@ Tpm2NvUndefineSpace (
IN TPMS_AUTH_COMMAND *AuthSession OPTIONAL=0D
);=0D
=0D
+/**=0D
+ This command allows removal of a platform-created NV Index that has TPMA=
_NV_POLICY_DELETE SET.=0D
+=0D
+ @param[in] NvIndex The NV Index.=0D
+ @param[in] IndexAuthSession Auth session context for the Index auth/=
policy=0D
+ @param[in] PlatAuthSession Auth session context for the Platform au=
th/policy=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_NOT_FOUND The command was returned successfully, b=
ut NvIndex is not found.=0D
+ @retval EFI_UNSUPPORTED Selected NvIndex does not support deleti=
on through this call.=0D
+ @retval EFI_SECURITY_VIOLATION Deletion is not authorized by current po=
licy session.=0D
+ @retval EFI_INVALID_PARAMETER The command was unsuccessful.=0D
+ @retval EFI_DEVICE_ERROR The command was unsuccessful.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+Tpm2NvUndefineSpaceSpecial (=0D
+ IN TPMI_RH_NV_INDEX NvIndex,=0D
+ IN TPMS_AUTH_COMMAND *IndexAuthSession OPTIONAL,=0D
+ IN TPMS_AUTH_COMMAND *PlatAuthSession OPTIONAL=0D
+ );=0D
+=0D
/**=0D
This command reads a value from an area in NV memory previously defined =
by TPM2_NV_DefineSpace().=0D
=0D
--=20
2.31.1.windows.1


[PATCH v9 32/32] UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

Brijesh Singh
 

From: Tom Lendacky <thomas.lendacky@amd.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Use the SEV-SNP AP Creation NAE event to create and launch APs under
SEV-SNP. This capability will be advertised in the SEV Hypervisor
Feature Support PCD (PcdSevEsHypervisorFeatures).

Cc: Michael Roth <michael.roth@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 3 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 3 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 44 +++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 12 +-
UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c | 70 +++++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 51 ++--
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c | 261 ++++++++++++++++++
7 files changed, 425 insertions(+), 19 deletions(-)
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 28764418d7c1..43c104b4da2c 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -22,9 +22,11 @@ [Defines]
#

[Sources.IA32]
+ Ia32/AmdSev.c
Ia32/MpFuncs.nasm

[Sources.X64]
+ X64/AmdSev.c
X64/MpFuncs.nasm

[Sources.common]
@@ -73,6 +75,7 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## SOMETIMES_CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdCpuApStatusCheckIntervalInMicroSeconds ## CONSUMES
+ gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures ## CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index cbc3c1460423..859b9a29fc19 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -22,9 +22,11 @@ [Defines]
#

[Sources.IA32]
+ Ia32/AmdSev.c
Ia32/MpFuncs.nasm

[Sources.X64]
+ X64/AmdSev.c
X64/MpFuncs.nasm

[Sources.common]
@@ -64,6 +66,7 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## SOMETIMES_CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## SOMETIMES_CONSUMES
+ gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr ## CONSUMES

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index c52b6157429b..48f6e933bb36 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -15,6 +15,7 @@

#include <Register/Intel/Cpuid.h>
#include <Register/Amd/Cpuid.h>
+#include <Register/Amd/Ghcb.h>
#include <Register/Intel/Msr.h>
#include <Register/Intel/LocalApic.h>
#include <Register/Intel/Microcode.h>
@@ -150,6 +151,7 @@ typedef struct {
UINT8 PlatformId;
UINT64 MicrocodeEntryAddr;
UINT32 MicrocodeRevision;
+ SEV_ES_SAVE_AREA *SevEsSaveArea;
} CPU_AP_DATA;

//
@@ -294,6 +296,7 @@ struct _CPU_MP_DATA {

BOOLEAN SevEsIsEnabled;
BOOLEAN SevSnpIsEnabled;
+ BOOLEAN UseSevEsAPMethod;
UINTN SevEsAPBuffer;
UINTN SevEsAPResetStackStart;
CPU_MP_DATA *NewCpuMpData;
@@ -790,5 +793,46 @@ ConfidentialComputingGuestHas (
CONFIDENTIAL_COMPUTING_GUEST_ATTR Attr
);

+/**
+ Issue RMPADJUST to adjust the VMSA attribute of an SEV-SNP page.
+
+ @param[in] PageAddress
+ @param[in] VmsaPage
+
+ @return RMPADJUST return value
+**/
+UINT32
+SevSnpRmpAdjust (
+ IN EFI_PHYSICAL_ADDRESS PageAddress,
+ IN BOOLEAN VmsaPage
+ );
+
+/**
+ Create an SEV-SNP AP save area (VMSA) for use in running the vCPU.
+
+ @param[in] CpuMpData Pointer to CPU MP Data
+ @param[in] CpuData Pointer to CPU AP Data
+ @param[in] ApicId APIC ID of the vCPU
+**/
+VOID
+SevSnpCreateSaveArea (
+ IN CPU_MP_DATA *CpuMpData,
+ IN CPU_AP_DATA *CpuData,
+ UINT32 ApicId
+ );
+
+/**
+ Create SEV-SNP APs.
+
+ @param[in] CpuMpData Pointer to CPU MP Data
+ @param[in] ProcessorNumber The handle number of specified processor
+ (-1 for all APs)
+**/
+VOID
+SevSnpCreateAP (
+ IN CPU_MP_DATA *CpuMpData,
+ IN INTN ProcessorNumber
+ );
+
#endif

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 657a73dca05e..7a3ef0015a31 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -93,7 +93,13 @@ GetWakeupBuffer (
EFI_PHYSICAL_ADDRESS StartAddress;
EFI_MEMORY_TYPE MemoryType;

- if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
+ if (ConfidentialComputingGuestHas (CCAttrAmdSevEs) &&
+ !ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {
+ //
+ // An SEV-ES-only guest requires the memory to be reserved. SEV-SNP, which
+ // is also considered SEV-ES, uses a different AP startup method, though,
+ // which does not have the same requirement.
+ //
MemoryType = EfiReservedMemoryType;
} else {
MemoryType = EfiBootServicesData;
@@ -373,7 +379,7 @@ RelocateApLoop (
MpInitLibWhoAmI (&ProcessorNumber);
CpuMpData = GetCpuMpData ();
MwaitSupport = IsMwaitSupport ();
- if (CpuMpData->SevEsIsEnabled) {
+ if (CpuMpData->UseSevEsAPMethod) {
StackStart = CpuMpData->SevEsAPResetStackStart;
} else {
StackStart = mReservedTopOfApStack;
@@ -422,7 +428,7 @@ MpInitChangeApLoopCallback (
CpuPause ();
}

- if (CpuMpData->SevEsIsEnabled && (CpuMpData->WakeupBuffer != (UINTN) -1)) {
+ if (CpuMpData->UseSevEsAPMethod && (CpuMpData->WakeupBuffer != (UINTN) -1)) {
//
// There are APs present. Re-use reserved memory area below 1MB from
// WakeupBuffer as the area to be used for transitioning to 16-bit mode
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
new file mode 100644
index 000000000000..a4702e298d98
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
@@ -0,0 +1,70 @@
+/** @file
+
+ AMD SEV helper function.
+
+ Copyright (c) 2021, AMD Incorporated. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "MpLib.h"
+
+/**
+ Create an SEV-SNP AP save area (VMSA) for use in running the vCPU.
+
+ @param[in] CpuMpData Pointer to CPU MP Data
+ @param[in] CpuData Pointer to CPU AP Data
+ @param[in] ApicId APIC ID of the vCPU
+**/
+VOID
+SevSnpCreateSaveArea (
+ IN CPU_MP_DATA *CpuMpData,
+ IN CPU_AP_DATA *CpuData,
+ UINT32 ApicId
+ )
+{
+ //
+ // SEV-SNP is not support on 32-bit build.
+ //
+ ASSERT (FALSE);
+}
+
+/**
+ Create SEV-SNP APs.
+
+ @param[in] CpuMpData Pointer to CPU MP Data
+ @param[in] ProcessorNumber The handle number of specified processor
+ (-1 for all APs)
+**/
+VOID
+SevSnpCreateAP (
+ IN CPU_MP_DATA *CpuMpData,
+ IN INTN ProcessorNumber
+ )
+{
+ //
+ // SEV-SNP is not support on 32-bit build.
+ //
+ ASSERT (FALSE);
+}
+
+/**
+ Issue RMPADJUST to adjust the VMSA attribute of an SEV-SNP page.
+
+ @param[in] PageAddress
+ @param[in] VmsaPage
+
+ @return RMPADJUST return value
+**/
+UINT32
+SevSnpRmpAdjust (
+ IN EFI_PHYSICAL_ADDRESS PageAddress,
+ IN BOOLEAN VmsaPage
+ )
+{
+ //
+ // RMPADJUST is not supported in 32-bit mode
+ //
+ return RETURN_UNSUPPORTED;
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index de94b05e2263..758412058fe6 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -295,10 +295,11 @@ GetApLoopMode (
ApLoopMode = ApInHltLoop;
}

- if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
+ if (ConfidentialComputingGuestHas (CCAttrAmdSevEs) &&
+ !ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {
//
- // For SEV-ES, force AP in Hlt-loop mode in order to use the GHCB
- // protocol for starting APs
+ // For SEV-ES (SEV-SNP is also considered SEV-ES), force AP in Hlt-loop
+ // mode in order to use the GHCB protocol for starting APs
//
ApLoopMode = ApInHltLoop;
}
@@ -758,7 +759,7 @@ ApWakeupFunction (
// to allow the APs to issue an AP_RESET_HOLD before the BSP possibly
// performs another INIT-SIPI-SIPI sequence.
//
- if (!CpuMpData->SevEsIsEnabled) {
+ if (!CpuMpData->UseSevEsAPMethod) {
InterlockedDecrement ((UINT32 *) &CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
}
}
@@ -772,7 +773,7 @@ ApWakeupFunction (
//
while (TRUE) {
DisableInterrupts ();
- if (CpuMpData->SevEsIsEnabled) {
+ if (CpuMpData->UseSevEsAPMethod) {
SevEsPlaceApHlt (CpuMpData);
} else {
CpuSleep ();
@@ -1056,9 +1057,12 @@ AllocateResetVector (
);
//
// The AP reset stack is only used by SEV-ES guests. Do not allocate it
- // if SEV-ES is not enabled.
+ // if SEV-ES is not enabled. An SEV-SNP guest is also considered
+ // an SEV-ES guest, but uses a different method of AP startup, eliminating
+ // the need for the allocation.
//
- if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
+ if (ConfidentialComputingGuestHas (CCAttrAmdSevEs) &&
+ !ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {
//
// Stack location is based on ProcessorNumber, so use the total number
// of processors for calculating the total stack area.
@@ -1108,7 +1112,7 @@ FreeResetVector (
// perform the restore as this will overwrite memory which has data
// needed by SEV-ES.
//
- if (!CpuMpData->SevEsIsEnabled) {
+ if (!CpuMpData->UseSevEsAPMethod) {
RestoreWakeupBuffer (CpuMpData);
}
}
@@ -1144,7 +1148,7 @@ WakeUpAP (
ResetVectorRequired = FALSE;

if (CpuMpData->WakeUpByInitSipiSipi ||
- CpuMpData->InitFlag != ApInitDone) {
+ CpuMpData->InitFlag != ApInitDone) {
ResetVectorRequired = TRUE;
AllocateResetVector (CpuMpData);
AllocateSevEsAPMemory (CpuMpData);
@@ -1185,7 +1189,7 @@ WakeUpAP (
}
if (ResetVectorRequired) {
//
- // For SEV-ES, the initial AP boot address will be defined by
+ // For SEV-ES and SEV-SNP, the initial AP boot address will be defined by
// PcdSevEsWorkAreaBase. The Segment/Rip must be the jump address
// from the original INIT-SIPI-SIPI.
//
@@ -1195,8 +1199,14 @@ WakeUpAP (

//
// Wakeup all APs
+ // Must use the INIT-SIPI-SIPI method for initial configuration in
+ // order to obtain the APIC ID.
//
- SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart);
+ if (CpuMpData->SevSnpIsEnabled && CpuMpData->InitFlag != ApInitConfig) {
+ SevSnpCreateAP (CpuMpData, -1);
+ } else {
+ SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart);
+ }
}
if (CpuMpData->InitFlag == ApInitConfig) {
if (PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) {
@@ -1286,7 +1296,7 @@ WakeUpAP (
CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;

//
- // For SEV-ES, the initial AP boot address will be defined by
+ // For SEV-ES and SEV-SNP, the initial AP boot address will be defined by
// PcdSevEsWorkAreaBase. The Segment/Rip must be the jump address
// from the original INIT-SIPI-SIPI.
//
@@ -1294,10 +1304,14 @@ WakeUpAP (
SetSevEsJumpTable (ExchangeInfo->BufferStart);
}

- SendInitSipiSipi (
- CpuInfoInHob[ProcessorNumber].ApicId,
- (UINT32) ExchangeInfo->BufferStart
- );
+ if (CpuMpData->SevSnpIsEnabled && CpuMpData->InitFlag != ApInitConfig) {
+ SevSnpCreateAP (CpuMpData, (INTN) ProcessorNumber);
+ } else {
+ SendInitSipiSipi (
+ CpuInfoInHob[ProcessorNumber].ApicId,
+ (UINT32) ExchangeInfo->BufferStart
+ );
+ }
}
//
// Wait specified AP waken up
@@ -1832,6 +1846,11 @@ MpInitLibInitialize (
CpuMpData->SevSnpIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevSnp);
CpuMpData->SevEsAPBuffer = (UINTN) -1;
CpuMpData->GhcbBase = PcdGet64 (PcdGhcbBase);
+ CpuMpData->UseSevEsAPMethod = CpuMpData->SevEsIsEnabled && !CpuMpData->SevSnpIsEnabled;
+
+ if (CpuMpData->SevSnpIsEnabled) {
+ ASSERT ((PcdGet64 (PcdGhcbHypervisorFeatures) & GHCB_HV_FEATURES_SNP_AP_CREATE) == GHCB_HV_FEATURES_SNP_AP_CREATE);
+ }

//
// Make sure no memory usage outside of the allocated buffer.
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
new file mode 100644
index 000000000000..303271abdaad
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
@@ -0,0 +1,261 @@
+/** @file
+
+ AMD SEV helper function.
+
+ Copyright (c) 2021, AMD Incorporated. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "MpLib.h"
+#include <Library/VmgExitLib.h>
+#include <Register/Amd/Fam17Msr.h>
+#include <Register/Amd/Ghcb.h>
+
+/**
+ Create an SEV-SNP AP save area (VMSA) for use in running the vCPU.
+
+ @param[in] CpuMpData Pointer to CPU MP Data
+ @param[in] CpuData Pointer to CPU AP Data
+ @param[in] ApicId APIC ID of the vCPU
+**/
+VOID
+SevSnpCreateSaveArea (
+ IN CPU_MP_DATA *CpuMpData,
+ IN CPU_AP_DATA *CpuData,
+ UINT32 ApicId
+ )
+{
+ SEV_ES_SAVE_AREA *SaveArea;
+ IA32_CR0 ApCr0;
+ IA32_CR0 ResetCr0;
+ IA32_CR4 ApCr4;
+ IA32_CR4 ResetCr4;
+ UINTN StartIp;
+ UINT8 SipiVector;
+ UINT32 RmpAdjustStatus;
+ UINT64 VmgExitStatus;
+ MSR_SEV_ES_GHCB_REGISTER Msr;
+ GHCB *Ghcb;
+ BOOLEAN InterruptState;
+ UINT64 ExitInfo1;
+ UINT64 ExitInfo2;
+
+ //
+ // Allocate a single page for the SEV-ES Save Area and initialize it.
+ //
+ SaveArea = AllocateReservedPages (1);
+ if (!SaveArea) {
+ return;
+ }
+ ZeroMem (SaveArea, EFI_PAGE_SIZE);
+
+ //
+ // Propogate the CR0.NW and CR0.CD setting to the AP
+ //
+ ResetCr0.UintN = 0x00000010;
+ ApCr0.UintN = CpuData->VolatileRegisters.Cr0;
+ if (ApCr0.Bits.NW) {
+ ResetCr0.Bits.NW = 1;
+ }
+ if (ApCr0.Bits.CD) {
+ ResetCr0.Bits.CD = 1;
+ }
+
+ //
+ // Propagate the CR4.MCE setting to the AP
+ //
+ ResetCr4.UintN = 0;
+ ApCr4.UintN = CpuData->VolatileRegisters.Cr4;
+ if (ApCr4.Bits.MCE) {
+ ResetCr4.Bits.MCE = 1;
+ }
+
+ //
+ // Convert the start IP into a SIPI Vector
+ //
+ StartIp = CpuMpData->MpCpuExchangeInfo->BufferStart;
+ SipiVector = (UINT8) (StartIp >> 12);
+
+ //
+ // Set the CS:RIP value based on the start IP
+ //
+ SaveArea->Cs.Base = SipiVector << 12;
+ SaveArea->Cs.Selector = SipiVector << 8;
+ SaveArea->Cs.Limit = 0xFFFF;
+ SaveArea->Cs.Attributes.Bits.Present = 1;
+ SaveArea->Cs.Attributes.Bits.Sbit = 1;
+ SaveArea->Cs.Attributes.Bits.Type = SEV_ES_RESET_CODE_SEGMENT_TYPE;
+ SaveArea->Rip = StartIp & 0xFFF;
+
+ //
+ // Set the remaining values as defined in APM for INIT
+ //
+ SaveArea->Ds.Limit = 0xFFFF;
+ SaveArea->Ds.Attributes.Bits.Present = 1;
+ SaveArea->Ds.Attributes.Bits.Sbit = 1;
+ SaveArea->Ds.Attributes.Bits.Type = SEV_ES_RESET_DATA_SEGMENT_TYPE;
+ SaveArea->Es = SaveArea->Ds;
+ SaveArea->Fs = SaveArea->Ds;
+ SaveArea->Gs = SaveArea->Ds;
+ SaveArea->Ss = SaveArea->Ds;
+
+ SaveArea->Gdtr.Limit = 0xFFFF;
+ SaveArea->Ldtr.Limit = 0xFFFF;
+ SaveArea->Ldtr.Attributes.Bits.Present = 1;
+ SaveArea->Ldtr.Attributes.Bits.Type = SEV_ES_RESET_LDT_TYPE;
+ SaveArea->Idtr.Limit = 0xFFFF;
+ SaveArea->Tr.Limit = 0xFFFF;
+ SaveArea->Ldtr.Attributes.Bits.Present = 1;
+ SaveArea->Ldtr.Attributes.Bits.Type = SEV_ES_RESET_TSS_TYPE;
+
+ SaveArea->Efer = 0x1000;
+ SaveArea->Cr4 = ResetCr4.UintN;
+ SaveArea->Cr0 = ResetCr0.UintN;
+ SaveArea->Dr7 = 0x0400;
+ SaveArea->Dr6 = 0xFFFF0FF0;
+ SaveArea->Rflags = 0x0002;
+ SaveArea->GPat = 0x0007040600070406ULL;
+ SaveArea->XCr0 = 0x0001;
+ SaveArea->Mxcsr = 0x1F80;
+ SaveArea->X87Ftw = 0x5555;
+ SaveArea->X87Fcw = 0x0040;
+
+ //
+ // Set the SEV-SNP specific fields for the save area:
+ // VMPL - always VMPL0
+ // SEV_FEATURES - equivalent to the SEV_STATUS MSR right shifted 2 bits
+ //
+ SaveArea->Vmpl = 0;
+ SaveArea->SevFeatures = AsmReadMsr64 (MSR_SEV_STATUS) >> 2;
+
+ //
+ // To turn the page into a recognized VMSA page, issue RMPADJUST:
+ // Target VMPL but numerically higher than current VMPL
+ // Target PermissionMask is not used
+ //
+ RmpAdjustStatus = SevSnpRmpAdjust (
+ (EFI_PHYSICAL_ADDRESS) (UINTN) SaveArea,
+ TRUE
+ );
+ ASSERT (RmpAdjustStatus == 0);
+
+ ExitInfo1 = (UINT64) ApicId << 32;
+ ExitInfo1 |= SVM_VMGEXIT_SNP_AP_CREATE;
+ ExitInfo2 = (UINT64) (UINTN) SaveArea;
+
+ Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+ Ghcb = Msr.Ghcb;
+
+ VmgInit (Ghcb, &InterruptState);
+ Ghcb->SaveArea.Rax = SaveArea->SevFeatures;
+ VmgSetOffsetValid (Ghcb, GhcbRax);
+ VmgExitStatus = VmgExit (
+ Ghcb,
+ SVM_EXIT_SNP_AP_CREATION,
+ ExitInfo1,
+ ExitInfo2
+ );
+ VmgDone (Ghcb, InterruptState);
+
+ ASSERT (VmgExitStatus == 0);
+ if (VmgExitStatus != 0) {
+ RmpAdjustStatus = SevSnpRmpAdjust (
+ (EFI_PHYSICAL_ADDRESS) (UINTN) SaveArea,
+ FALSE
+ );
+ if (RmpAdjustStatus == 0) {
+ FreePages (SaveArea, 1);
+ } else {
+ DEBUG ((DEBUG_INFO, "SEV-SNP: RMPADJUST failed, leaking VMSA page\n"));
+ }
+
+ SaveArea = NULL;
+ }
+
+ if (CpuData->SevEsSaveArea) {
+ RmpAdjustStatus = SevSnpRmpAdjust (
+ (EFI_PHYSICAL_ADDRESS) (UINTN) CpuData->SevEsSaveArea,
+ FALSE
+ );
+ if (RmpAdjustStatus == 0) {
+ FreePages (CpuData->SevEsSaveArea, 1);
+ } else {
+ DEBUG ((DEBUG_INFO, "SEV-SNP: RMPADJUST failed, leaking VMSA page\n"));
+ }
+ }
+
+ CpuData->SevEsSaveArea = SaveArea;
+}
+
+/**
+ Create SEV-SNP APs.
+
+ @param[in] CpuMpData Pointer to CPU MP Data
+ @param[in] ProcessorNumber The handle number of specified processor
+ (-1 for all APs)
+**/
+VOID
+SevSnpCreateAP (
+ IN CPU_MP_DATA *CpuMpData,
+ IN INTN ProcessorNumber
+ )
+{
+ CPU_INFO_IN_HOB *CpuInfoInHob;
+ CPU_AP_DATA *CpuData;
+ UINTN Index;
+ UINT32 ApicId;
+
+ ASSERT (CpuMpData->MpCpuExchangeInfo->BufferStart < 0x100000);
+
+ CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
+
+ if (ProcessorNumber < 0) {
+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+ if (Index != CpuMpData->BspNumber) {
+ CpuData = &CpuMpData->CpuData[Index];
+ ApicId = CpuInfoInHob[Index].ApicId,
+ SevSnpCreateSaveArea (CpuMpData, CpuData, ApicId);
+ }
+ }
+ } else {
+ Index = (UINTN) ProcessorNumber;
+ CpuData = &CpuMpData->CpuData[Index];
+ ApicId = CpuInfoInHob[ProcessorNumber].ApicId,
+ SevSnpCreateSaveArea (CpuMpData, CpuData, ApicId);
+ }
+}
+
+/**
+ Issue RMPADJUST to adjust the VMSA attribute of an SEV-SNP page.
+
+ @param[in] PageAddress
+ @param[in] VmsaPage
+
+ @return RMPADJUST return value
+**/
+UINT32
+SevSnpRmpAdjust (
+ IN EFI_PHYSICAL_ADDRESS PageAddress,
+ IN BOOLEAN VmsaPage
+ )
+{
+ UINT64 Rdx;
+
+ //
+ // The RMPADJUST instruction is used to set or clear the VMSA bit for a
+ // page. The VMSA change is only made when running at VMPL0 and is ignored
+ // otherwise. If too low a target VMPL is specified, the instruction can
+ // succeed without changing the VMSA bit when not running at VMPL0. Using a
+ // target VMPL level of 1, RMPADJUST will return a FAIL_PERMISSION error if
+ // not running at VMPL0, thus ensuring that the VMSA bit is set appropriately
+ // when no error is returned.
+ //
+ Rdx = 1;
+ if (VmsaPage) {
+ Rdx |= RMPADJUST_VMSA_PAGE_BIT;
+ }
+
+ return AsmRmpAdjust ((UINT64) PageAddress, 0, Rdx);
+}
--
2.25.1


[PATCH v9 31/32] OvmfPkg/AmdSev: expose the SNP reserved pages through configuration table

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Now that both the secrets and cpuid pages are reserved in the HOB,
extract the location details through fixed PCD and make it available
to the guest OS through the configuration table.

Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/OvmfPkg.dec | 1 +
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 ++++
.../Guid/ConfidentialComputingSevSnpBlob.h | 33 +++++++++++++++++++
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +++++++++++++
4 files changed, 64 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 8dde5198c7cd..79619d585954 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -124,6 +124,7 @@ [Guids]
gQemuKernelLoaderFsMediaGuid = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
gGrubFileGuid = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
gConfidentialComputingSecretGuid = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
+ gConfidentialComputingSevSnpBlobGuid = {0x067b1f5f, 0xcf26, 0x44c5, {0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42}}

[Ppis]
# PPI whose presence in the PPI database signals that the TPM base address
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 0676fcc5b6a4..9acf860cf25e 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -42,6 +42,13 @@ [FeaturePcd]

[FixedPcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+
+[Guids]
+ gConfidentialComputingSevSnpBlobGuid

[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h b/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
new file mode 100644
index 000000000000..c98e7a1dcccd
--- /dev/null
+++ b/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
@@ -0,0 +1,33 @@
+ /** @file
+ UEFI Configuration Table for exposing the SEV-SNP launch blob.
+
+ Copyright (c) 2021, Advanced Micro Devices Inc. All right reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+ **/
+
+#ifndef CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB_H_
+#define CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB_H_
+
+#include <Uefi/UefiBaseType.h>
+
+#define CONFIDENTIAL_COMPUTING_SNP_BLOB_GUID \
+ { 0x067b1f5f, \
+ 0xcf26, \
+ 0x44c5, \
+ { 0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42 }, \
+ }
+
+typedef struct {
+ UINT32 Header;
+ UINT16 Version;
+ UINT16 Reserved1;
+ UINT64 SecretsPhysicalAddress;
+ UINT32 SecretsSize;
+ UINT64 CpuidPhysicalAddress;
+ UINT32 CpuidLSize;
+} CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION;
+
+extern EFI_GUID gConfidentialComputingSevSnpBlobGuid;
+
+#endif
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index c66c4e9b9272..6e1ba35e02b8 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -17,8 +17,20 @@
#include <Library/DxeServicesTableLib.h>
#include <Library/MemEncryptSevLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Guid/ConfidentialComputingSevSnpBlob.h>
#include <Library/PcdLib.h>

+STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
+ SIGNATURE_32('A','M','D','E'),
+ 1,
+ 0,
+ (UINT64)(UINTN) FixedPcdGet32 (PcdOvmfSnpSecretsBase),
+ FixedPcdGet32 (PcdOvmfSnpSecretsSize),
+ (UINT64)(UINTN) FixedPcdGet32 (PcdOvmfCpuidBase),
+ FixedPcdGet32 (PcdOvmfCpuidSize),
+};
+
EFI_STATUS
EFIAPI
AmdSevDxeEntryPoint (
@@ -130,5 +142,16 @@ AmdSevDxeEntryPoint (
}
}

+ //
+ // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
+ // It contains the location for both the Secrets and CPUID page.
+ //
+ if (MemEncryptSevSnpIsEnabled ()) {
+ return gBS->InstallConfigurationTable (
+ &gConfidentialComputingSevSnpBlobGuid,
+ &mSnpBootDxeTable
+ );
+ }
+
return EFI_SUCCESS;
}
--
2.25.1


[PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map

Brijesh Singh
 

When SEV-SNP is active, the CPUID and Secrets memory range contains the
information that is used during the VM boot. The content need to be persist
across the kexec boot. Mark the memory range as Reserved in the EFI map
so that guest OS or firmware does not use the range as a system RAM.

Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 4 ++++
OvmfPkg/PlatformPei/Platform.h | 5 +++++
OvmfPkg/PlatformPei/AmdSev.c | 31 +++++++++++++++++++++++++++++
OvmfPkg/PlatformPei/MemDetect.c | 2 ++
4 files changed, 42 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index d048b692f155..0a89e1a0760e 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -112,6 +112,8 @@ [Pcd]


[FixedPcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
@@ -122,6 +124,8 @@ [FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize

[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 8b1d270c2b0b..4169019b4c07 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -102,6 +102,11 @@ AmdSevInitialize (
VOID
);

+VOID
+SevInitializeRam (
+ VOID
+ );
+
extern EFI_BOOT_MODE mBootMode;

extern BOOLEAN mS3Supported;
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index b71a4a7304f7..133382407bc5 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -414,3 +414,34 @@ AmdSevInitialize (
ASSERT_RETURN_ERROR (PcdStatus);

}
+
+/**
+ The function performs SEV specific region initialization.
+
+ **/
+VOID
+SevInitializeRam (
+ VOID
+ )
+{
+ if (MemEncryptSevSnpIsEnabled ()) {
+ //
+ // If SEV-SNP is enabled, reserve the Secrets and CPUID memory area.
+ //
+ // This memory range is given to the PSP by the hypervisor to populate
+ // the information used during the SNP VM boots, and it need to persist
+ // across the kexec boots. Mark it as EfiReservedMemoryType so that
+ // the guest firmware and OS does not use it as a system memory.
+ //
+ BuildMemoryAllocationHob (
+ (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfSnpSecretsBase),
+ (UINT64)(UINTN) PcdGet32 (PcdOvmfSnpSecretsSize),
+ EfiReservedMemoryType
+ );
+ BuildMemoryAllocationHob (
+ (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfCpuidBase),
+ (UINT64)(UINTN) PcdGet32 (PcdOvmfCpuidSize),
+ EfiReservedMemoryType
+ );
+ }
+}
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index d736b85e0d90..058bb394f0df 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -821,6 +821,8 @@ InitializeRamRegions (
{
QemuInitializeRam ();

+ SevInitializeRam ();
+
if (mS3Supported && mBootMode != BOOT_ON_S3_RESUME) {
//
// This is the memory range that will be used for PEI on S3 resume
--
2.25.1


[PATCH v9 24/32] OvmfPkg/PlatformPei: set the Hypervisor Features PCD

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Version 2 of the GHCB specification added the support to query the
hypervisor feature bitmap. The feature bitmap provide information
such as whether to use the AP create VmgExit or use the AP jump table
approach to create the APs. The MpInitLib will use the
PcdGhcbHypervisorFeatures to determine which method to use for creating
the AP.

Query the hypervisor feature and set the PCD accordingly.

Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 3 ++
OvmfPkg/PlatformPei/AmdSev.c | 56 +++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 233b9494f64b..d048b692f155 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -62,6 +62,7 @@ [LibraryClasses]
MtrrLib
MemEncryptSevLib
PcdLib
+ VmgExitLib

[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
@@ -107,6 +108,8 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled
gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr
+ gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures
+

[FixedPcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 5e2c891309d4..b71a4a7304f7 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -24,6 +24,12 @@

#include "Platform.h"

+STATIC
+UINT64
+GetHypervisorFeature (
+ VOID
+ );
+
/**
Initialize SEV-SNP support if running as an SEV-SNP guest.

@@ -36,11 +42,22 @@ AmdSevSnpInitialize (
{
EFI_PEI_HOB_POINTERS Hob;
EFI_HOB_RESOURCE_DESCRIPTOR *ResourceHob;
+ UINT64 HvFeatures;
+ EFI_STATUS PcdStatus;

if (!MemEncryptSevSnpIsEnabled ()) {
return;
}

+ //
+ // Query the hypervisor feature using the VmgExit and set the value in the
+ // hypervisor features PCD.
+ //
+ HvFeatures = GetHypervisorFeature ();
+ PcdStatus = PcdSet64S (PcdGhcbHypervisorFeatures, HvFeatures);
+ ASSERT_RETURN_ERROR (PcdStatus);
+
+
//
// Iterate through the system RAM and validate it.
//
@@ -91,6 +108,45 @@ SevEsProtocolFailure (
CpuDeadLoop ();
}

+/**
+ Get the hypervisor features bitmap
+
+**/
+STATIC
+UINT64
+GetHypervisorFeature (
+ VOID
+ )
+{
+ UINT64 Status;
+ GHCB *Ghcb;
+ MSR_SEV_ES_GHCB_REGISTER Msr;
+ BOOLEAN InterruptState;
+ UINT64 Features;
+
+ Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+ Ghcb = Msr.Ghcb;
+
+ //
+ // Initialize the GHCB
+ //
+ VmgInit (Ghcb, &InterruptState);
+
+ //
+ // Query the Hypervisor Features.
+ //
+ Status = VmgExit (Ghcb, SVM_EXIT_HYPERVISOR_FEATURES, 0, 0);
+ if ((Status != 0)) {
+ SevEsProtocolFailure (GHCB_TERMINATE_GHCB_GENERAL);
+ }
+
+ Features = Ghcb->SaveArea.SwExitInfo2;
+
+ VmgDone (Ghcb, InterruptState);
+
+ return Features;
+}
+
/**

This function can be used to register the GHCB GPA.
--
2.25.1


[PATCH v9 29/32] OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address

Brijesh Singh
 

The SetMemoryEncDec() is used by the higher level routines to set or clear
the page encryption mask for system RAM and Mmio address. When SEV-SNP is
active, in addition to set/clear page mask it also updates the RMP table.
The RMP table updates are required for the system RAM address and not
the Mmio address.

Add a new parameter in SetMemoryEncDec() to tell whether the specified
address is Mmio. If its Mmio then skip the page state change in the RMP
table.

Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
.../X64/PeiDxeVirtualMemory.c | 20 ++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index 56db1e4b6ecf..0bb86d768017 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -673,6 +673,7 @@ InternalMemEncryptSevCreateIdentityMap1G (
@param[in] Mode Set or Clear mode
@param[in] CacheFlush Flush the caches before applying the
encryption mask
+ @param[in] Mmio The physical address specified is Mmio

@retval RETURN_SUCCESS The attributes were cleared for the
memory region.
@@ -688,7 +689,8 @@ SetMemoryEncDec (
IN PHYSICAL_ADDRESS PhysicalAddress,
IN UINTN Length,
IN MAP_RANGE_MODE Mode,
- IN BOOLEAN CacheFlush
+ IN BOOLEAN CacheFlush,
+ IN BOOLEAN Mmio
)
{
PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
@@ -711,14 +713,15 @@ SetMemoryEncDec (

DEBUG ((
DEBUG_VERBOSE,
- "%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx Mode=%a CacheFlush=%u\n",
+ "%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx Mode=%a CacheFlush=%u Mmio=%u\n",
gEfiCallerBaseName,
__FUNCTION__,
Cr3BaseAddress,
PhysicalAddress,
(UINT64)Length,
(Mode == SetCBit) ? "Encrypt" : "Decrypt",
- (UINT32)CacheFlush
+ (UINT32)CacheFlush,
+ (UINT32)Mmio
));

//
@@ -760,7 +763,7 @@ SetMemoryEncDec (
//
// The InternalSetPageState() is used for setting the page state in the RMP table.
//
- if ((Mode == ClearCBit) && MemEncryptSevSnpIsEnabled ()) {
+ if (!Mmio && (Mode == ClearCBit) && MemEncryptSevSnpIsEnabled ()) {
InternalSetPageState (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), SevSnpPageShared, FALSE);
}

@@ -998,7 +1001,8 @@ InternalMemEncryptSevSetMemoryDecrypted (
PhysicalAddress,
Length,
ClearCBit,
- TRUE
+ TRUE,
+ FALSE
);
}

@@ -1031,7 +1035,8 @@ InternalMemEncryptSevSetMemoryEncrypted (
PhysicalAddress,
Length,
SetCBit,
- TRUE
+ TRUE,
+ FALSE
);
}

@@ -1064,6 +1069,7 @@ InternalMemEncryptSevClearMmioPageEncMask (
PhysicalAddress,
Length,
ClearCBit,
- FALSE
+ FALSE,
+ TRUE
);
}
--
2.25.1


[PATCH v9 23/32] UefiCpuPkg: add PcdGhcbHypervisorFeatures

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Version 2 of the GHCB specification added a new VMGEXIT that the guest
could use for querying the hypervisor features. One of the immediate
users for it will be an AP creation code. When SEV-SNP is enabled, the
guest can use the newly added AP_CREATE VMGEXIT to create the APs.

The MpInitLib will check the hypervisor feature, and if AP_CREATE is
available, it will use it.

See GHCB spec version 2 for more details on the VMGEXIT.

Cc: Michael Roth <michael.roth@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
UefiCpuPkg/UefiCpuPkg.dec | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 9dbaa407c399..c979a0a90a0e 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -400,5 +400,10 @@ [PcdsDynamic, PcdsDynamicEx]
# @Prompt Memory encryption attribute
gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0|UINT64|0x60000017

+ ## This dynamic PCD contains the hypervisor features value obtained through the GHCB HYPERVISOR
+ # features VMGEXIT defined in the version 2 of GHCB spec.
+ # @Prompt GHCB Hypervisor Features
+ gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures|0x0|UINT64|0x60000018
+
[UserExtensions.TianoCore."ExtraFiles"]
UefiCpuPkgExtra.uni
--
2.25.1


[PATCH v9 28/32] OvmfPkg/MemEncryptSevLib: change the page state in the RMP table

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The MemEncryptSev{Set,Clear}PageEncMask() functions are used to set or
clear the memory encryption attribute in the page table. When SEV-SNP
is active, we also need to change the page state in the RMP table so that
it is in sync with the memory encryption attribute change.

Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
.../X64/PeiDxeVirtualMemory.c | 34 +++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index f146f6d61cc5..56db1e4b6ecf 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -17,6 +17,7 @@
#include <Register/Cpuid.h>

#include "VirtualMemory.h"
+#include "SnpPageStateChange.h"

STATIC BOOLEAN mAddressEncMaskChecked = FALSE;
STATIC UINT64 mAddressEncMask;
@@ -695,10 +696,12 @@ SetMemoryEncDec (
PAGE_MAP_AND_DIRECTORY_POINTER *PageDirectoryPointerEntry;
PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
PAGE_TABLE_ENTRY *PageDirectory2MEntry;
+ PHYSICAL_ADDRESS OrigPhysicalAddress;
PAGE_TABLE_4K_ENTRY *PageTableEntry;
UINT64 PgTableMask;
UINT64 AddressEncMask;
BOOLEAN IsWpEnabled;
+ UINTN OrigLength;
RETURN_STATUS Status;

//
@@ -751,6 +754,22 @@ SetMemoryEncDec (

Status = EFI_SUCCESS;

+ //
+ // To maintain the security gurantees we must set the page to shared in the RMP
+ // table before clearing the memory encryption mask from the current page table.
+ //
+ // The InternalSetPageState() is used for setting the page state in the RMP table.
+ //
+ if ((Mode == ClearCBit) && MemEncryptSevSnpIsEnabled ()) {
+ InternalSetPageState (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), SevSnpPageShared, FALSE);
+ }
+
+ //
+ // Save the specified length and physical address (we need it later).
+ //
+ OrigLength = Length;
+ OrigPhysicalAddress = PhysicalAddress;
+
while (Length != 0)
{
//
@@ -923,6 +942,21 @@ SetMemoryEncDec (
//
CpuFlushTlb();

+ //
+ // SEV-SNP requires that all the private pages (i.e pages mapped encrypted) must be
+ // added in the RMP table before the access.
+ //
+ // The InternalSetPageState() is used for setting the page state in the RMP table.
+ //
+ if ((Mode == SetCBit) && MemEncryptSevSnpIsEnabled ()) {
+ InternalSetPageState (
+ OrigPhysicalAddress,
+ EFI_SIZE_TO_PAGES (OrigLength),
+ SevSnpPagePrivate,
+ FALSE
+ );
+ }
+
Done:
//
// Restore page table write protection, if any.
--
2.25.1


[PATCH v9 15/32] OvmfPkg/MemEncryptSevLib: add function to check the VMPL0

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Virtual Machine Privilege Level (VMPL) feature in the SEV-SNP
architecture allows a guest VM to divide its address space into four
levels. The level can be used to provide the hardware isolated
abstraction layers with a VM. The VMPL0 is the highest privilege, and
VMPL3 is the least privilege. Certain operations must be done by the
VMPL0 software, such as:

* Validate or invalidate memory range (PVALIDATE instruction)
* Allocate VMSA page (RMPADJUST instruction when VMSA=1)

The initial SEV-SNP support assumes that the guest is running on VMPL0.
Let's add function in the MemEncryptSevLib that can be used for checking
whether guest is booted under the VMPL0.

Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
.../X64/SnpPageStateChange.h | 5 ++
.../X64/SecSnpSystemRamValidate.c | 46 +++++++++++++++++++
.../X64/SnpPageStateChangeInternal.c | 1 -
3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
index 8bbdf06468b9..cc1318075523 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
@@ -28,4 +28,9 @@ InternalSetPageState (
IN BOOLEAN UseLargeEntry
);

+VOID
+SnpPageStateFailureTerminate (
+ VOID
+ );
+
#endif
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
index 64aab7f45b6d..3394094a65e5 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
@@ -14,6 +14,43 @@

#include "SnpPageStateChange.h"

+//
+// The variable used for the VMPL check.
+//
+STATIC UINT8 gVmpl0Data[4096];
+
+/**
+ The function checks whether SEV-SNP guest is booted under VMPL0.
+
+ @retval TRUE The guest is booted under VMPL0
+ @retval FALSE The guest is not booted under VMPL0
+ **/
+STATIC
+BOOLEAN
+SevSnpIsVmpl0 (
+ VOID
+ )
+{
+ UINT64 Rdx;
+ EFI_STATUS Status;
+
+ //
+ // There is no straightforward way to query the current VMPL level.
+ // The simplest method is to use the RMPADJUST instruction to change
+ // a page permission to a VMPL level-1, and if the guest kernel is
+ // launched at a level <= 1, then RMPADJUST instruction will return
+ // an error.
+ //
+ Rdx = 1;
+
+ Status = AsmRmpAdjust ((UINT64) gVmpl0Data, 0, Rdx);
+ if (EFI_ERROR (Status)) {
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
/**
Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.

@@ -32,5 +69,14 @@ MemEncryptSevSnpPreValidateSystemRam (
return;
}

+ //
+ // The page state change uses the PVALIDATE instruction. The instruction
+ // can be run on VMPL-0 only. If its not VMPL-0 guest then terminate
+ // the boot.
+ //
+ if (!SevSnpIsVmpl0 ()) {
+ SnpPageStateFailureTerminate ();
+ }
+
InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
index 506df12d4e51..653151d4a422 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
@@ -40,7 +40,6 @@ MemoryStateToGhcbOp (
return Cmd;
}

-STATIC
VOID
SnpPageStateFailureTerminate (
VOID
--
2.25.1


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

Brijesh Singh
 

From: Michael Roth <michael.roth@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.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Suggested-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@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 4dae0974c06a..de94b05e2263 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
--
2.25.1


[PATCH v9 26/32] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

An SEV-SNP guest requires that the physical address of the GHCB must
be registered with the hypervisor before using it. See the GHCB
specification section 2.3.2 for more details.

Cc: Michael Roth <michael.roth@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +
UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 54 ++++++++++++++++++++
4 files changed, 59 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 2107f3f705a2..45bc1de23e3c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -222,6 +222,7 @@ typedef struct {
//
BOOLEAN Enable5LevelPaging;
BOOLEAN SevEsIsEnabled;
+ BOOLEAN SevSnpIsEnabled;
UINTN GhcbBase;
} MP_CPU_EXCHANGE_INFO;

@@ -291,6 +292,7 @@ struct _CPU_MP_DATA {
BOOLEAN WakeUpByInitSipiSipi;

BOOLEAN SevEsIsEnabled;
+ BOOLEAN SevSnpIsEnabled;
UINTN SevEsAPBuffer;
UINTN SevEsAPResetStackStart;
CPU_MP_DATA *NewCpuMpData;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 16a1602c3079..4dae0974c06a 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -889,6 +889,7 @@ FillExchangeInfoData (
DEBUG ((DEBUG_INFO, "%a: 5-Level Paging = %d\n", gEfiCallerBaseName, ExchangeInfo->Enable5LevelPaging));

ExchangeInfo->SevEsIsEnabled = CpuMpData->SevEsIsEnabled;
+ ExchangeInfo->SevSnpIsEnabled = CpuMpData->SevSnpIsEnabled;
ExchangeInfo->GhcbBase = (UINTN) CpuMpData->GhcbBase;

//
@@ -1817,6 +1818,7 @@ MpInitLibInitialize (
CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + MaxLogicalProcessorNumber);
InitializeSpinLock(&CpuMpData->MpLock);
CpuMpData->SevEsIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevEs);
+ CpuMpData->SevSnpIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevSnp);
CpuMpData->SevEsAPBuffer = (UINTN) -1;
CpuMpData->GhcbBase = PcdGet64 (PcdGhcbBase);

diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
index 2e9368a374a4..01668638f245 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -92,6 +92,7 @@ struc MP_CPU_EXCHANGE_INFO
.ModeHighSegment: CTYPE_UINT16 1
.Enable5LevelPaging: CTYPE_BOOLEAN 1
.SevEsIsEnabled: CTYPE_BOOLEAN 1
+ .SevSnpIsEnabled CTYPE_BOOLEAN 1
.GhcbBase: CTYPE_UINTN 1
endstruc

diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
index 0ccafe25eca4..0034920b2f6b 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
@@ -15,6 +15,57 @@

%define SIZE_4KB 0x1000

+RegisterGhcbGpa:
+ ;
+ ; Register GHCB GPA when SEV-SNP is enabled
+ ;
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevSnpIsEnabled)]
+ cmp byte [edi], 1 ; SevSnpIsEnabled
+ jne RegisterGhcbGpaDone
+
+ ; Save the rdi and rsi to used for later comparison
+ push rdi
+ push rsi
+ mov edi, eax
+ mov esi, edx
+ or eax, 18 ; Ghcb registration request
+ wrmsr
+ rep vmmcall
+ rdmsr
+ mov r12, rax
+ and r12, 0fffh
+ cmp r12, 19 ; Ghcb registration response
+ jne GhcbGpaRegisterFailure
+
+ ; Verify that GPA is not changed
+ and eax, 0fffff000h
+ cmp edi, eax
+ jne GhcbGpaRegisterFailure
+ cmp esi, edx
+ jne GhcbGpaRegisterFailure
+ pop rsi
+ pop rdi
+ jmp RegisterGhcbGpaDone
+
+ ;
+ ; Request the guest termination
+ ;
+GhcbGpaRegisterFailure:
+ xor edx, edx
+ mov eax, 256 ; GHCB terminate
+ wrmsr
+ rep vmmcall
+
+ ; We should not return from the above terminate request, but if we do
+ ; then enter into the hlt loop.
+DoHltLoop:
+ cli
+ hlt
+ jmp DoHltLoop
+
+RegisterGhcbGpaDone:
+ OneTimeCallRet RegisterGhcbGpa
+
;
; The function checks whether SEV-ES is enabled, if enabled
; then setup the GHCB page.
@@ -39,6 +90,9 @@ SevEsSetupGhcb:
mov rdx, rax
shr rdx, 32
mov rcx, 0xc0010130
+
+ OneTimeCall RegisterGhcbGpa
+
wrmsr

SevEsSetupGhcbExit:
--
2.25.1

3781 - 3800 of 85635