[edk2-devel] [PATCH v4 3/3] OvmfPkg: Add sp800155Event3 support
The signatures for event2 or event3 are now valid TCG SP800155 event
types. Fixes uncrustify formatting.
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Reviewed-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
index 6ca29f5de0..5241f60891 100644
--- a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
+++ b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
@@ -821,11 +821,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
- NewEventData,
- TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
- sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ((CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0)))
{
return TRUE;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118613): https://edk2.groups.io/g/devel/message/118613
Mute This Topic: https://groups.io/mt/105945155/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v4 2/3] SecurityPkg: Recognize sp800155Event3 event
The signatures for event2 or event3 are now valid TCG SP800155 event
types. Fixes uncrustify formatting.
Cc: Jiewen Yao
Cc: Rahul Kumar
Reviewed-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index b8f50e25df..b55b6c12d2 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -812,11 +812,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
- NewEventData,
- TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
- sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ((CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0)))
{
return TRUE;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118612): https://edk2.groups.io/g/devel/message/118612
Mute This Topic: https://groups.io/mt/105945152/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v4 1/3] MdePkg: Add TcgSp800155Event3 type info
TCG PC Client Platform Firmware Profile 1.06 revision 52 of December
2023 added a new event signature and extended information about where a
reference measurement document for the firmware can be found.
Cc: Michael D Kinney
Cc: Liming Gao
Cc: Zhiguang Liu
Reviewed-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
.../IndustryStandard/UefiTcgPlatform.h| 38 ++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 61bd4e4667..aaee5d6c88 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -451,6 +451,7 @@ typedef struct tdTCG_PCClientTaggedEvent {
#define TCG_Sp800_155_PlatformId_Event_SIGNATURE "SP800-155 Event"
#define TCG_Sp800_155_PlatformId_Event2_SIGNATURE "SP800-155 Event2"
+#define TCG_Sp800_155_PlatformId_Event3_SIGNATURE "SP800-155 Event3"
typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
UINT8 Signature[16];
@@ -478,9 +479,44 @@ typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
// UINT8 FirmwareManufacturerStr[FirmwareManufacturerStrSize];
// UINT32 FirmwareManufacturerId;
// UINT8 FirmwareVersion;
- // UINT8 FirmwareVersion[FirmwareVersionSize]];
+ // UINT8 FirmwareVersion[FirmwareVersionSize];
} TCG_Sp800_155_PlatformId_Event2;
+typedef struct tdTCG_Sp800_155_PlatformId_Event3 {
+ UINT8 Signature[16];
+ //
+ // Where Vendor ID is an integer defined
+ // at http://www.iana.org/assignments/enterprisenumbers
+ //
+ UINT32 VendorId;
+ //
+ // 16-byte identifier of a given platform's static configuration of code
+ //
+ EFI_GUIDReferenceManifestGuid;
+ // UINT8 PlatformManufacturerStrSize;
+ // UINT8 PlatformManufacturerStr[PlatformManufacturerStrSize];
+ // UINT8 PlatformModelSize;
+ // UINT8 PlatformModel[PlatformModelSize];
+ // UINT8 PlatformVersionSize;
+ // UINT8 PlatformVersion[PlatformVersionSize];
+ // UINT8 PlatformModelSize;
+ // UINT8 PlatformModel[PlatformModelSize];
+ // UINT8 FirmwareManufacturerStrSize;
+ // UINT8 FirmwareManufacturerStr[FirmwareManufacturerStrSize];
+ // UINT32 FirmwareManufacturerId;
+ // UINT8 FirmwareVersion;
+ // UINT8 FirmwareVersion[FirmwareVersionSize];
+ //
+ // Below structure is newly added in TCG_Sp800_155_PlatformId_Event3
+ //
+ // UINT32 RimLocatorType;
+ // UINT32 RimLocatorLength;
+ // UINT8 RimLocator[RimLocatorLength];
+ // UINT32 PlatformCertLocatorType;
+ // UINT32 PlatformCertLocatorLength;
+ // UINT8 PlatformCertLocator[PlatformCertLocatorLength];
+} TCG_Sp800_155_PlatformId_Event3;
+
#define TCG_EfiStartupLocalityEvent_SIGNATURE "StartupLocality"
//
--
2.45.0.rc1.225.g2a3ae87e7f-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118611): https://edk2.groups.io/g/devel/message/118611
Mute This Topic: https://groups.io/mt/105945151/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v4 0/3] TCG_Sp800_155_PlatformId_Event3 support
In December 2023, the TCG published the PC Client Platform Firmware Profile version 1.06 revision 52. This revision includes a new event type for NIST SP 800-155 recommended signed BIOS reference measurements. The new type allows for the event log auditor to find local or remote copies of the signed reference measurements. Supporting this new event type eases the process of distributing signed reference measurements since the machine can now simply report where they can be found in a standard way. Changes since v3: - Fixed build error from 1 too many ')'s. - Fixed formatting for uncrustify. Changes since v2: - Removed errant spacing. Changes since v1: - MdePkg defines TCG_Sp800_155_PlatformId_Event3 instead of adding a comment about Event3 to Event2. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Cc: Jiewen Yao Cc: Rahul Kumar Cc: Ard Biesheuvel Cc: Gerd Hoffmann Reviewed-by: Jiewen Yao Dionna Glaze (3): MdePkg: Add TcgSp800155Event3 type info SecurityPkg: Recognize sp800155Event3 event OvmfPkg: Add sp800155Event3 support .../IndustryStandard/UefiTcgPlatform.h| 38 ++- OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 15 +--- SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 15 +--- 3 files changed, 57 insertions(+), 11 deletions(-) -- 2.45.0.rc1.225.g2a3ae87e7f-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118610): https://edk2.groups.io/g/devel/message/118610 Mute This Topic: https://groups.io/mt/105945150/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 0/3] TCG_Sp800_155_PlatformId_Event3 support
I had not passed some tests, apologies. I fixed the spacing issue and build failure with too many )s in https://github.com/tianocore/edk2/pull/5615. Shall I email a v4? On Sun, May 5, 2024 at 8:28 PM Yao, Jiewen wrote: > > Hi Dionna > I tried to create PR but I saw failure - > https://github.com/tianocore/edk2/pull/5628 > > Would you please clarify if you have tested the patch in EDKII CI, before you > submit the patch? > > > BTW: I have fixed a typo in the V3 patch. The "Reviewed-By" tag in 1/3 should > be "Reviewed-by". > > Thank you > Yao, Jiewen > > > > > -Original Message- > > From: Dionna Glaze > > Sent: Thursday, May 2, 2024 8:50 AM > > To: devel@edk2.groups.io > > Cc: Dionna Glaze ; Kinney, Michael D > > ; Liming Gao ; Liu, > > Zhiguang ; Yao, Jiewen ; > > Kumar, Rahul R ; Ard Biesheuvel > > ; Gerd Hoffmann > > Subject: [PATCH v3 0/3] TCG_Sp800_155_PlatformId_Event3 support > > > > In December 2023, the TCG published the PC Client Platform Firmware > > Profile version 1.06 revision 52. This revision includes a new event > > type for NIST SP 800-155 recommended signed BIOS reference measurements. > > The new type allows for the event log auditor to find local or remote > > copies of the signed reference measurements. > > > > Supporting this new event type eases the process of distributing signed > > reference measurements since the machine can now simply report where > > they can be found in a standard way. > > > > Changes since v2: > > - Removed errant spacing. > > Changes since v1: > > - MdePkg defines TCG_Sp800_155_PlatformId_Event3 instead of adding a > > comment about Event3 to Event2. > > > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Zhiguang Liu > > Cc: Jiewen Yao > > Cc: Rahul Kumar > > Cc: Ard Biesheuvel > > Cc: Gerd Hoffmann > > Reviewed-by: Jiewen Yao > > > > Dionna Glaze (3): > > MdePkg: Add TcgSp800155Event3 type info > > SecurityPkg: recognize sp800155Event3 event too > > OvmfPkg: add sp800155Event3 support > > > > .../IndustryStandard/UefiTcgPlatform.h| 38 ++- > > OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 9 - > > SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 9 - > > 3 files changed, 51 insertions(+), 5 deletions(-) > > > > -- > > 2.45.0.rc0.197.gbae5840b3b-goog -- -Dionna Glaze, PhD, CISSP (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118609): https://edk2.groups.io/g/devel/message/118609 Mute This Topic: https://groups.io/mt/105854725/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On Thu, May 2, 2024 at 8:59 AM Brian J. Johnson wrote: > > On 5/1/24 18:19, Dionna Glaze via groups.io wrote: > > On Wed, May 1, 2024 at 11:12 AM Leif Lindholm via groups.io > > wrote: > >> > >> On 2024-05-01 18:43, Michael D Kinney wrote: > >>> Hello, > >>> > >>> I would like to propose that TianoCore move all code review from email > >>> based code reviews to GitHub Pull Requests based code reviews. > >>> > >>> The proposed date to switch would be immediately after the next stable > >>> tag which is currently scheduled for May 24, 2024. > >> > >> Thanks Mike. > >> > >> I'm in favour of this change, and the date. > >> > >> I still want us to try to figure out how to retain review history beyond > >> what github decides we need, but I don't think it justifies indefinitely > >> delaying the switchover. And frankly, it will be easier to experiment > >> with what works and not after the switch. > > > > +1. UI-based interactions don't export well for archival-permalinking > > reasons, and Github archive behavior is for repositories only, not the > > reviews. > > But yes, wouldn't want to delay for a bot to echo conversations to > > devel@edk2.groups.io or some other solution. > > > > +1 from me as well. We need to maintain review history in some fairly > permanent manner, both the reviewed code and review comments. > > >> > >> / > >> Leif > >> > >>> Updates to the following Wiki page would be required to describe the > >>> required process when using GitHub Pull Requests for all code review > >>> related activity. > >>> > >>> > >>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > >>> > >>> A couple examples of the changes that would need to be documented are: > >>> > >>> * All contributors, maintainers, and reviewers must have GitHub IDs. > > > > It looks like this is already resolved for the existing > > Maintainers.txt with the `[username]` syntax, but I don't see any > > explanation of that expectation. Seems fine to me. > > > >>> * The commit message would no longer require Cc:, Reviewed-by:, Acked-by: > >>> or Tested-by: tags. The only required tag would be Signed-off-by. > > Would those tags be optional? Test and ack info can be helpful when > researching a change, to find people who may be knowledgeable about it. > > Similarly, the Reviewed-by info is nice to have in the history, without > having to dig it out of archives. But it's a bit awkward to add on > github: you have to push new commits with the Reviewed-by tags, but > that changes the SHAs, so it's not obvious that the commits you're > merging have the same code as the ones which were reviewed. For the > email flow, we trust maintainers to get this right. For the github > flow, are we deciding to rely exclusively on the PR archives? > > What if a maintainer decides to tweak a commit before merging it, eg. to > fix a trivial typo? With the email flow they just go ahead and do it. > With the github flow, would they need to post another PR, so it could > make it through the process and be merged? > > >>> * The Pull Request submitter is required to invite the required > >>> maintainers and reviewers to the pull request. This is the same > >>> set of maintainers and reviewers that are required to be listed in > >>> Cc: tags in today's process. > > > > This is not configured on tianocore/edk2 at the moment. I have no way > > to invite a reviewer. Is this a planned fix? > > > >>> * Maintainers are responsible for verifying that all conversations in > >>> the code review are resolved and that all review approvals from the > >>> required set of maintainers are present before setting the 'push' > >>> label. > > Will there be documentation on how to use the conversation resolution > feature? It's unclear to me whether the PR owner or the reviewer is > responsible for marking a conversation "resolved." > Github has branch security features that let you _require_ that all messages are resolved before merging, so that could be turned on. > >>> > >>> > >>> Please provide feedback > >>> 1) If you are not in favor of this change. > >>> 2) If you are not in favor of the proposed date of this change. > >>> 3) On the process changes you would like to see docum
[edk2-devel] [PATCH v3 3/3] OvmfPkg: add sp800155Event3 support
The signatures for event2 or event3 are now valid TCG SP800155 event
types.
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Reviewed-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
index 6ca29f5de0..d487f5c715 100644
--- a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
+++ b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
@@ -821,11 +821,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
+ ((CompareMem (
NewEventData,
TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0
{
return TRUE;
}
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118488): https://edk2.groups.io/g/devel/message/118488
Mute This Topic: https://groups.io/mt/105854728/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 2/3] SecurityPkg: recognize sp800155Event3 event too
The signatures for event2 or event3 are now valid TCG SP800155 event
types.
Cc: Jiewen Yao
Cc: Rahul Kumar
Reviewed-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index b8f50e25df..2f73237984 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -812,11 +812,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
+ ((CompareMem (
NewEventData,
TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0)))
{
return TRUE;
}
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118487): https://edk2.groups.io/g/devel/message/118487
Mute This Topic: https://groups.io/mt/105854727/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 1/3] MdePkg: Add TcgSp800155Event3 type info
TCG PC Client Platform Firmware Profile 1.06 revision 52 of December
2023 added a new event signature and extended information about where a
reference measurement document for the firmware can be found.
Cc: Michael D Kinney
Cc: Liming Gao
Cc: Zhiguang Liu
Reviewed-By: Jiewen Yao
Signed-off-by: Dionna Glaze
---
.../IndustryStandard/UefiTcgPlatform.h| 38 ++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 61bd4e4667..aaee5d6c88 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -451,6 +451,7 @@ typedef struct tdTCG_PCClientTaggedEvent {
#define TCG_Sp800_155_PlatformId_Event_SIGNATURE "SP800-155 Event"
#define TCG_Sp800_155_PlatformId_Event2_SIGNATURE "SP800-155 Event2"
+#define TCG_Sp800_155_PlatformId_Event3_SIGNATURE "SP800-155 Event3"
typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
UINT8 Signature[16];
@@ -478,9 +479,44 @@ typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
// UINT8 FirmwareManufacturerStr[FirmwareManufacturerStrSize];
// UINT32 FirmwareManufacturerId;
// UINT8 FirmwareVersion;
- // UINT8 FirmwareVersion[FirmwareVersionSize]];
+ // UINT8 FirmwareVersion[FirmwareVersionSize];
} TCG_Sp800_155_PlatformId_Event2;
+typedef struct tdTCG_Sp800_155_PlatformId_Event3 {
+ UINT8 Signature[16];
+ //
+ // Where Vendor ID is an integer defined
+ // at http://www.iana.org/assignments/enterprisenumbers
+ //
+ UINT32 VendorId;
+ //
+ // 16-byte identifier of a given platform's static configuration of code
+ //
+ EFI_GUIDReferenceManifestGuid;
+ // UINT8 PlatformManufacturerStrSize;
+ // UINT8 PlatformManufacturerStr[PlatformManufacturerStrSize];
+ // UINT8 PlatformModelSize;
+ // UINT8 PlatformModel[PlatformModelSize];
+ // UINT8 PlatformVersionSize;
+ // UINT8 PlatformVersion[PlatformVersionSize];
+ // UINT8 PlatformModelSize;
+ // UINT8 PlatformModel[PlatformModelSize];
+ // UINT8 FirmwareManufacturerStrSize;
+ // UINT8 FirmwareManufacturerStr[FirmwareManufacturerStrSize];
+ // UINT32 FirmwareManufacturerId;
+ // UINT8 FirmwareVersion;
+ // UINT8 FirmwareVersion[FirmwareVersionSize];
+ //
+ // Below structure is newly added in TCG_Sp800_155_PlatformId_Event3
+ //
+ // UINT32 RimLocatorType;
+ // UINT32 RimLocatorLength;
+ // UINT8 RimLocator[RimLocatorLength];
+ // UINT32 PlatformCertLocatorType;
+ // UINT32 PlatformCertLocatorLength;
+ // UINT8 PlatformCertLocator[PlatformCertLocatorLength];
+} TCG_Sp800_155_PlatformId_Event3;
+
#define TCG_EfiStartupLocalityEvent_SIGNATURE "StartupLocality"
//
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118486): https://edk2.groups.io/g/devel/message/118486
Mute This Topic: https://groups.io/mt/105854726/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 0/3] TCG_Sp800_155_PlatformId_Event3 support
In December 2023, the TCG published the PC Client Platform Firmware Profile version 1.06 revision 52. This revision includes a new event type for NIST SP 800-155 recommended signed BIOS reference measurements. The new type allows for the event log auditor to find local or remote copies of the signed reference measurements. Supporting this new event type eases the process of distributing signed reference measurements since the machine can now simply report where they can be found in a standard way. Changes since v2: - Removed errant spacing. Changes since v1: - MdePkg defines TCG_Sp800_155_PlatformId_Event3 instead of adding a comment about Event3 to Event2. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Cc: Jiewen Yao Cc: Rahul Kumar Cc: Ard Biesheuvel Cc: Gerd Hoffmann Reviewed-by: Jiewen Yao Dionna Glaze (3): MdePkg: Add TcgSp800155Event3 type info SecurityPkg: recognize sp800155Event3 event too OvmfPkg: add sp800155Event3 support .../IndustryStandard/UefiTcgPlatform.h| 38 ++- OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 9 - SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 9 - 3 files changed, 51 insertions(+), 5 deletions(-) -- 2.45.0.rc0.197.gbae5840b3b-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118485): https://edk2.groups.io/g/devel/message/118485 Mute This Topic: https://groups.io/mt/105854725/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On Wed, May 1, 2024 at 11:12 AM Leif Lindholm via groups.io wrote: > > On 2024-05-01 18:43, Michael D Kinney wrote: > > Hello, > > > > I would like to propose that TianoCore move all code review from email > > based code reviews to GitHub Pull Requests based code reviews. > > > > The proposed date to switch would be immediately after the next stable > > tag which is currently scheduled for May 24, 2024. > > Thanks Mike. > > I'm in favour of this change, and the date. > > I still want us to try to figure out how to retain review history beyond > what github decides we need, but I don't think it justifies indefinitely > delaying the switchover. And frankly, it will be easier to experiment > with what works and not after the switch. +1. UI-based interactions don't export well for archival-permalinking reasons, and Github archive behavior is for repositories only, not the reviews. But yes, wouldn't want to delay for a bot to echo conversations to devel@edk2.groups.io or some other solution. > > / > Leif > > > Updates to the following Wiki page would be required to describe the > > required process when using GitHub Pull Requests for all code review > > related activity. > > > > > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > > > > A couple examples of the changes that would need to be documented are: > > > > * All contributors, maintainers, and reviewers must have GitHub IDs. It looks like this is already resolved for the existing Maintainers.txt with the `[username]` syntax, but I don't see any explanation of that expectation. Seems fine to me. > > * The commit message would no longer require Cc:, Reviewed-by:, Acked-by: > >or Tested-by: tags. The only required tag would be Signed-off-by. > > * The Pull Request submitter is required to invite the required > >maintainers and reviewers to the pull request. This is the same > >set of maintainers and reviewers that are required to be listed in > >Cc: tags in today's process. This is not configured on tianocore/edk2 at the moment. I have no way to invite a reviewer. Is this a planned fix? > > * Maintainers are responsible for verifying that all conversations in > >the code review are resolved and that all review approvals from the > >required set of maintainers are present before setting the 'push' label. > > > > > > Please provide feedback > > 1) If you are not in favor of this change. > > 2) If you are not in favor of the proposed date of this change. > > 3) On the process changes you would like to see documented in the Wiki > > pages related to using GitHub Pull Request based code reviews. > > > > There is some prototype work to automate/simplify some of the PR based > > code review process steps. Those could be added over time as resources > > are available to finish and support them. > > > > Best regards, > > > > Mike > > > > > > > > > > > > > > > > -- -Dionna Glaze, PhD, CISSP (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118483): https://edk2.groups.io/g/devel/message/118483 Mute This Topic: https://groups.io/mt/105847510/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 3/3] OvmfPkg: add sp800155Event3 support
The signatures for event2 or event3 are now valid TCG SP800155 event
types.
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Reviewed-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
index 6ca29f5de0..d487f5c715 100644
--- a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
+++ b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
@@ -821,11 +821,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
+ ((CompareMem (
NewEventData,
TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0
{
return TRUE;
}
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118466): https://edk2.groups.io/g/devel/message/118466
Mute This Topic: https://groups.io/mt/105845526/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 2/3] SecurityPkg: recognize sp800155Event3 event too
The signatures for event2 or event3 are now valid TCG SP800155 event
types.
Cc: Jiewen Yao
Cc: Rahul Kumar
Reviewed-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index b8f50e25df..2f73237984 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -812,11 +812,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
+ ((CompareMem (
NewEventData,
TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0)))
{
return TRUE;
}
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118465): https://edk2.groups.io/g/devel/message/118465
Mute This Topic: https://groups.io/mt/105845525/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 1/3] MdePkg: Add TcgSp800155Event3 type info
TCG PC Client Platform Firmware Profile 1.06 revision 52 of December
2023 added a new event signature and extended information about where a
reference measurement document for the firmware can be found.
Cc: Michael D Kinney
Cc: Liming Gao
Cc: Zhiguang Liu
Signed-off-by: Dionna Glaze
---
.../IndustryStandard/UefiTcgPlatform.h| 40 ++-
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 61bd4e4667..54bdf3a339 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -451,6 +451,7 @@ typedef struct tdTCG_PCClientTaggedEvent {
#define TCG_Sp800_155_PlatformId_Event_SIGNATURE "SP800-155 Event"
#define TCG_Sp800_155_PlatformId_Event2_SIGNATURE "SP800-155 Event2"
+#define TCG_Sp800_155_PlatformId_Event3_SIGNATURE "SP800-155 Event3"
typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
UINT8 Signature[16];
@@ -463,7 +464,7 @@ typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
// 16-byte identifier of a given platform's static configuration of code
//
EFI_GUIDReferenceManifestGuid;
- //
+ //
// Below structure is newly added in TCG_Sp800_155_PlatformId_Event2.
//
// UINT8 PlatformManufacturerStrSize;
@@ -478,9 +479,44 @@ typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
// UINT8 FirmwareManufacturerStr[FirmwareManufacturerStrSize];
// UINT32 FirmwareManufacturerId;
// UINT8 FirmwareVersion;
- // UINT8 FirmwareVersion[FirmwareVersionSize]];
+ // UINT8 FirmwareVersion[FirmwareVersionSize];
} TCG_Sp800_155_PlatformId_Event2;
+typedef struct tdTCG_Sp800_155_PlatformId_Event3 {
+ UINT8 Signature[16];
+ //
+ // Where Vendor ID is an integer defined
+ // at http://www.iana.org/assignments/enterprisenumbers
+ //
+ UINT32 VendorId;
+ //
+ // 16-byte identifier of a given platform's static configuration of code
+ //
+ EFI_GUIDReferenceManifestGuid;
+ // UINT8 PlatformManufacturerStrSize;
+ // UINT8 PlatformManufacturerStr[PlatformManufacturerStrSize];
+ // UINT8 PlatformModelSize;
+ // UINT8 PlatformModel[PlatformModelSize];
+ // UINT8 PlatformVersionSize;
+ // UINT8 PlatformVersion[PlatformVersionSize];
+ // UINT8 PlatformModelSize;
+ // UINT8 PlatformModel[PlatformModelSize];
+ // UINT8 FirmwareManufacturerStrSize;
+ // UINT8 FirmwareManufacturerStr[FirmwareManufacturerStrSize];
+ // UINT32 FirmwareManufacturerId;
+ // UINT8 FirmwareVersion;
+ // UINT8 FirmwareVersion[FirmwareVersionSize];
+ //
+ // Below structure is newly added in TCG_Sp800_155_PlatformId_Event3
+ //
+ // UINT32 RimLocatorType;
+ // UINT32 RimLocatorLength;
+ // UINT8 RimLocator[RimLocatorLength];
+ // UINT32 PlatformCertLocatorType;
+ // UINT32 PlatformCertLocatorLength;
+ // UINT8 PlatformCertLocator[PlatformCertLocatorLength];
+} TCG_Sp800_155_PlatformId_Event3;
+
#define TCG_EfiStartupLocalityEvent_SIGNATURE "StartupLocality"
//
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118464): https://edk2.groups.io/g/devel/message/118464
Mute This Topic: https://groups.io/mt/105845524/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 0/3] TCG_Sp800_155_PlatformId_Event3 support
In December 2023, the TCG published the PC Client Platform Firmware Profile version 1.06 revision 52. This revision includes a new event type for NIST SP 800-155 recommended signed BIOS reference measurements. The new type allows for the event log auditor to find local or remote copies of the signed reference measurements. Supporting this new event type eases the process of distributing signed reference measurements since the machine can now simply report where they can be found in a standard way. Changes since v1: - MdePkg defines TCG_Sp800_155_PlatformId_Event3 instead of adding a comment about Event3 to Event2. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Cc: Jiewen Yao Cc: Rahul Kumar Cc: Ard Biesheuvel Cc: Gerd Hoffmann Reviewed-by: Jiewen Yao Dionna Glaze (3): MdePkg: Add TcgSp800155Event3 type info SecurityPkg: recognize sp800155Event3 event too OvmfPkg: add sp800155Event3 support .../IndustryStandard/UefiTcgPlatform.h| 40 ++- OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 9 - SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 9 - 3 files changed, 52 insertions(+), 6 deletions(-) -- 2.45.0.rc0.197.gbae5840b3b-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118463): https://edk2.groups.io/g/devel/message/118463 Mute This Topic: https://groups.io/mt/105845523/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 3/3] OvmfPkg: add sp800155Event3 support
The signatures for event2 or event3 are now valid TCG SP800155 event
types.
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Signed-off-by: Dionna Glaze
---
OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
index 6ca29f5de0..d487f5c715 100644
--- a/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
+++ b/OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
@@ -821,11 +821,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
+ ((CompareMem (
NewEventData,
TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0
{
return TRUE;
}
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118451): https://edk2.groups.io/g/devel/message/118451
Mute This Topic: https://groups.io/mt/105833240/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/3] SecurityPkg: recognize sp800155Event3 event too
The signatures for event2 or event3 are now valid TCG SP800155 event
types.
Cc: Jiewen Yao
Cc: Rahul Kumar
Signed-off-by: Dionna Glaze
---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index b8f50e25df..2f73237984 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -812,11 +812,16 @@ Is800155Event (
{
if TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
(NewEventSize >= sizeof (TCG_Sp800_155_PlatformId_Event2)) &&
- (CompareMem (
+ ((CompareMem (
NewEventData,
TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
sizeof (TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1
- ) == 0))
+ ) == 0) ||
+ (CompareMem (
+ NewEventData,
+ TCG_Sp800_155_PlatformId_Event3_SIGNATURE,
+ sizeof (TCG_Sp800_155_PlatformId_Event3_SIGNATURE) - 1
+ ) == 0)))
{
return TRUE;
}
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118450): https://edk2.groups.io/g/devel/message/118450
Mute This Topic: https://groups.io/mt/105833239/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 0/3] TCG_Sp800_155_PlatformId_Event3 support
In December 2023, the TCG published the PC Client Platform Firmware Profile version 1.06 revision 52. This revision includes a new event type for NIST SP 800-155 recommended signed BIOS reference measurements. The new type allows for the event log auditor to find local or remote copies of the signed reference measurements. Supporting this new event type eases the process of distributing signed reference measurements since the machine can now simply report where they can be found in a standard way. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Cc: Jiewen Yao Cc: Rahul Kumar Cc: Ard Biesheuvel Cc: Gerd Hoffmann Dionna Glaze (3): MdePkg: Add TcgSp800155Event3 type info SecurityPkg: recognize sp800155Event3 event too OvmfPkg: add sp800155Event3 support MdePkg/Include/IndustryStandard/UefiTcgPlatform.h | 12 +++- OvmfPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 9 +++-- SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 9 +++-- 3 files changed, 25 insertions(+), 5 deletions(-) -- 2.45.0.rc0.197.gbae5840b3b-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118448): https://edk2.groups.io/g/devel/message/118448 Mute This Topic: https://groups.io/mt/105833236/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/3] MdePkg: Add TcgSp800155Event3 type info
TCG PC Client Platform Firmware Profile 1.06 revision 52 of December
2023 added a new event signature and extended information about where a
reference measurement document for the firmware can be found.
Cc: Michael D Kinney
Cc: Liming Gao
Cc: Zhiguang Liu
Signed-off-by: Dionna Glaze
---
MdePkg/Include/IndustryStandard/UefiTcgPlatform.h | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 61bd4e4667..30df8302b1 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -451,6 +451,7 @@ typedef struct tdTCG_PCClientTaggedEvent {
#define TCG_Sp800_155_PlatformId_Event_SIGNATURE "SP800-155 Event"
#define TCG_Sp800_155_PlatformId_Event2_SIGNATURE "SP800-155 Event2"
+#define TCG_Sp800_155_PlatformId_Event3_SIGNATURE "SP800-155 Event3"
typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
UINT8 Signature[16];
@@ -478,7 +479,16 @@ typedef struct tdTCG_Sp800_155_PlatformId_Event2 {
// UINT8 FirmwareManufacturerStr[FirmwareManufacturerStrSize];
// UINT32 FirmwareManufacturerId;
// UINT8 FirmwareVersion;
- // UINT8 FirmwareVersion[FirmwareVersionSize]];
+ // UINT8 FirmwareVersion[FirmwareVersionSize];
+ //
+ // Below structure is newly added in TCG_Sp800_155_PlatformId_Event3
+ //
+ // UINT32 RimLocatorType;
+ // UINT32 RimLocatorLength;
+ // UINT8 RimLocator[RimLocatorLength];
+ // UINT32 PlatformCertLocatorType;
+ // UINT32 PlatformCertLocatorLength;
+ // UINT8 PlatformCertLocator[PlatformCertLocatorLength];
} TCG_Sp800_155_PlatformId_Event2;
#define TCG_EfiStartupLocalityEvent_SIGNATURE "StartupLocality"
--
2.45.0.rc0.197.gbae5840b3b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118449): https://edk2.groups.io/g/devel/message/118449
Mute This Topic: https://groups.io/mt/105833238/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.
On Mon, Mar 25, 2024 at 6:07 AM Mikko Ylinen wrote: > > > > > > > Looking at systemd-boot I see it will likewise not measure to both RTMR > > > and vTPM, but with reversed priority (use vTPM not RTMR in case both are > > > present). > > > > > > > Interesting. Thanks for this report. We'll push for the changed > > semantics here if the spec is indeed changed, and request partner > > distros in the CCC to include the updated systemd-boot. > > FWIW, my RTMRs patch to systemd was merged quite recently so it's not > included in any systemd release yet. (It was mainly implemented for the > UKI case that allows TDVF to boot a UKI image directly and then have the > image sections measured separately.) > Thank you, I've proposed a change in https://github.com/systemd/systemd/pull/31939 -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117087): https://edk2.groups.io/g/devel/message/117087 Mute This Topic: https://groups.io/mt/105070442/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.
On Fri, Mar 22, 2024 at 1:52 AM Gerd Hoffmann wrote: > > On Fri, Mar 22, 2024 at 02:39:20AM +, Yao, Jiewen wrote: > > Please aware that this option will cause potential security risk. > > > > In case that any the guest component only knows one of vTPM or RTMR, > > and only extends one of vTPM or RTMR, but the other one only verifies > > the other, then the chain of trust is broken. This solution is secure > > if and only if all guest components aware of coexistence, and can > > ensure all measurements are extended to both vTPM and RTMR. But I am > > not sure if all guest components are ready today. > > As far I know (it's been a while I looked at those patches) shim.efi and > grub.efi have support for EFI_CC_MEASUREMENT_PROTOCOL, but use the same > logic we have in DxeTpm2MeasureBootLib, i.e. they will not measure to > both RTMR and vTPM. Shim will measure into CC and then continue to measure into TPM https://github.com/rhboot/shim/blob/126a07ebc30bbd203b6966465b058da741b2654b/tpm.c#L164 GRUB2 has the same behavior. We can at least get coexistence supporting the current boot integrity strategy that Confidential Space is using, which is to depend on a dmverity initramfs whose root hash is in the kernel_cmdline, and a Linux kernel built with LOADPIN. The changes to Linux are proposed but not accepted precisely due to this conversation we're having now. I recall describing this to another CSP engineer at an IETF meeting and they claimed they used the same approach, but I can't remember if that was Oracle or another company. > > Looking at systemd-boot I see it will likewise not measure to both RTMR > and vTPM, but with reversed priority (use vTPM not RTMR in case both are > present). > Interesting. Thanks for this report. We'll push for the changed semantics here if the spec is indeed changed, and request partner distros in the CCC to include the updated systemd-boot. I think that since the current boot integrity story stops at PCR9, we have time to update this component before the attestation method evolves to support a less special-purpose system composition. > Linux kernel appears to not have EFI_CC_MEASUREMENT_PROTOCOL support. > > > Since this option caused a potential risk / misuse breaking the chain > > of trust, I recommend we have at least one more company to endorse the > > runtime co-existence of vTPM and RTMR. Also, I would like to hear the > > opinions from other companies. > > Rumors say intel is working on coconut-svsm support for tdx. That will > most likely allow to use a vTPM with tdx even without depending on the > virtualization host or cloud hyperscaler providing one. We will see VMs > with both RTMR and vTPM and surely need a strategy how guests should > deal with that situation, consistent across the whole boot stack and > not every component doing something different. > An ephemeral TPM that Coconut-svsm offers is qualitatively different than the persistent TPMs in CSPs today. Users of Confidential VMs all have different threat models that allow for trusting a CSP-managed vTPM for sealing keys but not for trusting unencrypted data in use. The boot stack attestation story will not be fully resolved for a long while, and a smooth transition is better than a jarring one. > Given that the vTPM might be provided by the hypervisor and thus not be > part of the TCB I can see that guests might want use both vTPM and RTMR. > So, yes, for that case coexistance makes sense. I'm not convinced it is > a good idea to make that a compile time option though. That will not > help to promote a consistent story ... > I agree, but it does mean we have to change the event log composition to describe the configured measurement services like described above. I think a static Pcd is okay to begin with. The idea for tracking these qualities is through software supply chain endorsements. Eventually that would look like the proposed in-toto's SCAI [1] but until then we'd have a bespoke format that we document and integrate with in an attestation verification service. [1] https://arxiv.org/pdf/2210.05813.pdf -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117055): https://edk2.groups.io/g/devel/message/117055 Mute This Topic: https://groups.io/mt/105070442/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.
On Thu, Mar 21, 2024 at 9:59 AM qinkun Bao wrote: > > From: Qinkun Bao > > The UEFI v2.10 spec defines the protocol EFI_CC_MEASUREMENT_PROTOCOL > to enable (for example) RTMR-based boot measurement for TDX VMs. > With the current UEFI spec’s “should not” wording and EDK2 > implementation, TPM measurement in TDVF is disabled when > RTMR measurement is enabled. > > Mutual exclusion of the CC measurement protocol and TCG measurement > protocol breaks backwards compatibility, which makes adoption of RTMRs > challenging. A virtualized TPM device (vTPM) managed by the host VMM > makes boot measurements visible to the VMM operator, but this is an > oft-requested feature that users can choose to accept. > > The TPM has been a standard for over a decade and many existing > applications rely on the TPM. Both inside and outside Google, > we have many users that require vTPM, including features that are > not easily available via RTMRs (e.g. sealing using keys that the > guest OS cannot access). > > This patch adds a non-default build option to allow the coexistence > of both the CC measurement and TCG protocols. Not included is a > vendor-specific measured event in the CC event log that indicates > whether a vTPM is attached or not. Thank you very much for starting this conversation. I'll carry forward more context from our more senior engineers. ' Not measuring into all possible measurement banks led to (CVE-2021-42299) with TPM PCR banks. Let's not repeat this problem. Requiring a monumental shift of all attestation-based applications to CC_MEASUREMENT_PROTOCOL and CEL in one step is going to make the technology very difficult to adopt. ' The combination of these requirements means careful rollout of attestation verification policies to match the updated behavior of the firmware. The measured boot components have to be known to correctly measure into all available measurement protocols and register banks. The firmware has to be known specifically which protocols it makes available. Now, how this is done is a vendor matter for now. If there is strong demand for making vTPM attachment status known for folks who really don't want a TPM and don't trust the VMM to not attach one anyway, we'll need to agree that the CEL should have an entry for an RTMR0 update detailing the combination of measurement protocols in use. Likewise PCR1 should have an event detailing the protocols in use if we want to make CC_MEASUREMENT_PROTOCOL usage configurable. Philosophizing aside that both protocols should be allowed to be active and that the spec should be updated to say something along the lines of "all measurement protocols that are active (i.e., any combination of EFI_CC_MEASUREMENT_PROTOCOL, EFI_TCG_PROTOCOL, EFI_TCG2_PROTOCOL) should have comparable events measured if any one protocol receives measurements. All measurement protocols that are active MUST measure comparable separator events if any protocol receives a separator event." This is crossing a spec boundary between USWG and TCG, so I don't know where specifically the language needs to go, but we need some language that implementers can use as justification for measuring into any found protocol and not just at most one. It's not lost on me that it is similarly difficult to say that all protocols that are discoverable need to have comparable events measured into them since that _also_ introduces an all-or-nothing migration problem, but at least that's more controllable from the attestation verification policy side than from the UEFI spec side. We're not adding new measurement protocols frequently, so we can get the entire boot chain ready for a new measurement protocol before it's made discoverable. > > Cc: Erdem Aktas > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Gerd Hoffmann > Cc: Tom Lendacky > Cc: Michael Roth > Signed-off-by: Qinkun Bao > --- > OvmfPkg/OvmfPkgX64.dsc | 9 - > .../DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c| 12 +++- > .../DxeTpmMeasurementLib/DxeTpmMeasurementLib.c | 6 ++ > 3 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 56c920168d..9bcee45047 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -32,7 +32,8 @@ >DEFINE SECURE_BOOT_ENABLE = FALSE >DEFINE SMM_REQUIRE = FALSE >DEFINE SOURCE_DEBUG_ENABLE = FALSE > - DEFINE CC_MEASUREMENT_ENABLE = FALSE > + DEFINE CC_MEASUREMENT_ENABLE = TRUE > + DEFINE CC_MEASUREMENT_AND_TCG2_COEXIST = FASLE > > !include OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc > > @@ -99,6 +100,11 @@ >INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable > !endif >RELEASE_*_*_GENFW_FLAGS = --zero > +!if $(CC_MEASUREMENT_ENABLE) == TRUE && $(CC_MEASUREMENT_AND_TCG2_COEXIST) > == TRUE > + MSFT:*_*_*_CC_FLAGS = /D CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE > + INTEL:*_*_*_CC_FLAGS = /D
[edk2-devel] [PATCH] OvmfPkg: Close mAcceptAllMemoryEvent
This event should only trigger once. It should be idempotent, but the allocation of the memory map itself is observable and can cause ExitBootServices to fail with a modified map key. Cc: Ard Biesheuvel Cc: Thomas Lendacky Cc: Erdem Aktas Cc: James Bottomley Cc: Jiewen Yao Cc: Min Xu Cc: Michael Roth Signed-off-by: Dionna Glaze --- OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 + 1 file changed, 1 insertion(+) diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c index 6391d1f775..f9baca90bd 100644 --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c @@ -124,6 +124,7 @@ AcceptAllMemory ( } gBS->FreePool (AllDescMap); + gBS->CloseEvent (mAcceptAllMemoryEvent); return Status; } -- 2.39.1.637.g21b0678d19-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100199): https://edk2.groups.io/g/devel/message/100199 Mute This Topic: https://groups.io/mt/96972431/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
> > Adding the diff. > > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > index 6391d1f775..df51c2c050 100644 > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > @@ -123,7 +123,7 @@ AcceptAllMemory ( > } > } > > - gBS->FreePool (AllDescMap); > + //gBS->FreePool (AllDescMap);^M > return Status; > } > > > > Thanks, > > Pankaj > > > Do you want to propose this patch or shall I? Seems like a necessary fix. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100189): https://edk2.groups.io/g/devel/message/100189 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
> > Do you have any pointers on the IVARS service? Documentation, guest > code, host code? > Agh, I thought for sure there was a public API for VM owners to view or change their UEFI variables, but I guess not. It's an instance-specific small data store for nonvolatile memory like vTPM and UEFI variables. It appears you can only set the variables through cloud API at instance creation time. But this is how instances can be shut down and brought back up on different machines and/or live migrate to other machines and still have access to UEFI variables' current values. Host code is all in Google's proprietary VMM, Vanadium, but the device backend is really rather simple. The data store service though, that's a matter of Cloud Scale Engineering. > Background: When moving to a SVSM-based setup where the svsm (with > vtpm emulation) runs in vmpl0 and the edk2 firmware in vmpl1 we might > likewise add a efi variable service to the svsm. > I thought EFI variables in Qemu were loaded and measured at launch (OVMF_VARS.fd). If you want the current values of all uefi variables in your SVSM attestation report, I think it's probably better to use the EFI_CC_MEASUREMENT_PROTOCOL, right? Or is it specifically going to be an SVSM service that attests itself with current stored variables, or at least variables that are considered important enough to measure? In any case, persistence in The Cloud (TM) remains a challenge in the CC space. Discussion about what we should do about that should remain on the coco mailing list. IVARS encrypts data with Google-managed keys, so it wouldn't be directly applicable to SVSM NVRAM. > If something usable already exists we don't need to reinvent the wheel. > Don't have to tell me twice. In the spirit of OSS collaboration and product integrity, I think any CC offering's firmware should be public and verifiably built. I'll keep pushing for that. > thanks & take care, > Gerd > -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100187): https://edk2.groups.io/g/devel/message/100187 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
> Not solving the issue. Now, getting 4 calls. See below: > > ConvertPages: range 100 - 41AEFFF covers multiple entries > ConvertPages: range 100 - 41AEFFF covers multiple entries > Accepting all memory > Accepting all memory > Accepting all memory > Accepting all memory > EFI stub: ERROR: exit_boot() failed! > EFI stub: ERROR: efi_main() failed! > StartImage failed: Invalid Parameter > Thanks, > Pankaj 4 calls is telling me that "Accepting all memory" is somehow modifying the memory map each call, but that shouldn't be happening. You've confirmed that the body of the loop is getting skipped after the first call? -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100135): https://edk2.groups.io/g/devel/message/100135 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
I'm rather confused at the moment how our internal testing succeeds
given the premise of the protocol is to use the specified behavior
that the OS must call get_memory_map again if ebs fails with
efi_invalid_parameter, but upstream does not appear to do this.
If you're able to make progress by applying this patch to your linux
build, then we might be back at square one, since the protocol's whole
purpose is to work with older SEV-SNP kernels.
diff --git a/drivers/firmware/efi/libstub/x86-stub.c
b/drivers/firmware/efi/libstub/x86-stub.c
index a0bfd31358ba..795db2315f35 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -747,6 +747,18 @@ static efi_status_t exit_boot(struct boot_params
*boot_params, void *handle)
/* Might as well exit boot services now */
status = efi_exit_boot_services(handle, , exit_boot_func);
+ /*
+* EBS may fail once with INVALID_PARAMETER, which means the
OS must call
+* get_memory_map again and try EBS one more time.
+*/
+ if (status == EFI_INVALID_PARAMETER) {
+ status = allocate_e820(boot_params, , _size);
+ if (status != EFI_SUCCESS)
+ return status;
+
+ status = efi_exit_boot_services(handle, , exit_boot_func);
+ }
+
if (status != EFI_SUCCESS)
return status;
On Mon, Feb 13, 2023 at 9:56 AM Gupta, Pankaj wrote:
>
>
> >> - If no memory is getting accepted at all, should guest boot fail with
> >> below errors?
> >
> > No, the guest should not error. EBS should return success on the
> > second call and permit progress.
> >
> >> - Why unaccepted memory not being set in my setup but works fine for
> >> you? Does it require any other change?
> >>
> >
> > We have an internal fork of EDK2 that we regularly rebase on top of
> > upstream, and we have our own hypervisor called Vanadium. So there's a
> > lot different. We don't have an easy way to test with upstream EDK2
> > and Qemu.
> > A recent import found incompatibilities with measured boot only in
> > SEV-SNP that we have disabled, but that's related to NVdata, which we
> > deal with differently in GCE due to the cloud IVARS service and our
> > allergy to SMM emulation. Should be unrelated.
> >
> > I've looked over our OvmfPkg.patch that we maintain after every rebase
> > and most everything is related to our paravirtualized UEFI package
> > that eschews SMM to talk to Vanadium directly through either shared
> > memory or port I/O depending on whether the guest OS owns cr3 or not.
> >
> > You've added a log for the if != unaccepted memory, but will you log
> > what status the function ultimately returns? And both the MapKey what
> > status CoreTerminateMemoryMap returns in DxeMain.c's
> > CoreExitBootServices? I'm wondering if maybe the EFI stub calling EBS
> > isn't calling GetMemoryMap to update the MapKey after the
> > invalid_param result that this semantics depends on. If the stub is
> > the Linux kernel's own stub, then it should be doing the right
> > thing...
>
> CoreTerminateMemoryMap::MapKey::18033 ^M
> CoreTerminateMemoryMap::Status::2
>
> CoreTerminateMemoryMap::MapKey::18035 ^M
> CoreTerminateMemoryMap::Status::2 ^M
>
> Thanks,
> Pankaj
>
>
>
--
-Dionna Glaze, PhD (she/her)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100121): https://edk2.groups.io/g/devel/message/100121
Mute This Topic: https://groups.io/mt/96534752/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
> > So, no memory is getting accepted. Questions below: > > - If no memory is getting accepted at all, should guest boot fail with >below errors? No, the guest should not error. EBS should return success on the second call and permit progress. > - Why unaccepted memory not being set in my setup but works fine for >you? Does it require any other change? > We have an internal fork of EDK2 that we regularly rebase on top of upstream, and we have our own hypervisor called Vanadium. So there's a lot different. We don't have an easy way to test with upstream EDK2 and Qemu. A recent import found incompatibilities with measured boot only in SEV-SNP that we have disabled, but that's related to NVdata, which we deal with differently in GCE due to the cloud IVARS service and our allergy to SMM emulation. Should be unrelated. I've looked over our OvmfPkg.patch that we maintain after every rebase and most everything is related to our paravirtualized UEFI package that eschews SMM to talk to Vanadium directly through either shared memory or port I/O depending on whether the guest OS owns cr3 or not. You've added a log for the if != unaccepted memory, but will you log what status the function ultimately returns? And both the MapKey what status CoreTerminateMemoryMap returns in DxeMain.c's CoreExitBootServices? I'm wondering if maybe the EFI stub calling EBS isn't calling GetMemoryMap to update the MapKey after the invalid_param result that this semantics depends on. If the stub is the Linux kernel's own stub, then it should be doing the right thing... > Thanks, > Pankaj > > > 9. Return successfully (one would hope) > > > >> Accepting all memory^M > >> Accepting all memory^M > >> EFI stub: ERROR: exit_boot() failed!^M > >> EFI stub: ERROR: efi_main() failed!^M > > > > This now does suggest that EBS is failing twice, since after the > > supposed no-op of the second log, the EFI stub's exit_boot claims > > failure. I can't reproduce this part. Would you try adding a log > > within the acceptance loop inside the if that checks for unaccepted > > memory? I'd be curious if the loop is indeed changing the map again, > > despite my claims at idempotency. > > > -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100119): https://edk2.groups.io/g/devel/message/100119 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
On Fri, Feb 10, 2023 at 5:56 AM Gupta, Pankaj wrote: > > On 2/9/2023 10:27 PM, Dionna Amalie Glaze wrote: > > On Thu, Feb 9, 2023 at 8:52 AM Dionna Amalie Glaze > > wrote: > >> > >>> With this patch I observe an issue where my Linux (6.2.0-rc7) guest > >>> recur to Bootloader menu again. I am testing this with SEV SNP (w/o > >>> UPM). Also, guest don't have lazy memory acceptance support. > >>> > >> > >> Thanks for the report. I'll try to reproduce it on our UEFI and if I'm > >> unable, then we'll discuss next steps. > >> > > > > I don't see this in our test Ubuntu 22.04 image from Canonical. Do you > > Ubuntu 22.04 guest by default run 5.15 kernel? But SEV SNP got > merged in 5.19. I don't know currently how we are handling accepting > the memory on "ExitBootServices" with or w/o guest supporting SNP. > It does, but I used the Qemu kernel injection pathway in Ovmf to run a build of 6.2.0-rc7. Our testing setup doesn't give the user a boot menu to select a kernel, so I wasn't aware that this return to bootmenu could happen. >This looks to me like it is entering the 'accept' path twice, and so > ExitBootServices() is failing twice, resulting in a failed boot. The double log is expected behavior because I didn't add a check for whether accepting all memory would be a no-op. The "Accepting all memory" message occurs twice if the guest does not have support for unaccepted memory following this control flow: 1. EBS 2. [...] Log "Accepting all memory" 3. Loop through all memory spaces 3a. If the memory space is unaccepted, accept it. 3b. Remove the unaccepted memory space. 3c. Add a conventional memory space back with the same range and capabilities. 4. EBS returns an error since the map key is different. 5. OS calls GetMemoryMap to get the updated key. 6. OS calls EBS with the updated key. 7. [...] Log "Accepting all memory" 8. Loop through all memory spaces 8a. There are no unaccepted memory spaces left, so nothing happens. 9. Return successfully (one would hope) >Accepting all memory^M >Accepting all memory^M >EFI stub: ERROR: exit_boot() failed!^M >EFI stub: ERROR: efi_main() failed!^M This now does suggest that EBS is failing twice, since after the supposed no-op of the second log, the EFI stub's exit_boot claims failure. I can't reproduce this part. Would you try adding a log within the acceptance loop inside the if that checks for unaccepted memory? I'd be curious if the loop is indeed changing the map again, despite my claims at idempotency. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100027): https://edk2.groups.io/g/devel/message/100027 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
On Thu, Feb 9, 2023 at 8:52 AM Dionna Amalie Glaze wrote: > > > With this patch I observe an issue where my Linux (6.2.0-rc7) guest > > recur to Bootloader menu again. I am testing this with SEV SNP (w/o > > UPM). Also, guest don't have lazy memory acceptance support. > > > > Thanks for the report. I'll try to reproduce it on our UEFI and if I'm > unable, then we'll discuss next steps. > I don't see this in our test Ubuntu 22.04 image from Canonical. Do you have a boot log you could send me? I'm not sure I understand what you mean by recurring to the Bootloader menu. On ExitBootServices, the memory key will end up changing and the caller will need to call GetMemoryMap and try EBS again, but I don't know why that would send you to a bootmenu. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99930): https://edk2.groups.io/g/devel/message/99930 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
> With this patch I observe an issue where my Linux (6.2.0-rc7) guest > recur to Bootloader menu again. I am testing this with SEV SNP (w/o > UPM). Also, guest don't have lazy memory acceptance support. > Thanks for the report. I'll try to reproduce it on our UEFI and if I'm unable, then we'll discuss next steps. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99911): https://edk2.groups.io/g/devel/message/99911 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg: Fix SevMemoryAcceptance memory attributes
> > > > This change is made given a request from Ard. The CC capability is not > > applied to other system memory ranges that probably should also have > > that capability, given that it's encrypted and accepted. I haven't > > considered carefully where EFI_MEMORY_CPU_CRYPTO should be added to > > conventional memory, given the acceptance happens before DXE > > initializes. Perhaps > > CoreConvertResourceDescriptorHobAttributesToCapabilities? This is more > > of a question to Ard and Thomas. > > > > It's not clear to me whether the CC attribute applies to the host or > the guest. From the guest PoV, there is really no distinction, whereas > on the host, I could imagine that only CC capable memory can be used > for handing out to VMs. > That's a good point. The UEFI spec language is hard to interpret here. Min or Jiewen, do you have more context on the EFI_MEMORY_CPU_CRYPTO attribute? -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99501): https://edk2.groups.io/g/devel/message/99501 Mute This Topic: https://groups.io/mt/96659595/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg: Fix SevMemoryAcceptance memory attributes
> > efi: mem94: [Conventional| | |CC| | | | | | | | | | | ] > range=[0x0001-0x00023fff] (5120MB) > > This does not have the cache capabilities one would expect for system > memory, UC|WC|WT|WB. > > After this change, the same entry becomes > > efi: mem94: [Conventional| | |CC| | | | | | | |WB|WT|WC|UC] > range=[0x0001-0x00023fff] (5120MB) > > This has all the expected attributes. > This change is made given a request from Ard. The CC capability is not applied to other system memory ranges that probably should also have that capability, given that it's encrypted and accepted. I haven't considered carefully where EFI_MEMORY_CPU_CRYPTO should be added to conventional memory, given the acceptance happens before DXE initializes. Perhaps CoreConvertResourceDescriptorHobAttributesToCapabilities? This is more of a question to Ard and Thomas. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99365): https://edk2.groups.io/g/devel/message/99365 Mute This Topic: https://groups.io/mt/96659595/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] OvmfPkg: Fix SevMemoryAcceptance memory attributes
The hard-coded attributes for the re-added memory space should instead
forward the replaced descriptor's capabilities, plus the
EFI_MEMORY_CPU_CRYPTO attribute.
Tested on Linux with efi=debug. Prior to this change, an 8GiB VM running
a kernel without unaccepted memory support shows this entry
efi: mem94: [Conventional| | |CC| | | | | | | | | | | ]
range=[0x0001-0x00023fff] (5120MB)
This does not have the cache capabilities one would expect for system
memory, UC|WC|WT|WB.
After this change, the same entry becomes
efi: mem94: [Conventional| | |CC| | | | | | | |WB|WT|WC|UC]
range=[0x0001-0x00023fff] (5120MB)
This has all the expected attributes.
Cc: Ard Biesheuvel
Cc: Erdem Aktas
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Min Xu
Cc: Tom Lendacky
Cc: Michael Roth
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..59d5ff759f 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -23,6 +23,10 @@
#include
#include
#include
+#include
+
+// Present, initialized, tested bits defined in MdeModulePkg/Core/Dxe/DxeMain.h
+#define EFI_MEMORY_INTERNAL_MASK 0x0700ULL
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
SIGNATURE_32 ('A','M', 'D', 'E'),
@@ -78,6 +82,7 @@ AcceptAllMemory (
UINTNNumEntries;
UINTNIndex;
EFI_STATUS Status;
+ UINT64 Capabilities;
DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
@@ -112,11 +117,14 @@ AcceptAllMemory (
break;
}
+Capabilities = EFI_MEMORY_CPU_CRYPTO | Desc->Capabilities;
Status = gDS->AddMemorySpace (
EfiGcdMemoryTypeSystemMemory,
Desc->BaseAddress,
Desc->Length,
-EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO |
EFI_MEMORY_RP
+// Allocable system memory resource capabilities as masked
+// in
MdeModulePkg/Core/Dxe/Mem/Page.c:PromoteMemoryResource
+Capabilities & ~(EFI_MEMORY_INTERNAL_MASK |
EFI_MEMORY_RUNTIME)
);
if (EFI_ERROR (Status)) {
break;
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99363): https://edk2.groups.io/g/devel/message/99363
Mute This Topic: https://groups.io/mt/96659595/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg: Correct memory type in PrePiDxeCis.h
> BTW, you need to add below guys in the CC list because they're the > reviewers/maintainers of the pkgs. > > MdePkg: > M: Michael D Kinney [mdkinney] > M: Liming Gao [lgao4] > R: Zhiguang Liu [LiuZhiguang001] > > MdeModulePkg > M: Jian J Wang [jwang36] > M: Liming Gao [lgao4] > CC'd, thanks. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99204): https://edk2.groups.io/g/devel/message/99204 Mute This Topic: https://groups.io/mt/96573795/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] MdeModulePkg: Correct memory type in PrePiDxeCis.h
The enumeration in MdePkg/Include/Pi/PiDxeCis.h has a duplicated entry,
so the 8th position in the list doesn't count as index 7. The value
EfiGcdMemoryTypeUnaccepted will have when added before
EfiGcdMemoryTypeMaximum will be 6.
Cc: Min M Xu
Cc: Jiewen Yao
Signed-off-by: Dionna Glaze
---
MdeModulePkg/Include/Pi/PrePiDxeCis.h | 2 +-
MdePkg/Include/Pi/PiDxeCis.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Include/Pi/PrePiDxeCis.h
b/MdeModulePkg/Include/Pi/PrePiDxeCis.h
index 113ac37924..9be71d2618 100644
--- a/MdeModulePkg/Include/Pi/PrePiDxeCis.h
+++ b/MdeModulePkg/Include/Pi/PrePiDxeCis.h
@@ -20,6 +20,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
/// After this memory region is defined in PI spec, it should be a value in
/// EFI_GCD_MEMORY_TYPE in PiDxeCis.h.
///
-#define EFI_GCD_MEMORY_TYPE_UNACCEPTED 7
+#define EFI_GCD_MEMORY_TYPE_UNACCEPTED 6
#endif
diff --git a/MdePkg/Include/Pi/PiDxeCis.h b/MdePkg/Include/Pi/PiDxeCis.h
index 27b219aa3f..bb7fb2c38a 100644
--- a/MdePkg/Include/Pi/PiDxeCis.h
+++ b/MdePkg/Include/Pi/PiDxeCis.h
@@ -64,7 +64,7 @@ typedef enum {
// /// EfiGcdMemoryTypeUnaccepted is defined in PrePiDxeCis.h because it has
not been
// /// defined in PI spec.
// EfiGcdMemoryTypeUnaccepted,
- EfiGcdMemoryTypeMaximum = 8
+ EfiGcdMemoryTypeMaximum = 7
} EFI_GCD_MEMORY_TYPE;
///
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99198): https://edk2.groups.io/g/devel/message/99198
Mute This Topic: https://groups.io/mt/96573795/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Minor bug: off-by-one in mGcdMemoryTypeNames for "Unaccepte" entry
I was debugging some stuff with unaccepted memory, and noticed that AddMemorySpace counted the unaccepted memory as "Unknown" because "Unaccepte" is mGcdMemoryTypeNames[6] instead of mGcdMemoryTypeNames[7]. https://github.com/tianocore/edk2/blob/720c25ab41400f9a3dfd0742da5a6d237991df5b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L108 -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99197): https://edk2.groups.io/g/devel/message/99197 Mute This Topic: https://groups.io/mt/96573507/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v11 4/4] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> Shouldn't this check be inside the if () below? Or are all resources > that start at or above 4 GiB guaranteed to be system memory? > > No need to resend - if needed, I can fix that up when applying. > Ah, yes that sounds right. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99104): https://edk2.groups.io/g/devel/message/99104 Mute This Topic: https://groups.io/mt/96553086/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v11 4/4] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
Instead of eagerly accepting all memory in PEI, only accept memory under
the 4GB address. This allows a loaded image to use the
MEMORY_ACCEPTANCE_PROTOCOL to disable the accept behavior and indicate
that it can interpret the memory type accordingly.
This classification is safe since ExitBootServices will accept and
reclassify the memory as conventional if the disable protocol is not
used.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Signed-off-by: Dionna Glaze
---
OvmfPkg/PlatformPei/AmdSev.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index e4e7b72e67..7d824cc282 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -63,6 +64,10 @@ AmdSevSnpInitialize (
for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB
(Hob)) {
if ((Hob.Raw != NULL) && (GET_HOB_TYPE (Hob) ==
EFI_HOB_TYPE_RESOURCE_DESCRIPTOR)) {
ResourceHob = Hob.ResourceDescriptor;
+ if (ResourceHob->PhysicalStart >= SIZE_4GB) {
+ResourceHob->ResourceType = BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED;
+continue;
+ }
if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
MemEncryptSevSnpPreValidateSystemRam (
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99102): https://edk2.groups.io/g/devel/message/99102
Mute This Topic: https://groups.io/mt/96553086/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v11 3/4] OvmfPkg: Implement AcceptAllUnacceptedMemory in AmdSevDxe
This protocol implementation disables the accept-all-memory behavior
of the BeforeExitBootServices event this driver adds.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 26
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 1 +
2 files changed, 27 insertions(+)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 37d1a3ff55..9d05a16c6e 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
@@ -143,6 +144,21 @@ ResolveUnacceptedMemory (
ASSERT_EFI_ERROR (Status);
}
+STATIC
+EFI_STATUS
+EFIAPI
+AllowUnacceptedMemory (
+ IN OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL *This
+ )
+{
+ mAcceptAllMemoryAtEBS = FALSE;
+ return EFI_SUCCESS;
+}
+
+STATIC
+OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL
+ mMemoryAcceptanceProtocol = { AllowUnacceptedMemory };
+
STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
AmdSevMemoryAccept
};
@@ -292,6 +308,16 @@ AmdSevDxeEntryPoint (
DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for
EventBeforeExitBootServices failed.\n"));
}
+Status = gBS->InstallProtocolInterface (
+,
+,
+EFI_NATIVE_INTERFACE,
+
+);
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Install OvmfSevMemoryAcceptanceProtocol
failed.\n"));
+}
+
//
// If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
// It contains the location for both the Secrets and CPUID page.
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 5b443d45bc..e7c7d526c9 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -49,6 +49,7 @@
[Protocols]
gEdkiiMemoryAcceptProtocolGuid
+ gOvmfSevMemoryAcceptanceProtocolGuid
[Guids]
gConfidentialComputingSevSnpBlobGuid
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99101): https://edk2.groups.io/g/devel/message/99101
Mute This Topic: https://groups.io/mt/96553085/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v11 2/4] MdePkg: Introduce the SevMemoryAcceptance protocol
The default behavior for unaccepted memory in SEV-SNP is to accept all
memory when ExitBootServices is called. An OS loader can use this
protocol to disable this behavior to assume responsibility for memory
acceptance and to affirm that the OS can handle the unaccepted memory
type.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/Include/Protocol/SevMemoryAcceptance.h | 42
OvmfPkg/OvmfPkg.dec| 1 +
2 files changed, 43 insertions(+)
diff --git a/OvmfPkg/Include/Protocol/SevMemoryAcceptance.h
b/OvmfPkg/Include/Protocol/SevMemoryAcceptance.h
new file mode 100644
index 00..c45b499006
--- /dev/null
+++ b/OvmfPkg/Include/Protocol/SevMemoryAcceptance.h
@@ -0,0 +1,42 @@
+/** @file
+ The file provides the protocol that disables the behavior that all memory
+ gets accepted at ExitBootServices(). This protocol is only meant to be called
+ by the OS loader, and not EDK2 itself. The SEV naming is due to the
coincidence
+ that only SEV-SNP needs this protocol, since SEV-SNP kernel support released
+ before kernel support for unaccepted memory. The technology enablement thus
+ does not strictly imply support for the unaccepted memory type.
+
+ Copyright (c) 2023, Google LLC. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SEV_MEMORY_ACCEPTANCE_H_
+#define SEV_MEMORY_ACCEPTANCE_H_
+
+#define OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID \
+ {0xc5a010fe, \
+ 0x38a7, \
+ 0x4531, \
+ {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
+
+typedef struct _OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL
+OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL;
+
+/**
+ @param This A pointer to a OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL.
+**/
+typedef
+ EFI_STATUS
+(EFIAPI *OVMF_SEV_ALLOW_UNACCEPTED_MEMORY)(
+ IN OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL *This
+ );
+
+///
+/// The OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL allows the OS loader to
+/// indicate to EDK2 that ExitBootServices should not accept all memory.
+///
+struct _OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL {
+ OVMF_SEV_ALLOW_UNACCEPTED_MEMORYAllowUnacceptedMemory;
+};
+
+#endif
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 1b521f2604..a22eb246c6 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -184,6 +184,7 @@
gEfiLegacyInterruptProtocolGuid = {0x31ce593d, 0x108a, 0x485d, {0xad,
0xb2, 0x78, 0xf2, 0x1f, 0x29, 0x66, 0xbe}}
gEfiVgaMiniPortProtocolGuid = {0xc7735a2f, 0x88f5, 0x4882, {0xae,
0x63, 0xfa, 0xac, 0x8c, 0x8b, 0x86, 0xb3}}
gOvmfLoadedX86LinuxKernelProtocolGuid = {0xa3edc05d, 0xb618, 0x4ff6, {0x95,
0x52, 0x76, 0xd7, 0x88, 0x63, 0x43, 0xc8}}
+ gOvmfSevMemoryAcceptanceProtocolGuid = {0xc5a010fe, 0x38a7, 0x4531, {0x8a,
0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
gQemuAcpiTableNotifyProtocolGuid = {0x928939b2, 0x4235, 0x462f, {0x95,
0x80, 0xf6, 0xa2, 0xb2, 0xc2, 0x1a, 0x4f}}
gEfiMpInitLibMpDepProtocolGuid= {0xbb00a5ca, 0x8ce, 0x462f, {0xa5,
0x37, 0x43, 0xc7, 0x4a, 0x82, 0x5c, 0xa4}}
gEfiMpInitLibUpDepProtocolGuid= {0xa9e7cef1, 0x5682, 0x42cc, {0xb1,
0x23, 0x99, 0x30, 0x97, 0x3f, 0x4a, 0x9f}}
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99100): https://edk2.groups.io/g/devel/message/99100
Mute This Topic: https://groups.io/mt/96553083/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v11 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
The added behavior is to accept all unaccepted memory at
ExitBootServices if the behavior is not disabled. This allows safe
upgrades for OS loaders to affirm their support for the unaccepted
memory type.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 97
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 1 +
2 files changed, 98 insertions(+)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index f7600c3c81..37d1a3ff55 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
@@ -34,6 +35,10 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
mSnpBootDxeTable = {
STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
+
+STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
+
#define IS_ALIGNED(x, y) x) & ((y) - 1)) == 0))
STATIC
@@ -62,6 +67,82 @@ AmdSevMemoryAccept (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+AcceptAllMemory (
+ VOID
+ )
+{
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
+ UINTNNumEntries;
+ UINTNIndex;
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
+
+ /*
+ * Get a copy of the memory space map to iterate over while
+ * changing the map.
+ */
+ Status = gDS->GetMemorySpaceMap (, );
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ for (Index = 0; Index < NumEntries; Index++) {
+CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc;
+
+Desc = [Index];
+if (Desc->GcdMemoryType != EFI_GCD_MEMORY_TYPE_UNACCEPTED) {
+ continue;
+}
+
+Status = AmdSevMemoryAccept (
+ NULL,
+ Desc->BaseAddress,
+ Desc->Length
+ );
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->RemoveMemorySpace (Desc->BaseAddress, Desc->Length);
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->AddMemorySpace (
+EfiGcdMemoryTypeSystemMemory,
+Desc->BaseAddress,
+Desc->Length,
+EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO |
EFI_MEMORY_RP
+);
+if (EFI_ERROR (Status)) {
+ break;
+}
+ }
+
+ gBS->FreePool (AllDescMap);
+ return Status;
+}
+
+VOID
+EFIAPI
+ResolveUnacceptedMemory (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+
+ if (!mAcceptAllMemoryAtEBS) {
+return;
+ }
+
+ Status = AcceptAllMemory ();
+ ASSERT_EFI_ERROR (Status);
+}
+
STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
AmdSevMemoryAccept
};
@@ -195,6 +276,22 @@ AmdSevDxeEntryPoint (
);
ASSERT_EFI_ERROR (Status);
+// SEV-SNP support does not automatically imply unaccepted memory support,
+// so make ExitBootServices accept all unaccepted memory if support is
+// not communicated.
+Status = gBS->CreateEventEx (
+EVT_NOTIFY_SIGNAL,
+TPL_CALLBACK,
+ResolveUnacceptedMemory,
+NULL,
+,
+
+);
+
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for
EventBeforeExitBootServices failed.\n"));
+}
+
//
// If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
// It contains the location for both the Secrets and CPUID page.
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index cd1b686c53..5b443d45bc 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -52,6 +52,7 @@
[Guids]
gConfidentialComputingSevSnpBlobGuid
+ gEfiEventBeforeExitBootServicesGuid
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99099): https://edk2.groups.io/g/devel/message/99099
Mute This Topic: https://groups.io/mt/96553081/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v11 0/4] Add safe unaccepted memory behavior
We make eager memory acceptance the default behavior at ExitBootServices for SEV-SNP machines by using the standard-enforced behavior that if the call returns an error code, then the map key is incorrect and the caller must re-call GetMemoryMap to ensure the contents are correct. Eager memory acceptance is implemented by using the UEFI v2.9-added EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES to check a support condition before changing all unaccepted memory type regions to conventional memory after first using the MemoryAccept protocol to accept all memory in each region. This update to the memory map only happens once, since there are no extra unaccepted memory regions to change on the forced second call to ExitBootServices. The new acceptance logic is required only for SEV-SNP since it is the only memory-accepting virtualization technology with kernel support live without unaccepted memory support. To allow the OS loader to prevent the eager acceptance, and thus pass the before-mentioned "support condition", we add a new protocol, OvmfSevMemoryAcceptance. This protocol has one interface, AllowUnacceptedMemory(). The OS loader can inform the UEFI that it supports the unaccepted memory type and accepts the responsibility to accept it. The OvmfSevMemoryAcceptance protocol is necessary for safe rollout of the unaccepted memory type in SEV-SNP-enabled kernels, given the gradual update of guest OS kernels. All images that support unaccepted memory must now locate and call this new OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL and call the AllowUnacceptedMemory function. Changes since v10: - AmdSevDxe called AcceptMemory directly without locating the MemoryAccept protocol. - The protocol is no longer a candidate for standardization and has moved to OvmfPkg/Include/Protocol. Changes since v9: - Renamed protocol to SevMemoryAcceptance. - Removed CocoDxe and moved all contained code to AmdSevDxe. - Renamed protocol header file to reference the bugzilla number. Changes since v8: - First 3 patches removed since they were submitted separately. - Later patches rebased on edk2/master and modified to work with the current locations and namings of the unaccepted memory constants. Changes since v7: - Rebased onto lazy accept v4 patch series, so memory accept protocol has the EDKII prefix, and the unaccepted memory type has the BZ3937 prefix. - Removed a bad #include to a header removed in v7. - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL as per the discussion on the buganizer issue. - Uncrustify formatting Changes since v6: - Added implementation of EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. - Changed callback protocol of v5 to instead use the standardized event group for before_exit_boot_services. Changes since v5: - Generic callback protocol moved to MdeModulePkg - Removed use of EFI_WARN_STALE_DATA and added comment that the callback should only return EFI_SUCCESS or EFI_INVALID_PARAMETER. - Removed errant log statement and fixed formatting. Changes since v4: - Commit message wording - Replaced direct change to DxeMain with a more generic callback protocol. - Implemented the direct change as an instance of the callback protocol from a new CocoDxe driver. - Replaced "enable" protocol with a "disable" protocol, since the name was confusing. The AcceptAllUnacceptedMemory protocol directly names the behavior that is disabling. Changes since v3: - "DxeMain accepts all memory" patch split into 3 to make each patch affect only one package at a time. Changes since v2: - Removed the redundant memory accept interface and added the accept behavior to the DXE implementation of MemEncryptSevSnpPreValidateSystemRam. - Fixed missing #include in >=4GB patch. Changes since v1: - Added a patch to classify SEV-SNP memory above 4GB unaccepted. - Fixed style problems in EfiMemoryAcceptProtocol implementation. Cc: Ard Biescheuvel Cc: "Min M. Xu" Cc: Gerd Hoffmann Cc: James Bottomley Cc: Tom Lendacky Cc: Jiewen Yao Cc: Erdem Aktas Cc: Andrew Fish Cc: "Michael D. Kinney" Signed-off-by: Dionna Glaze Dionna Glaze (4): OvmfPkg: Add memory acceptance event in AmdSevDxe MdePkg: Introduce the SevMemoryAcceptance protocol OvmfPkg: Implement AcceptAllUnacceptedMemory in AmdSevDxe OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted OvmfPkg/AmdSevDxe/AmdSevDxe.c | 123 OvmfPkg/AmdSevDxe/AmdSevDxe.inf| 2 + OvmfPkg/Include/Protocol/SevMemoryAcceptance.h | 42 +++ OvmfPkg/OvmfPkg.dec| 1 + OvmfPkg/PlatformPei/AmdSev.c | 5 + 5 files changed, 173 insertions(+) create mode 100644 OvmfPkg/Include/Protocol/SevMemoryAcceptance.h -- 2.39.1.456.gfc5497dd1b-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99098): https://edk2.groups.io/g/devel/message/99098 Mute This Topic:
Re: [edk2-devel] [PATCH v10 2/4] MdePkg: Introduce the SevMemoryAcceptance protocol
> As Gerd and I discussed before, this protocol should be in OvmfPkg. > Please move to > https://github.com/tianocore/edk2/tree/master/OvmfPkg/Include/Protocol > Ah, I misinterpreted your response to Gerd's message. v11 will have it moved. The CI seems to think I've redefined the protocol struct type or failed to use the typedef for declarations, but I don't see how that would be. Is this a false positive that I can ignore for the next iteration? https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=78271=results -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99086): https://edk2.groups.io/g/devel/message/99086 Mute This Topic: https://groups.io/mt/96534753/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
> > This driver is now both the producer and consumer of > gEdkiiMemoryAcceptProtocolGuid. > > Are there cases where the protocol we locate here could be different > from the one installed by this driver? If not, we can simplify this, > and just call AmdSevMemoryAccept() directly. > Ah right. There should not be another implementation. I can make that change. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99085): https://edk2.groups.io/g/devel/message/99085 Mute This Topic: https://groups.io/mt/96534752/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v10 4/4] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
Instead of eagerly accepting all memory in PEI, only accept memory under
the 4GB address. This allows a loaded image to use the
MEMORY_ACCEPTANCE_PROTOCOL to disable the accept behavior and indicate
that it can interpret the memory type accordingly.
This classification is safe since ExitBootServices will accept and
reclassify the memory as conventional if the disable protocol is not
used.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Signed-off-by: Dionna Glaze
---
OvmfPkg/PlatformPei/AmdSev.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index e4e7b72e67..7d824cc282 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -63,6 +64,10 @@ AmdSevSnpInitialize (
for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB
(Hob)) {
if ((Hob.Raw != NULL) && (GET_HOB_TYPE (Hob) ==
EFI_HOB_TYPE_RESOURCE_DESCRIPTOR)) {
ResourceHob = Hob.ResourceDescriptor;
+ if (ResourceHob->PhysicalStart >= SIZE_4GB) {
+ResourceHob->ResourceType = BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED;
+continue;
+ }
if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
MemEncryptSevSnpPreValidateSystemRam (
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99053): https://edk2.groups.io/g/devel/message/99053
Mute This Topic: https://groups.io/mt/96534757/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v10 3/4] OvmfPkg: Implement AcceptAllUnacceptedMemory in AmdSevDxe
This protocol implementation disables the accept-all-memory behavior
of the BeforeExitBootServices event this driver adds.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 26
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 1 +
2 files changed, 27 insertions(+)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 5eec76fea2..e98867afac 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
@@ -155,6 +156,21 @@ ResolveUnacceptedMemory (
ASSERT_EFI_ERROR (Status);
}
+STATIC
+EFI_STATUS
+EFIAPI
+AllowUnacceptedMemory (
+ IN BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL *This
+ )
+{
+ mAcceptAllMemoryAtEBS = FALSE;
+ return EFI_SUCCESS;
+}
+
+STATIC
+BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL
+ mMemoryAcceptanceProtocol = { AllowUnacceptedMemory };
+
STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
AmdSevMemoryAccept
};
@@ -304,6 +320,16 @@ AmdSevDxeEntryPoint (
DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for
EventBeforeExitBootServices failed.\n"));
}
+Status = gBS->InstallProtocolInterface (
+,
+,
+EFI_NATIVE_INTERFACE,
+
+);
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Install Bz3987SevMemoryAcceptanceProtocol
failed.\n"));
+}
+
//
// If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
// It contains the location for both the Secrets and CPUID page.
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 5b443d45bc..1e14e4e0ab 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -49,6 +49,7 @@
[Protocols]
gEdkiiMemoryAcceptProtocolGuid
+ gBz3987SevMemoryAcceptanceProtocolGuid
[Guids]
gConfidentialComputingSevSnpBlobGuid
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99052): https://edk2.groups.io/g/devel/message/99052
Mute This Topic: https://groups.io/mt/96534756/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v10 2/4] MdePkg: Introduce the SevMemoryAcceptance protocol
The default behavior for unaccepted memory in SEV-SNP is to accept all
memory when ExitBootServices is called. An OS loader can use this
protocol to disable this behavior to assume responsibility for memory
acceptance and to affirm that the OS can handle the unaccepted memory
type.
This is a candidate for standardization.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
MdePkg/Include/Protocol/Bz3987SevMemoryAcceptance.h | 44
MdePkg/MdePkg.dec | 3 ++
2 files changed, 47 insertions(+)
diff --git a/MdePkg/Include/Protocol/Bz3987SevMemoryAcceptance.h
b/MdePkg/Include/Protocol/Bz3987SevMemoryAcceptance.h
new file mode 100644
index 00..c3691e1c93
--- /dev/null
+++ b/MdePkg/Include/Protocol/Bz3987SevMemoryAcceptance.h
@@ -0,0 +1,44 @@
+/** @file
+ The file provides the protocol that disables the behavior that all memory
+ gets accepted at ExitBootServices(). This protocol is only meant to be called
+ by the OS loader, and not EDK2 itself. The SEV naming is due to the
coincidence
+ that only SEV-SNP needs this protocol, since SEV-SNP kernel support released
+ before kernel support for unaccepted memory. The technology enablement thus
+ does not strictly imply support for the unaccepted memory type.
+
+ Copyright (c) 2023, Google LLC. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SEV_MEMORY_ACCEPTANCE_H_
+#define SEV_MEMORY_ACCEPTANCE_H_
+
+#define BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID \
+ {0xc5a010fe, \
+ 0x38a7, \
+ 0x4531, \
+ {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
+
+typedef struct _BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL \
+ BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL;
+
+/**
+ @param This A pointer to a BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL.
+**/
+typedef
+ EFI_STATUS
+(EFIAPI *BZ3987_SEV_ALLOW_UNACCEPTED_MEMORY)(
+ IN BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL *This
+ );
+
+///
+/// The BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL allows the OS loader to
+/// indicate to EDK2 that ExitBootServices should not accept all memory.
+///
+struct _BZ3987_SEV_MEMORY_ACCEPTANCE_PROTOCOL {
+ BZ3987_SEV_ALLOW_UNACCEPTED_MEMORYAllowUnacceptedMemory;
+};
+
+extern EFI_GUID gBz3987SevMemoryAcceptanceProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3d08f20d15..b82d6e46a4 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1031,6 +1031,9 @@
gEfiPeiDelayedDispatchPpiGuid = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e,
0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}
[Protocols]
+ ## Include/Protocol/Bz3987SevMemoryAcceptance.h
+ gBz3987SevMemoryAcceptanceProtocolGuid = { 0xc5a010fe, 0x38a7, 0x4531,
{0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49 }}
+
## Include/Protocol/MemoryAccept.h
gEdkiiMemoryAcceptProtocolGuid = { 0x38c74800, 0x5590, 0x4db4, { 0xa0, 0xf3,
0x67, 0x5d, 0x9b, 0x8e, 0x80, 0x26 }}
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99051): https://edk2.groups.io/g/devel/message/99051
Mute This Topic: https://groups.io/mt/96534753/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe
The added behavior is to accept all unaccepted memory at
ExitBootServices if the behavior is not disabled. This allows safe
upgrades for OS loaders to affirm their support for the unaccepted
memory type.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 109
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 1 +
2 files changed, 110 insertions(+)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index f7600c3c81..5eec76fea2 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
@@ -34,6 +35,10 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
mSnpBootDxeTable = {
STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
+
+STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
+
#define IS_ALIGNED(x, y) x) & ((y) - 1)) == 0))
STATIC
@@ -62,6 +67,94 @@ AmdSevMemoryAccept (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+AcceptAllMemory (
+ IN EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory
+ )
+{
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
+ UINTNNumEntries;
+ UINTNIndex;
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
+
+ /*
+ * Get a copy of the memory space map to iterate over while
+ * changing the map.
+ */
+ Status = gDS->GetMemorySpaceMap (, );
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ for (Index = 0; Index < NumEntries; Index++) {
+CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc;
+
+Desc = [Index];
+if (Desc->GcdMemoryType != EFI_GCD_MEMORY_TYPE_UNACCEPTED) {
+ continue;
+}
+
+Status = AcceptMemory->AcceptMemory (
+ AcceptMemory,
+ Desc->BaseAddress,
+ Desc->Length
+ );
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->RemoveMemorySpace (Desc->BaseAddress, Desc->Length);
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->AddMemorySpace (
+EfiGcdMemoryTypeSystemMemory,
+Desc->BaseAddress,
+Desc->Length,
+EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO |
EFI_MEMORY_RP
+);
+if (EFI_ERROR (Status)) {
+ break;
+}
+ }
+
+ gBS->FreePool (AllDescMap);
+ return Status;
+}
+
+VOID
+EFIAPI
+ResolveUnacceptedMemory (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory;
+ EFI_STATUSStatus;
+
+ if (!mAcceptAllMemoryAtEBS) {
+return;
+ }
+
+ Status = gBS->LocateProtocol (
+ ,
+ NULL,
+ (VOID **)
+ );
+ if (Status == EFI_NOT_FOUND) {
+return;
+ }
+
+ ASSERT_EFI_ERROR (Status);
+
+ Status = AcceptAllMemory (AcceptMemory);
+ ASSERT_EFI_ERROR (Status);
+}
+
STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
AmdSevMemoryAccept
};
@@ -195,6 +288,22 @@ AmdSevDxeEntryPoint (
);
ASSERT_EFI_ERROR (Status);
+// SEV-SNP support does not automatically imply unaccepted memory support,
+// so make ExitBootServices accept all unaccepted memory if support is
+// not communicated.
+Status = gBS->CreateEventEx (
+EVT_NOTIFY_SIGNAL,
+TPL_CALLBACK,
+ResolveUnacceptedMemory,
+NULL,
+,
+
+);
+
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for
EventBeforeExitBootServices failed.\n"));
+}
+
//
// If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
// It contains the location for both the Secrets and CPUID page.
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index cd1b686c53..5b443d45bc 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -52,6 +52,7 @@
[Guids]
gConfidentialComputingSevSnpBlobGuid
+ gEfiEventBeforeExitBootServicesGuid
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99050): https://edk2.groups.io/g/devel/message/99050
Mute This Topic: https://groups.io/mt/96534752/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
[edk2-devel] [PATCH v10 0/4] Add safe unaccepted memory behavior
We make eager memory acceptance the default behavior at ExitBootServices for SEV-SNP machines by using the standard-enforced behavior that if the call returns an error code, then the map key is incorrect and the caller must re-call GetMemoryMap to ensure the contents are correct. Eager memory acceptance is implemented by using the UEFI v2.9-added EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES to check a support condition before changing all unaccepted memory type regions to conventional memory after first using the MemoryAccept protocol to accept all memory in each region. This update to the memory map only happens once, since there are no extra unaccepted memory regions to change on the forced second call to ExitBootServices. The new acceptance logic is technology-agnostic and usable across TEE technologies, so this patch series introduces a Confidenial Compute DXE driver called CocoDxe. To allow the OS loader to prevent the eager acceptance, and thus pass the before-mentioned "support condition", we add a new protocol, up for standardization, SevMemoryAcceptance. This protocol has one interface, AllowUnacceptedMemory(). The OS loader can inform the UEFI that it supports the unaccepted memory type and accepts the responsibility to accept it. The SevMemoryAcceptance protocol is necessary for safe rollout of the unaccepted memory type in SEV-SNP-enabled kernels, given the gradual update of guest OS kernels. OVMF cannot rely on the following implication MemEncryptSevIsEnabled () implies unaccepted memory is supported by the guest This implication does not hold for e.g., upstream Linux. All images that support unaccepted memory must now locate and call this new BZ3987_SEV_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the AllowUnacceptedMemory function. Changes since v9: - Renamed protocol to SevMemoryAcceptance. - Removed CocoDxe and moved all contained code to AmdSevDxe. - Renamed protocol header file to reference the bugzilla number. Changes since v8: - First 3 patches removed since they were submitted separately. - Later patches rebased on edk2/master and modified to work with the current locations and namings of the unaccepted memory constants. Changes since v7: - Rebased onto lazy accept v4 patch series, so memory accept protocol has the EDKII prefix, and the unaccepted memory type has the BZ3937 prefix. - Removed a bad #include to a header removed in v7. - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL as per the discussion on the buganizer issue. - Uncrustify formatting Changes since v6: - Added implementation of EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. - Changed callback protocol of v5 to instead use the standardized event group for before_exit_boot_services. Changes since v5: - Generic callback protocol moved to MdeModulePkg - Removed use of EFI_WARN_STALE_DATA and added comment that the callback should only return EFI_SUCCESS or EFI_INVALID_PARAMETER. - Removed errant log statement and fixed formatting. Changes since v4: - Commit message wording - Replaced direct change to DxeMain with a more generic callback protocol. - Implemented the direct change as an instance of the callback protocol from a new CocoDxe driver. - Replaced "enable" protocol with a "disable" protocol, since the name was confusing. The AcceptAllUnacceptedMemory protocol directly names the behavior that is disabling. Changes since v3: - "DxeMain accepts all memory" patch split into 3 to make each patch affect only one package at a time. Changes since v2: - Removed the redundant memory accept interface and added the accept behavior to the DXE implementation of MemEncryptSevSnpPreValidateSystemRam. - Fixed missing #include in >=4GB patch. Changes since v1: - Added a patch to classify SEV-SNP memory above 4GB unaccepted. - Fixed style problems in EfiMemoryAcceptProtocol implementation. Cc: Ard Biescheuvel Cc: "Min M. Xu" Cc: Gerd Hoffmann Cc: James Bottomley Cc: Tom Lendacky Cc: Jiewen Yao Cc: Erdem Aktas Cc: Andrew Fish Cc: "Michael D. Kinney" Signed-off-by: Dionna Glaze Dionna Glaze (4): OvmfPkg: Add memory acceptance event in AmdSevDxe MdePkg: Introduce the SevMemoryAcceptance protocol OvmfPkg: Implement AcceptAllUnacceptedMemory in AmdSevDxe OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted MdePkg/Include/Protocol/Bz3987SevMemoryAcceptance.h | 44 +++ MdePkg/MdePkg.dec | 3 + OvmfPkg/AmdSevDxe/AmdSevDxe.c | 135 OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 2 + OvmfPkg/PlatformPei/AmdSev.c| 5 + 5 files changed, 189 insertions(+) create mode 100644 MdePkg/Include/Protocol/Bz3987SevMemoryAcceptance.h -- 2.39.1.456.gfc5497dd1b-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99049): https://edk2.groups.io/g/devel/message/99049
Re: [edk2-devel] [PATCH v2] x86/efi: Safely enable unaccepted memory in UEFI
> > Why do you call boot with a bootloader a legacy feature? > Gerd answered this about EBS called from the bootloader. > > they'll only get a safe view of the memory map. I don't think it's right > > to choose unsafe behavior for a legacy setup. > > Present memory map with unaccepted memory to OS that doesn't about it is > perfectly safe. This portion of the memory will be ignored. It is "feature > not [yet] implemented" case. > SNP guest support is already in Linux, and it gets a full view of the memory given to the VM. If the firmware ever introduces unaccepted memory, then the kernel's behavior is retroactively broken without the "accept all if AllowUnacceptedMemory() not called" behavior of the UEFI. The memory that existed before becomes ignored. This is not the right approach IMO. > > > This patch adds complexity, breaks what works and the only upside will > > > turn into a dead weight soon. > > > > > > There's alternative to add option to instruct firmware to accept all > > > memory from VMM side. It will serve legacy OS that doesn't know about > > > unaccepted memory and it is also can be use by latency-sensitive users > > > later on (analog of qemu -mem-prealloc). > > > > > > > This means that users of a distro that has not enabled unaccepted > > memory support cannot simply start a VM with the usual command, but > > instead have to know a baroque extra flag to get access to all the > > memory that they configured the machine (and for a CSP customer, paid > > for). That's not a good experience. > > New features require enabling. It is not something new. > What I'm saying is that you're suggesting a feature _dis_abling requirement, which is an antipattern. Any SNP user right now would need to add a "don't use an unimplemented feature" flag to get access to all its memory again. > > With GCE at least, you can't (shouldn't) associate the boot feature > > flag with a disk image because disks are mutable. If a customer > > upgrades their kernel after initially starting their VM, they can't > > remove the flag due to the way image annotations work. > > I guess a new VM has to be created, right? Doesn't sound like a big deal > to me. > Usually it's not, but the retroactive need to create a new VM once the firmware adds UEFI v2.9 support with unaccepted memory is a big deal. > The old will not break with upgraded kernel. Just not get benefit of the > feature. > A user buys access to a high memory VM: 768GiB. They then shut down and bring it back up on a new firmware that uses unaccepted memory. That VM goes from 785GiB free memory to 3GiB free memory at boot. This is because all memory above 4GiB (and nothing there for the 3-4GiB MMIO hole) would be the unknown unaccepted memory type. We need the accept-all-if-support-not-acked semantics with the protocol. > > All of this headache goes away by adopting a small patch to the kernel > > that calls a 0-ary protocol interface and keeping safe acceptance > > behavior in the firmware. I think Gerd is right here that we should > > treat it as a transition feature that we can remove later. > > Removing a feature is harder than adding one. How do you define that > "later" has come? > Gerd's response of after 6.1-lts EOL is reasonable to me. At the same time, both SEV-SNP and TDX's Kconfig would need to strictly require unaccepted memory. The semantics of the UEFI under the proposed protocol is allowed to change the default behavior when the protocol is not exposed to the OS. The default would then be to always introduce unaccepted memory for TDX and SEV-SNP guests. To Gerd's point, removing "first in edk2, later in linux too" I think is backwards. We need all users of the protocol to agree that SEV-SNP and TDX strictly imply unaccepted memory support. Only then can we remove the protocol from EDK2. > Anyway, I think we walk in a circle. I consider it a misfeature. If you > want still go this path, please add my > > Nacked-by: Kirill A. Shutemov > Thanks for your time discussing. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98715): https://edk2.groups.io/g/devel/message/98715 Mute This Topic: https://groups.io/mt/96256524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] x86/efi: Safely enable unaccepted memory in UEFI
> > I still don't understand why we need to support every imaginable > > combination of firmware, bootloader and OS. Unaccepted memory only > > exists on a special kind of virtual machine, which provides very > > little added value unless you opt into the security and attestation > > features, which are all heavily based on firmware protocols. So why > > should care about a EFI-aware bootloader calling ExitBootServices() > > and subsequently doing a legacy boot of Linux on such systems? > > Why break what works? Some users want it. > The users that want legacy boot features will not be broken, they'll only get a safe view of the memory map. I don't think it's right to choose unsafe behavior for a legacy setup. > This patch adds complexity, breaks what works and the only upside will > turn into a dead weight soon. > > There's alternative to add option to instruct firmware to accept all > memory from VMM side. It will serve legacy OS that doesn't know about > unaccepted memory and it is also can be use by latency-sensitive users > later on (analog of qemu -mem-prealloc). > This means that users of a distro that has not enabled unaccepted memory support cannot simply start a VM with the usual command, but instead have to know a baroque extra flag to get access to all the memory that they configured the machine (and for a CSP customer, paid for). That's not a good experience. With GCE at least, you can't (shouldn't) associate the boot feature flag with a disk image because disks are mutable. If a customer upgrades their kernel after initially starting their VM, they can't remove the flag due to the way image annotations work. All of this headache goes away by adopting a small patch to the kernel that calls a 0-ary protocol interface and keeping safe acceptance behavior in the firmware. I think Gerd is right here that we should treat it as a transition feature that we can remove later. > -- > Kiryl Shutsemau / Kirill A. Shutemov -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98600): https://edk2.groups.io/g/devel/message/98600 Mute This Topic: https://groups.io/mt/96256524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2] x86/efi: Safely enable unaccepted memory in UEFI
This patch depends on Kirill A. Shutemov's series
[PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory
The UEFI v2.9 specification includes a new memory type to be used in
environments where the OS must accept memory that is provided from its
host. Before the introduction of this memory type, all memory was
accepted eagerly in the firmware. In order for the firmware to safely
stop accepting memory on the OS's behalf, the OS must affirmatively
indicate support to the firmware.
Enabling unaccepted memory requires calling a 0-argument enablement
protocol before ExitBootServices. This call is only made if the kernel
is compiled with UNACCEPTED_MEMORY=y
The naming of the protocol guid is dependent on the standardization of
the protocol, which is being discussed. Acceptance is contingent on
the kernel community's approval.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Cc: "Kirill A. Shutemov"
Cc: Dave Hansen
Signed-off-by: Dionna Glaze
---
drivers/firmware/efi/libstub/x86-stub.c | 37 +
include/linux/efi.h | 1 +
2 files changed, 38 insertions(+)
diff --git a/drivers/firmware/efi/libstub/x86-stub.c
b/drivers/firmware/efi/libstub/x86-stub.c
index a0bfd31358ba..abf31e5ade55 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -26,6 +26,17 @@ const efi_dxe_services_table_t *efi_dxe_table;
u32 image_offset __section(".data");
static efi_loaded_image_t *image = NULL;
+typedef union memory_acceptance_protocol memory_acceptance_protocol_t;
+union memory_acceptance_protocol {
+ struct {
+ efi_status_t (__efiapi *allow_unaccepted_memory)(
+ memory_acceptance_protocol_t *);
+ };
+ struct {
+ u32 allow_unaccepted_memory;
+ } mixed_mode;
+};
+
static efi_status_t
preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom
**__rom)
{
@@ -310,6 +321,30 @@ setup_memory_protection(unsigned long image_base, unsigned
long image_size)
#endif
}
+
+static void setup_unaccepted_memory(void)
+{
+ efi_guid_t mem_acceptance_proto = EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUID;
+ memory_acceptance_protocol_t *proto;
+ efi_status_t status;
+
+ if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
+ return;
+
+ /*
+* Enable unaccepted memory before calling exit boot services in order
+* for the UEFI to not accept all memory on EBS.
+*/
+ status = efi_bs_call(locate_protocol, _acceptance_proto, NULL,
+(void **));
+ if (status != EFI_SUCCESS)
+ return;
+
+ status = efi_call_proto(proto, allow_unaccepted_memory);
+ if (status != EFI_SUCCESS)
+ efi_err("Memory acceptance protocol failed\n");
+}
+
static const efi_char16_t apple[] = L"Apple";
static void setup_quirks(struct boot_params *boot_params,
@@ -899,6 +934,8 @@ asmlinkage unsigned long efi_main(efi_handle_t handle,
setup_quirks(boot_params, bzimage_addr, buffer_end - buffer_start);
+ setup_unaccepted_memory();
+
status = exit_boot(boot_params, handle);
if (status != EFI_SUCCESS) {
efi_err("exit_boot() failed!\n");
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 4b27519143f5..bfc0e4f2aba5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -391,6 +391,7 @@ void efi_native_runtime_setup(void);
#define EFI_RT_PROPERTIES_TABLE_GUID EFI_GUID(0xeb66918a, 0x7eef,
0x402a, 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9)
#define EFI_DXE_SERVICES_TABLE_GUIDEFI_GUID(0x05ad34ba, 0x6f02,
0x4214, 0x95, 0x2e, 0x4d, 0xa0, 0x39, 0x8e, 0x2b, 0xb9)
#define EFI_SMBIOS_PROTOCOL_GUID EFI_GUID(0x03583ff6, 0xcb36,
0x4940, 0x94, 0x7e, 0xb9, 0xb3, 0x9f, 0x4a, 0xfa, 0xf7)
+#define EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUIDEFI_GUID(0xc5a010fe, 0x38a7,
0x4531, 0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49)
#define EFI_IMAGE_SECURITY_DATABASE_GUID EFI_GUID(0xd719b2cb, 0x3d3a,
0x4596, 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
#define EFI_SHIM_LOCK_GUID EFI_GUID(0x605dab50, 0xe046,
0x4300, 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
--
2.39.0.314.g84b9a713c41-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98506): https://edk2.groups.io/g/devel/message/98506
Mute This Topic: https://groups.io/mt/96256524/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
> Kirill's _initial_ patch does #1. If anyone desperately wants #2, they > have mechanisms available to make a kernel with only #1 approximate #2. > A user on that kernel could allocate and memset()ing a bunch of memory. > Or, they could have a firmware stub accept the memory before booting the > real kernel. > > It also doesn't rule out having a runtime knob or a boot parameter > implement #2. It's not a lot of code, but it involves new ABI. > The new ABI is the safety problem. Without the new code, you have firmware that makes all but 3 GiB of memory unusable because it's classified as an unknown type. > However, *NONE* of this points me in the direction of saying that we > should have an OS/firmware protocol to negotiate whether the firmware or > OS does page acceptance other than the existing UEFI memory map bit. We know of distributions that are going to release SEV-SNP support without unaccepted memory support, and in so doing, tie the firmware's hands in trying to maintain safe behavior through a required default behavior of accepting all memory without explicit information from the OS in the form of this protocol. TDX support may also get released this way due to unexpected requirements from the linux community that push back Kirill's patches. They still haven't been thoroughly reviewed by a memory system expert, IIRC. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98503): https://edk2.groups.io/g/devel/message/98503 Mute This Topic: https://groups.io/mt/96236145/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] x86/efi: Safely enable unaccepted memory in UEFI
This patch depends on Kirill A. Shutemov's series
[PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory
The UEFI v2.9 specification includes a new memory type to be used in
environments where the OS must accept memory that is provided from its
host. Before the introduction of this memory type, all memory was
accepted eagerly in the firmware. In order for the firmware to safely
stop accepting memory on the OS's behalf, the OS must affirmatively
indicate support to the firmware.
Enabling unaccepted memory requires calling a 0-argument enablement
protocol before ExitBootServices. This call is only made if the kernel
is compiled with UNACCEPTED_MEMORY=y
The naming of the protocol guid is dependent on the standardization of
the protocol, which is being discussed. Acceptance is contingent on
the kernel community's approval.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Cc: "Kirill A. Shutemov"
Cc: Dave Hansen
Signed-off-by: Dionna Glaze
---
drivers/firmware/efi/libstub/x86-stub.c | 36 +
include/linux/efi.h | 1 +
2 files changed, 37 insertions(+)
diff --git a/drivers/firmware/efi/libstub/x86-stub.c
b/drivers/firmware/efi/libstub/x86-stub.c
index a0bfd31358ba..5e9ebfbb49e6 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -26,6 +26,17 @@ const efi_dxe_services_table_t *efi_dxe_table;
u32 image_offset __section(".data");
static efi_loaded_image_t *image = NULL;
+union memory_acceptance_protocol {
+ struct {
+ efi_status_t (__efiapi *allow_unaccepted_memory)(
+ union memory_acceptance_protocol *);
+ };
+ struct {
+ u32 allow_unaccepted_memory;
+ } mixed_mode;
+};
+typedef union memory_acceptance_protocol memory_acceptance_protocol_t;
+
static efi_status_t
preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom
**__rom)
{
@@ -310,6 +321,29 @@ setup_memory_protection(unsigned long image_base, unsigned
long image_size)
#endif
}
+
+static void setup_unaccepted_memory(void)
+{
+#ifdef CONFIG_UNACCEPTED_MEMORY
+ efi_guid_t mem_acceptance_proto = EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUID;
+ memory_acceptance_protocol_t *proto;
+ efi_status_t status;
+
+ /*
+* Enable unaccepted memory before calling exit boot services in order
+* for the UEFI to not accept all memory on EBS.
+*/
+ status = efi_bs_call(locate_protocol, _acceptance_proto, NULL,
+(void **));
+ if (status != EFI_SUCCESS)
+ return;
+
+ status = efi_call_proto(proto, allow_unaccepted_memory);
+ if (status != EFI_SUCCESS)
+ efi_err("Memory acceptance protocol failed\n");
+#endif
+}
+
static const efi_char16_t apple[] = L"Apple";
static void setup_quirks(struct boot_params *boot_params,
@@ -899,6 +933,8 @@ asmlinkage unsigned long efi_main(efi_handle_t handle,
setup_quirks(boot_params, bzimage_addr, buffer_end - buffer_start);
+ setup_unaccepted_memory();
+
status = exit_boot(boot_params, handle);
if (status != EFI_SUCCESS) {
efi_err("exit_boot() failed!\n");
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 4b27519143f5..bfc0e4f2aba5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -391,6 +391,7 @@ void efi_native_runtime_setup(void);
#define EFI_RT_PROPERTIES_TABLE_GUID EFI_GUID(0xeb66918a, 0x7eef,
0x402a, 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9)
#define EFI_DXE_SERVICES_TABLE_GUIDEFI_GUID(0x05ad34ba, 0x6f02,
0x4214, 0x95, 0x2e, 0x4d, 0xa0, 0x39, 0x8e, 0x2b, 0xb9)
#define EFI_SMBIOS_PROTOCOL_GUID EFI_GUID(0x03583ff6, 0xcb36,
0x4940, 0x94, 0x7e, 0xb9, 0xb3, 0x9f, 0x4a, 0xfa, 0xf7)
+#define EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUIDEFI_GUID(0xc5a010fe, 0x38a7,
0x4531, 0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49)
#define EFI_IMAGE_SECURITY_DATABASE_GUID EFI_GUID(0xd719b2cb, 0x3d3a,
0x4596, 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
#define EFI_SHIM_LOCK_GUID EFI_GUID(0x605dab50, 0xe046,
0x4300, 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
--
2.39.0.314.g84b9a713c41-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98500): https://edk2.groups.io/g/devel/message/98500
Mute This Topic: https://groups.io/mt/96250536/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
Thanks for your perspective, Dave. From what I understand, distributions lag behind, user kernel configurations can be varied, and Kirill's patch set is still untested with regards to memory latency of workloads. We may yet see folks opt for a slow boot for better latency. This protocol is for safety purposes, since "hope is not a strategy" as is commonly said at Google. On Fri, Jan 13, 2023 at 9:01 AM wrote: > > Hi Folks, > > My hope (from the x86 side at least) was that all functional Linux TDX guests > will have memory acceptance support. We don't have fully-functional guest > code yet and assuming that Linux memory acceptance support will be in place > before we get there. > > Basically, I was hoping that Linux could get away without needing any kind of > negotiation with the firmware. > > (btw, I'm replying in the web interface. Apologies for any odd formatting) > -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98499): https://edk2.groups.io/g/devel/message/98499 Mute This Topic: https://groups.io/mt/96236145/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v9 4/4] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
Instead of eagerly accepting all memory in PEI, only accept memory under
the 4GB address. This allows a loaded image to use the
MEMORY_ACCEPTANCE_PROTOCOL to disable the accept behavior and indicate
that it can interpret the memory type accordingly.
This classification is safe since ExitBootServices will accept and
reclassify the memory as conventional if the disable protocol is not
used.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Signed-off-by: Dionna Glaze
---
OvmfPkg/PlatformPei/AmdSev.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index e4e7b72e67..7d824cc282 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -63,6 +64,10 @@ AmdSevSnpInitialize (
for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB
(Hob)) {
if ((Hob.Raw != NULL) && (GET_HOB_TYPE (Hob) ==
EFI_HOB_TYPE_RESOURCE_DESCRIPTOR)) {
ResourceHob = Hob.ResourceDescriptor;
+ if (ResourceHob->PhysicalStart >= SIZE_4GB) {
+ResourceHob->ResourceType = BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED;
+continue;
+ }
if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
MemEncryptSevSnpPreValidateSystemRam (
--
2.39.0.314.g84b9a713c41-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98406): https://edk2.groups.io/g/devel/message/98406
Mute This Topic: https://groups.io/mt/96236152/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v9 3/4] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
This protocol implementation disables the accept-all-memory behavior
of the BeforeExitBootServices event this driver adds.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/CocoDxe/CocoDxe.c | 28
OvmfPkg/CocoDxe/CocoDxe.inf | 1 +
2 files changed, 29 insertions(+)
diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
index da16af32a3..57169b0481 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.c
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -18,11 +18,14 @@
#include
#include
#include
+#include
STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
+STATIC EFI_HANDLE mCocoDxeHandle = NULL;
+
STATIC
EFI_STATUS
AcceptAllMemory (
@@ -111,6 +114,21 @@ ResolveUnacceptedMemory (
ASSERT_EFI_ERROR (Status);
}
+STATIC
+EFI_STATUS
+EFIAPI
+AllowUnacceptedMemory (
+ IN BZ3987_MEMORY_ACCEPTANCE_PROTOCOL *This
+ )
+{
+ mAcceptAllMemoryAtEBS = FALSE;
+ return EFI_SUCCESS;
+}
+
+STATIC
+BZ3987_MEMORY_ACCEPTANCE_PROTOCOL
+ mMemoryAcceptanceProtocol = { AllowUnacceptedMemory };
+
EFI_STATUS
EFIAPI
CocoDxeEntryPoint (
@@ -143,5 +161,15 @@ CocoDxeEntryPoint (
DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for
EventBeforeExitBootServices failed.\n"));
}
+ Status = gBS->InstallProtocolInterface (
+ ,
+ ,
+ EFI_NATIVE_INTERFACE,
+
+ );
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Install Bz3987MemoryAcceptanceProtocol failed.\n"));
+ }
+
return EFI_SUCCESS;
}
diff --git a/OvmfPkg/CocoDxe/CocoDxe.inf b/OvmfPkg/CocoDxe/CocoDxe.inf
index 8d4452e94d..05c2651a89 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.inf
+++ b/OvmfPkg/CocoDxe/CocoDxe.inf
@@ -42,4 +42,5 @@
gEfiEventBeforeExitBootServicesGuid
[Protocols]
+ gBz3987MemoryAcceptanceProtocolGuid
gEdkiiMemoryAcceptProtocolGuid
--
2.39.0.314.g84b9a713c41-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98405): https://edk2.groups.io/g/devel/message/98405
Mute This Topic: https://groups.io/mt/96236151/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v9 2/4] MdePkg: Introduce the MemoryAcceptance protocol
The default behavior for unaccepted memory is to accept all memory
when ExitBootServices is called. An OS loader can use this protocol to
disable this behavior to assume responsibility for memory acceptance and
to affirm that the OS can handle the unaccepted memory type.
This is a candidate for standardization.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
MdePkg/Include/Protocol/MemoryAcceptance.h | 40 ++
MdePkg/MdePkg.dec | 3 ++
2 files changed, 43 insertions(+)
create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
diff --git a/MdePkg/Include/Protocol/MemoryAcceptance.h
b/MdePkg/Include/Protocol/MemoryAcceptance.h
new file mode 100644
index 00..0b305b016f
--- /dev/null
+++ b/MdePkg/Include/Protocol/MemoryAcceptance.h
@@ -0,0 +1,40 @@
+/** @file
+ The file provides the protocol that disables the behavior that all memory
+ gets accepted at ExitBootServices(). This protocol is only meant to be called
+ by the OS loader, and not EDK2 itself.
+
+ Copyright (c) 2022, Google LLC. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MEMORY_ACCEPTANCE_H_
+#define MEMORY_ACCEPTANCE_H_
+
+#define BZ3987_MEMORY_ACCEPTANCE_PROTOCOL_GUID \
+ {0xc5a010fe, \
+ 0x38a7, \
+ 0x4531, \
+ {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
+
+typedef struct _BZ3987_MEMORY_ACCEPTANCE_PROTOCOL
BZ3987_MEMORY_ACCEPTANCE_PROTOCOL;
+
+/**
+ @param This A pointer to a BZ3987_MEMORY_ACCEPTANCE_PROTOCOL.
+**/
+typedef
+ EFI_STATUS
+(EFIAPI *BZ3987_ALLOW_UNACCEPTED_MEMORY)(
+ IN BZ3987_MEMORY_ACCEPTANCE_PROTOCOL *This
+ );
+
+///
+/// The BZ3987_MEMORY_ACCEPTANCE_PROTOCOL allows the OS loader to
+/// indicate to EDK2 that ExitBootServices should not accept all memory.
+///
+struct _BZ3987_MEMORY_ACCEPTANCE_PROTOCOL {
+ BZ3987_ALLOW_UNACCEPTED_MEMORYAllowUnacceptedMemory;
+};
+
+extern EFI_GUID gBz3987MemoryAcceptanceProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3d08f20d15..bc3d897248 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1031,6 +1031,9 @@
gEfiPeiDelayedDispatchPpiGuid = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e,
0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}
[Protocols]
+ ## Include/Protocol/Bz3987MemoryAcceptance.h
+ gBz3987MemoryAcceptanceProtocolGuid = { 0xc5a010fe, 0x38a7, 0x4531, {0x8a,
0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49 }}
+
## Include/Protocol/MemoryAccept.h
gEdkiiMemoryAcceptProtocolGuid = { 0x38c74800, 0x5590, 0x4db4, { 0xa0, 0xf3,
0x67, 0x5d, 0x9b, 0x8e, 0x80, 0x26 }}
--
2.39.0.314.g84b9a713c41-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98404): https://edk2.groups.io/g/devel/message/98404
Mute This Topic: https://groups.io/mt/96236150/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v9 1/4] OvmfPkg: Introduce CocoDxe driver
This driver is meant as a join point for all Confidential Compute
technologies to put shared behavior that doesn't belong anywhere else.
The first behavior added here is to accept all unaccepted memory at
ExitBootServices if the behavior is not disabled. This allows safe
upgrades for OS loaders to affirm their support for the unaccepted
memory type.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
OvmfPkg/CocoDxe/CocoDxe.c| 147 +++
OvmfPkg/CocoDxe/CocoDxe.inf | 45 ++
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfPkgX64.fdf | 1 +
10 files changed, 200 insertions(+)
create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 36100f5fdc..5e5e9887bb 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -749,6 +749,7 @@
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
}
+ OvmfPkg/CocoDxe/CocoDxe.inf
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
#
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 5fb3b5d276..ae64693c28 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -302,6 +302,7 @@ INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
new file mode 100644
index 00..da16af32a3
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -0,0 +1,147 @@
+/** @file
+
+ Confidential Compute Dxe driver. This driver installs protocols that are
+ generic over confidential compute techonology.
+
+ Copyright (c) 2022, Google LLC. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
+
+STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
+
+STATIC
+EFI_STATUS
+AcceptAllMemory (
+ IN EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory
+ )
+{
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
+ UINTNNumEntries;
+ UINTNIndex;
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
+
+ /*
+ * Get a copy of the memory space map to iterate over while
+ * changing the map.
+ */
+ Status = gDS->GetMemorySpaceMap (, );
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ for (Index = 0; Index < NumEntries; Index++) {
+CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc;
+
+Desc = [Index];
+if (Desc->GcdMemoryType != EFI_GCD_MEMORY_TYPE_UNACCEPTED) {
+ continue;
+}
+
+Status = AcceptMemory->AcceptMemory (
+ AcceptMemory,
+ Desc->BaseAddress,
+ Desc->Length
+ );
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->RemoveMemorySpace (Desc->BaseAddress, Desc->Length);
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->AddMemorySpace (
+EfiGcdMemoryTypeSystemMemory,
+Desc->BaseAddress,
+Desc->Length,
+EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO |
EFI_MEMORY_RP
+);
+if (EFI_ERROR (Status)) {
+ break;
+}
+ }
+
+ gBS->FreePool (AllDescMap);
+ return Status;
+}
+
+VOID
+EFIAPI
+ResolveUnacceptedMemory (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory;
+ EFI_STATUSStatus;
+
+ if (!mAcceptAllMemoryAtEBS) {
+return;
+ }
+
+ Status = gBS->LocateProtocol (
+ ,
+ NULL,
+ (VOID **)
+ );
+ if (Status == EFI_NOT_FOUND) {
+return;
+ }
+
+ ASSERT_EFI_ERROR (Status);
+
+ Status = AcceptAllMemory (AcceptMemory);
+ ASSERT_EFI_ERROR (Status);
+}
+
+EFI_STATUS
+EFIAPI
+CocoDxeEntryPoint (
+ IN EFI_HANDLEImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Do nothing when confidential compute technologies that require memory
+ // acceptance are not enabled.
+ //
+ if (!MemEncryptSevSnpIsEnabled () &&
+ !MemEncryptTdxIsEnabled ())
+ {
+return EFI_UNSUPPORTED;
+ }
[edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
We make eager memory acceptance the default behavior at ExitBootServices by using the standard-enforced behavior that if the call returns an error code, then the map key is incorrect and the caller must re-call GetMemoryMap to ensure the contents are correct. Eager memory acceptance is implemented by using the UEFI v2.9-added EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES to check a support condition before changing all unaccepted memory type regions to conventional memory after first using the MemoryAccept protocol to accept all memory in each region. This update to the memory map only happens once, since there are no extra unaccepted memory regions to change on the forced second call to ExitBootServices. The new acceptance logic is technology-agnostic and usable across TEE technologies, so this patch series introduces a Confidenial Compute DXE driver called CocoDxe. To allow the OS loader to prevent the eager acceptance, and thus pass the before-mentioned "support condition", we add a new protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol. This protocol has one interface, Disable(). The OS loader can inform the UEFI that it supports the unaccepted memory type and accepts the responsibility to accept it. The MemoryAcceptance protocol is necessary for safe rollout of the unaccepted memory type, given the gradual update of guest OS kernels. OVMF cannot rely on the following implication (MemEncryptSevIsEnabled () || MemEncryptTdxIsEnabled ()) implies unaccepted memory is supported by the guest This implication does not hold for e.g., upstream Linux, and will not hold generally since both SEV-SNP and TDX define unaccepted memory support as optional rather than required. All images that support unaccepted memory must now locate and call this new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable function. Changes since v8: - First 3 patches removed since they were submitted separately. - Later patches rebased on edk2/master and modified to work with the current locations and namings of the unaccepted memory constants. Changes since v7: - Rebased onto lazy accept v4 patch series, so memory accept protocol has the EDKII prefix, and the unaccepted memory type has the BZ3937 prefix. - Removed a bad #include to a header removed in v7. - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL as per the discussion on the buganizer issue. - Uncrustify formatting Changes since v6: - Added implementation of EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. - Changed callback protocol of v5 to instead use the standardized event group for before_exit_boot_services. Changes since v5: - Generic callback protocol moved to MdeModulePkg - Removed use of EFI_WARN_STALE_DATA and added comment that the callback should only return EFI_SUCCESS or EFI_INVALID_PARAMETER. - Removed errant log statement and fixed formatting. Changes since v4: - Commit message wording - Replaced direct change to DxeMain with a more generic callback protocol. - Implemented the direct change as an instance of the callback protocol from a new CocoDxe driver. - Replaced "enable" protocol with a "disable" protocol, since the name was confusing. The AcceptAllUnacceptedMemory protocol directly names the behavior that is disabling. Changes since v3: - "DxeMain accepts all memory" patch split into 3 to make each patch affect only one package at a time. Changes since v2: - Removed the redundant memory accept interface and added the accept behavior to the DXE implementation of MemEncryptSevSnpPreValidateSystemRam. - Fixed missing #include in >=4GB patch. Changes since v1: - Added a patch to classify SEV-SNP memory above 4GB unaccepted. - Fixed style problems in EfiMemoryAcceptProtocol implementation. Cc: Ard Biescheuvel Cc: "Min M. Xu" Cc: Gerd Hoffmann Cc: James Bottomley Cc: Tom Lendacky Cc: Jiewen Yao Cc: Erdem Aktas Cc: Andrew Fish Cc: "Michael D. Kinney" Signed-off-by: Dionna Glaze Dionna Glaze (4): OvmfPkg: Introduce CocoDxe driver MdePkg: Introduce the MemoryAcceptance protocol OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted MdePkg/Include/Protocol/MemoryAcceptance.h | 40 + MdePkg/MdePkg.dec | 3 + OvmfPkg/AmdSev/AmdSevX64.dsc | 1 + OvmfPkg/AmdSev/AmdSevX64.fdf | 1 + OvmfPkg/CocoDxe/CocoDxe.c | 175 + OvmfPkg/CocoDxe/CocoDxe.inf| 46 ++ OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 + OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 + OvmfPkg/OvmfPkgIa32X64.dsc | 1 + OvmfPkg/OvmfPkgIa32X64.fdf | 1 + OvmfPkg/OvmfPkgX64.dsc | 1 + OvmfPkg/OvmfPkgX64.fdf | 1 + OvmfPkg/PlatformPei/AmdSev.c | 5 + 13 files changed, 277 insertions(+) create mode 100644
Re: [edk2-devel] [PATCH 3/3] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
Thanks for your explanation and fastidiousness, Laszlo. Much appreciated. On Thu, Jan 12, 2023 at 7:32 AM Laszlo Ersek wrote: > > On 1/12/23 14:38, Ard Biesheuvel wrote: > > On Thu, 12 Jan 2023 at 13:24, Laszlo Ersek wrote: > >> > >> On 1/12/23 00:08, Dionna Glaze via groups.io wrote: > >>> On Thu, Nov 10, 2022 at 8:55 AM Kinney, Michael D > >>> wrote: > >>>> > >>>> Hi Dionna, > >>>> > >>>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > >>>> > >>>> I think you are at step 13. If you have not done so already, > >>>> please update the commit messages with all the Reviewed-by and > >>>> Acked-by tags and make sure your branch is rebased against the > >>>> latest edk2/master and update the PR with that content and verify > >>>> that all EDK II CI checks pass. > >>>> > >>>> At that point, it is the responsibility of the EDK II Maintainers > >>>> to review the final state of the PR and set the "push" label if > >>>> everything looks correct. > > I didn't realize this had been adopted -- "upgrading" a contributor > submitted PR. (More below.) > > >>>> At this moment, we are in Soft Freeze and will be in Hard Freeze > >>>> for the next 2 weeks. If this is considered critical for the > >>>> stable tag release, then please send a request to Liming with > >>>> justification for stable tag. Otherwise, it will be merged after > >>>> the stable tag release. > >>>> > >>>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning > >>>> > >>>> Thanks, > >>>> > >>>> Mike > >>>> > >>> > >>> Hi Mike, my PR was closed without comment > >>> https://github.com/tianocore/edk2/pull/3623 so I rebased and created > >>> a new PR after the holidays and hard freeze. I hope this isn't > >>> considered bad practice https://github.com/tianocore/edk2/pull/3885 > >>> > >> > >> I closed the PR -- similarly to how I manually closed 400+ stale PRs > >> then -- because it was not a maintainer-submitted PR with the "push" > >> label set. In other words, it was just a personal build, for > >> triggering a CI run. > >> > >> And, in fact regardlessly of whether it was a "push"-labeled > >> maintainer PR or just a personal PR, the PR was almost two months > >> old. Clearly stale and abandoned -- per edk2 contribution workflow, > >> anyway. > >> > >> Please excuse me for not explaining this in a comment on the PR, but > >> (as I said above) I closed 400+ stale PRs manually within 10-15 > >> minutes on the github.com WebUI. The necessary muscle memory training > >> didn't allow for individual comments. > >> > >> (I actually tried scripting the mass-closure at first, with the "gh" > >> utility, but something in the github.com data model was broken, and > >> the server wouldn't allow me to close those PRs with the utility, > >> even though I had authenticated the utility under my account, with a > >> complete set of scopes -- see my report at > >> <https://github.com/cli/cli/discussions/6814>.) > >> > >> Regarding your new PR: it's again good for a personal CI run only. If > >> it completes fine, please post the patches to the mailing list, using > >> git-send-email. > >> > >> (From browsing recent list traffic, it seems that your colleague > >> Moritz Fischer has posted patches like that to > >> the list; please consider consulting with Moritz regarding the git > >> setup (SMTP server etc) withing Google. Cc'ing Moritz now.) > >> > >> When sending the patches like that, please CC the maintainers and > >> reviewers that are supposed to review them. Once they are happy with > >> the series, one of them will submit a PR with them, and set the > >> "push" label on the PR. When the CI run succeeds, the mergify bot > >> will merge *that* PR automatically. > >> > > > > I think we were already way past this with Dionna's work - numerous > > iterations have been posted and discussed, and the merge of the final > > version was only deferred because of the soft freeze. > > > > That may very well be, but the specific PR I closed (3623) was
Re: [edk2-devel] [PATCH 3/3] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
On Thu, Nov 10, 2022 at 8:55 AM Kinney, Michael D wrote: > > Hi Dionna, > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > > I think you are at step 13. If you have not done so already, please update > the > commit messages with all the Reviewed-by and Acked-by tags and make sure your > branch is rebased against the latest edk2/master and update the PR with that > content and verify that all EDK II CI checks pass. > > At that point, it is the responsibility of the EDK II Maintainers to review > the final state of the PR and set the "push" label if everything looks > correct. > > At this moment, we are in Soft Freeze and will be in Hard Freeze for the next > 2 weeks. If this is considered critical for the stable tag release, then > please send a request to Liming with justification for stable tag. Otherwise, > it will be merged after the stable tag release. > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning > > Thanks, > > Mike > Hi Mike, my PR was closed without comment https://github.com/tianocore/edk2/pull/3623 so I rebased and created a new PR after the holidays and hard freeze. I hope this isn't considered bad practice https://github.com/tianocore/edk2/pull/3885 -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98317): https://edk2.groups.io/g/devel/message/98317 Mute This Topic: https://groups.io/mt/94894288/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot
On Tue, Dec 6, 2022 at 5:26 PM gaoliming wrote:
>
> Dionna:
> I add my comments below.
>
> > -邮件原件-
> > 发件人: devel@edk2.groups.io 代表 Dionna Glaze
> > via groups.io
> > 发送时间: 2022年11月9日 5:16
> > 收件人: devel@edk2.groups.io
> > 抄送: Dionna Glaze ; Michael D Kinney
> > ; Ard Biesheuvel ; Gerd
> > Hoffman ; Jiewen Yao
> > 主题: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after
> > ReadyToBoot
> >
> > The Uefi v2.9 specification adds this event group in section 3.1.7,
> > with its GUID defined in the Related Definitions section of
> > EFI_BOOT_SERVICES.CreateEventEx() in chapter 7.
> >
> > Cc: "Michael D Kinney"
> > Cc: Ard Biesheuvel
> > Cc: Gerd Hoffman
> > Cc: Jiewen Yao
> >
> > Signed-off-by: Dionna Glaze
> > ---
> > MdePkg/Include/Library/UefiLib.h | 2 ++
> > MdePkg/Library/UefiLib/UefiLib.inf| 1 +
> > MdePkg/Library/UefiLib/UefiNotTiano.c | 18 ++
> > 3 files changed, 21 insertions(+)
> >
> > diff --git a/MdePkg/Include/Library/UefiLib.h
> > b/MdePkg/Include/Library/UefiLib.h
> > index be7da7fdf7..2c3342351e 100644
> > --- a/MdePkg/Include/Library/UefiLib.h
> > +++ b/MdePkg/Include/Library/UefiLib.h
> > @@ -890,6 +890,8 @@ UnicodeStringDisplayLength (
> > /**
> >Create, Signal, and Close the Ready to Boot event using
> > EfiSignalEventReadyToBoot().
> >
> > + If successful, then create, signal and close the After Ready to Boot
> > event.
> > +
> >This function abstracts the signaling of the Ready to Boot Event. The
> > Framework moved
> >from a proprietary to UEFI 2.0 based mechanism. This library abstracts
> > the caller
> >from how this event is created to prevent to code form having to change
> > with the
> > diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> > b/MdePkg/Library/UefiLib/UefiLib.inf
> > index 01ed92092d..5c4064eafa 100644
> > --- a/MdePkg/Library/UefiLib/UefiLib.inf
> > +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> > @@ -60,6 +60,7 @@
> >gEfiGlobalVariableGuid##
> > SOMETIMES_CONSUMES ## Variable
> >gEfiAcpi20TableGuid ##
> > SOMETIMES_CONSUMES ## SystemTable
> >gEfiAcpi10TableGuid ##
> > SOMETIMES_CONSUMES ## SystemTable
> > + gEfiEventAfterReadyToBootGuid ##
> > SOMETIMES_CONSUMES ## Event
> >
> > [Protocols]
> >gEfiDriverBindingProtocolGuid ##
> > SOMETIMES_PRODUCES
> > diff --git a/MdePkg/Library/UefiLib/UefiNotTiano.c
> > b/MdePkg/Library/UefiLib/UefiNotTiano.c
> > index d84e91fd01..04fe42f9f7 100644
> > --- a/MdePkg/Library/UefiLib/UefiNotTiano.c
> > +++ b/MdePkg/Library/UefiLib/UefiNotTiano.c
> > @@ -208,6 +208,8 @@ EfiCreateEventReadyToBootEx (
> > /**
> >Create, Signal, and Close the Ready to Boot event using
> > EfiSignalEventReadyToBoot().
> >
> > + If successful, then create, signal and close the After Ready to Boot
> > event.
> > +
> >This function abstracts the signaling of the Ready to Boot Event. The
> > Framework moved
> >from a proprietary to UEFI 2.0 based mechanism. This library abstracts
> > the caller
> >from how this event is created to prevent to code form having to change
> > with the
> > @@ -222,11 +224,27 @@ EfiSignalEventReadyToBoot (
> > {
> >EFI_STATUS Status;
> >EFI_EVENT ReadyToBootEvent;
> > + EFI_EVENT AfterReadyToBootEvent;
> >
> >Status = EfiCreateEventReadyToBoot ();
> >if (!EFI_ERROR (Status)) {
> > gBS->SignalEvent (ReadyToBootEvent);
> > gBS->CloseEvent (ReadyToBootEvent);
> > +return;
> > + }
> > +
> Return should not be here. This means ReadyToBoot event creates successfully
> and return.
> But, the behavior should be ReadyToBoot and AfterReadyToBoot event both trig.
>
Thanks Gao, given Michael's feedback about this patch not making much
sense in the series that adds before_exit_boot_services, we decided to
go ahead and consider v1 of this series as final. Let's let Robert
Phelps take it from here, since they appear to need it such that
they're in a better position to test their implementation.
> Thanks
> Liming
> > + /* Then immediately signal the after ready to boot event group */
> > + Status = gBS->CreateEventEx (
> > + EVT_NOTIFY_SIGNAL,
> > + TPL_CALLBACK,
> > + EfiEventEmptyFunctio
Re: [edk2-devel] [PATCH 3/3] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
> Reviewed-by: Michael D Kinney Great, thanks Michael. With all three patches reviewed/acked, does that mean they get merged, or what else has to be done? I'm new to this. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96199): https://edk2.groups.io/g/devel/message/96199 Mute This Topic: https://groups.io/mt/94894288/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/2] MdeModulePkg: Added call to signal New Event
> > I can create a new patch that contains, the two Event definitions and the > code that signals both events. I can use my patches that I already have and > then add the code from your submission that signals the After Ready to Boot > event to that. > > Rob I think since Jiewen and Ard already acked my patches for before_exit_boot_services, you might be better served by just rebasing your after_ready_to_boot changes on my v1 series, since Michael has said he doesn't want the after_ready_to_boot changes I had in v2. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96163): https://edk2.groups.io/g/devel/message/96163 Mute This Topic: https://groups.io/mt/94899157/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot
Okay, so drop v2 and go back to review v1? Nothing changed other than this. On Wed, Nov 9, 2022 at 9:35 AM Kinney, Michael D wrote: > > Hi Dionna, > > My mistake. I misread the UEFI 2.9 Spec content and thought they were both > for exit boot services. > > Your patch series with only Before Exit Boot Services is appropriate for your > change. > > The After Ready To Boot can be in its own patch series. > > Best regards, > > Mike > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of Dionna Glaze > > via groups.io > > Sent: Wednesday, November 9, 2022 9:12 AM > > To: devel@edk2.groups.io; Kinney, Michael D > > Cc: Ard Biesheuvel ; Gerd Hoffman ; > > Yao, Jiewen > > Subject: Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot > > after ReadyToBoot > > > > > I gave feedback about After Exit Boot Services event. > > > > > > > I saw no such event in the specification > > > > > Why is an After Ready To Boot signal now part of this series? > > > > > > > I thought that's the event you meant, since its mantis number is 2042, > > whereas before exit boot services is 2043. That fits the "same time" > > you mentioned in the other email. > > > > > > -- > > -Dionna Glaze, PhD (she/her) > > > > > > > > > -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96161): https://edk2.groups.io/g/devel/message/96161 Mute This Topic: https://groups.io/mt/94900168/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot
> I gave feedback about After Exit Boot Services event. > I saw no such event in the specification > Why is an After Ready To Boot signal now part of this series? > I thought that's the event you meant, since its mantis number is 2042, whereas before exit boot services is 2043. That fits the "same time" you mentioned in the other email. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96143): https://edk2.groups.io/g/devel/message/96143 Mute This Topic: https://groups.io/mt/94900168/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/2] MdeModulePkg: Added call to signal New Event
> > I am confused. I see patches related to ReadyToBoot and ExitBootServices and > they mix what is in description and what is in code. > Will you please comment on the patches in question where the error is? I don't follow what you're saying. > I recommend you coordinate and put together a clean set of patches for these > topics. > Sounds good. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96141): https://edk2.groups.io/g/devel/message/96141 Mute This Topic: https://groups.io/mt/94899157/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/2] MdeModulePkg: Added call to signal New Event
> So how should we want to handle this. Leave yours in or leave mine in. Given that I'm not particularly confident in how I've implemented the after_ready_to_boot spec, and you haven't implemented it, I'm not sure. I'm pursuing the before_exit_boot_services implementation to solve a problem in confidential compute as part of my job, so I think I have the motivation to keep pushing on this. Let's go with mine since it's further along. I'd appreciate a Reviewed-by if you agree with the implementations. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96138): https://edk2.groups.io/g/devel/message/96138 Mute This Topic: https://groups.io/mt/94899157/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/2] MdeModulePkg: Added call to signal New Event
> > This code signals the Before Exit Boot Services event at the beginning > of the ExitBootServices() function. This gives all the rest of the > code to prepare for the ExitBootServices event. > Hi Robert, I think we're both trying to do this at the same time. See the patch series "SEV-SNP accepted memory and BeforeExitBootServices" which suppose now is not appropriately titled since Michael asked me to add the AfterReadyToBoot event. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96122): https://edk2.groups.io/g/devel/message/96122 Mute This Topic: https://groups.io/mt/94899157/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot
The Uefi v2.9 specification adds this event group in section 3.1.7,
with its GUID defined in the Related Definitions section of
EFI_BOOT_SERVICES.CreateEventEx() in chapter 7.
Cc: "Michael D Kinney"
Cc: Ard Biesheuvel
Cc: Gerd Hoffman
Cc: Jiewen Yao
Signed-off-by: Dionna Glaze
---
MdePkg/Include/Library/UefiLib.h | 2 ++
MdePkg/Library/UefiLib/UefiLib.inf| 1 +
MdePkg/Library/UefiLib/UefiNotTiano.c | 18 ++
3 files changed, 21 insertions(+)
diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index be7da7fdf7..2c3342351e 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -890,6 +890,8 @@ UnicodeStringDisplayLength (
/**
Create, Signal, and Close the Ready to Boot event using
EfiSignalEventReadyToBoot().
+ If successful, then create, signal and close the After Ready to Boot event.
+
This function abstracts the signaling of the Ready to Boot Event. The
Framework moved
from a proprietary to UEFI 2.0 based mechanism. This library abstracts the
caller
from how this event is created to prevent to code form having to change with
the
diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
b/MdePkg/Library/UefiLib/UefiLib.inf
index 01ed92092d..5c4064eafa 100644
--- a/MdePkg/Library/UefiLib/UefiLib.inf
+++ b/MdePkg/Library/UefiLib/UefiLib.inf
@@ -60,6 +60,7 @@
gEfiGlobalVariableGuid## SOMETIMES_CONSUMES ##
Variable
gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ##
SystemTable
gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ##
SystemTable
+ gEfiEventAfterReadyToBootGuid ## SOMETIMES_CONSUMES ## Event
[Protocols]
gEfiDriverBindingProtocolGuid ## SOMETIMES_PRODUCES
diff --git a/MdePkg/Library/UefiLib/UefiNotTiano.c
b/MdePkg/Library/UefiLib/UefiNotTiano.c
index d84e91fd01..04fe42f9f7 100644
--- a/MdePkg/Library/UefiLib/UefiNotTiano.c
+++ b/MdePkg/Library/UefiLib/UefiNotTiano.c
@@ -208,6 +208,8 @@ EfiCreateEventReadyToBootEx (
/**
Create, Signal, and Close the Ready to Boot event using
EfiSignalEventReadyToBoot().
+ If successful, then create, signal and close the After Ready to Boot event.
+
This function abstracts the signaling of the Ready to Boot Event. The
Framework moved
from a proprietary to UEFI 2.0 based mechanism. This library abstracts the
caller
from how this event is created to prevent to code form having to change with
the
@@ -222,11 +224,27 @@ EfiSignalEventReadyToBoot (
{
EFI_STATUS Status;
EFI_EVENT ReadyToBootEvent;
+ EFI_EVENT AfterReadyToBootEvent;
Status = EfiCreateEventReadyToBoot ();
if (!EFI_ERROR (Status)) {
gBS->SignalEvent (ReadyToBootEvent);
gBS->CloseEvent (ReadyToBootEvent);
+return;
+ }
+
+ /* Then immediately signal the after ready to boot event group */
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ EfiEventEmptyFunction,
+ NULL,
+ ,
+ AfterReadyToBootEvent
+ );
+ if (!EFI_ERROR(Status)) {
+gBS->SignalEvent (AfterReadyToBootEvent);
+gBS->CloseEvent (AfterReadyToBootEvent);
}
}
--
2.38.1.431.g37b22c650d-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96108): https://edk2.groups.io/g/devel/message/96108
Mute This Topic: https://groups.io/mt/94900168/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 3/4] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
Location of notification is has been specified in UEFI v2.9.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Cc: Ray Ni
Acked-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++
2 files changed, 7 insertions(+)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index e4bca89577..35d5bf0dee 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -100,6 +100,7 @@
gEfiEventVirtualAddressChangeGuid ## CONSUMES ##
Event
## CONSUMES ## Event
## PRODUCES ## Event
+ gEfiEventBeforeExitBootServicesGuid
gEfiEventExitBootServicesGuid
gEfiHobMemoryAllocModuleGuid ## SOMETIMES_CONSUMES ## HOB
gEfiFirmwareFileSystem2Guid ## CONSUMES ##
GUID # Used to compare with FV's file system guid and get the FV's file system
format
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 5733f0c8ec..4683016ed7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -763,6 +763,12 @@ CoreExitBootServices (
{
EFI_STATUS Status;
+ //
+ // Notify other drivers of their last chance to use boot services
+ // before the memory map is terminated.
+ //
+ CoreNotifySignalList ();
+
//
// Disable Timer
//
--
2.38.1.431.g37b22c650d-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96107): https://edk2.groups.io/g/devel/message/96107
Mute This Topic: https://groups.io/mt/94900167/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 2/4] MdePkg: Add event groups for boot events
Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
Add EFI_EVENT_GROUP_AFTER_READY_TO_BOOT
Both defined in UEFI standard v2.9.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Signed-off-by: Dionna Glaze
---
MdePkg/Include/Guid/EventGroup.h | 10 ++
MdePkg/MdePkg.dec| 8 +++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Guid/EventGroup.h b/MdePkg/Include/Guid/EventGroup.h
index 063d1f7157..821b728e59 100644
--- a/MdePkg/Include/Guid/EventGroup.h
+++ b/MdePkg/Include/Guid/EventGroup.h
@@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
extern EFI_GUID gEfiEventExitBootServicesGuid;
+#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
+ { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b,
0xfc } }
+
+extern EFI_GUID gEfiEventBeforeExitBootServicesGuid;
+
#define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
{ 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51,
0x96 } }
@@ -29,6 +34,11 @@ extern EFI_GUID gEfiEventMemoryMapChangeGuid;
extern EFI_GUID gEfiEventReadyToBootGuid;
+#define EFI_EVENT_GROUP_AFTER_READY_TO_BOOT \
+ { 0x3a2a00ad, 0x98b9, 0x4cdf, { 0xa4, 0x78, 0x70, 0x27, 0x77, 0xf1, 0xc1, 0xb
} }
+
+extern EFI_GUID gEfiEventAfterReadyToBootGuid;
+
#define EFI_EVENT_GROUP_DXE_DISPATCH_GUID \
{ 0x7081e22f, 0xcac6, 0x4053, { 0x94, 0x68, 0x67, 0x57, 0x82, 0xcf, 0x88,
0xe5 }}
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6b6bfbec29..c4ccec935a 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -408,11 +408,17 @@
gEfiEventMemoryMapChangeGuid = { 0x78BEE926, 0x692F, 0x48FD, { 0x9E, 0xDB,
0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
## Include/Guid/EventGroup.h
- gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87,
0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+ gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87,
0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+
+ ## Include/Guid/EventGroup.h
+ gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44, { 0x80,
0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
## Include/Guid/EventGroup.h
gEfiEventExitBootServicesGuid = { 0x27ABF055, 0xB1B8, 0x4C26, { 0x80, 0x48,
0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
+ ## Include/Guid/EventGroup.h
+ gEfiEventAfterReadyToBootGuid = { 0x3A2A00AD, 0x98B9, 0x4CDF, { 0xA4, 0x78,
0x70, 0x27, 0x77, 0xF1, 0xC1, 0xB }}
+
## Include/Protocol/DebugPort.h
gEfiDebugPortVariableGuid = { 0xEBA4E8D2, 0x3858, 0x41EC, { 0xA2, 0x81,
0x26, 0x47, 0xBA, 0x96, 0x60, 0xD0 }}
--
2.38.1.431.g37b22c650d-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96106): https://edk2.groups.io/g/devel/message/96106
Mute This Topic: https://groups.io/mt/94900166/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 1/4] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
From: Sophia Wolf
When a guest OS does not support unaccepted memory, the unaccepted
memory must be accepted before returning a memory map to the caller.
EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Reviewed-by: Tom Lendacky
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55
++--
OvmfPkg/AmdSevDxe/AmdSevDxe.inf| 3 ++
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24
+++--
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 662d3c4ccb..f7600c3c81 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
SIGNATURE_32 ('A','M', 'D', 'E'),
@@ -31,6 +32,40 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
mSnpBootDxeTable = {
FixedPcdGet32 (PcdOvmfCpuidSize),
};
+STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+
+#define IS_ALIGNED(x, y) x) & ((y) - 1)) == 0))
+
+STATIC
+EFI_STATUS
+EFIAPI
+AmdSevMemoryAccept (
+ IN EDKII_MEMORY_ACCEPT_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS StartAddress,
+ IN UINTN Size
+ )
+{
+ //
+ // The StartAddress must be page-aligned, and the Size must be a positive
+ // multiple of SIZE_4KB. Use an assert instead of returning an erros since
+ // this is an EDK2-internal protocol.
+ //
+ ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
+ ASSERT (IS_ALIGNED (Size, SIZE_4KB));
+ ASSERT (Size != 0);
+
+ MemEncryptSevSnpPreValidateSystemRam (
+StartAddress,
+EFI_SIZE_TO_PAGES (Size)
+);
+
+ return EFI_SUCCESS;
+}
+
+STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
+ AmdSevMemoryAccept
+};
+
EFI_STATUS
EFIAPI
AmdSevDxeEntryPoint (
@@ -147,11 +182,23 @@ AmdSevDxeEntryPoint (
}
}
- //
- // If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
- // It contains the location for both the Secrets and CPUID page.
- //
if (MemEncryptSevSnpIsEnabled ()) {
+//
+// Memory acceptance began being required in SEV-SNP, so install the
+// memory accept protocol implementation for a SEV-SNP active guest.
+//
+Status = gBS->InstallProtocolInterface (
+,
+,
+EFI_NATIVE_INTERFACE,
+
+);
+ASSERT_EFI_ERROR (Status);
+
+//
+// If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
+// It contains the location for both the Secrets and CPUID page.
+//
return gBS->InstallConfigurationTable (
,
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 9acf860cf2..cd1b686c53 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -47,6 +47,9 @@
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+[Protocols]
+ gEdkiiMemoryAcceptProtocolGuid
+
[Guids]
gConfidentialComputingSevSnpBlobGuid
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index d3a95e4913..cbcdd46f52 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -14,6 +14,7 @@
#include
#include "SnpPageStateChange.h"
+#include "VirtualMemory.h"
/**
Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
@@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam (
IN UINTN NumPages
)
{
+ EFI_STATUS Status;
+
if (!MemEncryptSevSnpIsEnabled ()) {
return;
}
- //
- // All the pre-validation must be completed in the PEI phase.
- //
- ASSERT (FALSE);
+ // DXE pre-validation may happen with the memory accept protocol.
+ // The protocol should only be called outside the prevalidated ranges
+ // that the PEI stage code explicitly skips. Specifically, only memory
+ // ranges that are classified as unaccepted.
+ if (BaseAddress >= SIZE_4GB) {
+Status = InternalMemEncryptSevCreateIdentityMap1G (
+ 0,
+ BaseAddress,
+ EFI_PAGES_TO_SIZE (NumPages)
+ );
+if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
+ }
+
+ InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
}
--
2.38.1.431.g37b22c650d-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
[edk2-devel] [PATCH v2 0/4] SEV-SNP accepted memory and BeforeExitBootServices
This is the first half of the patch series [PATCH v8 0/7] Add safe unaccepted memory behavior These patches add SEV-SNP support for the MemoryAccept protocol, and implement an already standardized mechanism for performing any actions just before terminating the memory map. We implement a standardized event group from UEFI v2.9, EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides exactly the right invocation point for eagerly accepting memory if eager acceptance has not been disabled (i.e., unaccepted memory is enabled). The use of the BeforeExitBootServices addition will come in the second half of this series. Changes since v1: * Added EFI_EVENT_GROUP_AFTER_READY_TO_BOOT and interpretation. Cc: Ard Biescheuvel Cc: "Min M. Xu" Cc: Gerd Hoffmann Cc: James Bottomley Cc: Tom Lendacky Cc: Jiewen Yao Cc: Erdem Aktas Cc: Andrew Fish Cc: "Michael D. Kinney" Signed-off-by: Dionna Glaze Dionna Glaze (4): OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe MdePkg: Add event groups for boot events MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices MdePkg: Signal AfterReadyToBoot after ReadyToBoot MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c| 6 +++ MdePkg/Include/Guid/EventGroup.h | 10 MdePkg/Include/Library/UefiLib.h | 2 + MdePkg/Library/UefiLib/UefiLib.inf | 1 + MdePkg/Library/UefiLib/UefiNotTiano.c | 18 +++ MdePkg/MdePkg.dec | 8 ++- OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++-- OvmfPkg/AmdSevDxe/AmdSevDxe.inf| 3 ++ OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++-- 10 files changed, 119 insertions(+), 9 deletions(-) -- 2.38.1.431.g37b22c650d-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96104): https://edk2.groups.io/g/devel/message/96104 Mute This Topic: https://groups.io/mt/94900162/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
> The total time for memory accept is 4 part: >1) vBIOS early phase, single thread accept >2) vBIOS runtime phase, multi thread accept >3) OS early phase, single thread accept >4) OS runtime phase, multi thread accept > >We hope 1) is zero. >And we are trying to balance between 2) and 3). Do you have a sense of how once you have that balance determined you would specify accepting less than 4GB when an OS supports it, and accepting 4GB when an OS doesn't? I'm not familiar enough with vBIOS memory allocation to know how runtime stacks are created. If we're in the situation images are "self-contained" in that the only memory that must be accepted is just enough to fit the binary size of an image, and then the image uses AllocatePages to allocate its own stack and whatever else, then we could be okay to not require any pre-acceptance. The lazy accept Min proposed in Page.c seems like it'd be fine enough to handle all memory use prior to the accept decision at EBS. This doesn't appear to be the world in which we live, given the crashes I experienced in the above mentioned PR, but I could try again. If we're not in the "self-contained" image situation, I don't know what allocation decisions happen during the StartImage process. If an image expects memory to be accessible that isn't part of the image binary itself, other than the two tables passed into the entry point, I don't know what that would be. That implicit expectation would need to become explicit with a binary format change to all EFI modules, to state their requirements before starting. If an EFI module doesn't specify its requirements, we assume that the 4GB amount must be accepted. A bootloader would need to do something similar when parsing the OS binary header: what memory is needed? Accept that if specified, or accept the 4GB for safety. These are very tricky cross-community problems to solve, and I don't think it's appropriate to lump it in with a simple protocol that we know is safe and performs well enough. > > I hope to discuss more after we get the data. Do you think that'll happen before mid-December? Thanks, -Dionna On Tue, Nov 8, 2022 at 8:43 AM Dionna Amalie Glaze wrote: > > > > > BTW: Is there any data for AMD-SEV? > > > > Earlier this year, I tried to make the lazy accept patches work for > SEV-SNP, but the boot would always crash in the Qemu try boot kernel > step IIRC: > > https://github.com/AMDESE/ovmf/pull/4 > > Tom suggested accepting the first 4GiB and I didn't go back to try to > make the more complicated solution work, since accepting the HOB up to > the MMIO hole took roughly 30ms, which is well within our boot budget > given the outsized cost of pinning memory. The launch sequence (start, > update_data*, finish) for OVMF takes about 190ms, which we can't make > any lower. > > > > > Thank you > > Yao, Jiewen > > > > > > > -Original Message- > > > From: Dionna Amalie Glaze > > > Sent: Wednesday, November 9, 2022 12:23 AM > > > To: Yao, Jiewen > > > Cc: devel@edk2.groups.io; thomas.lenda...@amd.com; Ard Biescheuvel > > > ; Xu, Min M ; Gerd Hoffmann > > > ; James Bottomley ; Aktas, > > > Erdem ; Andrew Fish ; > > > Kinney, Michael D > > > Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory > > > behavior > > > > > > On Tue, Nov 8, 2022 at 8:09 AM Yao, Jiewen wrote: > > > > > > > > Hi Tom/Dionna > > > > I think this patch is addressing > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3987. > > > > > > > > For patch 1~3, I am OK for the API which we already agreed, such as > > > EfiMemoryAcceptProtocol and > > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. > > > > I have given Acked by: Jiewen Yao > > > > > > > > For patch 4, it changed the behavior to accept all. I don’t believe it > > > > should > > > be 4. It should be the latest one. (please correct me if I am wrong.) > > > > > > > > > > The acceptance protocol (5-6) and the accept-all behavior (4) could be > > > swapped in order, but they really only make sense as a pair. No client > > > would use a new protocol to disable the behavior of something that > > > doesn't happen. > > > The accept-all behavior (4) must be default before SEV-SNP introduces > > > unaccepted memory (7). > > > > > > > For patch 5~6, I cannot give R-B for > > > BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does > > > not address my concern. Please refer to the discussion in the Bugzilla. > > > > > > > > > > I don't think your concern is addressable given the constraints of a > > > smooth upgrade path. I think the right folks to discuss this with you > > > from the Intel side are Min Xu and Kirill Shutemov. Kirill will know > > > what is feasible for the OS to inform the UEFI of in terms of memory > > > required, and how to not break OSes that don't use the new protocol, > > > whereas Min should know where that kind of information should be > > > communicated. > > > > > > > > > > > If you want to split the patch series and submit 1~3 as standalone
[edk2-devel] [PATCH 3/3] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
Location of notification is has been specified in UEFI v2.9.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Cc: Ray Ni
Acked-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++
2 files changed, 7 insertions(+)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index e4bca89577..35d5bf0dee 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -100,6 +100,7 @@
gEfiEventVirtualAddressChangeGuid ## CONSUMES ##
Event
## CONSUMES ## Event
## PRODUCES ## Event
+ gEfiEventBeforeExitBootServicesGuid
gEfiEventExitBootServicesGuid
gEfiHobMemoryAllocModuleGuid ## SOMETIMES_CONSUMES ## HOB
gEfiFirmwareFileSystem2Guid ## CONSUMES ##
GUID # Used to compare with FV's file system guid and get the FV's file system
format
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 5733f0c8ec..4683016ed7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -763,6 +763,12 @@ CoreExitBootServices (
{
EFI_STATUS Status;
+ //
+ // Notify other drivers of their last chance to use boot services
+ // before the memory map is terminated.
+ //
+ CoreNotifySignalList ();
+
//
// Disable Timer
//
--
2.38.1.431.g37b22c650d-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96091): https://edk2.groups.io/g/devel/message/96091
Mute This Topic: https://groups.io/mt/94894288/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/3] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
Event group as defined in UEFI standard v2.9.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Acked-by: Jiewen Yao
Signed-off-by: Dionna Glaze
---
MdePkg/Include/Guid/EventGroup.h | 5 +
MdePkg/MdePkg.dec| 5 -
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Guid/EventGroup.h b/MdePkg/Include/Guid/EventGroup.h
index 063d1f7157..64bfd4bab9 100644
--- a/MdePkg/Include/Guid/EventGroup.h
+++ b/MdePkg/Include/Guid/EventGroup.h
@@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
extern EFI_GUID gEfiEventExitBootServicesGuid;
+#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
+ { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b,
0xfc } }
+
+extern EFI_GUID gEfiEventBeforeExitBootServicesGuid;
+
#define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
{ 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51,
0x96 } }
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6b6bfbec29..359a85ea10 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -408,7 +408,10 @@
gEfiEventMemoryMapChangeGuid = { 0x78BEE926, 0x692F, 0x48FD, { 0x9E, 0xDB,
0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
## Include/Guid/EventGroup.h
- gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87,
0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+ gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87,
0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+
+ ## Include/Guid/EventGroup.h
+ gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44, { 0x80,
0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
## Include/Guid/EventGroup.h
gEfiEventExitBootServicesGuid = { 0x27ABF055, 0xB1B8, 0x4C26, { 0x80, 0x48,
0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
--
2.38.1.431.g37b22c650d-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96090): https://edk2.groups.io/g/devel/message/96090
Mute This Topic: https://groups.io/mt/94894286/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/3] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
From: Sophia Wolf
When a guest OS does not support unaccepted memory, the unaccepted
memory must be accepted before returning a memory map to the caller.
EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Reviewed-by: Tom Lendacky
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55
++--
OvmfPkg/AmdSevDxe/AmdSevDxe.inf| 3 ++
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24
+++--
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 662d3c4ccb..f7600c3c81 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
SIGNATURE_32 ('A','M', 'D', 'E'),
@@ -31,6 +32,40 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
mSnpBootDxeTable = {
FixedPcdGet32 (PcdOvmfCpuidSize),
};
+STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+
+#define IS_ALIGNED(x, y) x) & ((y) - 1)) == 0))
+
+STATIC
+EFI_STATUS
+EFIAPI
+AmdSevMemoryAccept (
+ IN EDKII_MEMORY_ACCEPT_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS StartAddress,
+ IN UINTN Size
+ )
+{
+ //
+ // The StartAddress must be page-aligned, and the Size must be a positive
+ // multiple of SIZE_4KB. Use an assert instead of returning an erros since
+ // this is an EDK2-internal protocol.
+ //
+ ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
+ ASSERT (IS_ALIGNED (Size, SIZE_4KB));
+ ASSERT (Size != 0);
+
+ MemEncryptSevSnpPreValidateSystemRam (
+StartAddress,
+EFI_SIZE_TO_PAGES (Size)
+);
+
+ return EFI_SUCCESS;
+}
+
+STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
+ AmdSevMemoryAccept
+};
+
EFI_STATUS
EFIAPI
AmdSevDxeEntryPoint (
@@ -147,11 +182,23 @@ AmdSevDxeEntryPoint (
}
}
- //
- // If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
- // It contains the location for both the Secrets and CPUID page.
- //
if (MemEncryptSevSnpIsEnabled ()) {
+//
+// Memory acceptance began being required in SEV-SNP, so install the
+// memory accept protocol implementation for a SEV-SNP active guest.
+//
+Status = gBS->InstallProtocolInterface (
+,
+,
+EFI_NATIVE_INTERFACE,
+
+);
+ASSERT_EFI_ERROR (Status);
+
+//
+// If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
+// It contains the location for both the Secrets and CPUID page.
+//
return gBS->InstallConfigurationTable (
,
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 9acf860cf2..cd1b686c53 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -47,6 +47,9 @@
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+[Protocols]
+ gEdkiiMemoryAcceptProtocolGuid
+
[Guids]
gConfidentialComputingSevSnpBlobGuid
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index d3a95e4913..cbcdd46f52 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -14,6 +14,7 @@
#include
#include "SnpPageStateChange.h"
+#include "VirtualMemory.h"
/**
Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
@@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam (
IN UINTN NumPages
)
{
+ EFI_STATUS Status;
+
if (!MemEncryptSevSnpIsEnabled ()) {
return;
}
- //
- // All the pre-validation must be completed in the PEI phase.
- //
- ASSERT (FALSE);
+ // DXE pre-validation may happen with the memory accept protocol.
+ // The protocol should only be called outside the prevalidated ranges
+ // that the PEI stage code explicitly skips. Specifically, only memory
+ // ranges that are classified as unaccepted.
+ if (BaseAddress >= SIZE_4GB) {
+Status = InternalMemEncryptSevCreateIdentityMap1G (
+ 0,
+ BaseAddress,
+ EFI_PAGES_TO_SIZE (NumPages)
+ );
+if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
+ }
+
+ InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
}
--
2.38.1.431.g37b22c650d-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
[edk2-devel] [PATCH 0/3] SEV-SNP accepted memory and BeforeExitBootServices
This is the first half of the patch series [PATCH v8 0/7] Add safe unaccepted memory behavior These patches add SEV-SNP support for the MemoryAccept protocol, and implement an already standardized mechanism for performing any actions just before terminating the memory map. We implement a standardized event group from UEFI v2.9, EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides exactly the right invocation point for eagerly accepting memory if eager acceptance has not been disabled (i.e., unaccepted memory is enabled). The use of the BeforeExitBootServices addition will come in the second half of this series. Cc: Ard Biescheuvel Cc: "Min M. Xu" Cc: Gerd Hoffmann Cc: James Bottomley Cc: Tom Lendacky Cc: Jiewen Yao Cc: Erdem Aktas Cc: Andrew Fish Cc: "Michael D. Kinney" Signed-off-by: Dionna Glaze Dionna Glaze (3): OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c| 6 +++ MdePkg/Include/Guid/EventGroup.h | 5 ++ MdePkg/MdePkg.dec | 5 +- OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++-- OvmfPkg/AmdSevDxe/AmdSevDxe.inf| 3 ++ OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++-- 7 files changed, 90 insertions(+), 9 deletions(-) -- 2.38.1.431.g37b22c650d-goog -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96088): https://edk2.groups.io/g/devel/message/96088 Mute This Topic: https://groups.io/mt/94894284/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
> > BTW: Is there any data for AMD-SEV? > Earlier this year, I tried to make the lazy accept patches work for SEV-SNP, but the boot would always crash in the Qemu try boot kernel step IIRC: https://github.com/AMDESE/ovmf/pull/4 Tom suggested accepting the first 4GiB and I didn't go back to try to make the more complicated solution work, since accepting the HOB up to the MMIO hole took roughly 30ms, which is well within our boot budget given the outsized cost of pinning memory. The launch sequence (start, update_data*, finish) for OVMF takes about 190ms, which we can't make any lower. > > Thank you > Yao, Jiewen > > > > -Original Message- > > From: Dionna Amalie Glaze > > Sent: Wednesday, November 9, 2022 12:23 AM > > To: Yao, Jiewen > > Cc: devel@edk2.groups.io; thomas.lenda...@amd.com; Ard Biescheuvel > > ; Xu, Min M ; Gerd Hoffmann > > ; James Bottomley ; Aktas, > > Erdem ; Andrew Fish ; > > Kinney, Michael D > > Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory > > behavior > > > > On Tue, Nov 8, 2022 at 8:09 AM Yao, Jiewen wrote: > > > > > > Hi Tom/Dionna > > > I think this patch is addressing > > https://bugzilla.tianocore.org/show_bug.cgi?id=3987. > > > > > > For patch 1~3, I am OK for the API which we already agreed, such as > > EfiMemoryAcceptProtocol and > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. > > > I have given Acked by: Jiewen Yao > > > > > > For patch 4, it changed the behavior to accept all. I don’t believe it > > > should > > be 4. It should be the latest one. (please correct me if I am wrong.) > > > > > > > The acceptance protocol (5-6) and the accept-all behavior (4) could be > > swapped in order, but they really only make sense as a pair. No client > > would use a new protocol to disable the behavior of something that > > doesn't happen. > > The accept-all behavior (4) must be default before SEV-SNP introduces > > unaccepted memory (7). > > > > > For patch 5~6, I cannot give R-B for > > BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does > > not address my concern. Please refer to the discussion in the Bugzilla. > > > > > > > I don't think your concern is addressable given the constraints of a > > smooth upgrade path. I think the right folks to discuss this with you > > from the Intel side are Min Xu and Kirill Shutemov. Kirill will know > > what is feasible for the OS to inform the UEFI of in terms of memory > > required, and how to not break OSes that don't use the new protocol, > > whereas Min should know where that kind of information should be > > communicated. > > > > > > > > If you want to split the patch series and submit 1~3 as standalone one, > > that will match SEV to TDX on lazy accept part. I believe we may merge > > them after we get R-B from right person. > > > > > > > I can do that. > > > > > Thank you > > > Yao, Jiewen > > > > > > > > > > -Original Message- > > > > From: devel@edk2.groups.io On Behalf Of > > > > Lendacky, Thomas via groups.io > > > > Sent: Tuesday, November 8, 2022 10:38 PM > > > > To: Dionna Glaze ; devel@edk2.groups.io > > > > Cc: Ard Biescheuvel ; Xu, Min M > > ; > > > > Gerd Hoffmann ; James Bottomley > > > > ; Yao, Jiewen ; Aktas, > > Erdem > > > > ; Andrew Fish ; Kinney, > > > > Michael D > > > > Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory > > > > behavior > > > > > > > > On 10/24/22 15:41, Dionna Glaze wrote: > > > > > These seven patches build on the lazy-accept patch series > > > > > > > > > > "Introduce Lazy-accept for Tdx guest" > > > > > > > > Since the above series was accepted into the EDK2 tree, can this series > > > > also be pulled in so that both TDX and SNP can support unaccepted > > > > memory > > > > in the same release? > > > > > > > > Thanks, > > > > Tom > > > > > > > > > > > > > > by adding SEV-SNP support for the MemoryAccept protocol, and > > > > > importantly making eager memory acceptance the default behavior. > > > > > > > > > > We implement a standardized event group from UEFI v2.9, > > > > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides > > > > exactly > > > > > the right invocation point for eagerly accepting memory if eager > > > > > acceptance has not been disabled. > > > > > > > > > > To make use of this event group, we add a new driver that is meant to > > > > > carry behavior that is needed for all confidential compute > > technologies, > > > > > not just specific platforms, CocoDxe. In CocoDxe we implement the > > > > > default safe behavior to accept all unaccepted memory and invalidate > > > > > the MemoryMap on ExitBootServices. > > > > > > > > > > To allow the OS loader to prevent the eager acceptance, we add a new > > > > > protocol, up for standardization, > > AcceptAllUnacceptedMemoryProtocol. > > > > > This protocol has one interface, Disable(). The OS loader can inform > > the > > > > > UEFI that it supports the unaccepted memory type and accepts the > > > > > responsibility to accept it. > > > > > > > > >
Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
On Tue, Nov 8, 2022 at 8:09 AM Yao, Jiewen wrote: > > Hi Tom/Dionna > I think this patch is addressing > https://bugzilla.tianocore.org/show_bug.cgi?id=3987. > > For patch 1~3, I am OK for the API which we already agreed, such as > EfiMemoryAcceptProtocol and EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. > I have given Acked by: Jiewen Yao > > For patch 4, it changed the behavior to accept all. I don’t believe it should > be 4. It should be the latest one. (please correct me if I am wrong.) > The acceptance protocol (5-6) and the accept-all behavior (4) could be swapped in order, but they really only make sense as a pair. No client would use a new protocol to disable the behavior of something that doesn't happen. The accept-all behavior (4) must be default before SEV-SNP introduces unaccepted memory (7). > For patch 5~6, I cannot give R-B for BZ3987_MEMORY_ACCEPTANCE_PROTOCAL > proposed here because it does not address my concern. Please refer to the > discussion in the Bugzilla. > I don't think your concern is addressable given the constraints of a smooth upgrade path. I think the right folks to discuss this with you from the Intel side are Min Xu and Kirill Shutemov. Kirill will know what is feasible for the OS to inform the UEFI of in terms of memory required, and how to not break OSes that don't use the new protocol, whereas Min should know where that kind of information should be communicated. > > If you want to split the patch series and submit 1~3 as standalone one, that > will match SEV to TDX on lazy accept part. I believe we may merge them after > we get R-B from right person. > I can do that. > Thank you > Yao, Jiewen > > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of > > Lendacky, Thomas via groups.io > > Sent: Tuesday, November 8, 2022 10:38 PM > > To: Dionna Glaze ; devel@edk2.groups.io > > Cc: Ard Biescheuvel ; Xu, Min M ; > > Gerd Hoffmann ; James Bottomley > > ; Yao, Jiewen ; Aktas, Erdem > > ; Andrew Fish ; Kinney, > > Michael D > > Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory > > behavior > > > > On 10/24/22 15:41, Dionna Glaze wrote: > > > These seven patches build on the lazy-accept patch series > > > > > > "Introduce Lazy-accept for Tdx guest" > > > > Since the above series was accepted into the EDK2 tree, can this series > > also be pulled in so that both TDX and SNP can support unaccepted > > memory > > in the same release? > > > > Thanks, > > Tom > > > > > > > > by adding SEV-SNP support for the MemoryAccept protocol, and > > > importantly making eager memory acceptance the default behavior. > > > > > > We implement a standardized event group from UEFI v2.9, > > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides > > exactly > > > the right invocation point for eagerly accepting memory if eager > > > acceptance has not been disabled. > > > > > > To make use of this event group, we add a new driver that is meant to > > > carry behavior that is needed for all confidential compute technologies, > > > not just specific platforms, CocoDxe. In CocoDxe we implement the > > > default safe behavior to accept all unaccepted memory and invalidate > > > the MemoryMap on ExitBootServices. > > > > > > To allow the OS loader to prevent the eager acceptance, we add a new > > > protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol. > > > This protocol has one interface, Disable(). The OS loader can inform the > > > UEFI that it supports the unaccepted memory type and accepts the > > > responsibility to accept it. > > > > > > All images that support unaccepted memory must now locate and call > > this > > > new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call > > the Disable > > > function. > > > > > > Changes since v7: > > > - Rebased onto lazy accept v4 patch series, so memory accept protocol > > > has the EDKII prefix, and the unaccepted memory type has the BZ3937 > > > prefix. > > > - Removed a bad #include to a header removed in v7. > > > - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL > > as per the > > > discussion on the buganizer issue. > > > - Uncrustify formatting > > > > > > Changes since v6: > > > - Added implementation of > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. > > > - Changed callback protocol of v5 to instead use the standardized event > > > group for before_exit_boot_services. > > > > > > Changes since v5: > > > - Generic callback protocol moved to MdeModulePkg > > > - Removed use of EFI_WARN_STALE_DATA and added comment that the > > callback > > > should only return EFI_SUCCESS or EFI_INVALID_PARAMETER. > > > - Removed errant log statement and fixed formatting. > > > > > > Changes since v4: > > > - Commit message wording > > > - Replaced direct change to DxeMain with a more generic callback > > > protocol. > > > - Implemented the direct change as an instance of the callback protocol > > > from a new CocoDxe driver. > >
Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior
> > btw it still fails in CoreTerminateMemoryMap() with the current upstream > kernel which is not aware of the lazy memory acceptance, is this > something known? Thanks, > It wasn't known. I'll take a look. Thanks for the report. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95648): https://edk2.groups.io/g/devel/message/95648 Mute This Topic: https://groups.io/mt/94144524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior
On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy wrote: > > Hi Dionna, > > Thanks for updating the tree, builds nicely now! However the VM's kernel > does not boot - the guest kernel reports > > EFI stub: ERROR: exit_boot() failed! > > and hangs. I am not quite sure how it is supposed to work (still > learning) but "Accepting all memory" happens twice (should it?) and the > actual reason for the CoreExitBootService() failure is that MapKey != > mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9 > or 0x7AE1 vs 0x7AE3 (the diff is always 2). > "Accepting all memory" may happen twice, but it's idempotent. The debug_info log happening twice might be confusing, so I can change that if you'd like. The first accept will remove all unaccepted memory regions from the address space map. CoreExitBootService should fail the first time since the first accept will change the memory map. That failure means that the caller should GetMemoryMap again and try CoreExitBootService again. > How do you test it exactly, is there any command line change needed in > addition to enabling SNP? > > My guest kernel uses > https://patchew.org/linux/cover.1664298261.git.thomas.lenda...@amd.com/ > with the TDX prerequisite. Thanks, > It's a few name changes behind, but this branch of Linux is what I've been using: https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum Specific enablement patch here https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61 I incorporate Kirill's patch set v7 for basic unaccepted memory support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted memory, and I have a single patch that calls the protocol. This branch doesn't have Kirill's TDX patches. I've run it with a regular SEV-SNP enabled guest kernel too. At this point all the tests have used kernel injection rather than having the kernels all baked into the image. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95578): https://edk2.groups.io/g/devel/message/95578 Mute This Topic: https://groups.io/mt/94144524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
Location of notification is has been specified in UEFI v2.9.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Cc: Ray Ni
Signed-off-by: Dionna Glaze
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++
2 files changed, 7 insertions(+)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index e4bca89577..35d5bf0dee 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -100,6 +100,7 @@
gEfiEventVirtualAddressChangeGuid ## CONSUMES ##
Event
## CONSUMES ## Event
## PRODUCES ## Event
+ gEfiEventBeforeExitBootServicesGuid
gEfiEventExitBootServicesGuid
gEfiHobMemoryAllocModuleGuid ## SOMETIMES_CONSUMES ## HOB
gEfiFirmwareFileSystem2Guid ## CONSUMES ##
GUID # Used to compare with FV's file system guid and get the FV's file system
format
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 5733f0c8ec..4683016ed7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -763,6 +763,12 @@ CoreExitBootServices (
{
EFI_STATUS Status;
+ //
+ // Notify other drivers of their last chance to use boot services
+ // before the memory map is terminated.
+ //
+ CoreNotifySignalList ();
+
//
// Disable Timer
//
--
2.38.0.135.g90850a2211-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95532): https://edk2.groups.io/g/devel/message/95532
Mute This Topic: https://groups.io/mt/94544540/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v8 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
Instead of eagerly accepting all memory in PEI, only accept memory under
the 4GB address. This allows a loaded image to use the
MEMORY_ACCEPTANCE_PROTOCOL to disable the accept behavior and indicate
that it can interpret the memory type accordingly.
This classification is safe since ExitBootServices will accept and
reclassify the memory as conventional if the disable protocol is not
used.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Signed-off-by: Dionna Glaze
---
OvmfPkg/PlatformPei/AmdSev.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 385562b44c..4cb6da4437 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -63,6 +64,10 @@ AmdSevSnpInitialize (
for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB
(Hob)) {
if ((Hob.Raw != NULL) && (GET_HOB_TYPE (Hob) ==
EFI_HOB_TYPE_RESOURCE_DESCRIPTOR)) {
ResourceHob = Hob.ResourceDescriptor;
+ if (ResourceHob->PhysicalStart >= SIZE_4GB) {
+ResourceHob->ResourceType = BZ3937_RESOURCE_MEMORY_UNACCEPTED;
+continue;
+ }
if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
MemEncryptSevSnpPreValidateSystemRam (
--
2.38.0.135.g90850a2211-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95531): https://edk2.groups.io/g/devel/message/95531
Mute This Topic: https://groups.io/mt/94544539/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v8 6/7] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
This protocol implementation disables the accept-all-memory behavior
of the BeforeExitBootServices event this driver adds.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/CocoDxe/CocoDxe.c | 28
OvmfPkg/CocoDxe/CocoDxe.inf | 1 +
2 files changed, 29 insertions(+)
diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
index 98874e6cfc..14fbcf60d7 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.c
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -17,11 +17,14 @@
#include
#include
#include
+#include
STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
+STATIC EFI_HANDLE mCocoDxeHandle = NULL;
+
STATIC
EFI_STATUS
AcceptAllMemory (
@@ -110,6 +113,21 @@ ResolveUnacceptedMemory (
ASSERT_EFI_ERROR (Status);
}
+STATIC
+EFI_STATUS
+EFIAPI
+AllowUnacceptedMemory (
+ IN BZ3987_MEMORY_ACCEPTANCE_PROTOCOL *This
+ )
+{
+ mAcceptAllMemoryAtEBS = FALSE;
+ return EFI_SUCCESS;
+}
+
+STATIC
+BZ3987_MEMORY_ACCEPTANCE_PROTOCOL
+ mMemoryAcceptanceProtocol = { AllowUnacceptedMemory };
+
EFI_STATUS
EFIAPI
CocoDxeEntryPoint (
@@ -142,5 +160,15 @@ CocoDxeEntryPoint (
DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for
EventBeforeExitBootServices failed.\n"));
}
+ Status = gBS->InstallProtocolInterface (
+ ,
+ ,
+ EFI_NATIVE_INTERFACE,
+
+ );
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Install Bz3987MemoryAcceptanceProtocol failed.\n"));
+ }
+
return EFI_SUCCESS;
}
diff --git a/OvmfPkg/CocoDxe/CocoDxe.inf b/OvmfPkg/CocoDxe/CocoDxe.inf
index 8d4452e94d..05c2651a89 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.inf
+++ b/OvmfPkg/CocoDxe/CocoDxe.inf
@@ -42,4 +42,5 @@
gEfiEventBeforeExitBootServicesGuid
[Protocols]
+ gBz3987MemoryAcceptanceProtocolGuid
gEdkiiMemoryAcceptProtocolGuid
--
2.38.0.135.g90850a2211-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95530): https://edk2.groups.io/g/devel/message/95530
Mute This Topic: https://groups.io/mt/94544537/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v8 5/7] MdePkg: Introduce the MemoryAcceptance protocol
The default behavior for unaccepted memory is to accept all memory
when ExitBootServices is called. An OS loader can use this protocol to
disable this behavior to assume responsibility for memory acceptance and
to affirm that the OS can handle the unaccepted memory type.
This is a candidate for standardization.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
MdePkg/Include/Protocol/MemoryAcceptance.h | 40
MdePkg/MdePkg.dec | 3 ++
2 files changed, 43 insertions(+)
diff --git a/MdePkg/Include/Protocol/MemoryAcceptance.h
b/MdePkg/Include/Protocol/MemoryAcceptance.h
new file mode 100644
index 00..0b305b016f
--- /dev/null
+++ b/MdePkg/Include/Protocol/MemoryAcceptance.h
@@ -0,0 +1,40 @@
+/** @file
+ The file provides the protocol that disables the behavior that all memory
+ gets accepted at ExitBootServices(). This protocol is only meant to be called
+ by the OS loader, and not EDK2 itself.
+
+ Copyright (c) 2022, Google LLC. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MEMORY_ACCEPTANCE_H_
+#define MEMORY_ACCEPTANCE_H_
+
+#define BZ3987_MEMORY_ACCEPTANCE_PROTOCOL_GUID \
+ {0xc5a010fe, \
+ 0x38a7, \
+ 0x4531, \
+ {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
+
+typedef struct _BZ3987_MEMORY_ACCEPTANCE_PROTOCOL
BZ3987_MEMORY_ACCEPTANCE_PROTOCOL;
+
+/**
+ @param This A pointer to a BZ3987_MEMORY_ACCEPTANCE_PROTOCOL.
+**/
+typedef
+ EFI_STATUS
+(EFIAPI *BZ3987_ALLOW_UNACCEPTED_MEMORY)(
+ IN BZ3987_MEMORY_ACCEPTANCE_PROTOCOL *This
+ );
+
+///
+/// The BZ3987_MEMORY_ACCEPTANCE_PROTOCOL allows the OS loader to
+/// indicate to EDK2 that ExitBootServices should not accept all memory.
+///
+struct _BZ3987_MEMORY_ACCEPTANCE_PROTOCOL {
+ BZ3987_ALLOW_UNACCEPTED_MEMORYAllowUnacceptedMemory;
+};
+
+extern EFI_GUID gBz3987MemoryAcceptanceProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 359a85ea10..5c639c1b98 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1022,6 +1022,9 @@
gEfiPeiDelayedDispatchPpiGuid = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e,
0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}
[Protocols]
+ ## Include/Protocol/Bz3987MemoryAcceptance.h
+ gBz3987MemoryAcceptanceProtocolGuid = { 0xc5a010fe, 0x38a7, 0x4531, {0x8a,
0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49 }}
+
## Include/Protocol/MemoryAccept.h
gEdkiiMemoryAcceptProtocolGuid = { 0x38c74800, 0x5590, 0x4db4, { 0xa0, 0xf3,
0x67, 0x5d, 0x9b, 0x8e, 0x80, 0x26 }}
--
2.38.0.135.g90850a2211-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95529): https://edk2.groups.io/g/devel/message/95529
Mute This Topic: https://groups.io/mt/94544535/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v8 4/7] OvmfPkg: Introduce CocoDxe driver
This driver is meant as a join point for all Confidential Compute
technologies to put shared behavior that doesn't belong anywhere else.
The first behavior added here is to accept all unaccepted memory at
ExitBootServices if the behavior is not disabled. This allows safe
upgrades for OS loaders to affirm their support for the unaccepted
memory type.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Ard Biesheuvel
Cc: "Min M. Xu"
Cc: Andrew Fish
Cc: "Michael D. Kinney"
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
OvmfPkg/CocoDxe/CocoDxe.c| 146
OvmfPkg/CocoDxe/CocoDxe.inf | 45 ++
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfPkgX64.fdf | 1 +
10 files changed, 199 insertions(+)
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 90e8a213ef..ad6b73ca4a 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -747,6 +747,7 @@
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
}
+ OvmfPkg/CocoDxe/CocoDxe.inf
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
#
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 4658e1d30e..3717ec9094 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -302,6 +302,7 @@ INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
new file mode 100644
index 00..98874e6cfc
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -0,0 +1,146 @@
+/** @file
+
+ Confidential Compute Dxe driver. This driver installs protocols that are
+ generic over confidential compute techonology.
+
+ Copyright (c) 2022, Google LLC. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
+
+STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
+
+STATIC
+EFI_STATUS
+AcceptAllMemory (
+ IN EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory
+ )
+{
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
+ UINTNNumEntries;
+ UINTNIndex;
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
+
+ /*
+ * Get a copy of the memory space map to iterate over while
+ * changing the map.
+ */
+ Status = gDS->GetMemorySpaceMap (, );
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ for (Index = 0; Index < NumEntries; Index++) {
+CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc;
+
+Desc = [Index];
+if (Desc->GcdMemoryType != EfiGcdMemoryTypeUnaccepted) {
+ continue;
+}
+
+Status = AcceptMemory->AcceptMemory (
+ AcceptMemory,
+ Desc->BaseAddress,
+ Desc->Length
+ );
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->RemoveMemorySpace (Desc->BaseAddress, Desc->Length);
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = gDS->AddMemorySpace (
+EfiGcdMemoryTypeSystemMemory,
+Desc->BaseAddress,
+Desc->Length,
+EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO |
EFI_MEMORY_RP
+);
+if (EFI_ERROR (Status)) {
+ break;
+}
+ }
+
+ gBS->FreePool (AllDescMap);
+ return Status;
+}
+
+VOID
+EFIAPI
+ResolveUnacceptedMemory (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory;
+ EFI_STATUSStatus;
+
+ if (!mAcceptAllMemoryAtEBS) {
+return;
+ }
+
+ Status = gBS->LocateProtocol (
+ ,
+ NULL,
+ (VOID **)
+ );
+ if (Status == EFI_NOT_FOUND) {
+return;
+ }
+
+ ASSERT_EFI_ERROR (Status);
+
+ Status = AcceptAllMemory (AcceptMemory);
+ ASSERT_EFI_ERROR (Status);
+}
+
+EFI_STATUS
+EFIAPI
+CocoDxeEntryPoint (
+ IN EFI_HANDLEImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Do nothing when confidential compute technologies that require memory
+ // acceptance are not enabled.
+ //
+ if (!MemEncryptSevSnpIsEnabled () &&
+ !MemEncryptTdxIsEnabled ())
+ {
+return EFI_UNSUPPORTED;
+ }
+
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+
[edk2-devel] [PATCH v8 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
Event group as defined in UEFI standard v2.9.
Cc: Ard Biescheuvel
Cc: "Min M. Xu"
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Tom Lendacky
Cc: Jiewen Yao
Cc: Erdem Aktas
Signed-off-by: Dionna Glaze
---
MdePkg/Include/Guid/EventGroup.h | 5 +
MdePkg/MdePkg.dec| 5 -
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Guid/EventGroup.h b/MdePkg/Include/Guid/EventGroup.h
index 063d1f7157..64bfd4bab9 100644
--- a/MdePkg/Include/Guid/EventGroup.h
+++ b/MdePkg/Include/Guid/EventGroup.h
@@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
extern EFI_GUID gEfiEventExitBootServicesGuid;
+#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
+ { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b,
0xfc } }
+
+extern EFI_GUID gEfiEventBeforeExitBootServicesGuid;
+
#define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
{ 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51,
0x96 } }
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6b6bfbec29..359a85ea10 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -408,7 +408,10 @@
gEfiEventMemoryMapChangeGuid = { 0x78BEE926, 0x692F, 0x48FD, { 0x9E, 0xDB,
0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
## Include/Guid/EventGroup.h
- gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87,
0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+ gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87,
0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+
+ ## Include/Guid/EventGroup.h
+ gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44, { 0x80,
0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
## Include/Guid/EventGroup.h
gEfiEventExitBootServicesGuid = { 0x27ABF055, 0xB1B8, 0x4C26, { 0x80, 0x48,
0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
--
2.38.0.135.g90850a2211-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95527): https://edk2.groups.io/g/devel/message/95527
Mute This Topic: https://groups.io/mt/94544532/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
From: Sophia Wolf
When a guest OS does not support unaccepted memory, the unaccepted
memory must be accepted before returning a memory map to the caller.
EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.
Cc: Gerd Hoffmann
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Signed-off-by: Dionna Glaze
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55
++--
OvmfPkg/AmdSevDxe/AmdSevDxe.inf| 3 ++
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24
+++--
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 662d3c4ccb..f7600c3c81 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
SIGNATURE_32 ('A','M', 'D', 'E'),
@@ -31,6 +32,40 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
mSnpBootDxeTable = {
FixedPcdGet32 (PcdOvmfCpuidSize),
};
+STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+
+#define IS_ALIGNED(x, y) x) & ((y) - 1)) == 0))
+
+STATIC
+EFI_STATUS
+EFIAPI
+AmdSevMemoryAccept (
+ IN EDKII_MEMORY_ACCEPT_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS StartAddress,
+ IN UINTN Size
+ )
+{
+ //
+ // The StartAddress must be page-aligned, and the Size must be a positive
+ // multiple of SIZE_4KB. Use an assert instead of returning an erros since
+ // this is an EDK2-internal protocol.
+ //
+ ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
+ ASSERT (IS_ALIGNED (Size, SIZE_4KB));
+ ASSERT (Size != 0);
+
+ MemEncryptSevSnpPreValidateSystemRam (
+StartAddress,
+EFI_SIZE_TO_PAGES (Size)
+);
+
+ return EFI_SUCCESS;
+}
+
+STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
+ AmdSevMemoryAccept
+};
+
EFI_STATUS
EFIAPI
AmdSevDxeEntryPoint (
@@ -147,11 +182,23 @@ AmdSevDxeEntryPoint (
}
}
- //
- // If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
- // It contains the location for both the Secrets and CPUID page.
- //
if (MemEncryptSevSnpIsEnabled ()) {
+//
+// Memory acceptance began being required in SEV-SNP, so install the
+// memory accept protocol implementation for a SEV-SNP active guest.
+//
+Status = gBS->InstallProtocolInterface (
+,
+,
+EFI_NATIVE_INTERFACE,
+
+);
+ASSERT_EFI_ERROR (Status);
+
+//
+// If its SEV-SNP active guest then install the
CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
+// It contains the location for both the Secrets and CPUID page.
+//
return gBS->InstallConfigurationTable (
,
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 9acf860cf2..cd1b686c53 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -47,6 +47,9 @@
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+[Protocols]
+ gEdkiiMemoryAcceptProtocolGuid
+
[Guids]
gConfidentialComputingSevSnpBlobGuid
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index d3a95e4913..cbcdd46f52 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -14,6 +14,7 @@
#include
#include "SnpPageStateChange.h"
+#include "VirtualMemory.h"
/**
Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
@@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam (
IN UINTN NumPages
)
{
+ EFI_STATUS Status;
+
if (!MemEncryptSevSnpIsEnabled ()) {
return;
}
- //
- // All the pre-validation must be completed in the PEI phase.
- //
- ASSERT (FALSE);
+ // DXE pre-validation may happen with the memory accept protocol.
+ // The protocol should only be called outside the prevalidated ranges
+ // that the PEI stage code explicitly skips. Specifically, only memory
+ // ranges that are classified as unaccepted.
+ if (BaseAddress >= SIZE_4GB) {
+Status = InternalMemEncryptSevCreateIdentityMap1G (
+ 0,
+ BaseAddress,
+ EFI_PAGES_TO_SIZE (NumPages)
+ );
+if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
+ }
+
+ InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
}
--
2.38.0.135.g90850a2211-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95526):
[edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
These seven patches build on the lazy-accept patch series "Introduce Lazy-accept for Tdx guest" by adding SEV-SNP support for the MemoryAccept protocol, and importantly making eager memory acceptance the default behavior. We implement a standardized event group from UEFI v2.9, EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides exactly the right invocation point for eagerly accepting memory if eager acceptance has not been disabled. To make use of this event group, we add a new driver that is meant to carry behavior that is needed for all confidential compute technologies, not just specific platforms, CocoDxe. In CocoDxe we implement the default safe behavior to accept all unaccepted memory and invalidate the MemoryMap on ExitBootServices. To allow the OS loader to prevent the eager acceptance, we add a new protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol. This protocol has one interface, Disable(). The OS loader can inform the UEFI that it supports the unaccepted memory type and accepts the responsibility to accept it. All images that support unaccepted memory must now locate and call this new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable function. Changes since v7: - Rebased onto lazy accept v4 patch series, so memory accept protocol has the EDKII prefix, and the unaccepted memory type has the BZ3937 prefix. - Removed a bad #include to a header removed in v7. - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL as per the discussion on the buganizer issue. - Uncrustify formatting Changes since v6: - Added implementation of EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES. - Changed callback protocol of v5 to instead use the standardized event group for before_exit_boot_services. Changes since v5: - Generic callback protocol moved to MdeModulePkg - Removed use of EFI_WARN_STALE_DATA and added comment that the callback should only return EFI_SUCCESS or EFI_INVALID_PARAMETER. - Removed errant log statement and fixed formatting. Changes since v4: - Commit message wording - Replaced direct change to DxeMain with a more generic callback protocol. - Implemented the direct change as an instance of the callback protocol from a new CocoDxe driver. - Replaced "enable" protocol with a "disable" protocol, since the name was confusing. The AcceptAllUnacceptedMemory protocol directly names the behavior that is disabling. Changes since v3: - "DxeMain accepts all memory" patch split into 3 to make each patch affect only one package at a time. Changes since v2: - Removed the redundant memory accept interface and added the accept behavior to the DXE implementation of MemEncryptSevSnpPreValidateSystemRam. - Fixed missing #include in >=4GB patch. Changes since v1: - Added a patch to classify SEV-SNP memory above 4GB unaccepted. - Fixed style problems in EfiMemoryAcceptProtocol implementation. Cc: Ard Biescheuvel Cc: "Min M. Xu" Cc: Gerd Hoffmann Cc: James Bottomley Cc: Tom Lendacky Cc: Jiewen Yao Cc: Erdem Aktas Cc: Andrew Fish Cc: "Michael D. Kinney" Signed-off-by: Dionna Glaze Dionna Glaze (7): OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices OvmfPkg: Introduce CocoDxe driver MdePkg: Introduce the MemoryAcceptance protocol OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c| 6 + MdePkg/Include/Guid/EventGroup.h | 5 + MdePkg/Include/Protocol/MemoryAcceptance.h | 40 + MdePkg/MdePkg.dec | 8 +- OvmfPkg/AmdSev/AmdSevX64.dsc | 1 + OvmfPkg/AmdSev/AmdSevX64.fdf | 1 + OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++- OvmfPkg/AmdSevDxe/AmdSevDxe.inf| 3 + OvmfPkg/CocoDxe/CocoDxe.c | 174 OvmfPkg/CocoDxe/CocoDxe.inf| 46 ++ OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 + OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 + OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 ++- OvmfPkg/OvmfPkgIa32X64.dsc | 1 + OvmfPkg/OvmfPkgIa32X64.fdf | 1 + OvmfPkg/OvmfPkgX64.dsc | 1 + OvmfPkg/OvmfPkgX64.fdf | 1 + OvmfPkg/PlatformPei/AmdSev.c
Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior
> > Hey! > > Sorry if this was asked. I was wondering if this patchset is in some git repo > which I pull from as I struggle to get a buildable tree by applying this > patchset to whatever I can find. > > I tried https://github.com/deeglaze/edk2.git enable_umv7 but it does not > build (missing ExitBootServicesCallback.h and PrePiHob.h). > I tried rebasing it on top of https://github.com/mxu9/edk2.git > lazyaccept.v1/2/3/4 but it seems only v1 kinda works but see the above and > other versions have some reworks (EFI_RESOURCE_MEMORY_UNACCEPTED vs. > BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED. > > Thanks! Oh that would be because I forgot to git add those files (it built on my machine TM). The callback.h file is from a previous version of this series that can be removed. PrePiHob.h is from a lazy accept patch, "MdePkg: Add UEFI Unaccepted memory definition". I can send out a v8 that removes the unnecessary #include and changes the names to those suggested in the 3987 bug. I'll fix my github branch to include the missing file. Thanks for trying this patch series :) -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95505): https://edk2.groups.io/g/devel/message/95505 Mute This Topic: https://groups.io/mt/94144524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [PATCH V4 00/10] Introduce Lazy-accept for Tdx guest
> > Min: > > I understand that they are for the different purpose and usage. But, their > > protocol name are very similar. > Yes. They do look very similar. > > > If there is no better protocol name, I will also be fine. > Dionna, what's your thought? > The accept_all_unaccepted_memory name came from Ard's suggestion on an earlier patch series where I was using a Pcd. We're talking about the name over on the spec bug https://bugzilla.tianocore.org/show_bug.cgi?id=3987#c26 I think that any name concerning unaccepted memory is going to look a little same-y to others, so it's going to be troublesome if we require that they all strongly differ in name. I'm not strongly attached to the name, I just want to be conservative about what the name implies about the expected overall behavior of the system when that has not actually been decided upon. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95473): https://edk2.groups.io/g/devel/message/95473 Mute This Topic: https://groups.io/mt/94423128/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior
> > I suppose that's true. Would you prefer volatile versions of > > OsIndications/OsIndicationsSupported added to the spec, or this > > proposed one-off toggle protocol? No specified global variables seem > > overloadable with this duty. > > > > No it would have to be a completely separate variable, I don't think > we can use OsIndications for this. Certainly, I meant new variables that are volatile and also play a similar role as OsIndications[Supported]. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95472): https://edk2.groups.io/g/devel/message/95472 Mute This Topic: https://groups.io/mt/94144524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior
> That assumes the variable is non-volatile, but I suppose we could use > a volatile variable [other than OsIndications] as well. > I suppose that's true. Would you prefer volatile versions of OsIndications/OsIndicationsSupported added to the spec, or this proposed one-off toggle protocol? No specified global variables seem overloadable with this duty. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95452): https://edk2.groups.io/g/devel/message/95452 Mute This Topic: https://groups.io/mt/94144524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior
> > Can OsIndicationsSupported UEFI variable provide the similar functionality? > Similar, but not the same. If we use a UEFI variable, its value will persist across boots. This can be fine if you only boot one OS, but if you have two where one supports unaccepted memory and the other doesn't then you have a problem. The protocol here is specific to the code that will be calling ExitBootServices, and thus won't mess up another OS once we reboot and select it. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95251): https://edk2.groups.io/g/devel/message/95251 Mute This Topic: https://groups.io/mt/94144524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms PATCH v1 0/1] BoardModulePkg: Copy device path
Hi Abdul, I'm not a maintainer, just a contributor. I thought I'd pop in with an observation though. I'm not seeing any diffs in these emails. I'm not going to click a suspicious-looking link either. Likely other folks have the same reservation. Have you read the contributor guidelines on how to send in patches? https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process On Thu, Oct 13, 2022 at 8:12 AM Attar, AbdulLateef (Abdul Lateef) via groups.io wrote: > > [Public] > > Review please > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Attar, > AbdulLateef (Abdul Lateef) via groups.io > Sent: 16 September 2022 12:58 > To: devel@edk2.groups.io; Eric Dong ; Liming Gao > > Subject: Re: [edk2-devel] [edk2-platforms PATCH v1 0/1] BoardModulePkg: Copy > device path > > [Public] > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > [Public] > > Hello Maintainers, Reviewers, > Could you please review the patch? > Thanks > AbduL > > -Original Message- > From: Attar, AbdulLateef (Abdul Lateef) > Sent: 13 July 2022 09:49 > To: 'devel@edk2.groups.io' ; 'Eric Dong' > ; 'Liming Gao' > Subject: RE: [edk2-devel] [edk2-platforms PATCH v1 0/1] BoardModulePkg: Copy > device path > > Maintainers, Reviewers, > please review the patch. > Thanks > AbduL > > > -Original Message- > From: Attar, AbdulLateef (Abdul Lateef) > Sent: 25 April 2022 12:45 > To: devel@edk2.groups.io; Eric Dong ; Liming Gao > > Subject: RE: [edk2-devel] [edk2-platforms PATCH v1 0/1] BoardModulePkg: Copy > device path > > Hi, >Please review the patch. > Thanks > AbduL > > -Original Message- > From: Attar, AbdulLateef (Abdul Lateef) > Sent: 05 April 2022 11:25 > To: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) > ; Eric Dong ; Liming Gao > > Subject: RE: [edk2-devel] [edk2-platforms PATCH v1 0/1] BoardModulePkg: Copy > device path > > [Public] > > Hi, >Could you please review the patch? > Thanks > AbduL > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Abdul Lateef > Attar via groups.io > Sent: 15 March 2022 10:16 > To: devel@edk2.groups.io > Cc: Eric Dong ; Liming Gao > Subject: [edk2-devel] [edk2-platforms PATCH v1 0/1] BoardModulePkg: Copy > device path > > [CAUTION: External Email] > > (Resending patch with [edk2-platforms] prefix with correct emailid). > GCC compiler puts the DevicePath PCDs to the read-only section. During boot > if try to process the device path after PtrGetPtr it throws a page fault > exception. > > Hence making a local copy using DuplicateDevicePath() to avoid the page fault > exception. > > REF : > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fabdattar%2Fedk2-platforms%2Ftree%2FBoardModulePkgdata=05%7C01%7CAbdulLateef.Attar%40amd.com%7C8ed1e10f30a44c51ed8808da97e873c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637989321879784020%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=GL91Bvyavwj0eK%2F81WXVp7LDVSu2MHJudT%2BdNIa99jU%3Dreserved=0 > > Cc: Eric Dong > Cc: Liming Gao > > Abdul Lateef Attar (1): > BoardModulePkg: Copy device path before processing > > Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c | 19 > ++- > 1 file changed, 14 insertions(+), 5 deletions(-) > > -- > 2.25.1 > > > > > > > > > > > > > > > > > -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95160): https://edk2.groups.io/g/devel/message/95160 Mute This Topic: https://groups.io/mt/89792070/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [PATCH V4 00/10] Introduce Lazy-accept for Tdx guest
> The name of EDKII_MEMORY_ACCEPT_PROTOCOL indicates it is only used in edk2. Ah, yes I was basing my changes off probably a very old version of TDVF's patches that used the EFI_ naming convention, so folks looking at my branch might have been expecting that it'd be standardized. Cool. Just the one thing needed then. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95109): https://edk2.groups.io/g/devel/message/95109 Mute This Topic: https://groups.io/mt/94229046/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
