Date   

Re: [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

Guo Dong
 

A typo copy, should be:
Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo Dong
Sent: Tuesday, August 3, 2021 7:22 PM
To: devel@edk2.groups.io; chengchieh@google.com
Subject: Re: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload


Signed-off-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

HPET timer may fail to init after prior linux taking over.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 6 ++++++ UefiPayloadPkg/UefiPayloadPkg.fdf | 5 +++++
2 files changed, 11 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 54576ba485b7..e56e6f4a5379 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -438,7 +438,13 @@ [Components.X64]
NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
}

+!if $(BOOTLOADER) == "LINUXBOOT"
+ OvmfPkg/8254TimerDxe/8254Timer.inf
+ OvmfPkg/8259InterruptControllerDxe/8259.inf
+!else
PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!endif
+
MdeModulePkg/Universal/Metronome/Metronome.inf
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index 041fed842cd8..f57a8b4bf3d3 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -101,7 +101,12 @@ [FV.DXEFV]
INF UefiCpuPkg/CpuDxe/CpuDxe.inf
INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
INF MdeModulePkg/Application/UiApp/UiApp.inf
+!if $(BOOTLOADER) != "LINUXBOOT"
INF PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!else
+INF OvmfPkg/8254TimerDxe/8254Timer.inf
+INF OvmfPkg/8259InterruptControllerDxe/8259.inf
+!endif
INF MdeModulePkg/Universal/Metronome/Metronome.inf
INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
--
2.32.0.402.g57bb445576-goog


Re: [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

Guo Dong
 

A typo copy, should be:
Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo Dong
Sent: Tuesday, August 3, 2021 7:22 PM
To: devel@edk2.groups.io; chengchieh@google.com
Subject: Re: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256


Signed-off-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index e56e6f4a5379..8aa5f18cd35c 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -40,7 +40,7 @@ [Defines]
#
# CPU options
#
- DEFINE MAX_LOGICAL_PROCESSORS = 64
+ DEFINE MAX_LOGICAL_PROCESSORS = 256

#
# PCI options
--
2.32.0.402.g57bb445576-goog


Re: [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Guo Dong
 

why build runtime memory allocation hob here?
The PayloadEntry should already got the information and build a new HOB list to DXE core, will anyone access these region late?
If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and update commit message on this.

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (

// The UEFI payload FV
BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 (PcdPayloadFdMemSize), EfiBootServicesData);
+ // The UEFI payload config FV
+ BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, SIZE_64KB, EfiRuntimeServicesData);

//
// Build CPU memory space and IO space hob
--
2.32.0.402.g57bb445576-goog


Re: [PATCH v1 1/6] UefiPayloadPkg: Add LINUXBOOT payload target

Guo Dong
 

Move Linuxboot.h under UefiPayloadPkg/Library/LbParseLib since this header file is only used by this library.
Update LbParseLib.c following https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/
e.g.
+ UefiPayloadConfig* config;
+ int i;
Change to:
+ UefiPayloadConfig *Config;
+ UINTN Index;

And split below code into multiple lines for better readable.
+// Align the address and add memory rang to MemInfoCallback void
+AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
+ IN UINTN end, IN int type) {

Update OpenSsl submodule in a separate patch if required.

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 1/6] UefiPayloadPkg: Add LINUXBOOT payload target

Initial commit to support linuxboot payload.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 16 +-
UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf | 39 +++++
UefiPayloadPkg/Include/Linuxboot.h | 47 ++++++
UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 168 ++++++++++++++++++++
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c | 6 +-
CryptoPkg/Library/OpensslLib/openssl | 2 +-
6 files changed, 270 insertions(+), 8 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index bcedf1c746b4..54576ba485b7 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -33,6 +33,7 @@ [Defines]
#
# SBL: UEFI payload for Slim Bootloader
# COREBOOT: UEFI payload for coreboot
+ # LINUXBOOT: UEFI payload for linuxboot
#
DEFINE BOOTLOADER = SBL

@@ -93,6 +94,9 @@ [Defines]

[BuildOptions]
*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
+!if $(BOOTLOADER) == "LINUXBOOT"
+ *_*_*_CC_FLAGS = -D LINUXBOOT_PAYLOAD
+!endif
GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG
GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG
@@ -222,11 +226,13 @@ [LibraryClasses]
!endif
PlatformSupportLib|UefiPayloadPkg/Library/PlatformSupportLibNull/PlatformSupportLibNull.inf
!if $(UNIVERSAL_PAYLOAD) == FALSE
- !if $(BOOTLOADER) == "COREBOOT"
- BlParseLib|UefiPayloadPkg/Library/CbParseLib/CbParseLib.inf
- !else
- BlParseLib|UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
- !endif
+ !if $(BOOTLOADER) == "COREBOOT"
+ BlParseLib|UefiPayloadPkg/Library/CbParseLib/CbParseLib.inf
+ !elseif $(BOOTLOADER) == "LINUXBOOT"
+ BlParseLib|UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
+ !else
+ BlParseLib|UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
+ !endif
!endif

DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
new file mode 100644
index 000000000000..d75ba8db8cf3
--- /dev/null
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
@@ -0,0 +1,39 @@
+## @file
+# Linuxboot Table Parse Library.
+#
+# Copyright (c) 2021, the u-root Authors. All rights reserved.<BR> #
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = LbParseLib
+ FILE_GUID = DBA15E1E-4C16-47DF-93C0-AB5888ED14C3
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = BlParseLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources]
+ LbParseLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ UefiPayloadPkg/UefiPayloadPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ IoLib
+ DebugLib
+ PcdLib
+
+[Pcd]
+ gUefiPayloadPkgTokenSpaceGuid.PcdPayloadFdMemBase
diff --git a/UefiPayloadPkg/Include/Linuxboot.h b/UefiPayloadPkg/Include/Linuxboot.h
new file mode 100644
index 000000000000..34ca18069983
--- /dev/null
+++ b/UefiPayloadPkg/Include/Linuxboot.h
@@ -0,0 +1,47 @@
+/** @file
+ LinuxBoot PEI module include file.
+**/
+#ifndef _LINUXBOOT_PEI_H_INCLUDED_
+#define _LINUXBOOT_PEI_H_INCLUDED_
+
+#if defined(_MSC_VER)
+#pragma warning(disable : 4200)
+#endif
+
+#pragma pack(1)
+typedef struct SerialPortConfigStruct {
+ UINT32 Type;
+ UINT32 BaseAddr;
+ UINT32 Baud;
+ UINT32 RegWidth;
+ UINT32 InputHertz;
+ UINT32 UartPciAddr;
+} SerialPortConfig;
+
+typedef struct MemoryMapEntryStruct {
+ UINT64 Start;
+ UINT64 End;
+ UINT32 Type;
+} MemoryMapEntry;
+
+typedef struct UefiPayloadConfigStruct {
+ UINT64 Version;
+ UINT64 AcpiBase;
+ UINT64 AcpiSize;
+ UINT64 SmbiosBase;
+ UINT64 SmbiosSize;
+ SerialPortConfig SerialConfig;
+ UINT32 NumMemoryMapEntries;
+ MemoryMapEntry MemoryMapEntries[0];
+} UefiPayloadConfig;
+#pragma pack()
+
+#define UEFI_PAYLOAD_CONFIG_VERSION 1
+
+#define LINUXBOOT_MEM_RAM 1
+#define LINUXBOOT_MEM_DEFAULT 2
+#define LINUXBOOT_MEM_ACPI 3
+#define LINUXBOOT_MEM_NVS 4
+#define LINUXBOOT_MEM_RESERVED 5
+
+#endif // _LINUXBOOT_PEI_H_INCLUDED_
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
new file mode 100644
index 000000000000..34bfb6a1073f
--- /dev/null
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
@@ -0,0 +1,168 @@
+/** @file
+ This library will parse the linuxboot table in memory and extract
+those required
+ information.
+
+ Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/SmBios.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BlParseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Linuxboot.h>
+#include <Uefi/UefiBaseType.h>
+
+// Retrieve UefiPayloadConfig from Linuxboot's uefiboot
+UefiPayloadConfig* GetUefiPayLoadConfig() {
+ UefiPayloadConfig* config =
+ (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) -
+SIZE_64KB);
+ if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
+ DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get %d\n",
+ UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
+ CpuDeadLoop ();
+ }
+ return config;
+}
+
+// Align the address and add memory rang to MemInfoCallback void
+AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
+ IN UINTN end, IN int type) {
+ MEMROY_MAP_ENTRY MemoryMap;
+ UINTN AlignedStart;
+ UINTN AlignedEnd;
+ AlignedStart = ALIGN_VALUE(start, SIZE_4KB);
+ AlignedEnd = ALIGN_VALUE(end, SIZE_4KB);
+ // Conservative adjustment on Memory map. This should happen when
+booting from
+ // non UEFI bios and it may report a memory region less than 4KB.
+ if (AlignedStart > start && type != LINUXBOOT_MEM_RAM) {
+ AlignedStart -= SIZE_4KB;
+ }
+ if (AlignedEnd > end + 1 && type == LINUXBOOT_MEM_RAM) {
+ AlignedEnd -= SIZE_4KB;
+ }
+ MemoryMap.Base = AlignedStart;
+ MemoryMap.Size = AlignedEnd - AlignedStart;
+ MemoryMap.Type = type;
+ MemoryMap.Flag = 0;
+ MemInfoCallback(&MemoryMap, NULL);
+}
+
+/**
+ Acquire the memory information from the linuxboot table in memory.
+
+ @param MemInfoCallback The callback routine
+ @param Params Pointer to the callback routine parameter
+
+ @retval RETURN_SUCCESS Successfully find out the memory information.
+ @retval RETURN_NOT_FOUND Failed to find the memory information.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID*
+Params) {
+ UefiPayloadConfig* config;
+ int i;
+
+ config = GetUefiPayLoadConfig();
+
+ DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n",
+ config->NumMemoryMapEntries));
+
+ MemoryMapEntry* entry = &config->MemoryMapEntries[0];
+ for (i = 0; i < config->NumMemoryMapEntries; i++) {
+ DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
+ entry->End, entry->Type));
+ AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
+ entry++;
+ }
+ return RETURN_SUCCESS;
+}
+
+/**
+ Acquire acpi table and smbios table from linuxboot
+
+ @param SystemTableInfo Pointer to the system table info
+
+ @retval RETURN_SUCCESS Successfully find out the tables.
+ @retval RETURN_NOT_FOUND Failed to find the tables.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
+ UefiPayloadConfig* config;
+
+ config = GetUefiPayLoadConfig();
+ SystemTableInfo->AcpiTableBase = config->AcpiBase;
+ SystemTableInfo->AcpiTableSize = config->AcpiSize;
+
+ SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
+ SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
+
+ return RETURN_SUCCESS;
+}
+
+/**
+ Find the serial port information
+
+ @param SERIAL_PORT_INFO Pointer to serial port info structure
+
+ @retval RETURN_SUCCESS Successfully find the serial port information.
+ @retval RETURN_NOT_FOUND Failed to find the serial port information .
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
+ UefiPayloadConfig* config;
+ config = GetUefiPayLoadConfig();
+
+ SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
+ SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
+ SerialPortInfo->Type = config->SerialConfig.Type;
+ SerialPortInfo->Baud = config->SerialConfig.Baud;
+ SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
+ SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
+
+ return RETURN_SUCCESS;
+}
+
+/**
+ Find the video frame buffer information
+
+ @param GfxInfo Pointer to the EFI_PEI_GRAPHICS_INFO_HOB structure
+
+ @retval RETURN_SUCCESS Successfully find the video frame buffer
+information.
+ @retval RETURN_NOT_FOUND Failed to find the video frame buffer information .
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseGfxInfo(OUT EFI_PEI_GRAPHICS_INFO_HOB* GfxInfo) {
+ // Not supported
+ return RETURN_NOT_FOUND;
+}
+
+/**
+ Find the video frame buffer device information
+
+ @param GfxDeviceInfo Pointer to the EFI_PEI_GRAPHICS_DEVICE_INFO_HOB
+structure
+
+ @retval RETURN_SUCCESS Successfully find the video frame buffer
+information.
+ @retval RETURN_NOT_FOUND Failed to find the video frame buffer information.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseGfxDeviceInfo(OUT EFI_PEI_GRAPHICS_DEVICE_INFO_HOB* GfxDeviceInfo)
+{
+ return RETURN_NOT_FOUND;
+}
diff --git a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
index b0268f05069c..a4f714f765ea 100644
--- a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
+++ b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
@@ -40,8 +40,9 @@ AdjustRootBridgeResource (
IN PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G
)
{
+#ifndef LINUXBOOT_PAYLOAD
UINT64 Mask;
-
+#endif
//
// For now try to downgrade everything into MEM32 since
// - coreboot does not assign resource above 4GB @@ -80,7 +81,7 @@ AdjustRootBridgeResource (
PMemAbove4G->Base = MAX_UINT64;
PMemAbove4G->Limit = 0;
}
-
+#ifndef LINUXBOOT_PAYLOAD
//
// Align IO resource at 4K boundary
//
@@ -98,6 +99,7 @@ AdjustRootBridgeResource (
if (Mem->Base != MAX_UINT64) {
Mem->Base &= ~Mask;
}
+#endif
}

/**
diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl
index 52c587d60be6..e2e09d9fba11 160000
--- a/CryptoPkg/Library/OpensslLib/openssl
+++ b/CryptoPkg/Library/OpensslLib/openssl
@@ -1 +1 @@
-Subproject commit 52c587d60be67c337364b830dd3fdc15404a2f04
+Subproject commit e2e09d9fba1187f8d6aafaa34d4172f56f1ffb72
--
2.32.0.402.g57bb445576-goog


Re: [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

Guo Dong
 

Signed-off-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

HPET timer may fail to init after prior linux taking over.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 6 ++++++ UefiPayloadPkg/UefiPayloadPkg.fdf | 5 +++++
2 files changed, 11 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 54576ba485b7..e56e6f4a5379 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -438,7 +438,13 @@ [Components.X64]
NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
}

+!if $(BOOTLOADER) == "LINUXBOOT"
+ OvmfPkg/8254TimerDxe/8254Timer.inf
+ OvmfPkg/8259InterruptControllerDxe/8259.inf
+!else
PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!endif
+
MdeModulePkg/Universal/Metronome/Metronome.inf
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index 041fed842cd8..f57a8b4bf3d3 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -101,7 +101,12 @@ [FV.DXEFV]
INF UefiCpuPkg/CpuDxe/CpuDxe.inf
INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
INF MdeModulePkg/Application/UiApp/UiApp.inf
+!if $(BOOTLOADER) != "LINUXBOOT"
INF PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!else
+INF OvmfPkg/8254TimerDxe/8254Timer.inf
+INF OvmfPkg/8259InterruptControllerDxe/8259.inf
+!endif
INF MdeModulePkg/Universal/Metronome/Metronome.inf
INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
--
2.32.0.402.g57bb445576-goog


Re: [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

Guo Dong
 

Signed-off-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index e56e6f4a5379..8aa5f18cd35c 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -40,7 +40,7 @@ [Defines]
#
# CPU options
#
- DEFINE MAX_LOGICAL_PROCESSORS = 64
+ DEFINE MAX_LOGICAL_PROCESSORS = 256

#
# PCI options
--
2.32.0.402.g57bb445576-goog


回复: [edk2-platforms PATCH v6 0/4] Secure Boot default keys

gaoliming
 

Sunny:
I am OK to merge the reviewed patched first.

Thanks
Liming
-----邮件原件-----
发件人: Sunny Wang <Sunny.Wang@arm.com>
发送时间: 2021年8月3日 22:11
收件人: Grzegorz Bernacki <gjb@semihalf.com>; devel@edk2.groups.io;
ardb+tianocore@kernel.org
抄送: leif@nuviainc.com; Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com>; mw@semihalf.com;
upstream@semihalf.com; jiewen.yao@intel.com; jian.j.wang@intel.com;
min.m.xu@intel.com; lersek@redhat.com; Sami Mujawar
<Sami.Mujawar@arm.com>; afish@apple.com; ray.ni@intel.com;
jordan.l.justen@intel.com; rebecca@bsdio.com; grehan@freebsd.org;
Thomas Abraham <thomas.abraham@arm.com>; chasel.chiu@intel.com;
nathaniel.l.desimone@intel.com; gaoliming@byosoft.com.cn;
eric.dong@intel.com; michael.d.kinney@intel.com; zailiang.sun@intel.com;
yi.qian@intel.com; graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie;
Sunny Wang <Sunny.Wang@arm.com>
主题: RE: [edk2-platforms PATCH v6 0/4] Secure Boot default keys

Hi Ard and Maintainers,

For this patchset,
1/4 - Intel Platforms: add SecureBootVariableLib class resolution
2/4 - ARM Silicon and Platforms: add SecureBootVariableLib class
resolution
3/4 - RISC-V Platforms: add SecureBootVariableLib class resolution
4/4 - Platform/RaspberryPi: Enable default Secure Boot variables
initialization

Only Intel platform patch (1/4) hasn't got all Review-bys. I offline sent
a
reminder to Intel platform Maintainers. I think they may be busy with
other
things or need more time to review it.

Therefore, how about we merge another three patches (2/4, 3/4, and 4/4)
first?

Best Regards,
Sunny Wang

-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Wednesday, July 14, 2021 8:31 PM
To: devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>;
mw@semihalf.com; upstream@semihalf.com; jiewen.yao@intel.com;
jian.j.wang@intel.com; min.m.xu@intel.com; lersek@redhat.com; Sami
Mujawar <Sami.Mujawar@arm.com>; afish@apple.com; ray.ni@intel.com;
jordan.l.justen@intel.com; rebecca@bsdio.com; grehan@freebsd.org;
Thomas Abraham <thomas.abraham@arm.com>; chasel.chiu@intel.com;
nathaniel.l.desimone@intel.com; gaoliming@byosoft.com.cn;
eric.dong@intel.com; michael.d.kinney@intel.com; zailiang.sun@intel.com;
yi.qian@intel.com; graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie;
Grzegorz Bernacki <gjb@semihalf.com>
Subject: [edk2-platforms PATCH v6 0/4] Secure Boot default keys

This patchset is a consequence of "Secure Boot default keys"
patchset in edk2. It adds SecureBootVariableLib class resolution
for each platform which uses SecureBootConfigDxe and also
enables Secure Boot variables initialization for RPi4.
Previously these commits were part of edk2 patchset, but since
number of commits increased in v5 version, it is now separate
patchset.

Changes related to both edk2 & edk-platform versions:
Changes since v1:
- change names:
SecBootVariableLib => SecureBootVariableLib
SecBootDefaultKeysDxe => SecureBootDefaultKeysDxe
SecEnrollDefaultKeysApp => EnrollFromDefaultKeysApp
- change name of function CheckSetupMode to GetSetupMode
- remove ShellPkg dependecy from EnrollFromDefaultKeysApp
- rebase to master

Changes since v2:
- fix coding style for functions headers in SecureBootVariableLib.h
- add header to SecureBootDefaultKeys.fdf.inc
- remove empty line spaces in SecureBootDefaultKeysDxe files
- revert FAIL macro in EnrollFromDefaultKeysApp
- remove functions duplicates and add SecureBootVariableLib
to platforms which used it

Changes since v3:
- move SecureBootDefaultKeys.fdf.inc to ArmPlatformPkg
- leave duplicate of CreateTimeBasedPayload in PlatformVarCleanupLib
- fix typo in guid description

Changes since v4:
- reorder patches to make it bisectable
- split commits related to more than one platform
- move edk2-platform commits to separate patchset

Changes since v5:
- split SecureBootVariableLib into SecureBootVariableLib and
SecureBootVariableProvisionLib

Grzegorz Bernacki (4):
Intel Platforms: add SecureBootVariableLib class resolution
ARM Silicon and Platforms: add SecureBootVariableLib class resolution
RISC-V Platforms: add SecureBootVariableLib class resolution
Platform/RaspberryPi: Enable default Secure Boot variables
initialization

Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
| 2 ++
Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
| 2 ++
Platform/Intel/QuarkPlatformPkg/Quark.dsc
| 2 ++
Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
| 2 ++
Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
| 2 ++
Platform/Qemu/SbsaQemu/SbsaQemu.dsc
| 2 ++
Platform/RaspberryPi/RPi3/RPi3.dsc
| 2 ++
Platform/RaspberryPi/RPi4/RPi4.dsc
| 4 ++++
Platform/SiFive/U5SeriesPkg/FreedomU500VC707Board/U500.dsc
| 2 ++

Platform/SiFive/U5SeriesPkg/FreedomU540HiFiveUnleashedBoard/U540.dsc
| 2 ++
Platform/Socionext/DeveloperBox/DeveloperBox.dsc
| 5 +++++
Platform/RaspberryPi/RPi4/RPi4.fdf
| 2 ++
12 files changed, 29 insertions(+)

--
2.25.1

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


Re: [edk2-platforms PATCH v1 1/1] Platform/Intel/SimicsOpenBoardPkg: Fix PCD type of PcdVideo*Resolution

Michael D Kinney
 

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Takuto Naito
Sent: Thursday, July 29, 2021 7:54 AM
To: devel@edk2.groups.io
Cc: Takuto Naito <naitaku@gmail.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: [edk2-devel] [edk2-platforms PATCH v1 1/1] Platform/Intel/SimicsOpenBoardPkg: Fix PCD type of PcdVideo*Resolution

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

PcdVideoHorizontalResolution and PcdVideoVerticalResolutio are set in
the SimicsDxe module and consumed by the other module GraphicsConsoleDxe.
In this case, the type of these PCDs should be Dynamic.

Cc: Agyeman Prince <prince.agyeman@intel.com>
Signed-off-by: Takuto Naito <naitaku@gmail.com>
---
.../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc
b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc
index 251f46f812..88009b8f10 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc
+++ b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc
@@ -221,8 +221,6 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdSmiHandlerProfilePropertyMask|0x1
!endif
gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE
- gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|1024
- gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
gPcAtChipsetPkgTokenSpaceGuid.PcdHpetBaseAddress|0xFED00000

######################################
@@ -238,6 +236,8 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|1024
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600

######################################
# Board Configuration
--
2.25.1





Event: TianoCore Bug Triage - APAC / NAMO - 08/03/2021 #cal-reminder

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

Reminder: TianoCore Bug Triage - APAC / NAMO

When:
08/03/2021
6:30pm to 7:30pm
(UTC-07:00) America/Los Angeles

Where:
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d

Organizer: Liming Gao gaoliming@...

View Event

Description:

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao

 

________________________________________________________________________________

Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions

Or call in (audio only)

+1 916-245-6934,,77463821#   United States, Sacramento

Phone Conference ID: 774 638 21#

Find a local number | Reset PIN

Learn More | Meeting options


Re: [PATCH V4 2/3] OvmfPkg/Sec: Update the check logic in SevEsIsEnabled

Min Xu
 

On August 4, 2021 3:24 AM, Brijesh Singh wrote:
Hi Min,

On 8/2/21 8:18 PM, Min Xu wrote:
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

SevEsIsEnabled return TRUE if SevEsWorkArea->SevEsEnabled is non-zero.
It is correct when SevEsWorkArea is only used by SEV. After Intel TDX
is enabled in Ovmf, the SevEsWorkArea is shared by TDX and SEV. (This
is to avoid the waist of memory region in MEMFD). The value of
SevEsWorkArea->SevEsEnabled now is :
0 if in Legacy guest
1 if in SEV
2 if in Tdx guest
That's why the changes is made.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/Sec/SecMain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index
9db67e17b2aa..e166a9389a1a 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -828,7 +828,7 @@ SevEsIsEnabled (

SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32
(PcdSevEsWorkAreaBase);

- return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled !=
0));
+ return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled ==
+ 1));
}
This is wrong, we need to check the SevEs sub type and not the global Sev
enable. This also need to be broken into at least two commits

1. introduce the updated CcWorkArea structure 2. update the existing code to
use the CcWorkArea layout

If you are okay then I can rework and send the patch so that you can add the
TDX on top of it.
That will be great If you can rework the SEV parts. Thanks much Brijesh!
Thanks!
Xu, Min


Re: [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Update page alignment calculations

Tuan Phan
 

Reviewed-By: Tuan Phan <tuanphan@...>

On Jul 19, 2021, at 1:07 AM, Sunny Wang via groups.io <Sunny.Wang@...> wrote:

This is to fix the SCT BS.AllocatePages failures (not found) with the
case that the Start address is not aligned to 64k.
For example,
 The following is available memory region for testing:
   0000000082012000-00000000EB6D9FFF 00000000000696C8
 With the current page alignment calculation, we will get:
   Start address is 0x82020000
   PageNum is 0x696B8
 In BS.AllocatePages, it will make the end address align with 64k,
 so PageNum will be changed from 0x696B8 to 0x696C0. Therefore, the
 end address will become 0xEB6E0000 which is larger than 0xEB6D9FFF,
 so we get not found error in the end.

Therefore, the calculation for getting the PageNum should be updated
to PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000)) so that we won't get a
wrong PageNum to allocate a memory with a size larger than available
space's size.

With this solution, the example above will get 0x696A8 as calculated
PageNum. Then, in BS.AllocatePages, the PageNum will be changed from
0x696A8 to 0x696B0. Therefore, the end address will become 0xEB6D0000
that is smaller than 0xEB6D9FFF, so we get not found error in the end.

I also tested this solution on two ARM platforms (NXP1046A and RPi4).

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@...>
Cc: G Edhaya Chandran <edhaya.chandran@...>
Cc: Barton Gao <gaojie@...>
Signed-off-by: Sunny Wang <sunny.wang@...>
---
.../MemoryAllocationServicesBBTestFunction.c  | 110 +++++++++++-------
1 file changed, 66 insertions(+), 44 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
index bf8cd3b3..cdfac992 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
@@ -2,6 +2,7 @@

  Copyright 2006 - 2013 Unified EFI, Inc.<BR>
  Copyright (c) 2010 - 2013, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2021, ARM Limited. All rights reserved.

  This program and the accompanying materials
  are licensed and made available under the terms and conditions of the BSD License
@@ -24,7 +25,7 @@ Abstract:

--*/

-#include "SctLib.h"
+#include "SctLib.h"
#include "MemoryAllocationServicesBBTestMain.h"

