Date   

[PATCH 2/4] UefiPayloadPkg: define some PCD as DynamicEX PCD

Zhiguang Liu
 

Define some PCDs as DynamicEX PCD to be used as global variable.
Because PcdUartDefaultBaudRate is defined as DynamicEX, remove the code
to set it in platformlib. That code was actually redundant.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>

Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c =
| 5 -----
UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf =
| 1 -
UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/PlatformHookLib.c =
| 4 ----
UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/PlatformHookLib.inf=
| 1 -
UefiPayloadPkg/UefiPayloadPkg.dsc =
| 28 ++++++++++++++++++----------
5 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c b/Uef=
iPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c
index 72a17dc8a7..d8453e5957 100644
--- a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c
+++ b/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c
@@ -75,11 +75,6 @@ PlatformHookSerialPortInitialize (
return Status;=0D
}=0D
=0D
- Status =3D PcdSet64S (PcdUartDefaultBaudRate, SerialPortInfo.Baud);=0D
- if (RETURN_ERROR (Status)) {=0D
- return Status;=0D
- }=0D
-=0D
Status =3D PcdSet32S (PcdSerialClockRate, SerialPortInfo.InputHertz);=0D
if (RETURN_ERROR (Status)) {=0D
return Status;=0D
diff --git a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf b/U=
efiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
index 2415d99c64..3eeb94d8fa 100644
--- a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
+++ b/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
@@ -35,5 +35,4 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate ## PRODUCES=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride ## PRODUCES=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate ## PRODUCES=0D
- gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## PRODUCES=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdPciSerialParameters ## PRODUCES=0D
diff --git a/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/Platfor=
mHookLib.c b/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/Platfor=
mHookLib.c
index 6705f29505..bd433bdbe0 100644
--- a/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/PlatformHookLi=
b.c
+++ b/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/PlatformHookLi=
b.c
@@ -70,10 +70,6 @@ PlatformHookSerialPortInitialize (
if (RETURN_ERROR (Status)) {=0D
return Status;=0D
}=0D
- Status =3D PcdSet64S (PcdUartDefaultBaudRate, SerialPortInfo->BaudRate=
);=0D
- if (RETURN_ERROR (Status)) {=0D
- return Status;=0D
- }=0D
=0D
return RETURN_SUCCESS;=0D
}=0D
diff --git a/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/Platfor=
mHookLib.inf b/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/Platf=
ormHookLib.inf
index 41e05ddf54..2dfd8b1216 100644
--- a/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/PlatformHookLi=
b.inf
+++ b/UefiPayloadPkg/Library/UniversalPayloadPlatformHookLib/PlatformHookLi=
b.inf
@@ -38,4 +38,3 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase ## PRODUCES=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate ## PRODUCES=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride ## PRODUCES=0D
- gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## PRODUCES=0D
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayload=
Pkg.dsc
index ba54f2057f..d293211e46 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -308,11 +308,6 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE=0D
=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0=0D
-!if $(TARGET) =3D=3D DEBUG=0D
- gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE=0D
-!else=0D
- gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE=0D
-!endif=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE=0D
=0D
@@ -352,11 +347,6 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|$(SERIAL_FIFO_CONTRO=
L)=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|$(SERIAL_EXTE=
NDED_TX_FIFO_SIZE)=0D
=0D
- gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|$(UART_DEFAULT_BAUD_RATE=
)=0D
- gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|$(UART_DEFAULT_DATA_BITS=
)=0D
- gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|$(UART_DEFAULT_PARITY)=0D
- gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|$(UART_DEFAULT_STOP_BITS=
)=0D
- gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|$(DEFAULT_TERMINAL_TYPE)=
=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdPciSerialParameters|$(PCI_SERIAL_PARAM=
ETERS)=0D
=0D
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|$(MAX_LOGICAL_=
PROCESSORS)=0D
@@ -369,6 +359,24 @@
##########################################################################=
######=0D
=0D
[PcdsDynamicExDefault]=0D
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|$(UART_DEFAULT_BAUD_RATE=
)=0D
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|$(UART_DEFAULT_DATA_BITS=
)=0D
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|$(UART_DEFAULT_PARITY)=0D
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|$(UART_DEFAULT_STOP_BITS=
)=0D
+ gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|$(DEFAULT_TERMINAL_TYPE)=
=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize=0D
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds=0D
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode=0D
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress=0D
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize=0D
+!if $(TARGET) =3D=3D DEBUG=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE=0D
+!else=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE=0D
+!endif=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FAL=
SE=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0=0D
--=20
2.32.0.windows.2


[PATCH 1/4] UefiPayloadPkg: Add Fixed PCDs and use Macro to define the default value.

Zhiguang Liu
 

Add the three PCDs as fixed at build PCD:
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule
gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister
gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister
The default value is defined as Macro, so it can be passed in at build
command.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>

Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayload=
Pkg.dsc
index bcedf1c746..ba54f2057f 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -91,6 +91,13 @@
DEFINE EMU_VARIABLE_ENABLE =3D TRUE=0D
DEFINE DISABLE_RESET_SYSTEM =3D FALSE=0D
=0D
+ # Dfine the maximum size of the capsule image without a reset flag that =
the platform can support.=0D
+ DEFINE MAX_SIZE_NON_POPULATE_CAPSULE =3D 0xa00000=0D
+=0D
+ # Define RTC related register.=0D
+ DEFINE RTC_INDEX_REGISTER =3D 0x70=0D
+ DEFINE RTC_TARGET_REGISTER =3D 0x71=0D
+=0D
[BuildOptions]=0D
*_*_*_CC_FLAGS =3D -D DISABLE_NEW_DEPRECATED_INTERFACES=
=0D
GCC:*_UNIXGCC_*_CC_FLAGS =3D -DMDEPKG_NDEBUG=0D
@@ -324,7 +331,9 @@
!else=0D
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F=0D
!endif=0D
-=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule|$(MAX_SIZE_N=
ON_POPULATE_CAPSULE)=0D
+ gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)=
=0D
+ gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER=
)=0D
#=0D
# The following parameters are set by Library/PlatformHookLib=0D
#=0D
--=20
2.32.0.windows.2


[PATCH RESEND v1 2/2] Silicon/Qemu: Update MADT with GICv3 ITS info

Shashi Mallela
 

For Qemu sbsa-ref platforms,to enable detection of GICv3 Interrupt
Translation Service capability in the ACPI MADT,the GIC ITS structure
is created with the relevant values for each of its fields.The
existing MADT functionality is extended to include GIC ITS structure
presence as well.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Graeme Gregory <graeme@nuviainc.com>
Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
Silicon/Qemu/SbsaQemu/SbsaQemu.dec | 3 +++
Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 2 ++
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 2 ++
Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h | 10 ++++++++++
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 10 +++++++++-
5 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
index 9448852967b6..8654cc7c858c 100644
--- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
+++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
@@ -36,6 +36,9 @@ [PcdsFixedAtBuild.common]
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize|0x10000|UINT32|0x00000004
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress|0x10000000000|UINT64|0x00000005

