Date   

[PATCH RFC v3 11/22] 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 for the futher detail.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +
UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++++++++++++++++++
6 files changed, 58 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index d34419c2a524..48d7dfa4450f 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -76,3 +76,4 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONSUMES
+ gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled ## CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 36fcb96b5852..ab8279df596f 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -65,6 +65,7 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled ## CONSUMES
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONSUMES
+ gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled ## CONSUMES

[Ppis]
gEdkiiPeiShadowMicrocodePpiGuid ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index e88a5355c983..4abaa2243d0a 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -218,6 +218,7 @@ typedef struct {
//
BOOLEAN Enable5LevelPaging;
BOOLEAN SevEsIsEnabled;
+ BOOLEAN SevSnpIsEnabled;
UINTN GhcbBase;
} MP_CPU_EXCHANGE_INFO;

@@ -287,6 +288,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 dc2a54aa31e8..7cbcce101414 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1040,6 +1040,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;

//
@@ -2016,6 +2017,7 @@ MpInitLibInitialize (
CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + MaxLogicalProcessorNumber);
InitializeSpinLock(&CpuMpData->MpLock);
CpuMpData->SevEsIsEnabled = PcdGetBool (PcdSevEsIsEnabled);
+ CpuMpData->SevSnpIsEnabled = PcdGetBool (PcdSevSnpIsEnabled);
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/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 50df802d1fca..19939c093d2e 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -194,9 +194,60 @@ LongModeStart:
mov rdx, rax
shr rdx, 32
mov rcx, 0xc0010130
+
+ ;
+ ; Register GHCB GPA when SEV-SNP is enabled
+ ;
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevSnpIsEnabled)]
+ cmp byte [edi], 1 ; SevSnpIsEnabled
+ jne SetGhcbAddress
+
+ ; 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
+
+ ;
+ ; Program GHCB
+ ;
+SetGhcbAddress:
wrmsr
jmp CProcedureInvoke

+ ;
+ ; 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
+
GetApicId:
lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
cmp byte [edi], 1 ; SevEsIsEnabled
--
2.17.1


[PATCH RFC v3 10/22] OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest

Brijesh Singh
 

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

The SEV-SNP guest requires that GHCB GPA must be registered before using.
The GHCB GPA can be registred using the GhcbGPARegister().

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/AmdSev.c | 8 +++
OvmfPkg/Sec/SecMain.c | 79 +++++++++++++++++++++++++++++
3 files changed, 88 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 89c8e9627c86..e9a10146effd 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -52,6 +52,7 @@ [LibraryClasses]
BaseLib
CacheMaintenanceLib
DebugLib
+ GhcbRegisterLib
HobLib
IoLib
PciLib
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 81e40e0889aa..54b07622b4dd 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -14,6 +14,7 @@
#include <Library/DebugLib.h>
#include <Library/HobLib.h>
#include <Library/MemEncryptSevLib.h>
+#include <Library/GhcbRegisterLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/PcdLib.h>
#include <PiPei.h>
@@ -156,6 +157,13 @@ AmdSevEsInitialize (
"SEV-ES is enabled, %lu GHCB backup pages allocated starting at 0x%p\n",
(UINT64)GhcbBackupPageCount, GhcbBackupBase));

+ if (MemEncryptSevSnpIsEnabled ()) {
+ //
+ // SEV-SNP guest requires that GHCB GPA must be registered before using it.
+ //
+ GhcbRegister (GhcbBasePa);
+ }
+
AsmWriteMsr64 (MSR_SEV_ES_GHCB, GhcbBasePa);

//
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2aa..faa6891cca79 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -750,6 +750,74 @@ SevEsProtocolFailure (
CpuDeadLoop ();
}