#define ALLOCATEPAGES_MEMORYTYPE_NUM 16
@@ -700,14 +701,17 @@ BBTestAllocatePagesInterfaceTest (
        PageNum = (UINTN)Descriptor.NumberOfPages;
        Start   = Descriptor.PhysicalStart;

-        //
-        // Some memory types need more alignment than 4K, so
-        //
-        if (PageNum <= 0x10) {
+        //
+        // Calculate New Start address and PageNum with 64k alignment to
+        // cover the case that some memory types' alignment is more than
+        // 4k. If the available memory is less than 192k, the memory
+        // allocation call will be skipped.
+        //
+        if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
          break;
        }
-        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
-        PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
+        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
+        PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));

        Memory  = Start;

@@ -830,14 +834,17 @@ BBTestAllocatePagesInterfaceTest (
        PageNum = (UINTN)Descriptor.NumberOfPages;
        Start   = Descriptor.PhysicalStart;

-        //
-        // Some memory types need more alignment than 4K, so
-        //
-        if (PageNum <= 0x10) {
+        //
+        // Calculate New Start address and PageNum with 64k alignment to
+        // cover the case that some memory types' alignment is more than
+        // 4k. If the available memory is less than 192k, the memory
+        // allocation call will be skipped.
+        //
+        if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
          break;
        }
-        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
-        PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
+        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
+        PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));

        Memory  = Start;

@@ -953,14 +960,17 @@ BBTestAllocatePagesInterfaceTest (
        PageNum = (UINTN)Descriptor.NumberOfPages;
        Start   = Descriptor.PhysicalStart;

-        //
-        // Some memory types need more alignment than 4K, so
-        //
-        if (PageNum <= 0x10) {
+        //
+        // Calculate New Start address and PageNum with 64k alignment to
+        // cover the case that some memory types' alignment is more than
+        // 4k. If the available memory is less than 192k, the memory
+        // allocation call will be skipped.
+        //
+        if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
          break;
        }
-        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
-        PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
+        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
+        PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));

        Memory = Start + (SctLShiftU64 (PageNum/3, EFI_PAGE_SHIFT) & 0xFFFFFFFFFFFF0000);

@@ -1076,14 +1086,17 @@ BBTestAllocatePagesInterfaceTest (
        PageNum = (UINTN)Descriptor.NumberOfPages;
        Start   = Descriptor.PhysicalStart;

-        //
-        // Some memory types need more alignment than 4K, so
-        //
-        if (PageNum <= 0x10) {
+        //
+        // Calculate New Start address and PageNum with 64k alignment to
+        // cover the case that some memory types' alignment is more than
+        // 4k. If the available memory is less than 192k, the memory
+        // allocation call will be skipped.
+        //
+        if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
          break;
        }
-        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
-        PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
+        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
+        PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));

        Memory  = Start + (SctLShiftU64 (PageNum * 2 / 3, EFI_PAGE_SHIFT) & 0xFFFFFFFFFFFF0000);

@@ -1206,14 +1219,17 @@ BBTestAllocatePagesInterfaceTest (
        PageNum = (UINTN)Descriptor.NumberOfPages;
        Start   = Descriptor.PhysicalStart;

-        //
-        // Some memory types need more alignment than 4K, so
-        //
-        if (PageNum <= 0x10) {
+        //
+        // Calculate New Start address and PageNum with 64k alignment to
+        // cover the case that some memory types' alignment is more than
+        // 4k. If the available memory is less than 192k, the memory
+        // allocation call will be skipped.
+        //
+        if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
          break;
        }
-        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
-        PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
+        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
+        PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));

        Memory  = Start;

@@ -1329,14 +1345,17 @@ BBTestAllocatePagesInterfaceTest (
        PageNum = (UINTN)Descriptor.NumberOfPages;
        Start   = Descriptor.PhysicalStart;

-        //
-        // Some memory types need more alignment than 4K, so
-        //
-        if (PageNum <= 0x10) {
+        //
+        // Calculate New Start address and PageNum with 64k alignment to
+        // cover the case that some memory types' alignment is more than
+        // 4k. If the available memory is less than 192k, the memory
+        // allocation call will be skipped.
+        //
+        if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
          break;
        }
-        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
-        PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
+        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
+        PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));

        Memory  = Start;

