Date   

Re: [PATCH 1/1] RedfishClientPkg: Facilities of EDK2 Redfish Feature driver Env.

Nickle Wang
 

Thanks Abner. V2 patch is sent to fix this issue.

Nickle

-----Original Message-----
From: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
Sent: Wednesday, October 13, 2021 5:03 PM
To: Wang, Nickle (HPS SW) <nickle.wang@...>; devel@edk2.groups.io
Cc: Liming Gao <gaoliming@...>
Subject: RE: [PATCH 1/1] RedfishClientPkg: Facilities of EDK2 Redfish Feature driver Env.

It's fine this time, but please add [staging/RedfishClientPkg] when next time send the patch against edk2-staging.

Only one comment in below, others look good to me.

Reviewed-by: Abner Chang <abner.chang@...>


-----Original Message-----
From: Wang, Nickle (HPS SW)
Sent: Wednesday, October 13, 2021 4:31 PM
To: devel@edk2.groups.io
Cc: Wang, Nickle (HPS SW) <nickle.wang@...>; Chang, Abner (HPS
SW/FW Technologist) <abner.chang@...>; Liming Gao
<gaoliming@...>
Subject: [PATCH 1/1] RedfishClientPkg: Facilities of EDK2 Redfish
Feature driver Env.

Initial common header file and meta files for feature drivers.

Signed-off-by: Nickle Wang <nickle.wang@...>
Cc: Abner Chang <abner.chang@...>
Cc: Liming Gao <gaoliming@...>
---
.../Include/Guid/RedfishClientPkgTokenSpace.h | 20 +++
.../EdkIIRedfishResourceConfigProtocol.h | 129 ++++++++++++++++++
.../Include/RedfishCollectionCommon.h | 53 +++++++
.../Include/RedfishResourceCommon.h | 123 +++++++++++++++++
RedfishClientPkg/RedfishClientPkg.dec | 8 +-
5 files changed, 332 insertions(+), 1 deletion(-) create mode 100644
RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h
create mode 100644
RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigProtocol.h
create mode 100644 RedfishClientPkg/Include/RedfishCollectionCommon.h
create mode 100644 RedfishClientPkg/Include/RedfishResourceCommon.h

diff --git
a/RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h
b/RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h
new file mode 100644
index 0000000000..1cdf429da4
--- /dev/null
+++ b/RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h
@@ -0,0 +1,20 @@
+/** @file

+ GUID for RedfishClientPkg PCD Token Space

+

+ (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>

+

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

+

+**/

+

+#ifndef _REDFISHCLIENTPKG_TOKEN_SPACE_GUID_H_

+#define _REDFISHCLIENTPKG_TOKEN_SPACE_GUID_H_
Please remove the prefix "_"

Abner


+

+#define REDFISHCLIENTPKG_TOKEN_SPACE_GUID \

+ { \

+ 0x8c444dae, 0x728b, 0x48ee, { 0x9e, 0x19, 0x8f, 0x0a, 0x3d, 0x4e,
+ 0x9c,
0xc8 } \

+ }

+

+extern EFI_GUID gEfiRedfishClientPkgTokenSpaceGuid;

+

+#endif

diff --git
a/RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigProtocol
.h
b/RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigProtocol
.h
new file mode 100644
index 0000000000..d6c41dda52
--- /dev/null
+++
b/RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigProtocol
.h
@@ -0,0 +1,129 @@
+/** @file

+ This file defines the EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL
interface.

+

+ (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>

+

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

+

+**/

+

+#ifndef EDKII_REDFISH_RESOURCE_CONFIG_H_

+#define EDKII_REDFISH_RESOURCE_CONFIG_H_

+

+typedef struct _EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL;

+

+/**

+ Provising redfish resource by given URI.

+

+ @param[in] This Pointer to
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL instance.

+ @param[in] Uri Target URI to create resource.

+ @param[in] HttpPostMode TRUE if resource does not exist, HTTP
POST method is used.

+ FALSE if the resource exist but
+ some of properties are
missing,

+ HTTP PUT method is used.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+typedef

+EFI_STATUS

+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_PROVISIONING)
(

+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,

+ IN CHAR8 *Uri,

+ IN BOOLEAN HttpPostMode

+ );

+

+/**

+ Consume resource from given URI.

+

+ @param[in] This Pointer to
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL instance.

+ @param[in] Uri The target URI to consume.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+typedef

+EFI_STATUS

+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CONSUME) (

+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,

+ IN CHAR8 *Uri

+ );

+

+

+/**

+ Update resource to given URI.

+

+ @param[in] This Pointer to
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL instance.

+ @param[in] Uri The target URI to consume.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+typedef

+EFI_STATUS

+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_UPDATE) (

+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,

+ IN CHAR8 *Uri

+ );

+

+

+/**

+ Check resource on given URI.

+

+ @param[in] This Pointer to
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL instance.

+ @param[in] Uri The target URI to consume.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+typedef

+EFI_STATUS

+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CHECK) (

+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,

+ IN CHAR8 *Uri

+ );

+

+//

+// definition of REDFISH_SCHEMA_INFO

+//

+#define REDFISH_SCHEMA_STRING_SIZE (FixedPcdGet32
(PcdMaxRedfishSchemaStringSize))

+#define REDFISH_SCHEMA_VERSION_SIZE (FixedPcdGet32
(PcdMaxRedfishSchemaVersionSize))

+

+typedef struct _SCHEMA_INFO {

+ CHAR8 Schema[REDFISH_SCHEMA_STRING_SIZE];

+ CHAR8 Major[REDFISH_SCHEMA_VERSION_SIZE];

+ CHAR8 Minor[REDFISH_SCHEMA_VERSION_SIZE];

+ CHAR8 Errata[REDFISH_SCHEMA_VERSION_SIZE];

+} REDFISH_SCHEMA_INFO;

+

+/**

+ Get information about this protocol.

+

+ @param[in] This Pointer to
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL instance.

+ @param[out] Info The schema information.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+typedef

+EFI_STATUS

+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_GET_INFO) (

+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,

+ OUT REDFISH_SCHEMA_INFO *Info

+ );

+

+struct _EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL {

+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_PROVISIONING
Provisioning;

+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CONSUME Consume;

+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_UPDATE Update;

+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CHECK Check;

+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_GET_INFO GetInfo;

+};

+

+extern EFI_GUID gEdkIIRedfishResourceConfigProtocolGuid;

+

+#endif

diff --git a/RedfishClientPkg/Include/RedfishCollectionCommon.h
b/RedfishClientPkg/Include/RedfishCollectionCommon.h
new file mode 100644
index 0000000000..3962b361ed
--- /dev/null
+++ b/RedfishClientPkg/Include/RedfishCollectionCommon.h
@@ -0,0 +1,53 @@
+/** @file

+ Redfish feature driver collection common header file.

+

+ (C) Copyright 2020-2021 Hewlett Packard Enterprise Development
+ LP<BR>

+

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

+

+**/

+

+#ifndef EFI_REDFISH_COLLECTION_COMMON_H_

+#define EFI_REDFISH_COLLECTION_COMMON_H_

+

+//

+// Libraries

+//

+#include <Library/BaseMemoryLib.h>

+#include <Library/DebugLib.h>

+#include <Library/MemoryAllocationLib.h>

+#include <Library/PrintLib.h>

+#include <Library/PcdLib.h>

+#include <Library/RedfishLib.h>

+#include <Library/RedfishFeatureUtilityLib.h>

+#include <Library/UefiLib.h>

+#include <Library/UefiBootServicesTableLib.h>

+

+//

+// Protocols

+//

+#include <Protocol/EdkIIRedfishConfigHandler.h>

+#include <Protocol/EdkIIRedfishResourceConfigProtocol.h>

+#include <Protocol/EdkIIRedfishFeature.h>

+#include <Protocol/RestJsonStructure.h>

+#include <Protocol/RestEx.h>

+

+#define IS_EMPTY_STRING(a) ((a) == NULL || (a)[0] == '\0')

+#define REDFISH_DEBUG_TRACE DEBUG_INFO

+

+typedef struct _REDFISH_COLLECTION_PRIVATE {

+ EFI_REST_JSON_STRUCTURE_PROTOCOL *JsonStructProtocol;

+ EDKII_REDFISH_FEATURE_PROTOCOL *FeatureProtocol;

+ REDFISH_SERVICE RedfishService;

+ EDKII_REDFISH_CONFIG_HANDLER_PROTOCOL ConfigHandler;

+ EFI_EVENT Event;

+ CHAR8 *CollectionPath;

+ CHAR8 *CollectionJson;

+ REDFISH_PAYLOAD CollectionPayload;

+ REDFISH_RESPONSE RedResponse;

+} REDFISH_COLLECTION_PRIVATE;

+

+#define REDFISH_COLLECTION_PRIVATE_DATA_FROM_PROTOCOL(This) \

+ BASE_CR ((This), REDFISH_COLLECTION_PRIVATE, ConfigHandler)

+

+#endif

diff --git a/RedfishClientPkg/Include/RedfishResourceCommon.h
b/RedfishClientPkg/Include/RedfishResourceCommon.h
new file mode 100644
index 0000000000..1ba992bb69
--- /dev/null
+++ b/RedfishClientPkg/Include/RedfishResourceCommon.h
@@ -0,0 +1,123 @@
+/** @file

+ Redfish feature driver common header file.

+

+ (C) Copyright 2020-2021 Hewlett Packard Enterprise Development
+ LP<BR>

+

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

+

+**/

+

+#ifndef EFI_REDFISH_RESOURCE_COMMON_H_

+#define EFI_REDFISH_RESOURCE_COMMON_H_

+

+#define MAX_RED_PATH_LEN 128

+#define IS_EMPTY_STRING(a) ((a) == NULL || (a)[0] == '\0')

+#define REDFISH_DEBUG_TRACE DEBUG_INFO

+

+//

+// Libraries

+//

+#include <Library/DebugLib.h>

+#include <Library/BaseMemoryLib.h>

+#include <Library/MemoryAllocationLib.h>

+#include <Library/PrintLib.h>

+#include <Library/PcdLib.h>

+#include <Library/RedfishLib.h>

+#include <Library/RedfishFeatureUtilityLib.h>

+#include <Library/RedfishPlatformConfigLib.h>

+#include <Library/UefiLib.h>

+#include <Library/UefiBootServicesTableLib.h>

+

+//

+// Protocols

+//

+#include <Protocol/EdkIIRedfishConfigHandler.h>

+#include <Protocol/EdkIIRedfishResourceConfigProtocol.h>

+#include <Protocol/RestJsonStructure.h>

+#include <Protocol/RestEx.h>

+

+

+typedef struct _REDFISH_RESOURCE_COMMON_PRIVATE {

+ REDFISH_SERVICE RedfishService;

+ EFI_REST_JSON_STRUCTURE_PROTOCOL *JsonStructProtocol;

+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL RedfishResourceConfig;

+ EDKII_REDFISH_CONFIG_HANDLER_PROTOCOL ConfigHandler;

+ EFI_EVENT Event;

+ CHAR8 *Uri;

+ CHAR8 *Json;

+ REDFISH_PAYLOAD Payload;

+} REDFISH_RESOURCE_COMMON_PRIVATE;

+

+#define
REDFISH_RESOURCE_COMMON_PRIVATE_DATA_FROM_CONFIG_PROTOCO
L(This) \

+ BASE_CR ((This), REDFISH_RESOURCE_COMMON_PRIVATE,
ConfigHandler)

+

+#define
REDFISH_RESOURCE_COMMON_PRIVATE_DATA_FROM_RESOURCE_PROTO
COL(This) \

+ BASE_CR ((This), REDFISH_RESOURCE_COMMON_PRIVATE,
RedfishResourceConfig)

+

+

+/**

+ Consume resource from given URI.

+

+ @param[in] This Pointer to
REDFISH_RESOURCE_COMMON_PRIVATE instance.

+ @param[in] Json The JSON to consume.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+EFI_STATUS

+RedfishConsumeResourceCommon (

+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,

+ IN CHAR8 *Json

+ );

+

+/**

+ Provisioning redfish resource by given URI.

+

+ @param[in] This Pointer to EFI_HP_REDFISH_HII_PROTOCOL
instance.

+ @param[in] ResourceExist TRUE if resource exists, PUT method will be
used.

+ FALSE if resource does not exist POST method is used.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+EFI_STATUS

+RedfishProvisioningResourceCommon (

+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,

+ IN BOOLEAN ResourceExist

+ );

+

+/**

+ Check resource from given URI.

+

+ @param[in] This Pointer to
REDFISH_RESOURCE_COMMON_PRIVATE instance.

+ @param[in] Json The JSON to consume.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+EFI_STATUS

+RedfishCheckResourceCommon (

+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,

+ IN CHAR8 *Json

+ );

+

+/**

+ Update resource to given URI.

+

+ @param[in] This Pointer to
REDFISH_RESOURCE_COMMON_PRIVATE instance.

+ @param[in] Json The JSON to consume.

+

+ @retval EFI_SUCCESS Value is returned successfully.

+ @retval Others Some error happened.

+

+**/

+EFI_STATUS

+RedfishUpdateResourceCommon (

+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,

+ IN CHAR8 *Json

+ );

+

+#endif

diff --git a/RedfishClientPkg/RedfishClientPkg.dec
b/RedfishClientPkg/RedfishClientPkg.dec
index 8fa92d5b1e..f01509d78f 100644
--- a/RedfishClientPkg/RedfishClientPkg.dec
+++ b/RedfishClientPkg/RedfishClientPkg.dec
@@ -21,9 +21,12 @@


[Protocols]

## Include/Protocol/EdkIIRedfishFeature.h

- gEdkIIRedfishFeatureProtocolGuid = { 0x785CC694, 0x4930, 0xEFBF, {
0x2A, 0xCB, 0xA4, 0xB6, 0xA1, 0xCC, 0xAA, 0x34 } }

+ gEdkIIRedfishFeatureProtocolGuid = { 0x785CC694, 0x4930, 0xEFBF,
{ 0x2A, 0xCB, 0xA4, 0xB6, 0xA1, 0xCC, 0xAA, 0x34 } }

+ ## Include/Protocol/EdkIIRedfishResourceConfigProtocol.h

