Adds support for virtio-mmio devices to microvm.

While being at it also add the README, the
patch somehow disappeared from the first batch.

v2:
- qemu changes needed for this are merged and will be
in the 6.2 release (dec 2021).
- more verbose commit messages.

Gerd Hoffmann (5):
OvmfPkg/Microvm/fdt: add device tree support
OvmfPkg/Microvm/fdt: load fdt from fw_cfg
OvmfPkg/Microvm/fdt: add empty fdt
OvmfPkg/Microvm/virtio: add virtio-mmio support
OvmfPkg/Microvm: add README

OvmfPkg/Microvm/MicrovmX64.dsc | 8 ++++
OvmfPkg/Microvm/MicrovmX64.fdf | 3 ++
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/Platform.c | 58 +++++++++++++++++++++++++++++
OvmfPkg/Microvm/README | 50 +++++++++++++++++++++++++
5 files changed, 120 insertions(+)
create mode 100644 OvmfPkg/Microvm/README

--
2.31.1

Needed for hardware detection: virtio-mmio devices for now,
later also pcie root bridge.

Depends on patched qemu which actually provides an fdt:
https://gitlab.com/kraxel/qemu/-/commits/sirius/microvm-device-tree

https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/Platform.c | 40 +++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 67eb7aa7166b..56876184b17a 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -44,6 +44,7 @@ [Packages]

[Guids]
gEfiMemoryTypeInformationGuid
+ gFdtHobGuid

[LibraryClasses]
BaseLib
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index df2d9ad015aa..3c0cdba67c83 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -321,6 +321,45 @@ PciExBarInitialization (
);
}

+VOID
+MicrovmInitialization (
+ VOID
+ )
+{
+ FIRMWARE_CONFIG_ITEM FdtItem;
+ UINTN FdtSize;
+ UINTN FdtPages;
+ EFI_STATUS Status;
+ UINT64 *FdtHobData;
+ VOID *NewBase;
+
+ Status = QemuFwCfgFindFile ("etc/fdt", &FdtItem, &FdtSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a: no etc/fdt found in fw_cfg\n", __FUNCTION__));
+ return;
+ }
+
+ FdtPages = EFI_SIZE_TO_PAGES (FdtSize);
+ NewBase = AllocatePages (FdtPages);
+ if (NewBase == NULL) {
+ DEBUG ((DEBUG_INFO, "%a: AllocatePages failed\n", __FUNCTION__));
+ return;
+ }
+
+ QemuFwCfgSelectItem (FdtItem);
+ QemuFwCfgReadBytes (FdtSize, NewBase);
+
+ FdtHobData = BuildGuidHob (&gFdtHobGuid, sizeof (*FdtHobData));
+ if (FdtHobData == NULL) {
+ DEBUG ((DEBUG_INFO, "%a: BuildGuidHob failed\n", __FUNCTION__));
+ return;
+ }
+
+ DEBUG ((DEBUG_INFO, "%a: fdt at 0x%x (size %d)\n", __FUNCTION__,
+ NewBase, FdtSize));
+ *FdtHobData = (UINTN)NewBase;
+}
+
VOID
MiscInitialization (
VOID
@@ -368,6 +407,7 @@ MiscInitialization (
break;
case 0xffff: /* microvm */
DEBUG ((DEBUG_INFO, "%a: microvm\n", __FUNCTION__));
+ MicrovmInitialization();
PcdStatus = PcdSet16S (PcdOvmfHostBridgePciDevId,
MICROVM_PSEUDO_DEVICE_ID);
ASSERT_RETURN_ERROR (PcdStatus);
--
2.31.1

Add virtio-mmio support (VirtioMmioDeviceLib and VirtioFdtDxe).

With this patch added and a new enough qemu version (6.2+) edk2
will detect virtio-mmio devices, so it is possible to boot from
storage (virtio-blk, virtio-scsi) or network (virtio-net).

https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Microvm/MicrovmX64.dsc | 2 ++
OvmfPkg/Microvm/MicrovmX64.fdf | 1 +
2 files changed, 3 insertions(+)

diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 27d2024266c2..85afdca9beba 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -233,6 +233,7 @@ [LibraryClasses.common]
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
+ VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf

[LibraryClasses.common.SEC]
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
@@ -743,6 +744,7 @@ [Components]
# device tree
#
EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
+ OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf

#
# SMBIOS Support
diff --git a/OvmfPkg/Microvm/MicrovmX64.fdf b/OvmfPkg/Microvm/MicrovmX64.fdf
index cc8892a459ee..0bf20a702764 100644
--- a/OvmfPkg/Microvm/MicrovmX64.fdf
+++ b/OvmfPkg/Microvm/MicrovmX64.fdf
@@ -278,6 +278,7 @@ [FV.DXEFV]
INF OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf

INF EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
+INF OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf

!if $(TOOL_CHAIN_TAG) != "XCODE5"
INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
--
2.31.1

FdtClient is unhappy without a device tree, so add an empty fdt
which we can use in case etc/fdt is not present in fw_cfg.

On ARM machines a device tree is mandatory for hardware detection,
thats why FdtClient fails hard.

On microvm the device tree is only used to detect virtio-mmio devices
(this patch series) and the pcie host (future series). So edk2 can
continue with limited functionality in case no device tree is present:
no storage, no network, but serial console and direct kernel boot
works.

qemu release 6.2 & newer will provide a device tree for microvm.

https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/PlatformPei/Platform.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 3c0cdba67c83..5071389c1fb4 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -16,6 +16,7 @@
//
// The Library classes this module consumes
//
+#include <Library/BaseMemoryLib.h>
#include <Library/BaseLib.h>
#include <Library/DebugLib.h>
#include <Library/HobLib.h>
@@ -321,6 +322,18 @@ PciExBarInitialization (
);
}

+static const UINT8 EmptyFdt[] = {
+ 0xd0, 0x0d, 0xfe, 0xed, 0x00, 0x00, 0x00, 0x48,
+ 0x00, 0x00, 0x00, 0x38, 0x00, 0x00, 0x00, 0x48,
+ 0x00, 0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x11,
+ 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x09,
+};
+
VOID
MicrovmInitialization (
VOID
@@ -335,8 +348,9 @@ MicrovmInitialization (

Status = QemuFwCfgFindFile ("etc/fdt", &FdtItem, &FdtSize);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_INFO, "%a: no etc/fdt found in fw_cfg\n", __FUNCTION__));
- return;
+ DEBUG ((DEBUG_INFO, "%a: no etc/fdt found in fw_cfg, using dummy\n", __FUNCTION__));
+ FdtItem = 0;
+ FdtSize = sizeof(EmptyFdt);
}