@@ -1468,14 +1487,17 @@ BBTestAllocatePagesInterfaceTest (
        PageNum = (UINTN)Descriptor.NumberOfPages;
        Start   = Descriptor.PhysicalStart;

-        //
-        // Some memory types need more alignment than 4K, so
-        //
-        if (PageNum <= 0x10) {
+        //
+        // Calculate New Start address and PageNum with 64k alignment to
+        // cover the case that some memory types' alignment is more than
+        // 4k. If the available memory is less than 192k, the memory
+        // allocation call will be skipped.
+        //
+        if (PageNum < (3 * EFI_SIZE_TO_PAGES(0x10000))) {
          break;
        }
-        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
-        PageNum = PageNum - EFI_SIZE_TO_PAGES(0x10000);
+        Start   = (Start + 0xFFFF) & 0xFFFFFFFFFFFF0000;
+        PageNum = PageNum - (2 * EFI_SIZE_TO_PAGES(0x10000));

        Memory  = Start;

@@ -1923,4 +1945,4 @@ BBTestFreePoolInterfaceTest (

  FreeMemoryMap ();
  return EFI_SUCCESS;
-}
+}
--
2.31.0.windows.1








Re: [PATCH V4 3/3] OvmfPkg/ResetVector: Enable Intel TDX in ResetVector of Ovmf

Brijesh Singh
 

On 8/2/21 8:18 PM, Min Xu wrote:
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
Intel's Trust Domain Extensions (Intel TDX) refers to an Intel technology
that extends Virtual Machines Extensions (VMX) and Multi-Key Total Memory
Encryption (MKTME) with a new kind of virutal machines guest called a
Trust Domain (TD). A TD is desinged to run in a CPU mode that protects the
confidentiality of TD memory contents and the TD's CPU state from other
software, including the hosting Virtual-Machine Monitor (VMM), unless
explicitly shared by the TD itself.
Note: Intel TDX only is available on X64, so the Tdx related changes are
in X64 path. In IA32 path, there may be null stub to make the build
success.
This patch includes below major changes.
1. CC_WORK_AREA
Ovmf is able to run on different types of VM guest with ONE image. These
VM guest includes the Legacy guest, SEV guest and TDX guest. In
ResetVector CC_WORK_AREA is such a memory region to record the VM guest
information. Below is the definition of the work area.
typedef struct {
UINT8 Type; // 0 legacy, 1 SEV, 2 TDX
UINT8 SubType; // Depends on Type
UINT8 Rsvd[2]; // Reserved
union VM_GUEST {
TDX_WORK_AREA Tdx;
SEV_WORK_AREA Sev;
} Guest;
} CC_WORK_AREA_HEAD;
typedef struct {
... ...
} TDX_WORK_AREA
typedef struct {
... ...
} SEV_WORK_AREA
This patch need to be broken into multiple sub patches for the easy to review and keeping the code bisectable. The first thing we need to do is introduce the new CCWorkArea concept and update the existing Sev support to use the new CCWorkArea.

2. X64/IntelTdxMetadata.asm
IntelTdxMetadata describes the information about the image for VMM use.
For example, the base address and length of the TdHob, TdMailbox, etc.
Its offset is put in a GUID-ed structure which is appended in the GUID-ed
chain from a fixed GPA (0xffffffd0). Below are the items in TdxMetadata:
_Bfv:
Boot Firmware Volume
_Cfv:
Configuration Firmware Volume
_Stack:
Initial stack
_Heap:
Initial heap
_MailBox:
TDVF reserves the memory region so each AP can receive the message
sent by the guest OS.
_CcWorkarea:
Compute Confidential work area which is consumed by CC technologies,
such as SEV, TDX.
_TdHob:
VMM pass the resource information in TdHob to TDVF.
_TdxPageTable:
If 5-level page table is supported (GPAW is 52), a top level page
directory pointers (1 * 256TB entry) is generated in this page.
_OvmfPageTable:
Initial page table for standard Ovmf.
TDVF indicate above chunk of temporary initialized memory region (_Stack/
_Heap/_MailBox/_CcWorkarea/_TdHob/_TdxPageTables/OvmfPageTable) to support
TDVF code finishing the memory initialization. Because the other
unaccepted memory cannot be accessed until they're accepted.
Since AMD SEV has already defined some SEV specific memory region in
MEMFD. TDX re-use the memory regions defined by SEV.
- MailBox : PcdOvmfSecGhcbBackupBase|PcdOvmfSecGhcbBackupSize
- TdHob : PcdOvmfSecGhcbBase|PcdOvmfSecGhcbSize
- TdxPageTable : PcdOvmfSecGhcbPageTableBase|PcdOvmfSecGhcbPageTableSize
- CcWorkArea : PcdSevEsWorkAreaBase|PcdSevEsWorkAreaSize
3. Ia32/IntelTdx.asm
IntelTdx.asm includes below routines used in ResetVector
- IsTdx
Check if the running system is Tdx guest.
- InitTdxWorkarea
It initialize the CC_WORK_AREA. Because it is called by both BSP and
APs and to avoid the race condition, only BSP can initialize the
WORK_AREA. AP will wait until the field of TDX_WORK_AREA_PGTBL_READY
is set.
- ReloadFlat32
After reset all CPUs in TDX are initialized to 32-bit protected mode.
But GDT register is not set. So this routine loads the GDT and set the
CR0, then jump to Flat 32 protected mode. After that CR4 and other
registers are set.
- InitTdx
This routine wrap above 3 routines together to do Tdx initialization
in ResetVector phase.
- PostSetCr3PageTables64Tdx
It is called after SetCr3PageTables64 in Tdx guest to set CR0/CR4.
If GPAW is 52, then CR3 is adjusted as well.
- IsTdxEnabled
It is a OneTimeCall to probe if TDX is enabled by checking the
CC_WORK_AREA.
4. Ia32/AmdSev.asm
AmdSev.asm includes the SEV routines used in ResetVector. This patch
remove the code of clearing SEV_ES_WORK_AREA in CheckSevFeatures. The
clearing of SEV_ES_WORK_AREA is called at Main16 in Main.asm.
Yep, this part need to be done in a separate patch. We can do this refactoring and addition of the CCWorkArea outside of the TDX and SNP series.

5. Main.asm
Previously OvmfPkg/ResetVector use the Main.asm in UefiCpuPkg. There is
only Main16 entry point. Main32 entry point is needed in Main.asm because
of Intel TDX. To reduce the complexity of Main.asm in UefiCpuPkg, OvmfPkg
create its own Main.asm to meet the requirement of Intel TDX. There are
below changes in this Main.asm:
- A new entry point (Main32) is added. TDX guest will jump to Main32
from ResetVecotr.
- In Main16 entry point, after TransitionFromReal16To32BitFlat,
CC_WORK_AREA is cleared to 0.
6. Ia32/PageTables64.asm
GPAW of TDX can be 48 or 52, which determines the level of page table.
If Level-5(GPAW 52) paging is supported, then an extra page is needed
to hold the top level Page Directory Pointers (1 * 256TB entry).
7. Ia16/ResetVectorVtf0.asm
In Tdx all CPUs "reset" to run on 32-bit protected mode with flat
descriptor (paging disabled). But in Non-Td guest the initial state of
CPUs is 16-bit real mode. To resolve this conflict, BITS 16/32 is used
in the ResetVectorVtf0.asm. It checks the 32-bit protected mode or 16-bit
real mode, then jump to the corresponding entry point.
8. ResetVector.nasmb
TDX related macros and files are added in ResetVecotr.nasmb.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 39 +++
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 7 -
OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm | 9 +
OvmfPkg/ResetVector/Ia32/IntelTdx.asm | 265 +++++++++++++++++++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 113 +++++---
OvmfPkg/ResetVector/Main.asm | 121 +++++++++
OvmfPkg/ResetVector/ResetVector.inf | 12 +-
OvmfPkg/ResetVector/ResetVector.nasmb | 48 +++-
OvmfPkg/ResetVector/X64/IntelTdxMetadata.asm | 110 ++++++++
9 files changed, 679 insertions(+), 45 deletions(-)
create mode 100644 OvmfPkg/ResetVector/Ia32/IntelTdx.asm
create mode 100644 OvmfPkg/ResetVector/Main.asm
create mode 100644 OvmfPkg/ResetVector/X64/IntelTdxMetadata.asm
diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
index 7ec3c6e980c3..62feeacbee7b 100644
--- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
+++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
@@ -47,6 +47,24 @@ TIMES (15 - ((guidedStructureEnd - guidedStructureStart + 15) % 16)) DB 0
;
guidedStructureStart:
+%ifdef ARCH_X64
+;
+; TDX Metadata offset block
+;
+; If TdxMetadata.asm is included then we need below block which describes
+; the offset of TdxMetadata block in Ovmf image
+;
+; GUID : e47a6535-984a-4798-865e-4685a7bf8ec2
+;
+tdxMetadataOffsetStart:
+ DD (OVMF_IMAGE_SIZE_IN_KB * 1024 - (fourGigabytes - TdxMetadataGuid - 16))
+ DD tdxMetadataOffsetEnd - tdxMetadataOffsetStart
+ DB 0x35, 0x65, 0x7a, 0xe4, 0x4a, 0x98, 0x98, 0x47
+ DB 0x86, 0x5e, 0x46, 0x85, 0xa7, 0xbf, 0x8e, 0xc2
+tdxMetadataOffsetEnd:
+
+%endif
+
; SEV Hash Table Block
;
; This describes the guest ram area where the hypervisor should
@@ -158,10 +176,31 @@ resetVector:
;
; This is where the processor will begin execution
;
+; In IA32 we follow the standard reset vector flow. While in X64, Td guest
+; may be supported. Td guest requires the startup mode to be 32-bit
+; protected mode but the legacy VM startup mode is 16-bit real mode.
+; To make NASM generate such shared entry code that behaves correctly in
+; both 16-bit and 32-bit mode, more BITS directives are added.
+;
+%ifdef ARCH_IA32
+
nop
nop
jmp EarlyBspInitReal16
+%else
+
+ smsw ax
+ test al, 1
+ jz .Real
+BITS 32
+ jmp Main32
+BITS 16
+.Real:
+ jmp EarlyBspInitReal16
+
+%endif
+
ALIGN 16
fourGigabytes:
diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index aa95d06eaddb..d0d9890d5c4b 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -128,13 +128,6 @@ SevEsUnexpectedRespTerminate:
; If SEV is disabled then EAX will be zero.
;
CheckSevFeatures:
- ; Set the first byte of the workarea to zero to communicate to the SEC
- ; phase that SEV-ES is not enabled. If SEV-ES is enabled, the CPUID
- ; instruction will trigger a #VC exception where the first byte of the
- ; workarea will be set to one or, if CPUID is not being intercepted,
- ; the MSR check below will set the first byte of the workarea to one.
- mov byte[SEV_ES_WORK_AREA], 0
-
;
; Set up exception handlers to check for SEV-ES
; Load temporary RAM stack based on PCDs (see SevEsIdtVmmComm for
diff --git a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
index c6d0d898bcd1..ea8723efd417 100644
--- a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
+++ b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
@@ -17,6 +17,14 @@ Transition32FlatTo64Flat:
OneTimeCall SetCr3ForPageTables64
+ OneTimeCall PostSetCr3PageTables64Tdx
+
+ ;
+ ; If it is TDX, we're done and jump to enable paging
+ ;
+ cmp byte[CC_WORK_AREA], VM_GUEST_TDX
+ je EnablePaging
+
mov eax, cr4
bts eax, 5 ; enable PAE
mov cr4, eax
@@ -71,6 +79,7 @@ jumpTo64BitAndLandHere:
;
; Check if the second step of the SEV-ES mitigation is to be performed.
+ ; If it is Tdx, ebx is cleared in PostSetCr3PageTables64Tdx.
;
test ebx, ebx
jz InsnCompare
diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
new file mode 100644
index 000000000000..f40331a5cbce
--- /dev/null
+++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
@@ -0,0 +1,265 @@
+;------------------------------------------------------------------------------
+; @file
+; Intel TDX routines
+;
+; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;------------------------------------------------------------------------------
+
+%define SEC_DEFAULT_CR0 0x00000023
+%define SEC_DEFAULT_CR4 0x640
+
+%define VM_GUEST_TDX 2
+
+BITS 32
+
+;
+; Check if it is Intel Tdx
+;
+; Modified: EAX, EBX, ECX, EDX
+;
+; If it is Intel Tdx, EAX is zero
+; If it is not Intel Tdx, EAX is non-zero
+;
+IsTdx:
+ ;
+ ; CPUID (0)
+ ;
+ mov eax, 0
+ cpuid
+ cmp ebx, 0x756e6547 ; "Genu"
+ jne IsNotTdx
+ cmp edx, 0x49656e69 ; "ineI"
+ jne IsNotTdx
+ cmp ecx, 0x6c65746e ; "ntel"
+ jne IsNotTdx
+
+ ;
+ ; CPUID (1)
+ ;
+ mov eax, 1
+ cpuid
+ test ecx, 0x80000000
+ jz IsNotTdx
+
+ ;
+ ; CPUID[0].EAX >= 0x21?
+ ;
+ mov eax, 0
+ cpuid
+ cmp eax, 0x21
+ jl IsNotTdx
+
+ ;
+ ; CPUID (0x21,0)
+ ;
+ mov eax, 0x21
+ mov ecx, 0
+ cpuid
+
+ cmp ebx, 0x65746E49 ; "Inte"
+ jne IsNotTdx
+ cmp edx, 0x5844546C ; "lTDX"
+ jne IsNotTdx
+ cmp ecx, 0x20202020 ; " "
+ jne IsNotTdx
+
+ mov eax, 0
+ jmp ExitIsTdx
+
+IsNotTdx:
+ mov eax, 1
+
+ExitIsTdx:
+
+ OneTimeCallRet IsTdx
+
+;
+; Initialize Tdx work area (CC_WORK_AREA) if it is Tdx guest.
+; BSP and APs all go here. Only BSP can initialize the WORK_AREA
+;
+; typedef struct {
+; UINT8 Type; // 0 legacy, 1 SEV, 2 TDX
+; UINT8 SubType; // Depends on Type
+; UINT8 Rsvd[2]; // Reserved
+; union VM_GUEST {
+; TDX_WORK_AREA Tdx;
+; SEV_WORK_AREA Sev;
+; } Guest;
+; } CC_WORK_AREA_HEAD;
+;
+; typedef struct {
+; UINT8 IsPageLevel5;
+; UINT8 IsPageTableReady;
+; UINT8 Rsvd[2];
+; UINT32 Gpaw;
+; } TDX_WORK_AREA
+;
+; Param[in] EBP[6:0] CPU Supported GPAW (48 or 52)
+; Param[in] ESI[31:0] vCPU ID (BSP is 0, others are AP)
+;
+; Modified: EBP
+;
+InitTdxWorkarea:
+
+ ;
+ ; First check if it is Tdx
+ ;
+ OneTimeCall IsTdx
+
+ test eax, eax
+ jnz ExitInitTdxWorkarea
+
+ ;
+ ; In Td guest, BSP/AP shares the same entry point
+ ; BSP builds up the page table, while APs shouldn't do the same task.
+ ; Instead, APs just leverage the page table which is built by BSP.
+ ; APs will wait until the page table is ready.
+ ;
+ cmp esi, 0
+ je TdxBspEntry
+
+TdxApWait:
+ cmp byte[TDX_WORK_AREA_PGTBL_READY], 0
+ je TdxApWait
+ jmp ExitInitTdxWorkarea
+
+TdxBspEntry:
+ ;
+ ; Set Type and SubType of CC_WORK_AREA so that the
+ ; following code can use these information.
+ ;
+ mov byte[CC_WORK_AREA], VM_GUEST_TDX
+ mov byte[CC_WORK_AREA_SUBTYPE], 0
+
+ ;
+ ; EBP[6:0] CPU supported GPA width
+ ;
+ and ebp, 0x3f
+ cmp ebp, 52
+ jl NotPageLevel5
+ mov byte[TDX_WORK_AREA_PAGELEVEL5], 1
+
+NotPageLevel5:
+ mov DWORD[TDX_WORK_AREA_GPAW], ebp
+
+ExitInitTdxWorkarea:
+ OneTimeCallRet InitTdxWorkarea
+
+;
+; Load the GDT and set the CR0, then jump to Flat 32 protected mode.
+;
+; Modified: EAX, EBX, CR0, CR4, DS, ES, FS, GS, SS
+;
+ReloadFlat32:
+
+ cli
+ mov ebx, ADDR_OF(gdtr)
+ lgdt [ebx]
+
+ mov eax, SEC_DEFAULT_CR0
+ mov cr0, eax
+
+ jmp LINEAR_CODE_SEL:dword ADDR_OF(jumpToFlat32BitAndLandHere)
+
+jumpToFlat32BitAndLandHere:
+
+ mov eax, SEC_DEFAULT_CR4
+ mov cr4, eax
+
+ debugShowPostCode POSTCODE_32BIT_MODE
+
+ mov ax, LINEAR_SEL
+ mov ds, ax
+ mov es, ax
+ mov fs, ax
+ mov gs, ax
+ mov ss, ax
+
+ OneTimeCallRet ReloadFlat32
+
+;
+; Tdx initialization after entering into ResetVector
+;
+; Modified: EAX, EBX, ECX, EDX, EBP, EDI, ESP
+;
+InitTdx:
+ ;
+ ; Save EBX in EBP because EBX will be changed in ReloadFlat32
+ ;
+ mov ebp, ebx
+
+ ;
+ ; First load the GDT and jump to Flat32 mode
+ ;
+ OneTimeCall ReloadFlat32
+
+ ;
+ ; Initialization of Tdx work area
+ ;
+ OneTimeCall InitTdxWorkarea
+
+ OneTimeCallRet InitTdx
+
+;
+; Called after SetCr3PageTables64 in Tdx guest to set CR0/CR4.
+; If GPAW is 52, then CR3 is adjusted as well.
+;
+; Modified: EAX, EBX, CR0, CR3, CR4
+;
+PostSetCr3PageTables64Tdx:
+ ;
+ ; TDX_WORK_AREA was set in InitTdx if it is Tdx guest
+ ;
+ cmp byte[CC_WORK_AREA], VM_GUEST_TDX
+ jne ExitPostSetCr3PageTables64Tdx
+
+ mov eax, cr4
+ bts eax, 5 ; enable PAE
+
+ ;
+ ; byte[TDX_WORK_AREA_PAGELEVEL5] holds the indicator whether 52bit is
+ ; supported. if it is the case, need to set LA57 and use 5-level paging
+ ;
+ cmp byte[TDX_WORK_AREA_PAGELEVEL5], 0
+ jz TdxSetCr4
+ bts eax, 12
+
+TdxSetCr4:
+ mov cr4, eax
+ mov ebx, cr3
+
+ ;
+ ; if la57 is not set, we are ok
+ ; if using 5-level paging, adjust top-level page directory
+ ;
+ bt eax, 12
+ jnc TdxSetCr3
+ mov ebx, TDX_PT_ADDR (0)
+
+TdxSetCr3:
+ mov cr3, ebx
+
+ xor ebx, ebx
+
+ExitPostSetCr3PageTables64Tdx:
+ OneTimeCallRet PostSetCr3PageTables64Tdx
+
+;
+; Check if TDX is enabled
+;
+; Modified: EAX
+;
+; If TDX is enabled then EAX will be 1
+; If TDX is disabled then EAX will be 0.
+;
+IsTdxEnabled:
+ xor eax, eax
+ cmp byte[CC_WORK_AREA], VM_GUEST_TDX
+ jne NotTdx
+ mov eax, 1
+
+NotTdx:
+ OneTimeCallRet IsTdxEnabled
+
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index eacdb69ddb9f..1f4c0c50ac2e 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -37,11 +37,81 @@ BITS 32
PAGE_READ_WRITE + \
PAGE_PRESENT)
+TdxBuildExtraPageTables:
+ ;
+ ; Extra page tables built by Tdx guests
+ ;
+ xor eax, eax
+ mov ecx, 0x400
+tdClearTdxPageTablesMemoryLoop:
+ mov dword [ecx * 4 + TDX_PT_ADDR(0) - 4], eax
+ loop tdClearTdxPageTablesMemoryLoop
+
+ ;
+ ; Top level Page Directory Pointers (1 * 256TB entry)
+ ;
+ mov dword[TDX_PT_ADDR (0)], PT_ADDR(0) + PAGE_PDP_ATTR
+
+ ;
+ ; Set TDX_WORK_AREA_PGTBL_READY to notify APs to go
+ ;
+ mov byte[TDX_WORK_AREA_PGTBL_READY], 1
+
+ OneTimeCallRet TdxBuildExtraPageTables
+
+SevBuildGhcbPageTables:
+ ;
+ ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
+ ; This requires the 2MB page for this range be broken down into 512 4KB
+ ; pages. All will be marked encrypted, except for the GHCB.
+ ;
+ mov ecx, (GHCB_BASE >> 21)
+ mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
+ mov [ecx * 8 + PT_ADDR (0x2000)], eax
+
+ ;
+ ; Page Table Entries (512 * 4KB entries => 2MB)
+ ;
+ mov ecx, 512
+pageTableEntries4kLoop:
+ mov eax, ecx
+ dec eax
+ shl eax, 12
+ add eax, GHCB_BASE & 0xFFE0_0000
+ add eax, PAGE_4K_PDE_ATTR
+ mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
+ mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
+ loop pageTableEntries4kLoop
+
+ ;
+ ; Clear the encryption bit from the GHCB entry
+ ;
+ mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
+ mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
+
+ mov ecx, GHCB_SIZE / 4
+ xor eax, eax
+clearGhcbMemoryLoop:
+ mov dword[ecx * 4 + GHCB_BASE - 4], eax
+ loop clearGhcbMemoryLoop
+
+ OneTimeCallRet SevBuildGhcbPageTables
+
;
; Modified: EAX, EBX, ECX, EDX
;
SetCr3ForPageTables64:
+ cmp byte[CC_WORK_AREA], VM_GUEST_TDX
+ jne CheckSev
+
+ cmp byte[TDX_WORK_AREA_PGTBL_READY], 1
+ je SetCr3
+
+ xor edx, edx
+ jmp SevNotActive
+
+CheckSev:
OneTimeCall CheckSevFeatures
xor edx, edx
test eax, eax
@@ -101,44 +171,19 @@ pageTableEntriesLoop:
mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
loop pageTableEntriesLoop
+ OneTimeCall IsTdxEnabled
+ test eax, eax
+ jz IsSevEs
+
+ OneTimeCall TdxBuildExtraPageTables
+ jmp SetCr3
+
+IsSevEs:
OneTimeCall IsSevEsEnabled
test eax, eax
jz SetCr3
- ;
- ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
- ; This requires the 2MB page for this range be broken down into 512 4KB
- ; pages. All will be marked encrypted, except for the GHCB.
- ;
- mov ecx, (GHCB_BASE >> 21)
- mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
- mov [ecx * 8 + PT_ADDR (0x2000)], eax
-
- ;
- ; Page Table Entries (512 * 4KB entries => 2MB)
- ;
- mov ecx, 512
-pageTableEntries4kLoop:
- mov eax, ecx
- dec eax
- shl eax, 12
- add eax, GHCB_BASE & 0xFFE0_0000
- add eax, PAGE_4K_PDE_ATTR
- mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
- mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
- loop pageTableEntries4kLoop
-
- ;
- ; Clear the encryption bit from the GHCB entry
- ;
- mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
- mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
-
- mov ecx, GHCB_SIZE / 4
- xor eax, eax
-clearGhcbMemoryLoop:
- mov dword[ecx * 4 + GHCB_BASE - 4], eax
- loop clearGhcbMemoryLoop
+ OneTimeCall SevBuildGhcbPageTables
SetCr3:
;
diff --git a/OvmfPkg/ResetVector/Main.asm b/OvmfPkg/ResetVector/Main.asm
new file mode 100644
index 000000000000..fb1731728af5
--- /dev/null
+++ b/OvmfPkg/ResetVector/Main.asm
@@ -0,0 +1,121 @@
+;------------------------------------------------------------------------------
+; @file
+; Main routine of the pre-SEC code up through the jump into SEC
+;
+; Copyright (c) 2008 - 2009, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;------------------------------------------------------------------------------
+
+
+BITS 16
+
+;
+; Modified: EBX, ECX, EDX, EBP
+;
+; @param[in,out] RAX/EAX Initial value of the EAX register
+; (BIST: Built-in Self Test)
+; @param[in,out] DI 'BP': boot-strap processor, or
+; 'AP': application processor
+; @param[out] RBP/EBP Address of Boot Firmware Volume (BFV)
+; @param[out] DS Selector allowing flat access to all addresses
+; @param[out] ES Selector allowing flat access to all addresses
+; @param[out] FS Selector allowing flat access to all addresses
+; @param[out] GS Selector allowing flat access to all addresses
+; @param[out] SS Selector allowing flat access to all addresses
+;
+; @return None This routine jumps to SEC and does not return
+;
+Main16:
+ OneTimeCall EarlyInit16
+
+ ;
+ ; Transition the processor from 16-bit real mode to 32-bit flat mode
+ ;
+ OneTimeCall TransitionFromReal16To32BitFlat
+
+BITS 32
+%ifdef ARCH_X64
+ ;
+ ; Clear the first 2 bytes of CC_WORK_AREA.
+ ;
+ mov word[CC_WORK_AREA], 0
+
+ jmp SearchBfv
+
+;
+; Entry point of Main32
+;
+Main32:
+ OneTimeCall InitTdx
+
+SearchBfv:
+
+%endif
+ ;
+ ; Search for the Boot Firmware Volume (BFV)
+ ;
+ OneTimeCall Flat32SearchForBfvBase
+
+ ;
+ ; EBP - Start of BFV
+ ;
+
+ ;
+ ; Search for the SEC entry point
+ ;
+ OneTimeCall Flat32SearchForSecEntryPoint
+
+ ;
+ ; ESI - SEC Core entry point
+ ; EBP - Start of BFV
+ ;
+
+%ifdef ARCH_IA32
+
+ ;
+ ; Restore initial EAX value into the EAX register
+ ;
+ mov eax, esp
+
+ ;
+ ; Jump to the 32-bit SEC entry point
+ ;
+ jmp esi
+
+%else
+
+ ;
+ ; Transition the processor from 32-bit flat mode to 64-bit flat mode
+ ;
+ OneTimeCall Transition32FlatTo64Flat
+
+BITS 64
+
+ ;
+ ; Some values were calculated in 32-bit mode. Make sure the upper
+ ; 32-bits of 64-bit registers are zero for these values.
+ ;
+ mov rax, 0x00000000ffffffff
+ and rsi, rax
+ and rbp, rax
+ and rsp, rax
+
+ ;
+ ; RSI - SEC Core entry point
+ ; RBP - Start of BFV
+ ;
+
+ ;
+ ; Restore initial EAX value into the RAX register
+ ;
+ mov rax, rsp
+
+ ;
+ ; Jump to the 64-bit SEC entry point
+ ;
+ jmp rsi
+
+%endif
+
+
diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index d028c92d8cfa..0cf3a8036b97 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -1,7 +1,7 @@
## @file
# Reset Vector
#
-# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -35,6 +35,7 @@
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
+ gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase
@@ -43,6 +44,15 @@
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfImageSizeInKb
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataOffset
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdBfvBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataOffset
+ gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataSize
[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index acec46a32450..528a6603a20e 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -67,6 +67,44 @@
%error "This implementation inherently depends on PcdOvmfSecGhcbBase not straddling a 2MB boundary"
%endif
+ %define TDX_BFV_RAW_DATA_OFFSET FixedPcdGet32 (PcdBfvRawDataOffset)
+ %define TDX_BFV_RAW_DATA_SIZE FixedPcdGet32 (PcdBfvRawDataSize)
+ %define TDX_BFV_MEMORY_BASE FixedPcdGet32 (PcdBfvBase)
+ %define TDX_BFV_MEMORY_SIZE FixedPcdGet32 (PcdBfvRawDataSize)
+
+ %define TDX_CFV_RAW_DATA_OFFSET FixedPcdGet32 (PcdCfvRawDataOffset)
+ %define TDX_CFV_RAW_DATA_SIZE FixedPcdGet32 (PcdCfvRawDataSize)
+ %define TDX_CFV_MEMORY_BASE FixedPcdGet32 (PcdCfvBase),
+ %define TDX_CFV_MEMORY_SIZE FixedPcdGet32 (PcdCfvRawDataSize),
+
+ %define TDX_HEAP_MEMORY_BASE FixedPcdGet32 (PcdOvmfSecPeiTempRamBase)
+ %define TDX_HEAP_MEMORY_SIZE FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) / 2
+
+ %define TDX_STACK_MEMORY_BASE (TDX_HEAP_MEMORY_BASE + TDX_HEAP_MEMORY_SIZE)
+ %define TDX_STACK_MEMORY_SIZE FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) / 2
+
+ %define TDX_HOB_MEMORY_BASE FixedPcdGet32 (PcdOvmfSecGhcbBase)
+ %define TDX_HOB_MEMORY_SIZE FixedPcdGet32 (PcdOvmfSecGhcbSize)
+
+ %define TDX_MAILBOX_MEMORY_BASE FixedPcdGet32 (PcdOvmfSecGhcbBackupBase)
+ %define TDX_MAILBOX_MEMORY_SIZE FixedPcdGet32 (PcdOvmfSecGhcbBackupSize)
+
+ %define OVMF_PAGE_TABLE_BASE FixedPcdGet32 (PcdOvmfSecPageTablesBase)
+ %define OVMF_PAGE_TABLE_SIZE FixedPcdGet32 (PcdOvmfSecPageTablesSize)
+
+ %define TDX_EXTRA_PAGE_TABLE_BASE FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase)
+ %define TDX_EXTRA_PAGE_TABLE_SIZE FixedPcdGet32 (PcdOvmfSecGhcbPageTableSize)
+
+ %define CC_WORK_AREA_MEMORY_BASE FixedPcdGet32 (PcdSevEsWorkAreaBase)
+ %define CC_WORK_AREA_MEMORY_SIZE FixedPcdGet32 (PcdSevEsWorkAreaSize)
+ %define CC_WORK_AREA (CC_WORK_AREA_MEMORY_BASE)
+ %define CC_WORK_AREA_SUBTYPE (CC_WORK_AREA + 1)
+ %define TDX_WORK_AREA_PAGELEVEL5 (CC_WORK_AREA + 4)
+ %define TDX_WORK_AREA_PGTBL_READY (CC_WORK_AREA + 5)
+ %define TDX_WORK_AREA_GPAW (CC_WORK_AREA + 8)
+
+ %define TDX_PT_ADDR(Offset) (TDX_EXTRA_PAGE_TABLE_BASE + (Offset))
+
%define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + (Offset))
%define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
@@ -76,9 +114,12 @@
%define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 8)
%define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 16)
%define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) + FixedPcdGet32 (PcdOvmfSecPeiTempRamSize))
-%include "Ia32/Flat32ToFlat64.asm"
-%include "Ia32/AmdSev.asm"
-%include "Ia32/PageTables64.asm"
+
+ %include "X64/IntelTdxMetadata.asm"
+ %include "Ia32/IntelTdx.asm"
+ %include "Ia32/Flat32ToFlat64.asm"
+ %include "Ia32/AmdSev.asm"
+ %include "Ia32/PageTables64.asm"
%endif
%include "Ia16/Real16ToFlat32.asm"
@@ -91,5 +132,6 @@
%define SEV_LAUNCH_SECRET_SIZE FixedPcdGet32 (PcdSevLaunchSecretSize)
%define SEV_FW_HASH_BLOCK_BASE FixedPcdGet32 (PcdQemuHashTableBase)
%define SEV_FW_HASH_BLOCK_SIZE FixedPcdGet32 (PcdQemuHashTableSize)
+ %define OVMF_IMAGE_SIZE_IN_KB FixedPcdGet32 (PcdOvmfImageSizeInKb)
%include "Ia16/ResetVectorVtf0.asm"
diff --git a/OvmfPkg/ResetVector/X64/IntelTdxMetadata.asm b/OvmfPkg/ResetVector/X64/IntelTdxMetadata.asm
new file mode 100644
index 000000000000..97b1e915d899
--- /dev/null
+++ b/OvmfPkg/ResetVector/X64/IntelTdxMetadata.asm
@@ -0,0 +1,110 @@
+;------------------------------------------------------------------------------
+; @file
+; Tdx Virtual Firmware metadata
+;
+; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;------------------------------------------------------------------------------
+
+BITS 64
+
+%define TDX_METADATA_SECTION_TYPE_BFV 0
+%define TDX_METADATA_SECTION_TYPE_CFV 1
+%define TDX_METADATA_SECTION_TYPE_TD_HOB 2
+%define TDX_METADATA_SECTION_TYPE_TEMP_MEM 3
+%define TDX_METADATA_VERSION 1
+%define TDX_METADATA_ATTRIBUTES_EXTENDMR 0x00000001
+
+ALIGN 16
+TIMES (15 - ((TdxGuidedStructureEnd - TdxGuidedStructureStart + 15) % 16)) DB 0
+
+TdxGuidedStructureStart:
+
+;
+; TDVF meta data
+;
+TdxMetadataGuid:
+ DB 0xf3, 0xf9, 0xea, 0xe9, 0x8e, 0x16, 0xd5, 0x44
+ DB 0xa8, 0xeb, 0x7f, 0x4d, 0x87, 0x38, 0xf6, 0xae
+
+_Descriptor:
+ DB 'T','D','V','F' ; Signature
+ DD TdxGuidedStructureEnd - _Descriptor ; Length
+ DD TDX_METADATA_VERSION ; Version
+ DD (TdxGuidedStructureEnd - _Descriptor - 16)/32 ; Number of sections
+
+_Bfv:
+ DD TDX_BFV_RAW_DATA_OFFSET
+ DD TDX_BFV_RAW_DATA_SIZE
+ DQ TDX_BFV_MEMORY_BASE
+ DQ TDX_BFV_MEMORY_SIZE
+ DD TDX_METADATA_SECTION_TYPE_BFV
+ DD TDX_METADATA_ATTRIBUTES_EXTENDMR
+
+_Cfv:
+ DD TDX_CFV_RAW_DATA_OFFSET
+ DD TDX_CFV_RAW_DATA_SIZE
+ DQ TDX_CFV_MEMORY_BASE
+ DQ TDX_CFV_MEMORY_SIZE
+ DD TDX_METADATA_SECTION_TYPE_CFV
+ DD 0
+
+_Stack:
+ DD 0
+ DD 0
+ DQ TDX_STACK_MEMORY_BASE
+ DQ TDX_STACK_MEMORY_SIZE
+ DD TDX_METADATA_SECTION_TYPE_TEMP_MEM
+ DD 0
+
+_Heap:
+ DD 0
+ DD 0
+ DQ TDX_HEAP_MEMORY_BASE
+ DQ TDX_HEAP_MEMORY_SIZE
+ DD TDX_METADATA_SECTION_TYPE_TEMP_MEM
+ DD 0
+
+_MailBox:
+ DD 0
+ DD 0
+ DQ TDX_MAILBOX_MEMORY_BASE
+ DQ TDX_MAILBOX_MEMORY_SIZE
+ DD TDX_METADATA_SECTION_TYPE_TEMP_MEM
+ DD 0
+
+_CcWorkarea:
+ DD 0
+ DD 0
+ DQ CC_WORK_AREA_MEMORY_BASE
+ DQ CC_WORK_AREA_MEMORY_SIZE
+ DD TDX_METADATA_SECTION_TYPE_TEMP_MEM
+ DD 0
+
+_TdHob:
+ DD 0
+ DD 0
+ DQ TDX_HOB_MEMORY_BASE
+ DQ TDX_HOB_MEMORY_SIZE
+ DD TDX_METADATA_SECTION_TYPE_TD_HOB
+ DD 0
+
+_TdxPageTable:
+ DD 0
+ DD 0
+ DQ TDX_EXTRA_PAGE_TABLE_BASE
+ DQ TDX_EXTRA_PAGE_TABLE_SIZE
+ DD TDX_METADATA_SECTION_TYPE_TEMP_MEM
+ DD 0
+
+_OvmfPageTable:
+ DD 0
+ DD 0
+ DQ OVMF_PAGE_TABLE_BASE
+ DQ OVMF_PAGE_TABLE_SIZE
+ DD TDX_METADATA_SECTION_TYPE_TEMP_MEM
+ DD 0
+
+TdxGuidedStructureEnd:
+ALIGN 16