+/**
+ Determine if SEV-SNP is active.
+
+ @retval TRUE SEV-SNP is enabled
+ @retval FALSE SEV-SNP is not enabled
+
+**/
+STATIC
+BOOLEAN
+SevSnpIsEnabled (
+ VOID
+ )
+{
+ SEC_SEV_ES_WORK_AREA *SevEsWorkArea;
+
+ SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
+
+ return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevSnpEnabled != 0));
+}
+
+/**
+ The GHCB GPA registeration need to be done before the ProcessLibraryConstructorList()
+ is called. So use a local implementation instead of including the GhcbRegisterLib.
+
+ */
+STATIC
+VOID
+SevSnpGhcbRegister (
+ UINTN Address
+ )
+{
+ MSR_SEV_ES_GHCB_REGISTER Msr;
+ MSR_SEV_ES_GHCB_REGISTER CurrentMsr;
+ EFI_PHYSICAL_ADDRESS GuestFrameNumber;
+
+ GuestFrameNumber = Address >> EFI_PAGE_SHIFT;
+
+ //
+ // Save the current MSR Value
+ //
+ CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+ //
+ // Use the GHCB MSR Protocol to request to register the GPA.
+ //
+ Msr.GhcbPhysicalAddress = 0;
+ Msr.GhcbGpaRegister.Function = GHCB_INFO_GHCB_GPA_REGISTER_REQUEST;
+ Msr.GhcbGpaRegister.GuestFrameNumber = GuestFrameNumber;
+ AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+ AsmVmgExit ();
+
+ Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+ //
+ // If hypervisor responded with a different GPA than requested then fail.
+ //
+ if ((Msr.GhcbGpaRegister.Function != GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE) ||
+ (Msr.GhcbGpaRegister.GuestFrameNumber != GuestFrameNumber)) {
+ SevEsProtocolFailure (GHCB_TERMINATE_GHCB_GENERAL);
+ }
+
+ //
+ // Restore the MSR
+ //
+ AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);
+}
+
/**
Validate the SEV-ES/GHCB protocol level.

@@ -791,6 +859,17 @@ SevEsProtocolCheck (
SevEsProtocolFailure (GHCB_TERMINATE_GHCB_PROTOCOL);
}

+ //
+ // We cannot use the MemEncryptSevSnpIsEnabled () because the
+ // ProcessLibraryConstructorList () is not called yet.
+ //
+ if (SevSnpIsEnabled ()) {
+ //
+ // SEV-SNP guest requires that GHCB GPA must be registered before using it.
+ //
+ SevSnpGhcbRegister (FixedPcdGet32 (PcdOvmfSecGhcbBase));
+ }
+
//
// SEV-ES protocol checking succeeded, set the initial GHCB address
//
--
2.17.1


[PATCH RFC v3 08/22] OvmfPkg/ResetVector: invalidate the GHCB page

Brijesh Singh
 

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

When SEV-SNP is active, the GHCB page is mapped un-encrypted in the
initial page table built by the reset vector code. Just clearing the
encryption attribute from the page table is not enough. The page also
needs to be added as shared in the RMP table.

The GHCB page was part of the pre-validated memory range specified
through the SnpBootBlock GUID. To maintain the security guarantees,
we must invalidate the GHCB page before clearing the encryption
attribute from the page table, and add the page shared in the RMP
table.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 93 +++++++++++++++++++++++
1 file changed, 93 insertions(+)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 75e63d2a0561..f764825755f0 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -10,6 +10,8 @@

BITS 32

+%include "Nasm.inc"
+
%define PAGE_PRESENT 0x01
%define PAGE_READ_WRITE 0x02
%define PAGE_USER_SUPERVISOR 0x04
@@ -70,9 +72,87 @@ BITS 32
%define GHCB_HYPERVISOR_FEATURES_REQUEST 128
%define GHCB_HYPERVISOR_FEATURES_RESPONSE 129

+; GHCB Page Invalidate request and response protocol values
+;
+%define GHCB_PAGE_STATE_CHANGE_REQUEST 20
+%define GHCB_PAGE_STATE_CHANGE_RESPONSE 21
+%define GHCB_PAGE_STATE_SHARED 2
+
; GHCB request to terminate protocol values
%define GHCB_GENERAL_TERMINATE_REQUEST 255

+; If its an SEV-SNP guest then use the page state change VMGEXIT to invalidate
+; the GHCB page.
+;
+; Modified: EAX, EBX, ECX, EDX
+;
+InvalidateGHCBPage:
+ ; Check if it is SEV-SNP guest.
+ cmp byte[SEV_ES_WORK_AREA_SNP], 1
+ jne InvalidateGHCBPageDone
+
+ ; Check whether hypervisor features has SEV-SNP (BIT0) set to indicate that
+ ; hypervisor supports the page state change.
+ mov eax, dword[SEV_ES_WORK_AREA_HYPERVISOR_FEATURES]
+ bt eax, 0
+ jnc TerminateSevGuestLaunch
+
+ ; Use PVALIDATE instruction to invalidate the page
+ mov eax, GHCB_BASE
+ mov ecx, 0
+ mov edx, 0
+ PVALIDATE
+
+ ; Save the carry flag to be use later.
+ setc dl
+
+ ; If PVALIDATE fail then abort the launch.
+ cmp eax, 0
+ jne TerminateSevGuestLaunch
+
+ ; Check the carry flag to determine if RMP entry was updated.
+ cmp dl, 0
+ jne TerminateSevGuestLaunch
+
+ ; Ask hypervisor to change the page state to shared using the
+ ; Page State Change VMGEXIT.
+ ;
+ ; Setup GHCB MSR
+ ; GHCB_MSR[55:52] = Page Operation
+ ; GHCB_MSR[51:12] = Guest Physical Frame Number
+ ; GHCB_MSR[11:0] = Page State Change Request
+ ;
+ mov eax, (GHCB_BASE >> 12)
+ shl eax, 12
+ or eax, GHCB_PAGE_STATE_CHANGE_REQUEST
+ mov edx, (GHCB_PAGE_STATE_SHARED << 20)
+ mov ecx, 0xc0010130
+ wrmsr
+
+ ;
+ ; Issue VMGEXIT - NASM doesn't support the vmmcall instruction in 32-bit
+ ; mode, so work around this by temporarily switching to 64-bit mode.
+ ;
+BITS 64
+ rep vmmcall
+BITS 32
+
+ ;
+ ; Response GHCB MSR
+ ; GHCB_MSR[51:12] = Guest Physical Frame Number
+ ; GHCB_MSR[11:0] = Page State Change Response
+ ;
+ mov ecx, 0xc0010130
+ rdmsr
+ and eax, 0xfff
+ cmp eax, GHCB_PAGE_STATE_CHANGE_RESPONSE
+ jnz TerminateSevGuestLaunch
+ cmp edx, 0
+ jnz TerminateSevGuestLaunch
+
+InvalidateGHCBPageDone:
+ OneTimeCallRet InvalidateGHCBPage
+
; Check if Secure Encrypted Virtualization (SEV) features are enabled.
;
; Register usage is tight in this routine, so multiple calls for the
@@ -450,6 +530,19 @@ clearGhcbMemoryLoop:
;
OneTimeCall CheckHypervisorFeatures

+ ;
+ ; The page table built above cleared the memory encryption mask from the
+ ; GHCB_BASE (aka made it shared). When SEV-SNP is enabled, to maintain
+ ; the security guarantees, the page state transition from private to
+ ; shared must go through the page invalidation steps. Invalidate the
+ ; memory range before loading the page table below.
+ ;
+ ; NOTE: the invalidation must happen after zeroing the GHCB memory. This
+ ; is because, in the 32-bit mode all the access are considered private.
+ ; The invalidation before the zero'ing will cause a #VC.
+ ;
+ OneTimeCall InvalidateGHCBPage
+
SetCr3:
;
; Set CR3 now that the paging structures are available
--
2.17.1


[PATCH RFC v3 07/22] OvmfPkg/ResetVector: validate the data pages used in SEC phase

Brijesh Singh
 

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

An SEV-SNP guest requires that private memory (aka pages mapped encrypted)
must be validated before being accessed.

The validation process consist of the following sequence:

1) Set the memory encryption attribute in the page table (aka C-bit).
Note: If the processor is in non-PAE mode, then all the memory accesses
are considered private.
2) Add the memory range as private in the RMP table. This can be performed
using the Page State Change VMGEXIT defined in the GHCB specification.
3) Use the PVALIDATE instruction to set the Validated Bit in the RMP table.

During the guest creation time, the VMM encrypts the OVMF_CODE.fd using
the SEV-SNP firmware provided LAUNCH_UPDATE_DATA command. In addition to
encrypting the content, the command also validates the memory region.
This allows us to execute the code without going through the validation
sequence.

During execution, the reset vector need to access some data pages
(such as page tables, SevESWorkarea, Sec stack). The data pages are
accessed as private memory. The data pages are not part of the
OVMF_CODE.fd, so they were not validated during the guest creation.

There are two approaches we can take to validate the data pages before
the access:

a) Enhance the OVMF reset vector code to validate the pages as described
above (go through step 2 - 3).
OR
b) Validate the pages during the guest creation time. The SEV firmware
provides a command which can be used by the VMM to validate the pages
without affecting the measurement of the launch.

Approach #b seems much simpler; it does not require any changes to the
OVMF reset vector code.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/OvmfPkg.dec | 5 +++++
OvmfPkg/OvmfPkgX64.fdf | 9 ++++++++-
OvmfPkg/ResetVector/ResetVector.inf | 2 ++
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 5 +++++
OvmfPkg/ResetVector/ResetVector.nasmb | 2 ++
5 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index fdb5dacdc7fa..e00f8159f317 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -327,6 +327,11 @@ [PcdsFixedAtBuild]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|0x0|UINT32|0x47
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize|0x0|UINT32|0x48

+ ## The start and end of pre-validated memory region by the hypervisor
+ # through the SEV-SNP firmware.
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpHypervisorPreValidatedStart|0x0|UINT32|0x49
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpHypervisorPreValidatedEnd|0x0|UINT32|0x50
+
[PcdsDynamic, PcdsDynamicEx]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 1300af666c49..4c59001fb750 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -105,7 +105,14 @@ [FD.MEMFD]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
FV = DXEFV

-################################################################################
+##########################################################################################
+#
+# The range of the pages pre-validated through the SEV-SNP firmware while creating SEV-SNP guest
+#
+SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpHypervisorPreValidatedStart = $(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
+
+SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpHypervisorPreValidatedEnd = $(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
+##########################################################################################

[FV.SECFV]
FvNameGuid = 763BED0D-DE9F-48F5-81F1-3E90E1B1A015
diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index 8e52265602c3..2a75e909c769 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -49,3 +49,5 @@ [FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpHypervisorPreValidatedStart
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpHypervisorPreValidatedEnd
diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
index 05c7e32f46a0..769dd0bccfd9 100644
--- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
+++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
@@ -55,11 +55,16 @@ guidedStructureStart:
; the BIOS at a RAM area defined by SEV_SNP_CPUID_BASE. A hypervisor will
; locate this information using the SEV-SNP boot block GUID.
;
+; In order to boot the SEV-SNP guest the hypervisor must pre-validated the
+; memory range from SNP_HV_VALIDATED_START to SNP_HV_VALIDATED_END.
+;
; GUID (SEV-SNP boot block): bd39c0c2-2f8e-4243-83e8-1b74cebcb7d9
;
sevSnpBootBlockStart:
DD SNP_CPUID_BASE
DD SNP_CPUID_SIZE
+ DD SNP_HV_VALIDATED_START
+ DD SNP_HV_VALIDATED_END
DW sevSnpBootBlockEnd - sevSnpBootBlockStart
DB 0xC2, 0xC0, 0x39, 0xBD, 0x8e, 0x2F, 0x43, 0x42
DB 0x83, 0xE8, 0x1B, 0x74, 0xCE, 0xBC, 0xB7, 0xD9
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 36739096e7e1..465038e39de3 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -92,5 +92,7 @@
%define SEV_LAUNCH_SECRET_SIZE FixedPcdGet32 (PcdSevLaunchSecretSize)
%define SNP_CPUID_BASE FixedPcdGet32 (PcdOvmfSnpCpuidBase)
%define SNP_CPUID_SIZE FixedPcdGet32 (PcdOvmfSnpCpuidSize)
+ %define SNP_HV_VALIDATED_START FixedPcdGet32 (PcdOvmfSnpHypervisorPreValidatedStart)
+ %define SNP_HV_VALIDATED_END FixedPcdGet32 (PcdOvmfSnpHypervisorPreValidatedEnd)
%include "Ia16/ResetVectorVtf0.asm"

--
2.17.1


[PATCH RFC v3 06/22] OvmfPkg: reserve CPUID page for the SEV-SNP guest

Brijesh Singh
 

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

During the SEV-SNP guest launch sequence, two special pages need to be
inserted, the secrets and CPUID. The secrets page, contain the VM
platform communication keys. The guest BIOS and/or OS can use this key
to communicate with the SEV firmware to get the attestation report.
The CPUID page, contain the CPUIDs entries filtered through the AMD-SEV
firmware.

OvmfPkg already reserves the memory for the Secrets Page in the MEMFD.
Extend the MEMFD to reserve the memory for the CPUID page.

See SEV-SNP spec for more information on the content layout of the secrets
and CPUID page, and how it can be used by the SEV-SNP guest VM.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/OvmfPkg.dec | 6 ++++++
OvmfPkg/OvmfPkgX64.fdf | 3 +++
OvmfPkg/PlatformPei/PlatformPei.inf | 2 ++
OvmfPkg/ResetVector/ResetVector.inf | 2 ++
OvmfPkg/PlatformPei/MemDetect.c | 12 ++++++++++++
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 18 ++++++++++++++++++
OvmfPkg/ResetVector/ResetVector.nasmb | 2 ++
7 files changed, 45 insertions(+)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 6ae733f6e39f..fdb5dacdc7fa 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -321,6 +321,12 @@ [PcdsFixedAtBuild]
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43

+ ## The base address and size of the SEV-SNP CPUID Area provisioned by the
+ # SEV-SNP firmware. If this is set in the .fdf, the platform
+ # is responsible for protecting the area from DXE phase overwrites.
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|0x0|UINT32|0x47
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize|0x0|UINT32|0x48
+
[PcdsDynamic, PcdsDynamicEx]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 9126b8eb5014..1300af666c49 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -91,6 +91,9 @@ [FD.MEMFD]
0x00D000|0x001000
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize

+0x00E000|0x001000
+gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize
+
0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 3256ccfe88d8..89c8e9627c86 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -120,6 +120,8 @@ [FixedPcd]
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index dc38f68919cd..8e52265602c3 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -45,5 +45,7 @@ [Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize

[FixedPcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index c08aa2e45a53..483e92af8219 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -894,6 +894,18 @@ InitializeRamRegions (
EfiACPIMemoryNVS
);
}
+
+ if (MemEncryptSevSnpIsEnabled ()) {
+ //
+ // If SEV-SNP is enabled, reserve the CPUID page. The memory range should
+ // not be treated as a RAM by the guest OS, so, mark it as reserved.
+ //
+ BuildMemoryAllocationHob (
+ (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfSnpCpuidBase),
+ (UINT64)(UINTN) PcdGet32 (PcdOvmfSnpCpuidSize),
+ EfiReservedMemoryType
+ );
+ }
#endif
}

diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
index 9c0b5853a46f..05c7e32f46a0 100644
--- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
+++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
@@ -47,6 +47,24 @@ TIMES (15 - ((guidedStructureEnd - guidedStructureStart + 15) % 16)) DB 0
;
guidedStructureStart:

+;
+; SEV-SNP boot support
+;
+; sevSnpBlock:
+; For the initial boot of SEV-SNP guest, a CPUID page must be reserved by
+; the BIOS at a RAM area defined by SEV_SNP_CPUID_BASE. A hypervisor will
+; locate this information using the SEV-SNP boot block GUID.
+;
+; GUID (SEV-SNP boot block): bd39c0c2-2f8e-4243-83e8-1b74cebcb7d9
+;
+sevSnpBootBlockStart:
+ DD SNP_CPUID_BASE
+ DD SNP_CPUID_SIZE
+ DW sevSnpBootBlockEnd - sevSnpBootBlockStart
+ DB 0xC2, 0xC0, 0x39, 0xBD, 0x8e, 0x2F, 0x43, 0x42
+ DB 0x83, 0xE8, 0x1B, 0x74, 0xCE, 0xBC, 0xB7, 0xD9
+sevSnpBootBlockEnd:
+
;
; SEV Secret block
;
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 5beba3ecb290..36739096e7e1 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -90,5 +90,7 @@
%define SEV_ES_AP_RESET_IP FixedPcdGet32 (PcdSevEsWorkAreaBase)
%define SEV_LAUNCH_SECRET_BASE FixedPcdGet32 (PcdSevLaunchSecretBase)
%define SEV_LAUNCH_SECRET_SIZE FixedPcdGet32 (PcdSevLaunchSecretSize)
+ %define SNP_CPUID_BASE FixedPcdGet32 (PcdOvmfSnpCpuidBase)
+ %define SNP_CPUID_SIZE FixedPcdGet32 (PcdOvmfSnpCpuidSize)
%include "Ia16/ResetVectorVtf0.asm"

--
2.17.1


[PATCH RFC v3 09/22] OvmfPkg: add library to support registering GHCB GPA

Brijesh Singh
 

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

An SEV-SNP guest us required to perform GHCB GPA registration before
using a GHCB. See the GHCB spec section 2.5.2 for more details.

Add a library that can be called to perform the GHCB GPA registration.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/OvmfPkg.dec | 4 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
.../GhcbRegisterLib/GhcbRegisterLib.inf | 33 +++++++
OvmfPkg/Include/Library/GhcbRegisterLib.h | 27 ++++++
.../Library/GhcbRegisterLib/GhcbRegisterLib.c | 97 +++++++++++++++++++
7 files changed, 164 insertions(+)
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
create mode 100644 OvmfPkg/Include/Library/GhcbRegisterLib.h
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index e00f8159f317..3886d43bd3de 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -106,6 +106,10 @@ [LibraryClasses]
#
XenPlatformLib|Include/Library/XenPlatformLib.h

+ ## @libraryclass Register GHCB GPA
+ #
+ GhcbRegisterLib|Include/Library/GhcbRegisterLib.h
+
[Guids]
gUefiOvmfPkgTokenSpaceGuid = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
gEfiXenInfoGuid = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 33fbd767903e..7cbef8e82282 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -243,6 +243,7 @@ [LibraryClasses]
[LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
+ GhcbRegisterLib|OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf

[LibraryClasses.common.SEC]
TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index b13e5cfd9047..6b0bc02bd536 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -247,6 +247,7 @@ [LibraryClasses]
[LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
+ GhcbRegisterLib|OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf

[LibraryClasses.common.SEC]
TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index ea08e1fabc65..8d9a0a077601 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -247,6 +247,7 @@ [LibraryClasses]
[LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
VmgExitLib|OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
+ GhcbRegisterLib|OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf

[LibraryClasses.common.SEC]
TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf
diff --git a/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
new file mode 100644
index 000000000000..8cc39ef7153b
--- /dev/null
+++ b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
@@ -0,0 +1,33 @@
+## @file
+# GHCBRegisterLib Support Library.
+#
+# Copyright (C) 2021, Advanced Micro Devices, Inc. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = GhcbRegisterLib
+ FILE_GUID = 0e913c15-12cd-430b-8714-ffe85672a77b
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = GhcbRegisterLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = X64
+#
+
+[Sources.common]
+ GhcbRegisterLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
diff --git a/OvmfPkg/Include/Library/GhcbRegisterLib.h b/OvmfPkg/Include/Library/GhcbRegisterLib.h
new file mode 100644
index 000000000000..7d98b6eb36f8
--- /dev/null
+++ b/OvmfPkg/Include/Library/GhcbRegisterLib.h
@@ -0,0 +1,27 @@
+/** @file
+
+ Declarations of utility functions used for GHCB GPA registration.
+
+ Copyright (C) 2021, AMD Inc, All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _GHCB_REGISTER_LIB_H_
+#define _GHCB_REGISTER_LIB_H_
+
+/**
+
+ This function can be used to register the GHCB GPA.
+
+ @param[in] Address The physical address to registered.
+
+**/
+VOID
+EFIAPI
+GhcbRegister (
+ IN EFI_PHYSICAL_ADDRESS Address
+ );
+
+#endif // _GHCB_REGISTER_LIB_H_
diff --git a/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
new file mode 100644
index 000000000000..7fe0aad75a1a
--- /dev/null
+++ b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
@@ -0,0 +1,97 @@
+/** @file
+ GHCBRegister Support Library.
+
+ Copyright (C) 2021, Advanced Micro Devices, Inc. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Uefi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/VmgExitLib.h>
+#include <Library/GhcbRegisterLib.h>
+#include <Register/Amd/Msr.h>
+
+/**
+ Handle an SEV-SNP/GHCB protocol check failure.
+
+ Notify the hypervisor using the VMGEXIT instruction that the SEV-SNP guest
+ wishes to be terminated.
+
+ @param[in] ReasonCode Reason code to provide to the hypervisor for the
+ termination request.
+
+**/
+STATIC
+VOID
+SevEsProtocolFailure (
+ IN UINT8 ReasonCode
+ )
+{
+ MSR_SEV_ES_GHCB_REGISTER Msr;
+
+ //
+ // Use the GHCB MSR Protocol to request termination by the hypervisor
+ //
+ Msr.GhcbPhysicalAddress = 0;
+ Msr.GhcbTerminate.Function = GHCB_INFO_TERMINATE_REQUEST;
+ Msr.GhcbTerminate.ReasonCodeSet = GHCB_TERMINATE_GHCB;
+ Msr.GhcbTerminate.ReasonCode = ReasonCode;
+ AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+ AsmVmgExit ();
+
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
+
+/**
+
+ This function can be used to register the GHCB GPA.
+
+ @param[in] Address The physical address to be registered.
+
+**/
+VOID
+EFIAPI
+GhcbRegister (
+ IN EFI_PHYSICAL_ADDRESS Address
+ )
+{
+ MSR_SEV_ES_GHCB_REGISTER Msr;
+ MSR_SEV_ES_GHCB_REGISTER CurrentMsr;
+ EFI_PHYSICAL_ADDRESS GuestFrameNumber;
+
+ GuestFrameNumber = Address >> EFI_PAGE_SHIFT;
+
+ //
+ // Save the current MSR Value
+ //
+ CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+ //
+ // Use the GHCB MSR Protocol to request to register the GPA.
+ //
+ Msr.GhcbPhysicalAddress = 0;
+ Msr.GhcbGpaRegister.Function = GHCB_INFO_GHCB_GPA_REGISTER_REQUEST;
+ Msr.GhcbGpaRegister.GuestFrameNumber = GuestFrameNumber;
+ AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+ AsmVmgExit ();
+
+ Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+ //
+ // If hypervisor responded with a different GPA than requested then fail.
+ //
+ if ((Msr.GhcbGpaRegister.Function != GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE) ||
+ (Msr.GhcbGpaRegister.GuestFrameNumber != GuestFrameNumber)) {
+ SevEsProtocolFailure (GHCB_TERMINATE_GHCB_GENERAL);
+ }
+
+ //
+ // Restore the MSR
+ //
+ AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);
+}
--
2.17.1


