Date   

Event: Tools, CI, Code base construction meeting series - 10/03/2022 #cal-reminder

Group Notification <noreply@...>
 

Reminder: Tools, CI, Code base construction meeting series

When:
10/03/2022
4:30pm to 5:30pm
(UTC-07:00) America/Los Angeles

Where:
https://github.com/tianocore/edk2/discussions/2614

View Event

Description:

TianoCore community,

Microsoft and Intel will be hosting a series of open meetings to discuss build, CI, tools, and other related topics. If you are interested, have ideas/opinions please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft Teams.

MS Teams Link in following discussion: * https://github.com/tianocore/edk2/discussions/2614

Anyone is welcome to join.

MS Teams Browser Clients * https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client


Re: [edk2-platforms PATCH 0/2] Platform/RaspberryPi: SyncPcie() fixes

Jeremy Linton
 

Hi,

On 9/30/22 13:47, Adrien Thierry wrote:
Hi Jeremy,

If you just add the range tweak, does that fix the XHCI config in your setup
too?
I tested applying the range tweak in your patch, unfortunately it doesn't
seem to work on my setup. I'm still getting "usb 1-1: device descriptor > read/64, error -71" errors.
That is unfortunate. Which revision/how much RAM? Can you paste the before/after kernel/pci output like:

] brcm-pcie fd500000.pcie: host bridge /scb/pcie@7d500000 ranges:
] brcm-pcie fd500000.pcie: No bus range found for /scb/pcie@7d500000, using [bus 00-ff]
] brcm-pcie fd500000.pcie: MEM 0x0600000000..0x0603ffffff -> 0x00f8000000
] brcm-pcie fd500000.pcie: IB MEM 0x0000000000..0x00bfffffff -> 0x0000000000
] brcm-pcie fd500000.pcie: PCI host bridge to bus 0000:00
] pci_bus 0000:00: root bus resource [bus 00-ff]
] pci_bus 0000:00: root bus resource [mem 0x600000000-0x603ffffff] (bus address [0xf8000000-0xfbffffff])