Re: [PATCH V4 2/3] OvmfPkg/Sec: Update the check logic in SevEsIsEnabled

Brijesh Singh
 

Hi Min,

On 8/2/21 8:18 PM, Min Xu wrote:
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
SevEsIsEnabled return TRUE if SevEsWorkArea->SevEsEnabled is non-zero.
It is correct when SevEsWorkArea is only used by SEV. After Intel TDX
is enabled in Ovmf, the SevEsWorkArea is shared by TDX and SEV. (This
is to avoid the waist of memory region in MEMFD). The value of
SevEsWorkArea->SevEsEnabled now is :
0 if in Legacy guest
1 if in SEV
2 if in Tdx guest
That's why the changes is made.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/Sec/SecMain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2aa..e166a9389a1a 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -828,7 +828,7 @@ SevEsIsEnabled (
SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
- return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
+ return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled == 1));
}
This is wrong, we need to check the SevEs sub type and not the global Sev enable. This also need to be broken into at least two commits

1. introduce the updated CcWorkArea structure
2. update the existing code to use the CcWorkArea layout

If you are okay then I can rework and send the patch so that you can add the TDX on top of it.

thanks

VOID


Re: [PATCH v2 1/1] MdePkg: add definition of LINUX_EFI_INITRD_MEDIA_GUID