[PATCH RFC v3 05/22] OvmfPkg: reserve Secrets page in MEMFD

Brijesh Singh
 

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

When AMD SEV is enabled in the guest VM, a hypervisor need to insert a
secrets page.

When SEV-SNP is enabled, the secrets page contains the VM platform
communication keys. The guest BIOS and OS can use this key to communicate
with the SEV firmware to get attesation report. See the SEV-SNP firmware
spec for more details for the content of the secrets page.

When SEV and SEV-ES is enabled, the secrets page contains the information
provided by the guest owner after the attestation. See the SEV
LAUNCH_SECRET command for more details.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/OvmfPkgX64.dsc | 2 ++
OvmfPkg/OvmfPkgX64.fdf | 5 +++++
OvmfPkg/AmdSev/SecretPei/SecretPei.inf | 1 +
OvmfPkg/AmdSev/SecretPei/SecretPei.c | 15 ++++++++++++++-
4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 999738dc39cd..ea08e1fabc65 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -716,6 +716,7 @@ [Components]
OvmfPkg/SmmAccess/SmmAccessPei.inf
!endif
UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+ OvmfPkg/AmdSev/SecretPei/SecretPei.inf

!if $(TPM_ENABLE) == TRUE
OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
@@ -966,6 +967,7 @@ [Components]
OvmfPkg/PlatformDxe/Platform.inf
OvmfPkg/AmdSevDxe/AmdSevDxe.inf
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
+ OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf

!if $(SMM_REQUIRE) == TRUE
OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index d6be798fcadd..9126b8eb5014 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -88,6 +88,9 @@ [FD.MEMFD]
0x00C000|0x001000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize

+0x00D000|0x001000
+gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
+
0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize

@@ -179,6 +182,7 @@ [FV.PEIFV]
INF SecurityPkg/Tcg/TcgPei/TcgPei.inf
INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
!endif
+INF OvmfPkg/AmdSev/SecretPei/SecretPei.inf

################################################################################

@@ -314,6 +318,7 @@ [FV.DXEFV]
INF ShellPkg/Application/Shell/Shell.inf

INF MdeModulePkg/Logo/LogoDxe.inf
+INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf

#
# Network modules
diff --git a/OvmfPkg/AmdSev/SecretPei/SecretPei.inf b/OvmfPkg/AmdSev/SecretPei/SecretPei.inf
index 08be156c4bc0..9265f8adee12 100644
--- a/OvmfPkg/AmdSev/SecretPei/SecretPei.inf
+++ b/OvmfPkg/AmdSev/SecretPei/SecretPei.inf
@@ -26,6 +26,7 @@ [LibraryClasses]
HobLib
PeimEntryPoint
PcdLib
+ MemEncryptSevLib