+ gEdkIIRedfishResourceConfigProtocolGuid = { 0x6f164c68, 0xfb09,
+ 0x4646,
{ 0xa8, 0xd3, 0x24, 0x11, 0x5d, 0xab, 0x3e, 0xe7 } }



[Guids]

+ ## Include/Guid/RedfishClientPkgTokenSpace.h

gEfiRedfishClientPkgTokenSpaceGuid = { 0x8c444dae, 0x728b, 0x48ee,
{ 0x9e, 0x19, 0x8f, 0x0a, 0x3d, 0x4e, 0x9c, 0xc8 } }



[PcdsFixedAtBuild]

@@ -35,3 +38,6 @@
#


gEfiRedfishClientPkgTokenSpaceGuid.PcdEdkIIRedfishFeatureDriverStartup
E
ventGuid|{0xB3, 0x8F, 0xE8, 0x7C, 0xD7, 0x4B, 0x79, 0x46, 0x87, 0xA8,
ventGuid|0xA8,
0xD8, 0xDE, 0xE5, 0x0D, 0x2B}|VOID*|0x10000003



+[PcdsFixedAtBuild]

+
gEfiRedfishClientPkgTokenSpaceGuid.PcdMaxRedfishSchemaStringSize|32|
UINT32|0x10000001

+
gEfiRedfishClientPkgTokenSpaceGuid.PcdMaxRedfishSchemaVersionSize|8|
UINT32|0x10000002

--
2.31.1.windows.1


[PATCH V2 0/1] staging/RedfishClientPkg: Facilities of EDK2 Redfish Feature driver Env.

Nickle Wang
 

Address V1 review comment.

This commit adds below protocol and common header files for Redfish
feature drivers.
- edk2 Redfish resource config protocol: this protocol is used by feature
drivers for internal communication
- Redfish collection and resource common header file: these two files are
the common definition of Redfish client feature drivers.
- Redfish client package token space: the GUID definition of Redfish
client package token space.

Signed-off-by: Nickle Wang <nickle.wang@...>
Cc: Abner Chang <abner.chang@...>
Cc: Liming Gao <gaoliming@...>

Nickle Wang (1):
staging/RedfishClientPkg: Facilities of EDK2 Redfish Feature driver
Env.

.../Include/Guid/RedfishClientPkgTokenSpace.h | 20 +++
.../EdkIIRedfishResourceConfigProtocol.h | 129 ++++++++++++++++++
.../Include/RedfishCollectionCommon.h | 53 +++++++
.../Include/RedfishResourceCommon.h | 123 +++++++++++++++++
RedfishClientPkg/RedfishClientPkg.dec | 8 +-
5 files changed, 332 insertions(+), 1 deletion(-)
create mode 100644 RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h
create mode 100644 RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigProtocol.h
create mode 100644 RedfishClientPkg/Include/RedfishCollectionCommon.h
create mode 100644 RedfishClientPkg/Include/RedfishResourceCommon.h

--
2.31.1.windows.1


[PATCH V2 1/1] staging/RedfishClientPkg: Facilities of EDK2 Redfish Feature driver Env.

Nickle Wang
 

Initial common header file and meta files for feature drivers.

Signed-off-by: Nickle Wang <nickle.wang@...>
Cc: Abner Chang <abner.chang@...>
Cc: Liming Gao <gaoliming@...>
---
.../Include/Guid/RedfishClientPkgTokenSpace.h | 20 +++
.../EdkIIRedfishResourceConfigProtocol.h | 129 ++++++++++++++++++
.../Include/RedfishCollectionCommon.h | 53 +++++++
.../Include/RedfishResourceCommon.h | 123 +++++++++++++++++
RedfishClientPkg/RedfishClientPkg.dec | 8 +-
5 files changed, 332 insertions(+), 1 deletion(-)
create mode 100644 RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpac=
e.h
create mode 100644 RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceC=
onfigProtocol.h
create mode 100644 RedfishClientPkg/Include/RedfishCollectionCommon.h
create mode 100644 RedfishClientPkg/Include/RedfishResourceCommon.h

diff --git a/RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h b/R=
edfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h
new file mode 100644
index 0000000000..a2a09ed004
--- /dev/null
+++ b/RedfishClientPkg/Include/Guid/RedfishClientPkgTokenSpace.h
@@ -0,0 +1,20 @@
+/** @file=0D
+ GUID for RedfishClientPkg PCD Token Space=0D
+=0D
+ (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>=0D
+=0D
+ SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef REDFISHCLIENTPKG_TOKEN_SPACE_GUID_H_=0D
+#define REDFISHCLIENTPKG_TOKEN_SPACE_GUID_H_=0D
+=0D
+#define REDFISHCLIENTPKG_TOKEN_SPACE_GUID \=0D
+ { \=0D
+ 0x8c444dae, 0x728b, 0x48ee, { 0x9e, 0x19, 0x8f, 0x0a, 0x3d, 0x4e, 0x9c=
, 0xc8 } \=0D
+ }=0D
+=0D
+extern EFI_GUID gEfiRedfishClientPkgTokenSpaceGuid;=0D
+=0D
+#endif=0D
diff --git a/RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigPr=
otocol.h b/RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigProt=
ocol.h
new file mode 100644
index 0000000000..d6c41dda52
--- /dev/null
+++ b/RedfishClientPkg/Include/Protocol/EdkIIRedfishResourceConfigProtocol.h
@@ -0,0 +1,129 @@
+/** @file=0D
+ This file defines the EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL interface.=
=0D
+=0D
+ (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>=0D
+=0D
+ SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef EDKII_REDFISH_RESOURCE_CONFIG_H_=0D
+#define EDKII_REDFISH_RESOURCE_CONFIG_H_=0D
+=0D
+typedef struct _EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL EDKII_REDFISH_RESOU=
RCE_CONFIG_PROTOCOL;=0D
+=0D
+/**=0D
+ Provising redfish resource by given URI.=0D
+=0D
+ @param[in] This Pointer to EDKII_REDFISH_RESOURCE_CONFI=
G_PROTOCOL instance.=0D
+ @param[in] Uri Target URI to create resource.=0D
+ @param[in] HttpPostMode TRUE if resource does not exist, HTTP P=
OST method is used.=0D
+ FALSE if the resource exist but some of=
properties are missing,=0D
+ HTTP PUT method is used.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+typedef=0D
+EFI_STATUS=0D
+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_PROVISIONING) (=0D
+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,=0D
+ IN CHAR8 *Uri,=0D
+ IN BOOLEAN HttpPostMode=0D
+ );=0D
+=0D
+/**=0D
+ Consume resource from given URI.=0D
+=0D
+ @param[in] This Pointer to EDKII_REDFISH_RESOURCE_CONFI=
G_PROTOCOL instance.=0D
+ @param[in] Uri The target URI to consume.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+typedef=0D
+EFI_STATUS=0D
+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CONSUME) (=0D
+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,=0D
+ IN CHAR8 *Uri=0D
+ );=0D
+=0D
+=0D
+/**=0D
+ Update resource to given URI.=0D
+=0D
+ @param[in] This Pointer to EDKII_REDFISH_RESOURCE_CONFI=
G_PROTOCOL instance.=0D
+ @param[in] Uri The target URI to consume.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+typedef=0D
+EFI_STATUS=0D
+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_UPDATE) (=0D
+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,=0D
+ IN CHAR8 *Uri=0D
+ );=0D
+=0D
+=0D
+/**=0D
+ Check resource on given URI.=0D
+=0D
+ @param[in] This Pointer to EDKII_REDFISH_RESOURCE_CONFI=
G_PROTOCOL instance.=0D
+ @param[in] Uri The target URI to consume.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+typedef=0D
+EFI_STATUS=0D
+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CHECK) (=0D
+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,=0D
+ IN CHAR8 *Uri=0D
+ );=0D
+=0D
+//=0D
+// definition of REDFISH_SCHEMA_INFO=0D
+//=0D
+#define REDFISH_SCHEMA_STRING_SIZE (FixedPcdGet32 (PcdMaxRedfishSchemaStr=
ingSize))=0D
+#define REDFISH_SCHEMA_VERSION_SIZE (FixedPcdGet32 (PcdMaxRedfishSchemaVer=
sionSize))=0D
+=0D
+typedef struct _SCHEMA_INFO {=0D
+ CHAR8 Schema[REDFISH_SCHEMA_STRING_SIZE];=0D
+ CHAR8 Major[REDFISH_SCHEMA_VERSION_SIZE];=0D
+ CHAR8 Minor[REDFISH_SCHEMA_VERSION_SIZE];=0D
+ CHAR8 Errata[REDFISH_SCHEMA_VERSION_SIZE];=0D
+} REDFISH_SCHEMA_INFO;=0D
+=0D
+/**=0D
+ Get information about this protocol.=0D
+=0D
+ @param[in] This Pointer to EDKII_REDFISH_RESOURCE_CONFI=
G_PROTOCOL instance.=0D
+ @param[out] Info The schema information.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+typedef=0D
+EFI_STATUS=0D
+(EFIAPI *EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_GET_INFO) (=0D
+ IN EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL *This,=0D
+ OUT REDFISH_SCHEMA_INFO *Info=0D
+ );=0D
+=0D
+struct _EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL {=0D
+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_PROVISIONING Provisioning;=0D
+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CONSUME Consume;=0D
+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_UPDATE Update;=0D
+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_CHECK Check;=0D
+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL_GET_INFO GetInfo;=0D
+};=0D
+=0D
+extern EFI_GUID gEdkIIRedfishResourceConfigProtocolGuid;=0D
+=0D
+#endif=0D
diff --git a/RedfishClientPkg/Include/RedfishCollectionCommon.h b/RedfishCl=
ientPkg/Include/RedfishCollectionCommon.h
new file mode 100644
index 0000000000..3962b361ed
--- /dev/null
+++ b/RedfishClientPkg/Include/RedfishCollectionCommon.h
@@ -0,0 +1,53 @@
+/** @file=0D
+ Redfish feature driver collection common header file.=0D
+=0D
+ (C) Copyright 2020-2021 Hewlett Packard Enterprise Development LP<BR>=0D
+=0D
+ SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef EFI_REDFISH_COLLECTION_COMMON_H_=0D
+#define EFI_REDFISH_COLLECTION_COMMON_H_=0D
+=0D
+//=0D
+// Libraries=0D
+//=0D
+#include <Library/BaseMemoryLib.h>=0D
+#include <Library/DebugLib.h>=0D
+#include <Library/MemoryAllocationLib.h>=0D
+#include <Library/PrintLib.h>=0D
+#include <Library/PcdLib.h>=0D
+#include <Library/RedfishLib.h>=0D
+#include <Library/RedfishFeatureUtilityLib.h>=0D
+#include <Library/UefiLib.h>=0D
+#include <Library/UefiBootServicesTableLib.h>=0D
+=0D
+//=0D
+// Protocols=0D
+//=0D
+#include <Protocol/EdkIIRedfishConfigHandler.h>=0D
+#include <Protocol/EdkIIRedfishResourceConfigProtocol.h>=0D
+#include <Protocol/EdkIIRedfishFeature.h>=0D
+#include <Protocol/RestJsonStructure.h>=0D
+#include <Protocol/RestEx.h>=0D
+=0D
+#define IS_EMPTY_STRING(a) ((a) =3D=3D NULL || (a)[0] =3D=3D =
'\0')=0D
+#define REDFISH_DEBUG_TRACE DEBUG_INFO=0D
+=0D
+typedef struct _REDFISH_COLLECTION_PRIVATE {=0D
+ EFI_REST_JSON_STRUCTURE_PROTOCOL *JsonStructProtocol;=0D
+ EDKII_REDFISH_FEATURE_PROTOCOL *FeatureProtocol;=0D
+ REDFISH_SERVICE RedfishService;=0D
+ EDKII_REDFISH_CONFIG_HANDLER_PROTOCOL ConfigHandler;=0D
+ EFI_EVENT Event;=0D
+ CHAR8 *CollectionPath;=0D
+ CHAR8 *CollectionJson;=0D
+ REDFISH_PAYLOAD CollectionPayload;=0D
+ REDFISH_RESPONSE RedResponse;=0D
+} REDFISH_COLLECTION_PRIVATE;=0D
+=0D
+#define REDFISH_COLLECTION_PRIVATE_DATA_FROM_PROTOCOL(This) \=0D
+ BASE_CR ((This), REDFISH_COLLECTION_PRIVATE, ConfigHandler)=0D
+=0D
+#endif=0D
diff --git a/RedfishClientPkg/Include/RedfishResourceCommon.h b/RedfishClie=
ntPkg/Include/RedfishResourceCommon.h
new file mode 100644
index 0000000000..1ba992bb69
--- /dev/null
+++ b/RedfishClientPkg/Include/RedfishResourceCommon.h
@@ -0,0 +1,123 @@
+/** @file=0D
+ Redfish feature driver common header file.=0D
+=0D
+ (C) Copyright 2020-2021 Hewlett Packard Enterprise Development LP<BR>=0D
+=0D
+ SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef EFI_REDFISH_RESOURCE_COMMON_H_=0D
+#define EFI_REDFISH_RESOURCE_COMMON_H_=0D
+=0D
+#define MAX_RED_PATH_LEN 128=0D
+#define IS_EMPTY_STRING(a) ((a) =3D=3D NULL || (a)[0] =3D=3D '\0')=0D
+#define REDFISH_DEBUG_TRACE DEBUG_INFO=0D
+=0D
+//=0D
+// Libraries=0D
+//=0D
+#include <Library/DebugLib.h>=0D
+#include <Library/BaseMemoryLib.h>=0D
+#include <Library/MemoryAllocationLib.h>=0D
+#include <Library/PrintLib.h>=0D
+#include <Library/PcdLib.h>=0D
+#include <Library/RedfishLib.h>=0D
+#include <Library/RedfishFeatureUtilityLib.h>=0D
+#include <Library/RedfishPlatformConfigLib.h>=0D
+#include <Library/UefiLib.h>=0D
+#include <Library/UefiBootServicesTableLib.h>=0D
+=0D
+//=0D
+// Protocols=0D
+//=0D
+#include <Protocol/EdkIIRedfishConfigHandler.h>=0D
+#include <Protocol/EdkIIRedfishResourceConfigProtocol.h>=0D
+#include <Protocol/RestJsonStructure.h>=0D
+#include <Protocol/RestEx.h>=0D
+=0D
+=0D
+typedef struct _REDFISH_RESOURCE_COMMON_PRIVATE {=0D
+ REDFISH_SERVICE RedfishService;=0D
+ EFI_REST_JSON_STRUCTURE_PROTOCOL *JsonStructProtocol;=0D
+ EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL RedfishResourceConfig;=0D
+ EDKII_REDFISH_CONFIG_HANDLER_PROTOCOL ConfigHandler;=0D
+ EFI_EVENT Event;=0D
+ CHAR8 *Uri;=0D
+ CHAR8 *Json;=0D
+ REDFISH_PAYLOAD Payload;=0D
+} REDFISH_RESOURCE_COMMON_PRIVATE;=0D
+=0D
+#define REDFISH_RESOURCE_COMMON_PRIVATE_DATA_FROM_CONFIG_PROTOCOL(This) \=
=0D
+ BASE_CR ((This), REDFISH_RESOURCE_COMMON_PRIVATE, ConfigHandler)=
=0D
+=0D
+#define REDFISH_RESOURCE_COMMON_PRIVATE_DATA_FROM_RESOURCE_PROTOCOL(This) =
\=0D
+ BASE_CR ((This), REDFISH_RESOURCE_COMMON_PRIVATE, RedfishResourc=
eConfig)=0D
+=0D
+=0D
+/**=0D
+ Consume resource from given URI.=0D
+=0D
+ @param[in] This Pointer to REDFISH_RESOURCE_COMMON_PRIV=
ATE instance.=0D
+ @param[in] Json The JSON to consume.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+EFI_STATUS=0D
+RedfishConsumeResourceCommon (=0D
+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,=0D
+ IN CHAR8 *Json=0D
+ );=0D
+=0D
+/**=0D
+ Provisioning redfish resource by given URI.=0D
+=0D
+ @param[in] This Pointer to EFI_HP_REDFISH_HII_PROTOCOL =
instance.=0D
+ @param[in] ResourceExist TRUE if resource exists, PUT method wil=
l be used.=0D
+ FALSE if resource does not exist POST m=
ethod is used.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+EFI_STATUS=0D
+RedfishProvisioningResourceCommon (=0D
+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,=0D
+ IN BOOLEAN ResourceExist=0D
+ );=0D
+=0D
+/**=0D
+ Check resource from given URI.=0D
+=0D
+ @param[in] This Pointer to REDFISH_RESOURCE_COMMON_PRIV=
ATE instance.=0D
+ @param[in] Json The JSON to consume.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+EFI_STATUS=0D
+RedfishCheckResourceCommon (=0D
+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,=0D
+ IN CHAR8 *Json=0D
+ );=0D
+=0D
+/**=0D
+ Update resource to given URI.=0D
+=0D
+ @param[in] This Pointer to REDFISH_RESOURCE_COMMON_PRIV=
ATE instance.=0D
+ @param[in] Json The JSON to consume.=0D
+=0D
+ @retval EFI_SUCCESS Value is returned successfully.=0D
+ @retval Others Some error happened.=0D
+=0D
+**/=0D
+EFI_STATUS=0D
+RedfishUpdateResourceCommon (=0D
+ IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,=0D
+ IN CHAR8 *Json=0D
+ );=0D
+=0D
+#endif=0D
diff --git a/RedfishClientPkg/RedfishClientPkg.dec b/RedfishClientPkg/Redfi=
shClientPkg.dec
index 8fa92d5b1e..f01509d78f 100644
--- a/RedfishClientPkg/RedfishClientPkg.dec
+++ b/RedfishClientPkg/RedfishClientPkg.dec
@@ -21,9 +21,12 @@
=0D
[Protocols]=0D
## Include/Protocol/EdkIIRedfishFeature.h=0D
- gEdkIIRedfishFeatureProtocolGuid =3D { 0x785CC694, 0x4930, 0xEFBF, { 0x2=
A, 0xCB, 0xA4, 0xB6, 0xA1, 0xCC, 0xAA, 0x34 } }=0D
+ gEdkIIRedfishFeatureProtocolGuid =3D { 0x785CC694, 0x4930, 0xEFBF=
, { 0x2A, 0xCB, 0xA4, 0xB6, 0xA1, 0xCC, 0xAA, 0x34 } }=0D
+ ## Include/Protocol/EdkIIRedfishResourceConfigProtocol.h=0D
+ gEdkIIRedfishResourceConfigProtocolGuid =3D { 0x6f164c68, 0xfb09, 0x4646=
, { 0xa8, 0xd3, 0x24, 0x11, 0x5d, 0xab, 0x3e, 0xe7 } }=0D
=0D
[Guids]=0D
+ ## Include/Guid/RedfishClientPkgTokenSpace.h=0D
gEfiRedfishClientPkgTokenSpaceGuid =3D { 0x8c444dae, 0x728b, 0x48ee, =
{ 0x9e, 0x19, 0x8f, 0x0a, 0x3d, 0x4e, 0x9c, 0xc8 } }=0D
=0D
[PcdsFixedAtBuild]=0D
@@ -35,3 +38,6 @@
#=0D
gEfiRedfishClientPkgTokenSpaceGuid.PcdEdkIIRedfishFeatureDriverStartupEv=
entGuid|{0xB3, 0x8F, 0xE8, 0x7C, 0xD7, 0x4B, 0x79, 0x46, 0x87, 0xA8, 0xA8, =
0xD8, 0xDE, 0xE5, 0x0D, 0x2B}|VOID*|0x10000003=0D
=0D
+[PcdsFixedAtBuild]=0D
+ gEfiRedfishClientPkgTokenSpaceGuid.PcdMaxRedfishSchemaStringSize|32|UINT=
32|0x10000001=0D
+ gEfiRedfishClientPkgTokenSpaceGuid.PcdMaxRedfishSchemaVersionSize|8|UINT=
32|0x10000002=0D
--=20
2.31.1.windows.1


