Date   

Re: [PATCH 0/2] Fix 2 issues in TDVF

Yao, Jiewen
 

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

-----Original Message-----
From: Xu, Min M <min.m.xu@...>
Sent: Tuesday, May 31, 2022 10:31 PM
To: devel@edk2.groups.io
Cc: Xu, Min M <min.m.xu@...>; Aktas, Erdem
<erdemaktas@...>; Gerd Hoffmann <kraxel@...>; James
Bottomley <jejb@...>; Yao, Jiewen <jiewen.yao@...>; Tom
Lendacky <thomas.lendacky@...>
Subject: [PATCH 0/2] Fix 2 issues in TDVF

During the integration test with TDX upstreaming KVM/QEMU there are 2
issues are found. This patch-set are to fix these 2 issue.
- Fix TDVMCALL error in ApRunLoop.nasm
- Search EFI_RESOURCE_MEMORY_UNACCEPTED for Fw hoblist

Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Min Xu <min.m.xu@...>
*** BLURB HERE ***

Min Xu (2):
OvmfPkg: Fix TDVMCALL error in ApRunLoop.nasm
OvmfPkg: Search EFI_RESOURCE_MEMORY_UNACCEPTED for Fw hoblist

OvmfPkg/Library/PeilessStartupLib/Hob.c | 4 +++-
OvmfPkg/TdxDxe/X64/ApRunLoop.nasm | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)

--
2.29.2.windows.2


[PATCH v7 3/6] OvmfPkg/Platform: unfix PcdPciExpressBaseAddress

Gerd Hoffmann
 

Will be set by FdtPciHostBridgeLib, so it can't be an fixed when we
want use that library.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 4 +++-
OvmfPkg/PlatformPei/PlatformPei.inf | 2 +-
OvmfPkg/Library/PlatformInitLib/MemDetect.c | 4 ++--
OvmfPkg/Library/PlatformInitLib/Platform.c | 4 ++--
4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
index d0fab5cc1f4f..d2a0bec43452 100644
--- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
+++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
@@ -54,8 +54,10 @@ [LibraryClasses]
[LibraryClasses.X64]
TdxLib

