Date   

Re: [PATCH v2] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus

Sheng Wei
 

Hi Ray,
Thank you for sharing the solution. I am worrying on this issue now. I have found some produce did not support this lib.
I will update the patch.
BR
Sheng Wei

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: 2021年10月20日 8:29
To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
Cc: Kowalewski, Robert <robert.kowalewski@intel.com>; Huang, Jenny
<jenny.huang@intel.com>; Chaganty, Rangasai V
<rangasai.v.chaganty@intel.com>; Albecki, Mateusz
<mateusz.albecki@intel.com>
Subject: RE: [PATCH v2] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root
Bus

Can you rely on the PciRootBridgeIo protocol instances instead of this library?

It will make the driver usable in platforms that don't produce the
PciHostBridgeLib.

Thanks,
Rya

-----Original Message-----
From: Sheng, W <w.sheng@intel.com>
Sent: Monday, October 18, 2021 4:43 PM
To: devel@edk2.groups.io
Cc: Kowalewski, Robert <robert.kowalewski@intel.com>; Huang, Jenny
<jenny.huang@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty,
Rangasai V <rangasai.v.chaganty@intel.com>; Albecki, Mateusz
<mateusz.albecki@intel.com>
Subject: [PATCH v2] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI
Root Bus

Some system may has multi PCI root bus. It needs to use function
PciHostBridgeGetRootBridges () to get the root bus count. Then, scan
each root bus to get all devices.

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

Signed-off-by: Robert Kowalewski <robert.kowalewski@intel.com>
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Jenny Huang <jenny.huang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Robert Kowalewski <robert.kowalewski@intel.com>
Cc: Albecki Mateusz <mateusz.albecki@intel.com>
---
.../Feature/VTd/IntelVTdDxe/DmaProtection.h | 1 +
.../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c | 17
++++++++++++++---
.../Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf | 1 +
3 files changed, 16 insertions(+), 3 deletions(-)

diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
h
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
h
index a24fbc37..97061de5 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
h
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtect
+++ ion.h
@@ -23,6 +23,7 @@
#include <Library/PerformanceLib.h>
#include <Library/PrintLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/PciHostBridgeLib.h>