+ # ARM Generic Interrupt Controller ITS
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase|0|UINT64|0x0000000F
+
# PCDs complementing PCIe layout pulled into ACPI tables
# Limit = Base + Size - 1
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciIoLimit|0x0000ffff|UINT32|0x00000006
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index 9be34488eb7a..5616b73178ff 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -74,3 +74,5 @@ [FixedPcd]
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciSize
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize
+
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index c6de685bd2c4..2eb6577fd077 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -65,3 +65,5 @@ [FixedPcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
+
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
index 4d5b05ba17c6..5f9e9477bf6a 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
@@ -37,6 +37,16 @@
SBSAQEMU_MADT_GICR_SIZE /* DiscoveryRangeLength */ \
}

+// Macro for MADT GIC ITS Structure
+#define SBSAQEMU_MADT_GIC_ITS_INIT() { \
+ EFI_ACPI_6_0_GIC_ITS, /* Type */ \
+ sizeof (EFI_ACPI_6_0_GIC_ITS_STRUCTURE), /* Length */ \
+ EFI_ACPI_RESERVED_WORD, /* Reserved */ \
+ 0, /* GicItsId */ \
+ FixedPcdGet64 (PcdGicItsBase), /* PhysicalBaseAddress */ \
+ EFI_ACPI_RESERVED_DWORD /* Reserved */ \
+ }
+
#define SBSAQEMU_ACPI_SCOPE_OP_MAX_LENGTH 5