-[FixedPcd]
+[Pcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+
+[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 00372fa0ebb5..3cd83e6ec3e5 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -95,6 +95,7 @@ [Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr
gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
@@ -118,7 +119,6 @@ [Pcd]
[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 4c1dedf863c3..83a7b6726bb7 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -61,8 +61,8 @@ PlatformQemuUc32BaseInitialization (
// [PcdPciExpressBaseAddress, 4GB) range require a very small number of
// variable MTRRs (preferably 1 or 2).
//
- ASSERT (FixedPcdGet64 (PcdPciExpressBaseAddress) <= MAX_UINT32);
- PlatformInfoHob->Uc32Base = (UINT32)FixedPcdGet64 (PcdPciExpressBaseAddress);
+ ASSERT (PcdGet64 (PcdPciExpressBaseAddress) <= MAX_UINT32);
+ PlatformInfoHob->Uc32Base = (UINT32)PcdGet64 (PcdPciExpressBaseAddress);
return;
}

diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c
index 101074f6100d..60a30a01f3b5 100644
--- a/OvmfPkg/Library/PlatformInitLib/Platform.c
+++ b/OvmfPkg/Library/PlatformInitLib/Platform.c
@@ -154,7 +154,7 @@ PlatformMemMapInitialization (
// The MMCONFIG area is expected to fall between the top of low RAM and
// the base of the 32-bit PCI host aperture.
//
- PciExBarBase = FixedPcdGet64 (PcdPciExpressBaseAddress);
+ PciExBarBase = PcdGet64 (PcdPciExpressBaseAddress);
ASSERT (TopOfLowRam <= PciExBarBase);
ASSERT (PciExBarBase <= MAX_UINT32 - SIZE_256MB);
PciBase = (UINT32)(PciExBarBase + SIZE_256MB);
@@ -278,7 +278,7 @@ PciExBarInitialization (
// determined in AddressWidthInitialization(), i.e., 36 bits, will suffice
// for DXE's page tables to cover the MMCONFIG area.
//
- PciExBarBase.Uint64 = FixedPcdGet64 (PcdPciExpressBaseAddress);
+ PciExBarBase.Uint64 = PcdGet64 (PcdPciExpressBaseAddress);
ASSERT ((PciExBarBase.Uint32[1] & MCH_PCIEXBAR_HIGHMASK) == 0);
ASSERT ((PciExBarBase.Uint32[0] & MCH_PCIEXBAR_LOWMASK) == 0);

--
2.36.1


[PATCH v7 6/6] OvmfPkg/Microvm/pcie: add pcie support

Gerd Hoffmann
 

Link in pcie and host bridge bits. Enables support for PCIe in microvm
(qemu-system-x86_64 -M microvm,pcie=on).

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3777
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Microvm/MicrovmX64.dsc | 40 +++++++++++++++++++++-------------
OvmfPkg/Microvm/README | 2 +-
2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index f8fc977cb205..5b150a959c12 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -337,7 +337,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
!endif
UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
- PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+# PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+# PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+# PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf

@@ -354,7 +356,9 @@ [LibraryClasses.common.UEFI_DRIVER]
DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
!endif
UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
- PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+ PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+ PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf

[LibraryClasses.common.DXE_DRIVER]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -376,7 +380,9 @@ [LibraryClasses.common.DXE_DRIVER]
!if $(SOURCE_DEBUG_ENABLE) == TRUE
DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
!endif
- PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+ PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+ PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
@@ -392,7 +398,9 @@ [LibraryClasses.common.UEFI_APPLICATION]
!else
DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
!endif
- PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+ PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+ PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf

[LibraryClasses.common.DXE_SMM_DRIVER]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -413,7 +421,9 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf
!endif
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
- PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+ PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+ PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf

[LibraryClasses.common.SMM_CORE]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -429,7 +439,9 @@ [LibraryClasses.common.SMM_CORE]
!else
DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
!endif
- PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+ PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+ PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf

################################################################################
#
@@ -504,14 +516,6 @@ [PcdsFixedAtBuild]
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
!endif

- # This PCD is used to set the base address of the PCI express hierarchy. It
- # is only consulted when OVMF runs on Q35. In that case it is programmed into
- # the PCIEXBAR register.
- #
- # On Q35 machine types that QEMU intends to support in the long term, QEMU
- # never lets the RAM below 4 GB exceed 2816 MB.
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB0000000
-
!if $(SOURCE_DEBUG_ENABLE) == TRUE
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
!endif
@@ -579,6 +583,12 @@ [PcdsDynamicDefault]
gEfiMdePkgTokenSpaceGuid.PcdFSBClock|1000000000
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0

+ # set PcdPciExpressBaseAddress to MAX_UINT64, which signifies that this
+ # PCD and PcdPciDisableBusEnumeration below have not been assigned yet
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xFFFFFFFFFFFFFFFF
+ gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslation|0x0
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
+
# Set video resolution for text setup.
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
@@ -679,7 +689,7 @@ [Components]
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
<LibraryClasses>
- PciHostBridgeLib|MdeModulePkg/Library/PciHostBridgeLibNull/PciHostBridgeLibNull.inf
+ PciHostBridgeLib|OvmfPkg/Fdt/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
}
diff --git a/OvmfPkg/Microvm/README b/OvmfPkg/Microvm/README
index 540d39f2ec21..813920d92a60 100644
--- a/OvmfPkg/Microvm/README
+++ b/OvmfPkg/Microvm/README
@@ -29,7 +29,7 @@ features
[working] serial console
[working] direct kernel boot
[working] virtio-mmio support
- [in progress] pcie support
+ [working] pcie support

known limitations
-----------------
--
2.36.1


[PATCH v7 4/6] OvmfPkg/Microvm/pcie: no vbeshim please

Gerd Hoffmann
 

Those old windows versions which need the vbeshim hack
will not run on microvm anyway.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/QemuVideoDxe/VbeShim.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c b/OvmfPkg/QemuVideoDxe/VbeShim.c
index 8faa146b6cce..2a048211a823 100644
--- a/OvmfPkg/QemuVideoDxe/VbeShim.c
+++ b/OvmfPkg/QemuVideoDxe/VbeShim.c
@@ -156,6 +156,8 @@ InstallVbeShim (
case INTEL_Q35_MCH_DEVICE_ID:
Pam1Address = DRAMC_REGISTER_Q35 (MCH_PAM1);
break;
+ case MICROVM_PSEUDO_DEVICE_ID:
+ return;
default:
DEBUG ((
DEBUG_ERROR,
--
2.36.1


[PATCH v7 5/6] OvmfPkg/Microvm/pcie: mPhysMemAddressWidth tweak

Gerd Hoffmann
 

microvm places the 64bit mmio space at the end of the physical address
space. So mPhysMemAddressWidth must be correct, otherwise the pci host
bridge setup throws an error because it thinks the 64bit mmio window is
not addressable.

On microvm we can simply use standard cpuid to figure the address width
because the host-phys-bits option (-cpu ${name},host-phys-bits=on) is
forced to be enabled. Side note: For 'pc' and 'q35' this is not the
case for backward compatibility reasons.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Library/PlatformInitLib/MemDetect.c | 41 +++++++++++++++++++++
OvmfPkg/PlatformPei/Platform.c | 2 +-
2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 83a7b6726bb7..c28d7601f87e 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -491,6 +491,42 @@ PlatformGetFirstNonAddress (
return FirstNonAddress;
}

+/*
+ * Use CPUID to figure physical address width. Does *not* work
+ * reliable on qemu. For historical reasons qemu returns phys-bits=40
+ * even in case the host machine supports less than that.
+ *
+ * qemu has a cpu property (host-phys-bits={on,off}) to change that
+ * and make sure guest phys-bits are not larger than host phys-bits.,
+ * but it is off by default. Exception: microvm machine type
+ * hard-wires that property to on.
+ */
+VOID
+EFIAPI
+PlatformAddressWidthFromCpuid (
+ IN OUT EFI_HOB_PLATFORM_INFO *PlatformInfoHob
+ )
+{
+ UINT32 RegEax;
+
+ AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= 0x80000008) {
+ AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
+ PlatformInfoHob->PhysMemAddressWidth = (UINT8)RegEax;
+ } else {
+ PlatformInfoHob->PhysMemAddressWidth = 36;
+ }
+
+ PlatformInfoHob->FirstNonAddress = LShiftU64 (1, PlatformInfoHob->PhysMemAddressWidth);
+
+ DEBUG ((
+ DEBUG_INFO,
+ "%a: cpuid: phys-bits is %d\n",
+ __FUNCTION__,
+ PlatformInfoHob->PhysMemAddressWidth
+ ));
+}
+
/**
Initialize the PhysMemAddressWidth field in PlatformInfoHob based on guest RAM size.
**/
@@ -503,6 +539,11 @@ PlatformAddressWidthInitialization (
UINT64 FirstNonAddress;
UINT8 PhysMemAddressWidth;

+ if (PlatformInfoHob->HostBridgeDevId == 0xffff /* microvm */) {
+ PlatformAddressWidthFromCpuid (PlatformInfoHob);
+ return;
+ }
+
//
// As guest-physical memory size grows, the permanent PEI RAM requirements
// are dominated by the identity-mapping page tables built by the DXE IPL.
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index f006755d5fdb..009db67ee60a 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -357,12 +357,12 @@ InitializePlatform (

S3Verification ();
BootModeInitialization (&mPlatformInfoHob);
- AddressWidthInitialization (&mPlatformInfoHob);

//
// Query Host Bridge DID
//
mPlatformInfoHob.HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
+ AddressWidthInitialization (&mPlatformInfoHob);

MaxCpuCountInitialization (&mPlatformInfoHob);

--
2.36.1


[PATCH v7 0/6] OvmfPkg/Microvm/pcie: add pcie support

Gerd Hoffmann
 

Needs two little tweaks in PCI code because microvm supports mmio only.
Other than that just wire up the existing code (the PCIe host adapter
used by microvm is the same (virtual) hardware used by the arm/aarch64
virtual machines).

v7:
- allow non-existing io address space only in case
there are no io reservations (Mateusz Albecki)

v6:
- codestyle fix (Abner Chang).

v5:
- codestyle (uncrustify) fix.

v4:
- update PciHostBridge check (Abner Chang).

v3:
- rebase to latest master, adapt to PlatformInitLib.
- rework PhysMemAddressWidth handling for microvm.

v2:
- rebase to latest master
- pick up review tags

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3777

Gerd Hoffmann (6):
MdeModulePkg/PciHostBridge: io range is not mandatory
OvmfPkg/FdtPciHostBridgeLib: io range is not mandatory
OvmfPkg/Platform: unfix PcdPciExpressBaseAddress
OvmfPkg/Microvm/pcie: no vbeshim please
OvmfPkg/Microvm/pcie: mPhysMemAddressWidth tweak
OvmfPkg/Microvm/pcie: add pcie support

OvmfPkg/Microvm/MicrovmX64.dsc | 40 ++++++++++-------
.../PlatformInitLib/PlatformInitLib.inf | 4 +-
OvmfPkg/PlatformPei/PlatformPei.inf | 2 +-
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 6 +++
.../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 45 ++++++++++---------
OvmfPkg/Library/PlatformInitLib/MemDetect.c | 45 ++++++++++++++++++-
OvmfPkg/Library/PlatformInitLib/Platform.c | 4 +-
OvmfPkg/PlatformPei/Platform.c | 2 +-
OvmfPkg/QemuVideoDxe/VbeShim.c | 2 +
OvmfPkg/Microvm/README | 2 +-
10 files changed, 107 insertions(+), 45 deletions(-)

--
2.36.1


[PATCH v7 2/6] OvmfPkg/FdtPciHostBridgeLib: io range is not mandatory

Gerd Hoffmann
 

io range is not mandatory according to pcie spec,
so allow host bridges without io address space.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
.../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 45 ++++++++++---------
1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/OvmfPkg/Fdt/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/OvmfPkg/Fdt/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 98828e0b262b..14b41a533e96 100644
--- a/OvmfPkg/Fdt/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/OvmfPkg/Fdt/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -292,13 +292,8 @@ ProcessPciHost (
}
}

- if ((*IoSize == 0) || (*Mmio32Size == 0)) {
- DEBUG ((
- DEBUG_ERROR,
- "%a: %a space empty\n",
- __FUNCTION__,
- (*IoSize == 0) ? "IO" : "MMIO32"
- ));
+ if (*Mmio32Size == 0) {
+ DEBUG ((DEBUG_ERROR, "%a: MMIO32 space empty\n", __FUNCTION__));
return EFI_PROTOCOL_ERROR;
}

@@ -333,13 +328,15 @@ ProcessPciHost (
return Status;
}

- //
- // Map the MMIO window that provides I/O access - the PCI host bridge code
- // is not aware of this translation and so it will only map the I/O view
- // in the GCD I/O map.
- //
- Status = MapGcdMmioSpace (*IoBase + IoTranslation, *IoSize);
- ASSERT_EFI_ERROR (Status);
+ if (*IoSize != 0) {
+ //
+ // Map the MMIO window that provides I/O access - the PCI host bridge code
+ // is not aware of this translation and so it will only map the I/O view
+ // in the GCD I/O map.
+ //
+ Status = MapGcdMmioSpace (*IoBase + IoTranslation, *IoSize);
+ ASSERT_EFI_ERROR (Status);
+ }

return Status;
}
@@ -413,17 +410,21 @@ PciHostBridgeGetRootBridges (

AllocationAttributes = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM;

- Io.Base = IoBase;
- Io.Limit = IoBase + IoSize - 1;
+ if (IoSize != 0) {
+ Io.Base = IoBase;
+ Io.Limit = IoBase + IoSize - 1;
+ } else {
+ Io.Base = MAX_UINT64;
+ Io.Limit = 0;
+ }
+
Mem.Base = Mmio32Base;
Mem.Limit = Mmio32Base + Mmio32Size - 1;

- if (sizeof (UINTN) == sizeof (UINT64)) {
- MemAbove4G.Base = Mmio64Base;
- MemAbove4G.Limit = Mmio64Base + Mmio64Size - 1;
- if (Mmio64Size > 0) {
- AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
- }
+ if ((sizeof (UINTN) == sizeof (UINT64)) && (Mmio64Size != 0)) {
+ MemAbove4G.Base = Mmio64Base;
+ MemAbove4G.Limit = Mmio64Base + Mmio64Size - 1;
+ AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
} else {
//
// UEFI mandates a 1:1 virtual-to-physical mapping, so on a 32-bit
--
2.36.1


[PATCH v7 1/6] MdeModulePkg/PciHostBridge: io range is not mandatory

Gerd Hoffmann
 

io range is not mandatory according to pcie spec, so allow
pcie host bridge configurations without io window in case
there are no io reservations.

Signed-off-by: Gerd Hoffmann <kraxel@...>
Reviewed-by: Ard Biesheuvel <ardb@...>
---
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index b20bcd310ad5..354be6dbb313 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -1085,6 +1085,12 @@ NotifyPhase (
RootBridge->ResAllocNode[Index].Base = BaseAddress;
RootBridge->ResAllocNode[Index].Status = ResAllocated;
DEBUG ((DEBUG_INFO, "Success\n"));
+ } else if ((Index == TypeIo) &&
+ (RootBridge->Io.Base == MAX_UINT64) &&
+ (RootBridge->ResAllocNode[Index].Length == 0))
+ {
+ /* I/O is optional on PCIe */
+ DEBUG ((DEBUG_INFO, "Success (PCIe NoIO)\n"));
} else {
ReturnStatus = EFI_OUT_OF_RESOURCES;
DEBUG ((DEBUG_ERROR, "Out Of Resource!\n"));
--
2.36.1


Re: [PATCH 00/12] Introduce Bootlog DEBUG() output

Gerd Hoffmann
 

Hi,

Hmm. Maybe it's time to tackle the log performance problem for virtual
machines? Create a debug log device with DMA support, so we don't need
a vmexit for every single character we want log?
Of course, that doesn't work for native systems.
Yep. Maybe we should have both ;)

How does that relate to coreboot? coreboot has logging-to-memory too.
Not sure what the state is, there have been discussions on the coreboot
list about changing that from a pure text log to something structed with
timestamps a while back. Don't know whenever this did actually happen.

So, when adding logging-to-memory to edk2 it surely make sense to
coordinate that with coreboot, so we'll have both coreboot and edk2 logs
there in case edk2 runs as coreboot payload.
I'm working on getting a SerialPortLib that logs to CBMEM merged. It's on the list at the moment.
Some comments on DebugLibBootlog:
- SMM-phase logging can be implemented, but I'm not convinced that sharing DXE's buffer is entirely safe. Using SMM communicate could be safer, but would be more complicated. I stopped working on it when the return-on-investment was too low.
DebugLibBootlog supports multiple buffers. So we could have one for
coreboot, one for edk2 pei/dxe, one for edk2 smm, one for shim, one for
grub, ...

take care,
Gerd


Open Source Firmware Conference 2022

Christian Walter
 

Dear EDKII community,

the OSFC is the biggest open-source firmware conference worldwide made by and for developers. After two years we can finally meet in person again. This year's OSFC will take place in Gothenburg, Sweden from September 19th - 21th. We will have a two day conference, followed by a one-day hackathon where we can hack together on hardware and firmware.


Currently the Call for Participation is open, and I’d like to invite everyone in this community to submit talks and present new innovations, updates or problem talks. Additionally, you can already buy early bird tickets, so feel free to check out the homepage https://www.osfc.io for more information.

In the last years we saw multiple contributions from EDKII to the OSFC - and we would love to continue seeing this! So feel free to reach out to me if you have any questions!


Thank you!

Chris


--
Christian Walter
Head of Firmware Development / Cyber Security



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany

Sitz der Gesellschaft: Bochum
Handelsregister: Amtsgericht Bochum, HRB 17519
Geschäftsführung: Sebastian Deutsch, Eray Basar


回复: [edk2-devel] [edk2:PATCH v2] MdePkg/Acpi62: Add type 7 NFIT Platform Capabilities Structure support

gaoliming
 

Miki:
Can you add Platform Capabilities Structure definition together?

Thanks
Liming

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Miki Shindo
发送时间: 2022年5月12日 9:28
收件人: devel@edk2.groups.io
抄送: Michael D Kinney <michael.d.kinney@...>; Liming Gao
<gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>; Ray Ni
<ray.ni@...>
主题: [edk2-devel] [edk2:PATCH v2] MdePkg/Acpi62: Add type 7 NFIT
Platform Capabilities Structure support

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

This commit adds a definition type 7 Platform Capabilities Structure
for NFIT Table Structure Types. The type has been added
since ACPI Specification Version 6.2A.


Signed-off-by: Miki Shindo <miki.shindo@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Ray Ni <ray.ni@...>
Reviewed-by: Ray Ni <ray.ni@...>

---
MdePkg/Include/IndustryStandard/Acpi62.h | 1 +
MdePkg/Include/IndustryStandard/Acpi63.h | 1 +
MdePkg/Include/IndustryStandard/Acpi64.h | 1 +
3 files changed, 3 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Acpi62.h
b/MdePkg/Include/IndustryStandard/Acpi62.h
index 313db63044..aa115d475c 100644
--- a/MdePkg/Include/IndustryStandard/Acpi62.h
+++ b/MdePkg/Include/IndustryStandard/Acpi62.h
@@ -1486,6 +1486,7 @@ typedef struct {
#define
EFI_ACPI_6_2_NFIT_NVDIMM_CONTROL_REGION_STRUCTURE_TYPE
4

#define
EFI_ACPI_6_2_NFIT_NVDIMM_BLOCK_DATA_WINDOW_REGION_STRUCTU
RE_TYPE 5

#define EFI_ACPI_6_2_NFIT_FLUSH_HINT_ADDRESS_STRUCTURE_TYPE
6

+#define EFI_ACPI_6_2_NFIT_PLATFORM_CAPABILITIES_STRUCTURE_TYPE
7



//

// Definition for NFIT Structure Header

diff --git a/MdePkg/Include/IndustryStandard/Acpi63.h
b/MdePkg/Include/IndustryStandard/Acpi63.h
index b1e9d5db5b..a440bdfd48 100644
--- a/MdePkg/Include/IndustryStandard/Acpi63.h
+++ b/MdePkg/Include/IndustryStandard/Acpi63.h
@@ -1450,6 +1450,7 @@ typedef struct {
#define
EFI_ACPI_6_3_NFIT_NVDIMM_CONTROL_REGION_STRUCTURE_TYPE
4

#define
EFI_ACPI_6_3_NFIT_NVDIMM_BLOCK_DATA_WINDOW_REGION_STRUCTU
RE_TYPE 5

#define EFI_ACPI_6_3_NFIT_FLUSH_HINT_ADDRESS_STRUCTURE_TYPE
6

+#define EFI_ACPI_6_3_NFIT_PLATFORM_CAPABILITIES_STRUCTURE_TYPE
7



//

// Definition for NFIT Structure Header

diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h
b/MdePkg/Include/IndustryStandard/Acpi64.h
index 232697f228..88d01761f1 100644
--- a/MdePkg/Include/IndustryStandard/Acpi64.h
+++ b/MdePkg/Include/IndustryStandard/Acpi64.h
@@ -1493,6 +1493,7 @@ typedef struct {
#define
EFI_ACPI_6_4_NFIT_NVDIMM_CONTROL_REGION_STRUCTURE_TYPE
4

#define
EFI_ACPI_6_4_NFIT_NVDIMM_BLOCK_DATA_WINDOW_REGION_STRUCTU
RE_TYPE 5

#define EFI_ACPI_6_4_NFIT_FLUSH_HINT_ADDRESS_STRUCTURE_TYPE
6

+#define EFI_ACPI_6_4_NFIT_PLATFORM_CAPABILITIES_STRUCTURE_TYPE
7



//

// Definition for NFIT Structure Header

--
2.27.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89689): https://edk2.groups.io/g/devel/message/89689
Mute This Topic: https://groups.io/mt/91049816/4905953
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[gaoliming@...]
-=-=-=-=-=-=


回复: [edk2-devel] [PATCH edk2-platforms 0/3] Ext4Pkg: Add ext2/3 support and move crc16/32c to BaseLib

gaoliming
 

Pedro

 Thanks for your enhancement to support ext2/3 file system. Acked-by: Liming Gao <gaoliming@...> for this patch set.

 

Thanks

Liming

发件人: Pedro Falcato <pedro.falcato@...>
发送时间: 202261 5:33
收件人: edk2-devel-groups-io <devel@edk2.groups.io>; Pedro Falcato <pedro.falcato@...>
抄送: Leif Lindholm <leif@...>; Michael D Kinney <michael.d.kinney@...>; Liming Gao <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>
主题: Re: [edk2-devel] [PATCH edk2-platforms 0/3] Ext4Pkg: Add ext2/3 support and move crc16/32c to BaseLib

 

Ping. Please review now that the stable freeze is over.

 

On Wed, May 11, 2022 at 6:42 PM Pedro Falcato via groups.io <pedro.falcato=gmail.com@groups.io> wrote:

Ping. Could someone review these patches?

 

On Mon, Apr 25, 2022 at 6:14 PM Pedro Falcato via groups.io <pedro.falcato=gmail.com@groups.io> wrote:

Ping. If someone could take a look, it would be much appreciated.

 

On Thu, Apr 7, 2022 at 11:01 PM Pedro Falcato <pedro.falcato@...> wrote:

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

Hi all,

This patch-set attempts to address two open feature requests for Ext4Pkg
by adding ext2/3 support (id 3745) and moving crc16-ansi/crc32c to BaseLib (id 3871).

The previous patch-set regarding 3871 attempted to merge the different crc16 implementations
but failed because, contrary to what I thought, there are many, many different CRC16s which
are all slightly different. This one (plus the separate edk2 patch) attempts to just merge
CRC16-ANSI (confusingly, also known as CRC16) into BaseLib.

Since this patch set grew to be considerably different from the original, I didn't mark it
as v2 but rather a separate, new patch-set.

CC'ing the edk2-platforms stewards (as I cannot review my own code) and the CC's of the MdePkg
patch.

Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Pedro Falcato (3):
  Ext4Pkg: Replace the CRC implementations with BaseLib
  Ext4Pkg: Format using uncrustify
  Ext4Pkg: Add ext2/3 support

 Features/Ext4Pkg/Ext4Dxe/BlockGroup.c |  10 +-
 Features/Ext4Pkg/Ext4Dxe/BlockMap.c   | 279 +++++++++++++++++
 Features/Ext4Pkg/Ext4Dxe/Collation.c  |   4 +-
 Features/Ext4Pkg/Ext4Dxe/Crc16.c      |  75 -----
 Features/Ext4Pkg/Ext4Dxe/Crc32c.c     |  84 ------
 Features/Ext4Pkg/Ext4Dxe/Directory.c  |  13 +-
 Features/Ext4Pkg/Ext4Dxe/DiskUtil.c   |   6 +-
 Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h   |  30 +-
 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c    |  95 +++---
 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h    | 417 ++++++++++++++------------
 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf  |   3 +-
 Features/Ext4Pkg/Ext4Dxe/Extents.c    |  27 +-
 Features/Ext4Pkg/Ext4Dxe/File.c       |  19 +-
 Features/Ext4Pkg/Ext4Dxe/Inode.c      |  33 +-
 Features/Ext4Pkg/Ext4Dxe/Partition.c  |  12 +-
 Features/Ext4Pkg/Ext4Dxe/Superblock.c |  20 +-
 16 files changed, 640 insertions(+), 487 deletions(-)
 create mode 100644 Features/Ext4Pkg/Ext4Dxe/BlockMap.c
 delete mode 100644 Features/Ext4Pkg/Ext4Dxe/Crc16.c
 delete mode 100644 Features/Ext4Pkg/Ext4Dxe/Crc32c.c

--
2.35.1



--

Pedro Falcato



--

Pedro Falcato



--

Pedro Falcato


回复: [edk2-devel] Is it possible to inject compiler arguments and library dependencies from the build system?

gaoliming
 

Pedro:

 For the specific platform, you can add the compiler flag into [BuildOptions] in Platform.dsc file, and add NULL|xxxLib.inf into [LibraryClasses] section in Platform.dsc file so that this library is linked to every module.

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pedro Falcato
发送时间: 202261 5:43
收件人: edk2-devel-groups-io <devel@edk2.groups.io>
主题: [edk2-devel] Is it possible to inject compiler arguments and library dependencies from the build system?

 

Hi all,

 

Following https://edk2.groups.io/g/devel/message/89684, I want to add sanitizer support to upstream EDK2. Doing this in a non-intrusive way would mean that I need to force all modules to depend on the UBSAN/ASAN implementation lib and force them to use a specific compile flag. Is there a way to do this using the current build system?

 

Thanks,

Pedro


回复: [edk2-devel] [PATCH 1/1] MdePkg/BaseLib: Add CRC16-ANSI and CRC32c implementations

gaoliming
 

Pedro:

 Sorry for the late response. I am OK to add two new CRC APIs in BaseLib.

 

 For Crc32c, what difference is from current Crc32? Can it reuse current mCrcTable value?

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pedro Falcato
发送时间: 202261 5:34
收件人: edk2-devel-groups-io <devel@edk2.groups.io>; Pedro Falcato <pedro.falcato@...>
抄送: Michael D Kinney <michael.d.kinney@...>; Liming Gao <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>
主题: Re: [edk2-devel] [PATCH 1/1] MdePkg/BaseLib: Add CRC16-ANSI and CRC32c implementations

 

Ping. Please review it now that the freeze is over.

 

Thanks,

Pedro

 

On Wed, May 11, 2022 at 6:44 PM Pedro Falcato via groups.io <pedro.falcato=gmail.com@groups.io> wrote:

Ping. Could someone review it?

 

On Mon, Apr 25, 2022 at 6:13 PM Pedro Falcato via groups.io <pedro.falcato=gmail.com@groups.io> wrote:

Ping. If someone could take a look, it would be much appreciated.

 

On Thu, Apr 7, 2022 at 11:02 PM Pedro Falcato <pedro.falcato@...> wrote:

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

Add the CRC16-ANSI and CRC32C implementations previously found at
Features/Ext4Pkg/Ext4Dxe/Crc{16,32c}.c to BaseLib.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Signed-off-by: Pedro Falcato <pedro.falcato@...>
---
 MdePkg/Include/Library/BaseLib.h  |  35 +++++++-
 MdePkg/Library/BaseLib/CheckSum.c | 144 ++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 6aa0d972186e..f365de9f74a0 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -4503,6 +4503,40 @@ CalculateCrc32 (
   IN  UINTN  Length
   );

+/**
+   Calculates the CRC16-ANSI checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC16-ANSI checksum.
+**/
+UINT16
+EFIAPI
+CalculateCrc16Ansi (
+  IN  CONST VOID  *Buffer,
+  IN  UINTN       Length,
+  IN  UINT16      InitialValue
+  );
+
+/**
+   Calculates the CRC32c checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC32c checksum.
+**/
+UINT32
+EFIAPI
+CalculateCrc32c (
+  IN CONST VOID  *Buffer,
+  IN UINTN       Length,
+  IN UINT32      InitialValue
+  );
+
 //
 // Base Library CPU Functions
 //
@@ -4512,7 +4546,6 @@ CalculateCrc32 (

   @param  Context1        Context1 parameter passed into SwitchStack().
   @param  Context2        Context2 parameter passed into SwitchStack().
-
 **/
 typedef
 VOID
diff --git a/MdePkg/Library/BaseLib/CheckSum.c b/MdePkg/Library/BaseLib/CheckSum.c
index 28dee5ac6a15..b6a076573191 100644
--- a/MdePkg/Library/BaseLib/CheckSum.c
+++ b/MdePkg/Library/BaseLib/CheckSum.c
@@ -3,6 +3,7 @@
   algorithm.

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

 **/
@@ -618,3 +619,146 @@ CalculateCrc32 (

   return Crc ^ 0xffffffff;
 }
+
+GLOBAL_REMOVE_IF_UNREFERENCED STATIC CONST UINT16  mCrc16LookupTable[256] =
+{
+  0x0000, 0xC0C1, 0xC181, 0x0140, 0xC301, 0x03C0, 0x0280, 0xC241,
+  0xC601, 0x06C0, 0x0780, 0xC741, 0x0500, 0xC5C1, 0xC481, 0x0440,
+  0xCC01, 0x0CC0, 0x0D80, 0xCD41, 0x0F00, 0xCFC1, 0xCE81, 0x0E40,
+  0x0A00, 0xCAC1, 0xCB81, 0x0B40, 0xC901, 0x09C0, 0x0880, 0xC841,
+  0xD801, 0x18C0, 0x1980, 0xD941, 0x1B00, 0xDBC1, 0xDA81, 0x1A40,
+  0x1E00, 0xDEC1, 0xDF81, 0x1F40, 0xDD01, 0x1DC0, 0x1C80, 0xDC41,
+  0x1400, 0xD4C1, 0xD581, 0x1540, 0xD701, 0x17C0, 0x1680, 0xD641,
+  0xD201, 0x12C0, 0x1380, 0xD341, 0x1100, 0xD1C1, 0xD081, 0x1040,
+  0xF001, 0x30C0, 0x3180, 0xF141, 0x3300, 0xF3C1, 0xF281, 0x3240,
+  0x3600, 0xF6C1, 0xF781, 0x3740, 0xF501, 0x35C0, 0x3480, 0xF441,
+  0x3C00, 0xFCC1, 0xFD81, 0x3D40, 0xFF01, 0x3FC0, 0x3E80, 0xFE41,
+  0xFA01, 0x3AC0, 0x3B80, 0xFB41, 0x3900, 0xF9C1, 0xF881, 0x3840,
+  0x2800, 0xE8C1, 0xE981, 0x2940, 0xEB01, 0x2BC0, 0x2A80, 0xEA41,
+  0xEE01, 0x2EC0, 0x2F80, 0xEF41, 0x2D00, 0xEDC1, 0xEC81, 0x2C40,
+  0xE401, 0x24C0, 0x2580, 0xE541, 0x2700, 0xE7C1, 0xE681, 0x2640,
+  0x2200, 0xE2C1, 0xE381, 0x2340, 0xE101, 0x21C0, 0x2080, 0xE041,
+  0xA001, 0x60C0, 0x6180, 0xA141, 0x6300, 0xA3C1, 0xA281, 0x6240,
+  0x6600, 0xA6C1, 0xA781, 0x6740, 0xA501, 0x65C0, 0x6480, 0xA441,
+  0x6C00, 0xACC1, 0xAD81, 0x6D40, 0xAF01, 0x6FC0, 0x6E80, 0xAE41,
+  0xAA01, 0x6AC0, 0x6B80, 0xAB41, 0x6900, 0xA9C1, 0xA881, 0x6840,
+  0x7800, 0xB8C1, 0xB981, 0x7940, 0xBB01, 0x7BC0, 0x7A80, 0xBA41,
+  0xBE01, 0x7EC0, 0x7F80, 0xBF41, 0x7D00, 0xBDC1, 0xBC81, 0x7C40,
+  0xB401, 0x74C0, 0x7580, 0xB541, 0x7700, 0xB7C1, 0xB681, 0x7640,
+  0x7200, 0xB2C1, 0xB381, 0x7340, 0xB101, 0x71C0, 0x7080, 0xB041,
+  0x5000, 0x90C1, 0x9181, 0x5140, 0x9301, 0x53C0, 0x5280, 0x9241,
+  0x9601, 0x56C0, 0x5780, 0x9741, 0x5500, 0x95C1, 0x9481, 0x5440,
+  0x9C01, 0x5CC0, 0x5D80, 0x9D41, 0x5F00, 0x9FC1, 0x9E81, 0x5E40,
+  0x5A00, 0x9AC1, 0x9B81, 0x5B40, 0x9901, 0x59C0, 0x5880, 0x9841,
+  0x8801, 0x48C0, 0x4980, 0x8941, 0x4B00, 0x8BC1, 0x8A81, 0x4A40,
+  0x4E00, 0x8EC1, 0x8F81, 0x4F40, 0x8D01, 0x4DC0, 0x4C80, 0x8C41,
+  0x4400, 0x84C1, 0x8581, 0x4540, 0x8701, 0x47C0, 0x4680, 0x8641,
+  0x8201, 0x42C0, 0x4380, 0x8341, 0x4100, 0x81C1, 0x8081, 0x4040
+};
+
+/**
+   Calculates the CRC16-ANSI checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC16-ANSI checksum.
+**/
+UINT16
+EFIAPI
+CalculateCrc16Ansi (
+  IN CONST VOID  *Buffer,
+  IN UINTN       Length,
+  IN UINT16      InitialValue
+  )
+{
+  CONST UINT8  *Buf;
+  UINT16       Crc;
+
+  Buf = Buffer;
+
+  Crc = ~InitialValue;
+
+  while (Length-- != 0) {
+    Crc = mCrc16LookupTable[(Crc & 0xFF) ^ *(Buf++)] ^ (Crc >> 8);
+  }
+
+  return ~Crc;
+}
+
+GLOBAL_REMOVE_IF_UNREFERENCED STATIC CONST UINT32  mCrc32cLookupTable[256] = {
+  0x00000000, 0xf26b8303, 0xe13b70f7, 0x1350f3f4, 0xc79a971f, 0x35f1141c,
+  0x26a1e7e8, 0xd4ca64eb, 0x8ad958cf, 0x78b2dbcc, 0x6be22838, 0x9989ab3b,
+  0x4d43cfd0, 0xbf284cd3, 0xac78bf27, 0x5e133c24, 0x105ec76f, 0xe235446c,
+  0xf165b798, 0x030e349b, 0xd7c45070, 0x25afd373, 0x36ff2087, 0xc494a384,
+  0x9a879fa0, 0x68ec1ca3, 0x7bbcef57, 0x89d76c54, 0x5d1d08bf, 0xaf768bbc,
+  0xbc267848, 0x4e4dfb4b, 0x20bd8ede, 0xd2d60ddd, 0xc186fe29, 0x33ed7d2a,
+  0xe72719c1, 0x154c9ac2, 0x061c6936, 0xf477ea35, 0xaa64d611, 0x580f5512,
+  0x4b5fa6e6, 0xb93425e5, 0x6dfe410e, 0x9f95c20d, 0x8cc531f9, 0x7eaeb2fa,
+  0x30e349b1, 0xc288cab2, 0xd1d83946, 0x23b3ba45, 0xf779deae, 0x05125dad,
+  0x1642ae59, 0xe4292d5a, 0xba3a117e, 0x4851927d, 0x5b016189, 0xa96ae28a,
+  0x7da08661, 0x8fcb0562, 0x9c9bf696, 0x6ef07595, 0x417b1dbc, 0xb3109ebf,
+  0xa0406d4b, 0x522bee48, 0x86e18aa3, 0x748a09a0, 0x67dafa54, 0x95b17957,
+  0xcba24573, 0x39c9c670, 0x2a993584, 0xd8f2b687, 0x0c38d26c, 0xfe53516f,
+  0xed03a29b, 0x1f682198, 0x5125dad3, 0xa34e59d0, 0xb01eaa24, 0x42752927,
+  0x96bf4dcc, 0x64d4cecf, 0x77843d3b, 0x85efbe38, 0xdbfc821c, 0x2997011f,
+  0x3ac7f2eb, 0xc8ac71e8, 0x1c661503, 0xee0d9600, 0xfd5d65f4, 0x0f36e6f7,
+  0x61c69362, 0x93ad1061, 0x80fde395, 0x72966096, 0xa65c047d, 0x5437877e,
+  0x4767748a, 0xb50cf789, 0xeb1fcbad, 0x197448ae, 0x0a24bb5a, 0xf84f3859,
+  0x2c855cb2, 0xdeeedfb1, 0xcdbe2c45, 0x3fd5af46, 0x7198540d, 0x83f3d70e,
+  0x90a324fa, 0x62c8a7f9, 0xb602c312, 0x44694011, 0x5739b3e5, 0xa55230e6,
+  0xfb410cc2, 0x092a8fc1, 0x1a7a7c35, 0xe811ff36, 0x3cdb9bdd, 0xceb018de,
+  0xdde0eb2a, 0x2f8b6829, 0x82f63b78, 0x709db87b, 0x63cd4b8f, 0x91a6c88c,
+  0x456cac67, 0xb7072f64, 0xa457dc90, 0x563c5f93, 0x082f63b7, 0xfa44e0b4,
+  0xe9141340, 0x1b7f9043, 0xcfb5f4a8, 0x3dde77ab, 0x2e8e845f, 0xdce5075c,
+  0x92a8fc17, 0x60c37f14, 0x73938ce0, 0x81f80fe3, 0x55326b08, 0xa759e80b,
+  0xb4091bff, 0x466298fc, 0x1871a4d8, 0xea1a27db, 0xf94ad42f, 0x0b21572c,
+  0xdfeb33c7, 0x2d80b0c4, 0x3ed04330, 0xccbbc033, 0xa24bb5a6, 0x502036a5,
+  0x4370c551, 0xb11b4652, 0x65d122b9, 0x97baa1ba, 0x84ea524e, 0x7681d14d,
+  0x2892ed69, 0xdaf96e6a, 0xc9a99d9e, 0x3bc21e9d, 0xef087a76, 0x1d63f975,
+  0x0e330a81, 0xfc588982, 0xb21572c9, 0x407ef1ca, 0x532e023e, 0xa145813d,
+  0x758fe5d6, 0x87e466d5, 0x94b49521, 0x66df1622, 0x38cc2a06, 0xcaa7a905,
+  0xd9f75af1, 0x2b9cd9f2, 0xff56bd19, 0x0d3d3e1a, 0x1e6dcdee, 0xec064eed,
+  0xc38d26c4, 0x31e6a5c7, 0x22b65633, 0xd0ddd530, 0x0417b1db, 0xf67c32d8,
+  0xe52cc12c, 0x1747422f, 0x49547e0b, 0xbb3ffd08, 0xa86f0efc, 0x5a048dff,
+  0x8ecee914, 0x7ca56a17, 0x6ff599e3, 0x9d9e1ae0, 0xd3d3e1ab, 0x21b862a8,
+  0x32e8915c, 0xc083125f, 0x144976b4, 0xe622f5b7, 0xf5720643, 0x07198540,
+  0x590ab964, 0xab613a67, 0xb831c993, 0x4a5a4a90, 0x9e902e7b, 0x6cfbad78,
+  0x7fab5e8c, 0x8dc0dd8f, 0xe330a81a, 0x115b2b19, 0x020bd8ed, 0xf0605bee,
+  0x24aa3f05, 0xd6c1bc06, 0xc5914ff2, 0x37faccf1, 0x69e9f0d5, 0x9b8273d6,
+  0x88d28022, 0x7ab90321, 0xae7367ca, 0x5c18e4c9, 0x4f48173d, 0xbd23943e,
+  0xf36e6f75, 0x0105ec76, 0x12551f82, 0xe03e9c81, 0x34f4f86a, 0xc69f7b69,
+  0xd5cf889d, 0x27a40b9e, 0x79b737ba, 0x8bdcb4b9, 0x988c474d, 0x6ae7c44e,
+  0xbe2da0a5, 0x4c4623a6, 0x5f16d052, 0xad7d5351
+};
+
+/**
+   Calculates the CRC32c checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC32c checksum.
+**/
+UINT32
+EFIAPI
+CalculateCrc32c (
+  IN CONST VOID  *Buffer,
+  IN UINTN       Length,
+  IN UINT32      InitialValue
+  )
+{
+  CONST UINT8  *Buf;
+  UINT32       Crc;
+
+  Buf = Buffer;
+  Crc = ~InitialValue;
+
+  while (Length-- != 0) {
+    Crc = mCrc32cLookupTable[(Crc & 0xFF) ^ *(Buf++)] ^ (Crc >> 8);
+  }
+
+  return ~Crc;
+}
--
2.35.1



--

Pedro Falcato



--

Pedro Falcato



--

Pedro Falcato


回复: [edk2-devel] [PATCH 1/1] MdeModulePkg: Fix a typo

gaoliming
 

Shuzhen:

 The patch title can be MdeModulePkg DxeMain: Fix the typo in local variable name.

 

  With this change, Reviewed-by: Liming Gao <gaoliming@...>

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 zhangshuzhen@...
发送时间: 202261 10:21
收件人: devel@edk2.groups.io
主题: [edk2-devel] [PATCH 1/1] MdeModulePkg: Fix a typo

 

Correctly write 'DstBufAllocated' in CoreLoadPeImage().

Signed-off-by: Zhang Shuzhen <zhangshuzhen@...>
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 68bde5c15c..5bdf96b52c 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -581,7 +581,7 @@ CoreLoadPeImage (
   )
 {
   EFI_STATUS  Status;
-  BOOLEAN     DstBufAlocated;
+  BOOLEAN     DstBufAllocated;
   UINTN       Size;
 
   ZeroMem (&Image->ImageContext, sizeof (Image->ImageContext));
@@ -636,7 +636,7 @@ CoreLoadPeImage (
   //
   // Allocate memory of the correct memory type aligned on the required image boundary
   //
-  DstBufAlocated = FALSE;
+  DstBufAllocated = FALSE;
   if (DstBuffer == 0) {
     //
     // Allocate Destination Buffer as caller did not pass it in
@@ -702,7 +702,7 @@ CoreLoadPeImage (
       return Status;
     }
 
-    DstBufAlocated = TRUE;
+    DstBufAllocated = TRUE;
   } else {
     //
     // Caller provided the destination buffer
@@ -884,7 +884,7 @@ Done:
   // Free memory.
   //
 
-  if (DstBufAlocated) {
+  if (DstBufAllocated) {
     CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages);
     Image->ImageContext.ImageAddress = 0;
     Image->ImageBasePage             = 0;
--
2.36.1


回复: [PATCH v3 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.

gaoliming
 

Reviewed-by: Liming Gao <gaoliming@...>

One minor comment: the change in MdeModulePkg is not required, because this library instance has been specified in MdeLibs.dsc.inc

Thanks
Liming

-----邮件原件-----
发件人: zhihaoli <zhihao.li@...>
发送时间: 2022年6月1日 0:27
收件人: devel@edk2.groups.io
抄送: Jian J Wang <jian.j.wang@...>; Liming Gao
<gaoliming@...>; Ni Ray <ray.ni@...>
主题: [PATCH v3 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in
VariableSmm driver.

From: Zhihao Li <zhihao.li@...>

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

In UefiCpuPkg, there are a new Protocol with the new service
SmmWaitForAllProcessor(), which can be used by SMI handler
to optionally wait for other APs to complete SMM rendezvous in
relaxed AP mode.

This patch use the new service to let VariableSmm driver work
normally in relaxed AP mode.

Due to MdeModulePkg can not depend on UefiCpuPkg, use null version
implementation in MdePkg.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Ni Ray <ray.ni@...>

Signed-off-by: Zhihao Li <zhihao.li@...>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
| 10 +++++++++-
MdeModulePkg/MdeModulePkg.dsc
| 4 +++-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
| 3 ++-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
| 3 ++-
4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 517cae7b00f8..52a9b0e6b202 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -14,7 +14,7 @@
VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
ReclaimForOS(),

SmmVariableGetStatistics() should also do validation based on its own
knowledge.



-Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>

Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



@@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent


#include <Library/MmServicesTableLib.h>

#include <Library/VariablePolicyLib.h>

+#include <Library/SmmCpuRendezvousLib.h>



#include <Guid/SmmVariableCommon.h>

#include "Variable.h"

@@ -656,6 +657,13 @@ SmmVariableHandler (
goto EXIT;

}



+ if ((SmmVariableHeader->Attributes &
EFI_VARIABLE_NON_VOLATILE) != 0) {

+ if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {

+ DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP
check in SMM!\n"));

+ goto EXIT;

+ }

+ }

+

Status = VariableServiceSetVariable (

SmmVariableHeader->Name,

&SmmVariableHeader->Guid,

diff --git a/MdeModulePkg/MdeModulePkg.dsc
b/MdeModulePkg/MdeModulePkg.dsc
index b1d83461865e..1a3cf191bb5c 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -2,7 +2,7 @@
# EFI/PI Reference Module Package for All Architectures

#

# (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>

-# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>

# Copyright (c) Microsoft Corporation.

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -152,6 +152,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]

SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
bleLib.inf


LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
nf

SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf

+
SmmCpuRendezvousLib|MdePkg/Library/SmmCpuRendezvousLibNull/SmmC
puRendezvousLibNull.inf



[LibraryClasses.common.UEFI_DRIVER]

HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf

@@ -172,6 +173,7 @@ [LibraryClasses.common.MM_STANDALONE]

MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
aloneMmServicesTableLib.inf


LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
eMmLib.inf


MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
MemLib.inf

+
SmmCpuRendezvousLib|MdePkg/Library/SmmCpuRendezvousLibNull/SmmC
puRendezvousLibNull.inf



[LibraryClasses.ARM, LibraryClasses.AARCH64]

ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index eaa97a01c6e5..0bebd92b1626 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -18,7 +18,7 @@
# may not be modified without authorization. If platform fails to protect
these resources,

# the authentication service provided in this driver will be broken, and the
behavior is undefined.

#

-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>

# Copyright (c) Microsoft Corporation.

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

@@ -82,6 +82,7 @@ [LibraryClasses]
UefiBootServicesTableLib

VariablePolicyLib

VariablePolicyHelperLib

+ SmmCpuRendezvousLib



[Protocols]

gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES

diff --git
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
f
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
f
index d8c4f77e7f1f..595baaf70164 100644
---
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
f
+++
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
f
@@ -18,7 +18,7 @@
# may not be modified without authorization. If platform fails to protect
these resources,

# the authentication service provided in this driver will be broken, and the
behavior is undefined.

#

-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>

# Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>

# Copyright (c) Microsoft Corporation.

# SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -78,6 +78,7 @@ [LibraryClasses]
VarCheckLib

VariablePolicyLib

VariablePolicyHelperLib

+ SmmCpuRendezvousLib



[Protocols]

gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES

--
2.26.2.windows.1


Re: [PATCH 00/12] Introduce Bootlog DEBUG() output

Benjamin Doron
 

On Wed, Jun 1, 2022 at 05:33 AM, Gerd Hoffmann wrote:
Hmm. Maybe it's time to tackle the log performance problem for virtual
machines? Create a debug log device with DMA support, so we don't need
a vmexit for every single character we want log?
Of course, that doesn't work for native systems. I don't know how other developers perform debugging (possibly via a serial port), but I developed a library stack similar to this one to help me with GSoC last year (https://github.com/benjamindoron/edk2/commit/db888a928c1c6fc94f6a7670f3402718c10c01d2). It's WIP, modelled after the simple coreboot ringbuffer and is missing tracing facilities.

Regardless, having a true complement to PcdStatusCodeUseSerial would often be helpful, I suspect (presently, PcdStatusCodeUseMemory only logs the PI status code for the debug messages).
How does that relate to coreboot? coreboot has logging-to-memory too.
Not sure what the state is, there have been discussions on the coreboot
list about changing that from a pure text log to something structed with
timestamps a while back. Don't know whenever this did actually happen.

So, when adding logging-to-memory to edk2 it surely make sense to
coordinate that with coreboot, so we'll have both coreboot and edk2 logs
there in case edk2 runs as coreboot payload.
I'm working on getting a SerialPortLib that logs to CBMEM merged. It's on the list at the moment.

Some comments on DebugLibBootlog:
- It's possible to support ASSERTs. If PcdDebugPropertyMask & 0x31 == 0x01, then non-fatal ASSERTs are logged. As a DebugLib, this would require handling in `DebugAssert()`.
- SMM-phase logging can be implemented, but I'm not convinced that sharing DXE's buffer is entirely safe. Using SMM communicate could be safer, but would be more complicated. I stopped working on it when the return-on-investment was too low.

Regards,
Benjamin


[PATCH 1/1] MdeModulePkg: Fix a typo

zhangshuzhen@...
 

Correctly write 'DstBufAllocated' in CoreLoadPeImage().

Signed-off-by: Zhang Shuzhen <zhangshuzhen@...>
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 68bde5c15c..5bdf96b52c 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -581,7 +581,7 @@ CoreLoadPeImage (
   )
 {
   EFI_STATUS  Status;
-  BOOLEAN     DstBufAlocated;
+  BOOLEAN     DstBufAllocated;
   UINTN       Size;
 
   ZeroMem (&Image->ImageContext, sizeof (Image->ImageContext));
@@ -636,7 +636,7 @@ CoreLoadPeImage (
   //
   // Allocate memory of the correct memory type aligned on the required image boundary
   //
-  DstBufAlocated = FALSE;
+  DstBufAllocated = FALSE;
   if (DstBuffer == 0) {
     //
     // Allocate Destination Buffer as caller did not pass it in
@@ -702,7 +702,7 @@ CoreLoadPeImage (
       return Status;
     }
 
-    DstBufAlocated = TRUE;
+    DstBufAllocated = TRUE;
   } else {
     //
     // Caller provided the destination buffer
@@ -884,7 +884,7 @@ Done:
   // Free memory.
   //
 
-  if (DstBufAlocated) {
+  if (DstBufAllocated) {
     CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages);
     Image->ImageContext.ImageAddress = 0;
     Image->ImageBasePage             = 0;
--
2.36.1


Re: [PATCH V3] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

Lendacky, Thomas
 

On 6/1/22 07:25, Ard Biesheuvel wrote:
On Tue, 31 May 2022 at 16:40, Peter Gonda <pgonda@...> wrote:

The SEV-ES bit of Fn800-001F[EAX] - Bit 3 is used for a host to
determine support for running SEV-ES guests. It should not be checked by
a guest to determine if it is running under SEV-ES. The guest should use
the SEV_STATUS MSR Bit 1 to determine if SEV-ES is enabled. This check
was not part of the original SEV-ES support and was added in
a91b700e38. Removing the check makes this code consistent with the
Linux kernel

Fixes: a91b700e38 (Ovmf/ResetVector: Simplify and consolidate the SEV features checks)

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Brijesh Singh <brijesh.singh@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Marc Orr <marcorr@...>
Signed-off-by: Peter Gonda <pgonda@...>
Acked-by: Tom Lendacky <thomas.lendacky@...>

---
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 --------
1 file changed, 8 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 1f827da3b9..77692db27e 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -265,14 +265,6 @@ CheckSevFeatures:
; Set the work area header to indicate that the SEV is enabled
mov byte[WORK_AREA_GUEST_TYPE], 1

- ; Check for SEV-ES memory encryption feature:
- ; CPUID Fn8000_001F[EAX] - Bit 3
- ; CPUID raises a #VC exception if running as an SEV-ES guest
- mov eax, 0x8000001f
- cpuid
- bt eax, 3
- jnc GetSevEncBit
-
; Check if SEV-ES is enabled
; MSR_0xC0010131 - Bit 1 (SEV-ES enabled)
mov ecx, SEV_STATUS_MSR
Thanks Peter, I have queued this up.
I did wonder, though: the only remaining reference to GetSevEncBit is
a conditional branch that just precedes the label itself. This appears
to be a leftover from commit 63c50d3ff2854a76 ("OvmfPkg/ResetVector:
cache the SEV status MSR value in workarea") but it looks a bit dodgy.
Yes, it looks like the rdmsr and the GetSevEncBit: label can all be removed since the MSR value is now cached (a few lines above) and used for checks.

Thanks,
Tom


Re: [PATCH V3] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

Ard Biesheuvel
 

On Tue, 31 May 2022 at 16:40, Peter Gonda <pgonda@...> wrote:

The SEV-ES bit of Fn800-001F[EAX] - Bit 3 is used for a host to
determine support for running SEV-ES guests. It should not be checked by
a guest to determine if it is running under SEV-ES. The guest should use
the SEV_STATUS MSR Bit 1 to determine if SEV-ES is enabled. This check
was not part of the original SEV-ES support and was added in
a91b700e38. Removing the check makes this code consistent with the
Linux kernel

Fixes: a91b700e38 (Ovmf/ResetVector: Simplify and consolidate the SEV features checks)

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Brijesh Singh <brijesh.singh@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Marc Orr <marcorr@...>
Signed-off-by: Peter Gonda <pgonda@...>
Acked-by: Tom Lendacky <thomas.lendacky@...>

---
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 --------
1 file changed, 8 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 1f827da3b9..77692db27e 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -265,14 +265,6 @@ CheckSevFeatures:
; Set the work area header to indicate that the SEV is enabled
mov byte[WORK_AREA_GUEST_TYPE], 1

- ; Check for SEV-ES memory encryption feature:
- ; CPUID Fn8000_001F[EAX] - Bit 3
- ; CPUID raises a #VC exception if running as an SEV-ES guest
- mov eax, 0x8000001f
- cpuid
- bt eax, 3
- jnc GetSevEncBit
-
; Check if SEV-ES is enabled
; MSR_0xC0010131 - Bit 1 (SEV-ES enabled)
mov ecx, SEV_STATUS_MSR
Thanks Peter, I have queued this up.

I did wonder, though: the only remaining reference to GetSevEncBit is
a conditional branch that just precedes the label itself. This appears
to be a leftover from commit 63c50d3ff2854a76 ("OvmfPkg/ResetVector:
cache the SEV status MSR value in workarea") but it looks a bit dodgy.

6781 - 6800 of 96835