#include <Guid/EventGroup.h>
#include <Guid/Acpi.h>
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.
c
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.
c
index 2d9b4374..e717aeb2 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.
c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTa
+++ ble.c
@@ -682,6 +682,9 @@ ProcessDhrd (
UINT8 SecondaryBusNumber;
EFI_STATUS Status;
VTD_SOURCE_ID SourceId;
+ PCI_ROOT_BRIDGE *RootBridges;
+ UINTN RootBridgeCount;
+ UINTN Index;

mVtdUnitInformation[VtdIndex].VtdUnitBaseAddress =
(UINTN)DmarDrhd->RegisterBaseAddress;
DEBUG ((DEBUG_INFO," VTD (%d) BaseAddress - 0x%016lx\n",
VtdIndex, DmarDrhd->RegisterBaseAddress)); @@ -692,9 +695,17 @@
ProcessDhrd (
mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag = TRUE;
DEBUG ((DEBUG_INFO," ProcessDhrd: with INCLUDE ALL\n"));

- Status = ScanPciBus((VOID *)VtdIndex, DmarDrhd->SegmentNumber, 0,
ScanBusCallbackRegisterPciDevice);
- if (EFI_ERROR (Status)) {
- return Status;
+ RootBridges = PciHostBridgeGetRootBridges (&RootBridgeCount);
+ if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
+ return EFI_UNSUPPORTED;
+ }
+ DEBUG ((DEBUG_INFO,"Find %d root bridges\n", RootBridgeCount));
+ for (Index = 0; Index < RootBridgeCount; Index++) {
+ DEBUG ((DEBUG_INFO,"Scan root bridges : %d, Segment : %d, Bus :
+ 0x%02X\n", Index, RootBridges[Index].Segment,
RootBridges[Index].Bus.Base));
+ Status = ScanPciBus((VOID *)VtdIndex, (UINT16)
+ RootBridges[Index].Segment, (UINT8) RootBridges[Index].Bus.Base,
ScanBusCallbackRegisterPciDevice);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
}
} else {
mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag =
FALSE; diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.in
f
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.in
f
index 220636ad..25ff86f4 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.in
f
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDx
+++ e.inf
@@ -55,6 +55,7 @@
PerformanceLib
PrintLib
ReportStatusCodeLib
+ PciHostBridgeLib

[Guids]
gEfiEventExitBootServicesGuid ## CONSUMES ## Event
--
2.16.2.windows.1


回复: [edk2-devel][PATCH v2] FmpDevicePkg/FmpDxe: Use new Variable Lock interface

gaoliming
 

Jie:
Thanks for your update. I also miss this typo. Reviewed-by: Liming Gao
<gaoliming@byosoft.com.cn>

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Yang Jie
发送时间: 2021年10月19日 11:11
收件人: devel@edk2.groups.io
抄送: gaoliming@byosoft.com.cn; michael.d.kinney@intel.com;
guomin.jiang@intel.com; wei6.xu@intel.com; Yang Jie <jie.yang@intel.com>
主题: [edk2-devel][PATCH v2] FmpDevicePkg/FmpDxe: Use new Variable
Lock interface

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3655
The code in FmpDevicePkg call the deprecated interface
VariableLockRequestToLockc. So I changed the code in
FmpDevicePkg using RegisterBasicVariablePolicy, instead
of the deprecated interface.


Signed-off-by: Yang Jie <jie.yang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
---
FmpDevicePkg/FmpDevicePkg.dsc | 1 +
FmpDevicePkg/FmpDxe/FmpDxe.h | 4 +-
FmpDevicePkg/FmpDxe/FmpDxe.inf | 5 +-
FmpDevicePkg/FmpDxe/VariableSupport.c | 69 +++++++++++++--------------
4 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/FmpDevicePkg/FmpDevicePkg.dsc
b/FmpDevicePkg/FmpDevicePkg.dsc
index b420f52a08..7b1af285dd 100644
--- a/FmpDevicePkg/FmpDevicePkg.dsc
+++ b/FmpDevicePkg/FmpDevicePkg.dsc
@@ -53,6 +53,7 @@
DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf


DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/Base
DebugPrintErrorLevelLib.inf

PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf

+
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Vari
ablePolicyHelperLib.inf

!ifdef CONTINUOUS_INTEGRATION

BaseCryptLib|CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf

!else

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.h
b/FmpDevicePkg/FmpDxe/FmpDxe.h
index 1177b1828e..4d94a925b6 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.h
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.h
@@ -4,7 +4,7 @@
information provided through PCDs and libraries.



Copyright (c) Microsoft Corporation.<BR>

- Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.<BR>



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



@@ -33,11 +33,11 @@
#include <Library/FmpDependencyDeviceLib.h>

#include <Protocol/FirmwareManagement.h>

#include <Protocol/FirmwareManagementProgress.h>

-#include <Protocol/VariableLock.h>

#include <Guid/SystemResourceTable.h>

#include <Guid/EventGroup.h>

#include <LastAttemptStatus.h>

#include <FmpLastAttemptStatus.h>

+#include <Library/VariablePolicyHelperLib.h>



#define VERSION_STRING_NOT_SUPPORTED L"VERSION STRING NOT
SUPPORTED"

#define VERSION_STRING_NOT_AVAILABLE L"VERSION STRING NOT
AVAILABLE"

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.inf
b/FmpDevicePkg/FmpDxe/FmpDxe.inf
index eeb904a091..1c296388b0 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.inf
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.inf
@@ -4,7 +4,7 @@
# information provided through PCDs and libraries.

#

# Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>

-# Copyright (c) 2018 - 2020, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.<BR>

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

##

@@ -55,14 +55,15 @@
FmpDependencyLib

FmpDependencyCheckLib

FmpDependencyDeviceLib

+ VariablePolicyHelperLib



[Guids]

gEfiEndOfDxeEventGroupGuid



[Protocols]

- gEdkiiVariableLockProtocolGuid ## CONSUMES

gEfiFirmwareManagementProtocolGuid ## PRODUCES

gEdkiiFirmwareManagementProgressProtocolGuid ## PRODUCES

+ gEdkiiVariablePolicyProtocolGuid ## CONSUMES



[Pcd]

gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceStorageAccessEnable
## CONSUMES

diff --git a/FmpDevicePkg/FmpDxe/VariableSupport.c
b/FmpDevicePkg/FmpDxe/VariableSupport.c
index 86dd5b203b..c4b72a2ff9 100644
--- a/FmpDevicePkg/FmpDxe/VariableSupport.c
+++ b/FmpDevicePkg/FmpDxe/VariableSupport.c
@@ -3,7 +3,7 @@
firmware updates.



Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>

- Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.<BR>



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



@@ -729,29 +729,30 @@ SetLastAttemptVersionInVariable (
static

EFI_STATUS

LockFmpVariable (

- IN EFI_STATUS PreviousStatus,

- IN EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock,

- IN CHAR16 *VariableName

+ IN EFI_STATUS PreviousStatus,

+ IN EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy,

+ IN CHAR16 *VariableName

)

{

EFI_STATUS Status;



- Status = VariableLock->RequestToLock (

- VariableLock,

- VariableName,

- &gEfiCallerIdGuid

- );

- if (!EFI_ERROR (Status)) {

- return PreviousStatus;

+ // If success, go ahead and set the policies to protect the target
variables.

+ Status = RegisterBasicVariablePolicy (VariablePolicy,

+ &gEfiCallerIdGuid,

+ VariableName,

+
VARIABLE_POLICY_NO_MIN_SIZE,

+
VARIABLE_POLICY_NO_MAX_SIZE,

+
VARIABLE_POLICY_NO_MUST_ATTR,

+
VARIABLE_POLICY_NO_CANT_ATTR,

+
VARIABLE_POLICY_TYPE_LOCK_NOW);

+ if (EFI_ERROR (Status)) {

+ DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Failed to lock variable %g %s.
Status = %r\n",

+ mImageIdName,

+ &gEfiCallerIdGuid,

+ VariableName,

+ Status

+ ));

}

-

- DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Failed to lock variable %g %s.
Status = %r\n",

- mImageIdName,

- &gEfiCallerIdGuid,

- VariableName,

- Status

- ));

-

if (EFI_ERROR (PreviousStatus)) {

return PreviousStatus;

}

@@ -773,26 +774,22 @@ LockAllFmpVariables (
FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private

)

{

- EFI_STATUS Status;

- EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;

-

- VariableLock = NULL;

- Status = gBS->LocateProtocol (

- &gEdkiiVariableLockProtocolGuid,

- NULL,

- (VOID **)&VariableLock

- );

- if (EFI_ERROR (Status) || VariableLock == NULL) {

- DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Failed to locate Variable Lock
Protocol (%r).\n", mImageIdName, Status));

- return EFI_UNSUPPORTED;

+ EFI_STATUS Status;

+ EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;

+

+ // Locate the VariablePolicy protocol.

+ Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL,
(VOID**)&VariablePolicy );

+ if (EFI_ERROR (Status)) {

+ DEBUG ((DEBUG_ERROR, "FmpDxe %a - Could not locate VariablePolicy
protocol! %r\n", __FUNCTION__, Status));

+ return Status;

}



Status = EFI_SUCCESS;

- Status = LockFmpVariable (Status, VariableLock,
Private->VersionVariableName);

- Status = LockFmpVariable (Status, VariableLock,
Private->LsvVariableName);

- Status = LockFmpVariable (Status, VariableLock,
Private->LastAttemptStatusVariableName);

- Status = LockFmpVariable (Status, VariableLock,
Private->LastAttemptVersionVariableName);

- Status = LockFmpVariable (Status, VariableLock,
Private->FmpStateVariableName);

+ Status = LockFmpVariable (Status, VariablePolicy,
Private->VersionVariableName);

+ Status = LockFmpVariable (Status, VariablePolicy,
Private->LsvVariableName);

+ Status = LockFmpVariable (Status, VariablePolicy,
Private->LastAttemptStatusVariableName);

+ Status = LockFmpVariable (Status, VariablePolicy,
Private->LastAttemptVersionVariableName);

+ Status = LockFmpVariable (Status, VariablePolicy,
Private->FmpStateVariableName);



return Status;

}

--
2.26.2.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82268): https://edk2.groups.io/g/devel/message/82268
Mute This Topic: https://groups.io/mt/86431736/4905953
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[gaoliming@byosoft.com.cn]
-=-=-=-=-=-=


回复: [edk2-devel] [PATCH v6 1/3] MdeModulePkg/SortLib: Add QuickSort function on BaseLib

gaoliming
 

What new change is made in new version patch?

Thanks
Liming
-----邮件原件-----
发件人: Kuo, IanX <ianx.kuo@intel.com>
发送时间: 2021年10月19日 10:09
收件人: devel@edk2.groups.io; Kuo, IanX <ianx.kuo@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>
抄送: Chan, Amy <amy.chan@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang,
Jian J <jian.j.wang@intel.com>
主题: RE: [edk2-devel] [PATCH v6 1/3] MdeModulePkg/SortLib: Add
QuickSort function on BaseLib

@Liming Gao

Sorry to bother you. May I get your help for review by again ?

Thanks,
Ian Kuo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of IanX Kuo
Sent: Monday, October 18, 2021 12:21 PM
To: devel@edk2.groups.io
Cc: Chan, Amy <amy.chan@intel.com>; Ni, Ray <ray.ni@intel.com>; Kuo, IanX
<ianx.kuo@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>
Subject: [edk2-devel] [PATCH v6 1/3] MdeModulePkg/SortLib: Add QuickSort
function on BaseLib

From: IanX Kuo <ianx.kuo@intel.com>

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

Use QuickSort instead of QuickSortWorker

Cc: Ray Ni <ray.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: IanX Kuo <ianx.kuo@intel.com>
---
.../Library/BaseSortLib/BaseSortLib.c | 115 +----------------
.../Library/UefiSortLib/UefiSortLib.c | 116 +-----------------
2 files changed, 8 insertions(+), 223 deletions(-)

diff --git a/MdeModulePkg/Library/BaseSortLib/BaseSortLib.c
b/MdeModulePkg/Library/BaseSortLib/BaseSortLib.c
index a12c7bc0ec..0903943ee4 100644
--- a/MdeModulePkg/Library/BaseSortLib/BaseSortLib.c
+++ b/MdeModulePkg/Library/BaseSortLib/BaseSortLib.c
@@ -1,7 +1,7 @@
/** @file Library used for sorting routines. - Copyright (c) 2009 -
2018,
Intel Corporation. All rights reserved. <BR>+ Copyright (c) 2009 - 2021,
Intel
Corporation. All rights reserved. <BR> SPDX-License-Identifier:
BSD-2-Clause-Patent **/@@ -13,114 +13,6 @@
#include <Library/MemoryAllocationLib.h> #include <Library/SortLib.h>
-/**- Worker function for QuickSorting. This function is identical to
PerformQuickSort,- except that is uses the pre-allocated buffer so the in
place sorting does not need to- allocate and free buffers constantly.--
Each
element must be equal sized.-- if BufferToSort is NULL, then ASSERT.- if
CompareFunction is NULL, then ASSERT.- if Buffer is NULL, then ASSERT.--
if Count is < 2 then perform no action.- if Size is < 1 then perform no
action.-- @param[in, out] BufferToSort on call a Buffer of (possibly
sorted)
elements- on return a buffer of
sorted elements- @param[in] Count the number of
elements in the buffer to sort- @param[in] ElementSize Size of
an element in bytes- @param[in] CompareFunction The function to
call to perform the comparison- of
any 2 elements- @param[in] Buffer Buffer of size
ElementSize for use in swapping-**/-VOID-EFIAPI-QuickSortWorker (- IN
OUT VOID *BufferToSort,- IN CONST
UINTN Count,- IN CONST UINTN
ElementSize,- IN SORT_COMPARE
CompareFunction,- IN VOID
*Buffer- )-{- VOID *Pivot;- UINTN LoopCount;-
UINTN NextSwapLocation;-- ASSERT(BufferToSort != NULL);-
ASSERT(CompareFunction != NULL);- ASSERT(Buffer != NULL);-- if
( Count < 2- || ElementSize < 1- ){- return;- }--
NextSwapLocation = 0;-- //- // pick a pivot (we choose last element)-
//- Pivot = ((UINT8*)BufferToSort+((Count-1)*ElementSize));-- //- //
Now get the pivot such that all on "left" are below it- // and everything
"right" are above it- //- for ( LoopCount = 0- ; LoopCount < Count
-1- ; LoopCount++- ){- //- // if the element is less than
the pivot- //- if
(CompareFunction((VOID*)((UINT8*)BufferToSort+((LoopCount)*ElementSize)
),Pivot) <= 0){- //- // swap- //- CopyMem (Buffer,
(UINT8*)BufferToSort+(NextSwapLocation*ElementSize), ElementSize);-
CopyMem ((UINT8*)BufferToSort+(NextSwapLocation*ElementSize),
(UINT8*)BufferToSort+((LoopCount)*ElementSize), ElementSize);-
CopyMem ((UINT8*)BufferToSort+((LoopCount)*ElementSize), Buffer,
ElementSize);-- //- // increment NextSwapLocation- //-
NextSwapLocation++;- }- }- //- // swap pivot to it's final position
(NextSwapLocaiton)- //- CopyMem (Buffer, Pivot, ElementSize);-
CopyMem (Pivot, (UINT8*)BufferToSort+(NextSwapLocation*ElementSize),
ElementSize);- CopyMem
((UINT8*)BufferToSort+(NextSwapLocation*ElementSize), Buffer,
ElementSize);-- //- // Now recurse on 2 paritial lists. neither of
these
will have the 'pivot' element- // IE list is sorted left half, pivot
element,
sorted right half...- //- if (NextSwapLocation >= 2) {-
QuickSortWorker(- BufferToSort,- NextSwapLocation,-
ElementSize,- CompareFunction,- Buffer);- }-- if ((Count -
NextSwapLocation - 1) >= 2) {- QuickSortWorker(- (UINT8
*)BufferToSort + (NextSwapLocation+1) * ElementSize,- Count -
NextSwapLocation - 1,- ElementSize,- CompareFunction,-
Buffer);- }- return;-} /** Function to perform a Quick Sort alogrithm
on
a buffer of comparable elements. @@ -156,12 +48,13 @@ PerformQuickSort
(
Buffer = AllocateZeroPool(ElementSize); ASSERT(Buffer != NULL); -
QuickSortWorker(+ QuickSort ( BufferToSort, Count,
ElementSize, CompareFunction,- Buffer);+ Buffer+ );
FreePool(Buffer); return;diff --git
a/MdeModulePkg/Library/UefiSortLib/UefiSortLib.c
b/MdeModulePkg/Library/UefiSortLib/UefiSortLib.c
index 46dc443638..29d8735c22 100644
--- a/MdeModulePkg/Library/UefiSortLib/UefiSortLib.c
+++ b/MdeModulePkg/Library/UefiSortLib/UefiSortLib.c
@@ -1,7 +1,7 @@
/** @file Library used for sorting routines. - Copyright (c) 2009 -
2014,
Intel Corporation. All rights reserved. <BR>+ Copyright (c) 2009 - 2021,
Intel
Corporation. All rights reserved. <BR> SPDX-License-Identifier:
BSD-2-Clause-Patent **/@@ -29,115 +29,6 @@ STATIC
EFI_UNICODE_COLLATION_PROTOCOL *mUnicodeCollation = NULL;
} \ } -/**- Worker function
for QuickSorting. This function is identical to PerformQuickSort,-
except
that is uses the pre-allocated buffer so the in place sorting does not
need to-
allocate and free buffers constantly.-- Each element must be equal sized.
--
if BufferToSort is NULL, then ASSERT.- if CompareFunction is NULL, then
ASSERT.- if Buffer is NULL, then ASSERT.-- if Count is < 2 then perform
no
action.- if Size is < 1 then perform no action.-- @param[in, out]
BufferToSort on call a Buffer of (possibly sorted) elements-
on return a buffer of sorted elements- @param[in] Count
the number of elements in the buffer to sort- @param[in] ElementSize
Size of an element in bytes- @param[in] CompareFunction The
function to call to perform the comparison-
of any 2 elements- @param[in] Buffer Buffer of size
ElementSize for use in swapping-**/-VOID-EFIAPI-QuickSortWorker (- IN
OUT VOID *BufferToSort,- IN CONST
UINTN Count,- IN CONST UINTN
ElementSize,- IN SORT_COMPARE
CompareFunction,- IN VOID
*Buffer- )-{- VOID *Pivot;- UINTN LoopCount;-
UINTN NextSwapLocation;-- ASSERT(BufferToSort != NULL);-
ASSERT(CompareFunction != NULL);- ASSERT(Buffer != NULL);-- if
( Count < 2- || ElementSize < 1- ){- return;- }--
NextSwapLocation = 0;-- //- // pick a pivot (we choose last element)-
//- Pivot = ((UINT8*)BufferToSort+((Count-1)*ElementSize));-- //- //
Now get the pivot such that all on "left" are below it- // and everything
"right" are above it- //- for ( LoopCount = 0- ; LoopCount < Count
-1- ; LoopCount++- ){- //- // if the element is less than
the pivot- //- if
(CompareFunction((VOID*)((UINT8*)BufferToSort+((LoopCount)*ElementSize)
),Pivot) <= 0){- //- // swap- //- CopyMem (Buffer,
(UINT8*)BufferToSort+(NextSwapLocation*ElementSize), ElementSize);-
CopyMem ((UINT8*)BufferToSort+(NextSwapLocation*ElementSize),
(UINT8*)BufferToSort+((LoopCount)*ElementSize), ElementSize);-
CopyMem ((UINT8*)BufferToSort+((LoopCount)*ElementSize), Buffer,
ElementSize);-- //- // increment NextSwapLocation- //-
NextSwapLocation++;- }- }- //- // swap pivot to it's final position
(NextSwapLocaiton)- //- CopyMem (Buffer, Pivot, ElementSize);-
CopyMem (Pivot, (UINT8*)BufferToSort+(NextSwapLocation*ElementSize),
ElementSize);- CopyMem
((UINT8*)BufferToSort+(NextSwapLocation*ElementSize), Buffer,
ElementSize);-- //- // Now recurse on 2 paritial lists. neither of
these
will have the 'pivot' element- // IE list is sorted left half, pivot
element,
sorted right half...- //- if (NextSwapLocation >= 2) {-
QuickSortWorker(- BufferToSort,- NextSwapLocation,-
ElementSize,- CompareFunction,- Buffer);- }-- if ((Count -
NextSwapLocation - 1) >= 2) {- QuickSortWorker(- (UINT8
*)BufferToSort + (NextSwapLocation+1) * ElementSize,- Count -
NextSwapLocation - 1,- ElementSize,- CompareFunction,-
Buffer);- }-- return;-} /** Function to perform a Quick Sort alogrithm
on
a buffer of comparable elements. @@ -173,12 +64,13 @@ PerformQuickSort
(
Buffer = AllocateZeroPool(ElementSize); ASSERT(Buffer != NULL); -
QuickSortWorker(+ QuickSort ( BufferToSort, Count,
ElementSize, CompareFunction,- Buffer);+ Buffer+ );
FreePool(Buffer); return;--
2.30.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82204): https://edk2.groups.io/g/devel/message/82204
Mute This Topic: https://groups.io/mt/86406843/4830160
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ianx.kuo@intel.com]
-=-=-=-=-=-=


Re: [PATCH v4] UefiPayloadPkg: Remove SystemTableInfo GUID.

Guo Dong
 

+ SmbiosHob.Raw = GetFirstGuidHob(&gUniversalPayloadSmbiosTableGuid);
+ if (SmbiosHob.Raw == NULL) {
+ SmBiosTableHob = BuildGuidHob (&gUniversalPayloadSmbiosTableGuid, sizeof (UNIVERSAL_PAYLOAD_SMBIOS_TABLE));

In non-universal payload, there is no gUniversalPayloadSmbiosTableGuid in UEFI payload HOBs. SblParseLib could get it from
SBL HOB list as below:

ParseSmbiosTable (
OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
)
{
UNIVERSAL_PAYLOAD_SMBIOS_TABLE *BlSmbiosTable;

BlSmbiosTable = (UNIVERSAL_PAYLOAD_SMBIOS_TABLE *) GetGuidHobDataFromSbl (&gUniversalPayloadSmbiosTableGuid);
if (BlSmbiosTable == NULL) {
return RETURN_NOT_FOUND;
}

CopyMem (SmbiosTable, BlSmbiosTable, sizeof (UNIVERSAL_PAYLOAD_SMBIOS_TABLE));
return RETURN_SUCCESS;
}

So in the UefiPayloadEntry.c, we don't need change the HOB from the bootloader, we could just build the HOB as-is as below:

Status = ParseGfxDeviceInfo (&SmBiosTableHob);
if (!EFI_ERROR (Status)) {
NewSmBiosTableHob = BuildGuidHob (&gUniversalPayloadSmbiosTableGuid, sizeof (SmBiosTableHob));
ASSERT (NewSmBiosTableHob != NULL);
CopyMem (NewSmBiosTableHob, &SmBiosTableHob, sizeof (SmBiosTableHob));
DEBUG ((DEBUG_INFO, "Created SMBIOS table hob\n"));
}

Same changes for ParseAcpiTableInfo().


Thanks,
Guo

-----Original Message-----
From: Kesavan Balakrishnan, ThiyaguX <thiyagux.kesavan.balakrishnan@intel.com>
Sent: Tuesday, October 19, 2021 1:49 AM
To: devel@edk2.groups.io
Cc: Kesavan Balakrishnan, ThiyaguX <thiyagux.kesavan.balakrishnan@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; You, Benjamin <benjamin.you@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>
Subject: [PATCH v4] UefiPayloadPkg: Remove SystemTableInfo GUID.

SystemTableInfo GUID is not a Spec defined GUID.
The latest CBL and SBL produces ACPI and SMBIOS table information according to the Spec.
So removing the SystemTableInfo GUID implementation.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>

Signed-off-by: Guo Dong <guo.dong@intel.com>
Signed-off-by: Thiyagu Kesavan Balakrishnan <thiyagux.kesavan.balakrishnan@intel.com>
---
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h | 1 -
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 1 -
UefiPayloadPkg/Include/Guid/SystemTableInfoGuid.h | 26 --------------------------
UefiPayloadPkg/Include/Library/BlParseLib.h | 25 ++++++++++++++++++++-----
UefiPayloadPkg/Library/CbParseLib/CbParseLib.c | 40 +++++++++++++++++++++++++++++++---------
UefiPayloadPkg/Library/SblParseLib/SblParseLib.c | 36 ++++++++++++++++++++++--------------
UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf | 3 ++-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 64 +++++++++++++++++++++++++++++++++++-----------------------------
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h | 1 -
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf | 1 -
UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf | 1 -
11 files changed, 110 insertions(+), 89 deletions(-)

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
index 3332a30eae..b16ca4cc59 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
@@ -20,7 +20,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include <Library/HobLib.h>

#include <Guid/SmBios.h>
-#include <Guid/SystemTableInfoGuid.h>
#include <Guid/AcpiBoardInfoGuid.h>
#include <Guid/GraphicsInfoHob.h>

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
index 1ccb250991..96d85d2b1d 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
@@ -42,7 +42,6 @@
HobLib

[Guids]
- gUefiSystemTableInfoGuid
gUefiAcpiBoardInfoGuid
gEfiGraphicsInfoHobGuid

diff --git a/UefiPayloadPkg/Include/Guid/SystemTableInfoGuid.h b/UefiPayloadPkg/Include/Guid/SystemTableInfoGuid.h
deleted file mode 100644
index e742dd0ca5..0000000000
--- a/UefiPayloadPkg/Include/Guid/SystemTableInfoGuid.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/** @file
- This file defines the hob structure for system tables like ACPI, SMBIOS tables.
-
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
- SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef SYSTEM_TABLE_INFO_GUID_H_
-#define SYSTEM_TABLE_INFO_GUID_H_
-
-///
-/// System Table Information GUID
-///
-extern EFI_GUID gUefiSystemTableInfoGuid;
-
-typedef struct {
- UINT8 Revision;
- UINT8 Reserved0[3];
- UINT64 AcpiTableBase;
- UINT32 AcpiTableSize;
- UINT64 SmbiosTableBase;
- UINT32 SmbiosTableSize;
-} SYSTEM_TABLE_INFO;
-
-#endif
diff --git a/UefiPayloadPkg/Include/Library/BlParseLib.h b/UefiPayloadPkg/Include/Library/BlParseLib.h
index 7198e419bd..5b5063b4e5 100644
--- a/UefiPayloadPkg/Include/Library/BlParseLib.h
+++ b/UefiPayloadPkg/Include/Library/BlParseLib.h
@@ -13,8 +13,9 @@
#include <Guid/GraphicsInfoHob.h>
#include <Guid/MemoryMapInfoGuid.h>
#include <Guid/SerialPortInfoGuid.h>
-#include <Guid/SystemTableInfoGuid.h>
#include <Guid/AcpiBoardInfoGuid.h>
+#include <UniversalPayload/AcpiTable.h> #include
+<UniversalPayload/SmbiosTable.h>

#define GET_BOOTLOADER_PARAMETER() PcdGet64 (PcdBootloaderParameter)

@@ -55,9 +56,9 @@ ParseMemoryInfo (
);

/**
- Acquire acpi table and smbios table from slim bootloader
+ Acquire SMBIOS table from bootloader.

- @param SystemTableInfo Pointer to the system table info
+ @param SmbiosTable Pointer to the SMBIOS table info.

@retval RETURN_SUCCESS Successfully find out the tables.
@retval RETURN_NOT_FOUND Failed to find the tables.
@@ -65,10 +66,24 @@ ParseMemoryInfo (
**/
RETURN_STATUS
EFIAPI
-ParseSystemTable (
- OUT SYSTEM_TABLE_INFO *SystemTableInfo
+ParseSmbiosTable (
+ OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
);

+/**
+ Acquire ACPI table from bootloader.
+
+ @param AcpiTableHob Pointer to the ACPI table info.
+
+ @retval RETURN_SUCCESS Successfully find out the tables.
+ @retval RETURN_NOT_FOUND Failed to find the tables.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseAcpiTableInfo (
+ OUT UNIVERSAL_PAYLOAD_ACPI_TABLE *AcpiTableHob
+ );

/**
Find the serial port information
diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
index 46314e5566..61c5683260 100644
--- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
+++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
@@ -410,9 +410,9 @@ ParseMemoryInfo (


/**
- Acquire acpi table and smbios table from coreboot
+ Acquire SMBIOS table from coreboot.

- @param SystemTableInfo Pointer to the system table info
+ @param SmbiosTable Pointer to the SMBIOS table info.

@retval RETURN_SUCCESS Successfully find out the tables.
@retval RETURN_NOT_FOUND Failed to find the tables.
@@ -420,8 +420,8 @@ ParseMemoryInfo (
**/
RETURN_STATUS
EFIAPI
-ParseSystemTable (
- OUT SYSTEM_TABLE_INFO *SystemTableInfo
+ParseSmbiosTable (
+ OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
)
{
EFI_STATUS Status;
@@ -432,17 +432,39 @@ ParseSystemTable (
if (EFI_ERROR (Status)) {
return EFI_NOT_FOUND;
}
- SystemTableInfo->SmbiosTableBase = (UINT64) (UINTN)MemTable;
- SystemTableInfo->SmbiosTableSize = MemTableSize;
+ SmbiosTable->SmBiosEntryPoint = (UINT64) (UINTN)MemTable;
+
+ return RETURN_SUCCESS;
+}
+
+
+/**
+ Acquire ACPI table from coreboot.
+
+ @param AcpiTableHob Pointer to the ACPI table info.
+
+ @retval RETURN_SUCCESS Successfully find out the tables.
+ @retval RETURN_NOT_FOUND Failed to find the tables.
+
+**/
+
+RETURN_STATUS
+EFIAPI
+ParseAcpiTableInfo (
+ OUT UNIVERSAL_PAYLOAD_ACPI_TABLE *AcpiTableHob
+ )
+{
+ EFI_STATUS Status;
+ VOID *MemTable;
+ UINT32 MemTableSize;

Status = ParseCbMemTable (SIGNATURE_32 ('I', 'P', 'C', 'A'), &MemTable, &MemTableSize);
if (EFI_ERROR (Status)) {
return EFI_NOT_FOUND;
}
- SystemTableInfo->AcpiTableBase = (UINT64) (UINTN)MemTable;
- SystemTableInfo->AcpiTableSize = MemTableSize;
+ AcpiTableHob->Rsdp = (UINT64) (UINTN)MemTable;

- return Status;
+ return RETURN_SUCCESS;
}


diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
index eeb0dfe74a..5fa459255e 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
@@ -110,33 +110,41 @@ ParseMemoryInfo (
}

/**
- Acquire acpi table and smbios table from slim bootloader
+ Acquire SMBIOS table from slim bootloader.

- @param SystemTableInfo Pointer to the system table info
+ @param SmbiosTable Pointer to the SMBIOS table info.

- @retval RETURN_SUCCESS Successfully find out the tables.
@retval RETURN_NOT_FOUND Failed to find the tables.

**/
RETURN_STATUS
EFIAPI
-ParseSystemTable (
- OUT SYSTEM_TABLE_INFO *SystemTableInfo
+ParseSmbiosTable (
+ OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
)
{
- SYSTEM_TABLE_INFO *TableInfo;
+ DEBUG ((DEBUG_ERROR, "gUniversalPayloadSmbiosTableGuid should be
+produced by SlimBoot Loader.\n"));
+ return RETURN_NOT_FOUND;
+}

- TableInfo = (SYSTEM_TABLE_INFO *)GetGuidHobDataFromSbl (&gUefiSystemTableInfoGuid);
- if (TableInfo == NULL) {
- ASSERT (FALSE);
- return RETURN_NOT_FOUND;
- }

- CopyMem (SystemTableInfo, TableInfo, sizeof (SYSTEM_TABLE_INFO));
+/**
+ Acquire ACPI table from slim bootloader.

- return RETURN_SUCCESS;
-}
+ @param AcpiTableHob Pointer to the ACPI table info.

+ @retval RETURN_NOT_FOUND Failed to find the tables.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseAcpiTableInfo (
+ OUT UNIVERSAL_PAYLOAD_ACPI_TABLE *AcpiTableHob
+ )
+{
+ DEBUG ((DEBUG_ERROR, "gUniversalPayloadAcpiTableGuid should be
+produced by SlimBoot Loader.\n"));
+ return RETURN_NOT_FOUND;
+}

/**
Find the serial port information
diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
index 535cca58a6..d0de75433d 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
@@ -36,7 +36,8 @@
HobLib

[Guids]
- gUefiSystemTableInfoGuid
+ gUniversalPayloadAcpiTableGuid
+ gUniversalPayloadSmbiosTableGuid
gUefiSerialPortInfoGuid
gLoaderMemoryMapInfoGuid
gEfiGraphicsInfoHobGuid
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index 9efe01d094..491fd2d3bb 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -224,8 +224,6 @@ BuildHobFromBl (
)
{
EFI_STATUS Status;
- SYSTEM_TABLE_INFO SysTableInfo;
- SYSTEM_TABLE_INFO *NewSysTableInfo;
ACPI_BOARD_INFO *AcpiBoardInfo;
EFI_PEI_GRAPHICS_INFO_HOB GfxInfo;
EFI_PEI_GRAPHICS_INFO_HOB *NewGfxInfo;
@@ -233,6 +231,8 @@ BuildHobFromBl (
EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *NewGfxDeviceInfo;
UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmBiosTableHob;
UNIVERSAL_PAYLOAD_ACPI_TABLE *AcpiTableHob;
+ EFI_PEI_HOB_POINTERS SmbiosHob;
+ EFI_PEI_HOB_POINTERS AcpiHob;

//
// First find TOLUD
@@ -274,42 +274,48 @@ BuildHobFromBl (
}


- //
- // Create guid hob for system tables like acpi table and smbios table
- //
- Status = ParseSystemTable(&SysTableInfo);
- ASSERT_EFI_ERROR (Status);
- if (!EFI_ERROR (Status)) {
- NewSysTableInfo = BuildGuidHob (&gUefiSystemTableInfoGuid, sizeof (SYSTEM_TABLE_INFO));
- ASSERT (NewSysTableInfo != NULL);
- CopyMem (NewSysTableInfo, &SysTableInfo, sizeof (SYSTEM_TABLE_INFO));
- DEBUG ((DEBUG_INFO, "Detected Acpi Table at 0x%lx, length 0x%x\n", SysTableInfo.AcpiTableBase, SysTableInfo.AcpiTableSize));
- DEBUG ((DEBUG_INFO, "Detected Smbios Table at 0x%lx, length 0x%x\n", SysTableInfo.SmbiosTableBase, SysTableInfo.SmbiosTableSize));
- }
//
// Creat SmBios table Hob
//
- SmBiosTableHob = BuildGuidHob (&gUniversalPayloadSmbiosTableGuid, sizeof (UNIVERSAL_PAYLOAD_SMBIOS_TABLE));
- ASSERT (SmBiosTableHob != NULL);
- SmBiosTableHob->Header.Revision = UNIVERSAL_PAYLOAD_SMBIOS_TABLE_REVISION;
- SmBiosTableHob->Header.Length = sizeof (UNIVERSAL_PAYLOAD_SMBIOS_TABLE);
- SmBiosTableHob->SmBiosEntryPoint = SysTableInfo.SmbiosTableBase;
- DEBUG ((DEBUG_INFO, "Create smbios table gUniversalPayloadSmbiosTableGuid guid hob\n"));
-
+ SmbiosHob.Raw = GetFirstGuidHob(&gUniversalPayloadSmbiosTableGuid);
+ if (SmbiosHob.Raw == NULL) {
+ SmBiosTableHob = BuildGuidHob (&gUniversalPayloadSmbiosTableGuid, sizeof (UNIVERSAL_PAYLOAD_SMBIOS_TABLE));
+ ASSERT (SmBiosTableHob != NULL);
+ SmBiosTableHob->Header.Revision = UNIVERSAL_PAYLOAD_SMBIOS_TABLE_REVISION;
+ SmBiosTableHob->Header.Length = sizeof (UNIVERSAL_PAYLOAD_SMBIOS_TABLE);
+ DEBUG ((DEBUG_INFO, "Create smbios table gUniversalPayloadSmbiosTableGuid guid hob\n"));
+ Status = ParseSmbiosTable(SmBiosTableHob);
+ if (!EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Detected Smbios Table at 0x%lx\n", SmBiosTableHob->SmBiosEntryPoint));
+ }
+ }
+ else {
+ SmBiosTableHob = (UNIVERSAL_PAYLOAD_SMBIOS_TABLE *)(*(UINTN *)(GET_GUID_HOB_DATA (SmbiosHob.Raw)));
+ DEBUG ((DEBUG_INFO, "Detected Smbios Table at 0x%lx\n",
+ SmBiosTableHob->SmBiosEntryPoint));
+ }
//
// Creat ACPI table Hob
//
- AcpiTableHob = BuildGuidHob (&gUniversalPayloadAcpiTableGuid, sizeof (UNIVERSAL_PAYLOAD_ACPI_TABLE));
- ASSERT (AcpiTableHob != NULL);
- AcpiTableHob->Header.Revision = UNIVERSAL_PAYLOAD_ACPI_TABLE_REVISION;
- AcpiTableHob->Header.Length = sizeof (UNIVERSAL_PAYLOAD_ACPI_TABLE);
- AcpiTableHob->Rsdp = SysTableInfo.AcpiTableBase;
- DEBUG ((DEBUG_INFO, "Create smbios table gUniversalPayloadAcpiTableGuid guid hob\n"));
-
+ AcpiHob.Raw = GetFirstGuidHob(&gUniversalPayloadAcpiTableGuid);
+ if (AcpiHob.Raw == NULL) {
+ AcpiTableHob = BuildGuidHob (&gUniversalPayloadAcpiTableGuid, sizeof (UNIVERSAL_PAYLOAD_ACPI_TABLE));
+ ASSERT (AcpiTableHob != NULL);
+ AcpiTableHob->Header.Revision = UNIVERSAL_PAYLOAD_ACPI_TABLE_REVISION;
+ AcpiTableHob->Header.Length = sizeof (UNIVERSAL_PAYLOAD_ACPI_TABLE);
+ DEBUG ((DEBUG_INFO, "Create ACPI table gUniversalPayloadAcpiTableGuid guid hob\n"));
+ Status = ParseAcpiTableInfo(AcpiTableHob);
+ if (!EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Detected ACPI Table at 0x%lx\n", AcpiTableHob->Rsdp));
+ }
+ }
+ else {
+ AcpiTableHob = (UNIVERSAL_PAYLOAD_ACPI_TABLE *)(*(UINTN *)(GET_GUID_HOB_DATA (AcpiHob.Raw)));
+ DEBUG ((DEBUG_INFO, "Detected ACPI Table at 0x%lx\n",
+ AcpiTableHob->Rsdp)); }
//
// Create guid hob for acpi board information
//
- AcpiBoardInfo = BuildHobFromAcpi (SysTableInfo.AcpiTableBase);
+ AcpiBoardInfo = BuildHobFromAcpi (AcpiTableHob->Rsdp);
ASSERT (AcpiBoardInfo != NULL);

//
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
index 637ed9c20b..716430e3cb 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
@@ -27,7 +27,6 @@
#include <IndustryStandard/Acpi.h>
#include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
#include <Guid/SerialPortInfoGuid.h>
-#include <Guid/SystemTableInfoGuid.h>
#include <Guid/MemoryMapInfoGuid.h>
#include <Guid/AcpiBoardInfoGuid.h>
#include <Guid/GraphicsInfoHob.h>
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
index 96e4bb81f4..07a678bd46 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
@@ -59,7 +59,6 @@
[Guids]
gEfiMemoryTypeInformationGuid
gEfiFirmwareFileSystem2Guid
- gUefiSystemTableInfoGuid
gEfiGraphicsInfoHobGuid
gEfiGraphicsDeviceInfoHobGuid
gUefiAcpiBoardInfoGuid
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
index 928bd2e42b..a8576305ad 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
@@ -57,7 +57,6 @@
[Guids]
gEfiMemoryTypeInformationGuid
gEfiFirmwareFileSystem2Guid
- gUefiSystemTableInfoGuid
gEfiGraphicsInfoHobGuid
gEfiGraphicsDeviceInfoHobGuid
gUefiAcpiBoardInfoGuid
--
2.33.0.windows.2


Re: [PATCH v2] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus

Ni, Ray
 

Can you rely on the PciRootBridgeIo protocol instances instead of this library?

It will make the driver usable in platforms that don't produce the PciHostBridgeLib.

Thanks,
Rya

-----Original Message-----
From: Sheng, W <w.sheng@intel.com>
Sent: Monday, October 18, 2021 4:43 PM
To: devel@edk2.groups.io
Cc: Kowalewski, Robert <robert.kowalewski@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Ni, Ray <ray.ni@intel.com>;
Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Albecki, Mateusz <mateusz.albecki@intel.com>
Subject: [PATCH v2] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus

Some system may has multi PCI root bus. It needs to use function
PciHostBridgeGetRootBridges () to get the root bus count. Then,
scan each root bus to get all devices.

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

Signed-off-by: Robert Kowalewski <robert.kowalewski@intel.com>
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Jenny Huang <jenny.huang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Robert Kowalewski <robert.kowalewski@intel.com>
Cc: Albecki Mateusz <mateusz.albecki@intel.com>
---
.../Feature/VTd/IntelVTdDxe/DmaProtection.h | 1 +
.../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c | 17 ++++++++++++++---
.../Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf | 1 +
3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
index a24fbc37..97061de5 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
@@ -23,6 +23,7 @@
#include <Library/PerformanceLib.h>
#include <Library/PrintLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/PciHostBridgeLib.h>

#include <Guid/EventGroup.h>
#include <Guid/Acpi.h>
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
index 2d9b4374..e717aeb2 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
@@ -682,6 +682,9 @@ ProcessDhrd (
UINT8 SecondaryBusNumber;
EFI_STATUS Status;
VTD_SOURCE_ID SourceId;
+ PCI_ROOT_BRIDGE *RootBridges;
+ UINTN RootBridgeCount;
+ UINTN Index;

mVtdUnitInformation[VtdIndex].VtdUnitBaseAddress = (UINTN)DmarDrhd->RegisterBaseAddress;
DEBUG ((DEBUG_INFO," VTD (%d) BaseAddress - 0x%016lx\n", VtdIndex, DmarDrhd->RegisterBaseAddress));
@@ -692,9 +695,17 @@ ProcessDhrd (
mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag = TRUE;
DEBUG ((DEBUG_INFO," ProcessDhrd: with INCLUDE ALL\n"));

- Status = ScanPciBus((VOID *)VtdIndex, DmarDrhd->SegmentNumber, 0, ScanBusCallbackRegisterPciDevice);
- if (EFI_ERROR (Status)) {
- return Status;
+ RootBridges = PciHostBridgeGetRootBridges (&RootBridgeCount);
+ if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
+ return EFI_UNSUPPORTED;
+ }
+ DEBUG ((DEBUG_INFO,"Find %d root bridges\n", RootBridgeCount));
+ for (Index = 0; Index < RootBridgeCount; Index++) {
+ DEBUG ((DEBUG_INFO,"Scan root bridges : %d, Segment : %d, Bus : 0x%02X\n", Index, RootBridges[Index].Segment,
RootBridges[Index].Bus.Base));
+ Status = ScanPciBus((VOID *)VtdIndex, (UINT16) RootBridges[Index].Segment, (UINT8) RootBridges[Index].Bus.Base,
ScanBusCallbackRegisterPciDevice);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
}
} else {
mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag = FALSE;
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
index 220636ad..25ff86f4 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
@@ -55,6 +55,7 @@
PerformanceLib
PrintLib
ReportStatusCodeLib
+ PciHostBridgeLib

[Guids]
gEfiEventExitBootServicesGuid ## CONSUMES ## Event
--
2.16.2.windows.1


Re: [PATCH v2 1/1] OvmfPkg: Introduce 16MiB flash size for (primarily) Linuxboot

Devon Bautista <dbautista@...>
 

On 9/9/21 03:09, Philippe Mathieu-Daudé wrote:
On 9/3/21 7:26 AM, Devon Bautista wrote:
The largest size flash image currently available for OVMF builds, 4MiB,
is too small to insert a Linux kernel and initramfs into the DXEFV, and
is thus insufficient for testing Linuxboot builds via OVMF.

Introduce the FD_SIZE_16MB build macro (equivalently,
FD_SIZE_IN_KB=16384), which enlarges the full flash image to 16MiB, the
maximum size available for x86.
I understand this is unavoidable to remove the restriction, but this
will have a negative impact on clouds memory consumption. ARM clouds
are already suffering from having 64MiB flashes, see:
https://lore.kernel.org/all/20201116104216.439650-1-david.edmondson@oracle.com/
Is ARM still padding flash images with zeros up to 64MB?

Even so, this patch merely introduces the 16MB macro but does not make it the default. Genuinely, I am wondering how having this optional build macro would conflict with ARM's memory consumption if ARM is already using the default 4MB build target for OVMF, unless ARM is already using a 16MB build target downstream and this would conflict with that.
Some notes to extend the discussion.
* Why is QEMU using 2 flashes (CODE & DATA)?
My historical understanding is when OVMF was started, QEMU flash
model was not supporting sector/bank (write/erase) protection.
OVMF requirements were:
- CODE section ("secure", not modifiable by the guest)
- DATA section (modifiable)
The easier way to get the CODE secure is to use different devices,
one enforced in "read-only" mode.
Being a firmware for virtualized guests, it makes no sense to have
the guest upgrade the CODE: it is error-prone, and cheaper to do
directly on the host, rebooting the guest.
* Why not use a ROM for the CODE section and flash for the DATA one?
This is not clear to me. I suppose the firmware wanted to be able
to poll the hardware size, and the pflash allow that with CFI I/O
requests?
* Could we replace the CODE pflash by a ROM device?
QEMU provides the -fw_cfg device and versioned machines. To the best
of my knowledge it seems doable, with careful modifications in OVMF
and ArmVirt.
* What are the benefits of using a ROM for the CODE section?
- simpler code path, simpler to audit / review, safer
- reduce migration burden (no pflash device state)
- reduce memory consumption (backing file mmaped with MAP_SHARED)
* Is there similar problems with DATA section?
The biggest problem is the memory waste, the pflash model was
designed to be executable, modifiable, and as fast as possible
(for execution). This is achieved by copying the whole flash
content in an internal buffer. For DATA flash this is no speed
gain but high memory penalty.
* Can the DATA section memory consumption be reduced?
Yes, various suggestions were posted on QEMU mailing list, but
nothing accepted so far, this is still a work in progress, and
ideas are welcomed.
I'm unsure I have the experience necessary to make an informed comment on these points; I think Gerd and/or the other OVMF maintainers/reviewers would have better insight.

Best,
Devon


[PATCH 1/1] ArmPkg: Add SMC helper functions

Rebecca Cran
 

Add functions ArmCallSmc0/1/2/3 to do SMC calls with 0, 1, 2 or 3
arguments.
The functions return up to 3 values.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
ArmPkg/Include/Library/ArmSmcLib.h | 73 ++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmc.c | 122 ++++++++++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 3 +
ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c | 85 ++++++++++++++
4 files changed, 283 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmSmcLib.h b/ArmPkg/Include/Library/ArmSmcLib.h
index ced60b3c1147..343ae7f40ad2 100644
--- a/ArmPkg/Include/Library/ArmSmcLib.h
+++ b/ArmPkg/Include/Library/ArmSmcLib.h
@@ -1,5 +1,6 @@
/** @file
*
+* Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
* Copyright (c) 2012-2014, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -37,4 +38,76 @@ ArmCallSmc (
IN OUT ARM_SMC_ARGS *Args
);

+/** Trigger an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
#endif // ARM_SMC_LIB_H_
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmc.c b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
new file mode 100644
index 000000000000..d596003a857e
--- /dev/null
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
@@ -0,0 +1,122 @@
+/** @file
+ SMC helper functions.
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/ArmSmcLib.h>
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ )
+{
+ ARM_SMC_ARGS Args;
+ UINTN ErrorCode;
+
+ Args.Arg0 = Function;
+
+ if (Arg1 != NULL) {
+ Args.Arg1 = *Arg1;
+ }
+ if (Arg2 != NULL) {
+ Args.Arg2 = *Arg2;
+ }
+ if (Arg3 != NULL) {
+ Args.Arg3 = *Arg3;
+ }
+
+ ArmCallSmc (&Args);
+
+ ErrorCode = Args.Arg0;
+
+ if (Arg1 != NULL) {
+ *Arg1 = Args.Arg1;
+ }
+ if (Arg2 != NULL) {
+ *Arg2 = Args.Arg2;
+ }
+ if (Arg3 != NULL) {
+ *Arg3 = Args.Arg3;
+ }
+
+ return ErrorCode;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
index 4f4b09f4528a..a89f9203fb7e 100644
--- a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
@@ -20,6 +20,9 @@
[Sources.AARCH64]
AArch64/ArmSmc.S

+[Sources]
+ ArmSmc.c
+
[Packages]
MdePkg/MdePkg.dec
ArmPkg/ArmPkg.dec
diff --git a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
index 2d79aadaf1fa..ca1b8830a119 100644
--- a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
+++ b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
@@ -1,4 +1,5 @@
//
+// Copyright (c) 2021, NUVIA Inc. All rights reserved.
// Copyright (c) 2016, Linaro Limited. All rights reserved.
//
// SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -7,6 +8,7 @@

#include <Base.h>
#include <Library/ArmSmcLib.h>
+#include <IndustryStandard/ArmStdSmc.h>

VOID
ArmCallSmc (
@@ -14,3 +16,86 @@ ArmCallSmc (
)
{
}
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
--
2.31.1


[PATCH] MdeModulePkg/DxeCapsuleLibFmp: Add runtime SetImage support

Bob Morgan
 

Adds optional support for processing FMP capusle images after
ExitBootServices() if the ImageTypeIdGuid is mentioned in the new
PcdRuntimeFmpCapsuleImageTypeIdGuid list.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Signed-off-by: Bob Morgan <bobm@nvidia.com>
---
.../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 81 +++++++++---
.../DxeCapsuleLibFmp/DxeCapsuleRuntime.c | 119 ++++++++++++++++++
.../DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf | 4 +
MdeModulePkg/MdeModulePkg.dec | 7 +-
4 files changed, 192 insertions(+), 19 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 90942135d7..0000f91c6a 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -10,6 +10,7 @@
ValidateFmpCapsule(), and DisplayCapsuleImage() receives untrusted input and
performs basic validation.

+ Copyright (c) 2021, NVIDIA CORPORATION. All rights reserved.<BR>
Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -41,6 +42,11 @@
#include <Protocol/FirmwareManagementProgress.h>
#include <Protocol/DevicePath.h>

+BOOLEAN (EFIAPI *mLibAtRuntimeFunction) (VOID) = NULL;
+EFI_FIRMWARE_MANAGEMENT_PROTOCOL *mRuntimeFmp = NULL;
+VOID **mRuntimeFmpProtocolArray = NULL;
+EFI_GUID *mRuntimeFmpGuidArray = NULL;
+
EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable = NULL;
BOOLEAN mIsVirtualAddrConverted = FALSE;

@@ -551,6 +557,11 @@ DumpAllFmpInfo (
UINT32 PackageVersion;
CHAR16 *PackageVersionName;

+ // Dump not supported at runtime.
+ if ((mLibAtRuntimeFunction != NULL) && mLibAtRuntimeFunction ()) {
+ return;
+ }
+
Status = gBS->LocateHandleBuffer (
ByProtocol,
&gEfiFirmwareManagementProtocolGuid,
@@ -906,25 +917,35 @@ SetFmpImageData (
CHAR16 *AbortReason;
EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS ProgressCallback;

- Status = gBS->HandleProtocol(
- Handle,
- &gEfiFirmwareManagementProtocolGuid,
- (VOID **)&Fmp
- );
- if (EFI_ERROR(Status)) {
- return Status;
- }
+ // If not using optional runtime support, get FMP protocol for given Handle.
+ // Otherwise, use the one saved by ProcessFmpCapsuleImage().
+ if ((mLibAtRuntimeFunction == NULL) || !mLibAtRuntimeFunction ()) {
+ Status = gBS->HandleProtocol(
+ Handle,
+ &gEfiFirmwareManagementProtocolGuid,
+ (VOID **)&Fmp
+ );
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }

- //
- // Lookup Firmware Management Progress Protocol before SetImage() is called
- // This is an optional protocol that may not be present on Handle.
- //
- Status = gBS->HandleProtocol (
- Handle,
- &gEdkiiFirmwareManagementProgressProtocolGuid,
- (VOID **)&mFmpProgress
- );
- if (EFI_ERROR (Status)) {
+ //
+ // Lookup Firmware Management Progress Protocol before SetImage() is called
+ // This is an optional protocol that may not be present on Handle.
+ //
+ Status = gBS->HandleProtocol (
+ Handle,
+ &gEdkiiFirmwareManagementProgressProtocolGuid,
+ (VOID **)&mFmpProgress
+ );
+ if (EFI_ERROR (Status)) {
+ mFmpProgress = NULL;
+ }
+ } else {
+ if (mRuntimeFmp == NULL) {
+ return EFI_UNSUPPORTED;
+ }
+ Fmp = mRuntimeFmp;
mFmpProgress = NULL;
}

@@ -1259,6 +1280,30 @@ ProcessFmpCapsuleImage (
UpdateHardwareInstance = ImageHeader->UpdateHardwareInstance;
}

+ // Optional runtime FMP SetImage processing sequence
+ if ((mLibAtRuntimeFunction != NULL) && mLibAtRuntimeFunction () &&
+ (mRuntimeFmpProtocolArray != NULL)) {
+ mRuntimeFmp = NULL;
+ Index2 = 0;
+ while (mRuntimeFmpProtocolArray[Index2] != NULL) {
+ if (CompareGuid (&ImageHeader->UpdateImageTypeId,
+ &mRuntimeFmpGuidArray[Index2])) {
+ mRuntimeFmp = (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *)
+ mRuntimeFmpProtocolArray[Index2];
+ break;
+ }
+ Index2++;
+ }
+
+ Status = SetFmpImageData (NULL,
+ ImageHeader,
+ Index - FmpCapsuleHeader->EmbeddedDriverCount);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ continue;
+ }
+
Status = GetFmpHandleBufferByType (
&ImageHeader->UpdateImageTypeId,
UpdateHardwareInstance,
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
index f94044a409..6feb6dab79 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
@@ -1,6 +1,7 @@
/** @file
Capsule library runtime support.

+ Copyright (c) 2021, NVIDIA CORPORATION. All rights reserved.<BR>
Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -19,7 +20,11 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiRuntimeServicesTableLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiRuntimeLib.h>

+extern BOOLEAN (EFIAPI *mLibAtRuntimeFunction) (VOID);
+extern VOID **mRuntimeFmpProtocolArray;
+extern EFI_GUID *mRuntimeFmpGuidArray;
extern EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable;
extern BOOLEAN mIsVirtualAddrConverted;
EFI_EVENT mDxeRuntimeCapsuleLibVirtualAddressChangeEvent = NULL;
@@ -40,9 +45,121 @@ DxeCapsuleLibVirtualAddressChangeEvent (
)
{
gRT->ConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mEsrtTable);
+
+ if (mRuntimeFmpProtocolArray != NULL) {
+ VOID **FmpArrayEntry;
+
+ FmpArrayEntry = mRuntimeFmpProtocolArray;
+ while (*FmpArrayEntry != NULL) {
+ EfiConvertPointer (0x0, (VOID **) FmpArrayEntry);
+ FmpArrayEntry++;
+ }
+ EfiConvertPointer (0x0, (VOID **) &mRuntimeFmpProtocolArray);
+ }
+ if (mRuntimeFmpGuidArray != NULL) {
+ EfiConvertPointer (0x0, (VOID **) &mRuntimeFmpGuidArray);
+ }
+ if (mLibAtRuntimeFunction != NULL ) {
+ EfiConvertPointer (0x0, (VOID **) &mLibAtRuntimeFunction);
+ }
+
mIsVirtualAddrConverted = TRUE;
}

+/**
+ Initialize optional runtime FMP arrays to support FMP SetImage processing
+ after ExitBootServices() is called.
+
+ The ImageTypeIdGuids of runtime-capable FMP protocol drivers are extracted
+ from the PcdRuntimeFmpCapsuleImageTypeIdGuid list and their protocol
+ structure pointers are saved in the mRuntimeFmpProtocolArray for use during
+ UpdateCapsule() processing. UpdateHardwareInstance is not supported.
+
+**/
+STATIC
+VOID
+EFIAPI
+InitializeRuntimeFmpArrays (
+ VOID
+ )
+{
+ EFI_GUID *Guid;
+ UINTN NumHandles;
+ EFI_HANDLE *HandleBuffer;
+ EFI_STATUS Status;
+ UINTN Count;
+ UINTN Index;
+ UINTN FmpArrayIndex;
+
+ EFI_STATUS
+ GetFmpHandleBufferByType (
+ IN EFI_GUID *UpdateImageTypeId,
+ IN UINT64 UpdateHardwareInstance,
+ OUT UINTN *NoHandles, OPTIONAL
+ OUT EFI_HANDLE **HandleBuf, OPTIONAL
+ OUT BOOLEAN **ResetRequiredBuf OPTIONAL
+ );
+
+ Count = PcdGetSize (PcdRuntimeFmpCapsuleImageTypeIdGuid) / sizeof (GUID);
+ if (Count == 0) {
+ return;
+ }
+
+ // mRuntimeFmpProtocolArray is a NULL-terminated list of FMP protocol pointers
+ mRuntimeFmpProtocolArray = (VOID **)
+ AllocateRuntimeZeroPool ((Count + 1) * sizeof (VOID *));
+ if (mRuntimeFmpProtocolArray == NULL) {
+ DEBUG ((DEBUG_ERROR, "Error allocating mRuntimeFmpProtocolArray\n"));
+ return;
+ }
+ mRuntimeFmpGuidArray = (EFI_GUID *)
+ AllocateRuntimeZeroPool (Count * sizeof (EFI_GUID));
+ if (mRuntimeFmpGuidArray == NULL) {
+ DEBUG ((DEBUG_ERROR, "Error allocating mRuntimeFmpGuidArray"));
+ FreePool (mRuntimeFmpProtocolArray);
+ return;
+ }
+
+ // For each runtime ImageTypeIdGuid in the PCD, save its GUID and FMP protocol
+ FmpArrayIndex = 0;
+ Guid = PcdGetPtr (PcdRuntimeFmpCapsuleImageTypeIdGuid);
+ for (Index = 0; Index < Count; Index++, Guid++) {
+ mRuntimeFmpGuidArray[FmpArrayIndex] = *Guid;
+ HandleBuffer = NULL;
+ Status = GetFmpHandleBufferByType (Guid,
+ 0,
+ &NumHandles,
+ &HandleBuffer,
+ NULL);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR,
+ "Error finding FMP handle for runtime ImageTypeIdGuid=%g: %r\n",
+ Guid, Status));
+ continue;
+ }
+
+ if (NumHandles > 1) {
+ DEBUG ((DEBUG_ERROR,
+ "FMP runtime ImageTypeIdGuid=%g returned %u handles, only 1 supported\n",
+ Guid, NumHandles));
+ }
+ Status = gBS->HandleProtocol (HandleBuffer[0],
+ &gEfiFirmwareManagementProtocolGuid,
+ &mRuntimeFmpProtocolArray[FmpArrayIndex]);
+ FreePool (HandleBuffer);
+ if (EFI_ERROR(Status)) {
+ DEBUG ((DEBUG_ERROR,
+ "Error getting FMP protocol for runtime ImageTypeIdGuid=%g: %r\n",
+ Guid, Status));
+ continue;
+ }
+
+ FmpArrayIndex++;
+ }
+
+ mLibAtRuntimeFunction = EfiAtRuntime;
+}
+
/**
Notify function for event group EFI_EVENT_GROUP_READY_TO_BOOT.

@@ -93,6 +210,8 @@ DxeCapsuleLibReadyToBootEventNotify (
//
mEsrtTable->FwResourceCountMax = mEsrtTable->FwResourceCount;
}
+
+ InitializeRuntimeFmpArrays ();
}

/**
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
index bf56f4623f..7b3f5e04f8 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
@@ -49,6 +49,7 @@
PrintLib
HobLib
BmpSupportLib
+ PcdLib


[Protocols]
@@ -70,5 +71,8 @@
gEfiEventVirtualAddressChangeGuid ## CONSUMES ## Event
gEdkiiCapsuleOnDiskNameGuid ## SOMETIMES_CONSUMES ## GUID

+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdRuntimeFmpCapsuleImageTypeIdGuid
+
[Depex]
gEfiVariableWriteArchProtocolGuid
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 133e04ee86..869aa892f7 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -3,7 +3,7 @@
# It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library classes)
# and libraries instances, which are used for those modules.
#
-# Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
+# Copyright (c) 2019-2021, NVIDIA CORPORATION. All rights reserved.<BR>
# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
# (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP<BR>
@@ -2020,6 +2020,11 @@
# @Prompt Capsule On Disk Temp Relocation file name in PEI phase
gEfiMdeModulePkgTokenSpaceGuid.PcdCoDRelocationFileName|L"Cod.tmp"|VOID*|0x30001048

+ ## This PCD holds a list of GUIDs for the ImageTypeId to indicate the
+ # FMP is runtime capable.
+ # @Prompt A list of runtime-capable FMP ImageTypeId GUIDs
+ gEfiMdeModulePkgTokenSpaceGuid.PcdRuntimeFmpCapsuleImageTypeIdGuid|{0x0}|VOID*|0x30001049
+
## This PCD hold a list GUIDs for the ImageTypeId to indicate the
# FMP capsule is a system FMP.
# @Prompt A list of system FMP ImageTypeId GUIDs
--
2.17.1


[PATCH v10 23/32] UefiCpuPkg: add PcdGhcbHypervisorFeatures

Brijesh Singh
 

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

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

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

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

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

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

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


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

Brijesh Singh
 

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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


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

Brijesh Singh
 

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

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

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

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

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

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

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

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

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


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

Brijesh Singh
 

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

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

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 3c05b550e4bd..1c56ba275835 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -111,6 +111,8 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures

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

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

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

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

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

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


[PATCH v10 17/32] OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI phase

Brijesh Singh
 

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

The initial page built during the SEC phase is used by the
MemEncryptSevSnpValidateSystemRam() for the system RAM validation. The
page validation process requires using the PVALIDATE instruction; the
instruction accepts a virtual address of the memory region that needs
to be validated. If hardware encounters a page table walk failure (due
to page-not-present) then it raises #GP.

The initial page table built in SEC phase address up to 4GB. Add an
internal function to extend the page table to cover > 4GB. The function
builds 1GB entries in the page table for access > 4GB. This will provide
the support to call PVALIDATE instruction for the virtual address >
4GB in PEI phase.

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

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
index 21bbbd1c4f9c..9e5cdae25245 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
@@ -143,4 +143,28 @@ InternalMemEncryptSevClearMmioPageEncMask (
IN PHYSICAL_ADDRESS PhysicalAddress,
IN UINTN Length
);
+
+/**
+ Create 1GB identity mapping for the specified virtual address range.
+
+ The function is preliminary used by the SEV-SNP page state change
+ APIs to build the page table required before issuing the PVALIDATE
+ instruction. The function must be removed after the EDK2 core is
+ enhanced to do the lazy validation.
+
+ @param[in] Cr3BaseAddress Cr3 Base Address (if zero then use
+ current CR3)
+ @param[in] VirtualAddress Virtual address
+ @param[in] Length Length of virtual address range
+
+ @retval RETURN_INVALID_PARAMETER Number of pages is zero.
+
+**/
+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevCreateIdentityMap1G (
+ IN PHYSICAL_ADDRESS Cr3BaseAddress,
+ IN PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINTN Length
+ );
#endif
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index c696745f9d26..f146f6d61cc5 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -536,6 +536,121 @@ EnableReadOnlyPageWriteProtect (
AsmWriteCr0 (AsmReadCr0() | BIT16);
}

+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevCreateIdentityMap1G (
+ IN PHYSICAL_ADDRESS Cr3BaseAddress,
+ IN PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINTN Length
+ )
+{
+ PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
+ PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
+ UINT64 PgTableMask;
+ UINT64 AddressEncMask;
+ BOOLEAN IsWpEnabled;
+ RETURN_STATUS Status;
+
+ //
+ // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
+ //
+ PageMapLevel4Entry = NULL;
+
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx\n",
+ gEfiCallerBaseName,
+ __FUNCTION__,
+ Cr3BaseAddress,
+ PhysicalAddress,
+ (UINT64)Length
+ ));
+
+ if (Length == 0) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ //
+ // Check if we have a valid memory encryption mask
+ //
+ AddressEncMask = InternalGetMemEncryptionAddressMask ();
+ if (!AddressEncMask) {
+ return RETURN_ACCESS_DENIED;
+ }
+
+ PgTableMask = AddressEncMask | EFI_PAGE_MASK;
+
+
+ //
+ // Make sure that the page table is changeable.
+ //
+ IsWpEnabled = IsReadOnlyPageWriteProtected ();
+ if (IsWpEnabled) {
+ DisableReadOnlyPageWriteProtect ();
+ }
+
+ Status = EFI_SUCCESS;
+
+ while (Length)
+ {
+ //
+ // If Cr3BaseAddress is not specified then read the current CR3
+ //
+ if (Cr3BaseAddress == 0) {
+ Cr3BaseAddress = AsmReadCr3();
+ }
+
+ PageMapLevel4Entry = (VOID*) (Cr3BaseAddress & ~PgTableMask);
+ PageMapLevel4Entry += PML4_OFFSET(PhysicalAddress);
+ if (!PageMapLevel4Entry->Bits.Present) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a:%a: bad PML4 for Physical=0x%Lx\n",
+ gEfiCallerBaseName,
+ __FUNCTION__,
+ PhysicalAddress
+ ));
+ Status = RETURN_NO_MAPPING;
+ goto Done;
+ }
+
+ PageDirectory1GEntry = (VOID *)(
+ (PageMapLevel4Entry->Bits.PageTableBaseAddress <<
+ 12) & ~PgTableMask
+ );
+ PageDirectory1GEntry += PDP_OFFSET(PhysicalAddress);
+ if (!PageDirectory1GEntry->Bits.Present) {
+ PageDirectory1GEntry->Bits.Present = 1;
+ PageDirectory1GEntry->Bits.MustBe1 = 1;
+ PageDirectory1GEntry->Bits.MustBeZero = 0;
+ PageDirectory1GEntry->Bits.ReadWrite = 1;
+ PageDirectory1GEntry->Uint64 |= (UINT64)PhysicalAddress | AddressEncMask;
+ }
+
+ if (Length <= BIT30) {
+ Length = 0;
+ } else {
+ Length -= BIT30;
+ }
+
+ PhysicalAddress += BIT30;
+ }
+
+ //
+ // Flush TLB
+ //
+ CpuFlushTlb();
+
+Done:
+ //
+ // Restore page table write protection, if any.
+ //
+ if (IsWpEnabled) {
+ EnableReadOnlyPageWriteProtect ();
+ }
+
+ return Status;
+}