Re: [PATCH v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

Brijesh Singh
 

Thank Jiewen,

I have ping'ed UefiCpuPkg maintainer (Ray and Rahul) on every patch
which touches the UefiCpuPkg. If maintainer wants me to rework on
something then I will work accordingly. If they are okay with v11 then
now the merge will create a conflict (due to the TDX patches merge
commit). I have rebased my series to the recent master and have pushed
it here: https://github.com/AMDESE/ovmf/tree/snp-v12. I can post the
series if you prefer it.

thanks

On 10/23/21 8:46 PM, Yao, Jiewen via groups.io wrote:
Yes. I will try my best to merge.

I checked the patch set but I did not find the "R-B" from UefiCpuPkg maintainer. Neither from email nor from you v11.

Did I miss something?

Thank you
Yao Jiewen


-----Original Message-----
From: Brijesh Singh <brijesh.singh@...>
Sent: Saturday, October 23, 2021 12:13 PM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@...>; Xu, Min M <min.m.xu@...>;
Yao, Jiewen <jiewen.yao@...>; Tom Lendacky
<thomas.lendacky@...>; Justen, Jordan L <jordan.l.justen@...>;
Ard Biesheuvel <ardb+tianocore@...>; Erdem Aktas
<erdemaktas@...>; Michael Roth <Michael.Roth@...>; Gerd
Hoffmann <kraxel@...>; Brijesh Singh <brijesh.singh@...>
Subject: [PATCH v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

Hi Gerd and Jiewen,

CI was a bit unstable during my v10 submission, so, I was not able to
run it to the completion. Finally, I managed to get the CI going,
and it reported few Windows 32-bit build errors. The v11 fixes those build
errors. Please consider this for the merge.

Thank you so much for all your support in reviewing the series.

-----------------------------------------------------------------------------
BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=L41krO6G221HaIsG92FloIzgCDqMLAAsU26jaEMF7yw%3D&amp;reserved=0

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

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

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

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

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

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

Now that series contains all the basic support required to launch SEV-SNP
guest. We are still missing the Interrupt security feature provided by the
SNP. The feature will be added after the base support is accepted.

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=nVMSG%2FvSS2Wa21lu1lGrHr9OYX8hL7FoAcQXBBiCztc%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=G8Xg2glOGY2EjHpeQ3WM4gZChuI0k8QcLDTbpJiTplg%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fovmf%2Ftree%2Fsnp-v11&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=HMHFq8G%2FPqdhzNW3Ashmc4%2Bmv1RcDULD4vniofhiS54%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fwp-content%2Fresources%2F56421.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=YiPgZU87fdnl5rJpD0E2ue9aTKbqUwizuBrKxom0FiU%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=bfQsY4%2BRnlFGuD3Bg%2BFPb3lRgSGgpomNocXswHqkm%2F4%3D&amp;reserved=0

Change since v10:
* fix 'unresolved external symbol __allshl' link error when building I32 for
VS2017.

Changes since v9:
* Move CCAttrs Pcd define in MdePkg
* Add comment to indicate that allocating the identity map PT is temporary until
we get lazy validation

Changes since v8:
* drop the generic metadata and make it specific to SEV.

Changes since v7:
* Move SEV specific changes in MpLib in AmdSev file
* Update the GHCB register function to not restore the GHCB MSR because
we were already in the MSR protocol mode.
* Drop the SNP name from PcdSnpSecPreValidate.
* Add new section for GHCB memory in the OVMF metadata.

Change since v6:
* Drop the SNP boot block GUID and switch to using the Metadata guided
structure
proposed by Min in TDX series.
* Exclude the GHCB page from the pre-validated region. It simplifies the reset
vector code where we do not need to unvalidate the GHCB page.
* Now that GHCB page is not validated so move the VMPL check from reset
vector
code to the MemEncryptSevLib on the first page validation.
* Introduce the ConfidentialComputingGuestAttr PCD to communicate which
memory encryption is active so that MpInitLib can make use of it.
* Drop the SEVES specific PCD as the information can be communicated via
the ConfidentialComputingGuestAttr.
* Move the SNP specific AP creation function in AmdSev.c.
* Define the SNP Blob GUID in a new file.

Change since v5:
* When possible use the CPUID value from CPUID page
* Move the SEV specific functions from SecMain.c in AmdSev.c
* Rebase to the latest code
* Add the review feedback from Yao.

Change since v4:
* Use the correct MSR for the SEV_STATUS
* Add VMPL-0 check

Change since v3:
* ResetVector: move all SEV specific code in AmdSev.asm and add macros to
keep
the code readable.
* Drop extending the EsWorkArea to contain SNP specific state.
* Drop the GhcbGpa library and call the VmgExit directly to register GHCB GPA.
* Install the CC blob config table from AmdSevDxe instead of extending the
AmdSev/SecretsDxe for it.
* Add the separate PCDs for the SNP Secrets.

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

Brijesh Singh (28):
OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
UefiCpuPkg/MpInitLib: move SEV specific routines in AmdSev.c
OvmfPkg/ResetVector: move clearing GHCB in SecMain
OvmfPkg/ResetVector: introduce SEV metadata descriptor for VMM use
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/MemEncryptSevLib: add function to check the VMPL0
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
UefiCpuPkg: Define ConfidentialComputingGuestAttr
OvmfPkg/PlatformPei: set PcdConfidentialComputingAttr when SEV is
active
UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV
status
UefiCpuPkg: add PcdGhcbHypervisorFeatures
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Michael Roth (3):
OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values
OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values
UefiCpuPkg/MpInitLib: use BSP to do extended topology check

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

MdePkg/MdePkg.dec | 4 +
OvmfPkg/OvmfPkg.dec | 18 +
UefiCpuPkg/UefiCpuPkg.dec | 5 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 8 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 4 +
OvmfPkg/OvmfPkgIa32X64.dsc | 9 +-
OvmfPkg/OvmfPkgX64.dsc | 8 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 6 +
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 7 +
OvmfPkg/ResetVector/ResetVector.inf | 5 +
OvmfPkg/Sec/SecMain.inf | 4 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 6 +-
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 6 +-
.../Include/ConfidentialComputingGuestAttr.h | 25 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSevSnpBlob.h | 33 ++
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 +
.../X64/SnpPageStateChange.h | 36 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 24 +
OvmfPkg/PlatformPei/Platform.h | 5 +
OvmfPkg/Sec/AmdSev.h | 95 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 93 ++++
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 127 +++++
.../X64/SecSnpSystemRamValidate.c | 82 ++++
.../X64/SnpPageStateChangeInternal.c | 294 ++++++++++++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
OvmfPkg/PlatformPei/AmdSev.c | 231 +++++++++
OvmfPkg/PlatformPei/MemDetect.c | 2 +
OvmfPkg/Sec/AmdSev.c | 298 ++++++++++++
OvmfPkg/Sec/SecMain.c | 158 +------
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 239 ++++++++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 16 +-
UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c | 70 +++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 345 +++++---------
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 4 +-
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c | 261 ++++++++++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 17 +
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 86 +++-
OvmfPkg/ResetVector/ResetVector.nasmb | 18 +
OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm | 74 +++
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 200 ++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 100 +---
59 files changed, 3329 insertions(+), 528 deletions(-)
create mode 100644 MdePkg/Include/ConfidentialComputingGuestAttr.h
create mode 100644
OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Sec/AmdSev.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Sec/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
create mode 100644 OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm

--
2.25.1




Re: [PATCH v11 02/32] UefiCpuPkg/MpInitLib: move SEV specific routines in AmdSev.c

Brijesh Singh
 

Hi Ray and Rahul,

Any comment on this patch ? If you are okay with it then can I get Ack
or R-b ?

-Brijesh

On 10/22/21 11:13 PM, Brijesh Singh wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Move all the SEV specific function in AmdSev.c.

No functional change intended.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Michael Roth <michael.roth@...>
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Suggested-by: Jiewen Yao <Jiewen.yao@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 33 +++
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 239 ++++++++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 218 +---------------
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 119 +++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 100 ++------
7 files changed, 413 insertions(+), 298 deletions(-)
create mode 100644 UefiCpuPkg/Library/MpInitLib/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index d34419c2a524..6e510aa89120 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -28,6 +28,7 @@ [Sources.X64]
X64/MpFuncs.nasm

[Sources.common]
+ AmdSev.c
MpEqu.inc
DxeMpLib.c
MpLib.c
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 36fcb96b5852..2cbd9b8b8acc 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -28,6 +28,7 @@ [Sources.X64]
X64/MpFuncs.nasm

[Sources.common]
+ AmdSev.c
MpEqu.inc
PeiMpLib.c
MpLib.c
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index e88a5355c983..3d4446df8ce6 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -34,6 +34,9 @@
#include <Library/PcdLib.h>
#include <Library/MicrocodeLib.h>

+#include <Register/Amd/Fam17Msr.h>
+#include <Register/Amd/Ghcb.h>
+
#include <Guid/MicrocodePatchHob.h>

#define WAKEUP_AP_SIGNAL SIGNATURE_32 ('S', 'T', 'A', 'P')
@@ -741,5 +744,35 @@ PlatformShadowMicrocode (
IN OUT CPU_MP_DATA *CpuMpData
);

+/**
+ Allocate the SEV-ES AP jump table buffer.
+
+ @param[in, out] CpuMpData The pointer to CPU MP Data structure.
+**/
+VOID
+AllocateSevEsAPMemory (
+ IN OUT CPU_MP_DATA *CpuMpData
+ );
+
+/**
+ Program the SEV-ES AP jump table buffer.
+
+ @param[in] SipiVector The SIPI vector used for the AP Reset
+**/
+VOID
+SetSevEsJumpTable (
+ IN UINTN SipiVector
+ );
+
+/**
+ The function puts the AP in halt loop.
+
+ @param[in] CpuMpData The pointer to CPU MP Data structure.
+**/
+VOID
+SevEsPlaceApHlt (
+ CPU_MP_DATA *CpuMpData
+ );
+
#endif

diff --git a/UefiCpuPkg/Library/MpInitLib/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
new file mode 100644
index 000000000000..7dbf117c2b71
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
@@ -0,0 +1,239 @@
+/** @file
+ CPU MP Initialize helper function for AMD SEV.
+
+ Copyright (c) 2021, AMD Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "MpLib.h"
+#include <Library/VmgExitLib.h>
+
+/**
+ Get Protected mode code segment with 16-bit default addressing
+ from current GDT table.
+
+ @return Protected mode 16-bit code segment value.
+**/
+STATIC
+UINT16
+GetProtectedMode16CS (
+ VOID
+ )
+{
+ IA32_DESCRIPTOR GdtrDesc;
+ IA32_SEGMENT_DESCRIPTOR *GdtEntry;
+ UINTN GdtEntryCount;
+ UINT16 Index;
+
+ Index = (UINT16) -1;
+ AsmReadGdtr (&GdtrDesc);
+ GdtEntryCount = (GdtrDesc.Limit + 1) / sizeof (IA32_SEGMENT_DESCRIPTOR);
+ GdtEntry = (IA32_SEGMENT_DESCRIPTOR *) GdtrDesc.Base;
+ for (Index = 0; Index < GdtEntryCount; Index++) {
+ if (GdtEntry->Bits.L == 0 &&
+ GdtEntry->Bits.DB == 0 &&
+ GdtEntry->Bits.Type > 8) {
+ break;
+ }
+ GdtEntry++;
+ }
+ ASSERT (Index != GdtEntryCount);
+ return Index * 8;
+}
+
+/**
+ Get Protected mode code segment with 32-bit default addressing
+ from current GDT table.
+
+ @return Protected mode 32-bit code segment value.
+**/
+STATIC
+UINT16
+GetProtectedMode32CS (
+ VOID
+ )
+{
+ IA32_DESCRIPTOR GdtrDesc;
+ IA32_SEGMENT_DESCRIPTOR *GdtEntry;
+ UINTN GdtEntryCount;
+ UINT16 Index;
+
+ Index = (UINT16) -1;
+ AsmReadGdtr (&GdtrDesc);
+ GdtEntryCount = (GdtrDesc.Limit + 1) / sizeof (IA32_SEGMENT_DESCRIPTOR);
+ GdtEntry = (IA32_SEGMENT_DESCRIPTOR *) GdtrDesc.Base;
+ for (Index = 0; Index < GdtEntryCount; Index++) {
+ if (GdtEntry->Bits.L == 0 &&
+ GdtEntry->Bits.DB == 1 &&
+ GdtEntry->Bits.Type > 8) {
+ break;
+ }
+ GdtEntry++;
+ }
+ ASSERT (Index != GdtEntryCount);
+ return Index * 8;
+}
+
+/**
+ Reset an AP when in SEV-ES mode.
+
+ If successful, this function never returns.
+
+ @param[in] Ghcb Pointer to the GHCB
+ @param[in] CpuMpData Pointer to CPU MP Data
+
+**/
+VOID
+MpInitLibSevEsAPReset (
+ IN GHCB *Ghcb,
+ IN CPU_MP_DATA *CpuMpData
+ )
+{
+ EFI_STATUS Status;
+ UINTN ProcessorNumber;
+ UINT16 Code16, Code32;
+ AP_RESET *APResetFn;
+ UINTN BufferStart;
+ UINTN StackStart;
+
+ Status = GetProcessorNumber (CpuMpData, &ProcessorNumber);
+ ASSERT_EFI_ERROR (Status);
+
+ Code16 = GetProtectedMode16CS ();
+ Code32 = GetProtectedMode32CS ();
+
+ if (CpuMpData->WakeupBufferHigh != 0) {
+ APResetFn = (AP_RESET *) (CpuMpData->WakeupBufferHigh + CpuMpData->AddressMap.SwitchToRealNoNxOffset);
+ } else {
+ APResetFn = (AP_RESET *) (CpuMpData->MpCpuExchangeInfo->BufferStart + CpuMpData->AddressMap.SwitchToRealOffset);
+ }
+
+ BufferStart = CpuMpData->MpCpuExchangeInfo->BufferStart;
+ StackStart = CpuMpData->SevEsAPResetStackStart -
+ (AP_RESET_STACK_SIZE * ProcessorNumber);
+
+ //
+ // This call never returns.
+ //
+ APResetFn (BufferStart, Code16, Code32, StackStart);
+}
+
+/**
+ Allocate the SEV-ES AP jump table buffer.
+
+ @param[in, out] CpuMpData The pointer to CPU MP Data structure.
+**/
+VOID
+AllocateSevEsAPMemory (
+ IN OUT CPU_MP_DATA *CpuMpData
+ )
+{
+ if (CpuMpData->SevEsAPBuffer == (UINTN) -1) {
+ CpuMpData->SevEsAPBuffer =
+ CpuMpData->SevEsIsEnabled ? GetSevEsAPMemory () : 0;
+ }
+}
+
+/**
+ Program the SEV-ES AP jump table buffer.
+
+ @param[in] SipiVector The SIPI vector used for the AP Reset
+**/
+VOID
+SetSevEsJumpTable (
+ IN UINTN SipiVector
+ )
+{
+ SEV_ES_AP_JMP_FAR *JmpFar;
+ UINT32 Offset, InsnByte;
+ UINT8 LoNib, HiNib;
+
+ JmpFar = (SEV_ES_AP_JMP_FAR *) (UINTN) FixedPcdGet32 (PcdSevEsWorkAreaBase);
+ ASSERT (JmpFar != NULL);
+
+ //
+ // Obtain the address of the Segment/Rip location in the workarea.
+ // This will be set to a value derived from the SIPI vector and will
+ // be the memory address used for the far jump below.
+ //
+ Offset = FixedPcdGet32 (PcdSevEsWorkAreaBase);
+ Offset += sizeof (JmpFar->InsnBuffer);
+ LoNib = (UINT8) Offset;
+ HiNib = (UINT8) (Offset >> 8);
+
+ //
+ // Program the workarea (which is the initial AP boot address) with
+ // far jump to the SIPI vector (where XX and YY represent the
+ // address of where the SIPI vector is stored.
+ //
+ // JMP FAR [CS:XXYY] => 2E FF 2E YY XX
+ //
+ InsnByte = 0;
+ JmpFar->InsnBuffer[InsnByte++] = 0x2E; // CS override prefix
+ JmpFar->InsnBuffer[InsnByte++] = 0xFF; // JMP (FAR)
+ JmpFar->InsnBuffer[InsnByte++] = 0x2E; // ModRM (JMP memory location)
+ JmpFar->InsnBuffer[InsnByte++] = LoNib; // YY offset ...
+ JmpFar->InsnBuffer[InsnByte++] = HiNib; // XX offset ...
+
+ //
+ // Program the Segment/Rip based on the SIPI vector (always at least
+ // 16-byte aligned, so Rip is set to 0).
+ //
+ JmpFar->Rip = 0;
+ JmpFar->Segment = (UINT16) (SipiVector >> 4);
+}
+
+/**
+ The function puts the AP in halt loop.
+
+ @param[in] CpuMpData The pointer to CPU MP Data structure.
+**/
+VOID
+SevEsPlaceApHlt (
+ CPU_MP_DATA *CpuMpData
+ )
+{
+ MSR_SEV_ES_GHCB_REGISTER Msr;
+ GHCB *Ghcb;
+ UINT64 Status;
+ BOOLEAN DoDecrement;
+ BOOLEAN InterruptState;
+
+ DoDecrement = (BOOLEAN) (CpuMpData->InitFlag == ApInitConfig);
+
+ while (TRUE) {
+ Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+ Ghcb = Msr.Ghcb;
+
+ VmgInit (Ghcb, &InterruptState);
+
+ if (DoDecrement) {
+ DoDecrement = FALSE;
+
+ //
+ // Perform the delayed decrement just before issuing the first
+ // VMGEXIT with AP_RESET_HOLD.
+ //
+ InterlockedDecrement ((UINT32 *) &CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
+ }
+
+ Status = VmgExit (Ghcb, SVM_EXIT_AP_RESET_HOLD, 0, 0);
+ if ((Status == 0) && (Ghcb->SaveArea.SwExitInfo2 != 0)) {
+ VmgDone (Ghcb, InterruptState);
+ break;
+ }
+
+ VmgDone (Ghcb, InterruptState);
+ }
+
+ //
+ // Awakened in a new phase? Use the new CpuMpData
+ //
+ if (CpuMpData->NewCpuMpData != NULL) {
+ CpuMpData = CpuMpData->NewCpuMpData;
+ }
+
+ MpInitLibSevEsAPReset (Ghcb, CpuMpData);
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index b9a06747edbf..890945bc5994 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -596,117 +596,6 @@ InitializeApData (
SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateIdle);
}

-/**
- Get Protected mode code segment with 16-bit default addressing
- from current GDT table.
-
- @return Protected mode 16-bit code segment value.
-**/
-STATIC
-UINT16
-GetProtectedMode16CS (
- VOID
- )
-{
- IA32_DESCRIPTOR GdtrDesc;
- IA32_SEGMENT_DESCRIPTOR *GdtEntry;
- UINTN GdtEntryCount;
- UINT16 Index;
-
- Index = (UINT16) -1;
- AsmReadGdtr (&GdtrDesc);
- GdtEntryCount = (GdtrDesc.Limit + 1) / sizeof (IA32_SEGMENT_DESCRIPTOR);
- GdtEntry = (IA32_SEGMENT_DESCRIPTOR *) GdtrDesc.Base;
- for (Index = 0; Index < GdtEntryCount; Index++) {
- if (GdtEntry->Bits.L == 0 &&
- GdtEntry->Bits.DB == 0 &&
- GdtEntry->Bits.Type > 8) {
- break;
- }
- GdtEntry++;
- }
- ASSERT (Index != GdtEntryCount);
- return Index * 8;
-}
-
-/**
- Get Protected mode code segment with 32-bit default addressing
- from current GDT table.
-
- @return Protected mode 32-bit code segment value.
-**/
-STATIC
-UINT16
-GetProtectedMode32CS (
- VOID
- )
-{
- IA32_DESCRIPTOR GdtrDesc;
- IA32_SEGMENT_DESCRIPTOR *GdtEntry;
- UINTN GdtEntryCount;
- UINT16 Index;
-
- Index = (UINT16) -1;
- AsmReadGdtr (&GdtrDesc);
- GdtEntryCount = (GdtrDesc.Limit + 1) / sizeof (IA32_SEGMENT_DESCRIPTOR);
- GdtEntry = (IA32_SEGMENT_DESCRIPTOR *) GdtrDesc.Base;
- for (Index = 0; Index < GdtEntryCount; Index++) {
- if (GdtEntry->Bits.L == 0 &&
- GdtEntry->Bits.DB == 1 &&
- GdtEntry->Bits.Type > 8) {
- break;
- }
- GdtEntry++;
- }
- ASSERT (Index != GdtEntryCount);
- return Index * 8;
-}
-
-/**
- Reset an AP when in SEV-ES mode.
-
- If successful, this function never returns.
-
- @param[in] Ghcb Pointer to the GHCB
- @param[in] CpuMpData Pointer to CPU MP Data
-
-**/
-STATIC
-VOID
-MpInitLibSevEsAPReset (
- IN GHCB *Ghcb,
- IN CPU_MP_DATA *CpuMpData
- )
-{
- EFI_STATUS Status;
- UINTN ProcessorNumber;
- UINT16 Code16, Code32;
- AP_RESET *APResetFn;
- UINTN BufferStart;
- UINTN StackStart;
-
- Status = GetProcessorNumber (CpuMpData, &ProcessorNumber);
- ASSERT_EFI_ERROR (Status);
-
- Code16 = GetProtectedMode16CS ();
- Code32 = GetProtectedMode32CS ();
-
- if (CpuMpData->WakeupBufferHigh != 0) {
- APResetFn = (AP_RESET *) (CpuMpData->WakeupBufferHigh + CpuMpData->AddressMap.SwitchToRealNoNxOffset);
- } else {
- APResetFn = (AP_RESET *) (CpuMpData->MpCpuExchangeInfo->BufferStart + CpuMpData->AddressMap.SwitchToRealOffset);
- }
-
- BufferStart = CpuMpData->MpCpuExchangeInfo->BufferStart;
- StackStart = CpuMpData->SevEsAPResetStackStart -
- (AP_RESET_STACK_SIZE * ProcessorNumber);
-
- //
- // This call never returns.
- //
- APResetFn (BufferStart, Code16, Code32, StackStart);
-}
-
/**
This function will be called from AP reset code if BSP uses WakeUpAP.

@@ -884,47 +773,7 @@ ApWakeupFunction (
while (TRUE) {
DisableInterrupts ();
if (CpuMpData->SevEsIsEnabled) {
- MSR_SEV_ES_GHCB_REGISTER Msr;
- GHCB *Ghcb;
- UINT64 Status;
- BOOLEAN DoDecrement;
- BOOLEAN InterruptState;
-
- DoDecrement = (BOOLEAN) (CpuMpData->InitFlag == ApInitConfig);
-
- while (TRUE) {
- Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
- Ghcb = Msr.Ghcb;
-
- VmgInit (Ghcb, &InterruptState);
-
- if (DoDecrement) {
- DoDecrement = FALSE;
-
- //
- // Perform the delayed decrement just before issuing the first
- // VMGEXIT with AP_RESET_HOLD.
- //
- InterlockedDecrement ((UINT32 *) &CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
- }
-
- Status = VmgExit (Ghcb, SVM_EXIT_AP_RESET_HOLD, 0, 0);
- if ((Status == 0) && (Ghcb->SaveArea.SwExitInfo2 != 0)) {
- VmgDone (Ghcb, InterruptState);
- break;
- }
-
- VmgDone (Ghcb, InterruptState);
- }
-
- //
- // Awakened in a new phase? Use the new CpuMpData
- //
- if (CpuMpData->NewCpuMpData != NULL) {
- CpuMpData = CpuMpData->NewCpuMpData;
- }
-
- MpInitLibSevEsAPReset (Ghcb, CpuMpData);
+ SevEsPlaceApHlt (CpuMpData);
} else {
CpuSleep ();
}
@@ -1252,71 +1101,6 @@ FreeResetVector (
}
}

-/**
- Allocate the SEV-ES AP jump table buffer.
-
- @param[in, out] CpuMpData The pointer to CPU MP Data structure.
-**/
-VOID
-AllocateSevEsAPMemory (
- IN OUT CPU_MP_DATA *CpuMpData
- )
-{
- if (CpuMpData->SevEsAPBuffer == (UINTN) -1) {
- CpuMpData->SevEsAPBuffer =
- CpuMpData->SevEsIsEnabled ? GetSevEsAPMemory () : 0;
- }
-}
-
-/**
- Program the SEV-ES AP jump table buffer.
-
- @param[in] SipiVector The SIPI vector used for the AP Reset
-**/
-VOID
-SetSevEsJumpTable (
- IN UINTN SipiVector
- )
-{
- SEV_ES_AP_JMP_FAR *JmpFar;
- UINT32 Offset, InsnByte;
- UINT8 LoNib, HiNib;
-
- JmpFar = (SEV_ES_AP_JMP_FAR *) (UINTN) FixedPcdGet32 (PcdSevEsWorkAreaBase);
- ASSERT (JmpFar != NULL);
-
- //
- // Obtain the address of the Segment/Rip location in the workarea.
- // This will be set to a value derived from the SIPI vector and will
- // be the memory address used for the far jump below.
- //
- Offset = FixedPcdGet32 (PcdSevEsWorkAreaBase);
- Offset += sizeof (JmpFar->InsnBuffer);
- LoNib = (UINT8) Offset;
- HiNib = (UINT8) (Offset >> 8);
-
- //
- // Program the workarea (which is the initial AP boot address) with
- // far jump to the SIPI vector (where XX and YY represent the
- // address of where the SIPI vector is stored.
- //
- // JMP FAR [CS:XXYY] => 2E FF 2E YY XX
- //
- InsnByte = 0;
- JmpFar->InsnBuffer[InsnByte++] = 0x2E; // CS override prefix
- JmpFar->InsnBuffer[InsnByte++] = 0xFF; // JMP (FAR)
- JmpFar->InsnBuffer[InsnByte++] = 0x2E; // ModRM (JMP memory location)
- JmpFar->InsnBuffer[InsnByte++] = LoNib; // YY offset ...
- JmpFar->InsnBuffer[InsnByte++] = HiNib; // XX offset ...
-
- //
- // Program the Segment/Rip based on the SIPI vector (always at least
- // 16-byte aligned, so Rip is set to 0).
- //
- JmpFar->Rip = 0;
- JmpFar->Segment = (UINT16) (SipiVector >> 4);
-}
-
/**
This function will be called by BSP to wakeup AP.

diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
new file mode 100644
index 000000000000..0ccafe25eca4
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
@@ -0,0 +1,119 @@
+;------------------------------------------------------------------------------ ;
+; Copyright (c) 2021, AMD Inc. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+; AmdSev.nasm
+;
+; Abstract:
+;
+; This provides helper used by the MpFunc.nasm. If AMD SEV-ES is active
+; then helpers perform the additional setups (such as GHCB).
+;
+;-------------------------------------------------------------------------------
+
+%define SIZE_4KB 0x1000
+
+;
+; The function checks whether SEV-ES is enabled, if enabled
+; then setup the GHCB page.
+;
+SevEsSetupGhcb:
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
+ cmp byte [edi], 1 ; SevEsIsEnabled
+ jne SevEsSetupGhcbExit
+
+ ;
+ ; program GHCB
+ ; Each page after the GHCB is a per-CPU page, so the calculation programs
+ ; a GHCB to be every 8KB.
+ ;
+ mov eax, SIZE_4KB
+ shl eax, 1 ; EAX = SIZE_4K * 2
+ mov ecx, ebx
+ mul ecx ; EAX = SIZE_4K * 2 * CpuNumber
+ mov edi, esi
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (GhcbBase)
+ add rax, qword [edi]
+ mov rdx, rax
+ shr rdx, 32
+ mov rcx, 0xc0010130
+ wrmsr
+
+SevEsSetupGhcbExit:
+ OneTimeCallRet SevEsSetupGhcb
+
+;
+; The function checks whether SEV-ES is enabled, if enabled, use
+; the GHCB
+;
+SevEsGetApicId:
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
+ cmp byte [edi], 1 ; SevEsIsEnabled
+ jne SevEsGetApicIdExit
+
+ ;
+ ; Since we don't have a stack yet, we can't take a #VC
+ ; exception. Use the GHCB protocol to perform the CPUID
+ ; calls.
+ ;
+ mov rcx, 0xc0010130
+ rdmsr
+ shl rdx, 32
+ or rax, rdx
+ mov rdi, rax ; RDI now holds the original GHCB GPA
+
+ mov rdx, 0 ; CPUID function 0
+ mov rax, 0 ; RAX register requested
+ or rax, 4
+ wrmsr
+ rep vmmcall
+ rdmsr
+ cmp edx, 0bh
+ jb NoX2ApicSevEs ; CPUID level below CPUID_EXTENDED_TOPOLOGY
+
+ mov rdx, 0bh ; CPUID function 0x0b
+ mov rax, 040000000h ; RBX register requested
+ or rax, 4
+ wrmsr
+ rep vmmcall
+ rdmsr
+ test edx, 0ffffh
+ jz NoX2ApicSevEs ; CPUID.0BH:EBX[15:0] is zero
+
+ mov rdx, 0bh ; CPUID function 0x0b
+ mov rax, 0c0000000h ; RDX register requested
+ or rax, 4
+ wrmsr
+ rep vmmcall
+ rdmsr
+
+ ; Processor is x2APIC capable; 32-bit x2APIC ID is now in EDX
+ jmp RestoreGhcb
+
+NoX2ApicSevEs:
+ ; Processor is not x2APIC capable, so get 8-bit APIC ID
+ mov rdx, 1 ; CPUID function 1
+ mov rax, 040000000h ; RBX register requested
+ or rax, 4
+ wrmsr
+ rep vmmcall
+ rdmsr
+ shr edx, 24
+
+RestoreGhcb:
+ mov rbx, rdx ; Save x2APIC/APIC ID
+
+ mov rdx, rdi ; RDI holds the saved GHCB GPA
+ shr rdx, 32
+ mov eax, edi
+ wrmsr
+
+ mov rdx, rbx
+
+ ; x2APIC ID or APIC ID is in EDX
+ jmp GetProcessorNumber
+
+SevEsGetApicIdExit:
+ OneTimeCallRet SevEsGetApicId
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 50df802d1fca..f7f2937fafad 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -15,6 +15,15 @@
%include "MpEqu.inc"
extern ASM_PFX(InitializeFloatingPointUnits)

+%macro OneTimeCall 1
+ jmp %1
+%1 %+ OneTimerCallReturn:
+%endmacro
+
+%macro OneTimeCallRet 1
+ jmp %1 %+ OneTimerCallReturn
+%endmacro
+
DEFAULT REL

SECTION .text
@@ -144,6 +153,12 @@ SkipEnable5LevelPaging:
jmp far [edi]

BITS 64
+
+;
+; Required for the AMD SEV helper functions
+;
+%include "AmdSev.nasm"
+
LongModeStart:
mov esi, ebx
lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
@@ -175,94 +190,17 @@ LongModeStart:
add rax, qword [edi]
mov rsp, rax

- lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
- cmp byte [edi], 1 ; SevEsIsEnabled
- jne CProcedureInvoke
-
;
- ; program GHCB
- ; Each page after the GHCB is a per-CPU page, so the calculation programs
- ; a GHCB to be every 8KB.
+ ; Setup the GHCB when AMD SEV-ES active.
;
- mov eax, SIZE_4KB
- shl eax, 1 ; EAX = SIZE_4K * 2
- mov ecx, ebx
- mul ecx ; EAX = SIZE_4K * 2 * CpuNumber
- mov edi, esi
- add edi, MP_CPU_EXCHANGE_INFO_FIELD (GhcbBase)
- add rax, qword [edi]
- mov rdx, rax
- shr rdx, 32
- mov rcx, 0xc0010130
- wrmsr
+ OneTimeCall SevEsSetupGhcb
jmp CProcedureInvoke

GetApicId:
- lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
- cmp byte [edi], 1 ; SevEsIsEnabled
- jne DoCpuid
-
;
- ; Since we don't have a stack yet, we can't take a #VC
- ; exception. Use the GHCB protocol to perform the CPUID
- ; calls.
+ ; Use the GHCB protocol to get the ApicId when SEV-ES is active.
;
- mov rcx, 0xc0010130
- rdmsr
- shl rdx, 32
- or rax, rdx
- mov rdi, rax ; RDI now holds the original GHCB GPA
-
- mov rdx, 0 ; CPUID function 0
- mov rax, 0 ; RAX register requested
- or rax, 4
- wrmsr
- rep vmmcall
- rdmsr
- cmp edx, 0bh
- jb NoX2ApicSevEs ; CPUID level below CPUID_EXTENDED_TOPOLOGY
-
- mov rdx, 0bh ; CPUID function 0x0b
- mov rax, 040000000h ; RBX register requested
- or rax, 4
- wrmsr
- rep vmmcall
- rdmsr
- test edx, 0ffffh
- jz NoX2ApicSevEs ; CPUID.0BH:EBX[15:0] is zero
-
- mov rdx, 0bh ; CPUID function 0x0b
- mov rax, 0c0000000h ; RDX register requested
- or rax, 4
- wrmsr
- rep vmmcall
- rdmsr
-
- ; Processor is x2APIC capable; 32-bit x2APIC ID is now in EDX
- jmp RestoreGhcb
-
-NoX2ApicSevEs:
- ; Processor is not x2APIC capable, so get 8-bit APIC ID
- mov rdx, 1 ; CPUID function 1
- mov rax, 040000000h ; RBX register requested
- or rax, 4
- wrmsr
- rep vmmcall
- rdmsr
- shr edx, 24
-
-RestoreGhcb:
- mov rbx, rdx ; Save x2APIC/APIC ID
-
- mov rdx, rdi ; RDI holds the saved GHCB GPA
- shr rdx, 32
- mov eax, edi
- wrmsr
-
- mov rdx, rbx
-
- ; x2APIC ID or APIC ID is in EDX
- jmp GetProcessorNumber
+ OneTimeCall SevEsGetApicId

DoCpuid:
mov eax, 0


Re: [PATCH v11 20/32] UefiCpuPkg: Define ConfidentialComputingGuestAttr

Brijesh Singh
 

Hi Ray and Rahul,

Any comment on this patch ? If you are okay with it then can I get Ack
or R-b ?

-Brijesh

On 10/22/21 11:13 PM, Brijesh Singh wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

While initializing APs, the MpInitLib may need to know whether the
guest is running with active AMD SEV or Intel TDX memory encryption.

Add a new ConfidentialComputingGuestAttr PCD that can be used to query
the memory encryption attribute.

Cc: Michael Roth <michael.roth@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Eric Dong <eric.dong@...>
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Suggested-by: Jiewen Yao <jiewen.yao@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/MdePkg.dec | 4 +++
.../Include/ConfidentialComputingGuestAttr.h | 25 +++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 MdePkg/Include/ConfidentialComputingGuestAttr.h

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 8b18415b107a..cd903c35d2ff 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2396,5 +2396,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
# @Prompt FSB Clock.
gEfiMdePkgTokenSpaceGuid.PcdFSBClock|200000000|UINT32|0x0000000c

+ ## This dynamic PCD indicates the memory encryption attribute of the guest.
+ # @Prompt Memory encryption attribute
+ gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0|UINT64|0x0000002e
+
[UserExtensions.TianoCore."ExtraFiles"]
MdePkgExtra.uni
diff --git a/MdePkg/Include/ConfidentialComputingGuestAttr.h b/MdePkg/Include/ConfidentialComputingGuestAttr.h
new file mode 100644
index 000000000000..495b0df0ac33
--- /dev/null
+++ b/MdePkg/Include/ConfidentialComputingGuestAttr.h
@@ -0,0 +1,25 @@
+/** @file
+Definitions for Confidential Computing Attribute
+
+Copyright (c) 2021 AMD Inc. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef CONFIDENTIAL_COMPUTING_GUEST_ATTR_H_
+#define CONFIDENTIAL_COMPUTING_GUEST_ATTR_H_
+
+typedef enum {
+ /* The guest is running with memory encryption disabled. */
+ CCAttrNotEncrypted = 0,
+
+ /* The guest is running with AMD SEV memory encryption enabled. */
+ CCAttrAmdSev = 0x100,
+ CCAttrAmdSevEs = 0x101,
+ CCAttrAmdSevSnp = 0x102,
+
+ /* The guest is running with Intel TDX memory encryption enabled. */
+ CCAttrIntelTdx = 0x200,
+} CONFIDENTIAL_COMPUTING_GUEST_ATTR;
+
+#endif


Re: [PATCH v11 23/32] UefiCpuPkg: add PcdGhcbHypervisorFeatures

Brijesh Singh
 

Hi Ray and Rahul,

Any comment on this patch ? If you are okay with the approach then can I
get Ack or R-b ?

-Brijesh

On 10/22/21 11:13 PM, Brijesh Singh via groups.io wrote:
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@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Eric Dong <eric.dong@...>
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
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


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

Brijesh Singh
 

Hi Ray and Rahul,

Any comment on this patch ? If you are okay with the approach then can I
get Ack or R-b ?

-Brijesh

On 10/22/21 11:13 PM, Brijesh Singh via groups.io wrote:
From: Michael Roth <michael.roth@...>

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

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

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

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

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

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

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

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


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

Brijesh Singh
 

Hi Ray and Rahul,

Any comment on this patch ? If you are okay with the approach then can I
get Ack or R-b ?

-Brijesh

On 10/22/21 11:13 PM, Brijesh Singh via groups.io wrote:
From: Tom Lendacky <thomas.lendacky@...>

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@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Acked-by: Gerd Hoffmann <kraxel@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
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 d8372ffa9d5a..d134dc1326e8 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);
+}