FdtPages = EFI_SIZE_TO_PAGES (FdtSize);
@@ -346,8 +360,12 @@ MicrovmInitialization (
return;
}

- QemuFwCfgSelectItem (FdtItem);
- QemuFwCfgReadBytes (FdtSize, NewBase);
+ if (FdtItem) {
+ QemuFwCfgSelectItem (FdtItem);
+ QemuFwCfgReadBytes (FdtSize, NewBase);
+ } else {
+ CopyMem(NewBase, EmptyFdt, FdtSize);
+ }

FdtHobData = BuildGuidHob (&gFdtHobGuid, sizeof (*FdtHobData));
if (FdtHobData == NULL) {
--
2.31.1

Add fdt parser from EmbeddedPkg (FdtLib and FdtClientDxe) to MicrovmX64.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Microvm/MicrovmX64.dsc | 6 ++++++
OvmfPkg/Microvm/MicrovmX64.fdf | 2 ++
2 files changed, 8 insertions(+)

diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 617f92539518..27d2024266c2 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -232,6 +232,7 @@ [LibraryClasses.common]
VmgExitLib|OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
+ FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf

[LibraryClasses.common.SEC]
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
@@ -738,6 +739,11 @@ [Components]
#
MdeModulePkg/Universal/SerialDxe/SerialDxe.inf

+ #
+ # device tree
+ #
+ EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
+
#
# SMBIOS Support
#
diff --git a/OvmfPkg/Microvm/MicrovmX64.fdf b/OvmfPkg/Microvm/MicrovmX64.fdf
index 6314014f3de7..cc8892a459ee 100644
--- a/OvmfPkg/Microvm/MicrovmX64.fdf
+++ b/OvmfPkg/Microvm/MicrovmX64.fdf
@@ -277,6 +277,8 @@ [FV.DXEFV]
INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
INF OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf

+INF EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
+
!if $(TOOL_CHAIN_TAG) != "XCODE5"
INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
--
2.31.1

From: Michael Kubacki <michael.kubacki@...>

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

V2 of the PCH SPI PPI and PCH SPI Protocol were recently added to
IntelSiliconPkg. This change removes the v1 definitions.

V2 is intended to better support multiple silicon generations which
aligns with the goals of IntelSiliconPkg.

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Isaac Oram <isaac.w.oram@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Silicon/Intel/IntelSiliconPkg/Include/Ppi/Spi.h | 25 --
Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h | 301 -------------=
-------
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 7 -
3 files changed, 333 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Ppi/Spi.h b/Silicon/In=
tel/IntelSiliconPkg/Include/Ppi/Spi.h
deleted file mode 100644
index b2410bd17300..000000000000
--- a/Silicon/Intel/IntelSiliconPkg/Include/Ppi/Spi.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/** @file
- This file defines the PCH SPI PPI which implements the
- Intel(R) PCH SPI Host Controller Compatibility Interface.
-
- Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
- SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-#ifndef _PCH_SPI_PPI_H_
-#define _PCH_SPI_PPI_H_
-
-#include <Protocol/Spi.h>
-
-//
-// Extern the GUID for PPI users.
-//
-extern EFI_GUID gPchSpiPpiGuid;
-
-/**
- Reuse the PCH_SPI_PROTOCOL definitions
- This is possible becaues the PPI implementation does not rely on a Pei=
Service pointer,
- as it uses EDKII Glue Lib to do IO accesses
-**/
-typedef PCH_SPI_PROTOCOL PCH_SPI_PPI;
-
-#endif
diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h b/Silic=
on/Intel/IntelSiliconPkg/Include/Protocol/Spi.h
deleted file mode 100644
index c13dc5a5f5f5..000000000000
--- a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h
+++ /dev/null
@@ -1,301 +0,0 @@
-/** @file
- This file defines the PCH SPI Protocol which implements the
- Intel(R) PCH SPI Host Controller Compatibility Interface.
-
- Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
- SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-#ifndef _PCH_SPI_PROTOCOL_H_
-#define _PCH_SPI_PROTOCOL_H_
-
-//
-// Extern the GUID for protocol users.
-//
-extern EFI_GUID gPchSpiProtocolGuid;
-extern EFI_GUID gPchSmmSpiProtocolGuid;
-
-//
-// Forward reference for ANSI C compatibility
-//
-typedef struct _PCH_SPI_PROTOCOL PCH_SPI_PROTOCOL;
-
-//
-// SPI protocol data structures and definitions
-//
-
-/**
- Flash Region Type
-**/
-typedef enum {
- FlashRegionDescriptor,
- FlashRegionBios,
- FlashRegionMe,
- FlashRegionGbE,
- FlashRegionPlatformData,
- FlashRegionDer,
- FlashRegionSecondaryBios,
- FlashRegionuCodePatch,
- FlashRegionEC,
- FlashRegionDeviceExpansion2,
- FlashRegionIE,
- FlashRegion10Gbe_A,
- FlashRegion10Gbe_B,
- FlashRegion13,
- FlashRegion14,
- FlashRegion15,
- FlashRegionAll,
- FlashRegionMax
-} FLASH_REGION_TYPE;
-//
-// Protocol member functions
-//
-
-/**
- Read data from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
- @param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
- @param[out] Buffer The Pointer to caller-allocated buffer=
containing the dada received.
- It is the caller's responsibility to m=
ake sure Buffer is large enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- OUT UINT8 *Buffer
- );
-
-/**
- Write data to the flash part. Remark: Erase may be needed before write=
to the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
- @param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
- @param[in] Buffer Pointer to caller-allocated buffer con=
taining the data sent during the SPI cycle.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_WRITE) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- IN UINT8 *Buffer
- );
-
-/**
- Erase some area on the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
- @param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_ERASE) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount
- );
-
-/**
- Read SFDP data from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] ComponentNumber The Componen Number for chip select
- @param[in] Address The starting byte address for SFDP dat=
a read.
- @param[in] ByteCount Number of bytes in SFDP data portion o=
f the SPI cycle
- @param[out] SfdpData The Pointer to caller-allocated buffer=
containing the SFDP data received
- It is the caller's responsibility to m=
ake sure Buffer is large enough for the total number of bytes read
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_SFDP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT8 ComponentNumber,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- OUT UINT8 *SfdpData
- );
-
-/**
- Read Jedec Id from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] ComponentNumber The Componen Number for chip select
- @param[in] ByteCount Number of bytes in JedecId data portio=
n of the SPI cycle, the data size is 3 typically
- @param[out] JedecId The Pointer to caller-allocated buffer=
containing JEDEC ID received
- It is the caller's responsibility to m=
ake sure Buffer is large enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_JEDEC_ID) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT8 ComponentNumber,
- IN UINT32 ByteCount,
- OUT UINT8 *JedecId
- );
-
-/**
- Write the status register in the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] ByteCount Number of bytes in Status data portion=
of the SPI cycle, the data size is 1 typically
- @param[in] StatusValue The Pointer to caller-allocated buffer=
containing the value of Status register writing
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_WRITE_STATUS) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 ByteCount,
- IN UINT8 *StatusValue
- );
-
-/**
- Read status register in the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] ByteCount Number of bytes in Status data portion=
of the SPI cycle, the data size is 1 typically
- @param[out] StatusValue The Pointer to caller-allocated buffer=
containing the value of Status register received.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_STATUS) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 ByteCount,
- OUT UINT8 *StatusValue
- );
-
-/**
- Get the SPI region base and size, based on the enum type
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for for the base=
address which is listed in the Descriptor.
- @param[out] BaseAddress The Flash Linear Address for the Regio=
n 'n' Base
- @param[out] RegionSize The size for the Region 'n'
-
- @retval EFI_SUCCESS Read success
- @retval EFI_INVALID_PARAMETER Invalid region type given
- @retval EFI_DEVICE_ERROR The region is not used
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_GET_REGION_ADDRESS) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- OUT UINT32 *BaseAddress,
- OUT UINT32 *RegionSize
- );
-
-/**
- Read PCH Soft Strap Values
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] SoftStrapAddr PCH Soft Strap address offset from FPS=
BA.
- @param[in] ByteCount Number of bytes in SoftStrap data port=
ion of the SPI cycle
- @param[out] SoftStrapValue The Pointer to caller-allocated buffer=
containing PCH Soft Strap Value.
- If the value of ByteCount is 0, the da=
ta type of SoftStrapValue should be UINT16 and SoftStrapValue will be PCH=
Soft Strap Length
- It is the caller's responsibility to m=
ake sure Buffer is large enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_READ_PCH_SOFTSTRAP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 SoftStrapAddr,
- IN UINT32 ByteCount,
- OUT VOID *SoftStrapValue
- );
-
-/**
- Read CPU Soft Strap Values
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] SoftStrapAddr CPU Soft Strap address offset from FCP=
USBA.
- @param[in] ByteCount Number of bytes in SoftStrap data port=
ion of the SPI cycle.
- @param[out] SoftStrapValue The Pointer to caller-allocated buffer=
containing CPU Soft Strap Value.
- If the value of ByteCount is 0, the da=
ta type of SoftStrapValue should be UINT16 and SoftStrapValue will be PCH=
Soft Strap Length
- It is the caller's responsibility to m=
ake sure Buffer is large enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid=
.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormall=
y.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_READ_CPU_SOFTSTRAP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 SoftStrapAddr,
- IN UINT32 ByteCount,
- OUT VOID *SoftStrapValue
- );
-
-/**
- These protocols/PPI allows a platform module to perform SPI operations=
through the
- Intel PCH SPI Host Controller Interface.
-**/
-struct _PCH_SPI_PROTOCOL {
- /**
- This member specifies the revision of this structure. This field is =
used to
- indicate backwards compatible changes to the protocol.
- **/
- UINT8 Revision;
- PCH_SPI_FLASH_READ FlashRead; ///< Read data f=
rom the flash part.
- PCH_SPI_FLASH_WRITE FlashWrite; ///< Write data =
to the flash part. Remark: Erase may be needed before write to the flash =
part.
- PCH_SPI_FLASH_ERASE FlashErase; ///< Erase some =
area on the flash part.
- PCH_SPI_FLASH_READ_SFDP FlashReadSfdp; ///< Read SFDP d=
ata from the flash part.
- PCH_SPI_FLASH_READ_JEDEC_ID FlashReadJedecId; ///< Read Jedec =
Id from the flash part.
- PCH_SPI_FLASH_WRITE_STATUS FlashWriteStatus; ///< Write the s=
tatus register in the flash part.
- PCH_SPI_FLASH_READ_STATUS FlashReadStatus; ///< Read status=
register in the flash part.
- PCH_SPI_GET_REGION_ADDRESS GetRegionAddress; ///< Get the SPI=
region base and size
- PCH_SPI_READ_PCH_SOFTSTRAP ReadPchSoftStrap; ///< Read PCH So=
ft Strap Values
- PCH_SPI_READ_CPU_SOFTSTRAP ReadCpuSoftStrap; ///< Read CPU So=
ft Strap Values
-};
-
-/**
- PCH SPI PPI/PROTOCOL revision number
-
- Revision 1: Initial version
-**/
-#define PCH_SPI_SERVICES_REVISION 1
-
-#endif
diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/=
Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index 1704f9e02541..f950c3d1c72b 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -93,9 +93,6 @@ [Guids]
gFlashRegionMaxGuid =3D { 0x74c2e3c1, 0x8faa, 0x4659, {0=
xa7, 0xbb, 0x87, 0x1f, 0xbb, 0x61, 0xd3, 0xb4 } }
=20
[Ppis]
- ## Include/Ppi/Spi.h
- gPchSpiPpiGuid =3D { 0x104c7177, 0xc2e6, 0x44f0, { 0xae, 0xe3, 0x9d, 0=
x0d, 0x9a, 0x52, 0xca, 0xdf } }
-
## Include/Ppi/Spi2.h
gPchSpi2PpiGuid =3D { 0x63c40580, 0x10c4, 0x4a8e, { 0xb4, 0x16, 0x86, =
0x85, 0x25, 0x7e, 0xce, 0x04 } }
=20
@@ -105,10 +102,6 @@ [Ppis]
[Protocols]
## Protocols that provide services for the Intel(R) PCH SPI Host Contr=
oller Compatibility Interface
=20
- # Include/Protocol/Spi.h
- gPchSpiProtocolGuid =3D { 0xe007dec0, 0xccc3, 0x4c90, { 0x9c, 0xd0, =
0xef, 0x99, 0x38, 0x83, 0x28, 0xcf } }
- gPchSmmSpiProtocolGuid =3D { 0x4840e48e, 0xc264, 0x4fef, { 0xb9, 0x34,=
0x14, 0x84, 0x0c, 0x95, 0xd8, 0x3f } }
-
# Include/Protocol/Spi2.h
gPchSpi2ProtocolGuid =3D { 0x3a99abd1, 0x096c, 0x4399, { 0xb1, 0x68, 0=
x52, 0xaa, 0x52, 0x64, 0xce, 0x70 } }
gPchSmmSpi2ProtocolGuid =3D { 0x2d1c0c43, 0x20d3, 0x40ae, { 0x99, 0x07=
, 0x2d, 0xf0, 0xe7, 0x91, 0x21, 0xa5 } }
--=20
2.28.0.windows.1

From: Michael Kubacki <michael.kubacki@...>

This contents of the header file are not referenced in this
source file.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib=
/PeiPchPolicyUpdate.c | 1 -
Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib=
/PeiPolicyUpdateLib.inf | 1 -
2 files changed, 2 deletions(-)

diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPol=
icyUpdateLib/PeiPchPolicyUpdate.c b/Platform/Intel/WhiskeylakeOpenBoardPk=
g/Policy/Library/PeiPolicyUpdateLib/PeiPchPolicyUpdate.c
index ebce9b271a2d..049ca6a604bd 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpda=
teLib/PeiPchPolicyUpdate.c
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpda=
teLib/PeiPchPolicyUpdate.c
@@ -17,7 +17,6 @@
#include <Library/PchPcrLib.h>
#include <Library/PchSerialIoLib.h>
#include <Library/PchPcieRpLib.h>
-#include <Ppi/Spi.h>
#include <GpioConfig.h>
#include <Library/DebugLib.h>
#include <Library/PchGbeLib.h>
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPol=
icyUpdateLib/PeiPolicyUpdateLib.inf b/Platform/Intel/WhiskeylakeOpenBoard=
Pkg/Policy/Library/PeiPolicyUpdateLib/PeiPolicyUpdateLib.inf
index e44cf5f02ac7..af5d4dfeb7ca 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpda=
teLib/PeiPolicyUpdateLib.inf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpda=
teLib/PeiPolicyUpdateLib.inf
@@ -266,7 +266,6 @@ [Sources]
=20
[Ppis]
gWdtPpiGuid ## CONSUMES
- gPchSpi2PpiGuid ## CONSUMES
gSiPolicyPpiGuid ## CONSUMES
gSiPreMemPolicyPpiGuid ## CONSUMES
gPeiTbtPolicyPpiGuid ## CONSUMES
--=20
2.28.0.windows.1

From: Michael Kubacki <michael.kubacki@...>

This contents of the header file are not referenced in this
source file.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib/P=
eiPchPolicyUpdate.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolic=
yUpdateLib/PeiPchPolicyUpdate.c b/Platform/Intel/CometlakeOpenBoardPkg/Po=
licy/Library/PeiPolicyUpdateLib/PeiPchPolicyUpdate.c
index 4a8ffa7226da..76de6e90c728 100644
--- a/Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyUpdate=
Lib/PeiPchPolicyUpdate.c
+++ b/Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyUpdate=
Lib/PeiPchPolicyUpdate.c
@@ -17,7 +17,6 @@
#include <Library/PchPcrLib.h>
#include <Library/PchSerialIoLib.h>
#include <Library/PchPcieRpLib.h>
-#include <Ppi/Spi.h>
#include <GpioConfig.h>
#include <Library/DebugLib.h>
#include <Library/PchGbeLib.h>
--=20
2.28.0.windows.1

From: Michael Kubacki <michael.kubacki@...>

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

V2 of the PCH SPI PPI and PCH SPI Protocol were recently added to
IntelSiliconPkg. This change removes the v1 definitions.

V2 is intended to better support multiple silicon generations which
aligns with the goals of IntelSiliconPkg.

Minor changes are also made in board packages that have stale
references to the SPI PPI and Protocol.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
Cc: Ray Ni <ray.ni@...>
Cc: Isaac Oram <isaac.w.oram@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (3):
CometlakeOpenBoardPkg/PeiPolicyUpdateLib: Remove unneeded SPI header
WhiskeylakeOpenBoardPkg/PeiPolicyUpdateLib: Remove unneeded SPI header
IntelSiliconPkg: Remove SPI v1 PPI and Protocol definitions

Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib/P=
eiPchPolicyUpdate.c | 1 -
Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib=
/PeiPchPolicyUpdate.c | 1 -
Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib=
/PeiPolicyUpdateLib.inf | 1 -
Silicon/Intel/IntelSiliconPkg/Include/Ppi/Spi.h =
| 25 --
Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h =
| 301 --------------------
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec =
| 7 -
6 files changed, 336 deletions(-)
delete mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Ppi/Spi.h
delete mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h

--=20
2.28.0.windows.1

On 11/3/21 10:11, Gerd Hoffmann wrote:

Add build test for OvmfPkg/AmdSev.

Acked-by: Jiewen Yao <Jiewen.yao@...>
Acked-by: Ard Biesheuvel <ardb@...>
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
.../.azurepipelines/Ubuntu-GCC5.yml | 10 +++++
OvmfPkg/PlatformCI/AmdSevBuild.py | 37 +++++++++++++++++++
2 files changed, 47 insertions(+)
create mode 100644 OvmfPkg/PlatformCI/AmdSevBuild.py

Reviewed-by: Philippe Mathieu-Daude <philmd@...>

On 11/3/21 10:11, Gerd Hoffmann wrote:

Add build test for OvmfPkg/Microvm.

Acked-by: Jiewen Yao <Jiewen.yao@...>
Acked-by: Ard Biesheuvel <ardb@...>
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
.../.azurepipelines/Ubuntu-GCC5.yml | 10 +++++
OvmfPkg/PlatformCI/MicrovmBuild.py | 37 +++++++++++++++++++
2 files changed, 47 insertions(+)
create mode 100644 OvmfPkg/PlatformCI/MicrovmBuild.py

Reviewed-by: Philippe Mathieu-Daude <philmd@...>

On 11/3/21 10:11, Gerd Hoffmann wrote:

Skip the qemu boot test in case QEMU_SKIP is set to true.

Acked-by: Jiewen Yao <Jiewen.yao@...>
Acked-by: Ard Biesheuvel <ardb@...>
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/PlatformCI/PlatformBuildLib.py | 5 +++++
1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daude <philmd@...>

On 11/3/21 04:05, Michael D Kinney wrote:

When using will_return() on a pointer value, it must be
cast to UINTN to be compatible with 32-bit GCC builds.
This uses the same approach in samples provided in the
UnitTestFramworkPkg when passing pointer values to
UT_ASSERT_EQUAL().

Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <gaoliming@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Signed-off-by: Michael D Kinney <michael.d.kinney@...>
---
.../RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daude <philmd@...>

On 11/3/21 04:05, Michael D Kinney wrote:

When using UT_ASSERT_EQUAL() on a pointer value, it must be
cast to UINTN. This follows the samples provided with the
UnitTestFrameworkPkg.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Signed-off-by: Michael D Kinney <michael.d.kinney@...>
---
UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daude <philmd@...>

On November 3, 2021 2:51 PM, Gerd Hoffmann wrote:

+/**
+ Check if it is Tdx guest
+
+ @retval TRUE It is Tdx guest
+ @retval FALSE It is not Tdx guest
+**/
+BOOLEAN
+QemuFwCfgIsTdxGuest (

QemuFwCfgIsCC()

+ return (CcWorkAreaHeader != NULL && CcWorkAreaHeader-
GuestType == GUEST_TYPE_INTEL_TDX);

GuestType != GUEST_TYPE_NON_ENCRYPTED

if (MemEncryptSevIsEnabled ()) {
DEBUG ((DEBUG_INFO, "SEV: QemuFwCfg fallback to IO Port

interface.\n"));

+ } else if (QemuFwCfgIsTdxGuest ()) {

if (QemuFwCfgIsCC()

Hi, Gerd
I re-check the MemEncryptSevIsEnabled() and it doesn't simply check the GuestType. Instead it does more checking.
See https://github.com/tianocore/edk2/blob/master/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c#L34-L88

Brijesh, what's your thought about Gerd's suggestion?

Thanks
Min

On November 3, 2021 2:51 PM, Gerd Hoffmann wrote:

+/**
+ Check if it is Tdx guest
+
+ @retval TRUE It is Tdx guest
+ @retval FALSE It is not Tdx guest
+**/
+BOOLEAN
+QemuFwCfgIsTdxGuest (

QemuFwCfgIsCC()

+ return (CcWorkAreaHeader != NULL && CcWorkAreaHeader-
GuestType == GUEST_TYPE_INTEL_TDX);

GuestType != GUEST_TYPE_NON_ENCRYPTED

if (MemEncryptSevIsEnabled ()) {
DEBUG ((DEBUG_INFO, "SEV: QemuFwCfg fallback to IO Port

interface.\n"));

+ } else if (QemuFwCfgIsTdxGuest ()) {

if (QemuFwCfgIsCC()

Thanks for reminder. It will be updated in the next version.

Thanks
Min

On November 3, 2021 2:09 PM, Gerd Hoffmann wrote:

+++ b/UefiCpuPkg/Library/MpInitLib/X64/IntelTdcall.nasm
@@ -0,0 +1,120 @@
+;--------------------------------------------------------------------
+----------
+;*
+;* Copyright (c) 2020 - 2021, Intel Corporation. All rights
+reserved.<BR>
+;* SPDX-License-Identifier: BSD-2-Clause-Patent
+;*
+;*
+;--------------------------------------------------------------------
+----------
+
+DEFAULT REL
+SECTION .text
+
+%macro tdcall 0
+ db 0x66,0x0f,0x01,0xcc
+%endmacro

Hmm, could you just use TdxLib instead of bringing your own copy of the
assembler code?

My initial thought was to include TdxLib in the .dsc as little as possible. For example, DxeMpInitLib is included in OvmfPkg/Microvm/MicrovmX64.dsc. If TdxLib is used by DxeMpInitLib, then it has to be included in MicrovmX64.dsc as well.
So I copy the assemble code in MpInitLib.
But now VmgExitLib is extended to handle #VE exception ( TdxLib must be used ), so TdxLib has to be included in the .dsc where VmgExitLib is included.
I will update the MpInitLib to use TdxLib in the next version.

Thanks
Min

Hi Mike,

Thank you for this patch. These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On Tue, Nov 02, 2021 at 16:34:15 -0700, Andrew Fish via groups.io wrote:

No objections to this for any of my use-cases, but I'd like for one of
the BaseTools maintainers to comment on whether this breaks anything
with regards to EDK_TOOLS_PATH, or if we can finally get rid of that
and replace it with $WORKSPACE/BaseTools globally.

Our internal repo uses EDK_TOOLS_PATH. In our case it is
$(WORKSPACE)/edk2/BaseTools. We have a PACKAGES_PATH set to
$(WORKSPACE)/edk2 and magic happens.

So I’m thinking maybe:
$(EDK_TOOLS_PATH)/Bin/GccLto
Vs:
$(WORKSPACE)/BaseTools/Bin/GccLto

If EDK_TOOLS_PATH gets ripped out, it should be one atomic remove,
but let us not stick that on Bret.

Agreed. I'd be happy with your proposed solution.

/
Leif

Hi Rebecca,

Apologies, I missed reviewing this patch.

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar

Add a new MpServicesTest application under MdeModulePkg/Application that
exercises the EFI_MP_SERVICES_PROTOCOL.

Signed-off-by: Rebecca Cran <rebecca@...>
---
 MdeModulePkg/Application/MpServicesTest/MpServicesTest.c   | 433 ++++++++++++++++++++
 MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf |  38 ++
 MdeModulePkg/MdeModulePkg.dsc                              |   2 +
 3 files changed, 473 insertions(+)

diff --git a/MdeModulePkg/Application/MpServicesTest/MpServicesTest.c b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.c
new file mode 100644
index 000000000000..4eb06e6b7cbd
--- /dev/null
+++ b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.c
@@ -0,0 +1,433 @@
+/** @file
+
+    Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+    SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+#include <Library/DebugLib.h>
+#include <Library/RngLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Pi/PiMultiPhase.h>
+#include <Protocol/MpService.h>
+
+#define MAX_RANDOM_PROCESSOR_RETRIES 10
+
+#define AP_STARTUP_TEST_TIMEOUT_US  50000
+#define INFINITE_TIMEOUT            0
+
+#define RETURN_IF_EFI_ERROR(x)   \
+  if (EFI_ERROR (x)) {           \
+    Print (L"failed: %r\n", x);  \
+    return;                      \
+  }                              \
+  else {                         \
+    Print (L"done.\n");          \
+  }
+
+/** The procedure to run with the MP Services interface.
+
+  @param Buffer The procedure argument.
+
+**/
+STATIC
+VOID
+EFIAPI
+ApFunction (
+  IN OUT VOID *Buffer
+  )
+{
+}
+
+/** Displays information returned from MP Services Protocol.
+
+  @param Mp  The MP Services Protocol
+
+  @return The number of CPUs in the system.
+
+**/
+STATIC
+UINTN
+PrintProcessorInformation (
+  IN EFI_MP_SERVICES_PROTOCOL *Mp
+  )
+{
+  EFI_STATUS                 Status;
+  EFI_PROCESSOR_INFORMATION  CpuInfo;
+  UINTN                      Index;
+  UINTN                      NumCpu;
+  UINTN                      NumEnabledCpu;
+
+  Status = Mp->GetNumberOfProcessors (Mp, &NumCpu, &NumEnabledCpu);
+  if (EFI_ERROR (Status)) {
+    Print (L"GetNumberOfProcessors failed: %r\n", Status);
+  } else {
+    Print (L"Number of CPUs: %ld, Enabled: %d\n", NumCpu, NumEnabledCpu);
+  }
+
+  for (Index = 0; Index < NumCpu; Index++) {
+    Status = Mp->GetProcessorInfo (Mp, CPU_V2_EXTENDED_TOPOLOGY | Index, &CpuInfo);
+    if (EFI_ERROR (Status)) {
+      Print (L"GetProcessorInfo for Processor %d failed: %r\n", Index, Status);
+    } else {
+      Print (
+        L"Processor %d:\n"
+        L"\tID: %016lx\n"
+        L"\tStatus: %s | ",
+        Index,
+        CpuInfo.ProcessorId,
+        (CpuInfo.StatusFlag & PROCESSOR_AS_BSP_BIT) ? L"BSP" : L"AP"
+        );
+
+      Print (L"%s | ", (CpuInfo.StatusFlag & PROCESSOR_ENABLED_BIT) ? L"Enabled" : L"Disabled");
+      Print (L"%s\n", (CpuInfo.StatusFlag & PROCESSOR_HEALTH_STATUS_BIT) ? L"Healthy" : L"Faulted");
+
+      Print (
+        L"\tLocation: Package %d, Core %d, Thread %d\n"
+        L"\tExtended Information: Package %d, Module %d, Tile %d, Die %d, Core %d, Thread %d\n\n",
+        CpuInfo.Location.Package,
+        CpuInfo.Location.Core,
+        CpuInfo.Location.Thread,
+        CpuInfo.ExtendedInformation.Location2.Package,
+        CpuInfo.ExtendedInformation.Location2.Module,
+        CpuInfo.ExtendedInformation.Location2.Tile,
+        CpuInfo.ExtendedInformation.Location2.Die,
+        CpuInfo.ExtendedInformation.Location2.Core,
+        CpuInfo.ExtendedInformation.Location2.Thread
+        );
+    }
+  }
+
+  return NumCpu;
+}
+
+/** Returns the index of an enabled AP selected at random.
+
+  @param Mp             The MP Services Protocol.
+  @param ProcessorIndex The index of a random enabled AP.
+
+  @retval EFI_SUCCESS   An enabled processor was found and returned.
+  @retval EFI_NOT_FOUND A processor was unable to be selected.
+
+**/
+STATIC
+EFI_STATUS
+GetRandomEnabledProcessorIndex (
+  IN EFI_MP_SERVICES_PROTOCOL *Mp,
+  OUT UINTN *ProcessorIndex
+  )
+{
+  UINTN                      Index;
+  UINTN                      IndexOfEnabledCpu;
+  UINTN                      NumCpus;
+  UINTN                      NumEnabledCpus;
+  UINTN                      IndexOfEnabledCpuToUse;
+  UINT16                     RandomNumber;
+  BOOLEAN                    Success;
+  EFI_STATUS                 Status;
+  EFI_PROCESSOR_INFORMATION  CpuInfo;
+
+  IndexOfEnabledCpu = 0;
+
+  Success = GetRandomNumber16 (&RandomNumber);
+  ASSERT (Success == TRUE);
+
+  Status = Mp->GetNumberOfProcessors (Mp, &NumCpus, &NumEnabledCpus);
+  ASSERT_EFI_ERROR (Status);
+
+  if (NumEnabledCpus == 1) {
+    Print (L"All APs are disabled\n");
+    return EFI_NOT_FOUND;
+  }
+
+  IndexOfEnabledCpuToUse = RandomNumber % NumEnabledCpus;
+
+  for (Index = 0; Index < NumCpus; Index++) {
+    Status = Mp->GetProcessorInfo (Mp, Index, &CpuInfo);
+    ASSERT_EFI_ERROR (Status);
+    if ((CpuInfo.StatusFlag & PROCESSOR_ENABLED_BIT) &&
+        !(CpuInfo.StatusFlag & PROCESSOR_AS_BSP_BIT)) {
+      if (IndexOfEnabledCpuToUse == IndexOfEnabledCpu) {
+        *ProcessorIndex = Index;
+        Status = EFI_SUCCESS;
+        break;

[SAMI] Minor. I think it should be possible to return from here. In that case the check below if (Index == NumCpus) is not needed and EFI_NOT_FOUND can be returned at the end of the function.

+      }
+
+      IndexOfEnabledCpu++;
+    }
+  }
+
+  if (Index == NumCpus) {
+    Status = EFI_NOT_FOUND;
+  }
+
+  return Status;
+}
+
+/** Tests for the StartupThisAP function.
+
+  @param Mp The MP Services Protocol.
+
+**/
+STATIC
+VOID
+StartupThisApTests (
+  IN EFI_MP_SERVICES_PROTOCOL *Mp
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       ProcessorIndex;
+  UINT32      Retries;
+
+  Retries = 0;
+
+  do {
+    Status = GetRandomEnabledProcessorIndex (Mp, &ProcessorIndex);
+  } while (EFI_ERROR (Status) && Retries++ < MAX_RANDOM_PROCESSOR_RETRIES);
+
+  if (EFI_ERROR (Status)) {
+    return;
+  }
+
+  Print (
+    L"StartupThisAP on Processor %d with 0 (infinite) timeout...",
+    ProcessorIndex
+    );
+
+  Status = Mp->StartupThisAP (
+    Mp,
+    ApFunction,
+    ProcessorIndex,
+    NULL,
+    INFINITE_TIMEOUT,
+    NULL,
+    NULL
+    );

[SAMI] Please align the above code.

+
+  RETURN_IF_EFI_ERROR (Status);

[SAMI] I can see that the RETURN_IF_EFI_ERROR() macro is avoiding repetitive code. But I am concerned that the return statement in the macro is hiding the program flow.

+
+  Retries = 0;
+
+  do {
+    Status = GetRandomEnabledProcessorIndex (Mp, &ProcessorIndex);
+  } while (EFI_ERROR (Status) && Retries++ < MAX_RANDOM_PROCESSOR_RETRIES);
+
+  if (EFI_ERROR (Status)) {
+    return;
+  }
+
+  Print (
+    L"StartupThisAP on Processor %d with %dms timeout...",
+    ProcessorIndex,
+    AP_STARTUP_TEST_TIMEOUT_US / 1000
+    );
+  Status = Mp->StartupThisAP (
+                 Mp,
+                 ApFunction,
+                 ProcessorIndex,
+                 NULL,
+                 AP_STARTUP_TEST_TIMEOUT_US,
+                 NULL,
+                 NULL
+                 );
+  RETURN_IF_EFI_ERROR (Status);
+}
+
+/** Tests for the StartupAllAPs function.
+
+  @param Mp      The MP Services Protocol.
+  @param NumCpus The number of CPUs in the system.
+
+**/
+STATIC
+VOID
+StartupAllAPsTests (
+  IN EFI_MP_SERVICES_PROTOCOL *Mp,
+  IN UINTN NumCpus
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       Timeout;
+
+  Print (L"Running with SingleThread FALSE, 0 (infinite) timeout...");
+  Status = Mp->StartupAllAPs (Mp, ApFunction, FALSE, NULL, INFINITE_TIMEOUT, NULL, NULL);
+  RETURN_IF_EFI_ERROR (Status);
+
+  Timeout = NumCpus * AP_STARTUP_TEST_TIMEOUT_US;
+
+  Print (L"Running with SingleThread TRUE, %dms timeout...", Timeout / 1000);
+  Status = Mp->StartupAllAPs (
+                 Mp,
+                 ApFunction,
+                 TRUE,
+                 NULL,
+                 Timeout,
+                 NULL,
+                 NULL
+                 );
+  RETURN_IF_EFI_ERROR (Status);
+}
+
+/** Tests for the EnableDisableAP function.
+
+  @param Mp      The MP Services Protocol.
+  @param NumCpus The number of CPUs in the system.
+
+**/
+STATIC
+VOID
+EnableDisableAPTests (
+  IN EFI_MP_SERVICES_PROTOCOL *Mp,
+  IN UINTN                    NumCpus
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       Index;
+  UINT32      HealthFlag;
+
+  HealthFlag = 0;
+
+  for (Index = 1; Index < NumCpus; Index++) {
+    Print (L"Disabling Processor %d with HealthFlag faulted...", Index);
+    Status = Mp->EnableDisableAP (Mp, Index, FALSE, &HealthFlag);
+    RETURN_IF_EFI_ERROR (Status);
+  }
+
+  HealthFlag = PROCESSOR_HEALTH_STATUS_BIT;
+
+  for (Index = 1; Index < NumCpus; Index++) {
+    Print (L"Enabling Processor %d with HealthFlag healthy...", Index);
+    Status = Mp->EnableDisableAP (Mp, Index, TRUE, &HealthFlag);
+    RETURN_IF_EFI_ERROR (Status);
+  }
+}
+
+/** Tests for the SwitchBSP function.
+
+  @param Mp      The MP Services Protocol.
+  @param NumCpus The number of CPUs in the system.
+
+**/
+STATIC
+VOID
+SwitchBSPTests (
+  IN EFI_MP_SERVICES_PROTOCOL *Mp,
+  IN UINTN                    NumCpus
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       Index;
+
+  for (Index = 1; Index < NumCpus; Index++) {
+    Print (L"Switching BSP to Processor %d with EnableOldBSP FALSE...", Index);
+    Status = Mp->SwitchBSP (Mp, Index, FALSE);

[SAMI] The SwitchBsp call can only be performed by the current BSP. So, I am not sure if this would work in a for loop.

+    RETURN_IF_EFI_ERROR (Status);
+  }
+
+  for (Index = 0; Index < NumCpus; Index++) {
+    Print (L"Switching BSP to Processor %d with EnableOldBSP TRUE...", Index);
+    Status = Mp->SwitchBSP (Mp, Index, TRUE);
+    RETURN_IF_EFI_ERROR (Status);
+  }
+}
+
+/**
+  The user Entry Point for Application. The user code starts with this function
+  as the real entry point for the application.
+
+  @param[in] ImageHandle    The firmware allocated handle for the EFI image.
+  @param[in] SystemTable    A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS       The entry point is executed successfully.
+  @retval other             Some error occurs when executing this entry point.
+
+**/
+EFI_STATUS
+EFIAPI
+UefiMain (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS                Status;
+  EFI_MP_SERVICES_PROTOCOL  *Mp;
+  EFI_HANDLE                *pHandle;
+  UINTN                     HandleCount;
+  UINTN                     BspId;
+  UINTN                     NumCpus;
+  UINTN                     Index;
+
+  pHandle     = NULL;
+  HandleCount = 0;
+  BspId = 0;
+
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  &gEfiMpServiceProtocolGuid,
+                  NULL,
+                  &HandleCount,
+                  &pHandle
+                  );
+
+  if (EFI_ERROR (Status)) {
+    Print (L"Failed to locate EFI_MP_SERVICES_PROTOCOL (%r). Not installed on platform?\n", Status);
+    return EFI_NOT_FOUND;
+  }
+
+  for (Index = 0; Index < HandleCount; Index++) {
+    Status = gBS->OpenProtocol (
+                    *pHandle,
+                    &gEfiMpServiceProtocolGuid,
+                    (VOID **)&Mp,
+                    NULL,
+                    gImageHandle,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                    );
+
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    pHandle++;
+  }
+
+  Print (L"Exercising WhoAmI\n\n");
+  Status = Mp->WhoAmI (Mp, &BspId);
+  if (EFI_ERROR (Status)) {
+    Print (L"WhoAmI failed: %r\n", Status);
+    return Status;
+  } else {
+    Print (L"WhoAmI: %016lx\n", BspId);
+  }
+
+  Print (L"\n");
+  Print (
+    L"Exercising GetNumberOfProcessors and GetProcessorInformation with "
+    L"CPU_V2_EXTENDED_TOPOLOGY\n\n"
+    );
+  NumCpus = PrintProcessorInformation (Mp);
+  if (NumCpus < 2) {
+    Print (L"UP system found. Not running further tests.\n");
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Print (L"\n");
+  Print (L"Exercising StartupThisAP:\n\n");
+  StartupThisApTests (Mp);
+
+  Print (L"\n");
+  Print (L"Exercising StartupAllAPs:\n\n");
+  StartupAllAPsTests (Mp, NumCpus);
+
+  Print (L"\n");
+  Print (L"Exercising EnableDisableAP:\n\n");
+  EnableDisableAPTests (Mp, NumCpus);
+
+  Print (L"\n");
+  Print (L"Exercising SwitchBSP\n\n");
+  SwitchBSPTests (Mp, NumCpus);
+
+  gBS->CloseProtocol (pHandle, &gEfiMpServiceProtocolGuid, gImageHandle, NULL);
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
new file mode 100644
index 000000000000..8a21ca70d8fa
--- /dev/null
+++ b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
@@ -0,0 +1,38 @@
+## @file
+#  UEFI Application to exercise EFI_MP_SERVICES_PROTOCOL.
+#
+#  Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.29
+  BASE_NAME                      = MpServicesTest
+  FILE_GUID                      = 43e9defa-7209-4b0d-b136-cc4ca02cb469
+  MODULE_TYPE                    = UEFI_APPLICATION
+  VERSION_STRING                 = 0.1
+  ENTRY_POINT                    = UefiMain
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 AARCH64
+#
+
+[Sources]
+  MpServicesTest.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  RngLib
+  UefiApplicationEntryPoint
+  UefiLib
+
+[Protocols]
+  gEfiMpServiceProtocolGuid    ## CONSUMES
+
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index b1d83461865e..1cf5ccd30d40 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -164,6 +164,7 @@
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
+  RngLib|MdePkg/Library/DxeRngLib/DxeRngLib.inf
 
 [LibraryClasses.common.MM_STANDALONE]
   HobLib|MdeModulePkg/Library/BaseHobLibNull/BaseHobLibNull.inf
@@ -215,6 +216,7 @@
   MdeModulePkg/Application/HelloWorld/HelloWorld.inf
   MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
   MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.inf
+  MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
 
   MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
   MdeModulePkg/Logo/Logo.inf

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