/**
This function either sets or clears memory encryption bit for the memory
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
index cea7ecf96563..ee8b5bc8011f 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
@@ -10,9 +10,12 @@

#include <Uefi/UefiBaseType.h>
#include <Library/BaseLib.h>
+#include <Library/PcdLib.h>
+#include <Library/DebugLib.h>
#include <Library/MemEncryptSevLib.h>

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

typedef struct {
UINT64 StartAddress;
@@ -69,6 +72,7 @@ MemEncryptSevSnpPreValidateSystemRam (
{
PHYSICAL_ADDRESS EndAddress;
SNP_PRE_VALIDATED_RANGE OverlapRange;
+ EFI_STATUS Status;

if (!MemEncryptSevSnpIsEnabled ()) {
return;
@@ -76,6 +80,24 @@ MemEncryptSevSnpPreValidateSystemRam (

EndAddress = BaseAddress + EFI_PAGES_TO_SIZE (NumPages);

+ //
+ // The page table used in PEI can address up to 4GB memory. If we are asked to
+ // validate a range above the 4GB, then create an identity mapping so that the
+ // PVALIDATE instruction can execute correctly. If the page table entry is not
+ // present then PVALIDATE will #GP.
+ //
+ if (BaseAddress >= SIZE_4GB) {
+ Status = InternalMemEncryptSevCreateIdentityMap1G (
+ 0,
+ BaseAddress,
+ EFI_PAGES_TO_SIZE (NumPages)
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ }
+ }
+
while (BaseAddress < EndAddress) {
//
// Check if the range overlaps with the pre-validated ranges.
--
2.25.1


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

Brijesh Singh
 

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

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

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

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

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

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

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

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

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

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


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

Brijesh Singh
 

From: Michael Roth <michael.roth@amd.com>

During AP bringup, just after switching to long mode, APs will do some
cpuid calls to verify that the extended topology leaf (0xB) is available
so they can fetch their x2 APIC IDs from it. In the case of SEV-ES,
these cpuid instructions must be handled by direct use of the GHCB MSR
protocol to fetch the values from the hypervisor, since a #VC handler
is not yet available due to the AP's stack not being set up yet.

For SEV-SNP, rather than relying on the GHCB MSR protocol, it is
expected that these values would be obtained from the SEV-SNP CPUID
table instead. The actual x2 APIC ID (and 8-bit APIC IDs) would still
be fetched from hypervisor using the GHCB MSR protocol however, so
introducing support for the SEV-SNP CPUID table in that part of the AP
bring-up code would only be to handle the checks/validation of the
extended topology leaf.

Rather than introducing all the added complexity needed to handle these
checks via the CPUID table, instead let the BSP do the check in advance,
since it can make use of the #VC handler to avoid the need to scan the
SNP CPUID table directly, and add a flag in ExchangeInfo to communicate
the result of this check to APs.

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

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

#pragma pack()
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index b05eb4413a95..9a9d9100d543 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -853,6 +853,7 @@ FillExchangeInfoData (
UINTN Size;
IA32_SEGMENT_DESCRIPTOR *Selector;
IA32_CR4 Cr4;
+ UINT32 StdRangeMax;

ExchangeInfo = CpuMpData->MpCpuExchangeInfo;
ExchangeInfo->StackStart = CpuMpData->Buffer;
@@ -892,6 +893,16 @@ FillExchangeInfoData (
ExchangeInfo->SevSnpIsEnabled = CpuMpData->SevSnpIsEnabled;
ExchangeInfo->GhcbBase = (UINTN) CpuMpData->GhcbBase;

+ if (ExchangeInfo->SevSnpIsEnabled) {
+ AsmCpuid (CPUID_SIGNATURE, &StdRangeMax, NULL, NULL, NULL);
+ if (StdRangeMax >= CPUID_EXTENDED_TOPOLOGY) {
+ CPUID_EXTENDED_TOPOLOGY_EBX ExtTopoEbx;
+
+ AsmCpuid (CPUID_EXTENDED_TOPOLOGY, NULL, &ExtTopoEbx.Uint32, NULL, NULL);
+ ExchangeInfo->ExtTopoAvail = !!ExtTopoEbx.Bits.LogicalProcessors;
+ }
+ }
+
//
// Get the BSP's data of GDT and IDT
//
diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
index 01668638f245..aba53f57201c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -94,6 +94,7 @@ struc MP_CPU_EXCHANGE_INFO
.SevEsIsEnabled: CTYPE_BOOLEAN 1
.SevSnpIsEnabled CTYPE_BOOLEAN 1
.GhcbBase: CTYPE_UINTN 1
+ .ExtTopoAvail: CTYPE_BOOLEAN 1
endstruc

MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
index 0034920b2f6b..8bb1161fa0f7 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
@@ -118,6 +118,32 @@ SevEsGetApicId:
or rax, rdx
mov rdi, rax ; RDI now holds the original GHCB GPA

+ ;
+ ; For SEV-SNP, the recommended handling for getting the x2APIC ID
+ ; would be to use the SNP CPUID table to fetch CPUID.00H:EAX and
+ ; CPUID:0BH:EBX[15:0] instead of the GHCB MSR protocol vmgexits
+ ; below.
+ ;
+ ; To avoid the unecessary ugliness to accomplish that here, the BSP
+ ; has performed these checks in advance (where #VC handler handles
+ ; the CPUID table lookups automatically) and cached them in a flag
+ ; so those checks can be skipped here.
+ ;
+ mov eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevSnpIsEnabled)]
+ cmp al, 1
+ jne CheckExtTopoAvail
+
+ ;
+ ; Even with SEV-SNP, the actual x2APIC ID in CPUID.0BH:EDX
+ ; fetched from the hypervisor the same way SEV-ES does it.
+ ;
+ mov eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ExtTopoAvail)]
+ cmp al, 1
+ je GetApicIdSevEs
+ ; The 8-bit APIC ID fallback is also the same as with SEV-ES
+ jmp NoX2ApicSevEs
+
+CheckExtTopoAvail:
mov rdx, 0 ; CPUID function 0
mov rax, 0 ; RAX register requested
or rax, 4
@@ -136,6 +162,7 @@ SevEsGetApicId:
test edx, 0ffffh
jz NoX2ApicSevEs ; CPUID.0BH:EBX[15:0] is zero

+GetApicIdSevEs:
mov rdx, 0bh ; CPUID function 0x0b
mov rax, 0c0000000h ; RDX register requested
or rax, 4
--
2.25.1


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

Brijesh Singh
 

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

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

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

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

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

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

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

Status = EFI_SUCCESS;

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

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


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

Brijesh Singh
 

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

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

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

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

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

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

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

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

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

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

%define SIZE_4KB 0x1000

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

SevEsSetupGhcbExit:
--
2.25.1


[PATCH v10 21/32] OvmfPkg/PlatformPei: set PcdConfidentialComputingAttr when SEV is active

Brijesh Singh
 

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

The MpInitLib uses the ConfidentialComputingAttr PCD to determine whether
AMD SEV is active so that it can use the VMGEXITs defined in the GHCB
specification to create APs.

Cc: Michael Roth <michael.roth@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Suggested-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 3 +++
OvmfPkg/OvmfPkgIa32.dsc | 3 +++
OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
OvmfPkg/OvmfPkgX64.dsc | 3 +++
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/AmdSev.c | 15 +++++++++++++++
6 files changed, 28 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 2997929faa05..8f5876341e26 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -575,6 +575,9 @@ [PcdsDynamicDefault]

gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00

+ # Set ConfidentialComputing defaults
+ gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+
!if $(TPM_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
!endif
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 1dc069e42420..dbcfa5ab52ce 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -651,6 +651,9 @@ [PcdsDynamicDefault]
gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01

+ # Set ConfidentialComputing defaults
+ gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+
[PcdsDynamicHii]
!if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a766457e6bc6..e4597e7f03da 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -659,6 +659,9 @@ [PcdsDynamicDefault]
gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
!endif

+ # Set ConfidentialComputing defaults
+ gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+
[PcdsDynamicDefault.X64]
# IPv4 and IPv6 PXE Boot support.
gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 97b7cb40ff88..08837bf8ec97 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -662,6 +662,9 @@ [PcdsDynamicDefault]
gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01

+ # Set ConfidentialComputing defaults
+ gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+
[PcdsDynamicHii]
!if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 67eb7aa7166b..bada5ea14439 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -106,6 +106,7 @@ [Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber
gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled
+ gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr

[FixedPcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 391e7bbb7dbd..5e2c891309d4 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -20,6 +20,7 @@
#include <Register/Amd/Msr.h>
#include <Register/Intel/SmramSaveStateMap.h>
#include <Library/VmgExitLib.h>
+#include <ConfidentialComputingGuestAttr.h>

#include "Platform.h"

@@ -342,4 +343,18 @@ AmdSevInitialize (
// Check and perform SEV-ES initialization if required.
//
AmdSevEsInitialize ();
+
+ //
+ // Set the Confidential computing attr PCD to communicate which SEV
+ // technology is active.
+ //
+ if (MemEncryptSevSnpIsEnabled ()) {
+ PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrAmdSevSnp);
+ } else if (MemEncryptSevEsIsEnabled ()) {
+ PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrAmdSevEs);
+ } else {
+ PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrAmdSev);
+ }
+ ASSERT_RETURN_ERROR (PcdStatus);
+
}
--
2.25.1


[PATCH v10 25/32] MdePkg/GHCB: increase the GHCB protocol max version

Brijesh Singh
 

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

Now that OvmfPkg supports version 2 of the GHCB specification, bump the
protocol version.

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

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 8c5f46e4bb53..071aae0c9e09 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -24,7 +24,7 @@
#define VC_EXCEPTION 29

#define GHCB_VERSION_MIN 1
-#define GHCB_VERSION_MAX 1
+#define GHCB_VERSION_MAX 2

#define GHCB_STANDARD_USAGE 0

--
2.25.1


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

Brijesh Singh
 

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

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

Query the hypervisor feature and set the PCD accordingly.

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

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

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

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

#include "Platform.h"

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

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

if (!MemEncryptSevSnpIsEnabled ()) {
return;
}

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

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

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

1 - 20 of 82316