[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
diff --git a/OvmfPkg/AmdSev/SecretPei/SecretPei.c b/OvmfPkg/AmdSev/SecretPei/SecretPei.c
index ad491515dd5d..51eb094555aa 100644
--- a/OvmfPkg/AmdSev/SecretPei/SecretPei.c
+++ b/OvmfPkg/AmdSev/SecretPei/SecretPei.c
@@ -7,6 +7,7 @@
#include <PiPei.h>
#include <Library/HobLib.h>
#include <Library/PcdLib.h>
+#include <Library/MemEncryptSevLib.h>

EFI_STATUS
EFIAPI
@@ -15,10 +16,22 @@ InitializeSecretPei (
IN CONST EFI_PEI_SERVICES **PeiServices
)
{
+ UINTN Type;
+
+ //
+ // The location of the secret page should be marked reserved so that guest OS
+ // does not treated as a system RAM.
+ //
+ if (MemEncryptSevSnpIsEnabled ()) {
+ Type = EfiReservedMemoryType;
+ } else {
+ Type = EfiBootServicesData;
+ }
+
BuildMemoryAllocationHob (
PcdGet32 (PcdSevLaunchSecretBase),
PcdGet32 (PcdSevLaunchSecretSize),
- EfiBootServicesData
+ Type
);

return EFI_SUCCESS;
--
2.17.1


[PATCH RFC v3 04/22] OvmfPkg/MemEncryptSevLib: extend Es Workarea to include hv features

Brijesh Singh
 

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

The GHCB Version 2 introduces advertisement of features that are supported
by the hypervisor. The features value is saved in the SevEs workarea. Save
the value in the PCD for the later use.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 2 +
OvmfPkg/PlatformPei/AmdSev.c | 26 +++++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 122 +++++++++++++++++++++
OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
5 files changed, 152 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index bc1dcac48343..3256ccfe88d8 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -111,6 +111,7 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled
gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled
+ gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures

[FixedPcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 24507de55c5d..dd1c97d4a9a3 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -55,6 +55,8 @@ typedef struct _SEC_SEV_ES_WORK_AREA {
UINT64 RandomData;

UINT64 EncryptionMask;
+
+ UINT64 HypervisorFeatures;
} SEC_SEV_ES_WORK_AREA;

//
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 67b78fd5fa36..81e40e0889aa 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -43,6 +43,27 @@ AmdSevSnpInitialize (
ASSERT_RETURN_ERROR (PcdStatus);
}

+/**
+
+ Function to set the PcdHypervisorFeatures.
+**/
+STATIC
+VOID
+AmdSevHypervisorFeatures (
+ VOID
+ )
+{
+ SEC_SEV_ES_WORK_AREA *SevEsWorkArea;
+ RETURN_STATUS PcdStatus;
+
+ SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
+
+ PcdStatus = PcdSet64S (PcdGhcbHypervisorFeatures, SevEsWorkArea->HypervisorFeatures);
+ ASSERT_RETURN_ERROR (PcdStatus);
+
+ DEBUG ((DEBUG_INFO, "GHCB Hypervisor Features=0x%Lx\n", SevEsWorkArea->HypervisorFeatures));
+}
+
/**

Initialize SEV-ES support if running as an SEV-ES guest.
@@ -73,6 +94,11 @@ AmdSevEsInitialize (
PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
ASSERT_RETURN_ERROR (PcdStatus);

+ //
+ // Set the hypervisor features PCD.
+ //
+ AmdSevHypervisorFeatures ();
+
//
// Allocate GHCB and per-CPU variable pages.
// Since the pages must survive across the UEFI to OS transition
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 6838cdeec9c3..75e63d2a0561 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -62,6 +62,16 @@ BITS 32
%define GHCB_CPUID_REGISTER_SHIFT 30
%define CPUID_INSN_LEN 2

+; GHCB SEV Information MSR protocol
+%define GHCB_SEV_INFORMATION_REQUEST 2
+%define GHCB_SEV_INFORMATION_RESPONSE 1
+
+; GHCB Hypervisor features MSR protocol
+%define GHCB_HYPERVISOR_FEATURES_REQUEST 128
+%define GHCB_HYPERVISOR_FEATURES_RESPONSE 129
+
+; GHCB request to terminate protocol values
+%define GHCB_GENERAL_TERMINATE_REQUEST 255

; Check if Secure Encrypted Virtualization (SEV) features are enabled.
;
@@ -86,6 +96,13 @@ CheckSevFeatures:
; will set it to 1.
mov byte[SEV_ES_WORK_AREA_SNP], 0

+ ; Set the Hypervisor features field in the workarea to zero to communicate
+ ; to the hypervisor features to the SEC phase. The hypervisor feature is
+ ; filled during the call to CheckHypervisorFeatures.
+ mov eax, 0
+ mov dword[SEV_ES_WORK_AREA_HYPERVISOR_FEATURES], eax
+ mov dword[SEV_ES_WORK_AREA_HYPERVISOR_FEATURES + 4], eax
+
;
; Set up exception handlers to check for SEV-ES
; Load temporary RAM stack based on PCDs (see SevEsIdtVmmComm for
@@ -225,6 +242,106 @@ IsSevEsEnabled:
SevEsDisabled:
OneTimeCallRet IsSevEsEnabled

+; The version 2 of GHCB specification added the support to query the hypervisor features.
+; If the GHCB version is >=2 then read the hypervisor features.
+;
+; Modified: EAX, EBX, ECX, EDX
+;
+CheckHypervisorFeatures:
+ ; Get the SEV Information
+ ; Setup GHCB MSR
+ ; GHCB_MSR[11:0] = SEV information request
+ ;
+ mov edx, 0
+ mov eax, GHCB_SEV_INFORMATION_REQUEST
+ mov ecx, 0xc0010130
+ wrmsr
+
+ ;
+ ; Issue VMGEXIT - NASM doesn't support the vmmcall instruction in 32-bit
+ ; mode, so work around this by temporarily switching to 64-bit mode.
+ ;
+BITS 64
+ rep vmmcall
+BITS 32
+
+ ;
+ ; SEV Information Response GHCB MSR
+ ; GHCB_MSR[63:48] = Maximum protocol version
+ ; GHCB_MSR[47:32] = Minimum protocol version
+ ; GHCB_MSR[11:0] = SEV information response
+ ;
+ mov ecx, 0xc0010130
+ rdmsr
+ and eax, 0xfff
+ cmp eax, GHCB_SEV_INFORMATION_RESPONSE
+ jnz TerminateSevGuestLaunch
+ shr edx, 16
+ cmp edx, 2
+ jl CheckHypervisorFeaturesDone
+
+ ; Get the hypervisor features
+ ; Setup GHCB MSR
+ ; GHCB_MSR[11:0] = Hypervisor features request
+ ;
+ mov edx, 0
+ mov eax, GHCB_HYPERVISOR_FEATURES_REQUEST
+ mov ecx, 0xc0010130
+ wrmsr
+
+ ;
+ ; Issue VMGEXIT - NASM doesn't support the vmmcall instruction in 32-bit
+ ; mode, so work around this by temporarily switching to 64-bit mode.
+ ;
+BITS 64
+ rep vmmcall
+BITS 32
+
+ ;
+ ; Hypervisor features reponse
+ ; GHCB_MSR[63:12] = Features bitmap
+ ; GHCB_MSR[11:0] = Hypervisor features response
+ ;
+ mov ecx, 0xc0010130
+ rdmsr
+ mov ebx, eax
+ and eax, 0xfff
+ cmp eax, GHCB_HYPERVISOR_FEATURES_RESPONSE
+ jnz TerminateSevGuestLaunch
+
+ shr ebx, 12
+ mov dword[SEV_ES_WORK_AREA_HYPERVISOR_FEATURES], ebx
+ mov dword[SEV_ES_WORK_AREA_HYPERVISOR_FEATURES + 4], edx
+
+ jmp CheckHypervisorFeaturesDone
+TerminateSevGuestLaunch:
+ ;
+ ; Setup GHCB MSR
+ ; GHCB_MSR[23:16] = 0
+ ; GHCB_MSR[15:12] = 0
+ ; GHCB_MSR[11:0] = Terminate Request
+ ;
+ mov edx, 0
+ mov eax, GHCB_GENERAL_TERMINATE_REQUEST
+ mov ecx, 0xc0010130
+ wrmsr
+
+ ;
+ ; Issue VMGEXIT - NASM doesn't support the vmmcall instruction in 32-bit
+ ; mode, so work around this by temporarily switching to 64-bit mode.
+ ;
+BITS 64
+ rep vmmcall
+BITS 32
+
+TerminateSevGuestLaunchHlt:
+ cli
+ hlt
+ jmp TerminateSevGuestLaunchHlt
+
+CheckHypervisorFeaturesDone:
+ OneTimeCallRet CheckHypervisorFeatures
+
;
; Modified: EAX, EBX, ECX, EDX
;
@@ -328,6 +445,11 @@ clearGhcbMemoryLoop:
mov dword[ecx * 4 + GHCB_BASE - 4], eax
loop clearGhcbMemoryLoop

+ ;
+ ; It is SEV-ES guest, query the Hypervisor features
+ ;
+ OneTimeCall CheckHypervisorFeatures
+
SetCr3:
;
; Set CR3 now that the paging structures are available
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 1971557b1c00..5beba3ecb290 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -76,6 +76,7 @@
%define SEV_ES_WORK_AREA_SNP (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 1)
%define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 8)
%define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 16)
+ %define SEV_ES_WORK_AREA_HYPERVISOR_FEATURES (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 24)
%define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) + FixedPcdGet32 (PcdOvmfSecPeiTempRamSize))
%include "Ia32/Flat32ToFlat64.asm"
%include "Ia32/PageTables64.asm"
--
2.17.1


[PATCH RFC v3 03/22] OvmfPkg/MemEncryptSevLib: extend the workarea to include SNP enabled field

Brijesh Singh
 

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

Extend the workarea to include the SEV-SNP enabled fields. This will be set
when SEV-SNP is active in the guest VM.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 3 ++-
OvmfPkg/PlatformPei/AmdSev.c | 26 ++++++++++++++++++++++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 12 ++++++++++
OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 6ef77ba7bb21..bc1dcac48343 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -110,6 +110,7 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber
gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled
+ gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled

[FixedPcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 2425d8ba0a36..24507de55c5d 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -49,7 +49,8 @@ typedef struct {
//
typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
- UINT8 Reserved1[7];
+ UINT8 SevSnpEnabled;
+ UINT8 Reserved2[6];

UINT64 RandomData;

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index a8bf610022ba..67b78fd5fa36 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -22,6 +22,27 @@

#include "Platform.h"

+/**
+
+ Initialize SEV-SNP support if running as an SEV-SNP guest.
+
+ **/
+STATIC
+VOID
+AmdSevSnpInitialize (
+ VOID
+ )
+{
+ RETURN_STATUS PcdStatus;
+
+ if (!MemEncryptSevSnpIsEnabled ()) {
+ return;
+ }
+
+ PcdStatus = PcdSetBoolS (PcdSevSnpIsEnabled, TRUE);
+ ASSERT_RETURN_ERROR (PcdStatus);
+}
+
/**

Initialize SEV-ES support if running as an SEV-ES guest.
@@ -209,4 +230,9 @@ AmdSevInitialize (
// Check and perform SEV-ES initialization if required.
//
AmdSevEsInitialize ();
+
+ //
+ // Check and perform SEV-SNP initialization if required.
+ //
+ AmdSevSnpInitialize ();
}
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 5fae8986d9da..6838cdeec9c3 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -81,6 +81,11 @@ CheckSevFeatures:
; the MSR check below will set the first byte of the workarea to one.
mov byte[SEV_ES_WORK_AREA], 0

+ ; Set the SevSnpEnabled field in workarea to zero to communicate to the SEC
+ ; phase that SEV-SNP is not enabled. If SEV-SNP is enabled, this function
+ ; will set it to 1.
+ mov byte[SEV_ES_WORK_AREA_SNP], 0
+
;
; Set up exception handlers to check for SEV-ES
; Load temporary RAM stack based on PCDs (see SevEsIdtVmmComm for
@@ -136,6 +141,13 @@ CheckSevFeatures:
; phase that SEV-ES is enabled.
mov byte[SEV_ES_WORK_AREA], 1

+ bt eax, 2
+ jnc GetSevEncBit
+
+ ; Set the second byte of the workarea to one to communicate to the SEC
+ ; phase that the SEV-SNP is enabled
+ mov byte[SEV_ES_WORK_AREA_SNP], 1
+
GetSevEncBit:
; Get pte bit position to enable memory encryption
; CPUID Fn8000_001F[EBX] - Bits 5:0
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 5fbacaed5f9d..1971557b1c00 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -73,6 +73,7 @@
%define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase))
%define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize))
%define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase))
+ %define SEV_ES_WORK_AREA_SNP (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 1)
%define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 8)
%define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 16)
%define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) + FixedPcdGet32 (PcdOvmfSecPeiTempRamSize))
--
2.17.1


[PATCH RFC v3 02/22] OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()

Brijesh Singh
 

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

Create a function that can be used to determine if VM is running as an
SEV-SNP guest.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/Include/Library/MemEncryptSevLib.h | 12 +++++++++
.../DxeMemEncryptSevLibInternal.c | 27 +++++++++++++++++++
.../PeiMemEncryptSevLibInternal.c | 27 +++++++++++++++++++
.../SecMemEncryptSevLibInternal.c | 19 +++++++++++++
4 files changed, 85 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 76d06c206c8b..2425d8ba0a36 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -66,6 +66,18 @@ typedef enum {
MemEncryptSevAddressRangeError,
} MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE;

+/**
+ Returns a boolean to indicate whether SEV-SNP is enabled
+
+ @retval TRUE SEV-SNP is enabled
+ @retval FALSE SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+ VOID
+ );
+
/**
Returns a boolean to indicate whether SEV-ES is enabled.

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
index 2816f859a0c4..057129723824 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
@@ -19,6 +19,7 @@

STATIC BOOLEAN mSevStatus = FALSE;
STATIC BOOLEAN mSevEsStatus = FALSE;
+STATIC BOOLEAN mSevSnpStatus = FALSE;
STATIC BOOLEAN mSevStatusChecked = FALSE;

STATIC UINT64 mSevEncryptionMask = 0;
@@ -82,11 +83,37 @@ InternalMemEncryptSevStatus (
if (Msr.Bits.SevEsBit) {
mSevEsStatus = TRUE;
}
+
+ //
+ // Check MSR_0xC0010131 Bit 2 (Sev-Snp Enabled)
+ //
+ if (Msr.Bits.SevSnpBit) {
+ mSevSnpStatus = TRUE;
+ }
}

mSevStatusChecked = TRUE;
}

+/**
+ Returns a boolean to indicate whether SEV-SNP is enabled.
+
+ @retval TRUE SEV-SNP is enabled
+ @retval FALSE SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+ VOID
+ )
+{
+ if (!mSevStatusChecked) {
+ InternalMemEncryptSevStatus ();
+ }
+
+ return mSevSnpStatus;
+}
+
/**
Returns a boolean to indicate whether SEV-ES is enabled.

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
index e2fd109d120f..b561f211f577 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
@@ -19,6 +19,7 @@

STATIC BOOLEAN mSevStatus = FALSE;
STATIC BOOLEAN mSevEsStatus = FALSE;
+STATIC BOOLEAN mSevSnpStatus = FALSE;
STATIC BOOLEAN mSevStatusChecked = FALSE;

STATIC UINT64 mSevEncryptionMask = 0;
@@ -82,11 +83,37 @@ InternalMemEncryptSevStatus (
if (Msr.Bits.SevEsBit) {
mSevEsStatus = TRUE;
}
+
+ //
+ // Check MSR_0xC0010131 Bit 2 (Sev-Snp Enabled)
+ //
+ if (Msr.Bits.SevSnpBit) {
+ mSevSnpStatus = TRUE;
+ }
}

mSevStatusChecked = TRUE;
}

+/**
+ Returns a boolean to indicate whether SEV-SNP is enabled.
+
+ @retval TRUE SEV-SNP is enabled
+ @retval FALSE SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+ VOID
+ )
+{
+ if (!mSevStatusChecked) {
+ InternalMemEncryptSevStatus ();
+ }
+
+ return mSevSnpStatus;
+}
+
/**
Returns a boolean to indicate whether SEV-ES is enabled.

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
index 56d8f3f3183f..69852779e2ff 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -62,6 +62,25 @@ InternalMemEncryptSevStatus (
return ReadSevMsr ? AsmReadMsr32 (MSR_SEV_STATUS) : 0;
}

+/**
+ Returns a boolean to indicate whether SEV-SNP is enabled.
+
+ @retval TRUE SEV-SNP is enabled
+ @retval FALSE SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+ VOID
+ )
+{
+ MSR_SEV_STATUS_REGISTER Msr;
+
+ Msr.Uint32 = InternalMemEncryptSevStatus ();
+
+ return Msr.Bits.SevSnpBit ? TRUE : FALSE;
+}
+
/**
Returns a boolean to indicate whether SEV-ES is enabled.

--
2.17.1


[PATCH RFC v3 01/22] UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs

Brijesh Singh
 

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

Define the PCDs used by the MpLib while creating the AP when SEV-SNP is
active in the guest VMs.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
UefiCpuPkg/UefiCpuPkg.dec | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 62acb291f309..0ec25871a50f 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -396,5 +396,16 @@ [PcdsDynamic, PcdsDynamicEx]
# @Prompt SEV-ES Status
gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled|FALSE|BOOLEAN|0x60000016

+ ## This dynamic PCD indicates whether SEV-SNP is enabled
+ # TRUE - SEV-SNP is enabled
+ # FALSE - SEV-SNP is not enabled
+ # @Prompt SEV-SNP Status
+ gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled|FALSE|BOOLEAN|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.17.1


[RESEND PATCH RFC v3 00/22] Add AMD Secure Nested Paging (SEV-SNP) support

Brijesh Singh
 

(I missed adding devel@edk2.groups.io, resending the series)

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

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

The series builds on SNP pre-patch posted here: https://tinyurl.com/pu6admks

Additional resources
---------------------
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf

APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)

The complete source is available at
https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-2

GHCB spec:
https://developer.amd.com/wp-content/resources/56421.pdf

SEV-SNP firmware specification:
https://developer.amd.com/sev/

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>

Changes since v2:
* Add support for the AP creation.
* Use the module-scoping override to make AmdSevDxe use the IO port for PCI reads.
* Use the reserved memory type for CPUID and Secrets page.
*
Changes since v1:
* Drop the interval tree support to detect the pre-validated overlap region.
* Use an array to keep track of pre-validated regions.
* Add support to query the Hypervisor feature and verify that SNP feature is supported.
* Introduce MemEncryptSevClearMmioPageEncMask() to clear the C-bit from MMIO ranges.
* Pull the SevSecretDxe and SevSecretPei into OVMF package build.
* Extend the SevSecretDxe to expose confidential computing blob location through
EFI configuration table.

Brijesh Singh (21):
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/MemEncryptSevLib: extend the workarea to include SNP enabled
field
OvmfPkg/MemEncryptSevLib: extend Es Workarea to include hv features
OvmfPkg: reserve Secrets page in MEMFD
OvmfPkg: reserve CPUID page for the SEV-SNP guest
OvmfPkg/ResetVector: validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
OvmfPkg: add library to support registering GHCB GPA
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/MemEncryptSevLib: Change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table
MdePkg/GHCB: increase the GHCB protocol max version

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 21 ++
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 2 +
OvmfPkg/OvmfPkgIa32X64.dsc | 7 +-
OvmfPkg/OvmfPkgX64.dsc | 8 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 17 +-
OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf | 4 +
OvmfPkg/AmdSev/SecretPei/SecretPei.inf | 1 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
.../GhcbRegisterLib/GhcbRegisterLib.inf | 33 +++
OvmfPkg/PlatformPei/PlatformPei.inf | 5 +
OvmfPkg/ResetVector/ResetVector.inf | 4 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 ++
OvmfPkg/Include/Library/GhcbRegisterLib.h | 27 ++
OvmfPkg/Include/Library/MemEncryptSevLib.h | 31 +-
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 ++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 ++
OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 22 ++
OvmfPkg/AmdSev/SecretPei/SecretPei.c | 15 +-
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 ++
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 ++
.../X64/DxeSnpSystemRamValidate.c | 40 +++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++++
.../X64/SecSnpSystemRamValidate.c | 36 +++
.../X64/SnpPageStateChangeInternal.c | 230 +++++++++++++++
.../Library/GhcbRegisterLib/GhcbRegisterLib.c | 97 +++++++
OvmfPkg/PlatformPei/AmdSev.c | 81 ++++++
OvmfPkg/PlatformPei/MemDetect.c | 12 +
OvmfPkg/Sec/SecMain.c | 106 +++++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 274 ++++++++++++++++--
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 +++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 23 ++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 227 +++++++++++++++
OvmfPkg/ResetVector/ResetVector.nasmb | 6 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 ++++
52 files changed, 1956 insertions(+), 38 deletions(-)
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
create mode 100644 OvmfPkg/Include/Library/GhcbRegisterLib.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c

--
2.17.1


Re: [PATCH 30/43] OvmfPkg/Bhyve: consume PciHostBridgeLibScan

Rebecca Cran
 

On 5/26/21 2:14 PM, Laszlo Ersek wrote:
Switch the Bhyve platform from the "OvmfPkg/PciHostBridgeLib" instance to
the "OvmfPkg/PciHostBridgeLibScan" instance. Currently this is a no-op
functionally; we'll customize the "PciHostBridgeLibScan" instance later.

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Peter Grehan <grehan@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Cc: Rebecca Cran <rebecca@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 30a99e2a3425..d8792812ab21 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -663,13 +663,13 @@ [Components]
UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
UefiCpuPkg/CpuDxe/CpuDxe.inf
PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
<LibraryClasses>
- PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
+ PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLibScan/PciHostBridgeLibScan.inf
PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
}
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
<LibraryClasses>
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
Reviewed-by: Rebecca Cran <rebecca@...>


[PATCH 43/43] OvmfPkg: restrict XenPlatformLib to BdsDxe in the IA32, IA32X64, X64 DSCs

Laszlo Ersek
 

The "OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf"
library instance is used in the following platform DSC files in edk2:

OvmfPkg/OvmfPkgIa32.dsc
OvmfPkg/OvmfPkgIa32X64.dsc
OvmfPkg/OvmfPkgX64.dsc
OvmfPkg/OvmfXen.dsc

The Xen customizations are very light-weight in this
PlatformBootManagerLib instance. Isolating them statically, for the sake
of the first three DSC files, would save negligible binary code size, and
would likely worsen code complexity (by way of introducing new internal
interfaces) or blow up source code size (by duplicating almost the entire
lib instance source code). So for now, keep this one bit of Xen dynamism
even on QEMU.

However, because it's only PlatformBootManagerLib now that uses
XenPlatformLib (for the above-stated enlightenment), restrict the
XenPlatformLib class resolution in the first three DSC files to the only
DXE driver that consumes PlatformBootManagerLib (and therefore
XenPlatformLib): BdsDxe. This will cause a build failure later if someone
attempts to call a XenPlatformLib API (that is, tries to re-introduce Xen
enlightenment) in a different module in these non-Xen DSC files.

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/OvmfPkgIa32.dsc | 2 +-
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
OvmfPkg/OvmfPkgX64.dsc | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 7a37efd35664..f53efeae7986 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -223,13 +223,12 @@ [LibraryClasses]

ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
- XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf

!if $(TPM_ENABLE) == TRUE
Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf
Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
@@ -767,12 +766,13 @@ [Components]
MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
MdeModulePkg/Universal/Metronome/Metronome.inf
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
MdeModulePkg/Universal/BdsDxe/BdsDxe.inf {
<LibraryClasses>
+ XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
!ifdef $(CSM_ENABLE)
NULL|OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf
NULL|OvmfPkg/Csm/LegacyBootManagerLib/LegacyBootManagerLib.inf
!endif
}
MdeModulePkg/Logo/LogoDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index d6cc58a261d9..b3662e17f256 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -227,13 +227,12 @@ [LibraryClasses]

ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
- XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf

!if $(TPM_ENABLE) == TRUE
Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf
Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
@@ -781,12 +780,13 @@ [Components.X64]
MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
MdeModulePkg/Universal/Metronome/Metronome.inf
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
MdeModulePkg/Universal/BdsDxe/BdsDxe.inf {
<LibraryClasses>
+ XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
!ifdef $(CSM_ENABLE)
NULL|OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf
NULL|OvmfPkg/Csm/LegacyBootManagerLib/LegacyBootManagerLib.inf
!endif
}
MdeModulePkg/Logo/LogoDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index ab60c36eca27..0a237a905866 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -227,13 +227,12 @@ [LibraryClasses]

ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
- XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf

!if $(TPM_ENABLE) == TRUE
Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf
Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
@@ -779,12 +778,13 @@ [Components]
MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
MdeModulePkg/Universal/Metronome/Metronome.inf
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
MdeModulePkg/Universal/BdsDxe/BdsDxe.inf {
<LibraryClasses>
+ XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
!ifdef $(CSM_ENABLE)
NULL|OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf
NULL|OvmfPkg/Csm/LegacyBootManagerLib/LegacyBootManagerLib.inf
!endif
}
MdeModulePkg/Logo/LogoDxe.inf
--
2.19.1.3.g30247aa5d201


[PATCH 42/43] OvmfPkg/SmbiosPlatformDxe: split Xen entry point from QEMU entry point

Laszlo Ersek
 

Remove the SmbiosTablePublishEntry() function from "SmbiosPlatformDxe.c".
"SmbiosPlatformDxe.c" becomes hypervisor-agnostic.

Add SmbiosTablePublishEntry() back, simplified for QEMU, to the existent
file "Qemu.c". The GetQemuSmbiosTables() function no longer needs to be
declared in "SmbiosPlatformDxe.h"; "SmbiosPlatformDxe.h" becomes
hypervisor-agnostic.

Add SmbiosTablePublishEntry() back, renamed and simplified for Xen, to the
new, arch-independent file "Xen.c". (The existent Xen-specific C files are
arch-dependent.)

Update both INF files; remove the dependencies that are now superfluous in
each.

Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 14 ------
OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf | 12 ++---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 11 -----
OvmfPkg/SmbiosPlatformDxe/Qemu.c | 33 +++++++++++++
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 49 --------------------
OvmfPkg/SmbiosPlatformDxe/Xen.c | 49 ++++++++++++++++++++
6 files changed, 86 insertions(+), 82 deletions(-)

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 140fa16ac135..eaee73110d27 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -24,19 +24,12 @@ [Defines]
#

[Sources]
Qemu.c
SmbiosPlatformDxe.c
SmbiosPlatformDxe.h
- XenSmbiosPlatformDxe.h
-
-[Sources.IA32, Sources.X64]
- X86Xen.c
-
-[Sources.ARM, Sources.AARCH64]
- ArmXen.c

[Packages]
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec

[LibraryClasses]
@@ -44,22 +37,15 @@ [LibraryClasses]
MemoryAllocationLib
PcdLib
QemuFwCfgLib
UefiBootServicesTableLib
UefiDriverEntryPoint

-[LibraryClasses.IA32, LibraryClasses.X64]
- BaseLib
- HobLib
-
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated

[Protocols]
gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED

-[Guids.IA32, Guids.X64]
- gEfiXenInfoGuid
-
[Depex]
gEfiSmbiosProtocolGuid

diff --git a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
index 5a093c69afd9..7f4588e33d1e 100644
--- a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
@@ -13,51 +13,47 @@ [Defines]
INF_VERSION = 0x00010005
BASE_NAME = XenSmbiosPlatformDxe
FILE_GUID = c41f0579-5598-40f1-95db-3983c8ebbe2a
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0

- ENTRY_POINT = SmbiosTablePublishEntry
+ ENTRY_POINT = XenSmbiosTablePublishEntry

#
# The following information is for reference only and not required by the build tools.
#
# VALID_ARCHITECTURES = IA32 X64 EBC ARM AARCH64
#

[Sources]
- Qemu.c
SmbiosPlatformDxe.c
SmbiosPlatformDxe.h
+ Xen.c
XenSmbiosPlatformDxe.h

[Sources.IA32, Sources.X64]
X86Xen.c

[Sources.ARM, Sources.AARCH64]
ArmXen.c

[Packages]
MdePkg/MdePkg.dec
+
+[Packages.IA32, Packages.X64]
OvmfPkg/OvmfPkg.dec

[LibraryClasses]
DebugLib
- MemoryAllocationLib
- PcdLib
- QemuFwCfgLib
UefiBootServicesTableLib
UefiDriverEntryPoint

[LibraryClasses.IA32, LibraryClasses.X64]
BaseLib
HobLib

-[Pcd]
- gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated
-
[Protocols]
gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED

[Guids.IA32, Guids.X64]
gEfiXenInfoGuid

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 0ae2556fe800..213a7f39e91d 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -19,18 +19,7 @@
**/
EFI_STATUS
InstallAllStructures (
IN UINT8 *TableAddress
);

-/**
- Locates and extracts the QEMU SMBIOS table data if present in fw_cfg
-
- @return Address of extracted QEMU SMBIOS data
-
-**/
-UINT8 *
-GetQemuSmbiosTables (
- VOID
- );
-
#endif
diff --git a/OvmfPkg/SmbiosPlatformDxe/Qemu.c b/OvmfPkg/SmbiosPlatformDxe/Qemu.c
index fcfc3e33c28c..a668c6ac2123 100644
--- a/OvmfPkg/SmbiosPlatformDxe/Qemu.c
+++ b/OvmfPkg/SmbiosPlatformDxe/Qemu.c
@@ -45,6 +45,39 @@ GetQemuSmbiosTables (

QemuFwCfgSelectItem (Tables);
QemuFwCfgReadBytes (TablesSize, QemuTables);

return QemuTables;
}
+
+/**
+ Installs SMBIOS information for OVMF
+
+ @param ImageHandle Module's image handle
+ @param SystemTable Pointer of EFI_SYSTEM_TABLE
+
+ @retval EFI_SUCCESS Smbios data successfully installed
+ @retval Other Smbios data was not installed
+
+**/
+EFI_STATUS
+EFIAPI
+SmbiosTablePublishEntry (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+ UINT8 *SmbiosTables;
+
+ Status = EFI_NOT_FOUND;
+ //
+ // Add QEMU SMBIOS data if found
+ //
+ SmbiosTables = GetQemuSmbiosTables ();
+ if (SmbiosTables != NULL) {
+ Status = InstallAllStructures (SmbiosTables);
+ FreePool (SmbiosTables);
+ }
+
+ return Status;
+}
diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index f280a1852ddd..7bcf83762e9b 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -7,18 +7,16 @@
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

#include <IndustryStandard/SmBios.h> // SMBIOS_TABLE_TYPE0
#include <Library/DebugLib.h> // ASSERT_EFI_ERROR()
-#include <Library/MemoryAllocationLib.h> // FreePool()
#include <Library/UefiBootServicesTableLib.h> // gBS
#include <Protocol/Smbios.h> // EFI_SMBIOS_PROTOCOL

#include "SmbiosPlatformDxe.h"
-#include "XenSmbiosPlatformDxe.h"

#define TYPE0_STRINGS \
"EFI Development Kit II / OVMF\0" /* Vendor */ \
"0.0.0\0" /* BiosVersion */ \
"02/06/2015\0" /* BiosReleaseDate */
//
@@ -165,53 +163,6 @@ InstallAllStructures (
);
ASSERT_EFI_ERROR (Status);
}

return EFI_SUCCESS;
}
-
-
-/**
- Installs SMBIOS information for OVMF
-
- @param ImageHandle Module's image handle
- @param SystemTable Pointer of EFI_SYSTEM_TABLE
-
- @retval EFI_SUCCESS Smbios data successfully installed
- @retval Other Smbios data was not installed
-
-**/
-EFI_STATUS
-EFIAPI
-SmbiosTablePublishEntry (
- IN EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE *SystemTable
- )
-{
- EFI_STATUS Status;
- SMBIOS_TABLE_ENTRY_POINT *EntryPointStructure;
- UINT8 *SmbiosTables;
-
- Status = EFI_NOT_FOUND;
- //
- // Add Xen or QEMU SMBIOS data if found
- //
- EntryPointStructure = GetXenSmbiosTables ();
- if (EntryPointStructure != NULL) {
- SmbiosTables = (UINT8*)(UINTN)EntryPointStructure->TableAddress;
- } else {
- SmbiosTables = GetQemuSmbiosTables ();
- }
-
- if (SmbiosTables != NULL) {
- Status = InstallAllStructures (SmbiosTables);
-
- //
- // Free SmbiosTables if allocated by Qemu (i.e., NOT by Xen):
- //
- if (EntryPointStructure == NULL) {
- FreePool (SmbiosTables);
- }
- }
-
- return Status;
-}
diff --git a/OvmfPkg/SmbiosPlatformDxe/Xen.c b/OvmfPkg/SmbiosPlatformDxe/Xen.c
new file mode 100644
index 000000000000..75d9550913d1
--- /dev/null
+++ b/OvmfPkg/SmbiosPlatformDxe/Xen.c
@@ -0,0 +1,49 @@
+/** @file
+ This driver installs SMBIOS information for OVMF on Xen
+
+ Copyright (C) 2021, Red Hat, Inc.
+ Copyright (c) 2011, Bei Guan <gbtju85@...>
+ Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "SmbiosPlatformDxe.h"
+#include "XenSmbiosPlatformDxe.h"
+
+/**
+ Installs SMBIOS information for OVMF on Xen
+
+ @param ImageHandle Module's image handle
+ @param SystemTable Pointer of EFI_SYSTEM_TABLE
+
+ @retval EFI_SUCCESS Smbios data successfully installed
+ @retval Other Smbios data was not installed
+
+**/
+EFI_STATUS
+EFIAPI
+XenSmbiosTablePublishEntry (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+ SMBIOS_TABLE_ENTRY_POINT *EntryPointStructure;
+ UINT8 *SmbiosTables;
+
+ Status = EFI_NOT_FOUND;
+ //
+ // Add Xen SMBIOS data if found
+ //
+ EntryPointStructure = GetXenSmbiosTables ();
+ if (EntryPointStructure != NULL) {
+ SmbiosTables = (UINT8*)(UINTN)EntryPointStructure->TableAddress;
+ if (SmbiosTables != NULL) {
+ Status = InstallAllStructures (SmbiosTables);
+ }
+ }
+
+ return Status;
+}
--
2.19.1.3.g30247aa5d201


[PATCH 41/43] OvmfPkg/SmbiosPlatformDxe: create Xen-specific module INF file

Laszlo Ersek
 

"OvmfPkg/SmbiosPlatformDxe" is structured somewhat differently from the
drivers duplicated and trimmed thus far in this series. The final QEMU and
Xen versions will share a relatively significant amount of code, therefore
duplicating the whole driver is less useful, even temporarily. Instead,
duplicate the INF file, in preparation for customizing the entry point
function.

Because ArmVirtXen doesn't actually include OvmfPkg/SmbiosPlatformDxe [*],
there is only one platform that's supposed to consume the new driver:
OvmfXen. Switch OvmfXen to the new driver at once.

[*] See commit 164cf4038357 ("OvmfPkg: SmbiosPlatformDxe: restrict current
Xen code to IA32/X64", 2015-07-26).

This patch is best viewed with "git show --find-copies-harder".

Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/OvmfXen.dsc | 2 +-
OvmfPkg/OvmfXen.fdf | 2 +-
OvmfPkg/SmbiosPlatformDxe/{SmbiosPlatformDxe.inf => XenSmbiosPlatformDxe.inf} | 7 ++++---
3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 0986d9f5c356..3c1ca6bfd493 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -642,13 +642,13 @@ [Components]
# SMBIOS Support
#
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
<LibraryClasses>
NULL|OvmfPkg/Library/SmbiosVersionLib/DetectSmbiosVersionLib.inf
}
- OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+ OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf

#
# ACPI Support
#
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
OvmfPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf
index 9acc7c93b98b..aeb9336fd5b7 100644
--- a/OvmfPkg/OvmfXen.fdf
+++ b/OvmfPkg/OvmfXen.fdf
@@ -345,13 +345,13 @@ [FV.DXEFV]
!if $(SOURCE_DEBUG_ENABLE) == FALSE
INF MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
!endif
INF MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf

INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
-INF OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+INF OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf

INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
INF OvmfPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
INF MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
INF MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
similarity index 80%
copy from OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
copy to OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
index 140fa16ac135..5a093c69afd9 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
@@ -1,20 +1,21 @@
## @file
-# This driver installs SMBIOS information for OVMF
+# This driver installs SMBIOS information for OVMF on Xen
#
+# Copyright (C) 2021, Red Hat, Inc.
# Copyright (c) 2011, Bei Guan <gbtju85@...>
# Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##

[Defines]
INF_VERSION = 0x00010005
- BASE_NAME = SmbiosPlatformDxe
- FILE_GUID = 4110465d-5ff3-4f4b-b580-24ed0d06747a
+ BASE_NAME = XenSmbiosPlatformDxe
+ FILE_GUID = c41f0579-5598-40f1-95db-3983c8ebbe2a
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0

ENTRY_POINT = SmbiosTablePublishEntry

#
--
2.19.1.3.g30247aa5d201


[PATCH 40/43] OvmfPkg/SmbiosPlatformDxe: declare InstallAllStructures() in header file

Laszlo Ersek
 

Add an extern declaration for the InstallAllStructures() function to the
"SmbiosPlatformDxe.h" header file. (The leading comment block and the
prototype are simply copied from "SmbiosPlatformDxe.c".)

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 7a0bdbb2911f..0ae2556fe800 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -8,12 +8,23 @@

**/

#ifndef SMBIOS_PLATFORM_DXE_H_
#define SMBIOS_PLATFORM_DXE_H_

+/**
+ Install all structures from the given SMBIOS structures block
+
+ @param TableAddress SMBIOS tables starting address
+
+**/
+EFI_STATUS
+InstallAllStructures (
+ IN UINT8 *TableAddress
+ );
+
/**
Locates and extracts the QEMU SMBIOS table data if present in fw_cfg

@return Address of extracted QEMU SMBIOS data

**/
--
2.19.1.3.g30247aa5d201


[PATCH 39/43] OvmfPkg/SmbiosPlatformDxe: split GetXenSmbiosTables() decl. to new header

Laszlo Ersek
 

Move the declaration of the GetXenSmbiosTables() function to a new header
file called "XenSmbiosPlatformDxe.h". (The only declaration that remains
in "SmbiosPlatformDxe.h" for now is that of GetQemuSmbiosTables().)

Modify the pattern in "Maintainers.txt" so that the new file be covered in
the "OvmfPkg: Xen-related modules" section.

This patch is best viewed with "git show --no-renames".

Cc: Andrew Fish <afish@...>
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Maintainers.txt | 2 +-
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 14 --------------
OvmfPkg/SmbiosPlatformDxe/{ArmXen.c => XenSmbiosPlatformDxe.h} | 20 ++++++++++----------
OvmfPkg/SmbiosPlatformDxe/ArmXen.c | 2 +-
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
OvmfPkg/SmbiosPlatformDxe/X86Xen.c | 2 +-
7 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 6063c0c9f609..140fa16ac135 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -24,12 +24,13 @@ [Defines]
#

[Sources]
Qemu.c
SmbiosPlatformDxe.c
SmbiosPlatformDxe.h
+ XenSmbiosPlatformDxe.h

[Sources.IA32, Sources.X64]
X86Xen.c

[Sources.ARM, Sources.AARCH64]
ArmXen.c
diff --git a/Maintainers.txt b/Maintainers.txt
index e5f419e67f0c..751477e8e62a 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -499,13 +499,13 @@ F: OvmfPkg/Library/XenConsoleSerialPortLib/
F: OvmfPkg/Library/XenHypercallLib/
F: OvmfPkg/Library/XenIoMmioLib/
F: OvmfPkg/Library/XenPlatformLib/
F: OvmfPkg/Library/XenRealTimeClockLib/
F: OvmfPkg/OvmfXen.*
F: OvmfPkg/OvmfXenElfHeaderGenerator.c
-F: OvmfPkg/SmbiosPlatformDxe/*Xen.c
+F: OvmfPkg/SmbiosPlatformDxe/*Xen*
F: OvmfPkg/XenAcpiPlatformDxe/
F: OvmfPkg/XenBusDxe/
F: OvmfPkg/XenIoPciDxe/
F: OvmfPkg/XenIoPvhDxe/
F: OvmfPkg/XenPlatformPei/
F: OvmfPkg/XenPvBlkDxe/
diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index ad42a326418c..7a0bdbb2911f 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -8,26 +8,12 @@

**/

#ifndef SMBIOS_PLATFORM_DXE_H_
#define SMBIOS_PLATFORM_DXE_H_

-#include <IndustryStandard/SmBios.h> // SMBIOS_TABLE_ENTRY_POINT
-
-/**
- Locates the Xen SMBIOS data if it exists
-
- @return SMBIOS_TABLE_ENTRY_POINT Address of Xen SMBIOS data
-
-**/
-SMBIOS_TABLE_ENTRY_POINT *
-GetXenSmbiosTables (
- VOID
- );
-
-
/**
Locates and extracts the QEMU SMBIOS table data if present in fw_cfg

@return Address of extracted QEMU SMBIOS data

**/
diff --git a/OvmfPkg/SmbiosPlatformDxe/ArmXen.c b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.h
similarity index 56%
copy from OvmfPkg/SmbiosPlatformDxe/ArmXen.c
copy to OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.h
index c2847f905303..850a8b662cd5 100644
--- a/OvmfPkg/SmbiosPlatformDxe/ArmXen.c
+++ b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.h
@@ -1,28 +1,28 @@
/** @file
- Detect Xen SMBIOS data on ARM / AARCH64.
+ This driver installs SMBIOS information for OVMF on Xen

- Copyright (C) 2015, Red Hat, Inc.
+ Copyright (C) 2021, Red Hat, Inc.
Copyright (c) 2011, Bei Guan <gbtju85@...>
Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent
+
**/

-#include "SmbiosPlatformDxe.h"
+#ifndef XEN_SMBIOS_PLATFORM_DXE_H_
+#define XEN_SMBIOS_PLATFORM_DXE_H_
+
+#include <IndustryStandard/SmBios.h> // SMBIOS_TABLE_ENTRY_POINT

/**
Locates the Xen SMBIOS data if it exists

@return SMBIOS_TABLE_ENTRY_POINT Address of Xen SMBIOS data

**/
SMBIOS_TABLE_ENTRY_POINT *
GetXenSmbiosTables (
VOID
- )
-{
- //
- // Not implemented yet.
- //
- return NULL;
-}
+ );
+
+#endif
diff --git a/OvmfPkg/SmbiosPlatformDxe/ArmXen.c b/OvmfPkg/SmbiosPlatformDxe/ArmXen.c
index c2847f905303..3dd849bbc0b4 100644
--- a/OvmfPkg/SmbiosPlatformDxe/ArmXen.c
+++ b/OvmfPkg/SmbiosPlatformDxe/ArmXen.c
@@ -5,13 +5,13 @@
Copyright (c) 2011, Bei Guan <gbtju85@...>
Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent
**/