Re: [PATCH] UefiCpuPkg: Fix CPU stack guard support by aligning GDT buffer

Vitaly Cheptsov
 

Hello,

It has been over a month since the patch was sent. What is the state of the issue?

Best regards,
Vitaly

On 20 Sep 2021, at 19:15, vit9696 via [] <vit9696=protonmail.com@[]> wrote:

Just to make it clear, this is an immediate solution that is good enough to fix the bug. However, a more proper solution would be to introduce the _Alignas concept to EDK II. I would suggest the following macro in Base.h:

/**
Enforce custom alignment for a variable definition.
Similar to C11 alignas macro from stdalign.h, except it must be functional to support MSVC.

@param Alignment Numeric alignment to require.
**/
#ifdef _MSC_EXTENSIONS
#define ALIGNAS(Alignment) __declspec(align(Alignment))
#else
#define ALIGNAS(Alignment) _Alignas(Alignment)
#endif

If there is no disagreement on this, I can imagine submitting an update after this patch is merged.


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

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Eric Dong <eric.dong@...>
Cc: Michael Kinney <michael.d.kinney@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Jeff Fan <vanjeff_919@...>
Cc: Mikhail Krichanov <krichanov@...>
Cc: Marvin Häuser <mhaeuser@...>
Signed-off-by: Vitaly Cheptsov <cheptsov@...>
---
.../Library/CpuExceptionHandlerLib/DxeException.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index fd59f09ecd..12874811e1 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -22,7 +22,7 @@ EXCEPTION_HANDLER_DATA mExceptionHandlerData;