I tested it on a C0 with 8G and a B0 with 4G, and it seems to work on both, although the C0 might have been misbehaving a bit (likely unrelated). In both cases I swapped in the latest Pi foundation DT (https://github.com/raspberrypi/firmware/blob/master/boot/bcm2711-rpi-4-b.dtb) which uses the address you suggest, and it still seems to work either way (with or without the reset). If we reroll the PFTF firmware it is usually with the latest pi foundation DTs.

So the first patch makes sense, but I think it should probably be checking both DT property names (pci0,0 and pci1,0) since we probably want maximum compatibility with differing DT's the user might swap in, and the upstream linux change which changes the node from 1,0 to 0,0 is from august of last year, so not old at all..

The second one, I'm less sure about, since the primary thing your changing (AFAIK) is whether the XHCI BIOS handoff is being checked, and since EDK2 supports the hand-off and Linux doesn't throw the XHCI bios failure message I think we actually want to leave that in place. If you have debugging turned on, the XHCI/LegacyBios ownership control is the message you see during exit boot services that says "XhciClearBiosOwnership: called to clear BIOS ownership". I suspect the kernel patch is yet another case of "UBOOT and/or the pi foundation firmware is broken so we fixed the kernel"

Do you see a difference with/without the second patch?




Here's my SyncPcie function with the range tweak applied [1]
I'm running upstream linux 6.0-rc6 with the downstream device tree
provided in [2]
Thank you,
Adrien
[1] http://pastebin.test.redhat.com/1075875
[2] https://github.com/pftf/RPi4/releases/tag/v1.33


[PATCH v6 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

Dionna Glaze
 

Instead of eagerly accepting all memory in PEI, only accept memory under
the 4GB address. This allows a loaded image to use the
ACCEPT_ALL_UNACCEPTED_MEMORY_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 <ardb@...>
Cc: "Min M. Xu" <min.m.xu@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Tom Lendacky <Thomas.Lendacky@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Erdem Aktas <erdemaktas@...>

Signed-off-by: Dionna Glaze <dionnaglaze@...>
---
OvmfPkg/PlatformPei/AmdSev.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 385562b44c..2a52d6f491 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -16,6 +16,7 @@
#include <Library/MemEncryptSevLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/PcdLib.h>
+#include <Pi/PrePiHob.h>
#include <PiPei.h>
#include <Register/Amd/Msr.h>
#include <Register/Intel/SmramSaveStateMap.h>
@@ -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 = EFI_RESOURCE_MEMORY_UNACCEPTED;
+ continue;
+ }

if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
MemEncryptSevSnpPreValidateSystemRam (
--
2.38.0.rc1.362.ged0d419d3c-goog


[PATCH v6 6/7] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe

Dionna Glaze
 

This protocol implementation disables the accept-all-memory behavior
of the ExitBootServicesCallback instance thise driver adds.

Cc: Gerd Hoffmann <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Ard Biesheuvel <ardb@...>
Cc: "Min M. Xu" <min.m.xu@...>
Cc: Andrew Fish <afish@...>
Cc: "Michael D. Kinney" <michael.d.kinney@...>

Signed-off-by: Dionna Glaze <dionnaglaze@...>
---
OvmfPkg/CocoDxe/CocoDxe.c | 25 ++++++++++++++++++++
OvmfPkg/CocoDxe/CocoDxe.inf | 1 +
2 files changed, 26 insertions(+)

diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
index 9e9a405af1..abf2d2b055 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.c
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -16,6 +16,7 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/MemEncryptSevLib.h>
#include <Library/MemEncryptTdxLib.h>
+#include <Protocol/Bz3987AcceptAllUnacceptedMemory.h>
#include <Protocol/ExitBootServicesCallback.h>
#include <Protocol/MemoryAccept.h>

@@ -113,6 +114,21 @@ STATIC EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL mExitBootServicesCallbackProco
FALSE,
};

+STATIC
+EFI_STATUS
+EFIAPI
+DisableAcceptAllUnacceptedMemory (
+ IN BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL *This
+ )
+{
+ mExitBootServicesCallbackProcotol.Disabled = TRUE;
+ return EFI_SUCCESS;
+}
+
+STATIC
+BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL
+mAcceptAllUnacceptedMemoryProtocol = {DisableAcceptAllUnacceptedMemory};
+
EFI_STATUS
EFIAPI
CocoDxeEntryPoint (
@@ -140,5 +156,14 @@ CocoDxeEntryPoint (
DEBUG ((DEBUG_ERROR, "Install EdkiiExitBootServicesCallbackProtocol failed.\n"));
}

+ Status = gBS->InstallProtocolInterface (&mCocoDxeHandle,
+ &gBz3987AcceptAllUnacceptedMemoryProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mAcceptAllUnacceptedMemoryProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Install Bz3987AcceptAllUnacceptedMemoryProtocol failed.\n"));
+ }
+
return EFI_SUCCESS;
}
diff --git a/OvmfPkg/CocoDxe/CocoDxe.inf b/OvmfPkg/CocoDxe/CocoDxe.inf
index 3ff2a6fade..96ab3e1c68 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.inf
+++ b/OvmfPkg/CocoDxe/CocoDxe.inf
@@ -39,5 +39,6 @@
TRUE

[Protocols]
+ gBz3987AcceptAllUnacceptedMemoryProtocolGuid
gEdkiiExitBootServicesCallbackProtocolGuid
gEfiMemoryAcceptProtocolGuid
--
2.38.0.rc1.362.ged0d419d3c-goog


[PATCH v6 5/7] MdePkg: Introduce the AcceptAllUnacceptedMemory protocol

Dionna Glaze
 

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 <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Ard Biesheuvel <ardb@...>
Cc: "Min M. Xu" <min.m.xu@...>
Cc: Andrew Fish <afish@...>
Cc: "Michael D. Kinney" <michael.d.kinney@...>

Signed-off-by: Dionna Glaze <dionnaglaze@...>
---
MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h | 40 ++++++++++++++++++++
MdePkg/MdePkg.dec | 3 ++
2 files changed, 43 insertions(+)

diff --git a/MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h b/MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
new file mode 100644
index 0000000000..e50831836c
--- /dev/null
+++ b/MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.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.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#ifndef _ACCEPT_ALL_UNACCEPTED_MEMORY_H_
+#define _ACCEPT_ALL_UNACCEPTED_MEMORY_H_
+
+#define BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL_GUID \
+ {0xc5a010fe, \
+ 0x38a7, \
+ 0x4531, \
+ {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
+
+typedef struct _BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL
+ BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL;
+
+/**
+ @param This A pointer to a BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *BZ3987_DISABLE_ACCEPT_ALL_UNACCEPTED_MEMORY)(
+ IN BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL *This
+ );
+
+///
+/// The BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL allows the OS loader to
+/// indicate to EDK2 that ExitBootServices should not accept all memory.
+///
+struct _BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL {
+ BZ3987_DISABLE_ACCEPT_ALL_UNACCEPTED_MEMORY Disable;
+};
+
+extern EFI_GUID gBz3987AcceptAllUnacceptedMemoryProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index de3c56758b..b6b108586e 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1019,6 +1019,9 @@
gEfiPeiDelayedDispatchPpiGuid = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e, 0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}

[Protocols]
+ ## Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
+ gBz3987AcceptAllUnacceptedMemoryProtocolGuid = { 0xc5a010fe, 0x38a7, 0x4531, {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49 }}
+
## Include/Protocol/MemoryAccept.h
gEfiMemoryAcceptProtocolGuid = { 0x38c74800, 0x5590, 0x4db4, { 0xa0, 0xf3, 0x67, 0x5d, 0x9b, 0x8e, 0x80, 0x26 }}

--
2.38.0.rc1.362.ged0d419d3c-goog


[PATCH v6 4/7] OvmfPkg: Introduce CocoDxe driver

Dionna Glaze
 

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 protocol is not disabled. This allows safe
upgrades for OS loaders to affirm their support for the unaccepted
memory type.

Cc: Gerd Hoffmann <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Ard Biesheuvel <ardb@...>
Cc: "Min M. Xu" <min.m.xu@...>
Cc: Andrew Fish <afish@...>
Cc: "Michael D. Kinney" <michael.d.kinney@...>

Signed-off-by: Dionna Glaze <dionnaglaze@...>
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
OvmfPkg/CocoDxe/CocoDxe.c | 144 ++++++++++++++++++++
OvmfPkg/CocoDxe/CocoDxe.inf | 43 ++++++
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, 195 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 @@
<LibraryClasses>
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 0000000000..9e9a405af1
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -0,0 +1,144 @@
+/** @file
+
+ Confidential Compute Dxe driver. This driver installs protocols that are
+ generic over confidential compute techonology.
+
+ Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/MemEncryptSevLib.h>
+#include <Library/MemEncryptTdxLib.h>
+#include <Protocol/ExitBootServicesCallback.h>
+#include <Protocol/MemoryAccept.h>
+
+STATIC EFI_HANDLE mCocoDxeHandle = NULL;
+
+STATIC
+EFI_STATUS
+AcceptAllUnacceptedMemory (
+ IN EFI_MEMORY_ACCEPT_PROTOCOL *AcceptMemory
+ )
+{
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
+ UINTN NumEntries;
+ UINTN Index;
+ EFI_STATUS Status;
+ BOOLEAN AcceptedAny;
+
+ DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
+ AcceptedAny = FALSE;
+ Status = EFI_SUCCESS;
+ /*
+ * Get a copy of the memory space map to iterate over while
+ * changing the map.
+ */
+ Status = gDS->GetMemorySpaceMap (&NumEntries, &AllDescMap);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ for (Index = 0; Index < NumEntries; Index++) {
+ CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc;
+
+ Desc = &AllDescMap[Index];
+ if (Desc->GcdMemoryType != EfiGcdMemoryTypeUnaccepted) {
+ continue;
+ }
+
+ Status = AcceptMemory->AcceptMemory (
+ AcceptMemory,
+ Desc->BaseAddress,
+ Desc->Length
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gDS->RemoveMemorySpace(Desc->BaseAddress, Desc->Length);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypeSystemMemory,
+ Desc->BaseAddress,
+ Desc->Length,
+ EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_RP
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ AcceptedAny = TRUE;
+ }
+
+ // If any memory is accepted, cause ExitBootServices to fail with
+ // EFI_INVALID_PARAMETER in order to force the caller to refresh
+ // their view of the MemoryMap.
+ if (AcceptedAny) {
+ Status = EFI_INVALID_PARAMETER;
+ }
+
+done:
+ gBS->FreePool (AllDescMap);
+ return Status;
+}
+
+EFI_STATUS
+EFIAPI
+ResolveUnacceptedMemory (
+ IN EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL *This
+ )
+{
+ EFI_MEMORY_ACCEPT_PROTOCOL *AcceptMemory;
+ EFI_STATUS Status;
+
+ if (This->Disabled) {
+ return EFI_SUCCESS;
+ }
+
+ Status = gBS->LocateProtocol (&gEfiMemoryAcceptProtocolGuid, NULL,
+ (VOID **)&AcceptMemory);
+ if (Status == EFI_NOT_FOUND) {
+ return EFI_SUCCESS;
+ }
+ ASSERT_EFI_ERROR (Status);
+
+ return AcceptAllUnacceptedMemory(AcceptMemory);
+}
+
+STATIC EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL mExitBootServicesCallbackProcotol = {
+ ResolveUnacceptedMemory,
+ FALSE,
+};
+
+EFI_STATUS
+EFIAPI
+CocoDxeEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ 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->InstallProtocolInterface (&mCocoDxeHandle,
+ &gEdkiiExitBootServicesCallbackProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mExitBootServicesCallbackProcotol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Install EdkiiExitBootServicesCallbackProtocol failed.\n"));
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/CocoDxe/CocoDxe.inf b/OvmfPkg/CocoDxe/CocoDxe.inf
new file mode 100644
index 0000000000..3ff2a6fade
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.inf
@@ -0,0 +1,43 @@
+#/** @file
+#
+# Driver installs shared protocols needed for confidential compute
+# technologies.
+#
+# Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+ INF_VERSION = 1.25
+ BASE_NAME = CocoDxe
+ FILE_GUID = 08162f1e-5147-4d3e-b5a9-fa48c9808419
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = CocoDxeEntryPoint
+
+[Sources]
+ CocoDxe.c
+
+[Packages]
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ DxeServicesTableLib
+ MemEncryptSevLib
+ MemEncryptTdxLib
+ MemoryAllocationLib
+ UefiDriverEntryPoint
+
+[Depex]
+ TRUE
+
+[Protocols]
+ gEdkiiExitBootServicesCallbackProtocolGuid
+ gEfiMemoryAcceptProtocolGuid
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index c0c1a15b09..8136d50eb2 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -753,6 +753,7 @@
OvmfPkg/IoMmuDxe/IoMmuDxe.inf

OvmfPkg/TdxDxe/TdxDxe.inf
+ OvmfPkg/CocoDxe/CocoDxe.inf

#
# Variable driver stack (non-SMM)
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.fdf b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
index 6923eb8831..e612608c0c 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.fdf
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
@@ -269,6 +269,7 @@ INF ShellPkg/Application/Shell/Shell.inf
INF MdeModulePkg/Logo/LogoDxe.inf

INF OvmfPkg/TdxDxe/TdxDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf

#
# Usb Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index af566b953f..2cfb3fbc6b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -965,6 +965,7 @@
<LibraryClasses>
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
}
+ OvmfPkg/CocoDxe/CocoDxe.inf
OvmfPkg/IoMmuDxe/IoMmuDxe.inf

!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 80de4fa2c0..2ab7f3b95b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -343,6 +343,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

!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index f39d9cd117..3ead476b61 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -1036,6 +1036,7 @@
OvmfPkg/IoMmuDxe/IoMmuDxe.inf

OvmfPkg/TdxDxe/TdxDxe.inf
+ OvmfPkg/CocoDxe/CocoDxe.inf

!if $(SMM_REQUIRE) == TRUE
OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index c0f5a1ef3c..5dd452f42b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -370,6 +370,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

!if $(SMM_REQUIRE) == TRUE
--
2.38.0.rc1.362.ged0d419d3c-goog


[PATCH v6 3/7] MdeModulePkg: Invoke all ExitBootServicesCallback instances at ExitBootServices

Dionna Glaze
 

The protocol's intent is to allow drivers to install callbacks that can
modify the memory map at ExitBootServices time, so that any changes will
lead to the EFI_INVALID_PARAMETER error. This error is specified to require
the EBS caller to call GetMemoryMap again if it already had.

Cc: Gerd Hoffmann <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Ard Biesheuvel <ardb@...>
Cc: "Min M. Xu" <min.m.xu@...>
Cc: Andrew Fish <afish@...>
Cc: "Michael D. Kinney" <michael.d.kinney@...>
Cc: Ray Ni <ray.ni@...>

Signed-off-by: Dionna Glaze <dionnaglaze@...>
---
MdeModulePkg/Core/Dxe/DxeMain.h | 1 +
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 41 ++++++++++++++++++++
3 files changed, 43 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 815a6b4bd8..185b9dc3d6 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -45,6 +45,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/HiiPackageList.h>
#include <Protocol/SmmBase2.h>
#include <Protocol/PeCoffImageEmulator.h>
+#include <Protocol/ExitBootServicesCallback.h>
#include <Guid/MemoryTypeInformation.h>
#include <Guid/FirmwareFileSystem2.h>
#include <Guid/FirmwareFileSystem3.h>
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index e4bca89577..bdd9cf8222 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -153,6 +153,7 @@
gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES
gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
+ gEdkiiExitBootServicesCallbackProtocolGuid ## CONSUMES

# Arch Protocols
gEfiBdsArchProtocolGuid ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 5733f0c8ec..3270f9858d 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -744,6 +744,34 @@ CalculateEfiHdrCrc (
Hdr->CRC32 = Crc;
}

+/**
+ Invokes TerminateMemoryMapPrehook from the first located instance of
+ EdkiiExitBootServicesProtocol.
+**/
+STATIC
+EFI_STATUS
+InvokeTerminateMemoryMapPrehooks (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL *Callback;
+
+ Status = gBS->LocateHandle (
+ &gEdkiiExitBootServicesCallbackProtocolGuid,
+ NULL,
+ (VOID**)Callback
+ );
+ if (Status == EFI_NOT_FOUND) {
+ return EFI_SUCCESS;
+ }
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ return Callback->TerminateMemoryMapPrehook(Callback);
+}
+
/**
Terminates all boot services.

@@ -768,6 +796,19 @@ CoreExitBootServices (
//
gTimer->SetTimerPeriod (gTimer, 0);

+ //
+ // Invoke all protocols installed for ExitBootServices prior to
+ // CoreTerminateMemoryMap.
+ //
+ Status = InvokeTerminateMemoryMapPrehooks();
+ if (EFI_ERROR (Status)) {
+ //
+ // Notify other drivers that ExitBootServices failed
+ //
+ CoreNotifySignalList (&gEventExitBootServicesFailedGuid);
+ return Status;
+ }
+
//
// Terminate memory services if the MapKey matches
//
--
2.38.0.rc1.362.ged0d419d3c-goog


[PATCH v6 2/7] MdeModulePkg: Introduce ExitBootServicesCallbackProtocol

Dionna Glaze
 

This introduces a callback after the time that the timer is disabled and before
the MemoryMap is finalized.

This callback is useful to make final changes to the memory map due to protocols
initiated (or not initiated) by the OS loader.

Cc: Gerd Hoffmann <kraxel@...>
Cc: "Min M. Xu" <min.m.xu@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Ard Biesheuvel <ardb@...>
Cc: Andrew Fish <afish@...>
Cc: "Michael D. Kinney" <michael.d.kinney@...>

Signed-off-by: Dionna Glaze <dionnaglaze@...>
---
MdeModulePkg/Include/Protocol/ExitBootServicesCallback.h | 40 ++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 ++
2 files changed, 43 insertions(+)

diff --git a/MdeModulePkg/Include/Protocol/ExitBootServicesCallback.h b/MdeModulePkg/Include/Protocol/ExitBootServicesCallback.h
new file mode 100644
index 0000000000..5c9f973b79
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/ExitBootServicesCallback.h
@@ -0,0 +1,40 @@
+/** @file
+ The file provides the protocol that allows callbacks in ExitBootServices
+ immediately before TerminateMemoryMap.
+
+ Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#ifndef EXIT_BOOT_SERVICES_CALLBACK_H_
+#define EXIT_BOOT_SERVICES_CALLBACK_H_
+
+/* This protocol is internal to EDK2 only */
+
+#define EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL_GUID {0xf5684799, 0x9a33, 0x40f7, {0xa1, 0x5c, 0x10, 0x8e, 0x0e, 0x6b, 0x45, 0x25}}
+
+typedef struct _EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL
+ EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL;
+
+/**
+ @param This A pointer to a EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL.
+
+ The returned status may only be EFI_SUCCESS or EFI_INVALID_PARAMETER.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_TERMINATE_MEMORY_MAP_PREHOOK)(
+ IN EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL *This
+ );
+
+///
+/// The EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL allows callbacks in
+/// ExitBootServices immediately before TerminateMemoryMap.
+///
+struct _EDKII_EXIT_BOOT_SERVICES_CALLBACK_PROTOCOL {
+ EDKII_TERMINATE_MEMORY_MAP_PREHOOK TerminateMemoryMapPrehook;
+ BOOLEAN Disabled;
+};
+
+extern EFI_GUID gEdkiiExitBootServicesCallbackProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 58e6ab0048..add4e304ca 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -677,6 +677,9 @@
## Include/Protocol/VariablePolicy.h
gEdkiiVariablePolicyProtocolGuid = { 0x81D1675C, 0x86F6, 0x48DF, { 0xBD, 0x95, 0x9A, 0x6E, 0x4F, 0x09, 0x25, 0xC3 } }

+ ## Include/Protocol/ExitBootServicesCallback.h
+ gEdkiiExitBootServicesCallbackProtocolGuid = { 0xf5684799, 0x9a33, 0x40f7, {0xa1, 0x5c, 0x10, 0x8e, 0x0e, 0x6b, 0x45, 0x25 }}
+
[PcdsFeatureFlag]
## Indicates if the platform can support update capsule across a system reset.<BR><BR>
# TRUE - Supports update capsule across a system reset.<BR>
--
2.38.0.rc1.362.ged0d419d3c-goog


[PATCH v6 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe

Dionna Glaze
 

From: Sophia Wolf <phiawolf@...>

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 <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Dionna Glaze <dionnaglaze@...>
---
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..13fb58cc02 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Guid/ConfidentialComputingSevSnpBlob.h>
#include <Library/PcdLib.h>
+#include <Protocol/MemoryAccept.h>

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 EFI_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 EFI_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 (
+ &mAmdSevDxeHandle,
+ &gEfiMemoryAcceptProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mMemoryAcceptProtocol
+ );
+ 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 (
&gConfidentialComputingSevSnpBlobGuid,
&mSnpBootDxeTable
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 9acf860cf2..5ddddabc32 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -47,6 +47,9 @@
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize

+[Protocols]
+ gEfiMemoryAcceptProtocolGuid
+
[Guids]
gConfidentialComputingSevSnpBlobGuid

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index d3a95e4913..ee3710f7b3 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -14,6 +14,7 @@
#include <Library/MemEncryptSevLib.h>

#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.rc1.362.ged0d419d3c-goog


[PATCH v6 0/7] Add safe unaccepted memory behavior

Dionna Glaze
 

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 add a new protocol, ExitBootServicesCallbackProtocol, with a single
interface: TerminateMemoryMapPrehook(). We invoke all prehooks in
CoreExitBootServices after disabling the timer and before
TerminateMemoryMap. This gives hooks the chance to change the memory
map and cause ExitBootServices to fail with EFI_INVALID_PARAMETER.
The failure is specified to require the caller to update their view
of the MemoryMap and call ExitBootServices again.

To make use of this new protocol, 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, add another
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 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 <ardb@...>
Cc: "Min M. Xu" <min.m.xu@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Tom Lendacky <Thomas.Lendacky@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Andrew Fish <afish@...>
Cc: "Michael D. Kinney" <michael.d.kinney@...>

Dionna Glaze (7):
OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
MdeModulePkg: Introduce ExitBootServicesCallbackProtocol
MdeModulePkg: Invoke all ExitBootServicesCallback instances at
ExitBootServices
OvmfPkg: Introduce CocoDxe driver
MdePkg: Introduce the AcceptAllUnacceptedMemory protocol
OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

MdeModulePkg/Core/Dxe/DxeMain.h | 1 +
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 41 +++++
MdeModulePkg/Include/Protocol/ExitBootServicesCallback.h | 40 +++++
MdeModulePkg/MdeModulePkg.dec | 3 +
MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h | 40 +++++
MdePkg/MdePkg.dec | 3 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
OvmfPkg/CocoDxe/CocoDxe.c | 169 ++++++++++++++++++++
OvmfPkg/CocoDxe/CocoDxe.inf | 44 +++++
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 | 5 +
21 files changed, 429 insertions(+), 8 deletions(-)
create mode 100644 MdeModulePkg/Include/Protocol/ExitBootServicesCallback.h
create mode 100644 MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf

--
2.38.0.rc1.362.ged0d419d3c-goog


Re: [PATCH v2 3/6] IntelFsp2Pkg: Add CI YAML file

Michael Kubacki
 

+Other IntelFsp2Pkg & IntelFsp2WrapperPkg maintainers to To line

Please review the remaining patches in this patch series:

https://edk2.groups.io/g/devel/message/93859

Thanks,
Michael

On 10/3/2022 10:44 AM, Michael Kubacki wrote:
Another reminder.
On 9/22/2022 9:07 PM, Michael Kubacki wrote:
Review reminder

On 9/15/2022 3:41 PM, Michael Kubacki wrote:
Hi Chasel,

Your CI YAML file feedback in v1 is addressed now in v2.

Can you please provide your review on this patch and [PATCH v2 5/6]?

Note that I updated the commit message for this patch to remove the info about the build being broken since that was recently fixed. That update is in the branch:

https://github.com/makubacki/edk2/commit/c37e6dfa482ed075cd4ab6712e6d17b3cf17786a

With these reviews, the series will be covered.

Thanks,
Michael

On 9/15/2022 2:55 PM, Michael Kubacki wrote:
From: Michael Kubacki <michael.kubacki@...>

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

Adds IntelFsp2Pkg to the list of supported build packages for edk2
CI and defines an initial set of CI configuration options.

The compiler plugin is disabled as the package currently does not
build due to some changes in the FSP 2.4 interface addition.

Specifically, in commit df25a54 "Fsp24SecCore<API>.inf" files were
added to IntelFspPkg.dsc but the actual files were not added.

Simply removing these files from the DSC exposes a linker failure.

Recommendation:

1. Enable package CI (accept this change)
2. Add IntelFsp2Pkg.dsc to the "CompilerPlugin" "DscPath" in
    IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml to enable compilation
3. Verify compilation and all currently enabled package CI checks
    pass
4. Check-in fixes in (3) with change in (2)

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
  .pytool/CISettings.py             |  1 +
  IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml | 90 ++++++++++++++++++++
  2 files changed, 91 insertions(+)

diff --git a/.pytool/CISettings.py b/.pytool/CISettings.py
index cf9e0d77b19b..0205c26a58f8 100644
--- a/.pytool/CISettings.py
+++ b/.pytool/CISettings.py
@@ -54,6 +54,7 @@ class Settings(CiBuildSettingsManager, UpdateSettingsManager, SetupSettingsManag
                  "ArmVirtPkg",
                  "DynamicTablesPkg",
                  "EmulatorPkg",
+                "IntelFsp2Pkg",
                  "MdePkg",
                  "MdeModulePkg",
                  "NetworkPkg",
diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml b/IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml
new file mode 100644
index 000000000000..9ce401b20164
--- /dev/null
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml
@@ -0,0 +1,90 @@
+## @file
+# Core CI configuration for IntelFsp2Pkg
+#
+# Copyright (c) Microsoft Corporation
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+{
+    ## options defined .pytool/Plugin/LicenseCheck
+    "LicenseCheck": {
+        "IgnoreFiles": []
+    },
+
+    "EccCheck": {
+        ## Exception sample looks like below:
+        ## "ExceptionList": [
+        ##     "<ErrorID>", "<KeyWord>"
+        ## ]
+        "ExceptionList": [
+        ],
+        ## Both file path and directory path are accepted.
+        "IgnoreFiles": []
+    },
+
+    ## options defined .pytool/Plugin/CompilerPlugin
+    "CompilerPlugin": {
+        "DscPath": "IntelFsp2Pkg.dsc"
+    },
+
+    ## options defined .pytool/Plugin/HostUnitTestCompilerPlugin
+    "HostUnitTestCompilerPlugin": {
+        "DscPath": "" # Don't support this test
+    },
+
+    ## options defined .pytool/Plugin/CharEncodingCheck
+    "CharEncodingCheck": {
+        "IgnoreFiles": []
+    },
+
+    ## options defined .pytool/Plugin/DependencyCheck
+    "DependencyCheck": {
+        "AcceptableDependencies": [
+          "IntelFsp2Pkg/IntelFsp2Pkg.dec",
+          "MdeModulePkg/MdeModulePkg.dec",
+          "MdePkg/MdePkg.dec",
+          "UefiCpuPkg/UefiCpuPkg.dec"
+        ],
+        # For host based unit tests
+        "AcceptableDependencies-HOST_APPLICATION":[
+          "UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec"
+        ],
+        # For UEFI shell based apps
+        "AcceptableDependencies-UEFI_APPLICATION":[],
+        "IgnoreInf": []
+    },
+
+    ## options defined .pytool/Plugin/DscCompleteCheck
+    "DscCompleteCheck": {
+        "IgnoreInf": [""],
+        "DscPath": "IntelFsp2Pkg.dsc"
+    },
+
+    ## options defined .pytool/Plugin/HostUnitTestDscCompleteCheck
+    "HostUnitTestDscCompleteCheck": {
+        "IgnoreInf": [""],
+        "DscPath": "" # Don't support this test
+    },
+
+    ## options defined .pytool/Plugin/GuidCheck
+    "GuidCheck": {
+        "IgnoreGuidName": [],
+        "IgnoreGuidValue": [],
+        "IgnoreFoldersAndFiles": [],
+        "IgnoreDuplicates": [],
+    },
+
+    ## options defined .pytool/Plugin/LibraryClassCheck
+    "LibraryClassCheck": {
+        "IgnoreHeaderFile": []
+    },
+
+    ## options defined .pytool/Plugin/SpellCheck
+    "SpellCheck": {
+        "AuditOnly": True,           # Fails right now with over 270 errors
+        "IgnoreFiles": [],           # use gitignore syntax to ignore errors in matching files
+        "ExtendWords": [],           # words to extend to the dictionary for this package
+        "IgnoreStandardPaths": [],   # Standard Plugin defined paths that should be ignore
+        "AdditionalIncludePaths": [] # Additional paths to spell check (wildcards supported)
+    }
+}


Re: Can NULL pointer be a valid event?

Michael D Kinney
 

Hi Ayush,

Quick answer is that the UEFI Spec may not explicitly disallow NULLL, but in practice,
it will never return NULL.

===================================

EFI_EVENT is same as VOID*.

typedef VOID *EFI_EVENT

CreateEvent() returns a pointer to an Event, so it is really a double pointer.
CreateEvent() returns EFI_INVALID_PARAMETER if Event (pointer to EFI_EVENT structure) is NULL.

But CreateEvent/Ex() do not explicitly state that the pointer to the EFI_EVENT structure
returned cannot be address 0.

Internally to the EDK II, EFI_EVENT is a structure so it must be a valid pointer. Though I
would point out that even this is an implementation choice. An implementation could treat the
pointer to the EFI_EVENT as a handle number and could internally convert a handle number to a
structure pointer to further hide details of the event structure and prevent the reuse of the
same pointer value for different events across allocates/frees. The EDK II implementation
choice to use pointers instead of handles is for the smallest/fastest implementation.

It is possible to have a pointer to a structure at address 0. However, the EDK II implementations
of the UEFI services do not allow the use of memory at 0 for normal memory allocations. I am aware
of one use case of memory at 0 for an x86 IDT structure for 16-bit code. So it is not possible
for the EDK II implementation of an UEFI service that returns pointers to structures to return a
pointer value of 0. In fact, there are guard page features in EDK II that check if there is any
access to the first page of memory in the address range 0x0..0xFFF. So the real restriction EDK II
imposes is to never allocate a data structure in the first page of memory (0x0..0xFFF).

Given it would be possible to implement many UEFI services using handle numbers instead of
pointers. I would recommend those implementations do not use a handle value of 0. And instead
start at a handle value of at least 1.

Best regards,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dionna Glaze via groups.io
Sent: Monday, October 3, 2022 10:16 AM
To: devel@edk2.groups.io; ayushdevel1325@...
Subject: Re: [edk2-devel] Can NULL pointer be a valid event?

CreateEvent returns an EFI_STATUS. It expects the OUT parameter, a
pointer to an EFI_EVENT, to be non-NULL. A null pointer results in
EFI_INVALID_PARAMETER. If the CreateEvent is successful, then `event`
points to the newly created event. It's the caller's responsibility to
pass a pointer to valid writable memory.

On Mon, Oct 3, 2022 at 8:08 AM Ayush Singh <ayushdevel1325@...> wrote:

Hello everyone,

I wanted to ask if a NULL pointer can be returned as a valid event from `EFI_BOOT_SERVICES.CreateEvent()` or
`EFI_BOOT_SERVICES.CreateEventEx()`? Or does the specification state that a valid event pointer has to be non-NULL?

Yours Sincerely,
Ayush Singh


--
-Dionna Glaze, PhD (she/her)




Re: [PATCH v5 2/7] MdePkg: Introduce ExitBootServicesCallbackProtocol

Dionna Glaze
 


What Ray is getting to (I think) is that that means that it should be
defined in MdeModulePkg not MdePkg.
Oh, MemoryAcceptProtocol is also not specified AFAICT, so I thought
this is where it would go. I can move it.

--
-Dionna Glaze, PhD (she/her)


Re: Can NULL pointer be a valid event?

Dionna Glaze
 

CreateEvent returns an EFI_STATUS. It expects the OUT parameter, a
pointer to an EFI_EVENT, to be non-NULL. A null pointer results in
EFI_INVALID_PARAMETER. If the CreateEvent is successful, then `event`
points to the newly created event. It's the caller's responsibility to
pass a pointer to valid writable memory.

On Mon, Oct 3, 2022 at 8:08 AM Ayush Singh <ayushdevel1325@...> wrote:

Hello everyone,

I wanted to ask if a NULL pointer can be returned as a valid event from `EFI_BOOT_SERVICES.CreateEvent()` or `EFI_BOOT_SERVICES.CreateEventEx()`? Or does the specification state that a valid event pointer has to be non-NULL?

Yours Sincerely,
Ayush Singh
--
-Dionna Glaze, PhD (she/her)


Re: The principles of EDK2 module reconstruction for archs

Michael D Kinney
 

Hi Abner,

responses below.

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io
Sent: Sunday, October 2, 2022 10:37 PM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; quic_llindhol@...; Ni, Ray <ray.ni@...>;
Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L <sunilvl@...>
Cc: lichao <lichao@...>; Kirkendall, Garrett <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; He,
Jiangang <Jiangang.He@...>; Andrew Fish <afish@...>
Subject: Re: [edk2-devel] The principles of EDK2 module reconstruction for archs

[AMD Official Use Only - General]

Mike,
Agree. This can be mentioned on the Wiki page. Also, this would require the discussion between maintainer and contributor. I would
say maintainer has the responsibility to make sure an arch folder is only created when necessary.
Agreed


Do you agree with the arch concatenate solution for arch folder? That means IA32_X64 instead of X86 (I am fine with this)?
Yes

However, there is one scenario. Assume there were two arch folders IA32_X64 and RISCV64. Then ARM shares the code with IA32_X64,
we may rename IA32_X64 to IA32_X64_ARM.
Although the directory naming is not real a problem to the build, a separate ARM folder seems easier? Or we can just allow this
kind of folder naming structure, however we let maintainer to make the decision?
I would let the maintainer make the decision. For your example, another approach may be to move the IA32_X64 content up a level
so it is common and is used by IA32, X64, ARM. And leave RISCV64 folder for an arch that has special requirements. I think having
some flexibility in the guidelines is very beneficial. The main goal is for consistency when a specific guideline is followed.


Abner


-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Monday, October 3, 2022 1:18 PM
To: Chang, Abner <Abner.Chang@...>; devel@edk2.groups.io;
quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, AbdulLateef
(Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L
<sunilvl@...>; Kinney, Michael D
<michael.d.kinney@...>
Cc: lichao <lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; He,
Jiangang <Jiangang.He@...>; Andrew Fish <afish@...>
Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction for
archs

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


Abner,

I think another guideline is to minimize the number of subdirectories.

Only create them if it helps with the organization and long term maintenance
of the component.

Mike


-----Original Message-----
From: Chang, Abner <Abner.Chang@...>
Sent: Sunday, October 2, 2022 8:13 PM
To: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io; quic_llindhol@...; Ni, Ray
<ray.ni@...>; Attar, AbdulLateef (Abdul Lateef)
<AbdulLateef.Attar@...>; Sunil V L <sunilvl@...>
Cc: lichao <lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>;
He,
Jiangang <Jiangang.He@...>; Andrew Fish <afish@...>
Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction
for archs

[AMD Official Use Only - General]

Hi Mike and Leif,
First of all, we don't use arch folder if the module is mainly for a
specific arch in obviously. So we will have both common and arch-specific
files constructed under module/library root.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk
2
.groups.io%2Fg%2Fdevel%2Fmessage%2F94567&amp;data=05%7C01%7CA
bner.Chan
g%40amd.com%7Cd49cbbe6d3d942bd69a308daa4fea41b%7C3dd8961fe4884
e608e11a
82d994e183d%7C0%7C0%7C638003710850252776%7CUnknown%7CTWFpbGZ
sb3d8eyJWI
joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
000%7
C%7C%7C&amp;sdata=eiLOC0G9WZWKqm2ALcAiKr7SPBP5AgDdAxogHlv5pI
M%3D&amp;r
eserved=0
SmmCpuFeatureLib\Ia32
SmmCpuFeatureLib\X64
SmmCpuFeatureLib\SmmCpuFeatureLib.c
SmmCpuFeatureLib\SmmCpuFeatureLibCommon.c
SmmCpuFeatureLib\IntelSmmCPuFeaturesLib
SmmCpuFeatureLib\AmdlSmmCPuFeaturesLib


Could we concatenate architectures?
I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ?
Looks like below?

CpuDxe\IA32_X64\IA32\abc.nasm
CpuDxe\IA32_X64\X64\abc.nasm
CpuDxe\IA32_X64\CpuDxe.c
CpuDxe\IA32_X64\AmdCpuDxe.c
CpuDxe\IA32_X64\IntelCpuDxe.c
CpuDxe\RiscV64\CpuDxe.c
CpuDxe\ARM\CpuDxe.c
CpuDxe\
CpuDxeCommon.c // If required.
CpuDxe.inf // Use INF section arch modifier for X86, RISC-V
and ARM.
CpuDxeAmd.inf // Separate INF for AMD.

Seems ok, but is AARCH64_RISCV64 a real case? Or it means we can
create a folder "AARCH64_RISCV64" when there are some common files
shared by AARCH64 and RISCV64?

For the 32/64 bit support, it can still stay under module root and
don't need to assign a folder for them unless the arch has the different
implementation.
Regards,
Abner



-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Saturday, October 1, 2022 2:52 AM
To: devel@edk2.groups.io; quic_llindhol@...; Chang, Abner
<Abner.Chang@...>; Ni, Ray <ray.ni@...>; Attar,
AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L
<sunilvl@...>; Kinney, Michael D
<michael.d.kinney@...>
Cc: lichao <lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>;
He, Jiangang <Jiangang.He@...>; Andrew Fish <afish@...>
Subject: RE: [edk2-devel] The principles of EDK2 module
reconstruction for archs

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


Hi Leif,

Concatenation is a good idea. Makes it more obvious and can be
easily adopted for new CPU archs.

There is an additional case where an implementation does not have
differences based on CPU Arch or Vendor, but it does have
differences based on the bit- width of the CPU. Take BaseSafeIntLib as
an example.
It has source files for 32-bit CPUs, 64-bit CPUs, and CPU arch
specific file for Ebc because Ebc adapts to 32-bit or 64-bit
depending on the CPU type the EBC Interpreter is running.

MdePkg/Library/BaseSafeIntLib/
BaseSafeIntLib.inf
SafeIntLib.c
SafeIntLib32.c
SafeIntLib64.c
SafeIntLibEbc.c

Should we add "32" and "64" as supported suffices in file names?

If we wanted to put type content into a subdirectory, what would be
the suggested directory name for "32" and "64". Or do we want to
require this type of difference to always be files in top level directory of
the modules/library?

Best regards,

Mike


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Leif Lindholm
Sent: Friday, September 30, 2022 9:09 AM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>; Chang, Abner
<Abner.Chang@...>;
Ni, Ray <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef)
<AbdulLateef.Attar@...>; Sunil V L <sunilvl@...>
Cc: lichao <lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul
<Paul.Grimes@...>;
He, Jiangang <Jiangang.He@...>; Andrew Fish
<afish@...>
Subject: Re: [edk2-devel] The principles of EDK2 module
reconstruction for archs

I agree similar things will certainly happen for ARM/AARCH64,
which will probably be noticeable when I start eradicating ArmPkg
and putting the arch-standard bits into (mostly) MdePkg.

But I like the ability to see already at the filesystem level
which files belong to the architecture I'm currently working on and
which don't.

Could we concatenate architectures?
I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ?

/
Leif

On 2022-09-30 08:28, Michael D Kinney wrote:
Hi Abner,

One comment is on adding a new CPU type dir name of 'X86' for
content that is common for IA32/X64.

I can imagine a similar case for ARM/AARCH64 and for the RISCV
(32, 64, 128) cases.

I think I would prefer to drop X86 and if there are files that
are common to multiple CPU architectures then they are
considered common and are in top directory of module and the
file header and INF file can clearly document which CPU archs the file
applies.

Mike

-----Original Message-----
From: Chang, Abner <Abner.Chang@...>
Sent: Friday, September 30, 2022 12:11 AM
To: Ni, Ray <ray.ni@...>; Attar, AbdulLateef (Abdul
Lateef) <AbdulLateef.Attar@...>; Sunil V L
<sunilvl@...>; devel@edk2.groups.io; Kinney,
Michael D <michael.d.kinney@...>
Cc: lichao <lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul
<Paul.Grimes@...>; He, Jiangang <Jiangang.He@...>;
Leif
Lindholm <quic_llindhol@...>; Andrew Fish
<afish@...>
Subject: RE: [edk2-devel] The principles of EDK2 module
reconstruction for archs

[AMD Official Use Only - General]

Thanks Ray, here are my responses.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F
%2Fg
ithub.com%2Ftianocore-docs%2Fedk2-
CCodingStandardsSpecification
%2Fp
ull%2F2&amp;data=05%7C01%7CAbner.Chang%40amd.com%7C7c3292c8bd2
f4
86f
920908daa314e8e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
3800
1607445876768%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
JQ
IjoiV
2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata
=
HAq
ou4JyY1yxDthuQ1dEKcF7Q3o4W77Oo9rOOvkXNWU%3D&amp;reserved=0

@Kinney, Michael D we may also need your clarification on the
comments.


-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Thursday, September 29, 2022 3:42 PM
To: Attar, AbdulLateef (Abdul Lateef)
<AbdulLateef.Attar@...>; Chang, Abner
<Abner.Chang@...>; Sunil V L <sunilvl@...>;
devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; lichao
<lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul
<Paul.Grimes@...>; He, Jiangang <Jiangang.He@...>;
Leif Lindholm <quic_llindhol@...>; Andrew Fish
<afish@...>
Subject: RE: [edk2-devel] The principles of EDK2 module
reconstruction for archs

Caution: This message originated from an External Source. Use
proper caution when opening attachments, clicking links, or
responding.


Abner,
Comments in
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2
F%2F
gith
ub.com%2Ftianocore-docs%2Fedk2-
CCodingStandardsSpecification%2Fpull%2F2%23pullrequestreview-
1124763311&amp;data=05%7C01%7CAbner.Chang%40amd.com%7Cd825e24
8625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82d994e183d%7C0
%7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
7C%7C%7C&amp;sdata=RXxgpbEr6ivbqP1R6%2B3Rxl%2ByJAnaUJuaYYKdfCH
8jo8%3D&amp;reserved=0

We can discuss more in tomorrow's meeting.


-----Original Message-----
From: Attar, AbdulLateef (Abdul Lateef)
<AbdulLateef.Attar@...>
Sent: Thursday, September 29, 2022 3:11 PM
To: Chang, Abner <Abner.Chang@...>; Sunil V L
<sunilvl@...>; devel@edk2.groups.io; Ni, Ray
<ray.ni@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; lichao
<lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul
<Paul.Grimes@...>;
He,
Jiangang <Jiangang.He@...>; Leif Lindholm
<quic_llindhol@...>; Andrew Fish <afish@...>
Subject: RE: [edk2-devel] The principles of EDK2 module
reconstruction for archs

Hi Abner,
Looks good to me.
Reviewed-by: Abdul Lateef Attar <abdattar@...>

Thanks
AbduL

-----Original Message-----
From: Chang, Abner <Abner.Chang@...>
Sent: 28 September 2022 20:31
To: Sunil V L <sunilvl@...>;
devel@edk2.groups.io; ray.ni@...
Cc: Kinney, Michael D <michael.d.kinney@...>; lichao
<lichao@...>; Kirkendall, Garrett
<Garrett.Kirkendall@...>; Grimes, Paul
<Paul.Grimes@...>;
He,
Jiangang <Jiangang.He@...>; Attar, AbdulLateef (Abdul
Lateef) <AbdulLateef.Attar@...>; Leif Lindholm
<quic_llindhol@...>; Andrew Fish <afish@...>
Subject: RE: [edk2-devel] The principles of EDK2 module
reconstruction for archs

[AMD Official Use Only - General]

I just had created PR to update edkII C coding standard spec
for the file and directory naming. We can review and confirm
this update first and then go back to the principles of EDK2
module
reconstruction for archs.
Here is the PR:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2
F%2F
gith
ub.com%2Ftianocore-docs%2Fedk2-
&amp;data=05%7C01%7CAbner.Chang%40amd.c
om%7Cd825e248625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82
d994e18
3d%7C0%7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJ
WIjoiMC4wLj
AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%
7C%7C&a
mp;sdata=X4z9puj81nIGTqtMxC9igDZyBPOT6OTWSU%2BjoIowo%2BE%3D&a
mp;reserv
ed=0
CCodingStandardsSpecification/pull/2

The naming rule is mainly for the new module or new file IMO.
Some existing module may not meet the guidelines mentioned in
this
spec.
Thus we need the principles of EDK2 module reconstruction on
the existing module to support other processor archs and not
impacting the
existing platforms (e.g.
rename the INF file or directory to meet the guidelines).

Sunil, seems RISC-V CpuDxe meet the guideline. Please check it.
Just feel that having CpuDxe.c to Riscv64 folder is not
quite a best solution. I think at least we can abstract the
protocol structure and protocol installation under CpuDxe\
and have the arch implementation under arch folder. We can
discuss this later after we confirming the
guideline and principles.

Thanks
Abner

-----Original Message-----
From: Sunil V L <sunilvl@...>
Sent: Wednesday, September 28, 2022 3:34 PM
To: devel@edk2.groups.io; ray.ni@...
Cc: Chang, Abner <Abner.Chang@...>; Kinney, Michael D
<michael.d.kinney@...>; lichao <lichao@...>;
Kirkendall, Garrett <Garrett.Kirkendall@...>; Grimes,
Paul <Paul.Grimes@...>; He, Jiangang
<Jiangang.He@...>; Attar, AbdulLateef (Abdul Lateef)
<AbdulLateef.Attar@...>; Leif Lindholm
<quic_llindhol@...>; Andrew Fish <afish@...>
Subject: Re: [edk2-devel] The principles of EDK2 module
reconstruction for archs

Caution: This message originated from an External Source.
Use proper caution when opening attachments, clicking links, or
responding.


On Wed, Sep 28, 2022 at 03:33:45AM +0000, Ni, Ray wrote:
Hi Ray,

1. When a new arch's implementation is introduced to
the existing
module which was developed for the specific arch:

1. The folder reconstruction:

* Create arch folder for the existing arch implementation
[Ray] Do you move existing arch implementation to that arch
folder?
It will
break existing platforms a lot.

* Create the arch folder for the new introduced arch
[Ray] I agree. But if we don't create arch folder for
existing arch
implementation, the pkg layout will be a mess.

[Ray] Hard for me to understand all the principles here.
Maybe we review
existing code including to-be-upstreamed code and decide how
to go.
Could you please take a look below changes which is trying
to add RISC-V support for CpuDxe?
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2
F%2F
gith
ub.com%2Ftianocore%2Fedk2-
staging%2Fcommit%2Fbba1a11be47dd091734e185afbed73ea75708749&amp;
data=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08d
aa123e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947
2732494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sd
ata=Vq6pJLnn8yJrJhFZn7LfLbZzrtpG4n1VLWgAil6J38U%3D&amp;reserved=0
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2
F%2F
gith
ub.com%2Ftianocore%2Fedk2-
staging%2Fcommit%2F7fccf92a97a6d0618a20f10622220e78b3687906&amp;da
ta=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08daa1
23e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947273
2494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata
=xFmvUv58vh4AUAM17Qy9G5jZWFZlK2Ozt3njpG1e8%2BY%3D&amp;reserv
ed=0

What do you suggest with above example?

1) Common INF for all architectures - but modify INF alone,
no
X86 folder creation.

This is what I have done in the commit above. May be of
least impact to existing code since it is only INF change.
But like you mentioned this is bit weird that X86 files will
remain in root folder directly along with some common files.

2) Common INF (CpuDxe.inf) + create arch folders X86, X64,
IA32,
RiscV64 etc

IMO, this is probably the best approach. What would be the
challenges with this?

3) Separate INF for arch like CpuDxe.inf for x86,
CpuDxeRiscV64.inf for
RISC-V.

This again probably is not a good idea.

4) If the module/library is specific to one arch (ex:
SMM(X86), SBI(RISC-V)), then create separate INF.

Thanks!
Sunil









Can NULL pointer be a valid event?

 

Hello everyone,

I wanted to ask if a NULL pointer can be returned as a valid event from `EFI_BOOT_SERVICES.CreateEvent()` or `EFI_BOOT_SERVICES.CreateEventEx()`? Or does the specification state that a valid event pointer has to be non-NULL?

Yours Sincerely,
Ayush Singh


Re: [PATCH v5 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe

Lendacky, Thomas
 

On 9/30/22 18:06, Dionna Glaze wrote:
From: Sophia Wolf <phiawolf@...>
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 <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Dionna Glaze <dionnaglaze@...>
Just some formatting suggestions and one area of cleanup from previous version of the patch below. Assuming you take care of those:

Reviewed-by: Tom Lendacky <thomas.lendacky@...>

---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 57 ++++++++++++++++++--
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 ++
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++++++--
3 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 662d3c4ccb..77d3caa833 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Guid/ConfidentialComputingSevSnpBlob.h>
#include <Library/PcdLib.h>
+#include <Protocol/MemoryAccept.h>
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
SIGNATURE_32 ('A', 'M', 'D', 'E'),
@@ -31,6 +32,38 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
FixedPcdGet32 (PcdOvmfCpuidSize),
};
+STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+
+STATIC
+EFI_STATUS
+EFIAPI
+AmdSevMemoryAccept (
+ IN EFI_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 (((StartAddress & ~(SIZE_4KB - 1)) == 0) &&
+ ((Size & ~(SIZE_4KB - 1)) == 0) &&
+ (Size != 0));
Create a generic alignment check macro?

#define IS_ALIGNED(x, y) (((x) & ((y) - 1)) == 0)

Maybe keep the ASSERTs separate so they better identify which condition caused the assert, e.g.:

ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
ASSERT (IS_ALIGNED (Size, SIZE_4KB));
ASSERT (Size != 0);

?

Not sure if those are worth it or not, though.

+
+ MemEncryptSevSnpPreValidateSystemRam (
+ StartAddress,
+ EFI_SIZE_TO_PAGES (Size)
+ );
+
+ return EFI_SUCCESS;
+}
+
+STATIC EFI_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
+ AmdSevMemoryAccept
+};
+
EFI_STATUS
EFIAPI
AmdSevDxeEntryPoint (
@@ -147,11 +180,27 @@ 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 (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Install EfiMemoryAcceptProtocol failed.\n"));
+ }
Looks like this shouldn't be here.

+
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 (
+ &mAmdSevDxeHandle,
+ &gEfiMemoryAcceptProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mMemoryAcceptProtocol
+ );
Need to indent these two more spaces to align with the "s" in Install.

Thanks,
Tom

+ 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 (
&gConfidentialComputingSevSnpBlobGuid,
&mSnpBootDxeTable
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 9acf860cf2..5ddddabc32 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -47,6 +47,9 @@
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+[Protocols]
+ gEfiMemoryAcceptProtocolGuid
+
[Guids]
gConfidentialComputingSevSnpBlobGuid
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index d3a95e4913..ee3710f7b3 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -14,6 +14,7 @@
#include <Library/MemEncryptSevLib.h>
#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);
}


Re: [PATCH v1 1/2] SignedCapsulePkg: Add package CI YAML file

Michael Kubacki
 

Another reminder.

On 9/22/2022 9:06 PM, Michael Kubacki wrote:
Review reminder
On 9/15/2022 3:36 PM, Michael Kubacki wrote:
Hi Jian,

Can you please provide a review for this patch?

Mike Kinney has already given an R-b for [PATCH v1 2/2] so the series will be ready once this patch is reviewed.

Thanks,
Michael

On 9/7/2022 1:05 AM, Michael Kubacki wrote:
From: Michael Kubacki <michael.kubacki@...>

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

Adds the package as a supported package to .pytool/CISettings.py
and adds a CI YAML for the package so it can be run in CI.

Cc: Jian J Wang <jian.j.wang@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
  .pytool/CISettings.py                     |  1 +
  SignedCapsulePkg/SignedCapsulePkg.ci.yaml | 90 ++++++++++++++++++++
  2 files changed, 91 insertions(+)

diff --git a/.pytool/CISettings.py b/.pytool/CISettings.py
index cf9e0d77b19b..306e27893e58 100644
--- a/.pytool/CISettings.py
+++ b/.pytool/CISettings.py
@@ -62,6 +62,7 @@ class Settings(CiBuildSettingsManager, UpdateSettingsManager, SetupSettingsManag
                  "UefiCpuPkg",
                  "FmpDevicePkg",
                  "ShellPkg",
+                "SignedCapsulePkg",
                  "StandaloneMmPkg",
                  "FatPkg",
                  "CryptoPkg",
diff --git a/SignedCapsulePkg/SignedCapsulePkg.ci.yaml b/SignedCapsulePkg/SignedCapsulePkg.ci.yaml
new file mode 100644
index 000000000000..5f48613bd79f
--- /dev/null
+++ b/SignedCapsulePkg/SignedCapsulePkg.ci.yaml
@@ -0,0 +1,90 @@
+## @file
+# Core CI configuration for SignedCapsulePkg
+#
+# Copyright (c) Microsoft Corporation
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+{
+    ## options defined .pytool/Plugin/LicenseCheck
+    "LicenseCheck": {
+        "IgnoreFiles": []
+    },
+
+    "EccCheck": {
+        ## Exception sample looks like below:
+        ## "ExceptionList": [
+        ##     "<ErrorID>", "<KeyWord>"
+        ## ]
+        "ExceptionList": [
+        ],
+        ## Both file path and directory path are accepted.
+        "IgnoreFiles": []
+    },
+
+    ## options defined .pytool/Plugin/CompilerPlugin
+    "CompilerPlugin": {
+        "DscPath": "SignedCapsulePkg.dsc"
+    },
+
+    ## options defined .pytool/Plugin/HostUnitTestCompilerPlugin
+    "HostUnitTestCompilerPlugin": {
+        "DscPath": "" # Don't support this test
+    },
+
+    ## options defined .pytool/Plugin/CharEncodingCheck
+    "CharEncodingCheck": {
+        "IgnoreFiles": []
+    },
+
+    ## options defined .pytool/Plugin/DependencyCheck
+    "DependencyCheck": {
+        "AcceptableDependencies": [
+          "MdeModulePkg/MdeModulePkg.dec",
+          "MdePkg/MdePkg.dec",
+          "SecurityPkg/SecurityPkg.dec",
+          "SignedCapsulePkg/SignedCapsulePkg.dec"
+        ],
+        # For host based unit tests
+        "AcceptableDependencies-HOST_APPLICATION":[
+          "UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec"
+        ],
+        # For UEFI shell based apps
+        "AcceptableDependencies-UEFI_APPLICATION":[],
+        "IgnoreInf": []
+    },
+
+    ## options defined .pytool/Plugin/DscCompleteCheck
+    "DscCompleteCheck": {
+        "IgnoreInf": [""],
+        "DscPath": "SignedCapsulePkg.dsc"
+    },
+
+    ## options defined .pytool/Plugin/HostUnitTestDscCompleteCheck
+    "HostUnitTestDscCompleteCheck": {
+        "IgnoreInf": [""],
+        "DscPath": "" # Don't support this test
+    },
+
+    ## options defined .pytool/Plugin/GuidCheck
+    "GuidCheck": {
+        "IgnoreGuidName": [],
+        "IgnoreGuidValue": [],
+        "IgnoreFoldersAndFiles": [],
+        "IgnoreDuplicates": [],
+    },
+
+    ## options defined .pytool/Plugin/LibraryClassCheck
+    "LibraryClassCheck": {
+        "IgnoreHeaderFile": []
+    },
+
+    ## options defined .pytool/Plugin/SpellCheck
+    "SpellCheck": {
+        "AuditOnly": True,           # Failures need to be reviewed and resolved in the future
+        "IgnoreFiles": [],           # use gitignore syntax to ignore errors in matching files
+        "ExtendWords": [],           # words to extend to the dictionary for this package
+        "IgnoreStandardPaths": [],   # Standard Plugin defined paths that should be ignore
+        "AdditionalIncludePaths": [] # Additional paths to spell check (wildcards supported)
+    }
+}


Re: [PATCH v2 3/6] IntelFsp2Pkg: Add CI YAML file

Michael Kubacki
 

Another reminder.

On 9/22/2022 9:07 PM, Michael Kubacki wrote:
Review reminder
On 9/15/2022 3:41 PM, Michael Kubacki wrote:
Hi Chasel,

Your CI YAML file feedback in v1 is addressed now in v2.

Can you please provide your review on this patch and [PATCH v2 5/6]?

Note that I updated the commit message for this patch to remove the info about the build being broken since that was recently fixed. That update is in the branch:

https://github.com/makubacki/edk2/commit/c37e6dfa482ed075cd4ab6712e6d17b3cf17786a

With these reviews, the series will be covered.

Thanks,
Michael

On 9/15/2022 2:55 PM, Michael Kubacki wrote:
From: Michael Kubacki <michael.kubacki@...>

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

Adds IntelFsp2Pkg to the list of supported build packages for edk2
CI and defines an initial set of CI configuration options.

The compiler plugin is disabled as the package currently does not
build due to some changes in the FSP 2.4 interface addition.

Specifically, in commit df25a54 "Fsp24SecCore<API>.inf" files were
added to IntelFspPkg.dsc but the actual files were not added.

Simply removing these files from the DSC exposes a linker failure.

Recommendation:

1. Enable package CI (accept this change)
2. Add IntelFsp2Pkg.dsc to the "CompilerPlugin" "DscPath" in
    IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml to enable compilation
3. Verify compilation and all currently enabled package CI checks
    pass
4. Check-in fixes in (3) with change in (2)

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
  .pytool/CISettings.py             |  1 +
  IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml | 90 ++++++++++++++++++++
  2 files changed, 91 insertions(+)

diff --git a/.pytool/CISettings.py b/.pytool/CISettings.py
index cf9e0d77b19b..0205c26a58f8 100644
--- a/.pytool/CISettings.py
+++ b/.pytool/CISettings.py
@@ -54,6 +54,7 @@ class Settings(CiBuildSettingsManager, UpdateSettingsManager, SetupSettingsManag
                  "ArmVirtPkg",
                  "DynamicTablesPkg",
                  "EmulatorPkg",
+                "IntelFsp2Pkg",
                  "MdePkg",
                  "MdeModulePkg",
                  "NetworkPkg",
diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml b/IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml
new file mode 100644
index 000000000000..9ce401b20164
--- /dev/null
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.ci.yaml
@@ -0,0 +1,90 @@
+## @file
+# Core CI configuration for IntelFsp2Pkg
+#
+# Copyright (c) Microsoft Corporation
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+{
+    ## options defined .pytool/Plugin/LicenseCheck
+    "LicenseCheck": {
+        "IgnoreFiles": []
+    },
+
+    "EccCheck": {
+        ## Exception sample looks like below:
+        ## "ExceptionList": [
+        ##     "<ErrorID>", "<KeyWord>"
+        ## ]
+        "ExceptionList": [
+        ],
+        ## Both file path and directory path are accepted.
+        "IgnoreFiles": []
+    },
+
+    ## options defined .pytool/Plugin/CompilerPlugin
+    "CompilerPlugin": {
+        "DscPath": "IntelFsp2Pkg.dsc"
+    },
+
+    ## options defined .pytool/Plugin/HostUnitTestCompilerPlugin
+    "HostUnitTestCompilerPlugin": {
+        "DscPath": "" # Don't support this test
+    },
+
+    ## options defined .pytool/Plugin/CharEncodingCheck
+    "CharEncodingCheck": {
+        "IgnoreFiles": []
+    },
+
+    ## options defined .pytool/Plugin/DependencyCheck
+    "DependencyCheck": {
+        "AcceptableDependencies": [
+          "IntelFsp2Pkg/IntelFsp2Pkg.dec",
+          "MdeModulePkg/MdeModulePkg.dec",
+          "MdePkg/MdePkg.dec",
+          "UefiCpuPkg/UefiCpuPkg.dec"
+        ],
+        # For host based unit tests
+        "AcceptableDependencies-HOST_APPLICATION":[
+          "UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec"
+        ],
+        # For UEFI shell based apps
+        "AcceptableDependencies-UEFI_APPLICATION":[],
+        "IgnoreInf": []
+    },
+
+    ## options defined .pytool/Plugin/DscCompleteCheck
+    "DscCompleteCheck": {
+        "IgnoreInf": [""],
+        "DscPath": "IntelFsp2Pkg.dsc"
+    },
+
+    ## options defined .pytool/Plugin/HostUnitTestDscCompleteCheck
+    "HostUnitTestDscCompleteCheck": {
+        "IgnoreInf": [""],
+        "DscPath": "" # Don't support this test
+    },
+
+    ## options defined .pytool/Plugin/GuidCheck
+    "GuidCheck": {
+        "IgnoreGuidName": [],
+        "IgnoreGuidValue": [],
+        "IgnoreFoldersAndFiles": [],
+        "IgnoreDuplicates": [],
+    },
+
+    ## options defined .pytool/Plugin/LibraryClassCheck
+    "LibraryClassCheck": {
+        "IgnoreHeaderFile": []
+    },
+
+    ## options defined .pytool/Plugin/SpellCheck
+    "SpellCheck": {
+        "AuditOnly": True,           # Fails right now with over 270 errors
+        "IgnoreFiles": [],           # use gitignore syntax to ignore errors in matching files
+        "ExtendWords": [],           # words to extend to the dictionary for this package
+        "IgnoreStandardPaths": [],   # Standard Plugin defined paths that should be ignore
+        "AdditionalIncludePaths": [] # Additional paths to spell check (wildcards supported)
+    }
+}


Re: 回复: [edk2-devel] [RFC] Adoption of CodeQL in edk2

Michael Kubacki
 

I believe you are asking, when CodeQL CI is enabled in the edk2 project, how soon after will all of the issues be fixed so CI passes?

The process that will be used to enable CodeQL in CI will follow what is described in the "Enable One Query at a Time" section in the RFC (https://github.com/tianocore/edk2/discussions/3258).

As proposed in that section, there should not be a time when a new CodeQL CLI query is enabled that does not pass. Queries will be enabled one at a time. Each time a new query is enabled, the query enable and the corresponding changes will be staged on a branch that get merged to edk2 master in a single PR.

CodeQL CI will run in that PR and it must pass for the PR to be completed.

On 9/30/2022 2:33 AM, gaoliming wrote:
Michael:
 Could you estimate when  CodeQL CI  check can pass after CodeQL check is enabled?
Thanks
Liming
*发件人:*devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Michael D Kinney
*发送时间:*2022年9月30日9:03
*收件人:*devel@edk2.groups.io; mikuback@...; Kinney, Michael D <michael.d.kinney@...>
*主题:*Re: [edk2-devel] [RFC] Adoption of CodeQL in edk2
I just want to reiterate.  If there are no concerns or objections raised by Oct 4, then the
CodeQL static analysis will be phased into use in the edk2 repo and there will be code
changes made to address the issues identified by COdeQL and all future code changes
after a CodeQL check is enabled will be blocked until the CodeQL CI checks pass.
This will impact all future code changes and all developers will have to learn how to
interpret CodeQL reports and fix issues.
Thanks,
Mike
*From:*devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of *Michael Kubacki
*Sent:* Thursday, September 29, 2022 5:05 PM
*To:* Michael Kubacki <mikuback@... <mailto:mikuback@...>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
*Subject:* Re: [edk2-devel] [RFC] Adoption of CodeQL in edk2
If there's any further feedback on this RFC, please respond by Tuesday, October 4th. We plan to start implementing the changes later in the week.
Thanks,
Michael

1 - 20 of 94597