Date   

[PATCH v12 05/46] MdeModulePkg/DxeIplPeim: Support GHCB pages when creating page tables

Lendacky, Thomas
 

From: Tom Lendacky <thomas.lendacky@...>

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

GHCB pages must be mapped as shared pages, so modify the process of
creating identity mapped pagetable entries so that GHCB entries are
created without the encryption bit set. The GHCB range consists of
two pages per CPU, the first being the GHCB and the second being a
per-CPU variable page. Only the GHCB page is mapped as shared.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
---
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +
.../Core/DxeIplPeim/X64/VirtualMemory.h | 12 +++-
.../Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 +-
.../Core/DxeIplPeim/X64/DxeLoadFunc.c | 11 +++-
.../Core/DxeIplPeim/X64/VirtualMemory.c | 57 +++++++++++++++----
5 files changed, 70 insertions(+), 16 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index 3f1702854660..19b8a4c8aefa 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -115,6 +115,8 @@ [Pcd.IA32,Pcd.X64]
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize ## CONSUMES

[Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
index 2d0493f109e8..6b7c38a441d6 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
@@ -201,6 +201,8 @@ EnableExecuteDisableBit (
@param[in, out] PageEntry2M Pointer to 2M page entry.
@param[in] StackBase Stack base address.
@param[in] StackSize Stack size.
+ @param[in] GhcbBase GHCB page area base address.
+ @param[in] GhcbSize GHCB page area size.

**/
VOID
@@ -208,7 +210,9 @@ Split2MPageTo4K (
IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
IN OUT UINT64 *PageEntry2M,
IN EFI_PHYSICAL_ADDRESS StackBase,
- IN UINTN StackSize
+ IN UINTN StackSize,
+ IN EFI_PHYSICAL_ADDRESS GhcbBase,
+ IN UINTN GhcbSize
);

/**
@@ -217,6 +221,8 @@ Split2MPageTo4K (

@param[in] StackBase Stack base address.
@param[in] StackSize Stack size.
+ @param[in] GhcbBase GHCB page area base address.
+ @param[in] GhcbSize GHCB page area size.

@return The address of 4 level page map.

@@ -224,7 +230,9 @@ Split2MPageTo4K (
UINTN
CreateIdentityMappingPageTables (
IN EFI_PHYSICAL_ADDRESS StackBase,
- IN UINTN StackSize
+ IN UINTN StackSize,
+ IN EFI_PHYSICAL_ADDRESS GhcbBase,
+ IN UINTN GhcbkSize
);


diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 6e8ca824d469..284b34818ca7 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -123,7 +123,7 @@ Create4GPageTablesIa32Pae (
//
// Need to split this 2M page that covers stack range.
//
- Split2MPageTo4K (PhysicalAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
+ Split2MPageTo4K (PhysicalAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize, 0, 0);
} else {
//
// Fill in the Page Directory entries
@@ -282,7 +282,7 @@ HandOffToDxeCore (
//
// Create page table and save PageMapLevel4 to CR3
//
- PageTables = CreateIdentityMappingPageTables (BaseOfStack, STACK_SIZE);
+ PageTables = CreateIdentityMappingPageTables (BaseOfStack, STACK_SIZE, 0, 0);

//
// End of PEI phase signal
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
index f465eb1d8ac4..156a477d8467 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
@@ -35,6 +35,8 @@ HandOffToDxeCore (
UINT32 Index;
EFI_VECTOR_HANDOFF_INFO *VectorInfo;
EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi;
+ VOID *GhcbBase;
+ UINTN GhcbSize;

//
// Clear page 0 and mark it as allocated if NULL pointer detection is enabled.
@@ -81,12 +83,19 @@ HandOffToDxeCore (
TopOfStack = (VOID *) ((UINTN) BaseOfStack + EFI_SIZE_TO_PAGES (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);
TopOfStack = ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);

+ //
+ // Get the address and size of the GHCB pages
+ //
+ GhcbBase = (VOID *) PcdGet64 (PcdGhcbBase);
+ GhcbSize = PcdGet64 (PcdGhcbSize);
+
PageTables = 0;
if (FeaturePcdGet (PcdDxeIplBuildPageTables)) {
//
// Create page table and save PageMapLevel4 to CR3
//
- PageTables = CreateIdentityMappingPageTables ((EFI_PHYSICAL_ADDRESS) (UINTN) BaseOfStack, STACK_SIZE);
+ PageTables = CreateIdentityMappingPageTables ((EFI_PHYSICAL_ADDRESS) (UINTN) BaseOfStack, STACK_SIZE,
+ (EFI_PHYSICAL_ADDRESS) (UINTN) GhcbBase, GhcbSize);
} else {
//
// Set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 516cf908bc88..6831946c54d3 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -181,6 +181,8 @@ EnableExecuteDisableBit (
@param Size Size of the given physical memory.
@param StackBase Base address of stack.
@param StackSize Size of stack.
+ @param GhcbBase Base address of GHCB pages.
+ @param GhcbSize Size of GHCB area.

@retval TRUE Page table should be split.
@retval FALSE Page table should not be split.
@@ -190,7 +192,9 @@ ToSplitPageTable (
IN EFI_PHYSICAL_ADDRESS Address,
IN UINTN Size,
IN EFI_PHYSICAL_ADDRESS StackBase,
- IN UINTN StackSize
+ IN UINTN StackSize,
+ IN EFI_PHYSICAL_ADDRESS GhcbBase,
+ IN UINTN GhcbSize
)
{
if (IsNullDetectionEnabled () && Address == 0) {
@@ -209,6 +213,12 @@ ToSplitPageTable (
}
}

+ if (GhcbBase != 0) {
+ if ((Address < GhcbBase + GhcbSize) && ((Address + Size) > GhcbBase)) {
+ return TRUE;
+ }
+ }
+
return FALSE;
}
/**
@@ -322,6 +332,8 @@ AllocatePageTableMemory (
@param[in, out] PageEntry2M Pointer to 2M page entry.
@param[in] StackBase Stack base address.
@param[in] StackSize Stack size.
+ @param[in] GhcbBase GHCB page area base address.
+ @param[in] GhcbSize GHCB page area size.

**/
VOID
@@ -329,7 +341,9 @@ Split2MPageTo4K (
IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
IN OUT UINT64 *PageEntry2M,
IN EFI_PHYSICAL_ADDRESS StackBase,
- IN UINTN StackSize
+ IN UINTN StackSize,
+ IN EFI_PHYSICAL_ADDRESS GhcbBase,
+ IN UINTN GhcbSize
)
{
EFI_PHYSICAL_ADDRESS PhysicalAddress4K;
@@ -355,7 +369,20 @@ Split2MPageTo4K (
//
// Fill in the Page Table entries
//
- PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask;
+ PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K;
+
+ //
+ // The GHCB range consists of two pages per CPU, the GHCB and a
+ // per-CPU variable page. The GHCB page needs to be mapped as an
+ // unencrypted page while the per-CPU variable page needs to be
+ // mapped encrypted. These pages alternate in assignment.
+ //
+ if ((GhcbBase == 0)
+ || (PhysicalAddress4K < GhcbBase)
+ || (PhysicalAddress4K >= GhcbBase + GhcbSize)
+ || (((PhysicalAddress4K - GhcbBase) & SIZE_4KB) != 0)) {
+ PageTableEntry->Uint64 |= AddressEncMask;
+ }
PageTableEntry->Bits.ReadWrite = 1;

if ((IsNullDetectionEnabled () && PhysicalAddress4K == 0) ||
@@ -383,6 +410,8 @@ Split2MPageTo4K (
@param[in, out] PageEntry1G Pointer to 1G page entry.
@param[in] StackBase Stack base address.
@param[in] StackSize Stack size.
+ @param[in] GhcbBase GHCB page area base address.
+ @param[in] GhcbSize GHCB page area size.

**/
VOID
@@ -390,7 +419,9 @@ Split1GPageTo2M (
IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
IN OUT UINT64 *PageEntry1G,
IN EFI_PHYSICAL_ADDRESS StackBase,
- IN UINTN StackSize
+ IN UINTN StackSize,
+ IN EFI_PHYSICAL_ADDRESS GhcbBase,
+ IN UINTN GhcbSize
)
{
EFI_PHYSICAL_ADDRESS PhysicalAddress2M;
@@ -413,11 +444,11 @@ Split1GPageTo2M (

PhysicalAddress2M = PhysicalAddress;
for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) {
- if (ToSplitPageTable (PhysicalAddress2M, SIZE_2MB, StackBase, StackSize)) {
+ if (ToSplitPageTable (PhysicalAddress2M, SIZE_2MB, StackBase, StackSize, GhcbBase, GhcbSize)) {
//
// Need to split this 2M page that covers NULL or stack range.
//
- Split2MPageTo4K (PhysicalAddress2M, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
+ Split2MPageTo4K (PhysicalAddress2M, (UINT64 *) PageDirectoryEntry, StackBase, StackSize, GhcbBase, GhcbSize);
} else {
//
// Fill in the Page Directory entries
@@ -616,6 +647,8 @@ EnablePageTableProtection (

@param[in] StackBase Stack base address.
@param[in] StackSize Stack size.
+ @param[in] GhcbBase GHCB base address.
+ @param[in] GhcbSize GHCB size.

@return The address of 4 level page map.

@@ -623,7 +656,9 @@ EnablePageTableProtection (
UINTN
CreateIdentityMappingPageTables (
IN EFI_PHYSICAL_ADDRESS StackBase,
- IN UINTN StackSize
+ IN UINTN StackSize,
+ IN EFI_PHYSICAL_ADDRESS GhcbBase,
+ IN UINTN GhcbSize
)
{
UINT32 RegEax;
@@ -809,8 +844,8 @@ CreateIdentityMappingPageTables (
PageDirectory1GEntry = (VOID *) PageDirectoryPointerEntry;

for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectory1GEntry++, PageAddress += SIZE_1GB) {
- if (ToSplitPageTable (PageAddress, SIZE_1GB, StackBase, StackSize)) {
- Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry, StackBase, StackSize);
+ if (ToSplitPageTable (PageAddress, SIZE_1GB, StackBase, StackSize, GhcbBase, GhcbSize)) {
+ Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry, StackBase, StackSize, GhcbBase, GhcbSize);
} else {
//
// Fill in the Page Directory entries
@@ -840,11 +875,11 @@ CreateIdentityMappingPageTables (
PageDirectoryPointerEntry->Bits.Present = 1;

for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress += SIZE_2MB) {
- if (ToSplitPageTable (PageAddress, SIZE_2MB, StackBase, StackSize)) {
+ if (ToSplitPageTable (PageAddress, SIZE_2MB, StackBase, StackSize, GhcbBase, GhcbSize)) {
//
// Need to split this 2M page that covers NULL or stack range.
//
- Split2MPageTo4K (PageAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
+ Split2MPageTo4K (PageAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize, GhcbBase, GhcbSize);
} else {
//
// Fill in the Page Directory entries
--
2.27.0


[PATCH v12 04/46] MdePkg: Add a structure definition for the GHCB

Lendacky, Thomas
 

From: Tom Lendacky <thomas.lendacky@...>

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

The GHCB is used by an SEV-ES guest for communicating between the guest
and the hypervisor. Create the GHCB definition as defined by the GHCB
protocol definition.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>
Reviewed-by: Liming Gao <liming.gao@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
---
MdePkg/Include/Register/Amd/Ghcb.h | 166 +++++++++++++++++++++++++++++
1 file changed, 166 insertions(+)
create mode 100644 MdePkg/Include/Register/Amd/Ghcb.h

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
new file mode 100644
index 000000000000..54a80da0f6d7
--- /dev/null
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -0,0 +1,166 @@
+/** @file
+ Guest-Hypervisor Communication Block (GHCB) Definition.
+
+ Provides data types allowing an SEV-ES guest to interact with the hypervisor
+ using the GHCB protocol.
+
+ Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ SEV-ES Guest-Hypervisor Communication Block Standardization
+
+**/
+
+#ifndef __GHCB_H__
+#define __GHCB_H__
+
+#include <Base.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+
+#define UD_EXCEPTION 6
+#define GP_EXCEPTION 13
+#define VC_EXCEPTION 29
+
+#define GHCB_VERSION_MIN 1
+#define GHCB_VERSION_MAX 1
+
+#define GHCB_STANDARD_USAGE 0
+
+//
+// SVM Exit Codes
+//
+#define SVM_EXIT_DR7_READ 0x27ULL
+#define SVM_EXIT_DR7_WRITE 0x37ULL
+#define SVM_EXIT_RDTSC 0x6EULL
+#define SVM_EXIT_RDPMC 0x6FULL
+#define SVM_EXIT_CPUID 0x72ULL
+#define SVM_EXIT_INVD 0x76ULL
+#define SVM_EXIT_IOIO_PROT 0x7BULL
+#define SVM_EXIT_MSR 0x7CULL
+#define SVM_EXIT_VMMCALL 0x81ULL
+#define SVM_EXIT_RDTSCP 0x87ULL
+#define SVM_EXIT_WBINVD 0x89ULL
+#define SVM_EXIT_MONITOR 0x8AULL
+#define SVM_EXIT_MWAIT 0x8BULL
+#define SVM_EXIT_NPF 0x400ULL
+
+//
+// VMG Special Exit Codes
+//
+#define SVM_EXIT_MMIO_READ 0x80000001ULL
+#define SVM_EXIT_MMIO_WRITE 0x80000002ULL
+#define SVM_EXIT_NMI_COMPLETE 0x80000003ULL
+#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
+#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
+#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL
+
+//
+// IOIO Exit Information
+//
+#define IOIO_TYPE_STR BIT2
+#define IOIO_TYPE_IN 1
+#define IOIO_TYPE_INS (IOIO_TYPE_IN | IOIO_TYPE_STR)
+#define IOIO_TYPE_OUT 0
+#define IOIO_TYPE_OUTS (IOIO_TYPE_OUT | IOIO_TYPE_STR)
+
+#define IOIO_REP BIT3
+
+#define IOIO_ADDR_64 BIT9
+#define IOIO_ADDR_32 BIT8
+#define IOIO_ADDR_16 BIT7
+
+#define IOIO_DATA_32 BIT6
+#define IOIO_DATA_16 BIT5
+#define IOIO_DATA_8 BIT4
+#define IOIO_DATA_MASK (BIT6 | BIT5 | BIT4)
+#define IOIO_DATA_OFFSET 4
+#define IOIO_DATA_BYTES(x) (((x) & IOIO_DATA_MASK) >> IOIO_DATA_OFFSET)
+
+#define IOIO_SEG_ES 0
+#define IOIO_SEG_DS (BIT11 | BIT10)
+
+
+typedef enum {
+ GhcbCpl = 25,
+ GhcbRflags = 46,
+ GhcbRip,
+ GhcbRsp = 59,
+ GhcbRax = 63,
+ GhcbRcx = 97,
+ GhcbRdx,
+ GhcbRbx,
+ GhcbRbp = 101,
+ GhcbRsi,
+ GhcbRdi,
+ GhcbR8,
+ GhcbR9,
+ GhcbR10,
+ GhcbR11,
+ GhcbR12,
+ GhcbR13,
+ GhcbR14,
+ GhcbR15,
+ GhcbXCr0 = 125,
+} GHCB_REGISTER;
+
+typedef PACKED struct {
+ UINT8 Reserved1[203];
+ UINT8 Cpl;
+ UINT8 Reserved2[148];
+ UINT64 Dr7;
+ UINT8 Reserved3[144];
+ UINT64 Rax;
+ UINT8 Reserved4[264];
+ UINT64 Rcx;
+ UINT64 Rdx;
+ UINT64 Rbx;
+ UINT8 Reserved5[112];
+ UINT64 SwExitCode;
+ UINT64 SwExitInfo1;
+ UINT64 SwExitInfo2;
+ UINT64 SwScratch;
+ UINT8 Reserved6[56];
+ UINT64 XCr0;
+ UINT8 ValidBitmap[16];
+ UINT64 X87StateGpa;
+ UINT8 Reserved7[1016];
+} GHCB_SAVE_AREA;
+
+typedef PACKED struct {
+ GHCB_SAVE_AREA SaveArea;
+ UINT8 SharedBuffer[2032];
+ UINT8 Reserved1[10];
+ UINT16 ProtocolVersion;
+ UINT32 GhcbUsage;
+} GHCB;
+
+typedef union {
+ struct {
+ UINT32 Lower32Bits;
+ UINT32 Upper32Bits;
+ } Elements;
+
+ UINT64 Uint64;
+} GHCB_EXIT_INFO;
+
+typedef union {
+ struct {
+ UINT32 Vector:8;
+ UINT32 Type:3;
+ UINT32 ErrorCodeValid:1;
+ UINT32 Rsvd:19;
+ UINT32 Valid:1;
+ UINT32 ErrorCode;
+ } Elements;
+
+ UINT64 Uint64;
+} GHCB_EVENT_INJECTION;
+
+#define GHCB_EVENT_INJECTION_TYPE_INT 0
+#define GHCB_EVENT_INJECTION_TYPE_NMI 2
+#define GHCB_EVENT_INJECTION_TYPE_EXCEPTION 3
+#define GHCB_EVENT_INJECTION_TYPE_SOFT_INT 4
+
+#endif
--
2.27.0


[PATCH v12 03/46] MdePkg: Add the MSR definition for the GHCB register

Lendacky, Thomas
 

From: Tom Lendacky <thomas.lendacky@...>

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

For SEV-ES, the GHCB page address is stored in the GHCB MSR register
(0xc0010130). Define the register and the format used for register
during GHCB protocol negotiation.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>
Reviewed-by: Liming Gao <liming.gao@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 46 ++++++++++++++++++++++++++
1 file changed, 46 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index 6ef45a9b21d3..e4db09c5184c 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -17,6 +17,52 @@
#ifndef __FAM17_MSR_H__
#define __FAM17_MSR_H__

+/**
+ Secure Encrypted Virtualization - Encrypted State (SEV-ES) GHCB register
+
+**/
+#define MSR_SEV_ES_GHCB 0xc0010130
+
+/**
+ MSR information returned for #MSR_SEV_ES_GHCB
+**/
+typedef union {
+ struct {
+ UINT32 Function:12;
+ UINT32 Reserved1:20;
+ UINT32 Reserved2:32;
+ } GhcbInfo;
+
+ struct {
+ UINT8 Reserved[3];
+ UINT8 SevEncryptionBitPos;
+ UINT16 SevEsProtocolMin;
+ UINT16 SevEsProtocolMax;
+ } GhcbProtocol;
+
+ struct {
+ UINT32 Function:12;
+ UINT32 ReasonCodeSet:4;
+ UINT32 ReasonCode:8;
+ UINT32 Reserved1:8;
+ UINT32 Reserved2:32;
+ } GhcbTerminate;
+
+ VOID *Ghcb;
+
+ UINT64 GhcbPhysicalAddress;
+} MSR_SEV_ES_GHCB_REGISTER;
+
+#define GHCB_INFO_SEV_INFO 1
+#define GHCB_INFO_SEV_INFO_GET 2
+#define GHCB_INFO_CPUID_REQUEST 4
+#define GHCB_INFO_CPUID_RESPONSE 5
+#define GHCB_INFO_TERMINATE_REQUEST 256
+
+#define GHCB_TERMINATE_GHCB 0
+#define GHCB_TERMINATE_GHCB_GENERAL 0
+#define GHCB_TERMINATE_GHCB_PROTOCOL 1
+
/**
Secure Encrypted Virtualization (SEV) status register

--
2.27.0


[PATCH v12 02/46] UefiCpuPkg: Create PCD to be used in support of SEV-ES

Lendacky, Thomas
 

From: Tom Lendacky <thomas.lendacky@...>

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

A new dynamic UefiCpuPkg PCD is needed to support SEV-ES under OVMF:
- PcdSevEsIsEnabled: BOOLEAN value used to indicate if SEV-ES is enabled

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Reviewed-by: Eric Dong <eric.dong@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
---
UefiCpuPkg/UefiCpuPkg.dec | 6 ++++++
UefiCpuPkg/UefiCpuPkg.uni | 3 +++
2 files changed, 9 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 762badf5d239..df5d02bae6b4 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -370,5 +370,11 @@ [PcdsDynamic, PcdsDynamicEx]
# @ValidRange 0x80000001 | 0 - 1
gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x0|UINT8|0x60000015

+ ## This dynamic PCD indicates whether SEV-ES is enabled
+ # TRUE - SEV-ES is enabled
+ # FALSE - SEV-ES is not enabled
+ # @Prompt SEV-ES Status
+ gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled|FALSE|BOOLEAN|0x60000016
+
[UserExtensions.TianoCore."ExtraFiles"]
UefiCpuPkgExtra.uni
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 1780dfdc126d..f4a0c72f6293 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -278,3 +278,6 @@

#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApStatusCheckIntervalInMicroSeconds_PROMPT #language en-US "Periodic interval value in microseconds for AP status check in DXE.\n"
#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApStatusCheckIntervalInMicroSeconds_HELP #language en-US "Periodic interval value in microseconds for the status check of APs for StartupAllAPs() and StartupThisAP() executed in non-blocking mode in DXE phase.\n"
+
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSevEsIsEnabled_PROMPT #language en-US "Specifies whether SEV-ES is enabled"
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSevEsIsEnabled_HELP #language en-US "Set to TRUE when running as an SEV-ES guest, FALSE otherwise."
--
2.27.0


[PATCH v12 01/46] MdeModulePkg: Create PCDs to be used in support of SEV-ES

Lendacky, Thomas
 

From: Tom Lendacky <thomas.lendacky@...>

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

Two new dynamic MdeModulePkg PCDs are needed to support SEV-ES under OVMF:
- PcdGhcbBase: UINT64 value that is the base address of the GHCB
allocation.
- PcdGhcbSize: UINT64 value that is the size, in bytes, of the
GHCB allocation (size is dependent on the number of
APs).

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
---
MdeModulePkg/MdeModulePkg.dec | 9 +++++++++
MdeModulePkg/MdeModulePkg.uni | 8 ++++++++
2 files changed, 17 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 843e963ad34b..f8cd9239b4ce 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2051,6 +2051,15 @@ [PcdsDynamic, PcdsDynamicEx]
# @Prompt If there is any test key used by the platform.
gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed|FALSE|BOOLEAN|0x00030003

+ ## This dynamic PCD holds the base address of the GHCB pool allocation.
+ # @Prompt GHCB Pool Base Address
+ gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase|0|UINT64|0x00030007
+
+ ## This dynamic PCD holds the total size of the GHCB pool allocation.
+ # The amount of memory allocated for GHCBs is dependent on the number of APs.
+ # @Prompt GHCB Pool Size
+ gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
+
[PcdsDynamicEx]
## This dynamic PCD enables the default variable setting.
# Its value is the default store ID value. The default value is zero as Standard default.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 2007e0596c4f..2f8cca03e527 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1297,3 +1297,11 @@
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdTcgPfpMeasurementRevision_PROMPT #language en-US "TCG Platform Firmware Profile revision"

#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdTcgPfpMeasurementRevision_HELP #language en-US "Indicates which TCG Platform Firmware Profile revision the EDKII firmware follows."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_PROMPT #language en-US "GHCB Pool Base Address"
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify an address range that is not to be encrypted."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "GHCB Pool Base Size"
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the size of the address range that is not to be encrypted."
--
2.27.0


[PATCH v12 00/46] SEV-ES guest support

Lendacky, Thomas
 

From: Tom Lendacky <thomas.lendacky@...>

This patch series provides support for running EDK2/OVMF under SEV-ES.

Secure Encrypted Virtualization - Encrypted State (SEV-ES) expands on the
SEV support to protect the guest register state from the hypervisor. See
"AMD64 Architecture Programmer's Manual Volume 2: System Programming",
section "15.35 Encrypted State (SEV-ES)" [1].

In order to allow a hypervisor to perform functions on behalf of a guest,
there is architectural support for notifying a guest's operating system
when certain types of VMEXITs are about to occur. This allows the guest to
selectively share information with the hypervisor to satisfy the requested
function. The notification is performed using a new exception, the VMM
Communication exception (#VC). The information is shared through the
Guest-Hypervisor Communication Block (GHCB) using the VMGEXIT instruction.
The GHCB format and the protocol for using it is documented in "SEV-ES
Guest-Hypervisor Communication Block Standardization" [2].

The main areas of the EDK2 code that are updated to support SEV-ES are
around the exception handling support and the AP boot support.

Exception support is required starting in Sec, continuing through Pei
and into Dxe in order to handle #VC exceptions that are generated. Each
AP requires it's own GHCB page as well as a page to hold values specific
to that AP.

AP booting poses some interesting challenges. The INIT-SIPI-SIPI sequence
is typically used to boot the APs. However, the hypervisor is not allowed
to update the guest registers. The GHCB document [2] talks about how SMP
booting under SEV-ES is performed.

Since the GHCB page must be a shared (unencrypted) page, the processor
must be running in long mode in order for the guest and hypervisor to
communicate with each other. As a result, SEV-ES is only supported under
the X64 architecture.

[1] https://www.amd.com/system/files/TechDocs/24593.pdf
[2] https://developer.amd.com/wp-content/resources/56421.pdf

---

These patches are based on commit:
6074f57e5b19 ("MdePkg/Include/IndustryStandard: Main CXL header")

A version of the tree can be found at:
https://github.com/AMDESE/ovmf/tree/sev-es-v20

Cc: Andrew Fish <afish@...>
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Benjamin You <benjamin.you@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Cc: Guo Dong <guo.dong@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Leif Lindholm <leif@...>
Cc: Liming Gao <liming.gao@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Ray Ni <ray.ni@...>

Changes since v11:
- Make the XGETBV and VMGEXIT .nasm files buildable for all environments
and remove the updates that add these instructions to GccInline.c

Changes since v10:
- Fix conflicts around GccInline.c file after moving to latest commit
- Fix conflicts with OVMF PCD values after moving to latest commit

Changes since v9:
- Fixed bit field declarations in the GHCB structure to use UINT32
and not UINT64.
- Fixed a warning produced by VS2019 in the instruction parsing code
by expliciting casting a bit shift to an INT64.
- Sorted section entries in the OVMF VmgExitLib INF file.
- Moved the new Maintainers.txt entry so entries remain sorted.
- Documentation style fixes for return values.
- Miscellaneous code style fixes.

Changes since v8:
- Move IOIO exit info definitions into Ghcb.h file
- Add a macro for calculating IO instruction bytes (IOIO_DATA_BYTES)
- Exception handler support for debug registers
- Moved the DRx register saving changes into the UefiCpuPkg patch for
base #VC support in CpuExceptionHandlerLib.
- OvmfPkg VmgExitLib
- Remove the .uni file
- Update .inf file:
- New file location for VmgExitVcHandler.c
- Add additional Packages and LibraryClasses
- Introduce a header file to hold the #VC instruction parsing related
definitions
- Include additional #defines for instruction decoding to replace
hard coded values for things like instruction prefixes and escapes.
- Replace hardcoded CPUID values with values from existing header files
and use existing CR4 definition for accessing CR4 data.
- Change the type used for obtaining data addresses in the instruction
parsing
- Switch from INTN to UINT64 and use compiler conversions and casting
to perform the correct address calculation
- ResetVector code:
- Revert some inadvertant changes introduced in v7 for reserving the
SEV-ES work area memory and for checking the status of SEV-ES.
- AP Booting
- Provide support for non-broadcast INIT-SIPI-SIPI AP boot (minimize
code duplication by creating a function to set the AP jump table
vector address).
- Fix file/directory entry in maintainer changes.
- Various coding style fixes
- Commenting, if statements, etc.
- Various documentation style fixes

Changes since v7:
- Reserve the SEV-ES workarea when S3 is enabled
- Fix warnings issued by the Visual Studio compiler
- Create a NULL VmgExitLib instance that is used for VMGEXIT
related operations as well as #VC handling. Then create the full
VmgExitLib support only in OvmfPkg - where it will be used. This
removes a bunch of implementation code from platforms that will
not be using the functionality.
- Remove single use interfaces from the VmgExitLib (VmgMmioWrite
and VmgSetApJumpTable)

Changes since v6:
- Add function comments to all functions, including local functions
- Add function parameter direction to all functions (in/out)
- Add support for MMIO MOVZX/MOVSX instructions
- Ensure the per-CPU variable page remains encrypted
- Coding-style fixes as identified by Ecc

Changes since v5:
- Remove extraneous VmgExitLib usage
- Miscellaneous changes to address feedback (coding style, etc.)

Changes since v4:
- Move the SEV-ES protocol negotiation out of the SEC exception handler
and into the SecMain.c file. As a result:
- Move the SecGhcb related PCDs out of UefiCpuPkg and into OvmfPkg
- Combine SecAMDSevVcHandler.c and PeiDxeAMDSevVcHandler.c into a
single AMDSevVcHandler.c
- Consolidate VmgExitLib usage into common LibraryClasses sections
- Add documentation comments to the VmgExitLib functions

Changes since v3:
- Remove the need for the MP library finalization routine. The AP
jump table address will be held by the hypervisor rather than
communicated via the GHCB MSR. This removes some fragility around
the UEFI to OS transition.
- Rename the SEV-ES RIP reset area to SEV-ES workarea and use it to
communicate the SEV-ES status, so that SEC CPU exception handling is
only established for an SEV-ES guest.
- Fix SMM build breakageAdd around QemuFlashPtrWrite().
- Fix SMM build breakage by adding VC exception support the SMM CPU
exception handling.
- Add memory fencing around the invocation of AsmVmgExit().
- Clarify comments around the SEV-ES AP reset RIP values and usage.
- Move some PCD definitions from MdeModulePkg to UefiCpuPkg.
- Remove the 16-bit code selector definition from MdeModulePkg

Changes since v2:
- Added a way to locate the SEV-ES fixed AP RIP address for starting
AP's to avoid updating the actual flash image (build time location
that is identified with a GUID value).
- Create a VmgExit library to replace static inline functions.
- Move some PCDs to the appropriate packages
- Add support for writing to QEMU flash under SEV-ES
- Add additional MMIO opcode support
- Cleaned up the GHCB MSR CPUID protocol support

Changes since v1:
- Patches reworked to be more specific to the component/area being updated
and order of definition/usage
- Created a library for VMGEXIT-related functions to replace use of inline
functions
- Allocation method for GDT changed from AllocatePool to AllocatePages
- Early caching only enabled for SEV-ES guests
- Ensure AP loop mode set to halt loop mode for SEV-ES guests
- Reserved SEC GHCB-related memory areas when S3 is enabled

Tom Lendacky (46):
MdeModulePkg: Create PCDs to be used in support of SEV-ES
UefiCpuPkg: Create PCD to be used in support of SEV-ES
MdePkg: Add the MSR definition for the GHCB register
MdePkg: Add a structure definition for the GHCB
MdeModulePkg/DxeIplPeim: Support GHCB pages when creating page tables
MdePkg/BaseLib: Add support for the XGETBV instruction
MdePkg/BaseLib: Add support for the VMGEXIT instruction
UefiCpuPkg: Implement library support for VMGEXIT
OvmfPkg: Prepare OvmfPkg to use the VmgExitLib library
UefiPayloadPkg: Prepare UefiPayloadPkg to use the VmgExitLib library
UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception
OvmfPkg/VmgExitLib: Implement library support for VmgExitLib in OVMF
OvmfPkg/VmgExitLib: Add support for IOIO_PROT NAE events
OvmfPkg/VmgExitLib: Support string IO for IOIO_PROT NAE events
OvmfPkg/VmgExitLib: Add support for CPUID NAE events
OvmfPkg/VmgExitLib: Add support for MSR_PROT NAE events
OvmfPkg/VmgExitLib: Add support for NPF NAE events (MMIO)
OvmfPkg/VmgExitLib: Add support for WBINVD NAE events
OvmfPkg/VmgExitLib: Add support for RDTSC NAE events
OvmfPkg/VmgExitLib: Add support for RDPMC NAE events
OvmfPkg/VmgExitLib: Add support for INVD NAE events
OvmfPkg/VmgExitLib: Add support for VMMCALL NAE events
OvmfPkg/VmgExitLib: Add support for RDTSCP NAE events
OvmfPkg/VmgExitLib: Add support for MONITOR/MONITORX NAE events
OvmfPkg/VmgExitLib: Add support for MWAIT/MWAITX NAE events
OvmfPkg/VmgExitLib: Add support for DR7 Read/Write NAE events
OvmfPkg/MemEncryptSevLib: Add an SEV-ES guest indicator function
OvmfPkg: Add support to perform SEV-ES initialization
OvmfPkg: Create a GHCB page for use during Sec phase
OvmfPkg/PlatformPei: Reserve GHCB-related areas if S3 is supported
OvmfPkg: Create GHCB pages for use during Pei and Dxe phase
OvmfPkg/PlatformPei: Move early GDT into ram when SEV-ES is enabled
UefiCpuPkg: Create an SEV-ES workarea PCD
OvmfPkg: Reserve a page in memory for the SEV-ES usage
OvmfPkg/PlatformPei: Reserve SEV-ES work area if S3 is supported
OvmfPkg/ResetVector: Add support for a 32-bit SEV check
OvmfPkg/Sec: Add #VC exception handling for Sec phase
OvmfPkg/Sec: Enable cache early to speed up booting
OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Bypass flash detection with
SEV-ES
UefiCpuPkg: Add a 16-bit protected mode code segment descriptor
UefiCpuPkg/MpInitLib: Add CPU MP data flag to indicate if SEV-ES is
enabled
UefiCpuPkg: Allow AP booting under SEV-ES
OvmfPkg: Use the SEV-ES work area for the SEV-ES AP reset vector
OvmfPkg: Move the GHCB allocations into reserved memory
UefiCpuPkg/MpInitLib: Prepare SEV-ES guest APs for OS use
Maintainers.txt: Add reviewers for the OvmfPkg SEV-related files

MdeModulePkg/MdeModulePkg.dec | 9 +
OvmfPkg/OvmfPkg.dec | 9 +
UefiCpuPkg/UefiCpuPkg.dec | 17 +
OvmfPkg/OvmfPkgIa32.dsc | 6 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +
OvmfPkg/OvmfPkgX64.dsc | 6 +
OvmfPkg/OvmfXen.dsc | 1 +
UefiCpuPkg/UefiCpuPkg.dsc | 2 +
UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 2 +
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 2 +
OvmfPkg/OvmfPkgX64.fdf | 9 +
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +
MdePkg/Library/BaseLib/BaseLib.inf | 4 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 36 +
OvmfPkg/PlatformPei/PlatformPei.inf | 9 +
.../FvbServicesRuntimeDxe.inf | 2 +
OvmfPkg/ResetVector/ResetVector.inf | 8 +
OvmfPkg/Sec/SecMain.inf | 4 +
.../DxeCpuExceptionHandlerLib.inf | 1 +
.../PeiCpuExceptionHandlerLib.inf | 1 +
.../SecPeiCpuExceptionHandlerLib.inf | 1 +
.../SmmCpuExceptionHandlerLib.inf | 1 +
.../Xcode5SecPeiCpuExceptionHandlerLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
.../Library/VmgExitLibNull/VmgExitLibNull.inf | 27 +
.../Core/DxeIplPeim/X64/VirtualMemory.h | 12 +-
MdePkg/Include/Library/BaseLib.h | 31 +
MdePkg/Include/Register/Amd/Fam17Msr.h | 46 +
MdePkg/Include/Register/Amd/Ghcb.h | 166 ++
.../IndustryStandard/InstructionParsing.h | 83 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 12 +
.../QemuFlash.h | 13 +
UefiCpuPkg/CpuDxe/CpuGdt.h | 4 +-
UefiCpuPkg/Include/Library/VmgExitLib.h | 103 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 68 +-
.../Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 +-
.../Core/DxeIplPeim/X64/DxeLoadFunc.c | 11 +-
.../Core/DxeIplPeim/X64/VirtualMemory.c | 57 +-
.../MemEncryptSevLibInternal.c | 75 +-
OvmfPkg/Library/VmgExitLib/VmgExitLib.c | 159 ++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 1716 +++++++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 89 +
OvmfPkg/PlatformPei/MemDetect.c | 43 +
.../QemuFlash.c | 23 +-
.../QemuFlashDxe.c | 40 +
.../QemuFlashSmm.c | 16 +
OvmfPkg/Sec/SecMain.c | 188 +-
UefiCpuPkg/CpuDxe/CpuGdt.c | 8 +-
.../CpuExceptionCommon.c | 10 +-
.../PeiDxeSmmCpuException.c | 20 +-
.../SecPeiCpuException.c | 19 +
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 120 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 337 +++-
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 19 +
.../Library/VmgExitLibNull/VmgExitLibNull.c | 121 ++
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 2 +-
Maintainers.txt | 10 +
MdeModulePkg/MdeModulePkg.uni | 8 +
MdePkg/Library/BaseLib/Ia32/VmgExit.nasm | 37 +
MdePkg/Library/BaseLib/Ia32/XGetBv.nasm | 31 +
MdePkg/Library/BaseLib/X64/VmgExit.nasm | 32 +
MdePkg/Library/BaseLib/X64/XGetBv.nasm | 34 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 100 +
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 351 +++-
OvmfPkg/ResetVector/ResetVector.nasmb | 20 +
.../X64/ExceptionHandlerAsm.nasm | 17 +
.../X64/Xcode5ExceptionHandlerAsm.nasm | 17 +
UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 2 +-
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 15 +
UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 4 +-
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 370 +++-
.../Library/VmgExitLibNull/VmgExitLibNull.uni | 15 +
.../ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 9 +
UefiCpuPkg/UefiCpuPkg.uni | 11 +
75 files changed, 4772 insertions(+), 100 deletions(-)
create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
create mode 100644 UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
create mode 100644 MdePkg/Include/Register/Amd/Ghcb.h
create mode 100644 OvmfPkg/Include/IndustryStandard/InstructionParsing.h
create mode 100644 UefiCpuPkg/Include/Library/VmgExitLib.h
create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitLib.c
create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
create mode 100644 UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c
create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExit.nasm
create mode 100644 MdePkg/Library/BaseLib/Ia32/XGetBv.nasm
create mode 100644 MdePkg/Library/BaseLib/X64/VmgExit.nasm
create mode 100644 MdePkg/Library/BaseLib/X64/XGetBv.nasm
create mode 100644 OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
create mode 100644 UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.uni

--
2.27.0


Re: [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers

Liming Gao
 

Lefi:
Thanks for your comments.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
Sent: Monday, July 27, 2020 10:25 PM
To: devel@edk2.groups.io; Javeed, Ashraf <ashraf.javeed@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Gao, Liming <liming.gao@...>
Subject: Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers

Hi Ashraf, but also Mike, Liming.

I just saw this patch go in and have some comments.

On Fri, Jul 24, 2020 at 23:56:12 +0530, Javeed, Ashraf wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2611

Register definitions from chapter 7 of Compute Express Link
Specification Revision 1.1 are ported into the new Cxl11.h.
The CXL Flex Bus registers are based on the PCIe Extended Capability
DVSEC structure header, led to the inclusion of upgraded Pci.h.

Signed-off-by: Ashraf Javeed <ashraf.javeed@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>
--

V4: fix code style

V3: Copyright date fix

V2: Indentation and double declaration fix, copyright date update
---
MdePkg/Include/IndustryStandard/Cxl11.h | 569
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++++
MdePkg/Include/IndustryStandard/Pci.h | 6 ++----
2 files changed, 571 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h b/MdePkg/Include/IndustryStandard/Cxl11.h
new file mode 100644
index 0000000000..933c1ab817
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Cxl11.h
@@ -0,0 +1,569 @@
+/** @file
+ CXL 1.1 Register definitions
+
+ This file contains the register definitions based on the Compute Express Link
+ (CXL) Specification Revision 1.1.
+
+Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _CXL11_H_
+#define _CXL11_H_
We should not be adding macros with a leading _ - these are intended
for toolchain use.
This style is align to other header file. This is the file header macro to make sure the header file be included more than once.


+
+#include <IndustryStandard/Pci.h>
+//
+// DVSEC Vendor ID
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1 - Table 58
+// (subject to change as per CXL assigned Vendor ID)
+//
+#define INTEL_CXL_DVSEC_VENDOR_ID 0x8086
This is Cxl11.h - surely this should be CXL_DVSEC_VENDOR_ID_INTEL?
Ashraf: is it defined in spec?


+
+//
+// CXL Flex Bus Device default device and function number
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1
+//
+#define CXL_DEV_DEV 0
+#define CXL_DEV_FUNC 0
+
+//
+// Ensure proper structure formats
+//
+#pragma pack(1)
And this pragma has no function whatsoever with regards to any of the
register definition structs below. It would be much better if the
structs requiring packing (_DEVICE, _PORT, ...) were grouped together
and only those were given this treatment.

#pragma pack(1) is *not* a safe default.
I know pack(1) is for the compact structure layout.

+
+///
+/// The PCIe DVSEC for Flex Bus Device
+///@{
+typedef union {
+ struct {
+ UINT16 CacheCapable : 1; // bit 0
+ UINT16 IoCapable : 1; // bit 1
+ UINT16 MemCapable : 1; // bit 2
+ UINT16 MemHwInitMode : 1; // bit 3
+ UINT16 HdmCount : 2; // bit 4..5
+ UINT16 Reserved1 : 8; // bit 6..13
+ UINT16 ViralCapable : 1; // bit 14
+ UINT16 Reserved2 : 1; // bit 15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_CAPABILITY;
+
+typedef union {
+ struct {
+ UINT16 CacheEnable : 1; // bit 0
+ UINT16 IoEnable : 1; // bit 1
+ UINT16 MemEnable : 1; // bit 2
+ UINT16 CacheSfCoverage : 5; // bit 3..7
+ UINT16 CacheSfGranularity : 3; // bit 8..10
+ UINT16 CacheCleanEviction : 1; // bit 11
+ UINT16 Reserved1 : 2; // bit 12..13
+ UINT16 ViralEnable : 1; // bit 14
+ UINT16 Reserved2 : 1; // bit 15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_CONTROL;
+
+typedef union {
+ struct {
+ UINT16 Reserved1 : 14; // bit 0..13
+ UINT16 ViralStatus : 1; // bit 14
+ UINT16 Reserved2 : 1; // bit 15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_STATUS;
+
+typedef union {
+ struct {
+ UINT16 Reserved1 : 1; // bit 0
+ UINT16 Reserved2 : 1; // bit 1
+ UINT16 Reserved3 : 1; // bit 2
+ UINT16 Reserved4 : 13; // bit 3..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_DEVICE_CONTROL2;
+
+typedef union {
+ struct {
+ UINT16 Reserved1 : 1; // bit 0
+ UINT16 Reserved2 : 1; // bit 1
+ UINT16 Reserved3 : 14; // bit 2..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_DEVICE_STATUS2;
+
+typedef union {
+ struct {
+ UINT16 ConfigLock : 1; // bit 0
+ UINT16 Reserved1 : 15; // bit 1..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_LOCK;
+
+typedef union {
+ struct {
+ UINT32 MemorySizeHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 MemoryInfoValid : 1; // bit 0
+ UINT32 MemoryActive : 1; // bit 1
+ UINT32 MediaType : 3; // bit 2..4
+ UINT32 MemoryClass : 3; // bit 5..7
+ UINT32 DesiredInterleave : 3; // bit 8..10
+ UINT32 Reserved : 17; // bit 11..27
+ UINT32 MemorySizeLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_LOW;
+
+typedef union {
+ struct {
+ UINT32 MemoryBaseHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 Reserved : 28; // bit 0..27
+ UINT32 MemoryBaseLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_LOW;
+
+
+typedef union {
+ struct {
+ UINT32 MemorySizeHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 MemoryInfoValid : 1; // bit 0
+ UINT32 MemoryActive : 1; // bit 1
+ UINT32 MediaType : 3; // bit 2..4
+ UINT32 MemoryClass : 3; // bit 5..7
+ UINT32 DesiredInterleave : 3; // bit 8..10
+ UINT32 Reserved : 17; // bit 11..27
+ UINT32 MemorySizeLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_LOW;
+
+typedef union {
+ struct {
+ UINT32 MemoryBaseHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 Reserved : 28; // bit 0..27
+ UINT32 MemoryBaseLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_LOW;
+
+//
+// Flex Bus Device DVSEC ID
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1, Table 58
+//
+#define FLEX_BUS_DEVICE_DVSEC_ID 0
+
+//
+// PCIe DVSEC for Flex Bus Device
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1, Figure 95
+//
+typedef struct {
+ PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER Header; // offset 0
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_1 DesignatedVendorSpecificHeader1; // offset 4
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_2 DesignatedVendorSpecificHeader2; // offset 8
+ CXL_DVSEC_FLEX_BUS_DEVICE_CAPABILITY DeviceCapability; // offset 10
+ CXL_DVSEC_FLEX_BUS_DEVICE_CONTROL DeviceControl; // offset 12
+ CXL_DVSEC_FLEX_BUS_DEVICE_STATUS DeviceStatus; // offset 14
+ CXL_1_1_DVSEC_FLEX_BUS_DEVICE_CONTROL2 DeviceControl2; // offset 16
+ CXL_1_1_DVSEC_FLEX_BUS_DEVICE_STATUS2 DeviceStatus2; // offset 18
+ CXL_DVSEC_FLEX_BUS_DEVICE_LOCK DeviceLock; // offset 20
+ UINT16 Reserved; // offset 22
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_HIGH DeviceRange1SizeHigh; // offset 24
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_LOW DeviceRange1SizeLow; // offset 28
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_HIGH DeviceRange1BaseHigh; // offset 32
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_LOW DeviceRange1BaseLow; // offset 36
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_HIGH DeviceRange2SizeHigh; // offset 40
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_LOW DeviceRange2SizeLow; // offset 44
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_HIGH DeviceRange2BaseHigh; // offset 48
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_LOW DeviceRange2BaseLow; // offset 52
+} CXL_1_1_DVSEC_FLEX_BUS_DEVICE;
+///@}
+
+///
+/// PCIe DVSEC for FLex Bus Port
+///@{
+typedef union {
+ struct {
+ UINT16 CacheCapable : 1; // bit 0
+ UINT16 IoCapable : 1; // bit 1
+ UINT16 MemCapable : 1; // bit 2
+ UINT16 Reserved : 13; // bit 3..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_PORT_CAPABILITY;
+
+typedef union {
+ struct {
+ UINT16 CacheEnable : 1; // bit 0
+ UINT16 IoEnable : 1; // bit 1
+ UINT16 MemEnable : 1; // bit 2
+ UINT16 CxlSyncBypassEnable : 1; // bit 3
+ UINT16 DriftBufferEnable : 1; // bit 4
+ UINT16 Reserved : 3; // bit 5..7
+ UINT16 Retimer1Present : 1; // bit 8
+ UINT16 Retimer2Present : 1; // bit 9
+ UINT16 Reserved2 : 6; // bit 10..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_PORT_CONTROL;
+
+typedef union {
+ struct {
+ UINT16 CacheEnable : 1; // bit 0
+ UINT16 IoEnable : 1; // bit 1
+ UINT16 MemEnable : 1; // bit 2
+ UINT16 CxlSyncBypassEnable : 1; // bit 3
+ UINT16 DriftBufferEnable : 1; // bit 4
+ UINT16 Reserved : 3; // bit 5..7
+ UINT16 CxlCorrectableProtocolIdFramingError : 1; // bit 8
+ UINT16 CxlUncorrectableProtocolIdFramingError : 1; // bit 9
+ UINT16 CxlUnexpectedProtocolIdDropped : 1; // bit 10
+ UINT16 Reserved2 : 5; // bit 11..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_PORT_STATUS;
+
+//
+// Flex Bus Port DVSEC ID
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.3, Table 62
+//
+#define FLEX_BUS_PORT_DVSEC_ID 7
+
+//
+// PCIe DVSEC for Flex Bus Port
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.3, Figure 99
+//
+typedef struct {
+ PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER Header; // offset 0
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_1 DesignatedVendorSpecificHeader1; // offset 4
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_2 DesignatedVendorSpecificHeader2; // offset 8
+ CXL_1_1_DVSEC_FLEX_BUS_PORT_CAPABILITY PortCapability; // offset 10
+ CXL_1_1_DVSEC_FLEX_BUS_PORT_CONTROL PortControl; // offset 12
+ CXL_1_1_DVSEC_FLEX_BUS_PORT_STATUS PortStatus; // offset 14
+} CXL_1_1_DVSEC_FLEX_BUS_PORT;
+///@}
+
+///
+/// CXL 1.1 Upstream and Downstream Port Subsystem Component registers
+///
+
+/// The CXL.Cache and CXL.Memory Architectural register definitions
+/// Based on chapter 7.2.2 of Compute Express Link Specification Revision: 1.1
+///@{
+
+#define CXL_CAPABILITY_HEADER_OFFSET 0
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlCacheMemVersion : 4; // bit 20..23
+ UINT32 ArraySize : 8; // bit 24..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CAPABILITY_HEADER;
+
+#define CXL_RAS_CAPABILITY_HEADER_OFFSET 4
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlRasCapabilityPointer : 12; // bit 20..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_RAS_CAPABILITY_HEADER;
+
+#define CXL_SECURITY_CAPABILITY_HEADER_OFFSET 8
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlSecurityCapabilityPointer : 12; // bit 20..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_SECURITY_CAPABILITY_HEADER;
+
+#define CXL_LINK_CAPABILITY_HEADER_OFFSET 0xC
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlLinkCapabilityPointer : 12; // bit 20..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_LINK_CAPABILITY_HEADER;
+
+typedef union {
+ struct {
+ UINT32 CacheDataParity : 1; // bit 0..0
+ UINT32 CacheAddressParity : 1; // bit 1..1
+ UINT32 CacheByteEnableParity : 1; // bit 2..2
+ UINT32 CacheDataEcc : 1; // bit 3..3
+ UINT32 MemDataParity : 1; // bit 4..4
+ UINT32 MemAddressParity : 1; // bit 5..5
+ UINT32 MemByteEnableParity : 1; // bit 6..6
+ UINT32 MemDataEcc : 1; // bit 7..7
+ UINT32 ReInitThreshold : 1; // bit 8..8
+ UINT32 RsvdEncodingViolation : 1; // bit 9..9
+ UINT32 PoisonReceived : 1; // bit 10..10
+ UINT32 ReceiverOverflow : 1; // bit 11..11
+ UINT32 Reserved : 20; // bit 12..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_UNCORRECTABLE_ERROR_STATUS;
+
+typedef union {
+ struct {
+ UINT32 CacheDataParityMask : 1; // bit 0..0
+ UINT32 CacheAddressParityMask : 1; // bit 1..1
+ UINT32 CacheByteEnableParityMask : 1; // bit 2..2
+ UINT32 CacheDataEccMask : 1; // bit 3..3
+ UINT32 MemDataParityMask : 1; // bit 4..4
+ UINT32 MemAddressParityMask : 1; // bit 5..5
+ UINT32 MemByteEnableParityMask : 1; // bit 6..6
+ UINT32 MemDataEccMask : 1; // bit 7..7
+ UINT32 ReInitThresholdMask : 1; // bit 8..8
+ UINT32 RsvdEncodingViolationMask : 1; // bit 9..9
+ UINT32 PoisonReceivedMask : 1; // bit 10..10
+ UINT32 ReceiverOverflowMask : 1; // bit 11..11
+ UINT32 Reserved : 20; // bit 12..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_UNCORRECTABLE_ERROR_MASK;
+
+typedef union {
+ struct {
+ UINT32 CacheDataParitySeverity : 1; // bit 0..0
+ UINT32 CacheAddressParitySeverity : 1; // bit 1..1
+ UINT32 CacheByteEnableParitySeverity : 1; // bit 2..2
+ UINT32 CacheDataEccSeverity : 1; // bit 3..3
+ UINT32 MemDataParitySeverity : 1; // bit 4..4
+ UINT32 MemAddressParitySeverity : 1; // bit 5..5
+ UINT32 MemByteEnableParitySeverity : 1; // bit 6..6
+ UINT32 MemDataEccSeverity : 1; // bit 7..7
+ UINT32 ReInitThresholdSeverity : 1; // bit 8..8
+ UINT32 RsvdEncodingViolationSeverity : 1; // bit 9..9
+ UINT32 PoisonReceivedSeverity : 1; // bit 10..10
+ UINT32 ReceiverOverflowSeverity : 1; // bit 11..11
+ UINT32 Reserved : 20; // bit 12..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_UNCORRECTABLE_ERROR_SEVERITY;
+
+typedef union {
+ struct {
+ UINT32 CacheDataEcc : 1; // bit 0..0
+ UINT32 MemoryDataEcc : 1; // bit 1..1
+ UINT32 CrcThreshold : 1; // bit 2..2
+ UINT32 RetryThreshold : 1; // bit 3..3
+ UINT32 CachePoisonReceived : 1; // bit 4..4
+ UINT32 MemoryPoisonReceived : 1; // bit 5..5
+ UINT32 PhysicalLayerError : 1; // bit 6..6
+ UINT32 Reserved : 25; // bit 7..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CORRECTABLE_ERROR_STATUS;
+
+typedef union {
+ struct {
+ UINT32 CacheDataEccMask : 1; // bit 0..0
+ UINT32 MemoryDataEccMask : 1; // bit 1..1
+ UINT32 CrcThresholdMask : 1; // bit 2..2
+ UINT32 RetryThresholdMask : 1; // bit 3..3
+ UINT32 CachePoisonReceivedMask : 1; // bit 4..4
+ UINT32 MemoryPoisonReceivedMask : 1; // bit 5..5
+ UINT32 PhysicalLayerErrorMask : 1; // bit 6..6
+ UINT32 Reserved : 25; // bit 7..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CORRECTABLE_ERROR_MASK;
+
+typedef union {
+ struct {
+ UINT32 FirstErrorPointer : 4; // bit 0..3
+ UINT32 Reserved1 : 5; // bit 4..8
+ UINT32 MultipleHeaderRecordingCapability : 1; // bit 9..9
+ UINT32 Reserved2 : 3; // bit 10..12
+ UINT32 PoisonEnabled : 1; // bit 13..13
+ UINT32 Reserved3 : 18; // bit 14..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_ERROR_CAPABILITIES_AND_CONTROL;
+
+typedef struct {
+ CXL_1_1_UNCORRECTABLE_ERROR_STATUS UncorrectableErrorStatus;
+ CXL_1_1_UNCORRECTABLE_ERROR_MASK UncorrectableErrorMask;
+ CXL_1_1_UNCORRECTABLE_ERROR_SEVERITY UncorrectableErrorSeverity;
+ CXL_CORRECTABLE_ERROR_STATUS CorrectableErrorStatus;
+ CXL_CORRECTABLE_ERROR_MASK CorrectableErrorMask;
+ CXL_ERROR_CAPABILITIES_AND_CONTROL ErrorCapabilitiesAndControl;
+ UINT32 HeaderLog[16];
+} CXL_1_1_RAS_CAPABILITY_STRUCTURE;
+
+typedef union {
+ struct {
+ UINT32 DeviceTrustLevel : 2; // bit 0..1
+ UINT32 Reserved : 30; // bit 2..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_SECURITY_POLICY;
+
+typedef struct {
+ CXL_1_1_SECURITY_POLICY SecurityPolicy;
+} CXL_1_1_SECURITY_CAPABILITY_STRUCTURE;
+
+typedef union {
+ struct {
+ UINT64 CxlLinkVersionSupported : 4; // bit 0..3
+ UINT64 CxlLinkVersionReceived : 4; // bit 4..7
+ UINT64 LlrWrapValueSupported : 8; // bit 8..15
+ UINT64 LlrWrapValueReceived : 8; // bit 16..23
+ UINT64 NumRetryReceived : 5; // bit 24..28
+ UINT64 NumPhyReinitReceived : 5; // bit 29..33
+ UINT64 WrPtrReceived : 8; // bit 34..41
+ UINT64 EchoEseqReceived : 8; // bit 42..49
+ UINT64 NumFreeBufReceived : 8; // bit 50..57
+ UINT64 Reserved : 6; // bit 58..63
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_CAPABILITY;
+
+typedef union {
+ struct {
+ UINT16 LlReset : 1; // bit 0..0
+ UINT16 LlInitStall : 1; // bit 1..1
+ UINT16 LlCrdStall : 1; // bit 2..2
+ UINT16 InitState : 2; // bit 3..4
+ UINT16 LlRetryBufferConsumed : 8; // bit 5..12
+ UINT16 Reserved : 3; // bit 13..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_LINK_LAYER_CONTROL_AND_STATUS;
+
+typedef union {
+ struct {
+ UINT64 CacheReqCredits : 10; // bit 0..9
+ UINT64 CacheRspCredits : 10; // bit 10..19
+ UINT64 CacheDataCredits : 10; // bit 20..29
+ UINT64 MemReqRspCredits : 10; // bit 30..39
+ UINT64 MemDataCredits : 10; // bit 40..49
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_RX_CREDIT_CONTROL;
+
+typedef union {
+ struct {
+ UINT64 CacheReqCredits : 10; // bit 0..9
+ UINT64 CacheRspCredits : 10; // bit 10..19
+ UINT64 CacheDataCredits : 10; // bit 20..29
+ UINT64 MemReqRspCredits : 10; // bit 30..39
+ UINT64 MemDataCredits : 10; // bit 40..49
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_RX_CREDIT_RETURN_STATUS;
+
+typedef union {
+ struct {
+ UINT64 CacheReqCredits : 10; // bit 0..9
+ UINT64 CacheRspCredits : 10; // bit 10..19
+ UINT64 CacheDataCredits : 10; // bit 20..29
+ UINT64 MemReqRspCredits : 10; // bit 30..39
+ UINT64 MemDataCredits : 10; // bit 40..49
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_TX_CREDIT_STATUS;
+
+typedef union {
+ struct {
+ UINT32 AckForceThreshold : 8; // bit 0..7
+ UINT32 AckFLushRetimer : 10; // bit 8..17
+ } Bits;
+ UINT32 Uint32;
+} CXL_LINK_LAYER_ACK_TIMER_CONTROL;
+
+typedef union {
+ struct {
+ UINT32 MdhDisable : 1; // bit 0..0
+ UINT32 Reserved : 31; // bit 1..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_LINK_LAYER_DEFEATURE;
+
+typedef struct {
+ CXL_LINK_LAYER_CAPABILITY LinkLayerCapability;
+ CXL_LINK_LAYER_CONTROL_AND_STATUS LinkLayerControlStatus;
+ CXL_LINK_LAYER_RX_CREDIT_CONTROL LinkLayerRxCreditControl;
+ CXL_LINK_LAYER_RX_CREDIT_RETURN_STATUS LinkLayerRxCreditReturnStatus;
+ CXL_LINK_LAYER_TX_CREDIT_STATUS LinkLayerTxCreditStatus;
+ CXL_LINK_LAYER_ACK_TIMER_CONTROL LinkLayerAckTimerControl;
+ CXL_LINK_LAYER_DEFEATURE LinkLayerDefeature;
+} CXL_1_1_LINK_CAPABILITY_STRUCTURE;
+
+#define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180
+typedef union {
+ struct {
+ UINT32 Reserved1 : 4; // bit 0..3
+ UINT32 WeightedRoundRobinArbitrationWeight : 4; // bit 4..7
+ UINT32 Reserved2 : 24; // bit 8..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_IO_ARBITRATION_CONTROL;
+
+#define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET 0x1C0
+typedef union {
+ struct {
+ UINT32 Reserved1 : 4; // bit 0..3
+ UINT32 WeightedRoundRobinArbitrationWeight : 4; // bit 4..7
+ UINT32 Reserved2 : 24; // bit 8..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CACHE_MEMORY_ARBITRATION_CONTROL;
+///@}
+
+/// The CXL.RCRB base register definition
+/// Based on chapter 7.3 of Compute Express Link Specification Revision: 1.1
+///@{
+typedef union {
+ struct {
+ UINT64 RcrbEnable : 1; // bit 0..0
+ UINT64 Reserved : 12; // bit 1..12
+ UINT64 RcrbBaseAddress : 51; // bit 13..63
+ } Bits;
+ UINT64 Uint64;
+} CXL_RCRB_BASE;
+///@}
+
+#pragma pack()
+
+//
+// CXL Downstream / Upstream Port RCRB space register offsets
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.1 - Figure 97
+//
+#define CXL_PORT_RCRB_MEMBAR0_LOW_OFFSET 0x010
+#define CXL_PORT_RCRB_MEMBAR0_HIGH_OFFSET 0x014
+#define CXL_PORT_RCRB_EXTENDED_CAPABILITY_BASE_OFFSET 0x100
+
+#endif
diff --git a/MdePkg/Include/IndustryStandard/Pci.h b/MdePkg/Include/IndustryStandard/Pci.h
index 8ed96b992a..42c00ac762 100644
--- a/MdePkg/Include/IndustryStandard/Pci.h
+++ b/MdePkg/Include/IndustryStandard/Pci.h
But here is the thing that really disappoints me getting through
review - this change is completely, fundamentally unrelated to the
rest of this patch, and not even mentioned in the commit message. This
should have been a separate patch preceding this one.

@@ -1,7 +1,7 @@
/** @file
Support for the latest PCI standard.

-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -9,9 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef _PCI_H_
#define _PCI_H_

-#include <IndustryStandard/Pci30.h>
-#include <IndustryStandard/PciExpress21.h>
-#include <IndustryStandard/PciExpress30.h>
+#include <IndustryStandard/PciExpress50.h>
#include <IndustryStandard/PciCodeId.h>

#endif
Now, one final comment - and this is more of a project feature
suggestion:
Industry standard headers is something fairly special, even in
comparison with the rest of MdePkg. *I* would certainly like to ensure
I don't miss changes or additions to them.
Could we set up a dedicated group of reviewers for this folder only?
This need not affect the actual maintainership of MdePkg, just ensure
more eyeballs (or screen readers, braille terminals, ...) hit updates
here?

i.e. something like the below to Maintainers.txt:

F: MdePkg/Include/IndustryStandard/
R: Leif ...
R: ...
R: ...
This is a good suggestion. IndustryStandard needs more feedback.
Can you send the patch to update Maintainers.txt?

Thanks
Liming
/
Leif

--
2.21.0.windows.1




Re: [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers

Leif Lindholm
 

Hi Ashraf, but also Mike, Liming.

I just saw this patch go in and have some comments.

On Fri, Jul 24, 2020 at 23:56:12 +0530, Javeed, Ashraf wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2611

Register definitions from chapter 7 of Compute Express Link
Specification Revision 1.1 are ported into the new Cxl11.h.
The CXL Flex Bus registers are based on the PCIe Extended Capability
DVSEC structure header, led to the inclusion of upgraded Pci.h.

Signed-off-by: Ashraf Javeed <ashraf.javeed@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>
--

V4: fix code style

V3: Copyright date fix

V2: Indentation and double declaration fix, copyright date update
---
MdePkg/Include/IndustryStandard/Cxl11.h | 569 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
MdePkg/Include/IndustryStandard/Pci.h | 6 ++----
2 files changed, 571 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h b/MdePkg/Include/IndustryStandard/Cxl11.h
new file mode 100644
index 0000000000..933c1ab817
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Cxl11.h
@@ -0,0 +1,569 @@
+/** @file
+ CXL 1.1 Register definitions
+
+ This file contains the register definitions based on the Compute Express Link
+ (CXL) Specification Revision 1.1.
+
+Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _CXL11_H_
+#define _CXL11_H_
We should not be adding macros with a leading _ - these are intended
for toolchain use.

+
+#include <IndustryStandard/Pci.h>
+//
+// DVSEC Vendor ID
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1 - Table 58
+// (subject to change as per CXL assigned Vendor ID)
+//
+#define INTEL_CXL_DVSEC_VENDOR_ID 0x8086
This is Cxl11.h - surely this should be CXL_DVSEC_VENDOR_ID_INTEL?

+
+//
+// CXL Flex Bus Device default device and function number
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1
+//
+#define CXL_DEV_DEV 0
+#define CXL_DEV_FUNC 0
+
+//
+// Ensure proper structure formats
+//
+#pragma pack(1)
And this pragma has no function whatsoever with regards to any of the
register definition structs below. It would be much better if the
structs requiring packing (_DEVICE, _PORT, ...) were grouped together
and only those were given this treatment.

#pragma pack(1) is *not* a safe default.

+
+///
+/// The PCIe DVSEC for Flex Bus Device
+///@{
+typedef union {
+ struct {
+ UINT16 CacheCapable : 1; // bit 0
+ UINT16 IoCapable : 1; // bit 1
+ UINT16 MemCapable : 1; // bit 2
+ UINT16 MemHwInitMode : 1; // bit 3
+ UINT16 HdmCount : 2; // bit 4..5
+ UINT16 Reserved1 : 8; // bit 6..13
+ UINT16 ViralCapable : 1; // bit 14
+ UINT16 Reserved2 : 1; // bit 15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_CAPABILITY;
+
+typedef union {
+ struct {
+ UINT16 CacheEnable : 1; // bit 0
+ UINT16 IoEnable : 1; // bit 1
+ UINT16 MemEnable : 1; // bit 2
+ UINT16 CacheSfCoverage : 5; // bit 3..7
+ UINT16 CacheSfGranularity : 3; // bit 8..10
+ UINT16 CacheCleanEviction : 1; // bit 11
+ UINT16 Reserved1 : 2; // bit 12..13
+ UINT16 ViralEnable : 1; // bit 14
+ UINT16 Reserved2 : 1; // bit 15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_CONTROL;
+
+typedef union {
+ struct {
+ UINT16 Reserved1 : 14; // bit 0..13
+ UINT16 ViralStatus : 1; // bit 14
+ UINT16 Reserved2 : 1; // bit 15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_STATUS;
+
+typedef union {
+ struct {
+ UINT16 Reserved1 : 1; // bit 0
+ UINT16 Reserved2 : 1; // bit 1
+ UINT16 Reserved3 : 1; // bit 2
+ UINT16 Reserved4 : 13; // bit 3..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_DEVICE_CONTROL2;
+
+typedef union {
+ struct {
+ UINT16 Reserved1 : 1; // bit 0
+ UINT16 Reserved2 : 1; // bit 1
+ UINT16 Reserved3 : 14; // bit 2..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_DEVICE_STATUS2;
+
+typedef union {
+ struct {
+ UINT16 ConfigLock : 1; // bit 0
+ UINT16 Reserved1 : 15; // bit 1..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_DVSEC_FLEX_BUS_DEVICE_LOCK;
+
+typedef union {
+ struct {
+ UINT32 MemorySizeHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 MemoryInfoValid : 1; // bit 0
+ UINT32 MemoryActive : 1; // bit 1
+ UINT32 MediaType : 3; // bit 2..4
+ UINT32 MemoryClass : 3; // bit 5..7
+ UINT32 DesiredInterleave : 3; // bit 8..10
+ UINT32 Reserved : 17; // bit 11..27
+ UINT32 MemorySizeLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_LOW;
+
+typedef union {
+ struct {
+ UINT32 MemoryBaseHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 Reserved : 28; // bit 0..27
+ UINT32 MemoryBaseLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_LOW;
+
+
+typedef union {
+ struct {
+ UINT32 MemorySizeHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 MemoryInfoValid : 1; // bit 0
+ UINT32 MemoryActive : 1; // bit 1
+ UINT32 MediaType : 3; // bit 2..4
+ UINT32 MemoryClass : 3; // bit 5..7
+ UINT32 DesiredInterleave : 3; // bit 8..10
+ UINT32 Reserved : 17; // bit 11..27
+ UINT32 MemorySizeLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_LOW;
+
+typedef union {
+ struct {
+ UINT32 MemoryBaseHigh : 32; // bit 0..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_HIGH;
+
+typedef union {
+ struct {
+ UINT32 Reserved : 28; // bit 0..27
+ UINT32 MemoryBaseLow : 4; // bit 28..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_LOW;
+
+//
+// Flex Bus Device DVSEC ID
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1, Table 58
+//
+#define FLEX_BUS_DEVICE_DVSEC_ID 0
+
+//
+// PCIe DVSEC for Flex Bus Device
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1, Figure 95
+//
+typedef struct {
+ PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER Header; // offset 0
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_1 DesignatedVendorSpecificHeader1; // offset 4
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_2 DesignatedVendorSpecificHeader2; // offset 8
+ CXL_DVSEC_FLEX_BUS_DEVICE_CAPABILITY DeviceCapability; // offset 10
+ CXL_DVSEC_FLEX_BUS_DEVICE_CONTROL DeviceControl; // offset 12
+ CXL_DVSEC_FLEX_BUS_DEVICE_STATUS DeviceStatus; // offset 14
+ CXL_1_1_DVSEC_FLEX_BUS_DEVICE_CONTROL2 DeviceControl2; // offset 16
+ CXL_1_1_DVSEC_FLEX_BUS_DEVICE_STATUS2 DeviceStatus2; // offset 18
+ CXL_DVSEC_FLEX_BUS_DEVICE_LOCK DeviceLock; // offset 20
+ UINT16 Reserved; // offset 22
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_HIGH DeviceRange1SizeHigh; // offset 24
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_LOW DeviceRange1SizeLow; // offset 28
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_HIGH DeviceRange1BaseHigh; // offset 32
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_LOW DeviceRange1BaseLow; // offset 36
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_HIGH DeviceRange2SizeHigh; // offset 40
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_LOW DeviceRange2SizeLow; // offset 44
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_HIGH DeviceRange2BaseHigh; // offset 48
+ CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_LOW DeviceRange2BaseLow; // offset 52
+} CXL_1_1_DVSEC_FLEX_BUS_DEVICE;
+///@}
+
+///
+/// PCIe DVSEC for FLex Bus Port
+///@{
+typedef union {
+ struct {
+ UINT16 CacheCapable : 1; // bit 0
+ UINT16 IoCapable : 1; // bit 1
+ UINT16 MemCapable : 1; // bit 2
+ UINT16 Reserved : 13; // bit 3..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_PORT_CAPABILITY;
+
+typedef union {
+ struct {
+ UINT16 CacheEnable : 1; // bit 0
+ UINT16 IoEnable : 1; // bit 1
+ UINT16 MemEnable : 1; // bit 2
+ UINT16 CxlSyncBypassEnable : 1; // bit 3
+ UINT16 DriftBufferEnable : 1; // bit 4
+ UINT16 Reserved : 3; // bit 5..7
+ UINT16 Retimer1Present : 1; // bit 8
+ UINT16 Retimer2Present : 1; // bit 9
+ UINT16 Reserved2 : 6; // bit 10..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_PORT_CONTROL;
+
+typedef union {
+ struct {
+ UINT16 CacheEnable : 1; // bit 0
+ UINT16 IoEnable : 1; // bit 1
+ UINT16 MemEnable : 1; // bit 2
+ UINT16 CxlSyncBypassEnable : 1; // bit 3
+ UINT16 DriftBufferEnable : 1; // bit 4
+ UINT16 Reserved : 3; // bit 5..7
+ UINT16 CxlCorrectableProtocolIdFramingError : 1; // bit 8
+ UINT16 CxlUncorrectableProtocolIdFramingError : 1; // bit 9
+ UINT16 CxlUnexpectedProtocolIdDropped : 1; // bit 10
+ UINT16 Reserved2 : 5; // bit 11..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_1_1_DVSEC_FLEX_BUS_PORT_STATUS;
+
+//
+// Flex Bus Port DVSEC ID
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.3, Table 62
+//
+#define FLEX_BUS_PORT_DVSEC_ID 7
+
+//
+// PCIe DVSEC for Flex Bus Port
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.3, Figure 99
+//
+typedef struct {
+ PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER Header; // offset 0
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_1 DesignatedVendorSpecificHeader1; // offset 4
+ PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_2 DesignatedVendorSpecificHeader2; // offset 8
+ CXL_1_1_DVSEC_FLEX_BUS_PORT_CAPABILITY PortCapability; // offset 10
+ CXL_1_1_DVSEC_FLEX_BUS_PORT_CONTROL PortControl; // offset 12
+ CXL_1_1_DVSEC_FLEX_BUS_PORT_STATUS PortStatus; // offset 14
+} CXL_1_1_DVSEC_FLEX_BUS_PORT;
+///@}
+
+///
+/// CXL 1.1 Upstream and Downstream Port Subsystem Component registers
+///
+
+/// The CXL.Cache and CXL.Memory Architectural register definitions
+/// Based on chapter 7.2.2 of Compute Express Link Specification Revision: 1.1
+///@{
+
+#define CXL_CAPABILITY_HEADER_OFFSET 0
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlCacheMemVersion : 4; // bit 20..23
+ UINT32 ArraySize : 8; // bit 24..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CAPABILITY_HEADER;
+
+#define CXL_RAS_CAPABILITY_HEADER_OFFSET 4
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlRasCapabilityPointer : 12; // bit 20..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_RAS_CAPABILITY_HEADER;
+
+#define CXL_SECURITY_CAPABILITY_HEADER_OFFSET 8
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlSecurityCapabilityPointer : 12; // bit 20..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_SECURITY_CAPABILITY_HEADER;
+
+#define CXL_LINK_CAPABILITY_HEADER_OFFSET 0xC
+typedef union {
+ struct {
+ UINT32 CxlCapabilityId : 16; // bit 0..15
+ UINT32 CxlCapabilityVersion : 4; // bit 16..19
+ UINT32 CxlLinkCapabilityPointer : 12; // bit 20..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_LINK_CAPABILITY_HEADER;
+
+typedef union {
+ struct {
+ UINT32 CacheDataParity : 1; // bit 0..0
+ UINT32 CacheAddressParity : 1; // bit 1..1
+ UINT32 CacheByteEnableParity : 1; // bit 2..2
+ UINT32 CacheDataEcc : 1; // bit 3..3
+ UINT32 MemDataParity : 1; // bit 4..4
+ UINT32 MemAddressParity : 1; // bit 5..5
+ UINT32 MemByteEnableParity : 1; // bit 6..6
+ UINT32 MemDataEcc : 1; // bit 7..7
+ UINT32 ReInitThreshold : 1; // bit 8..8
+ UINT32 RsvdEncodingViolation : 1; // bit 9..9
+ UINT32 PoisonReceived : 1; // bit 10..10
+ UINT32 ReceiverOverflow : 1; // bit 11..11
+ UINT32 Reserved : 20; // bit 12..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_UNCORRECTABLE_ERROR_STATUS;
+
+typedef union {
+ struct {
+ UINT32 CacheDataParityMask : 1; // bit 0..0
+ UINT32 CacheAddressParityMask : 1; // bit 1..1
+ UINT32 CacheByteEnableParityMask : 1; // bit 2..2
+ UINT32 CacheDataEccMask : 1; // bit 3..3
+ UINT32 MemDataParityMask : 1; // bit 4..4
+ UINT32 MemAddressParityMask : 1; // bit 5..5
+ UINT32 MemByteEnableParityMask : 1; // bit 6..6
+ UINT32 MemDataEccMask : 1; // bit 7..7
+ UINT32 ReInitThresholdMask : 1; // bit 8..8
+ UINT32 RsvdEncodingViolationMask : 1; // bit 9..9
+ UINT32 PoisonReceivedMask : 1; // bit 10..10
+ UINT32 ReceiverOverflowMask : 1; // bit 11..11
+ UINT32 Reserved : 20; // bit 12..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_UNCORRECTABLE_ERROR_MASK;
+
+typedef union {
+ struct {
+ UINT32 CacheDataParitySeverity : 1; // bit 0..0
+ UINT32 CacheAddressParitySeverity : 1; // bit 1..1
+ UINT32 CacheByteEnableParitySeverity : 1; // bit 2..2
+ UINT32 CacheDataEccSeverity : 1; // bit 3..3
+ UINT32 MemDataParitySeverity : 1; // bit 4..4
+ UINT32 MemAddressParitySeverity : 1; // bit 5..5
+ UINT32 MemByteEnableParitySeverity : 1; // bit 6..6
+ UINT32 MemDataEccSeverity : 1; // bit 7..7
+ UINT32 ReInitThresholdSeverity : 1; // bit 8..8
+ UINT32 RsvdEncodingViolationSeverity : 1; // bit 9..9
+ UINT32 PoisonReceivedSeverity : 1; // bit 10..10
+ UINT32 ReceiverOverflowSeverity : 1; // bit 11..11
+ UINT32 Reserved : 20; // bit 12..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_UNCORRECTABLE_ERROR_SEVERITY;
+
+typedef union {
+ struct {
+ UINT32 CacheDataEcc : 1; // bit 0..0
+ UINT32 MemoryDataEcc : 1; // bit 1..1
+ UINT32 CrcThreshold : 1; // bit 2..2
+ UINT32 RetryThreshold : 1; // bit 3..3
+ UINT32 CachePoisonReceived : 1; // bit 4..4
+ UINT32 MemoryPoisonReceived : 1; // bit 5..5
+ UINT32 PhysicalLayerError : 1; // bit 6..6
+ UINT32 Reserved : 25; // bit 7..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CORRECTABLE_ERROR_STATUS;
+
+typedef union {
+ struct {
+ UINT32 CacheDataEccMask : 1; // bit 0..0
+ UINT32 MemoryDataEccMask : 1; // bit 1..1
+ UINT32 CrcThresholdMask : 1; // bit 2..2
+ UINT32 RetryThresholdMask : 1; // bit 3..3
+ UINT32 CachePoisonReceivedMask : 1; // bit 4..4
+ UINT32 MemoryPoisonReceivedMask : 1; // bit 5..5
+ UINT32 PhysicalLayerErrorMask : 1; // bit 6..6
+ UINT32 Reserved : 25; // bit 7..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CORRECTABLE_ERROR_MASK;
+
+typedef union {
+ struct {
+ UINT32 FirstErrorPointer : 4; // bit 0..3
+ UINT32 Reserved1 : 5; // bit 4..8
+ UINT32 MultipleHeaderRecordingCapability : 1; // bit 9..9
+ UINT32 Reserved2 : 3; // bit 10..12
+ UINT32 PoisonEnabled : 1; // bit 13..13
+ UINT32 Reserved3 : 18; // bit 14..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_ERROR_CAPABILITIES_AND_CONTROL;
+
+typedef struct {
+ CXL_1_1_UNCORRECTABLE_ERROR_STATUS UncorrectableErrorStatus;
+ CXL_1_1_UNCORRECTABLE_ERROR_MASK UncorrectableErrorMask;
+ CXL_1_1_UNCORRECTABLE_ERROR_SEVERITY UncorrectableErrorSeverity;
+ CXL_CORRECTABLE_ERROR_STATUS CorrectableErrorStatus;
+ CXL_CORRECTABLE_ERROR_MASK CorrectableErrorMask;
+ CXL_ERROR_CAPABILITIES_AND_CONTROL ErrorCapabilitiesAndControl;
+ UINT32 HeaderLog[16];
+} CXL_1_1_RAS_CAPABILITY_STRUCTURE;
+
+typedef union {
+ struct {
+ UINT32 DeviceTrustLevel : 2; // bit 0..1
+ UINT32 Reserved : 30; // bit 2..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_1_1_SECURITY_POLICY;
+
+typedef struct {
+ CXL_1_1_SECURITY_POLICY SecurityPolicy;
+} CXL_1_1_SECURITY_CAPABILITY_STRUCTURE;
+
+typedef union {
+ struct {
+ UINT64 CxlLinkVersionSupported : 4; // bit 0..3
+ UINT64 CxlLinkVersionReceived : 4; // bit 4..7
+ UINT64 LlrWrapValueSupported : 8; // bit 8..15
+ UINT64 LlrWrapValueReceived : 8; // bit 16..23
+ UINT64 NumRetryReceived : 5; // bit 24..28
+ UINT64 NumPhyReinitReceived : 5; // bit 29..33
+ UINT64 WrPtrReceived : 8; // bit 34..41
+ UINT64 EchoEseqReceived : 8; // bit 42..49
+ UINT64 NumFreeBufReceived : 8; // bit 50..57
+ UINT64 Reserved : 6; // bit 58..63
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_CAPABILITY;
+
+typedef union {
+ struct {
+ UINT16 LlReset : 1; // bit 0..0
+ UINT16 LlInitStall : 1; // bit 1..1
+ UINT16 LlCrdStall : 1; // bit 2..2
+ UINT16 InitState : 2; // bit 3..4
+ UINT16 LlRetryBufferConsumed : 8; // bit 5..12
+ UINT16 Reserved : 3; // bit 13..15
+ } Bits;
+ UINT16 Uint16;
+} CXL_LINK_LAYER_CONTROL_AND_STATUS;
+
+typedef union {
+ struct {
+ UINT64 CacheReqCredits : 10; // bit 0..9
+ UINT64 CacheRspCredits : 10; // bit 10..19
+ UINT64 CacheDataCredits : 10; // bit 20..29
+ UINT64 MemReqRspCredits : 10; // bit 30..39
+ UINT64 MemDataCredits : 10; // bit 40..49
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_RX_CREDIT_CONTROL;
+
+typedef union {
+ struct {
+ UINT64 CacheReqCredits : 10; // bit 0..9
+ UINT64 CacheRspCredits : 10; // bit 10..19
+ UINT64 CacheDataCredits : 10; // bit 20..29
+ UINT64 MemReqRspCredits : 10; // bit 30..39
+ UINT64 MemDataCredits : 10; // bit 40..49
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_RX_CREDIT_RETURN_STATUS;
+
+typedef union {
+ struct {
+ UINT64 CacheReqCredits : 10; // bit 0..9
+ UINT64 CacheRspCredits : 10; // bit 10..19
+ UINT64 CacheDataCredits : 10; // bit 20..29
+ UINT64 MemReqRspCredits : 10; // bit 30..39
+ UINT64 MemDataCredits : 10; // bit 40..49
+ } Bits;
+ UINT64 Uint64;
+} CXL_LINK_LAYER_TX_CREDIT_STATUS;
+
+typedef union {
+ struct {
+ UINT32 AckForceThreshold : 8; // bit 0..7
+ UINT32 AckFLushRetimer : 10; // bit 8..17
+ } Bits;
+ UINT32 Uint32;
+} CXL_LINK_LAYER_ACK_TIMER_CONTROL;
+
+typedef union {
+ struct {
+ UINT32 MdhDisable : 1; // bit 0..0
+ UINT32 Reserved : 31; // bit 1..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_LINK_LAYER_DEFEATURE;
+
+typedef struct {
+ CXL_LINK_LAYER_CAPABILITY LinkLayerCapability;
+ CXL_LINK_LAYER_CONTROL_AND_STATUS LinkLayerControlStatus;
+ CXL_LINK_LAYER_RX_CREDIT_CONTROL LinkLayerRxCreditControl;
+ CXL_LINK_LAYER_RX_CREDIT_RETURN_STATUS LinkLayerRxCreditReturnStatus;
+ CXL_LINK_LAYER_TX_CREDIT_STATUS LinkLayerTxCreditStatus;
+ CXL_LINK_LAYER_ACK_TIMER_CONTROL LinkLayerAckTimerControl;
+ CXL_LINK_LAYER_DEFEATURE LinkLayerDefeature;
+} CXL_1_1_LINK_CAPABILITY_STRUCTURE;
+
+#define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180
+typedef union {
+ struct {
+ UINT32 Reserved1 : 4; // bit 0..3
+ UINT32 WeightedRoundRobinArbitrationWeight : 4; // bit 4..7
+ UINT32 Reserved2 : 24; // bit 8..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_IO_ARBITRATION_CONTROL;
+
+#define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET 0x1C0
+typedef union {
+ struct {
+ UINT32 Reserved1 : 4; // bit 0..3
+ UINT32 WeightedRoundRobinArbitrationWeight : 4; // bit 4..7
+ UINT32 Reserved2 : 24; // bit 8..31
+ } Bits;
+ UINT32 Uint32;
+} CXL_CACHE_MEMORY_ARBITRATION_CONTROL;
+///@}
+
+/// The CXL.RCRB base register definition
+/// Based on chapter 7.3 of Compute Express Link Specification Revision: 1.1
+///@{
+typedef union {
+ struct {
+ UINT64 RcrbEnable : 1; // bit 0..0
+ UINT64 Reserved : 12; // bit 1..12
+ UINT64 RcrbBaseAddress : 51; // bit 13..63
+ } Bits;
+ UINT64 Uint64;
+} CXL_RCRB_BASE;
+///@}
+
+#pragma pack()
+
+//
+// CXL Downstream / Upstream Port RCRB space register offsets
+// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.1 - Figure 97
+//
+#define CXL_PORT_RCRB_MEMBAR0_LOW_OFFSET 0x010
+#define CXL_PORT_RCRB_MEMBAR0_HIGH_OFFSET 0x014
+#define CXL_PORT_RCRB_EXTENDED_CAPABILITY_BASE_OFFSET 0x100
+
+#endif
diff --git a/MdePkg/Include/IndustryStandard/Pci.h b/MdePkg/Include/IndustryStandard/Pci.h
index 8ed96b992a..42c00ac762 100644
--- a/MdePkg/Include/IndustryStandard/Pci.h
+++ b/MdePkg/Include/IndustryStandard/Pci.h
But here is the thing that really disappoints me getting through
review - this change is completely, fundamentally unrelated to the
rest of this patch, and not even mentioned in the commit message. This
should have been a separate patch preceding this one.

@@ -1,7 +1,7 @@
/** @file
Support for the latest PCI standard.

-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -9,9 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef _PCI_H_
#define _PCI_H_

-#include <IndustryStandard/Pci30.h>
-#include <IndustryStandard/PciExpress21.h>
-#include <IndustryStandard/PciExpress30.h>
+#include <IndustryStandard/PciExpress50.h>
#include <IndustryStandard/PciCodeId.h>

#endif
Now, one final comment - and this is more of a project feature
suggestion:
Industry standard headers is something fairly special, even in
comparison with the rest of MdePkg. *I* would certainly like to ensure
I don't miss changes or additions to them.
Could we set up a dedicated group of reviewers for this folder only?
This need not affect the actual maintainership of MdePkg, just ensure
more eyeballs (or screen readers, braille terminals, ...) hit updates
here?

i.e. something like the below to Maintainers.txt:

F: MdePkg/Include/IndustryStandard/
R: Leif ...
R: ...
R: ...

/
Leif

--
2.21.0.windows.1




Re: [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

Liming Gao
 

Leif:
VFR file has the similar case. VFR is converted to xxx.c, then compile it to obj file.

Bob:
Has BaseTools such detection if VFR and C source file have the same file name?

Thanks
Liming

-----Original Message-----
From: Leif Lindholm <leif@...>
Sent: Monday, July 27, 2020 9:58 PM
To: devel@edk2.groups.io; pierre.gondois@...; Masahisa Kojima <masahisa.kojima@...>
Cc: sami.mujawar@...; tomas.pilar@...; Feng, Bob C <bob.c.feng@...>; Gao, Liming <liming.gao@...>; Ard
Biesheuvel <ard.biesheuvel@...>
Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

Hi Pierre, (+Masahisa)

This commit (0a4aa20e8d44) made for an exciting start to my week.

Socionext's Developerbox failed to build for me, with the spectacular
error message:

/usr/lib/gcc-cross/aarch64-linux-gnu/8/../../../../aarch64-linux-gnu/bin/ld:
DWARF error: could not find abbrev number 5912
/tmp/ccKt4gaM.ltrans0.ltrans.o: in function `RegisterDevices':
<artificial>:(.text.RegisterDevices+0xb0): undefined reference to
`RegisterEmmc'

GCC49 (without lto) and CLANG38 profiles give much the same result,
with slightly less esoteric messages.

The reason for this turned out to be that edk2-platforms
Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/ has both an Emmc.asl
and an Emmc.c file, which after this patch both generate an Emmc.obj
in the same output directory.

I think the correct course of action is to fix this in the SynQuacer
driver, but I am reporting it here so we get it logged in the list
archives.

It would of course be good if the build system could detect and warn
over cases like this, rather than silently overwriting existing object
files.

Masahisa - since Ard is still on holiday, could you create a patch and
send out for me to review? Either one of the files needs to be
renamed, or we need to move the .asl files (Emmc.asl and Optee.asl)
into a subdirectory.

Best Regards,

Leif

On Wed, Jul 01, 2020 at 15:06:03 +0100, PierreGondois wrote:
From: Pierre Gondois <pierre.gondois@...>

The AmlToHex script and Posix/WindowsLike wrappers convert
an AML file to a .hex file, containing a C array storing
AML bytecode. This ".hex" file can then be included in a
C file, allowing to access the AML bytecode from this C
file.

The EDK2 build system doesn't allow to a depict dependency
orders between files of different languages. For instance,
in a module containing a ".c" file and a ".asl", the ".c"
file may or may not be built prior to the ".asl" file.
This prevents any inclusion of a generated ".hex" in a
".c" file since this later ".hex" file may or may not
have been created yet.

This patch modifies the AmlToC script to generate a C file
instead of a ".hex" file.
It also adds the generation of an intermediate ".amli" file
when compiling an ASL file, and adds a rule to convert this
".amli" to a C file.

This allows to generate a C file containing the AML bytecode
from an ASL file. This C file will then be handled by the EDK2
build system to generate an object file.
Thus, no file inclusion will be required anymore. The C file
requiring the AML bytecode as a C array, and the ASL file,
will be compiled independently. The C array must be defined
as an external symbol. The linker is resolving the
reference to the C array symbol.

To summarize, the flow goes as:
-1. ASL file is compiled to AML;
-2. AML file is copied to a ".amli" intermediate file;
-3. EDK2 build system applies the rule relevant to ".amli"
files. This is, calling the "AmlToC" script, generating
a C file from the ".amli" file;
-4. EDK2 build system applies the rule relevant to C files.
This is creating an object file.
-5. EDK2 build system links the object file containing the
AML bytecode with the object file requiring it.

Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
Suggested-by: Tomas Pilar <Tomas.Pilar@...>
---

The changes can be seen at: https://github.com/PierreARM/edk2/commits/803_Compile_AML_bytecode_array_into_OBJ_file_v5

Notes:
v1:
- Add a new rule to the build_rule.template file to
generate ".obj" files from .asl files, and modify
the AmlToC script accordingly. [Pierre]
v2:
- Restrict the rule to DXE_DRIVER. This allows to build
the OvmfPkg, which was not the case in v1. [Pierre]
v3:
- Changed "Signed-off-by" to "Suggested-by". [Bob]
v4:
- No modification. Re-sending the patch with base64
encoding to conserve the right line endings. [Bob]
v5:
- No modification. [Pierre]

BaseTools/Conf/build_rule.template | 15 +++-
BaseTools/Source/Python/AmlToC/AmlToC.py | 82 ++++++++------------
2 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template
index 0822b681fcd9f61c6508e6f93ffc31fa70fd7059..c034869915914936e28f64a6aadba08e0169da44 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -419,6 +419,7 @@

<OutputFile>
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli

<ExtraDependency>
$(MAKE_FILE)
@@ -428,14 +429,24 @@
"$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) /I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i >
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
"$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
- -AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(CP) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli

<Command.GCC>
Trim --asl-file --asl-deps -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i -i $(INC_LIST) ${src}
"$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) -I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i >
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
"$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
- -AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(CP) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli
+
+[Acpi-Machine-Language-File-to-C.DXE_DRIVER]
+ <InputFile>
+ ?.amli
+
+ <OutputFile>
+ ${s_path}(+)${s_base}.c
+
+ <Command>
+ -AmlToC ${src}

[C-Code-File.AcpiTable]
<InputFile>
diff --git a/BaseTools/Source/Python/AmlToC/AmlToC.py b/BaseTools/Source/Python/AmlToC/AmlToC.py
index 643db2910e37acfdd80ac18d288c921320a79ce1..346de7159de702d860bbd809ddbe8175f1493cfb 100644
--- a/BaseTools/Source/Python/AmlToC/AmlToC.py
+++ b/BaseTools/Source/Python/AmlToC/AmlToC.py
@@ -1,9 +1,9 @@
## @file
#
-# Convert an AML file to a .hex file containing the AML bytecode stored in a
+# Convert an AML file to a .c file containing the AML bytecode stored in a
# C array.
-# By default, "Tables\Dsdt.aml" will generate "Tables\Dsdt.hex".
-# "Tables\Dsdt.hex" will contain a C array named "dsdt_aml_code" that contains
+# By default, "Tables\Dsdt.aml" will generate "Tables\Dsdt.c".
+# "Tables\Dsdt.c" will contain a C array named "dsdt_aml_code" that contains
# the AML bytecode.
#
# Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
@@ -17,31 +17,26 @@ from Common.BuildToolError import *
import sys
import os

+__description__ = """
+Convert an AML file to a .c file containing the AML bytecode stored in a C
+array. By default, Tables\Dsdt.aml will generate Tables\Dsdt.c.
+Tables\Dsdt.c will contain a C array named "dsdt_aml_code" that contains
+the AML bytecode.
+"""
+
## Parse the command line arguments.
#
# @retval A argparse.NameSpace instance, containing parsed values.
#
def ParseArgs():
# Initialize the parser.
- Parser = argparse.ArgumentParser(
- description="Convert an AML file to a .hex file containing the AML " + \
- "bytecode stored in a C array. By default, " + \
- "\"Tables\\Dsdt.aml\" will generate" + \
- "\"Tables\\Dsdt.hex\". \"Tables\\Dsdt.hex\" will " + \
- "contain a C array named \"dsdt_aml_code\" that " + \
- "contains the AML bytecode."
- )
+ Parser = argparse.ArgumentParser(description=__description__)

# Define the possible arguments.
- Parser.add_argument(
- dest="InputFile",
- help="Path to an input AML file to generate a .hex file from."
- )
- Parser.add_argument(
- "-o", "--out-dir", dest="OutDir",
- help="Output directory where the .hex file will be generated. " + \
- "Default is the input file's directory."
- )
+ Parser.add_argument(dest="InputFile",
+ help="Path to an input AML file to generate a .c file from.")
+ Parser.add_argument("-o", "--out-dir", dest="OutDir",
+ help="Output directory where the .c file will be generated. Default is the input file's directory.")

# Parse the input arguments.
Args = Parser.parse_args()
@@ -55,9 +50,7 @@ def ParseArgs():
with open(Args.InputFile, "rb") as fIn:
Signature = str(fIn.read(4))
if ("DSDT" not in Signature) and ("SSDT" not in Signature):
- EdkLogger.info("Invalid file type. " + \
- "File does not have a valid " + \
- "DSDT or SSDT signature: %s" % Args.InputFile)
+ EdkLogger.info("Invalid file type. File does not have a valid DSDT or SSDT signature: {}".format(Args.InputFile))
return None

# Get the basename of the input file.
@@ -66,42 +59,39 @@ def ParseArgs():

# If no output directory is specified, output to the input directory.
if not Args.OutDir:
- Args.OutputFile = os.path.join(
- os.path.dirname(Args.InputFile),
- BaseName + ".hex"
- )
+ Args.OutputFile = os.path.join(os.path.dirname(Args.InputFile),
+ BaseName + ".c")
else:
if not os.path.exists(Args.OutDir):
os.mkdir(Args.OutDir)
- Args.OutputFile = os.path.join(Args.OutDir, BaseName + ".hex")
+ Args.OutputFile = os.path.join(Args.OutDir, BaseName + ".c")

Args.BaseName = BaseName

return Args

-## Convert an AML file to a .hex file containing the AML bytecode stored
+## Convert an AML file to a .c file containing the AML bytecode stored
# in a C array.
#
# @param InputFile Path to the input AML file.
-# @param OutputFile Path to the output .hex file to generate.
+# @param OutputFile Path to the output .c file to generate.
# @param BaseName Base name of the input file.
-# This is also the name of the generated .hex file.
+# This is also the name of the generated .c file.
#
-def AmlToHex(InputFile, OutputFile, BaseName):
+def AmlToC(InputFile, OutputFile, BaseName):

- MacroName = "__{}_HEX__".format(BaseName.upper())
ArrayName = BaseName.lower() + "_aml_code"
+ FileHeader =\
+"""
+// This file has been generated from:
+// -Python script: {}
+// -Input AML file: {}
+
+"""

with open(InputFile, "rb") as fIn, open(OutputFile, "w") as fOut:
# Write header.
- fOut.write("// This file has been generated from:\n" + \
- "// \tPython script: " + \
- os.path.abspath(__file__) + "\n" + \
- "// \tInput AML file: " + \
- os.path.abspath(InputFile) + "\n\n" + \
- "#ifndef {}\n".format(MacroName) + \
- "#define {}\n\n".format(MacroName)
- )
+ fOut.write(FileHeader.format(os.path.abspath(InputFile), os.path.abspath(__file__)))

# Write the array and its content.
fOut.write("unsigned char {}[] = {{\n ".format(ArrayName))
@@ -115,15 +105,12 @@ def AmlToHex(InputFile, OutputFile, BaseName):
byte = fIn.read(1)
fOut.write("\n};\n")

- # Write footer.
- fOut.write("#endif // {}\n".format(MacroName))
-
## Main method
#
# This method:
# 1- Initialize an EdkLogger instance.
# 2- Parses the input arguments.
-# 3- Converts an AML file to a .hex file containing the AML bytecode stored
+# 3- Converts an AML file to a .c file containing the AML bytecode stored
# in a C array.
#
# @retval 0 Success.
@@ -139,10 +126,9 @@ def Main():
if not CommandArguments:
return 1

- # Convert an AML file to a .hex file containing the AML bytecode stored
+ # Convert an AML file to a .c file containing the AML bytecode stored
# in a C array.
- AmlToHex(CommandArguments.InputFile, CommandArguments.OutputFile,
- CommandArguments.BaseName)
+ AmlToC(CommandArguments.InputFile, CommandArguments.OutputFile, CommandArguments.BaseName)
except Exception as e:
print(e)
return 1
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


Re: [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

Sami Mujawar
 

Hi Leif,

Would updating build_rule.template so that the AmlToC Script generates a C file with an AutoGen prefix be an option?

Regards,

Sami Mujawar

-----Original Message-----
From: Leif Lindholm <leif@...>
Sent: 27 July 2020 02:58 PM
To: devel@edk2.groups.io; Pierre Gondois <Pierre.Gondois@...>; Masahisa Kojima <masahisa.kojima@...>
Cc: Sami Mujawar <Sami.Mujawar@...>; Tomas Pilar <Tomas.Pilar@...>; bob.c.feng@...; liming.gao@...; Ard Biesheuvel <Ard.Biesheuvel@...>
Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

Hi Pierre, (+Masahisa)

This commit (0a4aa20e8d44) made for an exciting start to my week.

Socionext's Developerbox failed to build for me, with the spectacular error message:

/usr/lib/gcc-cross/aarch64-linux-gnu/8/../../../../aarch64-linux-gnu/bin/ld:
DWARF error: could not find abbrev number 5912
/tmp/ccKt4gaM.ltrans0.ltrans.o: in function `RegisterDevices':
<artificial>:(.text.RegisterDevices+0xb0): undefined reference to `RegisterEmmc'

GCC49 (without lto) and CLANG38 profiles give much the same result, with slightly less esoteric messages.

The reason for this turned out to be that edk2-platforms Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/ has both an Emmc.asl and an Emmc.c file, which after this patch both generate an Emmc.obj in the same output directory.

I think the correct course of action is to fix this in the SynQuacer driver, but I am reporting it here so we get it logged in the list archives.

It would of course be good if the build system could detect and warn over cases like this, rather than silently overwriting existing object files.

Masahisa - since Ard is still on holiday, could you create a patch and send out for me to review? Either one of the files needs to be renamed, or we need to move the .asl files (Emmc.asl and Optee.asl) into a subdirectory.

Best Regards,

Leif

On Wed, Jul 01, 2020 at 15:06:03 +0100, PierreGondois wrote:
From: Pierre Gondois <pierre.gondois@...>

The AmlToHex script and Posix/WindowsLike wrappers convert an AML file
to a .hex file, containing a C array storing AML bytecode. This ".hex"
file can then be included in a C file, allowing to access the AML
bytecode from this C file.

The EDK2 build system doesn't allow to a depict dependency orders
between files of different languages. For instance, in a module
containing a ".c" file and a ".asl", the ".c"
file may or may not be built prior to the ".asl" file.
This prevents any inclusion of a generated ".hex" in a ".c" file since
this later ".hex" file may or may not have been created yet.

This patch modifies the AmlToC script to generate a C file instead of
a ".hex" file.
It also adds the generation of an intermediate ".amli" file when
compiling an ASL file, and adds a rule to convert this ".amli" to a C
file.

This allows to generate a C file containing the AML bytecode from an
ASL file. This C file will then be handled by the EDK2 build system to
generate an object file.
Thus, no file inclusion will be required anymore. The C file requiring
the AML bytecode as a C array, and the ASL file, will be compiled
independently. The C array must be defined as an external symbol. The
linker is resolving the reference to the C array symbol.

To summarize, the flow goes as:
-1. ASL file is compiled to AML;
-2. AML file is copied to a ".amli" intermediate file; -3. EDK2
build system applies the rule relevant to ".amli"
files. This is, calling the "AmlToC" script, generating
a C file from the ".amli" file;
-4. EDK2 build system applies the rule relevant to C files.
This is creating an object file.
-5. EDK2 build system links the object file containing the
AML bytecode with the object file requiring it.

Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
Suggested-by: Tomas Pilar <Tomas.Pilar@...>
---

The changes can be seen at:
https://github.com/PierreARM/edk2/commits/803_Compile_AML_bytecode_arr
ay_into_OBJ_file_v5

Notes:
v1:
- Add a new rule to the build_rule.template file to
generate ".obj" files from .asl files, and modify
the AmlToC script accordingly. [Pierre]
v2:
- Restrict the rule to DXE_DRIVER. This allows to build
the OvmfPkg, which was not the case in v1. [Pierre]
v3:
- Changed "Signed-off-by" to "Suggested-by". [Bob]
v4:
- No modification. Re-sending the patch with base64
encoding to conserve the right line endings. [Bob]
v5:
- No modification. [Pierre]

BaseTools/Conf/build_rule.template | 15 +++-
BaseTools/Source/Python/AmlToC/AmlToC.py | 82 ++++++++------------
2 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template
b/BaseTools/Conf/build_rule.template
index
0822b681fcd9f61c6508e6f93ffc31fa70fd7059..c034869915914936e28f64a6aadb
a08e0169da44 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -419,6 +419,7 @@

<OutputFile>
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli

<ExtraDependency>
$(MAKE_FILE)
@@ -428,14 +429,24 @@
"$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) /I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
"$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
- -AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(CP) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli

<Command.GCC>
Trim --asl-file --asl-deps -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i -i $(INC_LIST) ${src}
"$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) -I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
"$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
- -AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(CP) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli
+
+[Acpi-Machine-Language-File-to-C.DXE_DRIVER]
+ <InputFile>
+ ?.amli
+
+ <OutputFile>
+ ${s_path}(+)${s_base}.c
+
+ <Command>
+ -AmlToC ${src}

[C-Code-File.AcpiTable]
<InputFile>
diff --git a/BaseTools/Source/Python/AmlToC/AmlToC.py
b/BaseTools/Source/Python/AmlToC/AmlToC.py
index
643db2910e37acfdd80ac18d288c921320a79ce1..346de7159de702d860bbd809ddbe
8175f1493cfb 100644
--- a/BaseTools/Source/Python/AmlToC/AmlToC.py
+++ b/BaseTools/Source/Python/AmlToC/AmlToC.py
@@ -1,9 +1,9 @@
## @file
#
-# Convert an AML file to a .hex file containing the AML bytecode
stored in a
+# Convert an AML file to a .c file containing the AML bytecode stored
+in a
# C array.
-# By default, "Tables\Dsdt.aml" will generate "Tables\Dsdt.hex".
-# "Tables\Dsdt.hex" will contain a C array named "dsdt_aml_code" that
contains
+# By default, "Tables\Dsdt.aml" will generate "Tables\Dsdt.c".
+# "Tables\Dsdt.c" will contain a C array named "dsdt_aml_code" that
+contains
# the AML bytecode.
#
# Copyright (c) 2020, ARM Limited. All rights reserved.<BR> @@ -17,31
+17,26 @@ from Common.BuildToolError import * import sys import os

+__description__ = """
+Convert an AML file to a .c file containing the AML bytecode stored
+in a C array. By default, Tables\Dsdt.aml will generate Tables\Dsdt.c.
+Tables\Dsdt.c will contain a C array named "dsdt_aml_code" that
+contains the AML bytecode.
+"""
+
## Parse the command line arguments.
#
# @retval A argparse.NameSpace instance, containing parsed values.
#
def ParseArgs():
# Initialize the parser.
- Parser = argparse.ArgumentParser(
- description="Convert an AML file to a .hex file containing the AML " + \
- "bytecode stored in a C array. By default, " + \
- "\"Tables\\Dsdt.aml\" will generate" + \
- "\"Tables\\Dsdt.hex\". \"Tables\\Dsdt.hex\" will " + \
- "contain a C array named \"dsdt_aml_code\" that " + \
- "contains the AML bytecode."
- )
+ Parser = argparse.ArgumentParser(description=__description__)

# Define the possible arguments.
- Parser.add_argument(
- dest="InputFile",
- help="Path to an input AML file to generate a .hex file from."
- )
- Parser.add_argument(
- "-o", "--out-dir", dest="OutDir",
- help="Output directory where the .hex file will be generated. " + \
- "Default is the input file's directory."
- )
+ Parser.add_argument(dest="InputFile",
+ help="Path to an input AML file to generate a .c file from.")
+ Parser.add_argument("-o", "--out-dir", dest="OutDir",
+ help="Output directory where the .c file will
+ be generated. Default is the input file's directory.")

# Parse the input arguments.
Args = Parser.parse_args()
@@ -55,9 +50,7 @@ def ParseArgs():
with open(Args.InputFile, "rb") as fIn:
Signature = str(fIn.read(4))
if ("DSDT" not in Signature) and ("SSDT" not in Signature):
- EdkLogger.info("Invalid file type. " + \
- "File does not have a valid " + \
- "DSDT or SSDT signature: %s" % Args.InputFile)
+ EdkLogger.info("Invalid file type. File does not have
+ a valid DSDT or SSDT signature: {}".format(Args.InputFile))
return None

# Get the basename of the input file.
@@ -66,42 +59,39 @@ def ParseArgs():

# If no output directory is specified, output to the input directory.
if not Args.OutDir:
- Args.OutputFile = os.path.join(
- os.path.dirname(Args.InputFile),
- BaseName + ".hex"
- )
+ Args.OutputFile = os.path.join(os.path.dirname(Args.InputFile),
+ BaseName + ".c")
else:
if not os.path.exists(Args.OutDir):
os.mkdir(Args.OutDir)
- Args.OutputFile = os.path.join(Args.OutDir, BaseName + ".hex")
+ Args.OutputFile = os.path.join(Args.OutDir, BaseName + ".c")

Args.BaseName = BaseName

return Args

-## Convert an AML file to a .hex file containing the AML bytecode
stored
+## Convert an AML file to a .c file containing the AML bytecode
+stored
# in a C array.
#
# @param InputFile Path to the input AML file.
-# @param OutputFile Path to the output .hex file to generate.
+# @param OutputFile Path to the output .c file to generate.
# @param BaseName Base name of the input file.
-# This is also the name of the generated .hex file.
+# This is also the name of the generated .c file.
#
-def AmlToHex(InputFile, OutputFile, BaseName):
+def AmlToC(InputFile, OutputFile, BaseName):

- MacroName = "__{}_HEX__".format(BaseName.upper())
ArrayName = BaseName.lower() + "_aml_code"
+ FileHeader =\
+"""
+// This file has been generated from:
+// -Python script: {}
+// -Input AML file: {}
+
+"""

with open(InputFile, "rb") as fIn, open(OutputFile, "w") as fOut:
# Write header.
- fOut.write("// This file has been generated from:\n" + \
- "// \tPython script: " + \
- os.path.abspath(__file__) + "\n" + \
- "// \tInput AML file: " + \
- os.path.abspath(InputFile) + "\n\n" + \
- "#ifndef {}\n".format(MacroName) + \
- "#define {}\n\n".format(MacroName)
- )
+ fOut.write(FileHeader.format(os.path.abspath(InputFile),
+ os.path.abspath(__file__)))

# Write the array and its content.
fOut.write("unsigned char {}[] = {{\n ".format(ArrayName))
@@ -115,15 +105,12 @@ def AmlToHex(InputFile, OutputFile, BaseName):
byte = fIn.read(1)
fOut.write("\n};\n")

- # Write footer.
- fOut.write("#endif // {}\n".format(MacroName))
-
## Main method
#
# This method:
# 1- Initialize an EdkLogger instance.
# 2- Parses the input arguments.
-# 3- Converts an AML file to a .hex file containing the AML bytecode stored
+# 3- Converts an AML file to a .c file containing the AML bytecode stored
# in a C array.
#
# @retval 0 Success.
@@ -139,10 +126,9 @@ def Main():
if not CommandArguments:
return 1

- # Convert an AML file to a .hex file containing the AML bytecode stored
+ # Convert an AML file to a .c file containing the AML
+ bytecode stored
# in a C array.
- AmlToHex(CommandArguments.InputFile, CommandArguments.OutputFile,
- CommandArguments.BaseName)
+ AmlToC(CommandArguments.InputFile,
+ CommandArguments.OutputFile, CommandArguments.BaseName)
except Exception as e:
print(e)
return 1
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

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 v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

Leif Lindholm
 

Hi Pierre, (+Masahisa)

This commit (0a4aa20e8d44) made for an exciting start to my week.

Socionext's Developerbox failed to build for me, with the spectacular
error message:

/usr/lib/gcc-cross/aarch64-linux-gnu/8/../../../../aarch64-linux-gnu/bin/ld:
DWARF error: could not find abbrev number 5912
/tmp/ccKt4gaM.ltrans0.ltrans.o: in function `RegisterDevices':
<artificial>:(.text.RegisterDevices+0xb0): undefined reference to
`RegisterEmmc'

GCC49 (without lto) and CLANG38 profiles give much the same result,
with slightly less esoteric messages.

The reason for this turned out to be that edk2-platforms
Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/ has both an Emmc.asl
and an Emmc.c file, which after this patch both generate an Emmc.obj
in the same output directory.

I think the correct course of action is to fix this in the SynQuacer
driver, but I am reporting it here so we get it logged in the list
archives.

It would of course be good if the build system could detect and warn
over cases like this, rather than silently overwriting existing object
files.

Masahisa - since Ard is still on holiday, could you create a patch and
send out for me to review? Either one of the files needs to be
renamed, or we need to move the .asl files (Emmc.asl and Optee.asl)
into a subdirectory.

Best Regards,

Leif

On Wed, Jul 01, 2020 at 15:06:03 +0100, PierreGondois wrote:
From: Pierre Gondois <pierre.gondois@...>

The AmlToHex script and Posix/WindowsLike wrappers convert
an AML file to a .hex file, containing a C array storing
AML bytecode. This ".hex" file can then be included in a
C file, allowing to access the AML bytecode from this C
file.

The EDK2 build system doesn't allow to a depict dependency
orders between files of different languages. For instance,
in a module containing a ".c" file and a ".asl", the ".c"
file may or may not be built prior to the ".asl" file.
This prevents any inclusion of a generated ".hex" in a
".c" file since this later ".hex" file may or may not
have been created yet.

This patch modifies the AmlToC script to generate a C file
instead of a ".hex" file.
It also adds the generation of an intermediate ".amli" file
when compiling an ASL file, and adds a rule to convert this
".amli" to a C file.

This allows to generate a C file containing the AML bytecode
from an ASL file. This C file will then be handled by the EDK2
build system to generate an object file.
Thus, no file inclusion will be required anymore. The C file
requiring the AML bytecode as a C array, and the ASL file,
will be compiled independently. The C array must be defined
as an external symbol. The linker is resolving the
reference to the C array symbol.

To summarize, the flow goes as:
-1. ASL file is compiled to AML;
-2. AML file is copied to a ".amli" intermediate file;
-3. EDK2 build system applies the rule relevant to ".amli"
files. This is, calling the "AmlToC" script, generating
a C file from the ".amli" file;
-4. EDK2 build system applies the rule relevant to C files.
This is creating an object file.
-5. EDK2 build system links the object file containing the
AML bytecode with the object file requiring it.

Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
Suggested-by: Tomas Pilar <Tomas.Pilar@...>
---

The changes can be seen at: https://github.com/PierreARM/edk2/commits/803_Compile_AML_bytecode_array_into_OBJ_file_v5

Notes:
v1:
- Add a new rule to the build_rule.template file to
generate ".obj" files from .asl files, and modify
the AmlToC script accordingly. [Pierre]
v2:
- Restrict the rule to DXE_DRIVER. This allows to build
the OvmfPkg, which was not the case in v1. [Pierre]
v3:
- Changed "Signed-off-by" to "Suggested-by". [Bob]
v4:
- No modification. Re-sending the patch with base64
encoding to conserve the right line endings. [Bob]
v5:
- No modification. [Pierre]

BaseTools/Conf/build_rule.template | 15 +++-
BaseTools/Source/Python/AmlToC/AmlToC.py | 82 ++++++++------------
2 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template
index 0822b681fcd9f61c6508e6f93ffc31fa70fd7059..c034869915914936e28f64a6aadba08e0169da44 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -419,6 +419,7 @@

<OutputFile>
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli

<ExtraDependency>
$(MAKE_FILE)
@@ -428,14 +429,24 @@
"$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) /I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
"$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
- -AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(CP) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli

<Command.GCC>
Trim --asl-file --asl-deps -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i -i $(INC_LIST) ${src}
"$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) -I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
"$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
- -AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
+ $(CP) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.amli
+
+[Acpi-Machine-Language-File-to-C.DXE_DRIVER]
+ <InputFile>
+ ?.amli
+
+ <OutputFile>
+ ${s_path}(+)${s_base}.c
+
+ <Command>
+ -AmlToC ${src}

[C-Code-File.AcpiTable]
<InputFile>
diff --git a/BaseTools/Source/Python/AmlToC/AmlToC.py b/BaseTools/Source/Python/AmlToC/AmlToC.py
index 643db2910e37acfdd80ac18d288c921320a79ce1..346de7159de702d860bbd809ddbe8175f1493cfb 100644
--- a/BaseTools/Source/Python/AmlToC/AmlToC.py
+++ b/BaseTools/Source/Python/AmlToC/AmlToC.py
@@ -1,9 +1,9 @@
## @file
#
-# Convert an AML file to a .hex file containing the AML bytecode stored in a
+# Convert an AML file to a .c file containing the AML bytecode stored in a
# C array.
-# By default, "Tables\Dsdt.aml" will generate "Tables\Dsdt.hex".
-# "Tables\Dsdt.hex" will contain a C array named "dsdt_aml_code" that contains
+# By default, "Tables\Dsdt.aml" will generate "Tables\Dsdt.c".
+# "Tables\Dsdt.c" will contain a C array named "dsdt_aml_code" that contains
# the AML bytecode.
#
# Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
@@ -17,31 +17,26 @@ from Common.BuildToolError import *
import sys
import os

+__description__ = """
+Convert an AML file to a .c file containing the AML bytecode stored in a C
+array. By default, Tables\Dsdt.aml will generate Tables\Dsdt.c.
+Tables\Dsdt.c will contain a C array named "dsdt_aml_code" that contains
+the AML bytecode.
+"""
+
## Parse the command line arguments.
#
# @retval A argparse.NameSpace instance, containing parsed values.
#
def ParseArgs():
# Initialize the parser.
- Parser = argparse.ArgumentParser(
- description="Convert an AML file to a .hex file containing the AML " + \
- "bytecode stored in a C array. By default, " + \
- "\"Tables\\Dsdt.aml\" will generate" + \
- "\"Tables\\Dsdt.hex\". \"Tables\\Dsdt.hex\" will " + \
- "contain a C array named \"dsdt_aml_code\" that " + \
- "contains the AML bytecode."
- )
+ Parser = argparse.ArgumentParser(description=__description__)

# Define the possible arguments.
- Parser.add_argument(
- dest="InputFile",
- help="Path to an input AML file to generate a .hex file from."
- )
- Parser.add_argument(
- "-o", "--out-dir", dest="OutDir",
- help="Output directory where the .hex file will be generated. " + \
- "Default is the input file's directory."
- )
+ Parser.add_argument(dest="InputFile",
+ help="Path to an input AML file to generate a .c file from.")
+ Parser.add_argument("-o", "--out-dir", dest="OutDir",
+ help="Output directory where the .c file will be generated. Default is the input file's directory.")

# Parse the input arguments.
Args = Parser.parse_args()
@@ -55,9 +50,7 @@ def ParseArgs():
with open(Args.InputFile, "rb") as fIn:
Signature = str(fIn.read(4))
if ("DSDT" not in Signature) and ("SSDT" not in Signature):
- EdkLogger.info("Invalid file type. " + \
- "File does not have a valid " + \
- "DSDT or SSDT signature: %s" % Args.InputFile)
+ EdkLogger.info("Invalid file type. File does not have a valid DSDT or SSDT signature: {}".format(Args.InputFile))
return None

# Get the basename of the input file.
@@ -66,42 +59,39 @@ def ParseArgs():

# If no output directory is specified, output to the input directory.
if not Args.OutDir:
- Args.OutputFile = os.path.join(
- os.path.dirname(Args.InputFile),
- BaseName + ".hex"
- )
+ Args.OutputFile = os.path.join(os.path.dirname(Args.InputFile),
+ BaseName + ".c")
else:
if not os.path.exists(Args.OutDir):
os.mkdir(Args.OutDir)
- Args.OutputFile = os.path.join(Args.OutDir, BaseName + ".hex")
+ Args.OutputFile = os.path.join(Args.OutDir, BaseName + ".c")

Args.BaseName = BaseName

return Args

-## Convert an AML file to a .hex file containing the AML bytecode stored
+## Convert an AML file to a .c file containing the AML bytecode stored
# in a C array.
#
# @param InputFile Path to the input AML file.
-# @param OutputFile Path to the output .hex file to generate.
+# @param OutputFile Path to the output .c file to generate.
# @param BaseName Base name of the input file.
-# This is also the name of the generated .hex file.
+# This is also the name of the generated .c file.
#
-def AmlToHex(InputFile, OutputFile, BaseName):
+def AmlToC(InputFile, OutputFile, BaseName):

- MacroName = "__{}_HEX__".format(BaseName.upper())
ArrayName = BaseName.lower() + "_aml_code"
+ FileHeader =\
+"""
+// This file has been generated from:
+// -Python script: {}
+// -Input AML file: {}
+
+"""

with open(InputFile, "rb") as fIn, open(OutputFile, "w") as fOut:
# Write header.
- fOut.write("// This file has been generated from:\n" + \
- "// \tPython script: " + \
- os.path.abspath(__file__) + "\n" + \
- "// \tInput AML file: " + \
- os.path.abspath(InputFile) + "\n\n" + \
- "#ifndef {}\n".format(MacroName) + \
- "#define {}\n\n".format(MacroName)
- )
+ fOut.write(FileHeader.format(os.path.abspath(InputFile), os.path.abspath(__file__)))

# Write the array and its content.
fOut.write("unsigned char {}[] = {{\n ".format(ArrayName))
@@ -115,15 +105,12 @@ def AmlToHex(InputFile, OutputFile, BaseName):
byte = fIn.read(1)
fOut.write("\n};\n")

- # Write footer.
- fOut.write("#endif // {}\n".format(MacroName))
-
## Main method
#
# This method:
# 1- Initialize an EdkLogger instance.
# 2- Parses the input arguments.
-# 3- Converts an AML file to a .hex file containing the AML bytecode stored
+# 3- Converts an AML file to a .c file containing the AML bytecode stored
# in a C array.
#
# @retval 0 Success.
@@ -139,10 +126,9 @@ def Main():
if not CommandArguments:
return 1

- # Convert an AML file to a .hex file containing the AML bytecode stored
+ # Convert an AML file to a .c file containing the AML bytecode stored
# in a C array.
- AmlToHex(CommandArguments.InputFile, CommandArguments.OutputFile,
- CommandArguments.BaseName)
+ AmlToC(CommandArguments.InputFile, CommandArguments.OutputFile, CommandArguments.BaseName)
except Exception as e:
print(e)
return 1
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


Re: [edk2-platforms] [PATCH v3 0/2] Add USB driver support

Meenakshi Aggarwal (OSS) <meenakshi.aggarwal@...>
 

Thanks Leif,

We will take care of same in future patches.

-----Original Message-----
From: Leif Lindholm <leif@...>
Sent: Friday, July 24, 2020 10:17 PM
To: Meenakshi Aggarwal (OSS) <meenakshi.aggarwal@...>
Cc: ard.biesheuvel@...; michael.d.kinney@...;
devel@edk2.groups.io; Varun Sethi <V.Sethi@...>
Subject: Re: [edk2-platforms] [PATCH v3 0/2] Add USB driver support

On Fri, Jul 10, 2020 at 01:33:54 +0530, Meenakshi Aggarwal wrote:
This patchset implement USB driver for DWC3 controller and enable USB
for LX2160A Platform.

Changes in v2:
- Indentation changes
- Incorporated review comments
- create EndOfDxe event and initialize USB in EndOfDxe
callback function.

Changes in v3:
- Passing Usb initialization function as init function
of RegisterNonDiscoverableMmioDevice()

Meenakshi Aggarwal (2):
Silicon/NXP: Add DWC3 USB controller initialization driver
Platform/NXP:LX2160: Enable support of USB controller
For series:
Reviewed-by: Leif Lindholm <leif@...>

However, in future, please rebase patches onto current master before
sumitting later revisions. This set conflicted with 7c552c1a2611
("Platform/NXP: LX2160aRdbPkg: Enable PlatformDxe driver") which was
pushed on 3 July.

Pushed (together with SATA series previously reviewed) as:
b972f17b329a..7de21b6aa3e0

Silicon/NXP/NxpQoriqLs.dec | 5 +
Silicon/NXP/LX2160A/LX2160A.dsc.inc | 4 +
Silicon/NXP/NxpQoriqLs.dsc.inc | 12 +
Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc | 1 +
Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf | 18 +-
Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf | 45 ++++
Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h | 138 ++++++++++
Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c | 275
++++++++++++++++++++
8 files changed, 495 insertions(+), 3 deletions(-) create mode
100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c

--
1.9.1


[PATCH v2] MdePkg Base.h: Delete prototype for __builtin_return_address

Jessica Clarke <jrtc27@...>
 

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

Being a compiler builtin, the type of __builtin_return_address is
already known to the compiler so no prototype is needed. Clang also
errors out when redeclaring certain builtins like this[1], though
currently only for ones with custom type checking. At the moment,
__builtin_return_address does not use custom type checking and so does
not trigger this error, however, the CHERI fork of LLVM, which will form
the basis of the toolchain for Arm's experimental Morello platform, does
use custom type checking for it, and so gives an error. Thus, simply
delete the unnecessary line.

[1] llvm/llvm-project@41af97137572ad6d4dafc872e7ecf6bbb08d4984

Cc: Leif Lindholm <leif@...>
Signed-off-by: Jessica Clarke <jrtc27@...>
---
Changes in v2:
* Shortened [1] reference to fit column limit. The bug report has the
full URL already, and this will still link correctly on GitHub.

MdePkg/Include/Base.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 85a091b9d5..8e4271f6ea 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -1273,7 +1273,6 @@ typedef UINTN RETURN_STATUS;
**/
#define RETURN_ADDRESS(L) ((L == 0) ? _ReturnAddress() : (VOID *) 0)
#elif defined (__GNUC__) || defined (__clang__)
- void * __builtin_return_address (unsigned int level);
/**
Get the return address of the calling function.

--
2.20.1


Re: [PATCH 10/15] OvmfPkg/OvmfPkg.ci.yaml: Add configuration for LicenseCheck

Laszlo Ersek
 

On 07/27/20 08:21, Zhang, Shenglei wrote:
Hi Laszlo,

VbeShim.h is existing in edk2 now. This plugin only checks the patches to be checked in.
So there's no need to add existing files to this section.
OK, thanks, we can always extend this stanza later, if needed.

Rebecca: once this patch is upstream, please post a separate patch for listing "OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.h" in "IgnoreFiles". Otherwise I won't be able to merge your patch at <https://edk2.groups.io/g/devel/message/62395>.


Shenglei: I have a question regarding IgnoreFiles syntax. In "MdeModulePkg/MdeModulePkg.ci.yaml", there are two syntaxes:

- The IgnoreFiles stanza for "CharEncodingCheck" uses pathnames that are relative to the *project* root:

## options defined ci/Plugin/CharEncodingCheck
"CharEncodingCheck": {
"IgnoreFiles": [
"MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/test/testc.c",
"MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/windows/testc.c"
]
},
- The IgnoreFiles stanza for "SpellCheck" uses pathnames that are relative to the *package* (not project) root:

"SpellCheck": {
...
"IgnoreFiles": [ # use gitignore syntax to ignore errors in matching files
"Library/LzmaCustomDecompressLib/Sdk/DOC/*"
],
How do we know whether a particular check's IgnoreFiles stanza requires project-root-relative or package-root-relative pathnames?

Thanks!
Laszlo

Thanks,
Shenglei

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, July 21, 2020 6:01 AM
To: Zhang, Shenglei <shenglei.zhang@...>; devel@edk2.groups.io
Cc: Justen, Jordan L <jordan.l.justen@...>; Ard Biesheuvel
<ard.biesheuvel@...>
Subject: Re: [PATCH 10/15] OvmfPkg/OvmfPkg.ci.yaml: Add configuration for
LicenseCheck

On 07/20/20 10:37, Shenglei Zhang wrote:
Add configuration IgnoreFiles for package config files.
So users can rely on this to skip license conflict for
some generated files.

Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
OvmfPkg/OvmfPkg.ci.yaml | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/OvmfPkg/OvmfPkg.ci.yaml b/OvmfPkg/OvmfPkg.ci.yaml
index 98992f0429ff..ed342d7a3d08 100644
--- a/OvmfPkg/OvmfPkg.ci.yaml
+++ b/OvmfPkg/OvmfPkg.ci.yaml
@@ -8,6 +8,10 @@
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
{
+ ## options defined .pytool/Plugin/LicenseCheck
+ "LicenseCheck": {
+ "IgnoreFiles": []
+ },
## options defined .pytool/Plugin/CompilerPlugin
"CompilerPlugin": {
"DscPath": "" # Don't support this test
Can you list the following file at once, please:

OvmfPkg/QemuVideoDxe/VbeShim.h

With that:

Reviewed-by: Laszlo Ersek <lersek@...>

Thanks
Laszlo


Re: [PATCH v8 2/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098)

Laszlo Ersek
 

On 07/24/20 11:54, Guomin Jiang wrote:
From: Michael Kubacki <michael.a.kubacki@...>

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

Introduces new changes to PeiCore to move the contents of temporary
RAM visible to the PeiCore to permanent memory. This expands on
pre-existing shadowing support in the PeiCore to perform the following
additional actions:

1. Migrate pointers in PPIs installed in PeiCore to the permanent
memory copy of PeiCore.

2. Copy all installed firmware volumes to permanent memory.

3. Relocate and fix up the PEIMs within the firmware volumes.

4. Convert all PPIs into the migrated firmware volume to the corresponding
PPI address in the permanent memory location.

This applies to PPIs and PEI notifications.

5. Convert all status code callbacks in the migrated firmware volume to
the corresponding address in the permanent memory location.

6. Update the FV HOB to the corresponding firmware volume in permanent
memory.

7. Use PcdMigrateTemporaryRamFirmwareVolumes to control if enable the
feature or not. when disable the PCD, the EvacuateTempRam() will
never be called.

The function control flow as below:
PeiCore()
DumpPpiList()
EvacuateTempRam()
ConvertPeiCorePpiPointers()
ConvertPpiPointersFv()
MigratePeimsInFv()
MigratePeim()
PeiGetPe32Data()
LoadAndRelocatePeCoffImageInPlace()
MigrateSecModulesInFv()
ConvertPpiPointersFv()
ConvertStatusCodeCallbacks()
ConvertFvHob()
RemoveFvHobsInTemporaryMemory()
DumpPpiList()

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <liming.gao@...>
Cc: Debkumar De <debkumar.de@...>
Cc: Harry Han <harry.han@...>
Cc: Catharine West <catharine.west@...>
Signed-off-by: Michael Kubacki <michael.a.kubacki@...>
---
MdeModulePkg/Core/Pei/PeiMain.inf | 2 +
MdeModulePkg/Core/Pei/PeiMain.h | 169 +++++++
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 417 +++++++++++++++++-
MdeModulePkg/Core/Pei/Image/Image.c | 130 +++++-
MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 82 ++++
MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 22 +-
MdeModulePkg/Core/Pei/Ppi/Ppi.c | 286 ++++++++++++
7 files changed, 1099 insertions(+), 9 deletions(-)
Acked-by: Laszlo Ersek <lersek@...>


Re: [PATCH v8 1/9] MdeModulePkg: Add new PCD to control the evacuate temporary memory feature (CVE-2019-11098)

Laszlo Ersek
 

On 07/24/20 11:54, Guomin Jiang wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614

The security researcher found that we can get control after NEM disable.

The reason is that the flash content reside in NEM at startup and the
code will get the content from flash directly after disable NEM.

To avoid this vulnerability, the feature will copy the PEIMs from
temporary memory to permanent memory and only execute the code in
permanent memory.

The vulnerability is exist in physical platform and haven't report in
virtual platform, so the virtual can disable the feature currently.

When enable the PcdMigrateTemporaryRamFirmwareVolumes, always shadow
all PEIMs no matter the condition of PcdShadowPeimOnBoot or
PcdShadowPeimOnS3Boot.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Guomin Jiang <guomin.jiang@...>
---
MdeModulePkg/MdeModulePkg.dec | 9 +++++++++
MdeModulePkg/MdeModulePkg.uni | 6 ++++++
2 files changed, 15 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 843e963ad34b..45874e9c8236 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1220,6 +1220,15 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# @Prompt Shadow Peim and PeiCore on boot
gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot|TRUE|BOOLEAN|0x30001029

+ ## Enable the feature that evacuate temporary memory to permanent memory or not<BR><BR>
+ # Set FALSE as default, if the developer need this feature to avoid this vulnerability, please
+ # enable it to shadow all PEIMs no matter the behavior controled by PcdShadowPeimOnBoot or
+ # PcdShadowPeimOnS3Boot<BR>
+ # TRUE - Evacuate temporary memory, the actions include copy memory, convert PPI pointers and so on.<BR>
+ # FALSE - Do nothing, for example, no copy memory, no convert PPI pointers and so on.<BR>
+ # @Prompt Evacuate temporary memory to permanent memory
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes|FALSE|BOOLEAN|0x3000102A
+
## The mask is used to control memory profile behavior.<BR><BR>
# BIT0 - Enable UEFI memory profile.<BR>
# BIT1 - Enable SMRAM profile.<BR>
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 2007e0596c4f..5235dee561ad 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -214,6 +214,12 @@
"TRUE - Shadow PEIM on S3 boot path after memory is ready.<BR>\n"
"FALSE - Not shadow PEIM on S3 boot path after memory is ready.<BR>"

+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMigrateTemporaryRamFirmwareVolumes_HELP #language en-US "Enable the feature that evacuate temporary memory to permanent memory or not.<BR><BR>\n"
+ "It will allocate page to save the temporary PEIMs resided in NEM(or CAR) to the permanent memory and change all pointers pointed to the NEM(or CAR) to permanent memory.<BR><BR>\n"
+ "After then, there are no pointer pointed to NEM(or CAR) and TOCTOU volnerability can be avoid.<BR><BR>\n"
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMigrateTemporaryRamFirmwareVolumes_PROMPT #language en-US "Enable the feature that evacuate temporary memory to permanent memory or not"
+
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemId_PROMPT #language en-US "Default OEM ID for ACPI table creation"

#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemId_HELP #language en-US "Default OEM ID for ACPI table creation, its length must be 0x6 bytes to follow ACPI specification."
Acked-by: Laszlo Ersek <lersek@...>


Re: [PATCH v4 0/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

Laszlo Ersek
 

Hi Vladimir,

On 07/23/20 22:50, Vladimir Olovyannikov via groups.io wrote:
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@...>
Tested-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Tested-By: Laszlo Ersek <lersek@...>
Cc: Zhichao Gao <zhichao.gao@...>
Cc: Maciej Rabeda <maciej.rabeda@...>
Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Siyuan Fu <siyuan.fu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Liming Gao <liming.gao@...>
Cc: Nd <nd@...>

This patchset introduces an http client utilizing EDK2 HTTP protocol, to
allow fast image downloading from http/https servers.
HTTP download speed is usually faster than tftp.
The client is based on the same approach as tftp dynamic command, and
uses the same UEFI Shell command line parameters. This makes it easy
integrating http into existing UEFI Shell scripts.
Note that to enable HTTP download, feature Pcd
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must be set to TRUE.

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

PATCH v4 changes:
Address comments based on Laszlo's testing:
- Fix .uni help file missing "\r\n" before RETURNVALUES section;
- delete the downloaded file in case of an error, unless
-k ("keep bad") option is provided on command line.

Allow download time measurement in seconds
by providing -m parameter on command line.
(1) "Tested-by" tags cannot be carried forward on a patch if there are
any changes to the patch. If you update a patch and people would like to
have their T-b's on the patch, then they'll have to retest the patch. So
every time you update a patch, please drop the previously given
Tested-by tags from it.


(2) I was about to retest this patch, but I also compared it with the
previous version (v3). I think the "-m" option should not be added, for
now anyway. For two reasons:

- The patch is already large, and I think there has been no ShellPkg
reviewer / maintainer feedback so far. I think we shouldn't make the
patch even larger, with new features. "-m" looks like an addition that
can be done separately, once the core feature is upstream.

(I consider "-k" a bugfix on the other hand. More precisely, I
consider the new behavior *without "-k"* a bugfix. So I certainly
welcome that.)

- The other reason is that the "-m" feature seems to introduce a
TimerLib dependency. Depending on TimerLib in a shell application is
wrong, IMO; in particular because this application / dynamic command
is extremely useful, so some people might easily want to download a
pre-built binary, and run it on their system for whatever purposes.
But TimerLib is platform-dependent, and that would break this binary
portability.

For some more background, please refer to commit 7a141b1306f6
("ShellPkg: remove superfluous TimerLib resolution", 2018-02-13).


... In fact, upon re-reading the above commit, I'm realizing the current
patch could break the "ShellPkg.dsc" build, because the patch
(incorrectly) introduces a TimerLib dependency in a ShellPkg module, but
"ShellPkg.dsc" (correctly) does not resolve the TimerLib class to any
instance.

And it does break:

$ build -a X64 -b NOOPT -t GCC48 -p ShellPkg/ShellPkg.dsc

ShellPkg/ShellPkg.dsc(...): error 4000: Instance of library class [TimerLib] is not found
in [ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf] [X64]
consumed by module [ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf]
One consequence of the above is that this patch would not pass a CI
build.


... Another reason this patch would not pass a CI build is a
"PatchCheck.py" failure:

ShellPkg/DynamicCommand: add HttpDynamicCommand
The commit message format is not valid:
* 'Tested-By' should be 'Tested-by'
* 'Tested-By' should be 'Tested-by'
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
The code passed all checks.
(Side comment: please use the clipboard for cutting and pasting feedback
tags from emails to commit messages. I have a keyboard shortcut for
inserting "Tested-by: Laszlo Ersek <lersek@...>", and so I know
for a fact that I never send an upper-case "By".)

I suggest running a personal CI build on github.com before submitting
the patch to the list (just open a pull request -- if it fails, you'll
get the error reports, and if it succeeds, then the mergify bot will
auto-close the successful personal build, without actually merging the
patch).


In summary, I suggest posting v5:

- with the Tested-by flags dropped (Samer and myself will have to
re-test),

- with the "-k" option preserved, but the "-m" option (and the
associated TimerLib dependency) removed.

Later on, I think "-m" could be added based on gRT->GetTime(), instead
of TimerLib.


Ray, Zhichao, when do you intend to review this patch? It does not make
much sense for Samer and myself to keep testing this patch if you're
going to start the review only later. The review feedback will probably
necessitate updates to the patch, which will invalidate Samer's testing
and mine. We've done a few rounds of testing; the patch certainly
deserves package maintainer review. Once no more changes looked
necessary from the reviewer side, I'd be happy to test the patch again.

Thanks
Laszlo


[PATCH v4 3/3] UefiPayloadPkg: Support variable size MMCONF space

Marcello Sylvester Bauer <marcello.bauer@...>
 

The default size is still 256MiB, but will be overwritten by
UefiPayloadPkg with the real MMCONF size.

e.g.: On embedded AMD platforms the MMCONF window size is usually
only 64MiB.

Fixes crash on platforms not exposing 256 buses.
Tested on:
* AMD Stoney Ridge

Signed-off-by: Patrick Rudolph <patrick.rudolph@...>
Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>
Cc: Patrick Rudolph <patrick.rudolph@...>
Cc: Christian Walter <christian.walter@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Benjamin You <benjamin.you@...>
---
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 1 +
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 1 +
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc b/UefiPayloadPkg/Uefi=
PayloadPkgIa32X64.dsc
index a768a8702c66..162cbf47a83f 100644
--- a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
@@ -363,6 +363,7 @@ [PcdsDynamicDefault]
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|31=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|100=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0=0D
=0D
##########################################################################=
######=0D
#=0D
diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf b/UefiPayloadPkg/=
BlSupportDxe/BlSupportDxe.inf
index 1371d5eb7952..cebc81135565 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
@@ -54,6 +54,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize=0D
=0D
[Depex]=0D
TRUE=0D
diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c b/UefiPayloadPkg/Bl=
SupportDxe/BlSupportDxe.c
index a3974dcc02f8..a746d0581ee3 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
@@ -155,13 +155,15 @@ BlDxeEntryPoint (
}=0D
=0D
//=0D
- // Set PcdPciExpressBaseAddress by HOB info=0D
+ // Set PcdPciExpressBaseAddress and PcdPciExpressBaseSize by HOB info=0D
//=0D
GuidHob =3D GetFirstGuidHob (&gUefiAcpiBoardInfoGuid);=0D
if (GuidHob !=3D NULL) {=0D
AcpiBoardInfo =3D (ACPI_BOARD_INFO *)GET_GUID_HOB_DATA (GuidHob);=0D
Status =3D PcdSet64S (PcdPciExpressBaseAddress, AcpiBoardInfo->PcieBas=
eAddress);=0D
ASSERT_EFI_ERROR (Status);=0D
+ Status =3D PcdSet64S (PcdPciExpressBaseSize, AcpiBoardInfo->PcieBaseSi=
ze);=0D
+ ASSERT_EFI_ERROR (Status);=0D
}=0D
=0D
return EFI_SUCCESS;=0D
--=20
2.27.0


[PATCH v4 2/3] MdePkg/BasePciExpressLib: Support variable size MMCONF

Marcello Sylvester Bauer <marcello.bauer@...>
 

Add support for arbitrary sized MMCONF by introducing a new PCD.

Signed-off-by: Patrick Rudolph <patrick.rudolph@...>
Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>
Cc: Patrick Rudolph <patrick.rudolph@...>
Cc: Christian Walter <christian.walter@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>
---
MdePkg/MdePkg.dec | 4 +
MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf | 6 +-
MdePkg/Include/Library/PciExpressLib.h | 5 +-
MdePkg/Library/BasePciExpressLib/PciExpressLib.c | 216 +++++++++++++=
++++---
4 files changed, 193 insertions(+), 38 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 73f6c2407357..812be75fb3b2 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2274,6 +2274,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynami=
c, PcdsDynamicEx]
# @Prompt PCI Express Base Address.=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000|UINT64|0x00=
00000a=0D
=0D
+ ## This value is used to set the size of PCI express hierarchy. The defa=
ult is 256 MB.=0D
+ # @Prompt PCI Express Base Size.=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0x10000000|UINT64|0x00000=
00f=0D
+=0D
## Default current ISO 639-2 language: English & French.=0D
# @Prompt Default Value of LangCodes Variable.=0D
gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultLangCodes|"engfraengfra"|=
VOID*|0x0000001c=0D
diff --git a/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf b/MdePk=
g/Library/BasePciExpressLib/BasePciExpressLib.inf
index a7edb74cde71..12734b022ac7 100644
--- a/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
+++ b/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
@@ -1,7 +1,7 @@
## @file=0D
-# Instance of PCI Express Library using the 256 MB PCI Express MMIO windo=
w.=0D
+# Instance of PCI Express Library using the variable size PCI Express MMI=
O window.=0D
#=0D
-# PCI Express Library that uses the 256 MB PCI Express MMIO window to per=
form=0D
+# PCI Express Library that uses the variable size PCI Express MMIO window=
to perform=0D
# PCI Configuration cycles. Layers on top of an I/O Library instance.=0D
#=0D
# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>=
=0D
@@ -38,4 +38,4 @@ [LibraryClasses]
=0D
[Pcd]=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ## CONSUMES=0D
-=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize ## CONSUMES=0D
diff --git a/MdePkg/Include/Library/PciExpressLib.h b/MdePkg/Include/Librar=
y/PciExpressLib.h
index 826fdcf7db6c..d78193a0a352 100644
--- a/MdePkg/Include/Library/PciExpressLib.h
+++ b/MdePkg/Include/Library/PciExpressLib.h
@@ -2,8 +2,9 @@
Provides services to access PCI Configuration Space using the MMIO PCI E=
xpress window.=0D
=0D
This library is identical to the PCI Library, except the access method f=
or performing PCI=0D
- configuration cycles must be through the 256 MB PCI Express MMIO window =
whose base address=0D
- is defined by PcdPciExpressBaseAddress.=0D
+ configuration cycles must be through the PCI Express MMIO window whose b=
ase address=0D
+ is defined by PcdPciExpressBaseAddress and size defined by PcdPciExpress=
BaseSize.=0D
+=0D
=0D
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
diff --git a/MdePkg/Library/BasePciExpressLib/PciExpressLib.c b/MdePkg/Libr=
ary/BasePciExpressLib/PciExpressLib.c
index 99a166c3609b..0311ecb3025f 100644
--- a/MdePkg/Library/BasePciExpressLib/PciExpressLib.c
+++ b/MdePkg/Library/BasePciExpressLib/PciExpressLib.c
@@ -22,7 +22,8 @@
=0D
/**=0D
Assert the validity of a PCI address. A valid PCI address should contain=
1's=0D
- only in the low 28 bits.=0D
+ only in the low 28 bits. PcdPciExpressBaseSize limits the size to the re=
al=0D
+ number of PCI busses in this segment.=0D
=0D
@param A The address to validate.=0D
=0D
@@ -79,6 +80,24 @@ GetPciExpressBaseAddress (
return (VOID*)(UINTN) PcdGet64 (PcdPciExpressBaseAddress);=0D
}=0D
=0D
+/**=0D
+ Gets the size of PCI Express.=0D
+=0D
+ This internal functions retrieves PCI Express Base Size via a PCD entry=
=0D
+ PcdPciExpressBaseSize.=0D
+=0D
+ @return The base size of PCI Express.=0D
+=0D
+**/=0D
+STATIC=0D
+UINTN=0D
+PcdPciExpressBaseSize (=0D
+ VOID=0D
+ )=0D
+{=0D
+ return (UINTN) PcdGet64 (PcdPciExpressBaseSize);=0D
+}=0D
+=0D
/**=0D
Reads an 8-bit PCI configuration register.=0D
=0D
@@ -91,7 +110,8 @@ GetPciExpressBaseAddress (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -101,6 +121,9 @@ PciExpressRead8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioRead8 ((UINTN) GetPciExpressBaseAddress () + Address);=0D
}=0D
=0D
@@ -117,7 +140,8 @@ PciExpressRead8 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -128,6 +152,9 @@ PciExpressWrite8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioWrite8 ((UINTN) GetPciExpressBaseAddress () + Address, Value)=
;=0D
}=0D
=0D
@@ -148,7 +175,8 @@ PciExpressWrite8 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -159,6 +187,9 @@ PciExpressOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioOr8 ((UINTN) GetPciExpressBaseAddress () + Address, OrData);=
=0D
}=0D
=0D
@@ -179,7 +210,8 @@ PciExpressOr8 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -190,6 +222,9 @@ PciExpressAnd8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioAnd8 ((UINTN) GetPciExpressBaseAddress () + Address, AndData)=
;=0D
}=0D
=0D
@@ -212,7 +247,8 @@ PciExpressAnd8 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -224,6 +260,9 @@ PciExpressAndThenOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioAndThenOr8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
AndData,=0D
@@ -249,7 +288,9 @@ PciExpressAndThenOr8 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..7.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configuration=
=0D
+ register.=0D
=0D
**/=0D
UINT8=0D
@@ -261,6 +302,9 @@ PciExpressBitFieldRead8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioBitFieldRead8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -289,7 +333,8 @@ PciExpressBitFieldRead8 (
Range 0..7.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -302,6 +347,9 @@ PciExpressBitFieldWrite8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioBitFieldWrite8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -334,7 +382,8 @@ PciExpressBitFieldWrite8 (
Range 0..7.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -347,6 +396,9 @@ PciExpressBitFieldOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioBitFieldOr8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -379,7 +431,8 @@ PciExpressBitFieldOr8 (
Range 0..7.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -392,6 +445,9 @@ PciExpressBitFieldAnd8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioBitFieldAnd8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -428,7 +484,8 @@ PciExpressBitFieldAnd8 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -442,6 +499,9 @@ PciExpressBitFieldAndThenOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) ~0;=0D
+ }=0D
return MmioBitFieldAndThenOr8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -464,7 +524,8 @@ PciExpressBitFieldAndThenOr8 (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -474,6 +535,9 @@ PciExpressRead16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioRead16 ((UINTN) GetPciExpressBaseAddress () + Address);=0D
}=0D
=0D
@@ -491,7 +555,8 @@ PciExpressRead16 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -502,6 +567,9 @@ PciExpressWrite16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioWrite16 ((UINTN) GetPciExpressBaseAddress () + Address, Value=
);=0D
}=0D
=0D
@@ -523,7 +591,8 @@ PciExpressWrite16 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -534,6 +603,9 @@ PciExpressOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioOr16 ((UINTN) GetPciExpressBaseAddress () + Address, OrData);=
=0D
}=0D
=0D
@@ -555,7 +627,8 @@ PciExpressOr16 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -566,6 +639,9 @@ PciExpressAnd16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioAnd16 ((UINTN) GetPciExpressBaseAddress () + Address, AndData=
);=0D
}=0D
=0D
@@ -589,7 +665,8 @@ PciExpressAnd16 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -601,6 +678,9 @@ PciExpressAndThenOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioAndThenOr16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
AndData,=0D
@@ -627,7 +707,9 @@ PciExpressAndThenOr16 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..15.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configurati=
on=0D
+ register.=0D
=0D
**/=0D
UINT16=0D
@@ -639,6 +721,9 @@ PciExpressBitFieldRead16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioBitFieldRead16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -668,7 +753,8 @@ PciExpressBitFieldRead16 (
Range 0..15.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -681,6 +767,9 @@ PciExpressBitFieldWrite16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioBitFieldWrite16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -714,7 +803,8 @@ PciExpressBitFieldWrite16 (
Range 0..15.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -727,6 +817,9 @@ PciExpressBitFieldOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioBitFieldOr16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -760,7 +853,8 @@ PciExpressBitFieldOr16 (
Range 0..15.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -773,6 +867,9 @@ PciExpressBitFieldAnd16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioBitFieldAnd16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -810,7 +907,8 @@ PciExpressBitFieldAnd16 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -824,6 +922,9 @@ PciExpressBitFieldAndThenOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) ~0;=0D
+ }=0D
return MmioBitFieldAndThenOr16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -846,7 +947,8 @@ PciExpressBitFieldAndThenOr16 (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT32=0D
@@ -856,6 +958,9 @@ PciExpressRead32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioRead32 ((UINTN) GetPciExpressBaseAddress () + Address);=0D
}=0D
=0D
@@ -873,7 +978,8 @@ PciExpressRead32 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=
=0D
=0D
**/=0D
UINT32=0D
@@ -884,6 +990,9 @@ PciExpressWrite32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioWrite32 ((UINTN) GetPciExpressBaseAddress () + Address, Value=
);=0D
}=0D
=0D
@@ -905,7 +1014,8 @@ PciExpressWrite32 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -916,6 +1026,9 @@ PciExpressOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioOr32 ((UINTN) GetPciExpressBaseAddress () + Address, OrData);=
=0D
}=0D
=0D
@@ -937,7 +1050,8 @@ PciExpressOr32 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -948,6 +1062,9 @@ PciExpressAnd32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioAnd32 ((UINTN) GetPciExpressBaseAddress () + Address, AndData=
);=0D
}=0D
=0D
@@ -971,7 +1088,8 @@ PciExpressAnd32 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -983,6 +1101,9 @@ PciExpressAndThenOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioAndThenOr32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
AndData,=0D
@@ -1009,7 +1130,9 @@ PciExpressAndThenOr32 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..31.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI=0D
+ configuration register.=0D
=0D
**/=0D
UINT32=0D
@@ -1021,6 +1144,9 @@ PciExpressBitFieldRead32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioBitFieldRead32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1050,7 +1176,8 @@ PciExpressBitFieldRead32 (
Range 0..31.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1063,6 +1190,9 @@ PciExpressBitFieldWrite32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioBitFieldWrite32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1096,7 +1226,8 @@ PciExpressBitFieldWrite32 (
Range 0..31.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1109,6 +1240,9 @@ PciExpressBitFieldOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioBitFieldOr32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1142,7 +1276,8 @@ PciExpressBitFieldOr32 (
Range 0..31.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1155,6 +1290,9 @@ PciExpressBitFieldAnd32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioBitFieldAnd32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1192,7 +1330,8 @@ PciExpressBitFieldAnd32 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1206,6 +1345,9 @@ PciExpressBitFieldAndThenOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) ~0;=0D
+ }=0D
return MmioBitFieldAndThenOr32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1235,7 +1377,8 @@ PciExpressBitFieldAndThenOr32 (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer receiving the data read.=0D
=0D
- @return Size read data from StartAddress.=0D
+ @retval (UINTN)~0 Invalid PCI address.=0D
+ @retval other Size read data from StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1249,6 +1392,9 @@ PciExpressReadBuffer (
UINTN ReturnValue;=0D
=0D
ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
+ if (StartAddress >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINTN) ~0;=0D
+ }=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
if (Size =3D=3D 0) {=0D
@@ -1335,7 +1481,8 @@ PciExpressReadBuffer (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer containing the data to wri=
te.=0D
=0D
- @return Size written to StartAddress.=0D
+ @retval (UINTN)~0 Invalid PCI address.=0D
+ @retval other Size written to StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1349,6 +1496,9 @@ PciExpressWriteBuffer (
UINTN ReturnValue;=0D
=0D
ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
+ if (StartAddress >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINTN) ~0;=0D
+ }=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
if (Size =3D=3D 0) {=0D
--=20
2.27.0


[PATCH v4 1/3] UefiPayloadPkg: Store the size of the MMCONF window

Marcello Sylvester Bauer <marcello.bauer@...>
 

From: Patrick Rudolph <patrick.rudolph@...>

Store the real size of the Pcie Memory Mapped Address Space.
This change is necessary to support variable size of MMCONF spaces.

Signed-off-by: Patrick Rudolph <patrick.rudolph@...>
Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>
Cc: Patrick Rudolph <patrick.rudolph@...>
Cc: Christian Walter <christian.walter@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Guo Dong <guo.dong@...>
Cc: Benjamin You <benjamin.you@...>
---
UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h | 1 +
UefiPayloadPkg/BlSupportPei/BlSupportPei.c | 3 +++
2 files changed, 4 insertions(+)

diff --git a/UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h b/UefiPayloadP=
kg/Include/Guid/AcpiBoardInfoGuid.h
index fe783fe5e14c..043b748ae4a9 100644
--- a/UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h
+++ b/UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h
@@ -24,6 +24,7 @@ typedef struct {
UINT64 PmTimerRegBase;=0D
UINT64 ResetRegAddress;=0D
UINT64 PcieBaseAddress;=0D
+ UINT64 PcieBaseSize;=0D
} ACPI_BOARD_INFO;=0D
=0D
#endif=0D
diff --git a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c b/UefiPayloadPkg/Bl=
SupportPei/BlSupportPei.c
index 22972453117a..a7e99f9ec6de 100644
--- a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
+++ b/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
@@ -240,8 +240,10 @@ Done:
if (MmCfgHdr !=3D NULL) {=0D
MmCfgBase =3D (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BAS=
E_ADDRESS_ALLOCATION_STRUCTURE *)((UINT8*) MmCfgHdr + sizeof (*MmCfgHdr));=
=0D
AcpiBoardInfo->PcieBaseAddress =3D MmCfgBase->BaseAddress;=0D
+ AcpiBoardInfo->PcieBaseSize =3D (MmCfgBase->EndBusNumber + 1 - MmCfgBa=
se->StartBusNumber) * 4096 * 32 * 8;=0D
} else {=0D
AcpiBoardInfo->PcieBaseAddress =3D 0;=0D
+ AcpiBoardInfo->PcieBaseSize =3D 0;=0D
}=0D
DEBUG ((DEBUG_INFO, "PmCtrl Reg 0x%lx\n", AcpiBoardInfo->PmCtrlRegBase=
));=0D
DEBUG ((DEBUG_INFO, "PmTimer Reg 0x%lx\n", AcpiBoardInfo->PmTimerRegBas=
e));=0D
@@ -250,6 +252,7 @@ Done:
DEBUG ((DEBUG_INFO, "PmEvt Reg 0x%lx\n", AcpiBoardInfo->PmEvtBase));=
=0D
DEBUG ((DEBUG_INFO, "PmGpeEn Reg 0x%lx\n", AcpiBoardInfo->PmGpeEnBase))=
;=0D
DEBUG ((DEBUG_INFO, "PcieBaseAddr 0x%lx\n", AcpiBoardInfo->PcieBaseAddre=
ss));=0D
+ DEBUG ((DEBUG_INFO, "PcieBaseSize 0x%lx\n", AcpiBoardInfo->PcieBaseSize)=
);=0D
=0D
//=0D
// Verify values for proper operation=0D
--=20
2.27.0