UINT8 mNewStack[CPU_STACK_SWITCH_EXCEPTION_NUMBER *
CPU_KNOWN_GOOD_STACK_SIZE];
-UINT8 mNewGdt[CPU_TSS_GDT_SIZE];
+UINT8 mNewGdt[CPU_TSS_GDT_SIZE + IA32_GDT_ALIGNMENT];

/**
Common exception handler.
@@ -238,6 +238,7 @@ InitializeCpuExceptionHandlersEx (
CPU_EXCEPTION_INIT_DATA EssData;
IA32_DESCRIPTOR Idtr;
IA32_DESCRIPTOR Gdtr;
+ UINT8 *Gdt;

//
// To avoid repeat initialization of default handlers, the caller should pass
@@ -259,6 +260,7 @@ InitializeCpuExceptionHandlersEx (
if (PcdGetBool (PcdCpuStackGuard)) {
if (InitData == NULL) {
SetMem (mNewGdt, sizeof (mNewGdt), 0);
+ Gdt = ALIGN_POINTER (mNewGdt, IA32_GDT_ALIGNMENT);

AsmReadIdtr (&Idtr);
AsmReadGdtr (&Gdtr);
@@ -270,11 +272,11 @@ InitializeCpuExceptionHandlersEx (
EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
EssData.X64.IdtTable = (VOID *)Idtr.Base;
EssData.X64.IdtTableSize = Idtr.Limit + 1;
- EssData.X64.GdtTable = mNewGdt;
- EssData.X64.GdtTableSize = sizeof (mNewGdt);
- EssData.X64.ExceptionTssDesc = mNewGdt + Gdtr.Limit + 1;
+ EssData.X64.GdtTable = Gdt;
+ EssData.X64.GdtTableSize = CPU_TSS_GDT_SIZE;
+ EssData.X64.ExceptionTssDesc = Gdt + Gdtr.Limit + 1;
EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
- EssData.X64.ExceptionTss = mNewGdt + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
+ EssData.X64.ExceptionTss = Gdt + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;

InitData = &EssData;


Re: [`edk2-devel][PATCH V3 0/8] Add SMM variable support for UEFI payload

Benjamin You
 

Reviewed-by: Benjamin You <benjamin.you@...>

-----Original Message-----
From: Dong, Guo <guo.dong@...>
Sent: Friday, October 22, 2021 11:46 PM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@...>; Ni, Ray <ray.ni@...>; Ma,
Maurice <maurice.ma@...>; You, Benjamin <benjamin.you@...>
Subject: [`edk2-devel][PATCH V3 0/8] Add SMM variable support for UEFI
payload

From: Guo Dong <guo.dong@...>

V3: Add SMM communication region EFI_ALLOCATED check
in UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
V2: Added SMM communication region size check
Fixed ECC reported issues and other minor update.

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

Currently UEFI payload uses emulated variable driver. So it could
not support secureboot and measured boot since both need NV variable
support.

EDKII already has SMM modules and variable modules. And modern Intel
platform supports SPI flash hardware sequence to operate flash. So it
is possible to have a common SPI module for Intel platforms.

This patch enhances UEFI payload to support SMM variable with a
common SPI library for Intel platforms. To avoid impact existing
usage, all the new modules are included under SMM_ENABLE and
VARIABLE_SUPPORT and by default SMM variable is not be enabled.

SMM variable could be enabled only when UNIVERSAL_PAYLOAD is set
since non-universal payload need update ParseLib to provide SMM
variable related infromation which is not in the plan.

Signed-off-by: Guo Dong <guo.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Benjamin You <benjamin.you@...>
Reviewed-by: Ray Ni <ray.ni@...>
Reviewed-by: Benjamin You <benjamin.you@...

Guo Dong (8):
UefiPayloadPkg: Add a common SmmAccessDxe module
UefiPayloadPkg: Add a common SMM control Runtime DXE module
UefiPayloadPkg: Add bootloader SMM support module
UefiPayloadPkg: Add SpiFlashLib
UefiPayloadPkg: Add FlashDeviceLib
UefiPayloadPkg: Add a common FVB SMM module
UefiPayloadPkg: Add a SMM dispatch module
UefiPayloadPkg: Add SMM support and SMM variable support

UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c | 431 +++++++
UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h | 41 +
UefiPayloadPkg/BlSupportSmm/BlSupportSmm.inf | 49 +
UefiPayloadPkg/FvbRuntimeDxe/FvbInfo.c | 151 +++
UefiPayloadPkg/FvbRuntimeDxe/FvbService.c | 1088 +++++++++++++++++
UefiPayloadPkg/FvbRuntimeDxe/FvbService.h | 187 +++
UefiPayloadPkg/FvbRuntimeDxe/FvbServiceSmm.c | 139 +++
UefiPayloadPkg/FvbRuntimeDxe/FvbSmm.inf | 71 ++
UefiPayloadPkg/FvbRuntimeDxe/FvbSmmCommon.h | 69 ++
.../Include/Guid/NvVariableInfoGuid.h | 24 +
.../Include/Guid/SmmRegisterInfoGuid.h | 48 +
.../Include/Guid/SmmS3CommunicationInfoGuid.h | 54 +
.../Include/Guid/SpiFlashInfoGuid.h | 38 +
.../Include/Library/FlashDeviceLib.h | 108 ++
UefiPayloadPkg/Include/Library/SpiFlashLib.h | 215 ++++
.../Library/FlashDeviceLib/FlashDeviceLib.c | 165 +++
.../Library/FlashDeviceLib/FlashDeviceLib.inf | 38 +
UefiPayloadPkg/Library/SpiFlashLib/PchSpi.c | 173 +++
UefiPayloadPkg/Library/SpiFlashLib/RegsSpi.h | 129 ++
.../Library/SpiFlashLib/SpiCommon.h | 208 ++++
.../Library/SpiFlashLib/SpiFlashLib.c | 857 +++++++++++++
.../Library/SpiFlashLib/SpiFlashLib.inf | 48 +
.../PchSmiDispatchSmm/PchSmiDispatchSmm.c | 455 +++++++
.../PchSmiDispatchSmm/PchSmiDispatchSmm.h | 37 +
.../PchSmiDispatchSmm/PchSmiDispatchSmm.inf | 51 +
UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.c | 254 ++++
UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.h | 37 +
UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.inf | 51 +
.../SmmControlRuntimeDxe.c | 256 ++++
.../SmmControlRuntimeDxe.inf | 50 +
UefiPayloadPkg/UefiPayloadPkg.dec | 10 +
UefiPayloadPkg/UefiPayloadPkg.dsc | 101 +-
UefiPayloadPkg/UefiPayloadPkg.fdf | 38 +-
33 files changed, 5660 insertions(+), 11 deletions(-)
create mode 100644 UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
create mode 100644 UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h
create mode 100644 UefiPayloadPkg/BlSupportSmm/BlSupportSmm.inf
create mode 100644 UefiPayloadPkg/FvbRuntimeDxe/FvbInfo.c
create mode 100644 UefiPayloadPkg/FvbRuntimeDxe/FvbService.c
create mode 100644 UefiPayloadPkg/FvbRuntimeDxe/FvbService.h
create mode 100644 UefiPayloadPkg/FvbRuntimeDxe/FvbServiceSmm.c
create mode 100644 UefiPayloadPkg/FvbRuntimeDxe/FvbSmm.inf
create mode 100644 UefiPayloadPkg/FvbRuntimeDxe/FvbSmmCommon.h
create mode 100644 UefiPayloadPkg/Include/Guid/NvVariableInfoGuid.h
create mode 100644 UefiPayloadPkg/Include/Guid/SmmRegisterInfoGuid.h
create mode 100644
UefiPayloadPkg/Include/Guid/SmmS3CommunicationInfoGuid.h
create mode 100644 UefiPayloadPkg/Include/Guid/SpiFlashInfoGuid.h
create mode 100644 UefiPayloadPkg/Include/Library/FlashDeviceLib.h
create mode 100644 UefiPayloadPkg/Include/Library/SpiFlashLib.h
create mode 100644 UefiPayloadPkg/Library/FlashDeviceLib/FlashDeviceLib.c
create mode 100644 UefiPayloadPkg/Library/FlashDeviceLib/FlashDeviceLib.inf
create mode 100644 UefiPayloadPkg/Library/SpiFlashLib/PchSpi.c
create mode 100644 UefiPayloadPkg/Library/SpiFlashLib/RegsSpi.h
create mode 100644 UefiPayloadPkg/Library/SpiFlashLib/SpiCommon.h
create mode 100644 UefiPayloadPkg/Library/SpiFlashLib/SpiFlashLib.c
create mode 100644 UefiPayloadPkg/Library/SpiFlashLib/SpiFlashLib.inf
create mode 100644
UefiPayloadPkg/PchSmiDispatchSmm/PchSmiDispatchSmm.c
create mode 100644
UefiPayloadPkg/PchSmiDispatchSmm/PchSmiDispatchSmm.h
create mode 100644
UefiPayloadPkg/PchSmiDispatchSmm/PchSmiDispatchSmm.inf
create mode 100644 UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.c
create mode 100644 UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.h
create mode 100644 UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.inf
create mode 100644
UefiPayloadPkg/SmmControlRuntimeDxe/SmmControlRuntimeDxe.c
create mode 100644
UefiPayloadPkg/SmmControlRuntimeDxe/SmmControlRuntimeDxe.inf

--
2.32.0.windows.2


Re: [PATCH v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

Brijesh Singh
 

I have not heard from UefiCpuPkg maintainers yet. Hopefully we get the
Ack or R-b. Ray asked me to move the AmdSev in the common file which I did.

thanks

On 10/23/21 8:46 PM, Yao, Jiewen via groups.io wrote:
Yes. I will try my best to merge.

I checked the patch set but I did not find the "R-B" from UefiCpuPkg maintainer. Neither from email nor from you v11.

Did I miss something?

Thank you
Yao Jiewen


-----Original Message-----
From: Brijesh Singh <brijesh.singh@...>
Sent: Saturday, October 23, 2021 12:13 PM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@...>; Xu, Min M <min.m.xu@...>;
Yao, Jiewen <jiewen.yao@...>; Tom Lendacky
<thomas.lendacky@...>; Justen, Jordan L <jordan.l.justen@...>;
Ard Biesheuvel <ardb+tianocore@...>; Erdem Aktas
<erdemaktas@...>; Michael Roth <Michael.Roth@...>; Gerd
Hoffmann <kraxel@...>; Brijesh Singh <brijesh.singh@...>
Subject: [PATCH v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

Hi Gerd and Jiewen,

CI was a bit unstable during my v10 submission, so, I was not able to
run it to the completion. Finally, I managed to get the CI going,
and it reported few Windows 32-bit build errors. The v11 fixes those build
errors. Please consider this for the merge.

Thank you so much for all your support in reviewing the series.

-----------------------------------------------------------------------------
BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=L41krO6G221HaIsG92FloIzgCDqMLAAsU26jaEMF7yw%3D&amp;reserved=0

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

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

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

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

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

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

Now that series contains all the basic support required to launch SEV-SNP
guest. We are still missing the Interrupt security feature provided by the
SNP. The feature will be added after the base support is accepted.

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=nVMSG%2FvSS2Wa21lu1lGrHr9OYX8hL7FoAcQXBBiCztc%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=G8Xg2glOGY2EjHpeQ3WM4gZChuI0k8QcLDTbpJiTplg%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fovmf%2Ftree%2Fsnp-v11&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=HMHFq8G%2FPqdhzNW3Ashmc4%2Bmv1RcDULD4vniofhiS54%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fwp-content%2Fresources%2F56421.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=YiPgZU87fdnl5rJpD0E2ue9aTKbqUwizuBrKxom0FiU%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=bfQsY4%2BRnlFGuD3Bg%2BFPb3lRgSGgpomNocXswHqkm%2F4%3D&amp;reserved=0

Change since v10:
* fix 'unresolved external symbol __allshl' link error when building I32 for
VS2017.

Changes since v9:
* Move CCAttrs Pcd define in MdePkg
* Add comment to indicate that allocating the identity map PT is temporary until
we get lazy validation

Changes since v8:
* drop the generic metadata and make it specific to SEV.

Changes since v7:
* Move SEV specific changes in MpLib in AmdSev file
* Update the GHCB register function to not restore the GHCB MSR because
we were already in the MSR protocol mode.
* Drop the SNP name from PcdSnpSecPreValidate.
* Add new section for GHCB memory in the OVMF metadata.

Change since v6:
* Drop the SNP boot block GUID and switch to using the Metadata guided
structure
proposed by Min in TDX series.
* Exclude the GHCB page from the pre-validated region. It simplifies the reset
vector code where we do not need to unvalidate the GHCB page.
* Now that GHCB page is not validated so move the VMPL check from reset
vector
code to the MemEncryptSevLib on the first page validation.
* Introduce the ConfidentialComputingGuestAttr PCD to communicate which
memory encryption is active so that MpInitLib can make use of it.
* Drop the SEVES specific PCD as the information can be communicated via
the ConfidentialComputingGuestAttr.
* Move the SNP specific AP creation function in AmdSev.c.
* Define the SNP Blob GUID in a new file.

Change since v5:
* When possible use the CPUID value from CPUID page
* Move the SEV specific functions from SecMain.c in AmdSev.c
* Rebase to the latest code
* Add the review feedback from Yao.

Change since v4:
* Use the correct MSR for the SEV_STATUS
* Add VMPL-0 check

Change since v3:
* ResetVector: move all SEV specific code in AmdSev.asm and add macros to
keep
the code readable.
* Drop extending the EsWorkArea to contain SNP specific state.
* Drop the GhcbGpa library and call the VmgExit directly to register GHCB GPA.
* Install the CC blob config table from AmdSevDxe instead of extending the
AmdSev/SecretsDxe for it.
* Add the separate PCDs for the SNP Secrets.

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

Brijesh Singh (28):
OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
UefiCpuPkg/MpInitLib: move SEV specific routines in AmdSev.c
OvmfPkg/ResetVector: move clearing GHCB in SecMain
OvmfPkg/ResetVector: introduce SEV metadata descriptor for VMM use
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/MemEncryptSevLib: add function to check the VMPL0
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
UefiCpuPkg: Define ConfidentialComputingGuestAttr
OvmfPkg/PlatformPei: set PcdConfidentialComputingAttr when SEV is
active
UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV
status
UefiCpuPkg: add PcdGhcbHypervisorFeatures
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Michael Roth (3):
OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values
OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values
UefiCpuPkg/MpInitLib: use BSP to do extended topology check

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

MdePkg/MdePkg.dec | 4 +
OvmfPkg/OvmfPkg.dec | 18 +
UefiCpuPkg/UefiCpuPkg.dec | 5 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 8 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 4 +
OvmfPkg/OvmfPkgIa32X64.dsc | 9 +-
OvmfPkg/OvmfPkgX64.dsc | 8 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 6 +
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 7 +
OvmfPkg/ResetVector/ResetVector.inf | 5 +
OvmfPkg/Sec/SecMain.inf | 4 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 6 +-
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 6 +-
.../Include/ConfidentialComputingGuestAttr.h | 25 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSevSnpBlob.h | 33 ++
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 +
.../X64/SnpPageStateChange.h | 36 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 24 +
OvmfPkg/PlatformPei/Platform.h | 5 +
OvmfPkg/Sec/AmdSev.h | 95 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 93 ++++
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 127 +++++
.../X64/SecSnpSystemRamValidate.c | 82 ++++
.../X64/SnpPageStateChangeInternal.c | 294 ++++++++++++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
OvmfPkg/PlatformPei/AmdSev.c | 231 +++++++++
OvmfPkg/PlatformPei/MemDetect.c | 2 +
OvmfPkg/Sec/AmdSev.c | 298 ++++++++++++
OvmfPkg/Sec/SecMain.c | 158 +------
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 239 ++++++++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 16 +-
UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c | 70 +++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 345 +++++---------
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 4 +-
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c | 261 ++++++++++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 17 +
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 86 +++-
OvmfPkg/ResetVector/ResetVector.nasmb | 18 +
OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm | 74 +++
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 200 ++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 100 +---
59 files changed, 3329 insertions(+), 528 deletions(-)
create mode 100644 MdePkg/Include/ConfidentialComputingGuestAttr.h
create mode 100644
OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Sec/AmdSev.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Sec/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
create mode 100644 OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm

--
2.25.1




Re: [PATCH V10 0/4] Add Intel TDX support in OvmfPkg/ResetVector

Yao, Jiewen
 

Merged https://github.com/tianocore/edk2/pull/2142

f079e9b450b3896bb00eb7a9fed3a6ec7ed3cd04.. 8b76f235340922a6d293bff05978ba57d3b498e1

-----Original Message-----
From: Xu, Min M <min.m.xu@...>
Sent: Thursday, October 21, 2021 8:18 AM
To: devel@edk2.groups.io
Cc: Xu, Min M <min.m.xu@...>; Ard Biesheuvel
<ardb+tianocore@...>; Gerd Hoffmann <kraxel@...>; Justen,
Jordan L <jordan.l.justen@...>; Brijesh Singh <brijesh.singh@...>;
Erdem Aktas <erdemaktas@...>; James Bottomley
<jejb@...>; Yao, Jiewen <jiewen.yao@...>; Tom Lendacky
<thomas.lendacky@...>
Subject: [PATCH V10 0/4] Add Intel TDX support in OvmfPkg/ResetVector

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

Intel's Trust Domain Extensions (Intel TDX) refers to an Intel technology
that extends Virtual Machines Extensions (VMX) and Multi-Key Total Memory
Encryption (MKTME) with a new kind of virutal machines guest called a
Trust Domain (TD). A TD is desinged to run in a CPU mode that protects the
confidentiality of TD memory contents and the TD's CPU state from other
software, including the hosting Virtual-Machine Monitor (VMM), unless
explicitly shared by the TD itself.

The patch-sets to support Intel TDX in OvmfPkg is split into several
waves. This is wave-1 which adds Intel TDX support in OvmfPkg/ResetVector.
Note: TDX only works in X64.

Patch #1: Ovmf uses its own Main.asm to reduce the complexity of Main.asm
in UefiCpuPkg. This Main.asm is an unmodified copy from
UefiCpuPkg/ReseteVector/Vtf0 (so no functional change) and the actual
changes for tdx come as incremental patches.

Patch #2: WORK_AREA_GUEST_TYPE is cleared in Main.asm instead of in
WORK_AREA_GUEST_TYPE.

Patch #3: Introduce IntelTdxMetadata.asm which describes the information
about the image for VMM use.

Patch #4: Enable TDX in OvmfPkg/ResetVector for ARCH_X64.

[TDX]: https://software.intel.com/content/dam/develop/external/us/en/
documents/tdx-whitepaper-final9-17.pdf

[TDVF]: https://software.intel.com/content/dam/develop/external/us/en/
documents/tdx-virtual-firmware-design-guide-rev-1.pdf

Code is at https://github.com/mxu9/edk2/tree/tdvf_wave1.v10

v10 changes:
- Clear the OVMF_WORK_AREA in both ARCH_IA32 and ARCH_X64.
- Update the ReloadFlat32 based on the review comments.
- Other minor changes and update some comments.

v9 changes:
- Introduce IntelTdxMetadata.asm in a separate commit.
- Use absolute offset for the start of TdxMetadata so that VMM can
easily reach to the start of the metadata.

v8 changes:
- Create a separate commit for Main.asm.
- Create a separate commit for the clearance of WORK_AREA_GUEST_TYPE.
- Fix some inaccurate comments.

v7 changes:
- Refine the offset of TdxMetadata and remove the definition of
PcdOvmfImageSizeInKB
- Use MOV CR* instead of smsw in ResetVector
- Remove the new field (SubType) in
CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER.

v6 changes:
- Remove the 5-level paging support. 5-level paging enabling is *NOT*
super critical for TDX enabling at this moment. It will be enabled
later in a separate patch.
- Add a new field (SubType) in
CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER
to record the VM Guest SubType.
- In Main16 entry point, after TransitionFromReal16To32BitFlat,
WORK_AREA_GUEST_TYPE is cleared to 0. WORK_AREA_GUEST_TYPE was
previously cleared in SetCr3ForPageTables64 (see commit ab77b60).
This doesn't work after TDX is introduced in Ovmf. It is because all
TDX CPUs (BSP and APs) start to run from 0xfffffff0. In previous code
WORK_AREA_GUEST_TYPE will be cleared multi-times in TDX guest. So for
SEV and Legacy guest it is moved to Main16 entry point (after
TransitionFromReal16To32BitFlat). For TDX guest WORK_AREA_GUEST_TYPE
is cleared and set in InitTdxWorkarea.
- Make the return result of IsTdx be consistent with IsTdxEnabled.
- Fix some typo in the code comments.

v5 changes:
- Remove the changes of OVMF_WORK_AREA because Commit ab77b60 covers
those changes.
- Refine the TDX related changes in PageTables64.asm and
Flat32ToFlat64.asm.
- Add CheckTdxFeaturesBeforeBuildPagetables to check Non-Tdx, Tdx-BSP or
Tdx-APs. This routine is called before building page tables.

v4 changes:
- Refine the PageTables64.asm and Flat32ToFlat64.asm to enable TDX.
- Refine SEV_ES_WORK_AREA so that SEV/TDX/Legach guest all can use this
memory region. https://edk2.groups.io/g/devel/message/78345 is the
discussion.
- AmdSev.asm is removed because Brijesh Singh has done it in
https://edk2.groups.io/g/devel/message/78241.

v3 changes:
- Refine PageTables64.asm and Flat32ToFlat64.asm based on the review
comments in [ReviewComment-1] and [ReviewComment-2].
- SEV codes are in AmdSev.asm
- TDX codes are in IntelTdx.asm
- Main.asm is created in OvmfPkg/ResetVector. The one in
UefiCpuPkg/ResetVector/Vtf0 is not used.
- Init32.asm/ReloadFlat32.asm in UefiCpuPkg/ResetVector/Vtf0/Ia32 are
deleted. They're moved to OvmfPkg/ResetVector/Ia32.
- InitTdx.asm is renamed to InteTdx.asm

v2 changes:
- Move InitTdx.asm and ReloadFlat32.asm from UefiCpuPkg/ResetVector/Vtf0
to OvmfPkg/ResetVector. Init32.asm is created which is a null stub of
32-bit initialization. In Main32 just simply call Init32. It makes
the Main.asm in UefiCpuPkg/ResetVector clean and clear.
- Init32.asm/InitTdx.asm/ReloadFlat32.asm are created under
OvmfPkg/ResetVector/Ia32.
- Update some descriptions of the patch-sets.
- Update the REF link in cover letter.
- Add Ard Biesheuvel in Cc list.

v1: https://edk2.groups.io/g/devel/message/77675

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Brijesh Singh <brijesh.singh@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Min Xu <min.m.xu@...>

Min Xu (4):
OvmfPkg: Copy Main.asm from UefiCpuPkg to OvmfPkg's ResetVector
OvmfPkg: Clear WORK_AREA_GUEST_TYPE in Main.asm
OvmfPkg: Add IntelTdxMetadata.asm
OvmfPkg: Enable TDX in ResetVector

OvmfPkg/OvmfPkg.dec | 9 +
OvmfPkg/OvmfPkgDefines.fdf.inc | 9 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 39 ++++
OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm | 11 +
OvmfPkg/ResetVector/Ia32/IntelTdx.asm | 222 +++++++++++++++++++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 22 +-
OvmfPkg/ResetVector/Main.asm | 121 ++++++++++
OvmfPkg/ResetVector/ResetVector.inf | 9 +
OvmfPkg/ResetVector/ResetVector.nasmb | 28 +++
OvmfPkg/ResetVector/X64/IntelTdxMetadata.asm | 115 ++++++++++
10 files changed, 581 insertions(+), 4 deletions(-)
create mode 100644 OvmfPkg/ResetVector/Ia32/IntelTdx.asm
create mode 100644 OvmfPkg/ResetVector/Main.asm
create mode 100644 OvmfPkg/ResetVector/X64/IntelTdxMetadata.asm

--
2.29.2.windows.2


Re: [PATCH v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

Yao, Jiewen
 

Yes. I will try my best to merge.

I checked the patch set but I did not find the "R-B" from UefiCpuPkg maintainer. Neither from email nor from you v11.

Did I miss something?

Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@...>
Sent: Saturday, October 23, 2021 12:13 PM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@...>; Xu, Min M <min.m.xu@...>;
Yao, Jiewen <jiewen.yao@...>; Tom Lendacky
<thomas.lendacky@...>; Justen, Jordan L <jordan.l.justen@...>;
Ard Biesheuvel <ardb+tianocore@...>; Erdem Aktas
<erdemaktas@...>; Michael Roth <Michael.Roth@...>; Gerd
Hoffmann <kraxel@...>; Brijesh Singh <brijesh.singh@...>
Subject: [PATCH v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

Hi Gerd and Jiewen,

CI was a bit unstable during my v10 submission, so, I was not able to
run it to the completion. Finally, I managed to get the CI going,
and it reported few Windows 32-bit build errors. The v11 fixes those build
errors. Please consider this for the merge.

Thank you so much for all your support in reviewing the series.

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

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

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

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

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

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

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

Now that series contains all the basic support required to launch SEV-SNP
guest. We are still missing the Interrupt security feature provided by the
SNP. The feature will be added after the base support is accepted.

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

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

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

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

SEV-SNP firmware specification:
https://www.amd.com/system/files/TechDocs/56860.pdf

Change since v10:
* fix 'unresolved external symbol __allshl' link error when building I32 for
VS2017.

Changes since v9:
* Move CCAttrs Pcd define in MdePkg
* Add comment to indicate that allocating the identity map PT is temporary until
we get lazy validation

Changes since v8:
* drop the generic metadata and make it specific to SEV.

Changes since v7:
* Move SEV specific changes in MpLib in AmdSev file
* Update the GHCB register function to not restore the GHCB MSR because
we were already in the MSR protocol mode.
* Drop the SNP name from PcdSnpSecPreValidate.
* Add new section for GHCB memory in the OVMF metadata.

Change since v6:
* Drop the SNP boot block GUID and switch to using the Metadata guided
structure
proposed by Min in TDX series.
* Exclude the GHCB page from the pre-validated region. It simplifies the reset
vector code where we do not need to unvalidate the GHCB page.
* Now that GHCB page is not validated so move the VMPL check from reset
vector
code to the MemEncryptSevLib on the first page validation.
* Introduce the ConfidentialComputingGuestAttr PCD to communicate which
memory encryption is active so that MpInitLib can make use of it.
* Drop the SEVES specific PCD as the information can be communicated via
the ConfidentialComputingGuestAttr.
* Move the SNP specific AP creation function in AmdSev.c.
* Define the SNP Blob GUID in a new file.

Change since v5:
* When possible use the CPUID value from CPUID page
* Move the SEV specific functions from SecMain.c in AmdSev.c
* Rebase to the latest code
* Add the review feedback from Yao.

Change since v4:
* Use the correct MSR for the SEV_STATUS
* Add VMPL-0 check

Change since v3:
* ResetVector: move all SEV specific code in AmdSev.asm and add macros to
keep
the code readable.
* Drop extending the EsWorkArea to contain SNP specific state.
* Drop the GhcbGpa library and call the VmgExit directly to register GHCB GPA.
* Install the CC blob config table from AmdSevDxe instead of extending the
AmdSev/SecretsDxe for it.
* Add the separate PCDs for the SNP Secrets.

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

Brijesh Singh (28):
OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
UefiCpuPkg/MpInitLib: move SEV specific routines in AmdSev.c
OvmfPkg/ResetVector: move clearing GHCB in SecMain
OvmfPkg/ResetVector: introduce SEV metadata descriptor for VMM use
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/MemEncryptSevLib: add function to check the VMPL0
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
UefiCpuPkg: Define ConfidentialComputingGuestAttr
OvmfPkg/PlatformPei: set PcdConfidentialComputingAttr when SEV is
active
UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV
status
UefiCpuPkg: add PcdGhcbHypervisorFeatures
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Michael Roth (3):
OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values
OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values
UefiCpuPkg/MpInitLib: use BSP to do extended topology check

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

MdePkg/MdePkg.dec | 4 +
OvmfPkg/OvmfPkg.dec | 18 +
UefiCpuPkg/UefiCpuPkg.dec | 5 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 8 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 4 +
OvmfPkg/OvmfPkgIa32X64.dsc | 9 +-
OvmfPkg/OvmfPkgX64.dsc | 8 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 6 +
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 7 +
OvmfPkg/ResetVector/ResetVector.inf | 5 +
OvmfPkg/Sec/SecMain.inf | 4 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 6 +-
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 6 +-
.../Include/ConfidentialComputingGuestAttr.h | 25 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSevSnpBlob.h | 33 ++
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 +
.../X64/SnpPageStateChange.h | 36 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 24 +
OvmfPkg/PlatformPei/Platform.h | 5 +
OvmfPkg/Sec/AmdSev.h | 95 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 93 ++++
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 127 +++++
.../X64/SecSnpSystemRamValidate.c | 82 ++++
.../X64/SnpPageStateChangeInternal.c | 294 ++++++++++++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
OvmfPkg/PlatformPei/AmdSev.c | 231 +++++++++
OvmfPkg/PlatformPei/MemDetect.c | 2 +
OvmfPkg/Sec/AmdSev.c | 298 ++++++++++++
OvmfPkg/Sec/SecMain.c | 158 +------
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 239 ++++++++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 16 +-
UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c | 70 +++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 345 +++++---------
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 4 +-
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c | 261 ++++++++++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 17 +
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 86 +++-
OvmfPkg/ResetVector/ResetVector.nasmb | 18 +
OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm | 74 +++
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 200 ++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 100 +---
59 files changed, 3329 insertions(+), 528 deletions(-)
create mode 100644 MdePkg/Include/ConfidentialComputingGuestAttr.h
create mode 100644
OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Sec/AmdSev.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Sec/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
create mode 100644 OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm

--
2.25.1


Event: TianoCore Design Meeting - APAC/NAMO - 10/29/2021 #cal-reminder

devel@edk2.groups.io Calendar <noreply@...>
 

Reminder: TianoCore Design Meeting - APAC/NAMO

When:
10/29/2021
9:30am to 10:30am
(UTC+08:00) Asia/Shanghai

Where:
Microsoft Teams

Organizer: Ray Ni ray.ni@...

View Event

Description:

TOPIC

  1. NA

For more info, see here: https://www.tianocore.org/design-meeting/


Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 119 715 416 0

Alternate VTC dialing instructions

Learn More | Meeting options


Re: [edk2-libc Patch 1/1] AppPkg/Applications/Python: Remove py2.7.2 support from edk2-libc

Michael D Kinney
 

Can you please send V2 of the patch series with the Readme changes in its own patch?

Thanks,

Mike

-----Original Message-----
From: Jayaprakash, N <n.jayaprakash@...>
Sent: Friday, October 22, 2021 4:23 AM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io
Cc: Rebecca Cran <rebecca@...>
Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] AppPkg/Applications/Python: Remove py2.7.2 support from edk2-libc

Hi Mike,

Could you look into this and let me know if there is anything else need to be done.

Regards,
JP
-----Original Message-----
From: Jayaprakash, N
Sent: 20 October 2021 23:15
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io
Cc: Rebecca Cran <rebecca@...>
Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] AppPkg/Applications/Python: Remove py2.7.2 support from edk2-libc

Hi Mike,

Thanks for the review comments.

The PythonReadMe.txt available @ https://github.com/tianocore/edk2-
libc/blob/master/AppPkg/Applications/Python/PythonReadMe.txt
is the readme file for Py2.7.2 and we don't need to retain this file. So I have deleted this file as part of the patch
sent for review.

Py 2.7.10 and Py 3.6.8 have their respective readme files as Py2710ReadMe.txt @ https://github.com/jpshivakavi/edk2-
libc/tree/master/AppPkg/Applications/Python/Python-2.7.10
Py368ReadMe.txt @ https://github.com/jpshivakavi/edk2-libc/tree/master/AppPkg/Applications/Python/Python-3.6.8


Besides this, I have taken care of all the other documentation changes required as given below

Updated the readme.md file from this location and removed the reference to Py2.7.2 license
https://github.com/tianocore/edk2-libc/blob/master/Readme.md

AppPkg/Applications/Python/Python-2.7.2/Tools/pybench
AppPkg/Applications/Python/Python-2.7.2

Updated the readme.txt from the below location to remove references to 2.7.2 and replace it with 3.6.8 references.
https://github.com/tianocore/edk2-libc/blob/master/AppPkg/ReadMe.txt
Also updated the version of this readme file along with the date
Version 1.03
18 Oct. 2021


Besides documentation changes following changes have been done to delete py 2.7.2 support from edk2-libc
Updated the AppPkg.dsc file to remove the Python 2.7.2 inf references.
https://github.com/jpshivakavi/edk2-libc/blob/remove_py272_support/AppPkg/AppPkg.dsc


Removed all files and folders corresponding to Py2.7.2 support from
https://github.com/jpshivakavi/edk2-libc/tree/master/AppPkg/Applications/Python
Efi\
Ia32\
PyMod-2.7.2\
Python-2.7.2\
X64\
PythonCore.inf // Inf file for py 2.7.2
PythonReadme.txt // Readme file for Py 2.7.2


Let me know if there is anything else needed.

Regards,
JP

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: 20 October 2021 21:35
To: devel@edk2.groups.io; Jayaprakash, N <n.jayaprakash@...>; Kinney, Michael D <michael.d.kinney@...>
Cc: Rebecca Cran <rebecca@...>
Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] AppPkg/Applications/Python: Remove py2.7.2 support from edk2-libc

Hi JP,

Can you also update the documentation to remove references to Python 2.x or update for Python 3.x?

For example, the following file has Python 2.x references.

https://github.com/tianocore/edk2-libc/blob/master/AppPkg/Applications/Python/PythonReadMe.txt

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Jayaprakash, N
Sent: Tuesday, October 19, 2021 8:43 PM
To: devel@edk2.groups.io
Cc: Rebecca Cran <rebecca@...>; Kinney, Michael D
<michael.d.kinney@...>; Jayaprakash, N <n.jayaprakash@...>
Subject: [edk2-devel] [edk2-libc Patch 1/1]
AppPkg/Applications/Python: Remove py2.7.2 support from edk2-libc





Re: Return EFI_INVALID_PARAMETER if attribute only has EFI_VARIABLE_NON_VOLATILE set

Heinrich Schuchardt
 

On 10/21/21 12:18, Sunny Wang wrote:
Hi Liming, Hao, and all

Now we’re checking the SCT runtime variable service test case.
https://github.com/tianocore/edk2-test/blob/92a0343c1553342c53fae9d9d646b763add232c0/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestConformance.c#L3401
<https://github.com/tianocore/edk2-test/blob/92a0343c1553342c53fae9d9d646b763add232c0/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestConformance.c#L3401>
and have a question below.

Is there any use case to call the runtime variable service functions
with the Attributes that only has EFI_VARIABLE_NON_VOLATILE set?

We checked UEFI spec, documents, and current EDK2 implementation. There
is no specific description or any implementation for this. However,
there seems an implication that EFI_VARIABLE_NON_VOLATILE must be set
with at least EFI_VARIABLE_BOOTSERVICE_ACCESS.  Actually, it looks like
making NO sense to have a variable attribute combination that doesn’t
have any XXXXX_ACCESS attribute (BS, RT, or AT) set.

Therefore, we think only having EFI_VARIABLE_NON_VOLATILE set may be an
invalid case and would like to add a check into the EDK2 variable driver
to return EFI_INVALID_PARAMETER.  What do you guys think?
The Self Certification Test (SCT) II Case Specification, June 2017
explicitly forbids this value for QueryVariableInfo():

<cite>
5.2.1.4.5

Call QueryVariableInfo service with the Attributes:

* EFI_VARIABLE_NON_VOLATILE
* EFI_VARIABLE_RUNTIME_ACCESS
* EFI_VARIABLE_NON_VOLATILE|EFI_VARIABLE_RUNTIME_ACCESS

The returned code must be EFI_INVALID_PARAMETER.
</cite>

This corresponds to the UEFI specification saying:
<cite>
QueryVariableInfo()
Status Codes returned
EFI_INVALID_PARAMETER:
An invalid combination of attribute bits was supplied.
</cite>

A variable being not accessible at BootTime seems not to be foreseen by
the specification:

<cite>
SetVariable()
...
If software uses a nonvolatile variable, it should use a variable that
is only accessible at boot services time if possible.
...
Attributes that have EFI_VARIABLE_RUNTIME_ACCESS set must also have
EFI_VARIABLE_BOOTSERVICE_ACCESS set.
</cite>

This sounds like a nonvolatile variable should always be accessible at
boot services time. But an explicit rule forbidding the creation of
inaccessible variables, i.e. without EFI_VARIABLE_BOOTSERVICE_ACCESS, is
missing.

It would be good to have a paragraph in the specification that
unambiguously defines which combination of attribute bits are valid.

How about:

"All variables must be created with attribute bit
EFI_VARIABLE_BOOTSERVICE_ACCESS."

Best regards

Heinrich


Best Regards,

Sunny


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

Ard Biesheuvel
 

On Sat, 23 Oct 2021 at 09:34, Nhi Pham <nhi@...> wrote:

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

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

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

The PlatformBootManagerLib library derived from
ArmPkg/Library/PlatformBootManagerLib.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>

Signed-off-by: Nhi Pham <nhi@...>
Acked-by: Ard Biesheuvel <ardb@...>
Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Acked-by: Moritz Fischer <moritzf@...>
Merged as #2114

Thanks,

---
Changes from v2:
* Added description for PlatformRegisterFvBootOption function to
resolve the CI failure.

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

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

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


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

Nhi Pham
 

Hi Ard,

I've just sent the v3 for fixing this CI issue. Please help check it.
v3 patch: https://edk2.groups.io/g/devel/message/82598

On 23/10/2021 09:34, Moritz Fischer wrote:
On Fri, Oct 22, 2021 at 11:08 AM Ard Biesheuvel <ardb@...> wrote:
This patch triggers CI failures

https://github.com/tianocore/edk2/pull/2114

Please take a look and resubmit if there is anything to fix.
Looks like a missing comment?
Thanks, Moritz for pointing out.

Best regards,

Nhi

On Wed, 13 Oct 2021 at 20:43, Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@...> wrote:
Ackd-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Acked-by: Moritz Fischer <moritzf@...>
Any update on getting this reviewed/merged? We have downstream platforms that depend on this and would like to avoid duplication of similar functionality in platform code.

Thanks!
--Samer


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

On Tue, 7 Sept 2021 at 05:40, Nhi Pham <nhi@...>
wrote:
LinuxBoot is a firmware that replaces specific firmware functionality
like the UEFI DXE phase with a Linux kernel and runtime. It is built-in
UEFI image like an application, which is executed at the end of DXE
phase.

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

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

The PlatformBootManagerLib library derived from
ArmPkg/Library/PlatformBootManagerLib.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>

Signed-off-by: Nhi Pham <nhi@...>
Acked-by: Ard Biesheuvel <ardb@...>

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

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

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


[PATCH v3 1/1] ArmPkg: Implement PlatformBootManagerLib for LinuxBoot

Nhi Pham
 

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

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

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

The PlatformBootManagerLib library derived from
ArmPkg/Library/PlatformBootManagerLib.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>

Signed-off-by: Nhi Pham <nhi@...>
Acked-by: Ard Biesheuvel <ardb@...>
Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Acked-by: Moritz Fischer <moritzf@...>
---
Changes from v2:
* Added description for PlatformRegisterFvBootOption function to
resolve the CI failure.

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

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

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

9781 - 9800 of 92312