-#include "SmbiosPlatformDxe.h"
+#include "XenSmbiosPlatformDxe.h"

/**
Locates the Xen SMBIOS data if it exists

@return SMBIOS_TABLE_ENTRY_POINT Address of Xen SMBIOS data

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 757bec879e4a..f280a1852ddd 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -12,12 +12,13 @@
#include <Library/DebugLib.h> // ASSERT_EFI_ERROR()
#include <Library/MemoryAllocationLib.h> // FreePool()
#include <Library/UefiBootServicesTableLib.h> // gBS
#include <Protocol/Smbios.h> // EFI_SMBIOS_PROTOCOL

#include "SmbiosPlatformDxe.h"
+#include "XenSmbiosPlatformDxe.h"

#define TYPE0_STRINGS \
"EFI Development Kit II / OVMF\0" /* Vendor */ \
"0.0.0\0" /* BiosVersion */ \
"02/06/2015\0" /* BiosReleaseDate */
//
diff --git a/OvmfPkg/SmbiosPlatformDxe/X86Xen.c b/OvmfPkg/SmbiosPlatformDxe/X86Xen.c
index e0b1b29f80db..0acedf8995da 100644
--- a/OvmfPkg/SmbiosPlatformDxe/X86Xen.c
+++ b/OvmfPkg/SmbiosPlatformDxe/X86Xen.c
@@ -9,13 +9,13 @@
**/