#define SBSAQEMU_ACPI_SCOPE_NAME { '_', 'S', 'B', '_' }
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index b8901030ecd0..9d6a4b36cbe9 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -91,6 +91,9 @@ AddMadtTable (
// Initialize GIC Redistributor Structure
EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();

+ // Initialize GIC ITS Structure
+ EFI_ACPI_6_0_GIC_ITS_STRUCTURE Gic_Its = SBSAQEMU_MADT_GIC_ITS_INIT();
+
// Get CoreCount which was determined eariler after parsing device tree
NumCores = PcdGet32 (PcdCoreCount);

@@ -98,7 +101,8 @@ AddMadtTable (
TableSize = sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
(sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) +
sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) +
- sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
+ sizeof (EFI_ACPI_6_0_GICR_STRUCTURE) +
+ sizeof (EFI_ACPI_6_0_GIC_ITS_STRUCTURE);

Status = gBS->AllocatePages (
AllocateAnyPages,
@@ -138,6 +142,10 @@ AddMadtTable (
CopyMem (New, &Gicr, sizeof (EFI_ACPI_6_0_GICR_STRUCTURE));
New += sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);

+ // GIC ITS Structure
+ CopyMem (New, &Gic_Its, sizeof (EFI_ACPI_6_0_GIC_ITS_STRUCTURE));
+ New += sizeof (EFI_ACPI_6_0_GIC_ITS_STRUCTURE);
+
AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);

Status = AcpiTable->InstallAcpiTable (
--
2.27.0


[PATCH RESEND v1 1/2] Platform/Qemu/SbsaQemu/SbsaQemu.dsc: define GICv3 ITS base address

Shashi Mallela
 

Define new pcd setting for specifying the base address of GICv3
Interrupt Translation Service.For Qemu sbsa-ref platforms,this
enables the detection of GIC ITS capability within the GIC ITS
structure of ACPI MADT.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Graeme Gregory <graeme@nuviainc.com>
Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index 063a45b3cee4..a2959eb4acce 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -408,7 +408,7 @@ [PcdsFixedAtBuild.common]
# ARM General Interrupt Controller
#
gArmTokenSpaceGuid.PcdGicDistributorBase|0x40060000
- gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x40080000
+ gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x400B0000

## Default Terminal Type
## 0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
@@ -439,6 +439,9 @@ [PcdsFixedAtBuild.common]
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase|0x60110000
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize|0x00010000

+ # GIC ITS
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase|0x40070000
+
# PL011 - Serial Terminal
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x60000000

--
2.27.0


[PATCH RESEND v1 0/2] Add GIC ITS entry to MADT

Shashi Mallela
 

This patchset implements ACPI MADT functionality extension to include
the GICv3 ITS functionality. This enables devices to use message
signalled LPI interrupts in addition to SPIs,PPIs supported on ARM
SBSA reference qemu platforms.

Shashi Mallela (2):
Platform/Qemu/SbsaQemu/SbsaQemu.dsc: define GICv3 ITS base address
Silicon/Qemu: Update MADT with GICv3 ITS info

Silicon/Qemu/SbsaQemu/SbsaQemu.dec | 3 +++
Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 5 ++++-
Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 2 ++
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 2 ++
Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h | 10 ++++++++++
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 10 +++++++++-
6 files changed, 30 insertions(+), 2 deletions(-)

--
2.27.0


Re: Is there any use case of FirmwarePerformanceStandaloneMm.inf now?

Dandan Bi
 

Hi Ray,

I think for now it will not cause any issue when standalone mm is launched.
As the functionality of collecting StandaloneMm performance data itself is missing in Edk2.
So, it's ok to remove the related logic in FirmwarePerformanceStandaloneMm.inf now.

But later if we want to collect the StandaloneMm performance data, we should add the support in Edk2 like what SmmPerformanceLib/SmmCorePerformanceLib have done for SMM.


Thanks,
Dandan

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, August 6, 2021 9:42 AM
To: devel@edk2.groups.io; kuqin12@gmail.com; Bi, Dandan
<dandan.bi@intel.com>; kun.q@outlook.com
Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
gaoliming <gaoliming@byosoft.com.cn>; Yao, Jiewen
<jiewen.yao@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>;
'Sean Brogan' <sean.brogan@microsoft.com>
Subject: RE: [edk2-devel] Is there any use case of
FirmwarePerformanceStandaloneMm.inf now?

It looks like a good topic to discuss in TianoCore Open Design meeting😊

Question to Dandan's proposal: Does it cause any conflict (or help) when
standalone mm is launched from PEI?

Thanks,
Ray

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun
Qin
Sent: Friday, August 6, 2021 6:49 AM
To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com>;
kun.q@outlook.com
Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; gaoliming <gaoliming@byosoft.com.cn>; Yao,
Jiewen <jiewen.yao@intel.com>; Bret Barkelew
<Bret.Barkelew@microsoft.com>; 'Sean Brogan'
<sean.brogan@microsoft.com>
Subject: Re: [edk2-devel] Is there any use case of
FirmwarePerformanceStandaloneMm.inf now?

Hi Dandan,

Thanks for letting me know. I added Bret and Sean to the thread for
broader view in our scope.

But currently our StandaloneMm Core does not report performance data
to FirmwarePerformanceStandaloneMm module.

Is the idea to centralize the performance report collection job to
SmmCorePerformanceLib and remove the FirmwarePerformance**Mm
driver?
Is there any plan to support a Standalone instance once the
traditional MM version is functional?

Thanks,
Kun


On 08/05/2021 04:44, Dandan Bi wrote:
Hi Kun,

I plan to make some change for FirmwarePerformanceSmm.inf, may also
update the behavior of FirmwarePerformanceStandaloneMm.inf as they
are sharing codes now.

And I saw you are the submitter of this driver. Could you help
clarify following questions ? Thanks in advance.

1. Do you have the use case to leverage
FirmwarePerformanceStandaloneMm.inf to collect Standalone MM
performance data now?
2. Do you have any Library/module used by StandaloneMmCore to
collect
Standalone MM performance data and report the data to
FirmwarePerformanceStandaloneMm like the
SmmCorePerformanceLib used
for SMM core?
3. I plan to move some logic from FirmwarePerformanceDataTableSmm
to
SmmCorePerformanceLib as below. Do you think is it ok just to remove
them from FirmwarePerformanceStandaloneMm.inf now?

If there is not any module to report Standalone MM performance data
to FirmwarePerformanceStandaloneMm.inf, I think it should be OK to
remove them from FirmwarePerformanceStandaloneMm now.

Change:

SMM performance data collection now:

1. SmmCorePerformanceLib collect all the performance data in SMM and
report the data to FirmwarePerformanceDataTableSmm through status
code. **
2. DxeCorePerformanceLib will communicate with
FirmwarePerformanceDataTableSmm to get the SMM performance
data and
allocate performance table to store all the performance data.

Now I want to simplify the process to make DxeCorePerformanceLib
communicate with SmmCorePerformanceLib directly to collect SMM
performance data, so FirmwarePerformanceDataTableSmm don’t need
to
get the SMM performance data from SmmCorePerformanceLib and
register
SMI handler for the communication with DxeCorePerformanceLib.

For FirmwarePerformanceStandaloneMm.inf, just remove this logic if
there is no module to prepare MM performance data to it now.

Thanks,

Dandan




Re: [edk2-platforms][PATCH v5 43/46] KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI API

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
Tested-by: Benjamin Doron <benjamin.doron00@gmail.com>

-----Original Message-----
From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
Sent: Tuesday, August 3, 2021 10:39 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v5 43/46]
KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI API

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

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

Updates usage of gPchSpiPpiGuid to use the new interface that identifies SPI
flash regions by GUID.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---

Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiS
erialPortLibSpiFlash.c | 4 ++--

Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiS
erialPortLibSpiFlash.inf | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
index fc48bdc6fccb..fe8883a8af29 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFla
+++ sh/PeiSerialPortLibSpiFlash.c
@@ -98,7 +98,7 @@ SerialPortWrite (
LinearOffset = (UINT32) (FixedPcdGet32 (PcdFlashNvDebugMessageBase)
- FixedPcdGet32 (PcdFlashAreaBaseAddress));
Status = PchSpiPpi->FlashErase (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
NvMessageAreaSize
);
@@ -118,7 +118,7 @@ SerialPortWrite (

Status = PchSpiPpi->FlashWrite (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
BytesWritten,
(UINT8 *) &Buffer[SourceBufferOffset] diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
index b959cd1f4612..b8ae214f0920 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFla
+++ sh/PeiSerialPortLibSpiFlash.inf
@@ -43,6 +43,7 @@ [Ppis]
gPchSpiPpiGuid

[Guids]
+ gFlashRegionBiosGuid
gSpiFlashDebugHobGuid

[Pcd]
--
2.28.0.windows.1


Re: [edk2-platforms][PATCH v5 44/46] KabylakeOpenBoardPkg/KabylakeRvp3: Add PeiSerialPortlibSpiFlash to build

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
Tested-by: Benjamin Doron <benjamin.doron00@gmail.com>

-----Original Message-----
From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
Sent: Tuesday, August 3, 2021 10:39 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v5 44/46]
KabylakeOpenBoardPkg/KabylakeRvp3: Add PeiSerialPortlibSpiFlash to build

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

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

This library is part of KabylakeOpenBoardPkg but is currently not built
anywhere. This change adds the library to the KabylakeRvp3 build to ensure
it can always build properly if not linked elsewhere.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc |
2 ++
1 file changed, 2 insertions(+)

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
index f29393579c06..18cfa31db8a2 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
+++
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
@@ -392,6 +392,8 @@ [Components.IA32]
!endif
$(PLATFORM_BOARD_PACKAGE)/BiosInfo/BiosInfo.inf

+
+
$(PLATFORM_BOARD_PACKAGE)/Library/PeiSerialPortLibSpiFlash/PeiSerialP
o
+ rtLibSpiFlash.inf
+
#######################################
# DXE Components
#######################################
--
2.28.0.windows.1


Re: [edk2-platforms][PATCH v5 43/46] KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI API

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
Tested-by: Benjamin Doron <benjamin.doron00@gmail.com>

-----Original Message-----
From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
Sent: Tuesday, August 3, 2021 11:01 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v5 43/46]
KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI API

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

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

Updates usage of gPchSpiPpiGuid to use the new interface that identifies SPI
flash regions by GUID.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---

Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiS
erialPortLibSpiFlash.c | 4 ++--

Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiS
erialPortLibSpiFlash.inf | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
index fc48bdc6fccb..fe8883a8af29 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFla
+++ sh/PeiSerialPortLibSpiFlash.c
@@ -98,7 +98,7 @@ SerialPortWrite (
LinearOffset = (UINT32) (FixedPcdGet32 (PcdFlashNvDebugMessageBase)
- FixedPcdGet32 (PcdFlashAreaBaseAddress));
Status = PchSpiPpi->FlashErase (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
NvMessageAreaSize
);
@@ -118,7 +118,7 @@ SerialPortWrite (

Status = PchSpiPpi->FlashWrite (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
BytesWritten,
(UINT8 *) &Buffer[SourceBufferOffset] diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
index b959cd1f4612..b8ae214f0920 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFla
+++ sh/PeiSerialPortLibSpiFlash.inf
@@ -43,6 +43,7 @@ [Ppis]
gPchSpiPpiGuid

[Guids]
+ gFlashRegionBiosGuid
gSpiFlashDebugHobGuid

[Pcd]
--
2.28.0.windows.1


Re: [edk2-platforms][PATCH v5 43/46] KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI API

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
Tested-by: Benjamin Doron <benjamin.doron00@gmail.com>

-----Original Message-----
From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
Sent: Tuesday, August 3, 2021 11:01 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v5 43/46]
KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI API

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

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

Updates usage of gPchSpiPpiGuid to use the new interface that identifies SPI
flash regions by GUID.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---

Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiS
erialPortLibSpiFlash.c | 4 ++--

Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiS
erialPortLibSpiFlash.inf | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
index fc48bdc6fccb..fe8883a8af29 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFla
+++ sh/PeiSerialPortLibSpiFlash.c
@@ -98,7 +98,7 @@ SerialPortWrite (
LinearOffset = (UINT32) (FixedPcdGet32 (PcdFlashNvDebugMessageBase)
- FixedPcdGet32 (PcdFlashAreaBaseAddress));
Status = PchSpiPpi->FlashErase (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
NvMessageAreaSize
);
@@ -118,7 +118,7 @@ SerialPortWrite (

Status = PchSpiPpi->FlashWrite (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
BytesWritten,
(UINT8 *) &Buffer[SourceBufferOffset] diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
index b959cd1f4612..b8ae214f0920 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/P
eiSerialPortLibSpiFlash.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFla
+++ sh/PeiSerialPortLibSpiFlash.inf
@@ -43,6 +43,7 @@ [Ppis]
gPchSpiPpiGuid

[Guids]
+ gFlashRegionBiosGuid
gSpiFlashDebugHobGuid

[Pcd]
--
2.28.0.windows.1


Re: Is there any use case of FirmwarePerformanceStandaloneMm.inf now?

Dandan Bi
 

Hi Kun,

Thank you for the information.

Yes, the idea is to centralize the SMM performance report collection job to SmmCorePerformanceLib, and DxeCorePerformanceLib could communicate with SmmCorePerformanceLib for all SMM performance data rather than through FirmwarePerformance**Mm driver.
But we will not remove FirmwarePerformance**Mm driver, just remove the code logic related to **MM performance data which will be covered by **mmCorePerformanceLib.
FirmwarePerformance**Mm driver will still collect some performance data in S3 like the S3SuspendStart, S3SuspendEnd. We don't introduce any incompatible change.

If now StandaloneMm Core does not report performance data to FirmwarePerformanceStandaloneMm module, I think it should be ok to remove the handling of **MM performance data both in FirmwarePerformanceSmm and FirmwarePerformanceStandaloneMm now.
For SMM, the removed logic will be handled by SmmCorePerformanceLib
For StandaloneMm, I think we can submit an Edk2 feature to add a StandaloneMmCore instance with the same functionality of SmmCorePerformanceLib. As now the functionality of collecting StandaloneMm performance data is missing in Edk2.



Thanks,
Dandan

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com>
Sent: Friday, August 6, 2021 6:49 AM
To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com>;
kun.q@outlook.com
Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
gaoliming <gaoliming@byosoft.com.cn>; Yao, Jiewen
<jiewen.yao@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>;
'Sean Brogan' <sean.brogan@microsoft.com>
Subject: Re: [edk2-devel] Is there any use case of
FirmwarePerformanceStandaloneMm.inf now?

Hi Dandan,

Thanks for letting me know. I added Bret and Sean to the thread for broader
view in our scope.

But currently our StandaloneMm Core does not report performance data to
FirmwarePerformanceStandaloneMm module.

Is the idea to centralize the performance report collection job to
SmmCorePerformanceLib and remove the FirmwarePerformance**Mm
driver? Is there any plan to support a Standalone instance once the
traditional MM version is functional?

Thanks,
Kun


On 08/05/2021 04:44, Dandan Bi wrote:
Hi Kun,

I plan to make some change for FirmwarePerformanceSmm.inf, may also
update the behavior of FirmwarePerformanceStandaloneMm.inf as they
are
sharing codes now.

And I saw you are the submitter of this driver. Could you help clarify
following questions ? Thanks in advance.

1. Do you have the use case to leverage
FirmwarePerformanceStandaloneMm.inf to collect Standalone MM
performance data now?
2. Do you have any Library/module used by StandaloneMmCore to collect
Standalone MM performance data and report the data to
FirmwarePerformanceStandaloneMm like the SmmCorePerformanceLib
used
for SMM core?
3. I plan to move some logic from FirmwarePerformanceDataTableSmm to
SmmCorePerformanceLib as below. Do you think is it ok just to remove
them from FirmwarePerformanceStandaloneMm.inf now?

If there is not any module to report Standalone MM performance data to
FirmwarePerformanceStandaloneMm.inf, I think it should be OK to remove
them from FirmwarePerformanceStandaloneMm now.

Change:

SMM performance data collection now:

1. SmmCorePerformanceLib collect all the performance data in SMM and
report the data to FirmwarePerformanceDataTableSmm through status
code. **
2. DxeCorePerformanceLib will communicate with
FirmwarePerformanceDataTableSmm to get the SMM performance data
and
allocate performance table to store all the performance data.

Now I want to simplify the process to make DxeCorePerformanceLib
communicate with SmmCorePerformanceLib directly to collect SMM
performance data, so FirmwarePerformanceDataTableSmm don't need to
get
the SMM performance data from SmmCorePerformanceLib and register
SMI
handler for the communication with DxeCorePerformanceLib.

For FirmwarePerformanceStandaloneMm.inf, just remove this logic if
there is no module to prepare MM performance data to it now.

Thanks,

Dandan


Re: [edk2-platforms: PATCH V5] Platform/Intel: Correct CPU APIC IDs

Ni, Ray
 

1. I see lots of coding style changes in the patch. Can you please separate these changes that don't impact functionality in a standalone patch?

+ CpuIdMapPtr->AcpiProcessorId = ((UINT32)ProcessorInfoBuffer.Location.Package * (UINT32)mNumberOfCpus * 2) + Index;
2. Per ACPI spec, this value should match to the _UID value of processor object in ACPI table.
What mechanism is used to guarantee the match?

The original logic uses PCD value FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuThreadCount) to guarantee the match.

We need to finalize the correct mechanism for the _UID match requirement in ACPI spec first.
Then we can continue review the following reorder logic because the following logic depends on the initial order.

Thanks,
Ray


Re: Is there any use case of FirmwarePerformanceStandaloneMm.inf now?

Ni, Ray
 

It looks like a good topic to discuss in TianoCore Open Design meeting😊

Question to Dandan's proposal: Does it cause any conflict (or help) when standalone mm is launched from PEI?

Thanks,
Ray

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
Sent: Friday, August 6, 2021 6:49 AM
To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com>; kun.q@outlook.com
Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; gaoliming <gaoliming@byosoft.com.cn>; Yao,
Jiewen <jiewen.yao@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; 'Sean Brogan' <sean.brogan@microsoft.com>
Subject: Re: [edk2-devel] Is there any use case of FirmwarePerformanceStandaloneMm.inf now?

Hi Dandan,

Thanks for letting me know. I added Bret and Sean to the thread for
broader view in our scope.

But currently our StandaloneMm Core does not report performance data to
FirmwarePerformanceStandaloneMm module.

Is the idea to centralize the performance report collection job to
SmmCorePerformanceLib and remove the FirmwarePerformance**Mm driver? Is
there any plan to support a Standalone instance once the traditional MM
version is functional?

Thanks,
Kun


On 08/05/2021 04:44, Dandan Bi wrote:
Hi Kun,

I plan to make some change for FirmwarePerformanceSmm.inf, may also
update the behavior of FirmwarePerformanceStandaloneMm.inf as they are
sharing codes now.

And I saw you are the submitter of this driver. Could you help clarify
following questions ? Thanks in advance.

1. Do you have the use case to leverage
FirmwarePerformanceStandaloneMm.inf to collect Standalone MM
performance data now?
2. Do you have any Library/module used by StandaloneMmCore to collect
Standalone MM performance data and report the data to
FirmwarePerformanceStandaloneMm like the SmmCorePerformanceLib used
for SMM core?
3. I plan to move some logic from FirmwarePerformanceDataTableSmm to
SmmCorePerformanceLib as below. Do you think is it ok just to remove
them from FirmwarePerformanceStandaloneMm.inf now?

If there is not any module to report Standalone MM performance data to
FirmwarePerformanceStandaloneMm.inf, I think it should be OK to remove
them from FirmwarePerformanceStandaloneMm now.

Change:

SMM performance data collection now:

1. SmmCorePerformanceLib collect all the performance data in SMM and
report the data to FirmwarePerformanceDataTableSmm through status
code. **
2. DxeCorePerformanceLib will communicate with
FirmwarePerformanceDataTableSmm to get the SMM performance data and
allocate performance table to store all the performance data.

Now I want to simplify the process to make DxeCorePerformanceLib
communicate with SmmCorePerformanceLib directly to collect SMM
performance data, so FirmwarePerformanceDataTableSmm don’t need to get
the SMM performance data from SmmCorePerformanceLib and register SMI
handler for the communication with DxeCorePerformanceLib.

For FirmwarePerformanceStandaloneMm.inf, just remove this logic if there
is no module to prepare MM performance data to it now.

Thanks,

Dandan




Re: [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

Michael Kubacki
 

Thanks for the feedback, I addressed it in a v2 series:
https://edk2.groups.io/g/devel/message/78767

Regards,
Michael

On 8/5/2021 7:14 PM, Nate DeSimone wrote:
Hi Michael,
Here is a summary of my feedback:
1. Patch 2/5: DxeCheckTcgTrustedBoot.c: line 44 - I think we should also have a ZeroMem ((VOID *) &ProtocolCapability, sizeof (ProtocolCapability)) before setting the size.
2. Patch 3/5: SmmTestPointCheckLib.c: line 112 - I think we should have a #define that describes whatever "6" means in this context.
Everything else looks good!
Thanks,
Nate

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
Kubacki
Sent: Thursday, August 5, 2021 7:57 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [edk2-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg:
TestPointCheckLib bug fixes and improvements

From: Michael Kubacki <mailto:michael.kubacki@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3531
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3518
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3520
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3521

This patch series groups together several bug fixes and improvements to
TestPointCheckLib. The first patch is required for the others since it fixes a
MinPlatformPkg build issue that occurs with the current edk2/master branch.

Cc: Chasel Chiu <mailto:chasel.chiu@intel.com>
Cc: Nate DeSimone <mailto:nathaniel.l.desimone@intel.com>
Cc: Liming Gao <mailto:gaoliming@byosoft.com.cn>
Cc: Eric Dong <mailto:eric.dong@intel.com>
Signed-off-by: Michael Kubacki <mailto:michael.kubacki@microsoft.com>

Michael Kubacki (5):
MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional


Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckA
cpi.c | 32 +++++------

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
miHandlerInstrument.c | 4 +-

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
mmInfo.c | 56 ++++++++++----------

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTc
gTrustedBoot.c | 1 +

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
ntCheckLib.c | 15 +++++-

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPo
intCheckLib.c | 4 +-
Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
| 10 ++--

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
ntCheckLib.inf | 1 +
8 files changed, 70 insertions(+), 53 deletions(-)

--
2.28.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78710): https://edk2.groups.io/g/devel/message/78710
Mute This Topic: https://groups.io/mt/84686301/1767664
Group Owner: mailto:devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[nathaniel.l.desimone@intel.com]
-=-=-=-=-=-=


[edk2-platforms][PATCH v2 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional

Michael Kubacki
 

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

Makes the OutTable parameter in DumpAcpiRsdt() and DumpAcpiXsdt()
optional since the pointer passed can be NULL if the Signature
pointer is also NULL.

Can fix a potential failure in TestPointCheckAcpi().

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckAcp=
i.c | 32 ++++++++++----------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeCheckAcpi.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointChe=
ckLib/DxeCheckAcpi.c
index cd8f538f7f3f..3d75e5012a4c 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckAcpi.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckAcpi.c
@@ -477,7 +477,7 @@ DumpAcpiTable (
)
{
EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
- =20
+
if (Table =3D=3D NULL) {
return ;
}
@@ -535,7 +535,7 @@ CheckAcpiTableResource (
)
{
EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
- =20
+
if (Table =3D=3D NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -592,7 +592,7 @@ EFI_STATUS
DumpAcpiRsdt (
IN EFI_ACPI_DESCRIPTION_HEADER *Rsdt,
IN UINT32 *Signature, OPTIONAL
- OUT VOID **OutTable,
+ OUT VOID **OutTable, OPTIONAL
IN BOOLEAN DumpPrint,
IN BOOLEAN CheckResource
)
@@ -610,7 +610,7 @@ DumpAcpiRsdt (
=20
if (OutTable !=3D NULL) {
*OutTable =3D NULL;
- } else {
+ } else if ((OutTable =3D=3D NULL) && (Signature !=3D NULL)) {
return EFI_INVALID_PARAMETER;
}
=20
@@ -632,7 +632,7 @@ DumpAcpiRsdt (
*OutTable =3D Table;
}
}
- =20
+
if (OutTable !=3D NULL) {
if (*OutTable =3D=3D NULL) {
return EFI_NOT_FOUND;
@@ -646,7 +646,7 @@ EFI_STATUS
DumpAcpiXsdt (
IN EFI_ACPI_DESCRIPTION_HEADER *Xsdt,
IN UINT32 *Signature, OPTIONAL
- OUT VOID **OutTable,
+ OUT VOID **OutTable, OPTIONAL
IN BOOLEAN DumpPrint,
IN BOOLEAN CheckResource
)
@@ -662,16 +662,16 @@ DumpAcpiXsdt (
if (Xsdt =3D=3D NULL) {
return EFI_INVALID_PARAMETER;
}
- =20
+
if (OutTable !=3D NULL) {
*OutTable =3D NULL;
- } else {
+ } else if ((OutTable =3D=3D NULL) && (Signature !=3D NULL)) {
return EFI_INVALID_PARAMETER;
}
=20
ReturnStatus =3D EFI_SUCCESS;
EntryCount =3D (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /=
sizeof(UINT64);
- =20
+
BasePtr =3D (UINTN)(Xsdt + 1);
for (Index =3D 0; Index < EntryCount; Index ++) {
CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), size=
of(UINT64));
@@ -783,7 +783,7 @@ TestPointCheckAcpi (
if (Status =3D=3D EFI_NOT_FOUND) {
Status =3D DumpAcpiWithGuid (&gEfiAcpi10TableGuid, NULL, NULL, TRUE,=
FALSE);
}
- =20
+
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_ERROR, "No ACPI table\n"));
TestPointLibAppendErrorString (
@@ -796,7 +796,7 @@ TestPointCheckAcpi (
}
=20
DEBUG ((DEBUG_INFO, "=3D=3D=3D=3D TestPointCheckAcpi - Exit\n"));
- =20
+
return Status;
}
=20
@@ -806,9 +806,9 @@ TestPointCheckAcpiGcdResource (
)
{
EFI_STATUS Status;
- =20
+
DEBUG ((DEBUG_INFO, "=3D=3D=3D=3D TestPointCheckAcpiGcdResource - Ente=
r\n"));
- =20
+
//
// Check the ACPI existence
//
@@ -816,7 +816,7 @@ TestPointCheckAcpiGcdResource (
if (Status =3D=3D EFI_NOT_FOUND) {
Status =3D DumpAcpiWithGuid (&gEfiAcpi10TableGuid, NULL, NULL, FALSE=
, FALSE);
}
- =20
+
if (!EFI_ERROR(Status)) {
//
// Then check resource in ACPI and GCD
@@ -828,7 +828,7 @@ TestPointCheckAcpiGcdResource (
Status =3D DumpAcpiWithGuid (&gEfiAcpi10TableGuid, NULL, NULL, FAL=
SE, TRUE);
}
}
- =20
+
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_ERROR, "ACPI table resource not in GCD\n"));
TestPointLibAppendErrorString (
@@ -840,7 +840,7 @@ TestPointCheckAcpiGcdResource (
);
}
DEBUG ((DEBUG_INFO, "=3D=3D=3D=3D TestPointCheckAcpiGcdResource - Exit=
\n"));
- =20
+
return Status;
}
=20
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v2 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking

Michael Kubacki
 

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

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

The current logic depends on a particular order in which the
descriptors for three or more regions are placed in the array to
perform proper adjacency checking. When three or more regions are
all adjacent, but neighboring descriptors are not adjacent, the
logic can improperly report a failure. Adjust the logic so that
all descriptors are checked for adjacency.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmm=
Info.c | 56 ++++++++++----------
1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeCheckSmmInfo.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPoint=
CheckLib/DxeCheckSmmInfo.c
index c493750a27e6..f15f76eab574 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckSmmInfo.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckSmmInfo.c
@@ -59,34 +59,36 @@ CheckSmramDescriptor (
)
{
UINTN Index;
- UINT64 Base;
+ UINTN Index2;
UINT64 Length;
+ BOOLEAN AdjacentRegion;
=20
- Base =3D 0;
Length =3D 0;
for (Index =3D 0; Index < NumberOfSmmReservedRegions; Index++) {
- if (Base =3D=3D 0) {
- Base =3D Descriptor[Index].PhysicalStart;
- Length =3D Descriptor[Index].PhysicalSize;
+ AdjacentRegion =3D FALSE;
+ for (Index2 =3D 0; Index2 < NumberOfSmmReservedRegions; Index2++) {
+ if ((NumberOfSmmReservedRegions =3D=3D 1)
+ || (Descriptor[Index].PhysicalStart + Descriptor[Index].Physic=
alSize =3D=3D Descriptor[Index2].PhysicalStart)
+ || (Descriptor[Index2].PhysicalStart + Descriptor[Index2].Phys=
icalSize =3D=3D Descriptor[Index].PhysicalStart)) {
+ AdjacentRegion =3D TRUE;
+ break;
+ }
+ }
+
+ if (AdjacentRegion =3D=3D TRUE) {
+ Length +=3D Descriptor[Index].PhysicalSize;
} else {
- if (Base + Length =3D=3D Descriptor[Index].PhysicalStart) {
- Length =3D Length + Descriptor[Index].PhysicalSize;
- } else if (Descriptor[Index].PhysicalStart + Descriptor[Index].Phy=
sicalSize =3D=3D Base) {
- Base =3D Descriptor[Index].PhysicalStart;
- Length =3D Length + Descriptor[Index].PhysicalSize;
- } else {
- DEBUG ((DEBUG_ERROR, "Smram is not adjacent\n"));
- TestPointLibAppendErrorString (
- PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
- NULL,
- TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_COD=
E \
- TEST_POINT_DXE_SMM_READY_TO_LOCK=20
- TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_S=
TRING
- );
- return EFI_INVALID_PARAMETER;
- }
+ DEBUG ((DEBUG_ERROR, "Smram is not adjacent\n"));
+ TestPointLibAppendErrorString (
+ PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
+ NULL,
+ TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_CODE =
\
+ TEST_POINT_DXE_SMM_READY_TO_LOCK
+ TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_STR=
ING
+ );
+ return EFI_INVALID_PARAMETER;
}
- } =20
+ }
=20
if (Length !=3D GetPowerOfTwo64 (Length)) {
DEBUG ((DEBUG_ERROR, "Smram is not aligned\n"));
@@ -94,7 +96,7 @@ CheckSmramDescriptor (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_CODE \
- TEST_POINT_DXE_SMM_READY_TO_LOCK=20
+ TEST_POINT_DXE_SMM_READY_TO_LOCK
TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_STRIN=
G
);
return EFI_INVALID_PARAMETER;
@@ -111,14 +113,14 @@ TestPointCheckSmmInfo (
EFI_SMM_ACCESS2_PROTOCOL *SmmAccess;
UINTN Size;
EFI_SMRAM_DESCRIPTOR *SmramRanges;
- =20
+
DEBUG ((DEBUG_INFO, "=3D=3D=3D=3D TestPointCheckSmmInfo - Enter\n"));
- =20
+
Status =3D gBS->LocateProtocol (&gEfiSmmAccess2ProtocolGuid, NULL, (VO=
ID **)&SmmAccess);
if (EFI_ERROR (Status)) {
goto Done ;
}
- =20
+
Size =3D 0;
Status =3D SmmAccess->GetCapabilities (SmmAccess, &Size, NULL);
ASSERT (Status =3D=3D EFI_BUFFER_TOO_SMALL);
@@ -128,7 +130,7 @@ TestPointCheckSmmInfo (
=20
Status =3D SmmAccess->GetCapabilities (SmmAccess, &Size, SmramRanges);
ASSERT_EFI_ERROR (Status);
- =20
+
DEBUG ((DEBUG_INFO, "SMRAM Info\n"));
DumpSmramDescriptor (Size / sizeof (EFI_SMRAM_DESCRIPTOR), SmramRanges=
);
=20
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v2 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index

Michael Kubacki
 

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

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

TestPointSmmEndOfDxeSmrrFunctional() uses the incorrect byte index
to skip the test. It should use byte 6 instead of byte 5.

Also defines a macro "TEST_POINT_INDEX_BYTE6_SMM" for the byte index
6 value.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPoin=
tCheckLib.c | 26 +++++++++++---------
Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h =
| 1 +
2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/SmmTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/Test=
PointCheckLib/SmmTestPointCheckLib.c
index 4b4f874c7bbc..75200969d3e7 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTes=
tPointCheckLib.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTes=
tPointCheckLib.c
@@ -109,7 +109,7 @@ TestPointSmmEndOfDxeSmrrFunctional (
EFI_STATUS Status;
BOOLEAN Result;
=20
- if ((mFeatureImplemented[5] & TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUN=
CTIONAL) =3D=3D 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM] & TEST_POINT_BYTE=
6_SMM_END_OF_DXE_SMRR_FUNCTIONAL) =3D=3D 0) {
return EFI_SUCCESS;
}
=20
@@ -125,7 +125,7 @@ TestPointSmmEndOfDxeSmrrFunctional (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 5,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL
);
}
@@ -156,7 +156,8 @@ TestPointSmmReadyToLockSmmMemoryAttributeTableFunctio=
nal (
EFI_STATUS Status;
BOOLEAN Result;
=20
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_M=
EMORY_ATTRIBUTE_TABLE_FUNCTIONAL) =3D=3D 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM] &
+ TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_MEMORY_ATTRIBUTE_TABLE_FUNCTI=
ONAL) =3D=3D 0) {
return EFI_SUCCESS;
}
=20
@@ -173,7 +174,7 @@ TestPointSmmReadyToLockSmmMemoryAttributeTableFunctio=
nal (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_MEMORY_ATTRIBUTE_TABLE_FUNC=
TIONAL
);
}
@@ -202,7 +203,8 @@ TestPointSmmReadyToLockSecureSmmCommunicationBuffer (
EFI_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable;
UINTN MemoryAttributesTableSize;
=20
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECUR=
E_SMM_COMMUNICATION_BUFFER) =3D=3D 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM] &
+ TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER) =
=3D=3D 0) {
return EFI_SUCCESS;
}
=20
@@ -248,7 +250,8 @@ TestPointSmmReadyToBootSmmPageProtection (
EFI_STATUS Status;
BOOLEAN Result;
=20
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_P=
AGE_LEVEL_PROTECTION) =3D=3D 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM]
+ & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION) =3D=3D=
0) {
return EFI_SUCCESS;
}
=20
@@ -264,7 +267,7 @@ TestPointSmmReadyToBootSmmPageProtection (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION
);
}
@@ -280,7 +283,7 @@ TestPointSmmReadyToBootSmmPageProtection (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFF=
ER
);
}
@@ -312,7 +315,8 @@ TestPointSmmReadyToBootSmmPageProtectionHandler (
TEST_POINT_SMM_COMMUNICATION_UEFI_GCD_MAP_INFO *CommData;
UINTN TempCommBufferSize=
;
=20
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_P=
AGE_LEVEL_PROTECTION) =3D=3D 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM]
+ & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION) =3D=3D=
0) {
return EFI_SUCCESS;
}
=20
@@ -390,14 +394,14 @@ TestPointSmmReadyToBootSmmPageProtectionHandler (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFF=
ER
);
} else {
TestPointLibClearFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFF=
ER
);
}
diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheck=
Lib.h b/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
index 7265e2268961..73c30522179c 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
@@ -738,6 +738,7 @@ TestPointSmmExitBootServices (
#define TEST_POINT_SMM_READY_TO_BOOT =
L" - SMM Ready To Boot - "
#define TEST_POINT_SMM_EXIT_BOOT_SERVICES =
L" - SMM Exit Boot Services - "
=20
+#define TEST_POINT_INDEX_BYTE6_SMM =
6
#define TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL =
BIT0
#define TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_MEMORY_ATTRIBUTE_TABLE_FU=
NCTIONAL BIT1
#define TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFF=
ER BIT2
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v2 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol

Michael Kubacki
 

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

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

Per the protocol definition, the caller must allocate the input
structure and set the size field. TestPointCheckTcgTrustedBoot()
does not do this which can result in an EFI_BUFFER_TOO_SMALL error.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcg=
TrustedBoot.c | 3 +++
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoin=
tCheckLib.inf | 1 +
2 files changed, 4 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeCheckTcgTrustedBoot.c b/Platform/Intel/MinPlatformPkg/Test/Library/Te=
stPointCheckLib/DxeCheckTcgTrustedBoot.c
index 2a04f86fedac..5ec32fd2e875 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckTcgTrustedBoot.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckTcgTrustedBoot.c
@@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <PiDxe.h>
#include <Library/TestPointCheckLib.h>
#include <Library/TestPointLib.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
#include <Library/PrintLib.h>
@@ -41,6 +42,8 @@ TestPointCheckTcgTrustedBoot (
goto Done;
}
=20
+ ZeroMem ((VOID *) &ProtocolCapability, sizeof (ProtocolCapability));
+ ProtocolCapability.Size =3D (UINT8) sizeof (ProtocolCapability);
Status =3D Tcg2->GetCapability (Tcg2, &ProtocolCapability);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_ERROR, "Tcg2->GetCapability - %r\n", Status));
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/Te=
stPointCheckLib/DxeTestPointCheckLib.inf
index 54b4015d6767..195729e0ff5b 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.inf
@@ -18,6 +18,7 @@ [Defines]
=20
[LibraryClasses]
BaseLib
+ BaseMemoryLib
DebugLib
DxeServicesTableLib
UefiBootServicesTableLib
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v2 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue

Michael Kubacki
 

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

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

The MessageLength field of EFI_MM_COMMUNICATE_HEADER as defined in
MdePkg/Include/Protocol/MmCommunication.h was updated to a fixed
size as opposed to UINTN to avoid ambiguity between different
caller enviornments.

This change updates the MessageLength usage in MinPlatformPkg to
support the new field structure, in turn, fixing a build issue.

Original edk2 change:
https://bugzilla.tianocore.org/show_bug.cgi?id=3D3398

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmi=
HandlerInstrument.c | 4 ++--
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoin=
tCheckLib.c | 15 ++++++++++++++-
Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c =
| 10 +++++-----
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoin=
tCheckLib.inf | 1 +
4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeCheckSmiHandlerInstrument.c b/Platform/Intel/MinPlatformPkg/Test/Libr=
ary/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c
index 3ceeb821fb95..80e8d26f4e1d 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckSmiHandlerInstrument.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckSmiHandlerInstrument.c
@@ -106,7 +106,7 @@ GetSmiHandlerProfileDatabase(
CommGetInfo->Header.ReturnStatus =3D (UINT64)-1;
CommGetInfo->DataSize =3D 0;
=20
- CommSize =3D sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLen=
gth;
+ CommSize =3D OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)Com=
mHeader->MessageLength;
Status =3D SmmCommunication->Communicate(SmmCommunication, CommBuffer,=
&CommSize);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_INFO, "SmiHandlerProfile: SmmCommunication - %r\n", St=
atus));
@@ -139,7 +139,7 @@ GetSmiHandlerProfileDatabase(
CommGetData->Header.DataLength =3D sizeof(*CommGetData);
CommGetData->Header.ReturnStatus =3D (UINT64)-1;
=20
- CommSize =3D sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLen=
gth;
+ CommSize =3D OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)Com=
mHeader->MessageLength;
Buffer =3D (UINT8 *)CommHeader + CommSize;
Size -=3D CommSize;
=20
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/Test=
PointCheckLib/DxeTestPointCheckLib.c
index c012e0afcbaa..e5efbd059954 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.c
@@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
#include <Library/BaseMemoryLib.h>
+#include <Library/SafeIntLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <IndustryStandard/Acpi.h>
#include <IndustryStandard/DmaRemappingReportingTable.h>
@@ -520,6 +521,7 @@ TestPointDxeSmmReadyToBootSmmPageProtection (
UINTN MemoryAttributesTa=
bleSize;
EFI_STATUS Status;
UINTN CommSize;
+ UINT64 LongCommSize;
UINT8 *CommBuffer;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
@@ -620,7 +622,18 @@ TestPointDxeSmmReadyToBootSmmPageProtection (
(UINTN)CommData->UefiMemoryAttributeTableSize
);
=20
- CommSize =3D OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + CommHeader-=
MessageLength;
+ Status =3D SafeUint64Add (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data)=
, CommHeader->MessageLength, &LongCommSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: Lo=
ngCommSize calculation - %r\n", Status));
+ return EFI_SUCCESS;
+ }
+
+ Status =3D SafeUint64ToUintn (LongCommSize, &CommSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: Co=
mmSize conversion - %r\n", Status));
+ return EFI_SUCCESS;
+ }
+
Status =3D SmmCommunication->Communicate(SmmCommunication, CommBuffer,=
&CommSize);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: Sm=
mCommunication - %r\n", Status));
diff --git a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPoin=
tStubDxe.c b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPoin=
tStubDxe.c
index 3cc5ccfef6f4..8416b36f56ae 100644
--- a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDx=
e.c
+++ b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDx=
e.c
@@ -122,7 +122,7 @@ GetTestPointDataSmm (
CommGetInfo->Header.ReturnStatus =3D (UINT64)-1;
CommGetInfo->DataSize =3D 0;
=20
- CommSize =3D sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLen=
gth;
+ CommSize =3D OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)Com=
mHeader->MessageLength;
Status =3D SmmCommunication->Communicate(SmmCommunication, CommBuffer,=
&CommSize);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_INFO, "SmiHandlerTestPoint: SmmCommunication - %r\n", =
Status));
@@ -155,7 +155,7 @@ GetTestPointDataSmm (
CommGetData->Header.DataLength =3D sizeof(*CommGetData);
CommGetData->Header.ReturnStatus =3D (UINT64)-1;
=20
- CommSize =3D sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLen=
gth;
+ CommSize =3D OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)Com=
mHeader->MessageLength;
Buffer =3D (UINT8 *)CommHeader + CommSize;
Size -=3D CommSize;
=20
@@ -233,7 +233,7 @@ PublishSmmTestPoint (
TestPoint =3D mSmmTestPointDatabase;
while ((UINTN)TestPoint < (UINTN)mSmmTestPointDatabase + mSmmTestPoint=
DatabaseSize) {
TestPointSize =3D GetTestPointInfoSize (TestPoint, (UINTN)mSmmTestPo=
intDatabase + mSmmTestPointDatabaseSize - (UINTN)TestPoint);
- =20
+
TestPointLibSetTable (TestPoint, TestPointSize);
=20
TestPoint =3D (ADAPTER_INFO_PLATFORM_TEST_POINT *)((UINTN)TestPoint =
+ TestPointSize);
@@ -286,7 +286,7 @@ OnReadyToBoot (
EFI_EVENT ReadyToBootLaterEvent;
=20
gBS->CloseEvent (Event);
- =20
+
Status =3D gBS->CreateEvent (
EVT_NOTIFY_SIGNAL,
TPL_CALLBACK,
@@ -295,7 +295,7 @@ OnReadyToBoot (
&ReadyToBootLaterEvent
);
ASSERT_EFI_ERROR (Status);
- =20
+
gBS->SignalEvent (ReadyToBootLaterEvent);
}
=20
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/Te=
stPointCheckLib/DxeTestPointCheckLib.inf
index 2ae1db4ee483..54b4015d6767 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.inf
@@ -32,6 +32,7 @@ [LibraryClasses]
TestPointLib
PciSegmentLib
PciSegmentInfoLib
+ SafeIntLib
=20
[Packages]
MinPlatformPkg/MinPlatformPkg.dec
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

Michael Kubacki
 

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

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3531
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3518
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3520
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3521

This patch series groups together several bug fixes and improvements
to TestPointCheckLib. The first patch is required for the others
since it fixes a MinPlatformPkg build issue that occurs with the
current edk2/master branch.

V2 changes:
1. Added Reviewed-by replies received for v1 series
2. [v1 2/5]: Added a ZeroMem() for the ProtocolCapability buffer
3. [v1 3/5]: Added a #define for the byte index 6 parameter

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Michael Kubacki (5):
MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckAcp=
i.c | 32 +++++------
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmi=
HandlerInstrument.c | 4 +-
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmm=
Info.c | 56 ++++++++++----------
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcg=
TrustedBoot.c | 3 ++
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoin=
tCheckLib.c | 15 +++++-
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPoin=
tCheckLib.c | 26 +++++----
Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c =
| 10 ++--
Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h =
| 1 +
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoin=
tCheckLib.inf | 2 +
9 files changed, 87 insertions(+), 62 deletions(-)

--=20
2.28.0.windows.1

5321 - 5340 of 84031