Jeff Brasen
 

Mike, do the comments in this patch work for you? Would be nice to get this in prior to the 8/9 freeze if possible.

Thanks,

Jeff


From: Ard Biesheuvel <ardb@...>
Sent: Tuesday, July 27, 2021 10:48 AM
To: Jeff Brasen <jbrasen@...>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel <ardb+tianocore@...>; Michael Kinney <michael.d.kinney@...>; Jordan Justen <jordan.l.justen@...>; Liming Gao (Byosoft address) <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>
Subject: Re: [PATCH v2 1/1] MdePkg: add definition of LINUX_EFI_INITRD_MEDIA_GUID
 
External email: Use caution opening links or attachments


On Tue, 27 Jul 2021 at 18:45, Jeff Brasen <jbrasen@...> wrote:
>
> Add LINUX_EFI_INITRD_MEDIA_GUID to our collection of GUID definitions,
> it can be used in a media device path to specify a Linux style initrd
> that can be loaded by the OS using the LoadFile2 protocol.
>
> Move these defines to MdePkg from OvmfPkg as these are relevant to
> non-OVMF targets as well.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2564
> Signed-off-by: Jeff Brasen <jbrasen@...>

Acked-by: Ard Biesheuvel <ardb@...>

> ---
>  MdePkg/MdePkg.dec                          |  5 ++++
>  OvmfPkg/OvmfPkg.dec                        |  1 -
>  MdePkg/Include/Guid/LinuxEfiInitrdMedia.h  | 31 ++++++++++++++++++++++
>  OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h | 17 ------------
>  4 files changed, 36 insertions(+), 18 deletions(-)
>  create mode 100644 MdePkg/Include/Guid/LinuxEfiInitrdMedia.h
>  delete mode 100644 OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h
>
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index c5319fdd71ca..a28a2daaffa8 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -818,6 +818,11 @@ [Guids]
>    #
>    gTianoCustomDecompressGuid     = { 0xA31280AD, 0x481E, 0x41B6, { 0x95, 0xE8, 0x12, 0x7F, 0x4C, 0x98, 0x47, 0x79 }}
>
> +  #
> +  # GUID used to provide initrd to linux via LoadFile2 protocol
> +  #
> +  gLinuxEfiInitrdMediaGuid       = {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> +
>  [Guids.IA32, Guids.X64]
>    ## Include/Guid/Cper.h
>    gEfiIa32X64ErrorTypeCacheCheckGuid = { 0xA55701F5, 0xE3EF, 0x43de, { 0xAC, 0x72, 0x24, 0x9B, 0x57, 0x3F, 0xAD, 0x2C }}
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 6ae733f6e39f..3153f5ae4540 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -118,7 +118,6 @@ [Guids]
>    gMicrosoftVendorGuid                  = {0x77fa9abd, 0x0359, 0x4d32, {0xbd, 0x60, 0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b}}
>    gEfiLegacyBiosGuid                    = {0x2E3044AC, 0x879F, 0x490F, {0x97, 0x60, 0xBB, 0xDF, 0xAF, 0x69, 0x5F, 0x50}}
>    gEfiLegacyDevOrderVariableGuid        = {0xa56074db, 0x65fe, 0x45f7, {0xbd, 0x21, 0x2d, 0x2b, 0xdd, 0x8e, 0x96, 0x52}}
> -  gLinuxEfiInitrdMediaGuid              = {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
>    gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
>    gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
>    gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
> diff --git a/MdePkg/Include/Guid/LinuxEfiInitrdMedia.h b/MdePkg/Include/Guid/LinuxEfiInitrdMedia.h
> new file mode 100644
> index 000000000000..0e7db8bd8140
> --- /dev/null
> +++ b/MdePkg/Include/Guid/LinuxEfiInitrdMedia.h
> @@ -0,0 +1,31 @@
> +/** @file
> +  GUID definition for the Linux Initrd media device path
> +
> +  Linux distro boot generally relies on an initial ramdisk (initrd)
> +  which is provided by the loader, and which contains additional kernel
> +  modules (for storage and network, for instance), and the initial user
> +  space startup code, i.e., the code which brings up the user space side
> +  of the entire OS.
> +
> +  In order to provide a standard method to locate this file,
> +  the GUID defined in this file is used to describe the device path
> +  for a LoadFile2 Protocol instance that is responsible for loading the initrd file.
> +
> +  The kernel EFI Stub will locate and use this instance to load the initrd,
> +  therefore the firmware/loader should install an instance of this to load the
> +  relevant initrd.
> +
> +  Copyright (c) 2020, Arm, Ltd. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef LINUX_EFI_INITRD_MEDIA_GUID_H__
> +#define LINUX_EFI_INITRD_MEDIA_GUID_H__
> +
> +#define LINUX_EFI_INITRD_MEDIA_GUID \
> +  {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> +
> +extern EFI_GUID gLinuxEfiInitrdMediaGuid;
> +
> +#endif
> diff --git a/OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h b/OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h
> deleted file mode 100644
> index 83fc3fc79aa6..000000000000
> --- a/OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/** @file
> -  GUID definition for the Linux Initrd media device path
> -
> -  Copyright (c) 2020, Arm, Ltd. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#ifndef LINUX_EFI_INITRD_MEDIA_GUID_H__
> -#define LINUX_EFI_INITRD_MEDIA_GUID_H__
> -
> -#define LINUX_EFI_INITRD_MEDIA_GUID \
> -  {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> -
> -extern EFI_GUID gLinuxEfiInitrdMediaGuid;
> -
> -#endif
> --
> 2.25.1
>


Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree

Sayanta Pattanayak
 

Hi,

Please find my response inline.

Regards,
Sayanta

-----Original Message-----
From: Ard Biesheuvel <ardb@kernel.org>
Sent: Sunday, August 1, 2021 10:08 PM
To: Sayanta Pattanayak <Sayanta.Pattanayak@arm.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Sami Mujawar
<Sami.Mujawar@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Bret
Barkelew <Bret.Barkelew@microsoft.com>; Jiewen Yao
<jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to
populate StMM boot data from device tree

(correct Achin's email address, cc other replyers)

On Sun, 1 Aug 2021 at 18:36, Ard Biesheuvel <ardb@kernel.org> wrote:

On Fri, 30 Jul 2021 at 19:35, Sayanta Pattanayak
<sayanta.pattanayak@arm.com> wrote:

Introduce support to populate StMM boot data via DTS parsing.
Why? Don't we have FF-A manifests for this? I would expect the secure
partition manager to marshal this data into the appropriate format
when necessary.
I may not have presented this patch properly.
The key objective of this patch is that in a FF-A Secure partition manager which has StandaloneMM as S-EL0 partition, the StMM boot data could be passed by partition manager through DT and StMM prepares the boot data by parsing the DT.
In existing solution, secure partition manager has StMM specific logic to fetch the StMM boot data and pass it on to StMM through sharedbuffer. Having FF-A manifest for secure partition, DT in this case, will allow Secure partition manager to not have StMM or any other secure partition specific logic to consolidate boot data and pass on.
In the context of this patch, secure partition manager(from EL3) passes the manifest or DT address as booting argument to StMM. StMM will just have the logic to parse the DT and prepare boot data structure, so the manifest is actually part of partition manager codebase.
StMM as secure partition can be used across various types of Secure partition manager, so some amount of uniformity is needed to follow same DT properties for StMM across all kind of Secure partition managers.

The DTB is
passed as a boot argument by a binary of higer exception level.
Previously it was achieved by placing the boot data structure in a
shared buffer and the address of this shared buffer was passed by
the binary of higher exception level. Now either of the option can
be used for populating StMM boot info.

StMM boot information structure binding in device tree can be of
following prototype. Property values are not mentioned here.

bootarg {
compatible = "bootargs";
h_type = <..>;
h_version = <..>;
h_size = <..>;
h_attr = <..>;
sp_mem_base = <..>;
sp_mem_limit = <..>;
sp_image_base = <..>;
sp_stack_base = <..>;
sp_heap_base = <..>;
sp_ns_comm_buf_base = <..>;
sp_shared_buf_base = <..>;
sp_image_size = <..>;
sp_pcpu_stack_size = <..>;
sp_heap_size = <..>;
sp_ns_comm_buf_size = <..>;
sp_shared_buf_size = <..>;
num_sp_mem_regions = <..>;
num_cpus = <..>;
};

Addition of DTS supoort involves a dependency on FdtLib from
EmbeddedPkg.

Signed-off-by: Sayanta Pattanayak <sayanta.pattanayak@arm.com>
I don't think we should apply this change. DT is not part of the
original SPM or current FF-A spec, right? So please fix this in the
S-EL1 component instead.


---
Link to github branch with this patch -
https://github.com/SayantaP-arm/edk2/tree/stmm-dts

StandaloneMmPkg/StandaloneMmPkg.dsc | 1
+
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
reEntryPoint.inf | 3 +

StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
n
eMmCoreEntryPoint.c | 153 ++++++++++++++++++--
3 files changed, 143 insertions(+), 14 deletions(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index 0c45df95e2dd..e3a3a6ee3ba1 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -49,6 +49,7 @@
HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHo
bLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf

MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
MemLib
.inf
+ FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemor
yAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
aloneMmServicesTableLib.inf
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
Cor
eEntryPoint.inf
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
Cor
eEntryPoint.inf index 4fa426f58ef4..0a2e519dd664 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
Cor
eEntryPoint.inf
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneM
+++ mCoreEntryPoint.inf
@@ -30,6 +30,7 @@
X64/StandaloneMmCoreEntryPoint.c

[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
StandaloneMmPkg/StandaloneMmPkg.dec
@@ -40,10 +41,12 @@
[LibraryClasses]
BaseLib
DebugLib
+ FdtLib

[LibraryClasses.AARCH64]
StandaloneMmMmuLib
ArmSvcLib
+ FdtLib

[Guids]
gMpInformationHobGuid
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
l
oneMmCoreEntryPoint.c
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
l
oneMmCoreEntryPoint.c index 6c50f470aa35..cc09d75dac36 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
l
oneMmCoreEntryPoint.c
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Sta
+++ ndaloneMmCoreEntryPoint.c
@@ -16,6 +16,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/MmramMemoryReserve.h> #include
<Guid/MpInformation.h>

+#include <libfdt.h>
#include <Library/ArmMmuLib.h>
#include <Library/ArmSvcLib.h>
#include <Library/DebugLib.h>
@@ -45,33 +46,31 @@ STATIC CONST UINT32 mSpmMinorVerFfa =
SPM_MINOR_VERSION_FFA;
PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT CpuDriverEntryPoint =
NULL;

/**
- Retrieve a pointer to and print the boot information passed by
privileged
- secure firmware.
+ Prints boot information.

- @param [in] SharedBufAddress The pointer memory shared with
privileged
- firmware.
+ This function prints the boot information, which is passed by
+ privileged secure firmware through shared buffer or other mechanism.

+ @param [in] PayloadBootInfo Pointer to StandaloneMM Boot Info
structure.
**/
-EFI_SECURE_PARTITION_BOOT_INFO *
-GetAndPrintBootinformation (
- IN VOID *SharedBufAddress
+VOID
+PrintBootinformation (
+ IN EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo
)
{
- EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
EFI_SECURE_PARTITION_CPU_INFO *PayloadCpuInfo;
UINTN Index;

- PayloadBootInfo = (EFI_SECURE_PARTITION_BOOT_INFO *)
SharedBufAddress;

if (PayloadBootInfo == NULL) {
DEBUG ((DEBUG_ERROR, "PayloadBootInfo NULL\n"));
- return NULL;
+ return;
}

if (PayloadBootInfo->Header.Version != BOOT_PAYLOAD_VERSION) {
DEBUG ((DEBUG_ERROR, "Boot Information Version Mismatch.
Current=0x%x, Expected=0x%x.\n",
PayloadBootInfo->Header.Version, BOOT_PAYLOAD_VERSION));
- return NULL;
+ return;
}

DEBUG ((DEBUG_INFO, "NumSpMemRegions - 0x%x\n",
PayloadBootInfo->NumSpMemRegions));
@@ -96,7 +95,7 @@ GetAndPrintBootinformation (

if (PayloadCpuInfo == NULL) {
DEBUG ((DEBUG_ERROR, "PayloadCpuInfo NULL\n"));
- return NULL;
+ return;
}

for (Index = 0; Index < PayloadBootInfo->NumCpus; Index++) { @@
-105,7 +104,7 @@ GetAndPrintBootinformation (
DEBUG ((DEBUG_INFO, "Flags - 0x%x\n",
PayloadCpuInfo[Index].Flags));
}

- return PayloadBootInfo;
+ return;
}

/**
@@ -194,6 +193,119 @@ DelegatedEventLoop (
}
}

+/**
+ Populates StandAloneMM boot information structure.
+
+ This function receives dtb Address, where StMM Boot information
+ specific properties will be looked out to form the booting
+ structure of type EFI_SECURE_PARTITION_BOOT_INFO. At first, the
+ properties for StandAloneMM ConfigSize and Memory limit will be
+ checked out. Boot information will be stored at address (Memory
+ Limit - ConfigSize). Thereafter all boot information specific
+ properties will be parsed and corresponding values will be obtained.
+
+ @param [out] BootInfo Pointer, where Boot Info structure will be
populated.
+ @param [in] DtbAddress Address of the Device tree from where Boot
+ information will be fetched.
+**/
+VOID
+PopulateBootinformation (
+ OUT EFI_SECURE_PARTITION_BOOT_INFO **BootInfo,
+ IN VOID *DtbAddress
+)
+{
+ INT32 Offset;
+ CONST UINT32 *Property;
+ CONST UINT64 *Property64;
+ UINT32 ConfigSize;
+ UINT64 SpMemLimit;
+ EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
+
+ Offset = fdt_node_offset_by_compatible (DtbAddress, -1,
+ "config-size"); if (Offset < 0) {
+ DEBUG ((DEBUG_WARN, "Total Config Size is not defined\n")); }
+ else {
+ Property = fdt_getprop (DtbAddress, Offset, "size", NULL);
+ if (Property) {
+ ConfigSize = fdt32_to_cpu (*Property);
+ DEBUG ((DEBUG_INFO, "stmm dtb config-size = 0x%x \n",
ConfigSize));
+ }
+ }
+
+ Offset = fdt_node_offset_by_compatible (DtbAddress, -1,
+ "bootargs"); if (Offset >= 0) {
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_mem_limit",
NULL);
+ SpMemLimit = fdt64_to_cpu (*Property64); }
+
+ if (SpMemLimit && ConfigSize)
+ PayloadBootInfo =
+ (EFI_SECURE_PARTITION_BOOT_INFO *)(SpMemLimit - ConfigSize);
+
+ if (PayloadBootInfo) {
+ PayloadBootInfo->SpMemLimit = SpMemLimit;
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_type", NULL);
+ PayloadBootInfo->Header.Type = (UINT8) fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_version", NULL);
+ PayloadBootInfo->Header.Version = (UINT8)
+ fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_size", NULL);
+ PayloadBootInfo->Header.Size = (UINT8) fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_attr", NULL);
+ PayloadBootInfo->Header.Attr = fdt32_to_cpu(*Property);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_mem_base",
NULL);
+ PayloadBootInfo->SpMemBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_image_base",
NULL);
+ PayloadBootInfo->SpImageBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_stack_base",
NULL);
+ PayloadBootInfo->SpStackBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_heap_base",
NULL);
+ PayloadBootInfo->SpHeapBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset,
"sp_ns_comm_buf_base", NULL);
+ PayloadBootInfo->SpNsCommBufBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset,
"sp_shared_buf_base", NULL);
+ PayloadBootInfo->SpSharedBufBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_image_size",
NULL);
+ PayloadBootInfo->SpImageSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_pcpu_stack_size",
NULL);
+ PayloadBootInfo->SpPcpuStackSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_heap_size", NULL);
+ PayloadBootInfo->SpHeapSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset,
"sp_ns_comm_buf_size", NULL);
+ PayloadBootInfo->SpNsCommBufSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_shared_buf_size",
NULL);
+ PayloadBootInfo->SpPcpuSharedBufSize =
+ fdt64_to_cpu(*Property64);
+
+ Property = fdt_getprop (DtbAddress, Offset, "num_sp_mem_regions",
NULL);
+ PayloadBootInfo->NumSpMemRegions = fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "num_cpus", NULL);
+ PayloadBootInfo->NumCpus = fdt32_to_cpu(*Property);
+
+ PayloadBootInfo->CpuInfo =
+ (EFI_SECURE_PARTITION_CPU_INFO *)((UINT64)PayloadBootInfo +
+
+ sizeof(EFI_SECURE_PARTITION_BOOT_INFO));
+ }
+
+ *BootInfo = PayloadBootInfo;
+
+ return;
+}
+
/**
Query the SPM version, check compatibility and return success if
compatible.

@@ -313,6 +425,7 @@ _ModuleEntryPoint (
VOID *TeData;
UINTN TeDataSize;
EFI_PHYSICAL_ADDRESS ImageBase;
+ VOID *DtbAddress;

// Get Secure Partition Manager Version Information
Status = GetSpmVersion ();
@@ -320,12 +433,24 @@ _ModuleEntryPoint (
goto finish;
}

- PayloadBootInfo = GetAndPrintBootinformation (SharedBufAddress);
+ // In cookie1 the DTB address is passed. With reference to DTB,
+ Boot // info structure can be populated.
+ // If cookie1 doesn't have any value, then Boot info is copied
+ from // Sharedbuffer.
+ if (cookie1) {
+ DtbAddress = (void *)cookie1;
+ PopulateBootinformation (&PayloadBootInfo, DtbAddress); } else
+ {
+ PayloadBootInfo = (EFI_SECURE_PARTITION_BOOT_INFO
+ *)SharedBufAddress; }
+
if (PayloadBootInfo == NULL) {
Status = EFI_UNSUPPORTED;
goto finish;
}

+ PrintBootinformation (PayloadBootInfo);
+
// Locate PE/COFF File information for the Standalone MM core module
Status = LocateStandaloneMmCorePeCoffData (
(EFI_FIRMWARE_VOLUME_HEADER *)
PayloadBootInfo->SpImageBase,
--
2.17.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [edk2-platforms PATCH v5 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy.

Ard Biesheuvel
 

On Tue, 3 Aug 2021 at 15:10, Ard Biesheuvel <ardb@kernel.org> wrote:

On Tue, 3 Aug 2021 at 15:08, Pete Batard <pete@akeo.ie> wrote:

Hi Ard,

I thought the R-b from Sunny was enough.

For what is worth, I briefly tested these changes for v4.
Thus:

On 2021.08.03 14:03, Ard Biesheuvel wrote:
On Tue, 3 Aug 2021 at 15:00, Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com> wrote:

Ard,

Now that the EDK2 changes are merged (aaecef38b9440a65809cbdaf9d97029f4eeb), I think these RPi specific changes are ready to be merged as well.
I only see acks from Sunny though. This is why I asked Pete and Andrei
to chime in as well.
...

Reviewed-by: Pete Batard <pete@akeo.ie>
Tested-by: Pete Batard <pete@akeo.ie>
Thanks Pete. I did not want to assume that stakeholders like yourself
are ok with such changes if they did not get involved in the
discussion.

I will merge these now.
Pushed as 3ccc21d918c..2f0188b56ef4

Thanks to Grzegorz and others involved for finding a way to address
this boot protocol issue elegantly!

--
Ard.


Re: Proposing a new area of the edk2-test repository

Bret Barkelew
 

Yeah, go for the patch.

 

- Bret

 

From: Samer El-Haj-Mahmoud via groups.io
Sent: Tuesday, August 3, 2021 6:10 AM
To: Nelson, Eric; Bret Barkelew; devel@edk2.groups.io; G Edhaya Chandran; gaojie@...; Kinney, Michael D
Cc: Samer El-Haj-Mahmoud
Subject: [EXTERNAL] Re: [edk2-devel] Proposing a new area of the edk2-test repository

 

I would think just sending the code contribution patch is sufficient.

 

 

From: Nelson, Eric <eric.nelson@...>
Sent: Wednesday, July 28, 2021 3:05 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Bret Barkelew <Bret.Barkelew@...>; devel@edk2.groups.io; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; Kinney, Michael D <michael.d.kinney@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

 

Adding ResumeOK.efi tool under /edk2-test/test-tools/TestToolsPkg would be great.

 

Should I propose this in the RFC and DEVEL mailing lists as a next step?

 

Thanks,

__e

 

 

From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Sent: Friday, July 9, 2021 1:12 PM
To: Bret Barkelew <Bret.Barkelew@...>; devel@edk2.groups.io; Nelson, Eric <eric.nelson@...>; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; Kinney, Michael D <michael.d.kinney@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

Interesting, thanks for sharing Bret. Some of those tests seem to be x64 specific (SMM tests), and some can be more generic like MorLockTestApp

 

Like I said earlier, I am not against adding test tools to edk2-test. That in fact is welcomed, especially if their usefulness in validating the solutions extend beyond specific implementations.

 

What would a good tree structure look like to accommodate misc tools? Today we have

 

/edk2-test/uefi-sct/SctPkg

 

How about something like this?

/edk2-test/test-tools/TestToolsPkg

or /edk2-test/ TestToolsPkg

 

The “ResumeOK” can be placed there

 

Any other ideas?

 

 

From: Bret Barkelew <Bret.Barkelew@...>
Sent: Thursday, June 24, 2021 12:25 AM
To: devel@edk2.groups.io; eric.nelson@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; Kinney, Michael D <michael.d.kinney@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

Fun fact! Mu also has a number of apps and things that we could work on moving to EDK2 if there were a suitable location. Right now, many of them are here:

mu_plus/UefiTestingPkg at release/202102 · microsoft/mu_plus (github.com)

 

- Bret

 

From: Nelson, Eric via groups.io
Sent: Wednesday, June 23, 2021 3:38 PM
To: Samer El-Haj-Mahmoud; G Edhaya Chandran; gaojie@...; devel@edk2.groups.io; Kinney, Michael D
Subject: [EXTERNAL] Re: [edk2-devel] Proposing a new area of the edk2-test repository

 

 

I have created a few other internal apps that build under WinTestPkg, although ResumeOK.efi is the only one I have received permissions to release sources for at this time.

And yes, they are primarily intended for validating Windows requirements.

I had some issues with my apps, needing to use different libraries than MdeModulePkg, and found it easier to create my own package, and use the libs I want.

 

__e

 

 

From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Sent: Wednesday, June 23, 2021 1:56 PM
To: Nelson, Eric <eric.nelson@...>; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

+edk2 list

 

I am not against adding additional test tools to edk2-test. Just feel like there is a need to organize and have a strategy, rather than just use edk2-test as a dumping group of miscellaneous tools.

 

There is already a place for apps under https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Application

 

We also have a number of EDK2 misc applications that use edk2-libc in https://github.com/tianocore/edk2-libc/tree/master/AppPkg/Applications

 

A couple of questions:

  • Do you expect more apps from WinTestPkg to be contributed to TianoCore? And are they all around testing specific Windows requirements? If so, then having an edk2-test/WinTestPkg makes sense to me, as you will have a collection of useful testing app targeting specific area.
    • But what about other OSes?
  • If this is a one-off test app and other WinTestPkg apps are not going to be contributed, then does it make sense to put this under MdeModulePkg/Application ?

 

 

 

From: Nelson, Eric <eric.nelson@...>
Sent: Wednesday, June 23, 2021 3:10 PM
To: G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

 

Hi Edhay,

 

Do you have any more questions?

What do you think of creating another directory in edk2-test, for other test apps, in addition to uefi-sct, such as ResumeOK.efi?

 

Thanks,

__e

 

 

From: Nelson, Eric
Sent: Tuesday, June 15, 2021 12:00 PM
To: G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

 

Hi Edhay,

 

ResumeOK.efi is a tool I wrote from the HelloWorld example, that validates Windows resume from S4 requirements, specifically that the memory-map run-time memory regions don’t change, and secondly that PCI devices don’t disappear from the system, both conditions would cause Windows to fail to resume from S4.

 

You install the tool to the root of the ESP, and set it as the default/top entry in the boot manager, and launch it.  (Disable Secure Boot.)

 

It runs warm, cold, and 60s ACPI RTC wake cycles, infinitely looking for errors.

 

ResumeOK.efi writes a file to the root of the ESP, ResumeOK.map, which contains the ACPI Facs->HardwareSignature, a list of the PCI devices in the system, and a copy of its memory map, from the first time it runs.

 

During each test pass, it runs a barrage of tests:

 

  1. Free memory test – does the available memory match the memory map saved in ResumeOK.map
  2. HW signature check – does the system still have the same HW signature as saved in the ResumeOK.map
  3. Allocation test – all the available memory is allocated, and then the memory map is checked if the run-time regions match ResumeOK.map.

 

If any of the tests fail, then the new/missing PCI devices are listed (HW signature fail case), or the memory descriptor that changed, it’s location, and current and previous type and size.

 

I have received permission from Intel to *try* to release the source under Edk2-test.

 

I’ve included a 64-bit binary, if you want to give it a test drive.

 

Make sure Secure Boot is off.

Also, it is required to manually delete any ResumeOK.map on the ESP, before beginning a new test pass.

 

The tool also supports a host of EFI Shell commands:

 

Resumeok.efi MEMMAP – displays Windows coalesced view of the current memory map

ResumeOK.efi ROKMAP – displays Windows coalesced view of the memory saved in ResumeOK.map

ResumeOK.efi RTDATA – displays an analysis of RT_Data pool usage

ResumeOK.efi NORESET – run one test pass, but suppress automatic SX cycling

 

These are the files that build it:

 

Edk2\WinTestPkg\Application

Edk2\WinTestPkg\WinTestPkg.dec

Edk2\WinTestPkg\WinTestPkg.dsc

Edk2\WinTestPkg\Application\ResumeOK

Edk2\WinTestPkg\Application\ResumeOK\AcpiTbl.c

Edk2\WinTestPkg\Application\ResumeOK\AcpiTbl.h

Edk2\WinTestPkg\Application\ResumeOK\AppSupport.c

Edk2\WinTestPkg\Application\ResumeOK\BitMap.c

Edk2\WinTestPkg\Application\ResumeOK\BitMap.h

Edk2\WinTestPkg\Application\ResumeOK\EfiFileLib.c

Edk2\WinTestPkg\Application\ResumeOK\EfiFileLib.h

Edk2\WinTestPkg\Application\ResumeOK\pci.c

Edk2\WinTestPkg\Application\ResumeOK\Pci.h

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.c

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.h

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.inf

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.uni

Edk2\WinTestPkg\Application\ResumeOK\ResumeOKExtra.uni

Edk2\WinTestPkg\Application\ResumeOK\RtData.c

Edk2\WinTestPkg\Application\ResumeOK\TimeBaseLib.c

 

Thanks,

__e

 

 

From: G Edhaya Chandran <Edhaya.Chandran@...>
Sent: Monday, June 14, 2021 9:36 PM
To: Nelson, Eric <eric.nelson@...>; gaojie@...
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

Hi Eric,

 

    Thanks for reaching out to us.

Can we get more details of the tool?

 

Is this tool already open sourced or could you send us the basic documentation pertaining to it.

 

With Warm Regards,
Edhay

 

 

From: Nelson, Eric <eric.nelson@...>
Sent: 15 June 2021 04:23
To: gaojie@...; G Edhaya Chandran <Edhaya.Chandran@...>
Subject: Proposing a new area of the edk2-test repository

 

 

Hello SCT maintainers,

 

I’m looking to release source to a UEFI validation tool that has been a big hit with platform BIOS validation teams, so it can help other PC vendors.

 

My coworker Michael Kinney suggested I reach out to you directly about the idea of creating a new top level directory in the edk2-test repro for other test apps, and I could be maintainer.

 

What do you think of creating another directory in edk2-test, for other test apps, in addition to uefi-sct?

 

Thanks!

__e

 

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.

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.

 

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.

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.

 


Re: [PATCH edk2-test 1/1] uefi-sct/SctPkg: correct print code for EFI_MEMORY_TYPE

Samer El-Haj-Mahmoud
 

Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Heinrich
Schuchardt via groups.io
Sent: Friday, April 30, 2021 3:40 PM
To: EDK II Development <devel@edk2.groups.io>
Cc: Eric Jin <eric.jin@intel.com>; G Edhaya Chandran
<Edhaya.Chandran@arm.com>; Barton Gao <gaojie@byosoft.com.cn>; Arvin
Chen <arvinx.chen@intel.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Heinrich Schuchardt <xypron.glpk@gmx.de>
Subject: [edk2-devel] [PATCH edk2-test 1/1] uefi-sct/SctPkg: correct print code
for EFI_MEMORY_TYPE

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

EFI_MEMORY_TYPE is an enum. SctPrint expects an UINTN when printing with
%d. Add missing conversions in MemoryAllocationServicesBBTestFunction.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
.../MemoryAllocationServicesBBTestFunction.c | 98 +++++++++----------
1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c b/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c
index bf8cd3b3afa4..e545b3cfc5b8 100644
--- a/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c
+++ b/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c
@@ -417,7 +417,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -437,7 +437,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -455,7 +455,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -478,7 +478,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -512,7 +512,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -532,7 +532,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory <= Descriptor.PhysicalStart +
SctLShiftU64 (Descriptor.NumberOfPages, EFI_PAGE_SHIFT) -
@@ -554,7 +554,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex],
+ (UINTN)AllocatePagesMemoryType[TypeIndex],
Descriptor.PhysicalStart,
Descriptor.NumberOfPages,
Memory
@@ -589,7 +589,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory2 & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -609,7 +609,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if ( Memory2 <= Descriptor.PhysicalStart +
SctLShiftU64 (Descriptor.NumberOfPages, EFI_PAGE_SHIFT) -
@@ -631,7 +631,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex],
+ (UINTN)AllocatePagesMemoryType[TypeIndex],
Memory2
);
if (Memory != 0) {
@@ -650,7 +650,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -670,7 +670,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -694,7 +694,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -739,7 +739,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -759,7 +759,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -779,7 +779,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -797,7 +797,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -824,7 +824,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -869,7 +869,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -889,7 +889,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -909,7 +909,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -927,7 +927,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -947,7 +947,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -992,7 +992,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1012,7 +1012,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start + (SctLShiftU64 (PageNum/3, EFI_PAGE_SHIFT) &
0xFFFFFFFFFFFF0000)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1032,7 +1032,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1050,7 +1050,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1070,7 +1070,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1115,7 +1115,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1135,7 +1135,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start + (SctLShiftU64 (PageNum * 2 / 3, EFI_PAGE_SHIFT) &
0xFFFFFFFFFFFF0000)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1155,7 +1155,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1173,7 +1173,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1200,7 +1200,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1245,7 +1245,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1265,7 +1265,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1285,7 +1285,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1303,7 +1303,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1323,7 +1323,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1377,7 +1377,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1397,7 +1397,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1417,7 +1417,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
if (PageNum != 1) {
@@ -1442,7 +1442,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1462,7 +1462,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1507,7 +1507,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1527,7 +1527,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1547,7 +1547,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1565,7 +1565,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1656,7 +1656,7 @@ BBTestFreePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
continue;
}
@@ -1685,7 +1685,7 @@ BBTestFreePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}

--
2.30.2



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74678): https://edk2.groups.io/g/devel/message/74678
Mute This Topic: https://groups.io/mt/82490304/1945644
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
mahmoud@arm.com]
-=-=-=-=-=-=
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.


Re: [PATCH v2 0/4] ArmPlatformPkg: Add support to generate HEST ACPI table

Sami Mujawar
 

Hi Omkar,

How did you check that the HEST table is populated correctly?

There is no HEST parser in Acpiview at the moment. Do you plan to add an HEST parser to Acpiview?

Regards,

Sami Mujawar

On 02/08/2021 01:49 PM, Sami Mujawar wrote:
Hi Omkar,

Thank you for this patch series and for the clear explaination below.

The explaination below is very useful for anyone who is trying to understand the code.

Since the cover letter will not be part of the patch commit messages, would it be possible to include this explanation:

1. as part of a commit message for one of the patches in this series (patch 2/4 or 3/4).

Or

2. in a Readme.md file

Regards,

Sami Mujawar


On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Changes since v1:
- Helper added for HEST ACPI table generation.
- Rebased to the latest upstream code.

Hardware Error Source Table (HEST)[1] and Software Delegated Exception Interface
(SDEI)[2] ACPI tables are used to acomplish firmware first error handling.This
patch series introduces a framework to build and install the HEST ACPI table
dynamically.

The following figure illustrates the possible usage of the dyanamic
generation of HEST ACPI table.

NS | S
+--------------------------------------+--------------------------------------+
| | |
|+-------------------------------------+---------------------+ |
|| +---------------------+--------------------+| |
|| | | || |
|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
|| |HestTable | || HestErrorSource | | | HestErrorSource ||| | DMC-620 ||
|| | DXE | || DXE | | | StandaloneMM ||| |Standalone MM||
|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
|| |GHESv2 | || |
|| +---------------------+--------------------+| |
|| +--------------------+ | | |
|| |PlatformErrorHandler| | | |
|| | DXE | | | |
|| +--------------------+ | | |
||FF FWK | | |
|+-------------------------------------+---------------------+ |
| | |
+--------------------------------------+--------------------------------------+
|
Figure: Firmware First Error Handling approach.

All the hardware error sources are added to HEST table as GHESv2[3] error source
descriptors. The framework comprises of following DXE and MM drivers:

- HestTableDxe:
Builds HEST table header and allows appending error source descriptors to the
HEST table. Also provides protocol interface to install the built HEST table.

- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
These two drivers together retrieve all possible error source descriptors of
type GHESv2 from the MM drivers implementing HEST Error Source Descriptor
protocol. Once all the descriptors are collected HestErrorSourceDxe appends
it to HEST table using HestTableDxe driver.
- PlatformErrorHandlerDxe:
Builds and installs SDEI ACPI table. This driver does not initialize(load)
until HestErrorSourceDxe driver has finished appending all possible GHESv2
error source descriptors to the HEST table. Once that is complete using the
HestTableDxe driver it installs the HEST table.

This patch series provides reference implementation for DMC-620 Dynamic Memory
Controller[4] that has RAS feature enabled. This is platform code
implemented as Standalone MM driver in edk2-platforms.

References:
[1] : ACPI 6.3, Table 18-382, Hardware Error Source Table
[2] : SDEI Platform Design Document, revision b, 10 Appendix C, ACPI table
definitions for SDEI
[3] : ACPI Reference Specification 6.3, Table 18-393 GHESv2 Structure
[4] : DMC620 Dynamic Memory Controller, revision r1p0
[5] : UEFI Reference Specification 2.8, Appendix N - Common Platform Error
Record
[6] : UEFI Reference Specification 2.8, Section N.2.5 Memory Error Section

Link to github branch with the patches in this series -
https://github.com/omkkul01/edk2/tree/ras_firmware_first_edk2

Omkar Anand Kulkarni (4):
ArmPlatformPkg: Allow dynamic generation of HEST ACPI table
ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
ArmPlatformPkg: retreive error source descriptors from MM
ArmPlatformPkg: Add helpers for HEST table generation

ArmPlatformPkg/ArmPlatformPkg.dec | 12 +
.../Drivers/Apei/HestDxe/HestDxe.inf | 49 +++
.../HestMmErrorSources/HestErrorSourceDxe.inf | 44 +++
.../HestErrorSourceStandaloneMm.inf | 51 +++
.../HestMmErrorSourceCommon.h | 37 ++
ArmPlatformPkg/Include/HestAcpiHeader.h | 49 +++
.../Include/Protocol/HestErrorSourceInfo.h | 64 ++++
ArmPlatformPkg/Include/Protocol/HestTable.h | 71 ++++
ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c | 354 ++++++++++++++++++
.../HestMmErrorSources/HestErrorSourceDxe.c | 308 +++++++++++++++
.../HestErrorSourceStandaloneMm.c | 312 +++++++++++++++
11 files changed, 1351 insertions(+)
create mode 100644 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
create mode 100644 ArmPlatformPkg/Include/HestAcpiHeader.h
create mode 100644 ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
create mode 100644 ArmPlatformPkg/Include/Protocol/HestTable.h
create mode 100644 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c


Re: [RFC PATCH v5 07/28] OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase

Brijesh Singh
 

On 7/31/21 3:44 AM, Erdem Aktas wrote:
On Wed, Jun 30, 2021 at 5:54 AM Brijesh Singh <brijesh.singh@amd.com> wrote:

a) Enhance the OVMF reset vector code to validate the pages as described
above (go through step 2 - 3).
OR
b) Validate the pages during the guest creation time. The SEV firmware
provides a command which can be used by the VMM to validate the pages
without affecting the measurement of the launch.
Are you referring to the PAGE_TYPE_UNMEASURED? Does it not affect the
measurement , PAGE_INFO will be still measured, right?
Yes. The unmeasured here means the contents of the page is not measured but the PAGE_INFO is measured for all the pages added before the VM launch.


Approach #b seems much simpler; it does not require any changes to the
OVMF reset vector code.
I am worried about verifying the measurement. I understand the secret
page and cpuid page being part of measurement because both of them are
mentioned in the AMD SNP SPEC but now we are introducing a new
parameters (all the 4KB page addresses between SNP_HV_VALIDATED_START
and SNP_HV_VALIDATED_END) that VM owner needs to know to calculate the
measurement and verify the attestation.
The page info of both the secrets and cpuid page also need to be measured. In order to calculate the expected measurement, a caller need to know the page_info for the secrets and cpuid. To get the page_info for the CPUID and Secrets they must read the OVMF reset GUID. While at it, they can also get the the range of the unmeasured pages. I don't see that being a big issue. Having said so, as I described in the patch, its not only option. It was easier for implementation without compromising the security.


Sorry if I am overthinking or missing something here.
-Erdem

11141 - 11160 of 89694