#include <Library/BaseLib.h> // AsciiStrnCmp()
#include <Library/HobLib.h> // GetFirstGuidHob()
#include <Pi/PiHob.h> // EFI_HOB_GUID_TYPE

-#include "SmbiosPlatformDxe.h"
+#include "XenSmbiosPlatformDxe.h"

#define XEN_SMBIOS_PHYSICAL_ADDRESS 0x000EB000
#define XEN_SMBIOS_PHYSICAL_END 0x000F0000

/**
Validates the SMBIOS entry point structure
--
2.19.1.3.g30247aa5d201


[PATCH 38/43] OvmfPkg/SmbiosPlatformDxe: locate SMBIOS protocol in InstallAllStructures()

Laszlo Ersek
 

Locate the SMBIOS protocol internally to the InstallAllStructures()
function. This has no performance impact (InstallAllStructures() is only
called once), but moving the code from the entry point function makes the
latter smaller. And that will be useful when we split the entry point
function to two versions.

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 30 +++++++++-----------
1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 6d73173aa512..757bec879e4a 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -92,27 +92,38 @@ SmbiosTableLength (
}


/**
Install all structures from the given SMBIOS structures block

- @param Smbios SMBIOS protocol
@param TableAddress SMBIOS tables starting address

**/
EFI_STATUS
InstallAllStructures (
- IN EFI_SMBIOS_PROTOCOL *Smbios,
IN UINT8 *TableAddress
)
{
+ EFI_SMBIOS_PROTOCOL *Smbios;
EFI_STATUS Status;
SMBIOS_STRUCTURE_POINTER SmbiosTable;
EFI_SMBIOS_HANDLE SmbiosHandle;
BOOLEAN NeedSmbiosType0;

+ //
+ // Find the SMBIOS protocol
+ //
+ Status = gBS->LocateProtocol (
+ &gEfiSmbiosProtocolGuid,
+ NULL,
+ (VOID**)&Smbios
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
SmbiosTable.Raw = TableAddress;
if (SmbiosTable.Raw == NULL) {
return EFI_INVALID_PARAMETER;
}

NeedSmbiosType0 = TRUE;
@@ -173,41 +184,28 @@ EFIAPI
SmbiosTablePublishEntry (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
EFI_STATUS Status;
- EFI_SMBIOS_PROTOCOL *Smbios;
SMBIOS_TABLE_ENTRY_POINT *EntryPointStructure;
UINT8 *SmbiosTables;

- //
- // Find the SMBIOS protocol
- //
- Status = gBS->LocateProtocol (
- &gEfiSmbiosProtocolGuid,
- NULL,
- (VOID**)&Smbios
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
Status = EFI_NOT_FOUND;
//
// Add Xen or QEMU SMBIOS data if found
//
EntryPointStructure = GetXenSmbiosTables ();
if (EntryPointStructure != NULL) {
SmbiosTables = (UINT8*)(UINTN)EntryPointStructure->TableAddress;
} else {
SmbiosTables = GetQemuSmbiosTables ();
}

if (SmbiosTables != NULL) {
- Status = InstallAllStructures (Smbios, SmbiosTables);
+ Status = InstallAllStructures (SmbiosTables);

//
// Free SmbiosTables if allocated by Qemu (i.e., NOT by Xen):
//
if (EntryPointStructure == NULL) {
FreePool (SmbiosTables);
--
2.19.1.3.g30247aa5d201


[PATCH 37/43] OvmfPkg/SmbiosPlatformDxe: return EFI_NOT_FOUND if there is no SMBIOS data

Laszlo Ersek
 

According to the function-top comment, SmbiosTablePublishEntry() is
supposed to return an error code if no SMBIOS data is found, from either
GetXenSmbiosTables() or GetQemuSmbiosTables(). Currently the function
returns EFI_SUCCESS in this case however (propagated from
gBS->LocateProtocol()). Make the return code match the documentation.

(This issue is not too important, but it gets in the way of splitting the
entry point function next.)

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2122
Signed-off-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 9bfc9f14f1a5..6d73173aa512 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -189,12 +189,13 @@ SmbiosTablePublishEntry (
(VOID**)&Smbios
);
if (EFI_ERROR (Status)) {
return Status;
}

+ Status = EFI_NOT_FOUND;
//
// Add Xen or QEMU SMBIOS data if found
//
EntryPointStructure = GetXenSmbiosTables ();
if (EntryPointStructure != NULL) {
SmbiosTables = (UINT8*)(UINTN)EntryPointStructure->TableAddress;
--
2.19.1.3.g30247aa5d201

15021 - 15040 of 90673