Date   

Re: [edk2-platforms PATCH v2] Marvell: Armada7k8k/OcteonTx: Add missing _STA methods in ACPI tables

Marcin Wojtas
 

Hi Ard,

śr., 11 sie 2021 o 12:58 Marcin Wojtas <mw@...> napisał(a):

Hi Ard,

śr., 11 sie 2021 o 12:42 Ard Biesheuvel <ardb@...> napisał(a):

On Wed, 11 Aug 2021 at 00:04, Marcin Wojtas <mw@...> wrote:

BBR 1.0 spec says that _STA is required for each device in DSDT or SSDT.
Fix that for all platforms with the Marvell SoC's.

Signed-off-by: Marcin Wojtas <mw@...>
Did you add back the _STA methods that I removed from the secondary
UARTs you introduced in the original series?
Yes, this patch adds _STA to the relevant COM2 nodes in
Armada80x0McBin and Cn913xDbA DSDT files.
Do you have any further comments to this patch?

Best regards,
Marcin


---
Changelog:
v1->v2:
* Rebase on top of tree

Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl | 56 +++++++++++++++
Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl | 76 ++++++++++++++++++++
Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl | 76 ++++++++++++++++++++
Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl | 12 ++++
Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl | 60 ++++++++++++++++
5 files changed, 280 insertions(+)

diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
index 345c1e4dd6..88e38efeeb 100644
--- a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
+++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
@@ -20,21 +20,37 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x000) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU1)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x001) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU2)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x100) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x101) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}

Device (AHC0)
@@ -42,6 +58,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
Name (_HID, "LNRO001E") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CLS, Package (0x03) // _CLS: Class Code
{
0x01,
@@ -67,6 +87,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
Name (_HID, "MRVL0002") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -96,6 +120,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
Name (_HID, "MRVL0004") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -123,6 +151,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -142,6 +174,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -160,6 +196,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
{
Name (_HID, "MRVL0001") // _HID: Hardware ID
Name (_CID, "HISI0031") // _CID: Compatible ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_ADR, FixedPcdGet64(PcdSerialRegisterBase)) // _ADR: Address
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -186,6 +226,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
{
Name (_HID, "MRVL0100") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
@@ -208,6 +252,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
Name (_HID, "MRVL0110") // _HID: Hardware ID
Name (_CCA, 0x01) // Cache-coherent controller
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xf2000000 , 0x100000)
@@ -286,6 +334,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
{
Name (_HID, "PRP0001") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xF2760000, 0x7D)
@@ -312,6 +364,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA7K", 3)
Name (_SEG, 0x00) // _SEG: PCI Segment
Name (_BBN, 0x00) // _BBN: BIOS Bus Number
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_PRT, Package () // _PRT: PCI Routing Table
{
Package () { 0xFFFF, 0x0, 0x0, 0x40 },
diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl
index 91401c74c8..77d3aebaf1 100644
--- a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl
+++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl
@@ -20,21 +20,37 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x000) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU1)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x001) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU2)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x100) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x101) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}

Device (AHC0)
@@ -42,6 +58,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "LNRO001E") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CLS, Package (0x03) // _CLS: Class Code
{
0x01,
@@ -67,6 +87,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "LNRO001E") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CLS, Package (0x03) // _CLS: Class Code
{
0x01,
@@ -92,6 +116,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0002") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -122,6 +150,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0004") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -151,6 +183,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -170,6 +206,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -189,6 +229,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x02) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -207,6 +251,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "MRVL0001") // _HID: Hardware ID
Name (_CID, "HISI0031") // _CID: Compatible ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_ADR, FixedPcdGet64(PcdSerialRegisterBase)) // _ADR: Address
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -233,6 +281,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "MRVL0100") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
@@ -251,6 +303,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0110") // _HID: Hardware ID
Name (_CCA, 0x01) // Cache-coherent controller
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xf2000000 , 0x100000)
@@ -309,6 +365,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "MRVL0100") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
@@ -327,6 +387,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0110") // _HID: Hardware ID
Name (_CCA, 0x01) // Cache-coherent controller
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xf4000000 , 0x100000)
@@ -385,6 +449,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "PRP0001") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xF2760000, 0x7D)
@@ -405,6 +473,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "PRP0001") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xF4760000, 0x7D)
@@ -431,6 +503,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_SEG, 0x00) // _SEG: PCI Segment
Name (_BBN, 0x00) // _BBN: BIOS Bus Number
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_PRT, Package () // _PRT: PCI Routing Table
{
Package () { 0xFFFF, 0x0, 0x0, 0x40 },
diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl
index 7931dc3ef8..a7d1c76e07 100644
--- a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl
+++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl
@@ -20,21 +20,37 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x000) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU1)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x001) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU2)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x100) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x101) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}

Device (AHC0)
@@ -42,6 +58,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "LNRO001E") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CLS, Package (0x03) // _CLS: Class Code
{
0x01,
@@ -92,6 +112,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0002") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -123,6 +147,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0004") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -151,6 +179,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -170,6 +202,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -189,6 +225,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x02) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -208,6 +248,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0001") // _HID: Hardware ID
Name (_CID, "HISI0031") // _CID: Compatible ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_ADR, FixedPcdGet64(PcdSerialRegisterBase)) // _ADR: Address
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -235,6 +279,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0001") // _HID: Hardware ID
Name (_CID, "HISI0031") // _CID: Compatible ID
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_ADR, ARMADA80X0_MCBIN_DBG2_UART_REG_BASE) // _ADR: Address
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -261,6 +309,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "MRVL0100") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
@@ -278,6 +330,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "MRVL0101") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
@@ -312,6 +368,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0110") // _HID: Hardware ID
Name (_CCA, 0x01) // Cache-coherent controller
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xf2000000 , 0x100000)
@@ -351,6 +411,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_HID, "MRVL0110") // _HID: Hardware ID
Name (_CCA, 0x01) // Cache-coherent controller
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xf4000000 , 0x100000)
@@ -429,6 +493,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "PRP0001") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xF2760000, 0x7D)
@@ -449,6 +517,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
{
Name (_HID, "PRP0001") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xF4760000, 0x7D)
@@ -475,6 +547,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "ARMADA8K", 3)
Name (_SEG, 0x00) // _SEG: PCI Segment
Name (_BBN, 0x00) // _BBN: BIOS Bus Number
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_PRT, Package () // _PRT: PCI Routing Table
{
Package () { 0xFFFF, 0x0, 0x0, 0x40 },
diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
index 8377b13763..d6619e367b 100644
--- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
+++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
@@ -20,6 +20,10 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2, "MVEBU ", "CN9131", 3)
Name (_HID, "LNRO001E") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CLS, Package (0x03) // _CLS: Class Code
{
0x01,
@@ -45,6 +49,10 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2, "MVEBU ", "CN9131", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x02) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -63,6 +71,10 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2, "MVEBU ", "CN9131", 3)
Name (_HID, "MRVL0110") // _HID: Hardware ID
Name (_CCA, 0x01) // Cache-coherent controller
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xf4000000 , 0x100000)
diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl
index 8c098cd14c..7335e443c6 100644
--- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl
+++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl
@@ -21,21 +21,37 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x000) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU1)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x001) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU2)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x100) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}
Device (CPU3)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x101) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
}

Device (AHC0)
@@ -43,6 +59,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "LNRO001E") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CLS, Package (0x03) // _CLS: Class Code
{
0x01,
@@ -68,6 +88,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "MRVL0003") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -99,6 +123,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "MRVL0004") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -127,6 +155,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -146,6 +178,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }

Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -165,6 +201,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "MRVL0001") // _HID: Hardware ID
Name (_CID, "HISI0031") // _CID: Compatible ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_ADR, FixedPcdGet64(PcdSerialRegisterBase)) // _ADR: Address
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -192,6 +232,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "MRVL0001") // _HID: Hardware ID
Name (_CID, "HISI0031") // _CID: Compatible ID
Name (_UID, 0x01) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_ADR, CN913X_DBG2_UART_REG_BASE) // _ADR: Address
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
@@ -218,6 +262,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
{
Name (_HID, "MRVL0100") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
@@ -240,6 +288,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_HID, "MRVL0110") // _HID: Hardware ID
Name (_CCA, 0x01) // Cache-coherent controller
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xf2000000 , 0x100000)
@@ -318,6 +370,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
{
Name (_HID, "PRP0001") // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xF2760000, 0x7D)
@@ -344,6 +400,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", "CN9130", 3)
Name (_SEG, 0x00) // _SEG: PCI Segment
Name (_BBN, 0x00) // _BBN: BIOS Bus Number
Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
+ Method (_STA) // _STA: Device status
+ {
+ Return (0xF)
+ }
Name (_PRT, Package () // _PRT: PCI Routing Table
{
Package () { 0xFFFF, 0x0, 0x0, 0x40 },
--
2.29.0


Re: [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table

Omkar Anand Kulkarni
 

Hi All,

Sorry for the [edk2-platforms] prefix attached to the subject line. This is edk2 patch series. Will fix the prefix in next version.

- Omkar.

Changes since v2:
- Addressed the comments given by Sami.
- Added Readme file with all cover letter information.
- Rebased to the latest upstream code.

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

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

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

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

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

- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
These two drivers together retrieve all possible error source descriptors of
type GHESv2 from the MM drivers implementing HEST Error Source
Descriptor
protocol. Once all the descriptors are collected HestErrorSourceDxe
appends
it to HEST table using HestTableDxe driver.

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

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

ArmPlatformPkg/ArmPlatformPkg.dec | 10 +
MdeModulePkg/MdeModulePkg.dec | 3 +
.../HestMmErrorSources/HestErrorSourceDxe.inf | 45 +++
.../HestErrorSourceStandaloneMm.inf | 51 +++
.../Universal/Apei/HestDxe/HestDxe.inf | 49 +++
.../HestMmErrorSourceCommon.h | 37 ++
.../Include/Protocol/HestErrorSourceInfo.h | 64 ++++
EmbeddedPkg/Include/Library/AcpiLib.h | 20 ++
MdeModulePkg/Include/Protocol/HestTable.h | 71 ++++
MdePkg/Include/Protocol/MmCommunication.h | 2 +
.../HestMmErrorSources/HestErrorSourceDxe.c | 309 +++++++++++++++++
.../HestErrorSourceStandaloneMm.c | 312 +++++++++++++++++
MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c | 318
++++++++++++++++++
.../Drivers/HestMmErrorSources/Readme.md | 66 ++++
14 files changed, 1357 insertions(+)
create mode 100644
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
create mode 100644
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandalone
Mm.inf
create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf
create mode 100644
ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommo
n.h
create mode 100644
ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
create mode 100644 MdeModulePkg/Include/Protocol/HestTable.h
create mode 100644
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
create mode 100644
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandalone
Mm.c
create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
create mode 100644
ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md

--
2.17.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79741): https://edk2.groups.io/g/devel/message/79741
Mute This Topic: https://groups.io/mt/85104604/4857533
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[omkar.kulkarni@...]
-=-=-=-=-=-=
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[edk2-platforms][PATCH v3 5/5] ArmPlatformPkg: Add Readme file

Omkar Anand Kulkarni
 

Added a readme file that explains the software framework for dynamic
generation of HEST table.

Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md | 66 +++++++++++++++=
+++++
1 file changed, 66 insertions(+)

diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md b/ArmPla=
tformPkg/Drivers/HestMmErrorSources/Readme.md
new file mode 100644
index 000000000000..1b6f0713cb9a
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md
@@ -0,0 +1,66 @@
+Hardware Error Source Table (HEST)[1] and Software Delegated Exception I=
nterface
+(SDEI)[2] ACPI tables are used to acomplish firmware first error handlin=
g.This
+patch series introduces a framework to build and install the HEST ACPI t=
able
+dynamically.
+
+The following figure illustrates the possible usage of the dyanamic
+generation of HEST ACPI table.
+
+ NS | S
++--------------------------------------+--------------------------------=
------+
+| | =
|
+|+-------------------------------------+---------------------+ =
|
+|| +---------------------+--------------------+| =
|
+|| | | || =
|
+|| +-----------+ |+------------------+ | +-----------------+|| +--------=
-----+|
+|| |HestTable | || HestErrorSource | | | HestErrorSource ||| | DMC-620=
||
+|| | DXE | || DXE | | | StandaloneMM ||| |Standalo=
ne MM||
+|| +-----------+ |+------------------+ | +-----------------+|| +--------=
-----+|
+|| |GHESv2 | || =
|
+|| +---------------------+--------------------+| =
|
+|| +--------------------+ | | =
|
+|| |PlatformErrorHandler| | | =
|
+|| | DXE | | | =
|
+|| +--------------------+ | | =
|
+||FF FWK | | =
|
+|+-------------------------------------+---------------------+ =
|
+| | =
|
++--------------------------------------+--------------------------------=
------+
+ |
+ Figure: Dynamic Hest Table Generation.
+
+All the hardware error sources are added to HEST table as GHESv2[3] erro=
r source
+descriptors. The framework comprises of following DXE and MM drivers:
+
+- HestTableDxe:
+ Builds HEST table header and allows appending error source descriptors=
to the
+ HEST table. Also provides protocol interface to install the built HEST=
table.
+
+- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
+ These two drivers together retrieve all possible error source descript=
ors of
+ type GHESv2 from the MM drivers implementing HEST Error Source Descrip=
tor
+ protocol. Once all the descriptors are collected HestErrorSourceDxe ap=
pends
+ it to HEST table using HestTableDxe driver.
+
+- PlatformErrorHandlerDxe:
+ Builds and installs SDEI ACPI table. This driver does not initialize(l=
oad)
+ until HestErrorSourceDxe driver has finished appending all possible GH=
ESv2
+ error source descriptors to the HEST table. Once that is complete usin=
g the
+ HestTableDxe driver it installs the HEST table.
+
+This patch series provides reference implementation for DMC-620 Dynamic =
Memory
+Controller[4] that has RAS feature enabled. This is platform code
+implemented as Standalone MM driver in edk2-platforms.
+
+References:
+[1] : ACPI 6.3, Table 18-382, Hardware Error Source Table
+[2] : SDEI Platform Design Document, revision b, 10 Appendix C, ACPI tab=
le
+ definitions for SDEI
+[3] : ACPI Reference Specification 6.3, Table 18-393 GHESv2 Structure
+[4] : DMC620 Dynamic Memory Controller, revision r1p0
+[5] : UEFI Reference Specification 2.8, Appendix N - Common Platform Err=
or
+ Record
+[6] : UEFI Reference Specification 2.8, Section N.2.5 Memory Error Secti=
on
+
+Link to github branch with the patches in this series -
+https://github.com/omkkul01/edk2/tree/ras_firmware_first_edk2
--=20
2.17.1


[edk2-platforms][PATCH v3 4/5] EmbeddedPkg: Add helpers for HEST table generation

Omkar Anand Kulkarni
 

Add helper macros for the generation of the HEST ACPI table. Macros to
initialize the HEST GHESv2 Notification Structure and Error Status
Structure are introduced.

Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
EmbeddedPkg/Include/Library/AcpiLib.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/EmbeddedPkg/Include/Library/AcpiLib.h b/EmbeddedPkg/Include/=
Library/AcpiLib.h
index c142446d9d59..6de067823011 100644
--- a/EmbeddedPkg/Include/Library/AcpiLib.h
+++ b/EmbeddedPkg/Include/Library/AcpiLib.h
@@ -22,6 +22,7 @@
#define ARM_GAS16(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 16, 0, EFI_=
ACPI_5_0_WORD, Address }
#define ARM_GAS32(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 32, 0, EFI_=
ACPI_5_0_DWORD, Address }
#define ARM_GASN(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 0, 0, EFI_=
ACPI_5_0_DWORD, Address }
+#define ARM_GAS64(Address) { EFI_ACPI_6_3_SYSTEM_MEMORY, 64, 0, EFI_=
ACPI_6_3_QWORD, Address }
=20
//
// Macros for the Multiple APIC Description Table (MADT)
@@ -89,6 +90,25 @@
WatchdogTimerGSIV, WatchdogTimerFlags =
\
}
=20
+//
+// HEST table GHESv2 type related structure.
+// Helper Macro to initialize the HEST GHESv2 Notification Structure.
+// Refer Table 18-394 in ACPI Specification, Version 6.3.
+//
+#define EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE_INIT(Type, =
\
+ PollInterval, EventId) =
\
+ { =
\
+ Type, =
\
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE), =
\
+ {0, 0, 0, 0, 0, 0, 0}, /* ConfigurationWriteEnable */ =
\
+ PollInterval, =
\
+ EventId, =
\
+ 0, /* Poll Interval Threshold Value */ =
\
+ 0, /* Poll Interval Threshold Window */ =
\
+ 0, /* Error Threshold Value */ =
\
+ 0 /* Error Threshold Window */ =
\
+ }
+
typedef
BOOLEAN
(EFIAPI *EFI_LOCATE_ACPI_CHECK) (
--=20
2.17.1


[edk2-platforms][PATCH v3 3/5] ArmPlatformPkg: retreive error source descriptors from MM

Omkar Anand Kulkarni
 

Add a driver that retreives error source descriptors from MM and
populates those into the HEST ACPI table. The error source descriptors
that are available from the MM side are retreived using MM Communicate 2
protocol.

The first call into the MM returns the size of MM Communicate buffer
required to hold all error source descriptor info. The communication
buffer of that size is then allocated and the second call into MM
returns the error source descriptors in the communication buffer.
The retreived error source descriptors are then appended to the HEST
table.

Co-authored-by: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
ArmPlatformPkg/ArmPlatformPkg.dec =
| 7 +
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf =
| 45 +++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.in=
f | 51 ++++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h =
| 37 +++
MdePkg/Include/Protocol/MmCommunication.h =
| 2 +
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c =
| 309 +++++++++++++++++++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c =
| 312 ++++++++++++++++++++
7 files changed, 763 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatfo=
rmPkg.dec
index 446d93b880f9..a400f175726e 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -52,6 +52,8 @@
=20
[Guids.common]
gArmPlatformTokenSpaceGuid =3D { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4,=
0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
+ gArmPlatformHestErrorSourcesGuid =3D { 0x76b8ab43, 0x822d, 0x4b00, { 0=
x9f, 0xd0, 0xf4, 0xa5, 0x35, 0x82, 0x47, 0x0a } }
+ gMmHestGetErrorSourceInfoGuid =3D { 0x7d602951, 0x678e, 0x4cc4, { 0x98=
, 0xd9, 0xe3, 0x76, 0x04, 0xf6, 0x93, 0x0d } }
=20
[PcdsFeatureFlag.common]
gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores|FALSE|BOO=
LEAN|0x00000004
@@ -128,5 +130,10 @@
=20
gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
=20
+[PcdsFixedAtBuild, PcdsPatchableInModule]
+ ## ACPI CPER memory space
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase|0x00000=
000|UINT64|0x00000046
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize|0x00000=
000|UINT64|0x00000047
+
[Protocols.common]
gMmHestErrorSourceDescProtocolGuid =3D { 0x560bf236, 0xa4a8, 0x4d69, {=
0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe=
.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
new file mode 100644
index 000000000000..b16ff2353aa5
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
@@ -0,0 +1,45 @@
+## @file
+# DXE driver to get secure error sources.
+#
+# DXE driver to retrieve the error source descriptors from Standalone M=
M and
+# append those to the HEST table.
+#
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION =3D 0x0001001A
+ BASE_NAME =3D HestMmErrorSourceDxe
+ FILE_GUID =3D 76b8ab43-822d-4b00-9fd0-f4a53582470=
a
+ MODULE_TYPE =3D DXE_DRIVER
+ VERSION_STRING =3D 1.0
+ ENTRY_POINT =3D HestErrorSourceInitialize
+
+[Sources.common]
+ HestErrorSourceDxe.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ DxeServicesTableLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[Guids]
+ gMmHestGetErrorSourceInfoGuid ## PRODUCES
+
+[Protocols]
+ gHestTableProtocolGuid ## CONSUMES
+ gEfiMmCommunication2ProtocolGuid
+
+[Depex]
+ gHestTableProtocolGuid AND gEfiMmCommunication2ProtocolGuid
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceSta=
ndaloneMm.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSource=
StandaloneMm.inf
new file mode 100644
index 000000000000..9d566de9bec3
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandalone=
Mm.inf
@@ -0,0 +1,51 @@
+## @file
+# HEST error source gateway Standalone MM driver.
+#
+# Collects HEST error source descriptors,by communicating with all the =
MM
+# drivers implementing the HEST error source descriptor protocol.
+#
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION =3D 0x0001001A
+ BASE_NAME =3D HestErrorSourceStandaloneMm
+ FILE_GUID =3D 3ddbebcc-9841-4ef8-87fa-305843c1922=
d
+ MODULE_TYPE =3D MM_STANDALONE
+ VERSION_STRING =3D 1.0
+ PI_SPECIFICATION_VERSION =3D 0x00010032
+ ENTRY_POINT =3D StandaloneMmHestErrorSourceInitiali=
ze
+
+[Sources]
+ HestErrorSourceStandaloneMm.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+ ArmLib
+ ArmSvcLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ StandaloneMmDriverEntryPoint
+
+[Protocols]
+ gMmHestErrorSourceDescProtocolGuid
+
+[Guids]
+ gMmHestGetErrorSourceInfoGuid ##PRODUCES
+ gEfiStandaloneMmNonSecureBufferGuid
+
+[FixedPcd]
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize
+
+[Depex]
+ TRUE
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceC=
ommon.h b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceComm=
on.h
new file mode 100644
index 000000000000..6ddc6bd21922
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
@@ -0,0 +1,37 @@
+/** @file
+ Data structures for error source descriptor information.
+
+ This data structure forms the CommBuffer part of the MM Communication
+ protocol used for communicating the Hardware Error sources form MM to
+ Non-MM environment.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef HEST_ERROR_SOURCE_DESCRIPTOR_H_
+#define HEST_ERROR_SOURCE_DESCRIPTOR_H_
+
+#define HEST_ERROR_SOURCE_DESC_INFO_SIZE \
+ (OFFSET_OF (HEST_ERROR_SOURCE_DESC_INFO, ErrSourceDescList))
+
+//
+// Data Structure to communicate the error source descriptor information=
from
+// Standalone MM.
+//
+typedef struct {
+ //
+ // Total count of error source descriptors.
+ //
+ UINTN ErrSourceDescCount;
+ //
+ // Total size of all the error source descriptors.
+ //
+ UINTN ErrSourceDescSize;
+ //
+ // Array of error source descriptors that is ErrSourceDescSize in size=
.
+ //
+ UINT8 ErrSourceDescList[1];
+} HEST_ERROR_SOURCE_DESC_INFO;
+
+#endif // HEST_ERROR_SOURCE_DESCRIPTOR_H_
diff --git a/MdePkg/Include/Protocol/MmCommunication.h b/MdePkg/Include/P=
rotocol/MmCommunication.h
index 34c3e2b5a9e3..acde54971fd9 100644
--- a/MdePkg/Include/Protocol/MmCommunication.h
+++ b/MdePkg/Include/Protocol/MmCommunication.h
@@ -12,6 +12,8 @@
#ifndef _MM_COMMUNICATION_H_
#define _MM_COMMUNICATION_H_
=20
+#define MM_COMMUNICATE_HEADER_SIZE (OFFSET_OF (EFI_MM_COMMUNICATE_HEADER=
, Data))
+
#pragma pack(1)
=20
///
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe=
.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
new file mode 100644
index 000000000000..28de8daf2f5e
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
@@ -0,0 +1,309 @@
+/** @file
+ Collects and appends the HEST error source descriptors from the MM dri=
vers.
+
+ The drivers entry point locates the MM Communication protocol and call=
s into
+ Standalone MM to get the HEST error sources length and count. It also
+ retrieves descriptor information. The information is then used to buil=
d the
+ HEST table using the HEST table generation protocol.
+
+ This driver collects the secure error source descriptor information fr=
om the
+ MM drviers that implement HEST error source protocol. Instead of direc=
tly
+ communicating with the individual MM drivers, it calls into
+ HestErrorSourceStandaloneMM driver which is a gatway MM driver. This M=
M driver
+ in-turn communicates with individual MM drivers collecting the error s=
ource
+ descriptor information.
+
+ Once all the error source descriptor information is retrieved the driv=
er
+ appends the descriptors to HEST table using the HestDxe driver.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/MmCommunication2.h>
+#include <Protocol/HestTable.h>
+#include "HestMmErrorSourceCommon.h"
+
+STATIC HEST_TABLE_PROTOCOL *mHestProtocol;
+STATIC EFI_MM_COMMUNICATION2_PROTOCOL *mMmCommunication2;
+
+/**
+ Retrieve the error source descriptors from Standalone MM.
+
+ Initialize the MM comminication buffer by assigning the MM service to
+ invoke as gMmHestGetErrorSourceInfoGuid. Use the MM communication
+ protocol to retrieve the error source descriptors.
+
+ @param[in] CommBuffSize Size of communicate buffer.
+ @param[in, out] CommBuffer The communicate buffer.
+
+ @retval EFI_SUCCESS MM Communicate protocol call successful.
+ @retval Other MM Communicate protocol call failed.
+**/
+STATIC
+EFI_STATUS
+GetErrorSourceDescriptors (
+ IN UINTN CommBuffSize,
+ IN OUT EFI_MM_COMMUNICATE_HEADER **CommBuffer
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Initialize the CommBuffer with MM Communicate metadata.
+ //
+ CopyGuid (&(*CommBuffer)->HeaderGuid, &gMmHestGetErrorSourceInfoGuid);
+ (*CommBuffer)->MessageLength =3D
+ CommBuffSize -
+ sizeof ((*CommBuffer)->HeaderGuid) -
+ sizeof ((*CommBuffer)->MessageLength);
+
+ //
+ // Call into the Standalone MM using the MM Communicate protocol.
+ //
+ Status =3D mMmCommunication2->Communicate (
+ mMmCommunication2,
+ NULL,
+ (VOID *)*CommBuffer,
+ NULL
+ );
+
+ return Status;
+}
+
+/**
+ Collect HEST error source descriptors from all Standalone MM drivers a=
nd
+ append them to the HEST table.
+
+ Use MM Communication Protocol to communicate and collect the error sou=
rce
+ descriptor information from Standalone MM. Check for the required buff=
er size
+ returned by the MM driver. Allocate buffer of adequate size and call a=
gain
+ into MM.
+
+ @retval EFI_SUCCESS Successful to collect and append the er=
ror
+ source.
+ descriptors to HEST table.
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failure.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+AppendMmErrorSources (
+ VOID
+ )
+{
+ EFI_MM_COMMUNICATE_HEADER *CommunicationHeader =3D NULL;
+ HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceDescInfo;
+ EFI_STATUS Status;
+ UINTN CommBufferSize;
+
+ //
+ // Retrieve the count, length and the actual eror source descriptors f=
rom
+ // the MM drivers. Do this by performing two MM Communicate calls, in =
the
+ // first call pass CommBuffer which is atleast of the size of error so=
urce
+ // descriptor info structure. Followed by another communicate call wit=
h
+ // CommBuffer allocated to required buffer size to hold all descriptor=
s.
+ //
+ // Allocate CommBuffer atleast the size of error source descriptor inf=
o
+ // structure.
+ CommBufferSize =3D
+ MM_COMMUNICATE_HEADER_SIZE + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
+ CommunicationHeader =3D AllocateZeroPool (CommBufferSize);
+ if (CommunicationHeader =3D=3D NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for CommunicationHeader\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Make the first MM Communicate call to HestErrorSourceStandaloneMM g=
ateway
+ // driver, which returns the required buffer size adequate to hold all=
the
+ // desctriptor information.
+ //
+ Status =3D GetErrorSourceDescriptors (CommBufferSize, &CommunicationHe=
ader);
+ if ((EFI_ERROR (Status)) &&
+ (Status !=3D EFI_BUFFER_TOO_SMALL)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: MM Communicate protocol call failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ FreePool (CommunicationHeader);
+ return Status;
+ }
+
+ // Check for the length of Error Source descriptors.
+ ErrorSourceDescInfo =3D
+ (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+ if ((ErrorSourceDescInfo->ErrSourceDescSize =3D=3D 0) ||
+ (ErrorSourceDescInfo->ErrSourceDescCount =3D=3D 0)) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HesErrorSourceDxe: HEST error source(s) not found\n"
+ ));
+ FreePool (CommunicationHeader);
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // Allocate CommBuffer of required size to accomodate all the error so=
urce
+ // descriptors. Required size of communication buffer =3D
+ // MM communicate metadata. + (error source desc info struct + error s=
ource
+ // descriptor size).
+ //
+
+ CommBufferSize =3D
+ MM_COMMUNICATE_HEADER_SIZE +
+ HEST_ERROR_SOURCE_DESC_INFO_SIZE +
+ ErrorSourceDescInfo->ErrSourceDescSize;
+
+ // Free old MM Communicate buffer and allocate a new buffer of require=
d size.
+ FreePool (CommunicationHeader);
+ CommunicationHeader =3D AllocateZeroPool (CommBufferSize);
+ if (CommunicationHeader =3D=3D NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for CommunicationHeader\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Make second MM Communicate call to HestErrorSourceStandaloneMM driv=
er to
+ // get the error source descriptors from the MM drivers.
+ //
+ Status =3D GetErrorSourceDescriptors (CommBufferSize, &CommunicationHe=
ader);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: MM Communicate protocol failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ FreePool (CommunicationHeader);
+ return Status;
+ }
+
+ //
+ // Retrieve the HEST error source descriptor information. Ensure that =
there
+ // is a valid list of error source descriptors.
+ //
+ ErrorSourceDescInfo =3D
+ (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+ if (ErrorSourceDescInfo->ErrSourceDescList =3D=3D NULL) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HestErrorSourceDxe: Error source descriptor list is empty"
+ ));
+ FreePool (CommunicationHeader);
+ return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "HestErrorSourceDxe: ErrorSources: TotalCount =3D %d TotalLength =3D=
%d \n",
+ ErrorSourceDescInfo->ErrSourceDescCount,
+ ErrorSourceDescInfo->ErrSourceDescSize
+ ));
+
+ //
+ // Append the error source descriptors to HEST table using the HEST ta=
ble
+ // generation protocol.
+ //
+ Status =3D mHestProtocol->AppendErrorSourceDescriptors (
+ ErrorSourceDescInfo->ErrSourceDescList,
+ ErrorSourceDescInfo->ErrSourceDescSize,
+ ErrorSourceDescInfo->ErrSourceDescCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to append error source(s), status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ }
+
+ FreePool (CommunicationHeader);
+ return Status;
+}
+
+/**
+ The Entry Point for HEST Error Source Dxe driver.
+
+ Locates the HEST Table generation and MM Communication2 protocols. Usi=
ng the
+ MM Communication2, the driver collects the Error Source Descriptor(s) =
from
+ Standalone MM. It then appends those Error Source Descriptor(s) to the=
Hest
+ table using the HEST Table generation protocol.
+
+ @param[in] ImageHandle The firmware allocated handle for the Efi ima=
ge.
+ @param[in] SystemTable A pointer to the Efi System Table.
+
+ @retval EFI_SUCCESS The entry point is executed successfully.
+ @retval Other Some error occurred when executing this entry po=
int.
+**/
+EFI_STATUS
+EFIAPI
+HestErrorSourceInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status =3D gBS->LocateProtocol (
+ &gHestTableProtocolGuid,
+ NULL,
+ (VOID **)&mHestProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate HEST table generation protocol, status:%r\n"=
,
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ Status =3D gBS->LocateProtocol (
+ &gEfiMmCommunication2ProtocolGuid,
+ NULL,
+ (VOID **)&mMmCommunication2
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate MMCommunication2 driver protocol, status:%r\=
n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ //
+ // Append HEST error sources retrieved from StandaloneMM, if any, into=
the
+ // HEST ACPI table.
+ //
+ Status =3D AppendMmErrorSources ();
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed appending error source desc to HEST table, status:%r\n=
",
+ __FUNCTION__,
+ Status
+ ));
+ }
+ return EFI_SUCCESS;
+}
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceSta=
ndaloneMm.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceSt=
andaloneMm.c
new file mode 100644
index 000000000000..75dc31438180
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandalone=
Mm.c
@@ -0,0 +1,312 @@
+/** @file
+ MM HEST error source gateway driver.
+
+ This MM driver installs a handler which can be used to retrieve the er=
ror
+ source descriptors from the all MM drivers implementing the HEST error=
source
+ descriptor protocol.
+
+ The MM driver acts as a single point of contact to collect secure hard=
ware
+ error sources from the MM drivers. It loops over all the MM drivers th=
at
+ implement HEST error source descriptor protocol and collects error sou=
rce
+ descriptor information along with the error source count and length.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Base.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Protocol/HestErrorSourceInfo.h>
+
+#include "HestMmErrorSourceCommon.h"
+
+STATIC EFI_MM_SYSTEM_TABLE *mMmst =3D NULL;
+
+/**
+ Returns an array of handles that implement the HEST error source descr=
iptor
+ protocol.
+
+ Passing HandleBuffer as NULL will return the actual size of the buffer
+ required to hold the array of handles implementing the protocol.
+
+ @param[in, out] HandleBufferSize The size of the HandleBuffer.
+ @param[out] HandleBuffer A pointer to the buffer containing =
the list
+ of handles.
+
+ @retval EFI_SUCCESS The array of handles returned in HandleBuffer.
+ @retval EFI_NOT_FOUND No implementation present for the protocol.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+GetHestErrorSourceProtocolHandles (
+ IN OUT UINTN *HandleBufferSize,
+ OUT EFI_HANDLE **HandleBuffer
+ )
+{
+ EFI_STATUS Status;
+
+ Status =3D mMmst->MmLocateHandle (
+ ByProtocol,
+ &gMmHestErrorSourceDescProtocolGuid,
+ NULL,
+ HandleBufferSize,
+ *HandleBuffer
+ );
+ if ((EFI_ERROR (Status)) &&
+ (Status !=3D EFI_BUFFER_TOO_SMALL))
+ {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: No implementation of MmHestErrorSourceDescProtocol found, \
+ Status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ return Status;
+}
+
+/**
+ Mmi handler to retrieve HEST error source descriptor information.
+
+ Handler for Mmi service that returns the supported HEST error source
+ descriptors in MM. This handler populates the CommBuffer with the
+ list of all error source descriptors, prepended with the length and
+ the number of descriptors populated into CommBuffer.
+
+ @param[in] DispatchHandle The unique handle assigned to this ha=
ndler by
+ MmiHandlerRegister().
+ @param[in] Context Points to an optional handler context=
that
+ is specified when the handler was reg=
istered.
+ @param[in, out] CommBuffer Buffer used for communication of HEST=
error
+ source descriptors.
+ @param[in, out] CommBufferSize The size of the CommBuffer.
+
+ @retval EFI_SUCCESS CommBuffer has valid data.
+ @retval EFI_BAD_BUFFER_SIZE CommBufferSize not adequate.
+ @retval EFI_OUT_OF_RESOURCES System out of memory resources.
+ @retval EFI_INVALID_PARAMETER Invalid CommBufferSize recieved.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HestErrorSourcesInfoMmiHandler (
+ IN EFI_HANDLE DispatchHandle,
+ IN CONST VOID *Context, OPTIONAL
+ IN OUT VOID *CommBuffer, OPTIONAL
+ IN UINTN *CommBufferSize OPTIONAL
+ )
+{
+ MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *HestErrSourceDescProtocolHandle;
+ HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceInfoList;
+ EFI_HANDLE *HandleBuffer;
+ EFI_STATUS Status;
+ UINTN HandleCount;
+ UINTN HandleBufferSize;
+ UINTN Index;
+ UINTN SourceCount =3D 0;
+ UINTN SourceLength =3D 0;
+ VOID *ErrorSourcePtr;
+ UINTN TotalSourceLength =3D 0;
+ UINTN TotalSourceCount =3D 0;
+
+ if (*CommBufferSize < HEST_ERROR_SOURCE_DESC_INFO_SIZE) {
+ //
+ // Ensures that the communication buffer has enough space to atleast=
hold
+ // the ErrSourceDescCount and ErrSourceDescSize elements of the
+ // HEST_ERROR_SOURCE_DESC_INFO structure.
+ //
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Invalid CommBufferSize parameter\n",
+ __FUNCTION__
+ ));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Get all handles that implement the HEST error source descriptor pro=
tocol.
+ // Get the buffer size required to store list of handles for the proto=
col.
+ //
+ HandleBuffer =3D NULL;
+ HandleBufferSize =3D 0;
+ Status =3D GetHestErrorSourceProtocolHandles (&HandleBufferSize, &Hand=
leBuffer);
+ if ((Status =3D=3D EFI_NOT_FOUND) ||
+ (HandleBufferSize =3D=3D 0))
+ {
+ return Status;
+ }
+
+ // Allocate memory for HandleBuffer of size HandleBufferSize.
+ HandleBuffer =3D AllocateZeroPool (HandleBufferSize);
+ if (HandleBuffer =3D=3D NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for HandleBuffer\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ // Get the list of handles.
+ Status =3D GetHestErrorSourceProtocolHandles (&HandleBufferSize, &Hand=
leBuffer);
+ if ((EFI_ERROR (Status)) ||
+ (HandleBuffer =3D=3D NULL))
+ {
+ FreePool (HandleBuffer);
+ return Status;
+ }
+
+ // Count of handles for the protocol.
+ HandleCount =3D HandleBufferSize / sizeof (EFI_HANDLE);
+
+ //
+ // Loop to get the count and length of the error source descriptors.
+ //
+ // This loop collects and adds the length of error source descriptors =
and
+ // its count from all the the MM drivers implementing HEST error sourc=
e.
+ // descriptor protocol. The total length and count values retrieved he=
lp
+ // to determine weather the CommBuffer is big enough to hold the descr=
iptor
+ // information.
+ // As mentioned in the HEST error source descriptor protocol definitio=
n,
+ // Buffer parameter set to NULL ensures only length and the count valu=
es
+ // are returned from the driver and no error source information is cop=
ied to
+ // Buffer.
+ //
+ for (Index =3D 0; Index < HandleCount; ++Index) {
+ Status =3D mMmst->MmHandleProtocol (
+ HandleBuffer[Index],
+ &gMmHestErrorSourceDescProtocolGuid,
+ (VOID **)&HestErrSourceDescProtocolHandle
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ //
+ // Protocol called with Buffer parameter passed as NULL, must return
+ // error source length and error count for that driver.
+ //
+ Status =3D HestErrSourceDescProtocolHandle->GetHestErrorSourceDescri=
ptors (
+ HestErrSourceDescProtoco=
lHandle,
+ NULL,
+ &SourceLength,
+ &SourceCount
+ );
+ if (Status =3D=3D EFI_INVALID_PARAMETER) {
+ TotalSourceLength +=3D SourceLength;
+ TotalSourceCount +=3D SourceCount;
+ }
+ }
+
+ // Set the count and length in the error source descriptor.
+ ErrorSourceInfoList =3D (HEST_ERROR_SOURCE_DESC_INFO *)(CommBuffer);
+ ErrorSourceInfoList->ErrSourceDescCount =3D TotalSourceCount;
+ ErrorSourceInfoList->ErrSourceDescSize =3D TotalSourceLength;
+
+ //
+ // Check the size of CommBuffer, it should atleast be of size
+ // TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE.
+ //
+ TotalSourceLength =3D TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_=
SIZE;
+ if ((*CommBufferSize) < TotalSourceLength) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Invalid CommBufferSize parameter\n",
+ __FUNCTION__
+ ));
+ FreePool (HandleBuffer);
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ //
+ // CommBuffer size is adequate to return all the error source descript=
ors.
+ // So go ahead and populate it with the error source descriptor inform=
ation.
+ //
+
+ // Buffer pointer to append the Error Descriptors data.
+ ErrorSourcePtr =3D ErrorSourceInfoList->ErrSourceDescList;
+
+ //
+ // Loop to retrieve error source descriptors information.
+ //
+ // Calls into each MM driver that implement the HEST error source desc=
riptor
+ // protocol. Here the Buffer parameter passed to the protocol service =
is
+ // valid. So the MM driver when called copies the descriptor informati=
on.
+ //
+ for (Index =3D 0; Index < HandleCount; ++Index) {
+ Status =3D mMmst->MmHandleProtocol (
+ HandleBuffer[Index],
+ &gMmHestErrorSourceDescProtocolGuid,
+ (VOID **)&HestErrSourceDescProtocolHandle
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ Status =3D HestErrSourceDescProtocolHandle->GetHestErrorSourceDescri=
ptors (
+ HestErrSourceDescProtoco=
lHandle,
+ (VOID **)&ErrorSourcePtr=
,
+ &SourceLength,
+ &SourceCount
+ );
+ if (Status =3D=3D EFI_SUCCESS) {
+ ErrorSourcePtr +=3D SourceLength;
+ }
+ }
+
+ // Free the buffer holding all the protocol handles.
+ FreePool (HandleBuffer);
+
+ return Status;
+}
+
+/**
+ Entry point for this Stanalone MM driver.
+
+ Registers an Mmi handler that retrieves the error source descriptors f=
rom all
+ the MM drivers implementing the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL.
+
+ @param[in] ImageHandle The firmware allocated handle for the EFI ima=
ge.
+ @param[in] SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The entry point registered handler successfully.
+ @retval Other Some error occurred when executing this entry po=
int.
+**/
+EFI_STATUS
+EFIAPI
+StandaloneMmHestErrorSourceInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_MM_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_HANDLE DispatchHandle;
+ EFI_STATUS Status;
+
+ ASSERT (SystemTable !=3D NULL);
+ mMmst =3D SystemTable;
+
+ Status =3D mMmst->MmiHandlerRegister (
+ HestErrorSourcesInfoMmiHandler,
+ &gMmHestGetErrorSourceInfoGuid,
+ &DispatchHandle
+ );
+ if (EFI_ERROR(Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Mmi handler registration failed with status : %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ return Status;
+}
--=20
2.17.1


[edk2-platforms][PATCH v3 2/5] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL

Omkar Anand Kulkarni
 

Add the protocol definition of the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
protocol. This protocol can be implemented by MM drivers to publish
error source descriptors that have to be populated into HEST table.

Co-authored-by: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
ArmPlatformPkg/ArmPlatformPkg.dec | 3 +
ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h | 64 +++++++++++++=
+++++++
2 files changed, 67 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatfo=
rmPkg.dec
index 3a25ddcdc8ca..446d93b880f9 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -127,3 +127,6 @@
gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|300000000|UINT32|0x0=
0000022
=20
gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
+
+[Protocols.common]
+ gMmHestErrorSourceDescProtocolGuid =3D { 0x560bf236, 0xa4a8, 0x4d69, {=
0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
diff --git a/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h b/ArmP=
latformPkg/Include/Protocol/HestErrorSourceInfo.h
new file mode 100644
index 000000000000..86662ea7af57
--- /dev/null
+++ b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
@@ -0,0 +1,64 @@
+/** @file
+ MM protocol to get the secure error source descriptor information.
+
+ MM Drivers must implement this protocol in order to publish secure sid=
e
+ error source descriptor information to OSPM through the HEST ACPI tabl=
e.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MM_HEST_ERROR_SOURCE_DESC_
+#define MM_HEST_ERROR_SOURCE_DESC_
+
+#define MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_GUID \
+ { \
+ 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x=
9d, 0x91 } \
+ }
+
+typedef struct MmHestErrorSourceDescProtocol
+ MM_HEST_ERROR_SOURCE_DESC_PROTOCOL;
+
+/**
+ Get HEST Secure Error Source Descriptors.
+
+ The MM drivers implementing this protocol must convey the total count =
and
+ total length of the error sources the driver has along with the actual=
error
+ source descriptor(s).
+
+ Passing NULL as Buffer parameter shall return EFI_INVALID_PARAMETR wit=
h the
+ total length and count of the error source descriptor(s) it supports.
+
+ @param[in] This MM_HEST_ERROR_SOURCE_DESC_PROTOCOL in=
stance.
+ @param[out] Buffer Buffer to be appended with the error
+ source descriptors information.
+ @param[out] ErrorSourcesLength Total length of all the error source
+ descriptors.
+ @param[out] ErrorSourceCount Count of total error source descripto=
rs
+ supported by the driver.
+
+ retval EFI_SUCCESS If the Buffer is valid and is filled wit=
h valid
+ Error Source descriptor data.
+ retval EFI_INVALID_PARAMTER Buffer is NULL.
+ retval Other If no error source descriptor informatio=
n is
+ available.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS) (
+ IN MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *This,
+ OUT VOID **Buffer,
+ OUT UINTN *ErrorSourcesLength,
+ OUT UINTN *ErrorSourcesCount
+ );
+
+//
+// Protocol declaration
+//
+struct MmHestErrorSourceDescProtocol {
+ MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS GetHestErrorSourceDescriptors;
+};
+
+extern EFI_GUID gMmHestErrorSourceDescProtocolGuid;
+
+#endif // MM_HEST_ERROR_SOURCE_DESC_
--=20
2.17.1


[edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of HEST ACPI table

Omkar Anand Kulkarni
 

Introduce the HEST table generation protocol that allows platforms to
build the table with multiple error source descriptors and install the
table. The protocol provides two interfaces. The first interface allows
for adding multiple error source descriptors into the HEST table. The
second interface can then be used to dynamically install the fully
populated HEST table. This allows multiple drivers and/or libraries to
dynamically register error source descriptors into the HEST table.

Co-authored-by: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
MdeModulePkg/MdeModulePkg.dec | 3 +
MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf | 49 +++
MdeModulePkg/Include/Protocol/HestTable.h | 71 +++++
MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c | 318 ++++++++++++++++++=
++
4 files changed, 441 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.de=
c
index ad84421cf3f7..2cb4ba69d6bf 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -663,6 +663,9 @@
## Include/Protocol/VariablePolicy.h
gEdkiiVariablePolicyProtocolGuid =3D { 0x81D1675C, 0x86F6, 0x48DF, { 0=
xBD, 0x95, 0x9A, 0x6E, 0x4F, 0x09, 0x25, 0xC3 } }
=20
+ ## Arm Platform HEST table generation protocol
+ gHestTableProtocolGuid =3D { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d,=
0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
+
[PcdsFeatureFlag]
## Indicates if the platform can support update capsule across a syste=
m reset.<BR><BR>
# TRUE - Supports update capsule across a system reset.<BR>
diff --git a/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf b/MdeModuleP=
kg/Universal/Apei/HestDxe/HestDxe.inf
new file mode 100644
index 000000000000..91c7385bf7ff
--- /dev/null
+++ b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf
@@ -0,0 +1,49 @@
+## @file
+# Dxe driver that creates and publishes the HEST table.
+#
+# This driver creates HEST header and provides protocol service to appe=
nd
+# and install the HEST table.
+#
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION =3D 0x0001001A
+ BASE_NAME =3D HestDxe
+ FILE_GUID =3D 933099a2-ef71-4e00-82aa-a79b1e0a3b3=
8
+ MODULE_TYPE =3D DXE_DRIVER
+ VERSION_STRING =3D 1.0
+ ENTRY_POINT =3D HestInitialize
+
+[Sources.Common]
+ HestDxe.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+ ArmLib
+ BaseMemoryLib
+ DebugLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[Protocols]
+ gEfiAcpiTableProtocolGuid ## PROTOCOL ALWAYS_CONSUMED
+ gHestTableProtocolGuid ## PRODUCES
+
+[FixedPcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
+
+[Depex]
+ TRUE
diff --git a/MdeModulePkg/Include/Protocol/HestTable.h b/MdeModulePkg/Inc=
lude/Protocol/HestTable.h
new file mode 100644
index 000000000000..3b2e1f7d9203
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/HestTable.h
@@ -0,0 +1,71 @@
+/** @file
+ Builds and installs the HEST ACPI table.
+
+ Define the protocol interface that allows HEST ACPI table to be create=
d,
+ populated with error record descriptions and installation of the HEST =
ACPI
+ table.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef HEST_TABLE_H_
+#define HEST_TABLE_H_
+
+#define HEST_TABLE_PROTOCOL_GUID \
+ { \
+ 0x705bdcd9, 0x8c47, 0x457e, \
+ { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } \
+ }
+
+/**
+ Append HEST error source descriptor protocol service.
+
+ Protocol service used to append newly collected error source descripto=
rs to
+ to an already created HEST table.
+
+ @param[in] ErrorSourceDescriptorList List of Error Source Descri=
ptors.
+ @param[in] ErrorSourceDescriptorListSize Total Size of Error Source
+ Descriptors.
+ @param[in] ErrorSourceDescriptorCount Total count of error source
+ descriptors.
+
+ @retval EFI_SUCCESS Appending the error source descriptors
+ successful.
+ @retval EFI_OUT_OF_RESOURCES Buffer reallocation failed for the Hes=
t
+ table.
+ @retval EFI_INVALID_PARAMETER Null ErrorSourceDescriptorList param o=
r
+**/
+typedef
+EFI_STATUS
+(EFIAPI *APPEND_ERROR_SOURCE_DESCRIPTOR) (
+ IN CONST VOID *ErrorSourceDescriptorList,
+ IN UINTN ErrorSourceDescriptorListSize,
+ IN UINTN ErrorSourceDescriptorCount
+ );
+
+/**
+ Install HEST table protocol service.
+
+ Installs the HEST table that has been appended with the error source
+ descriptors. The checksum of this table is calculated and updated in
+ the table header. The HEST table is then installed.
+
+ @retval EFI_SUCCESS HEST table is installed successfully.
+ @retval EFI_ABORTED HEST table is NULL.
+ @retval Other Install service call failed.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *INSTALL_HEST_TABLE) (VOID);
+
+//
+// HEST_TABLE_PROTOCOL enables creation and installation of HEST table
+//
+typedef struct {
+ APPEND_ERROR_SOURCE_DESCRIPTOR AppendErrorSourceDescriptors;
+ INSTALL_HEST_TABLE InstallHestTable;
+} HEST_TABLE_PROTOCOL;
+
+extern EFI_GUID gHestTableProtocolGuid;
+#endif // HEST_TABLE_H_
diff --git a/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c b/MdeModulePkg=
/Universal/Apei/HestDxe/HestDxe.c
new file mode 100644
index 000000000000..87f21acf61f4
--- /dev/null
+++ b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
@@ -0,0 +1,318 @@
+/** @file
+ Builds and installs the HEST ACPI table.
+
+ This driver installs a protocol that can be used to create and install=
a HEST
+ ACPI table. The protocol allows one or more error source producers to =
add the
+ error source descriptors into the HEST table. It also allows the resul=
ting
+ HEST table to be installed.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - ACPI 6.3, Table 18-382, Hardware Error Source Table
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/AcpiSystemDescriptionTable.h>
+#include <Protocol/AcpiTable.h>
+#include <Protocol/HestTable.h>
+
+typedef struct {
+ VOID *HestTable; /// Pointer to HEST table.
+ UINT32 CurrentTableSize; /// Current size of HEST table.
+} HEST_DXE_DRIVER_DATA;
+
+STATIC EFI_ACPI_TABLE_PROTOCOL *mAcpiTableProtocol =3D NULL;
+STATIC HEST_DXE_DRIVER_DATA mHestDriverData;
+
+/**
+ Allocate memory for the HEST table header and populate it.
+
+ Helper function for this driver, populates the HEST table header. Call=
ed once
+ in the beginning on first invocation of append error source descriptor
+ protocol service.
+
+ @retval EFI_SUCCESS On successful creation of HEST header.
+ @retval EFI_OUT_OF_RESOURCES If system is out of memory recources.
+**/
+STATIC
+EFI_STATUS
+BuildHestHeader (
+ VOID
+ )
+{
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeader;
+
+ //
+ // Allocate memory for the HEST table header.
+ //
+ mHestDriverData.HestTable =3D
+ AllocateZeroPool (sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_H=
EADER));
+ if (mHestDriverData.HestTable =3D=3D NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ mHestDriverData.CurrentTableSize =3D
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER);
+
+ HestHeader =3D (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+ mHestDriverData.HestTable;
+
+ //
+ // Populate the values of the HEST table header.
+ //
+ HestHeader->Header.Signature =3D
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_SIGNATURE;
+ HestHeader->Header.Revision =3D
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_REVISION;
+ CopyMem (
+ &HestHeader->Header.OemId,
+ FixedPcdGetPtr (PcdAcpiDefaultOemId),
+ sizeof (HestHeader->Header.OemId)
+ );
+ HestHeader->Header.OemTableId =3D FixedPcdGet64 (PcdAcpiDefaultOemTabl=
eId);
+ HestHeader->Header.OemRevision =3D PcdGet32 (PcdAcpiDefaultOemRevision=
);
+ HestHeader->Header.CreatorId =3D PcdGet32 (PcdAcpiDefaultCreatorId);
+ HestHeader->Header.CreatorRevision =3D PcdGet32 (PcdAcpiDefaultCreator=
Revision);
+ HestHeader->ErrorSourceCount =3D 0;
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Append HEST error source descriptor protocol service.
+
+ Protocol service used to append newly collected error source descripto=
rs to
+ to an already created HEST table.
+
+ @param[in] ErrorSourceDescriptorList List of Error Source Descri=
ptors.
+ @param[in] ErrorSourceDescriptorListSize Total Size of Error Source
+ Descriptors.
+ @param[in] ErrorSourceDescriptorCount Total count of error source
+ descriptors.
+
+ @retval EFI_SUCCESS Appending the error source descriptors
+ successful.
+ @retval EFI_OUT_OF_RESOURCES Buffer reallocation failed for the Hes=
t
+ table.
+ @retval EFI_INVALID_PARAMETER Null ErrorSourceDescriptorList param o=
r
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AppendErrorSourceDescriptor (
+ IN CONST VOID *ErrorSourceDescriptorList,
+ IN UINTN ErrorSourceDescriptorListSize,
+ IN UINTN ErrorSourceDescriptorCount
+ )
+{
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeaderPtr;
+ EFI_STATUS Status;
+ UINT32 NewTableSize;
+ VOID *ErrorDescriptorPtr;
+
+ if ((ErrorSourceDescriptorList =3D=3D NULL) ||
+ (ErrorSourceDescriptorListSize =3D=3D 0)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Create a HEST table header if not already created.
+ //
+ if (mHestDriverData.HestTable =3D=3D NULL) {
+ Status =3D BuildHestHeader ();
+ if (Status =3D=3D EFI_OUT_OF_RESOURCES) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to build HEST header, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
+
+ //
+ // Resize the existing HEST table buffer to accommodate the incoming e=
rror
+ // source descriptors.
+ //
+ NewTableSize =3D mHestDriverData.CurrentTableSize +
+ ErrorSourceDescriptorListSize;
+ mHestDriverData.HestTable =3D ReallocatePool (
+ mHestDriverData.CurrentTableSize,
+ NewTableSize,
+ mHestDriverData.HestTable
+ );
+ if (mHestDriverData.HestTable =3D=3D NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to reallocate memory for HEST table\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Copy the incoming error source descriptors into HEST table.
+ //
+ ErrorDescriptorPtr =3D (VOID *)mHestDriverData.HestTable +
+ mHestDriverData.CurrentTableSize;
+ HestHeaderPtr =3D (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+ mHestDriverData.HestTable;
+ CopyMem (
+ ErrorDescriptorPtr,
+ ErrorSourceDescriptorList,
+ ErrorSourceDescriptorListSize
+ );
+ mHestDriverData.CurrentTableSize =3D NewTableSize;
+ HestHeaderPtr->Header.Length =3D mHestDriverData.CurrentTableSize;
+ HestHeaderPtr->ErrorSourceCount +=3D ErrorSourceDescriptorCount;
+
+ DEBUG ((
+ DEBUG_INFO,
+ "HestDxe: %d Error source descriptor(s) added \n",
+ ErrorSourceDescriptorCount
+ ));
+ return EFI_SUCCESS;
+}
+
+/**
+ Install HEST table protocol service.
+
+ Installs the HEST table that has been appended with the error source
+ descriptors. The checksum of this table is calculated and updated in
+ the table header. The HEST table is then installed.
+
+ @retval EFI_SUCCESS HEST table is installed successfully.
+ @retval EFI_ABORTED HEST table is NULL.
+ @retval Other Install service call failed.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InstallHestAcpiTable (
+ VOID
+ )
+{
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeader;
+ EFI_STATUS Status;
+ UINTN AcpiTableHandle;
+
+ //
+ // Check if we have valid HEST table data. If not, there no hardware e=
rror
+ // sources supported by the platform and no HEST table to publish, ret=
urn.
+ //
+ if (mHestDriverData.HestTable =3D=3D NULL) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HestDxe: No data available to generate HEST table\n"
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // Valid data for HEST table found. Update the header checksum and ins=
tall the
+ // HEST table.
+ //
+ HestHeader =3D (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+ mHestDriverData.HestTable;
+
+ Status =3D mAcpiTableProtocol->InstallAcpiTable (
+ mAcpiTableProtocol,
+ HestHeader,
+ HestHeader->Header.Length,
+ &AcpiTableHandle
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: HEST table installation failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ //
+ // Free the HEST table buffer.
+ //
+ FreePool (mHestDriverData.HestTable);
+ DEBUG ((
+ DEBUG_INFO,
+ "HestDxe: Installed HEST table \n"
+ ));
+ return Status;
+}
+
+//
+// HEST table generation protocol instance.
+//
+STATIC HEST_TABLE_PROTOCOL mHestProtocol =3D {
+ AppendErrorSourceDescriptor,
+ InstallHestAcpiTable
+};
+
+/**
+ The Entry Point for HEST Dxe driver.
+
+ This function installs the HEST table creation and installation protoc=
ol
+ services.
+
+ @param[in] ImageHandle Handle to the Efi image.
+ @param[in] SystemTable A pointer to the Efi System Table.
+
+ @retval EFI_SUCCESS On successful installation of protocol services=
and
+ location the ACPI table protocol.
+ @retval Other On Failure to locate ACPI table protocol or ins=
tall
+ of HEST table generation protocol.
+**/
+EFI_STATUS
+EFIAPI
+HestInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_HANDLE Handle =3D NULL;
+ EFI_STATUS Status;
+
+ Status =3D gBS->LocateProtocol (
+ &gEfiAcpiTableProtocolGuid,
+ NULL,
+ (VOID **)&mAcpiTableProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate ACPI table protocol, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ Status =3D gBS->InstallProtocolInterface (
+ &Handle,
+ &gHestTableProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mHestProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to install HEST table generation protocol status: %r\n=
",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ return Status;
+}
--=20
2.17.1


[edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table

Omkar Anand Kulkarni
 

Changes since v2:
- Addressed the comments given by Sami.
- Added Readme file with all cover letter information.
- Rebased to the latest upstream code.

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

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

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

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

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

- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
These two drivers together retrieve all possible error source descripto=
rs of
type GHESv2 from the MM drivers implementing HEST Error Source Descript=
or
protocol. Once all the descriptors are collected HestErrorSourceDxe app=
ends
it to HEST table using HestTableDxe driver.

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

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

ArmPlatformPkg/ArmPlatformPkg.dec | 10 +
MdeModulePkg/MdeModulePkg.dec | 3 +
.../HestMmErrorSources/HestErrorSourceDxe.inf | 45 +++
.../HestErrorSourceStandaloneMm.inf | 51 +++
.../Universal/Apei/HestDxe/HestDxe.inf | 49 +++
.../HestMmErrorSourceCommon.h | 37 ++
.../Include/Protocol/HestErrorSourceInfo.h | 64 ++++
EmbeddedPkg/Include/Library/AcpiLib.h | 20 ++
MdeModulePkg/Include/Protocol/HestTable.h | 71 ++++
MdePkg/Include/Protocol/MmCommunication.h | 2 +
.../HestMmErrorSources/HestErrorSourceDxe.c | 309 +++++++++++++++++
.../HestErrorSourceStandaloneMm.c | 312 +++++++++++++++++
MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c | 318 ++++++++++++++++++
.../Drivers/HestMmErrorSources/Readme.md | 66 ++++
14 files changed, 1357 insertions(+)
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSo=
urceDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSo=
urceStandaloneMm.inf
create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmError=
SourceCommon.h
create mode 100644 ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
create mode 100644 MdeModulePkg/Include/Protocol/HestTable.h
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSo=
urceDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSo=
urceStandaloneMm.c
create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md

--=20
2.17.1


Re: [PATCH v2 4/4] ArmPlatformPkg: Add helpers for HEST table generation

Omkar Anand Kulkarni
 

Hi Sami,

Thanks for reviewing this patch. Please find my response inline.

Regards,
Omkar

Hi Omkar,

Thank you for this patch.

I have a minor suggestion marked inline as [SAMI], other than that this patch
looks good to me.

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

Regards,

Sami Mujawar


On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Add helper macros for the generation of the HEST ACPI table. Macros to
initialize the HEST GHESv2 Notification Structure and Error Status
Structure are introduced.

Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
ArmPlatformPkg/Include/HestAcpiHeader.h | 49 ++++++++++++++++++++
1 file changed, 49 insertions(+)

diff --git a/ArmPlatformPkg/Include/HestAcpiHeader.h
b/ArmPlatformPkg/Include/HestAcpiHeader.h
new file mode 100644
index 000000000000..5112ee5b22c5
--- /dev/null
+++ b/ArmPlatformPkg/Include/HestAcpiHeader.h
@@ -0,0 +1,49 @@
+/** @file
+ HEST table helper macros.
+
+ Macro definitions to initialize the HEST ACPI table specific structures.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - ACPI Reference Specification 6.3
+ - UEFI Reference Specification 2.8 **/
+
+#ifndef HEST_ACPI_HEADER_
+#define HEST_ACPI_HEADER_
+
+#include <IndustryStandard/Acpi.h>
+
+//
+// HEST table GHESv2 type related structures.
+//
+// Helper Macro to initialize the HEST GHESv2 Notification Structure.
+// Refer Table 18-394 in ACPI Specification, Version 6.3.
+#define
EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE_INIT(Type,
\
+ PollInterval, EventId) \
+ { \
+ Type, \
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE),
\
+ {0, 0, 0, 0, 0, 0, 0}, /* ConfigurationWriteEnable */ \
+ PollInterval, \
+ EventId, \
+ 0, /* Poll Interval Threshold Value */ \
+ 0, /* Poll Interval Threshold Window */ \
+ 0, /* Error Threshold Value */ \
+ 0 /* Error Threshold Window */ \
+ }
+
+// Helper Macro to initialize the HEST GHESv2 Error Status Structure.
+// Refer Section 5.2.3.2 in ACPI Specification, Version 6.3.
+#define
EFI_ACPI_6_3_GENERIC_ERROR_STATUS_STRUCTURE_INIT(Address) \
[SAMI] Would it be possible to define ARM_GAS64() in
EmbeddedPkg\Include\Library\AcpiLib.h instead of this macro?
Similarly, can
EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE_INIT()
macro also be placed in EmbeddedPkg\Include\Library\AcpiLib.h
[/SAMI]
Ack.

- Omkar

+ { \
+ 0, /* UINT8 Address Space ID */ \
+ 64, /* Register Bit Width */ \
+ 0, /* Register Bit Offset */ \
+ 4, /* Access Size */ \
+ Address /* CPER/Read Ack Addr */ \
+ }
+
+#endif /* HEST_ACPI_HEADER_ */


Re: [PATCH v2 3/4] ArmPlatformPkg: retreive error source descriptors from MM

Omkar Anand Kulkarni
 

Hi Sami,

Thanks for the patch review. Please find my response inline.

Regards,
Omkar

Hi Omkar,
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar

On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Add a driver that retreives error source descriptors from MM and
populates those into the HEST ACPI table. The error source descriptors
that are available from the MM side are retreived using MM Communicate 2
protocol.

The first call into the MM returns the size of MM Communicate buffer
required to hold all error source descriptor info. The communication
buffer of that size is then allocated and the second call into MM
returns the error source descriptors in the communication buffer.
The retreived error source descriptors are then appended to the HEST
table.

Co-authored-by: Thomas Abraham mailto:thomas.abraham@...
Signed-off-by: Omkar Anand Kulkarni mailto:omkar.kulkarni@...
---
ArmPlatformPkg/ArmPlatformPkg.dec | 7 +
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf | 44 +++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf | 51 ++++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h | 37 +++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c | 308 +++++++++++++++++++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c | 312 ++++++++++++++++++++
[SAMI] Should this patch be split into 2?

The reason of keep this a single patch is the 2 drivers, Dxe and its MM counterpart work together to collect the error source descriptors.

Thought its logical to keep them together as they contribute to a single task which is collection of secure error source descriptors.

6 files changed, 759 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 4f062292663b..846b3e863aa9 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -52,6 +52,8 @@

[Guids.common]
gArmPlatformTokenSpaceGuid = { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
+ gArmPlatformHestErrorSourcesGuid = { 0x76b8ab43, 0x822d, 0x4b00, { 0x9f, 0xd0, 0xf4, 0xa5, 0x35, 0x82, 0x47, 0x0a } }
+ gMmHestGetErrorSourceInfoGuid = { 0x7d602951, 0x678e, 0x4cc4, { 0x98, 0xd9, 0xe3, 0x76, 0x04, 0xf6, 0x93, 0x0d } }

[PcdsFeatureFlag.common]
gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores|FALSE|BOOLEAN|0x00000004
@@ -128,6 +130,11 @@

gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033

+[PcdsFixedAtBuild, PcdsPatchableInModule]
+ ## ACPI CPER memory space
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase|0x00000000|UINT64|0x00000046
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize|0x00000000|UINT64|0x00000047
+
[Protocols.common]
## Arm Platform HEST table generation protocol
gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
new file mode 100644
index 000000000000..5227dea91630
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
@@ -0,0 +1,44 @@
+## @file
+# DXE driver to get secure error sources.
+#
+# DXE driver to retrieve the error source descriptors from Standalone MM and
+# append those to the HEST table.
+#
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = HestMmErrorSourceDxe
+ FILE_GUID = 76b8ab43-822d-4b00-9fd0-f4a53582470a
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = HestErrorSourceInitialize
+
+[Sources.common]
+ HestErrorSourceDxe.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdePkg/MdePkg.dec
+ StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ DxeServicesTableLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[Guids]
+ gMmHestGetErrorSourceInfoGuid ## PRODUCES
+
+[Protocols]
+ gHestTableProtocolGuid ## CONSUMES
+ gEfiMmCommunication2ProtocolGuid
+
+[Depex]
+ gHestTableProtocolGuid AND gEfiMmCommunication2ProtocolGuid
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
new file mode 100644
index 000000000000..9d566de9bec3
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
@@ -0,0 +1,51 @@
+## @file
+# HEST error source gateway Standalone MM driver.
+#
+# Collects HEST error source descriptors,by communicating with all the MM
+# drivers implementing the HEST error source descriptor protocol.
+#
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = HestErrorSourceStandaloneMm
+ FILE_GUID = 3ddbebcc-9841-4ef8-87fa-305843c1922d
+ MODULE_TYPE = MM_STANDALONE
+ VERSION_STRING = 1.0
+ PI_SPECIFICATION_VERSION = 0x00010032
+ ENTRY_POINT = StandaloneMmHestErrorSourceInitialize
+
+[Sources]
+ HestErrorSourceStandaloneMm.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+ ArmLib
+ ArmSvcLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ StandaloneMmDriverEntryPoint
+
+[Protocols]
+ gMmHestErrorSourceDescProtocolGuid
+
+[Guids]
+ gMmHestGetErrorSourceInfoGuid ##PRODUCES
+ gEfiStandaloneMmNonSecureBufferGuid
+
+[FixedPcd]
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize
+
+[Depex]
+ TRUE
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
new file mode 100644
index 000000000000..6ddc6bd21922
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
@@ -0,0 +1,37 @@
+/** @file
+ Data structures for error source descriptor information.
+
+ This data structure forms the CommBuffer part of the MM Communication
+ protocol used for communicating the Hardware Error sources form MM to
+ Non-MM environment.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef HEST_ERROR_SOURCE_DESCRIPTOR_H_
+#define HEST_ERROR_SOURCE_DESCRIPTOR_H_
+
+#define HEST_ERROR_SOURCE_DESC_INFO_SIZE \
+ (OFFSET_OF (HEST_ERROR_SOURCE_DESC_INFO, ErrSourceDescList))
[SAMI] I feel there can be a simple way to do this, see the comments below.

Okay, please see comment below.

- Omkar

+
+//
+// Data Structure to communicate the error source descriptor information from
+// Standalone MM.
+//
+typedef struct {
+ //
+ // Total count of error source descriptors.
+ //
+ UINTN ErrSourceDescCount;
+ //
+ // Total size of all the error source descriptors.
+ //
[SAMI] Does the Total size also include the size of ErrSourceDescCount and ErrSourceDescSize? 

No it only indicates the size of error source descriptors. HEST_ERROR_SOURCE_DESC_INFO struct represents the data part of the MM Communication buffer. So CommBuffSize parameter for the MM Communication protocol takes care of the size for the entire structure.

- Omkar

+ UINTN ErrSourceDescSize;
[SAMI] Can the first 2 fields of this structure be moved to a structure called HEST_ERROR_SOURCE_DESC_HEADER? I think it may simplify computing of the size of HEST_ERROR_SOURCE_DESC_INFO.
[/SAMI]

It was added to make code more intuitive, referring to example implementation for struct SMM_FTW_COMMUNICATE_FUNCTION_HEADER in Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h

- Omkar

+ //
+ // Array of error source descriptors that is ErrSourceDescSize in size.
+ //
+ UINT8 ErrSourceDescList[1];
+} HEST_ERROR_SOURCE_DESC_INFO;
+
+#endif // HEST_ERROR_SOURCE_DESCRIPTOR_H_
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
new file mode 100644
index 000000000000..acfb0fc9e838
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
@@ -0,0 +1,308 @@
+/** @file
+ Collects and appends the HEST error source descriptors from the MM drivers.
+
+ The drivers entry point locates the MM Communication protocol and calls into
+ Standalone MM to get the HEST error sources length and count. It also
+ retrieves descriptor information. The information is then used to build the
+ HEST table using the HEST table generation protocol.
+
+ This driver collects the secure error source descriptor information from the
+ MM drviers that implement HEST error source protocol. Instead of directly
+ communicating with the individual MM drivers, it calls into
+ HestErrorSourceStandaloneMM driver which is a gatway MM driver. This MM driver
+ in-turn communicates with individual MM drivers collecting the error source
+ descriptor information.
+
+ Once all the error source descriptor information is retrieved the driver
+ appends the descriptors to HEST table using the HestDxe driver.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/MmCommunication2.h>
+#include <Protocol/HestTable.h>
+#include "HestMmErrorSourceCommon.h"
+
+#define MM_COMMUNICATE_HEADER_SIZE (OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data))
[SAMI] Can this definition be moved to MdePkg\Include\Protocol\MmCommunication.h, please ?

Ack.

- Omkar

+
+STATIC HEST_TABLE_PROTOCOL *mHestProtocol;
+STATIC EFI_MM_COMMUNICATION2_PROTOCOL *mMmCommunication2;
+
+/**
+ Retrieve the error source descriptors from Standalone MM.
+
+ Initialize the MM comminication buffer by assigning the MM service to
+ invoke as gMmHestGetErrorSourceInfoGuid. Use the MM communication
+ protocol to retrieve the error source descriptors.
+
+ @param[in] CommBuffSize Size of communicate buffer.
+ @param[in, out] CommBuffer The communicate buffer.
+
+ @retval EFI_SUCCESS MM Communicate protocol call successful.
+ @retval Other MM Communicate protocol call failed.
+**/
+STATIC
+EFI_STATUS
+GetErrorSourceDescriptors (
+ IN UINTN CommBuffSize,
+ IN OUT EFI_MM_COMMUNICATE_HEADER **CommBuffer
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Initialize the CommBuffer with MM Communicate metadata.
+ //
+ CopyGuid (&(*CommBuffer)->HeaderGuid, &gMmHestGetErrorSourceInfoGuid);
+ (*CommBuffer)->MessageLength =
+ CommBuffSize -
+ sizeof ((*CommBuffer)->HeaderGuid) -
+ sizeof ((*CommBuffer)->MessageLength);
+
+ //
+ // Call into the Standalone MM using the MM Communicate protocol.
+ //
+ Status = mMmCommunication2->Communicate (
+ mMmCommunication2,
+ (VOID *)*CommBuffer,
+ (VOID *)*CommBuffer,
[SAMI] Can you check if the third parameter to Communicate() is correct, please?

Ack. Passing only 3rd parameter is sufficient as mmcommunicate2 protocol works only on virtual commbuffer address param.

- Omkar

+ NULL
+ );
+
+ return Status;
+}
+
+/**
+ Collect HEST error source descriptors from all Standalone MM drivers and
+ append them to the HEST table.
+
+ Use MM Communication Protocol to communicate and collect the error source
+ descriptor information from Standalone MM. Check for the required buffer size
+ returned by the MM driver. Allocate buffer of adequate size and call again
+ into MM.
+
+ @retval EFI_SUCCESS Successful to collect and append the error
+ source.
+ descriptors to HEST table.
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failure.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+AppendMmErrorSources (VOID)
[SAMI] VOID and ) should be on a separate line. Can you check the other patches in this series as well, please?

Ack.

- Omkar

+{
+ EFI_MM_COMMUNICATE_HEADER *CommunicationHeader = NULL;
+ HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceDescInfo;
+ EFI_STATUS Status;
+ UINTN CommBufferSize;
+
+ //
+ // Retrieve the count, length and the actual eror source descriptors from
+ // the MM drivers. Do this by performing two MM Communicate calls, in the
+ // first call pass CommBuffer which is atleast of the size of error source
+ // descriptor info structure. Followed by another communicate call with
+ // CommBuffer allocated to required buffer size to hold all descriptors.
+ //
+ // Allocate CommBuffer atleast the size of error source descriptor info
+ // structure.
+ CommBufferSize =
+ MM_COMMUNICATE_HEADER_SIZE + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
+ CommunicationHeader = AllocatePool (CommBufferSize);
[SAMI] Would it be better to use AllocateZeroPool() ?

Ack.

- Omkar

+ if (CommunicationHeader == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for CommunicationHeader\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Make the first MM Communicate call to HestErrorSourceStandaloneMM gateway
+ // driver, which returns the required buffer size adequate to hold all the
+ // desctriptor information.
+ //
+ Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
+ if ((EFI_ERROR (Status)) &&
+ (Status != EFI_BAD_BUFFER_SIZE)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: MM Communicate protocol call failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ FreePool (CommunicationHeader);
+ return Status;
+ }
+
+ // Check for the length of Error Source descriptors.
+ ErrorSourceDescInfo =
+ (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+ if ((ErrorSourceDescInfo->ErrSourceDescSize == 0) ||
+ (ErrorSourceDescInfo->ErrSourceDescCount == 0)) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HesErrorSourceDxe: HEST error source(s) not found\n"
+ ));
+ FreePool (CommunicationHeader);
+ return EFI_SUCCESS;
[SAMI] return EFI_NOT_FOUND ?

Ack.

- Omkar

+ }
+
+ //
+ // Allocate CommBuffer of required size to accomodate all the error source
+ // descriptors. Required size of communication buffer =
+ // MM communicate metadata. + (error source desc info struct + error source
+ // descriptor size).
+ //
+ CommBufferSize =
+ MM_COMMUNICATE_HEADER_SIZE +
+ HEST_ERROR_SOURCE_DESC_INFO_SIZE +
+ ErrorSourceDescInfo->ErrSourceDescSize;
+
+ // Free old MM Communicate buffer and allocate a new buffer of required size.
+ FreePool (CommunicationHeader);
+ CommunicationHeader = AllocatePool (CommBufferSize);
+ if (CommunicationHeader == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for CommunicationHeader\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Make second MM Communicate call to HestErrorSourceStandaloneMM driver to
+ // get the error source descriptors from the MM drivers.
+ //
+ Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: MM Communicate protocol failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ FreePool (CommunicationHeader);
+ return Status;
+ }
+
+ //
+ // Retrieve the HEST error source descriptor information. Ensure that there
+ // is a valid list of error source descriptors.
+ //
+ ErrorSourceDescInfo =
+ (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+ if (ErrorSourceDescInfo->ErrSourceDescList == NULL) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HestErrorSourceDxe: Error source descriptor list is empty"
+ ));
+ FreePool (CommunicationHeader);
+ return EFI_SUCCESS;
[SAMI] Can EFI_NOT_FOUND be returned here?

Ack.

- Omkar

+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "HestErrorSourceDxe: ErrorSources: TotalCount = %d TotalLength = %d \n",
+ ErrorSourceDescInfo->ErrSourceDescCount,
+ ErrorSourceDescInfo->ErrSourceDescSize
+ ));
+
+ //
+ // Append the error source descriptors to HEST table using the HEST table
+ // generation protocol.
+ //
+ Status = mHestProtocol->AppendErrorSourceDescriptors (
+ ErrorSourceDescInfo->ErrSourceDescList,
+ ErrorSourceDescInfo->ErrSourceDescSize,
+ ErrorSourceDescInfo->ErrSourceDescCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to append error source(s), status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ }
+
+ FreePool (CommunicationHeader);
+ return Status;
+}
+
+/**
+ The Entry Point for HEST Error Source Dxe driver.
+
+ Locates the HEST Table generation and MM Communication2 protocols. Using the
+ MM Communication2, the driver collects the Error Source Descriptor(s) from
+ Standalone MM. It then appends those Error Source Descriptor(s) to the Hest
+ table using the HEST Table generation protocol.
+
+ @param[in] ImageHandle The firmware allocated handle for the Efi image.
+ @param[in] SystemTable A pointer to the Efi System Table.
+
+ @retval EFI_SUCCESS The entry point is executed successfully.
+ @retval Other Some error occurred when executing this entry point.
+**/
+EFI_STATUS
+EFIAPI
+HestErrorSourceInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = gBS->LocateProtocol (
+ &gHestTableProtocolGuid,
+ NULL,
+ (VOID **)&mHestProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate HEST table generation protocol, status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gBS->LocateProtocol (
+ &gEfiMmCommunication2ProtocolGuid,
+ NULL,
+ (VOID **)&mMmCommunication2
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate MMCommunication2 driver protocol, status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ //
+ // Append HEST error sources retrieved from StandaloneMM, if any, into the
+ // HEST ACPI table.
+ //
+ Status = AppendMmErrorSources ();
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed appending error source desc to HEST table, status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ }
+ return EFI_SUCCESS;
+}
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
new file mode 100644
index 000000000000..c7b2304fc494
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
@@ -0,0 +1,312 @@
+/** @file
+ MM HEST error source gateway driver.
+
+ This MM driver installs a handler which can be used to retrieve the error
+ source descriptors from the all MM drivers implementing the HEST error source
+ descriptor protocol.
+
+ The MM driver acts as a single point of contact to collect secure hardware
+ error sources from the MM drivers. It loops over all the MM drivers that
+ implement HEST error source descriptor protocol and collects error source
+ descriptor information along with the error source count and length.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Base.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Protocol/HestErrorSourceInfo.h>
+
+#include "HestMmErrorSourceCommon.h"
+
+STATIC EFI_MM_SYSTEM_TABLE *mMmst = NULL;
+
+/**
+ Returns an array of handles that implement the HEST error source descriptor
+ protocol.
+
+ Passing HandleBuffer as NULL will return the actual size of the buffer
+ required to hold the array of handles implementing the protocol.
+
+ @param[in, out] HandleBufferSize The size of the HandleBuffer.
+ @param[out] HandleBuffer A pointer to the buffer containing the list
+ of handles.
+
+ @retval EFI_SUCCESS The array of handles returned in HandleBuffer.
+ @retval EFI_NOT_FOUND No implementation present for the protocol.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+GetHestErrorSourceProtocolHandles (
+ IN OUT UINTN *HandleBufferSize,
+ OUT EFI_HANDLE **HandleBuffer
+ )
+{
+ EFI_STATUS Status;
+
+ Status = mMmst->MmLocateHandle (
+ ByProtocol,
+ &gMmHestErrorSourceDescProtocolGuid,
+ NULL,
+ HandleBufferSize,
+ *HandleBuffer
+ );
+ if ((EFI_ERROR (Status)) &&
+ (Status != EFI_BUFFER_TOO_SMALL))
+ {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: No implementation of MmHestErrorSourceDescProtocol found, \
+ Status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ return Status;
+}
+
+/**
+ Mmi handler to retrieve HEST error source descriptor information.
+
+ Handler for Mmi service that returns the supported HEST error source
+ descriptors in MM. This handler populates the CommBuffer with the
+ list of all error source descriptors, prepended with the length and
+ the number of descriptors populated into CommBuffer.
+
+ @param[in] DispatchHandle The unique handle assigned to this handler by
+ MmiHandlerRegister().
+ @param[in] Context Points to an optional handler context that
+ is specified when the handler was registered.
+ @param[in, out] CommBuffer Buffer used for communication of HEST error
+ source descriptors.
+ @param[in, out] CommBufferSize The size of the CommBuffer.
+
+ @retval EFI_SUCCESS CommBuffer has valid data.
+ @retval EFI_BAD_BUFFER_SIZE CommBufferSize not adequate.
+ @retval EFI_OUT_OF_RESOURCES System out of memory resources.
+ @retval EFI_INVALID_PARAMETER Invalid CommBufferSize recieved.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HestErrorSourcesInfoMmiHandler (
+ IN EFI_HANDLE DispatchHandle,
+ IN CONST VOID *Context, OPTIONAL
+ IN OUT VOID *CommBuffer, OPTIONAL
+ IN OUT UINTN *CommBufferSize OPTIONAL
+ )
+{
+ MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *HestErrSourceDescProtocolHandle;
+ HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceInfoList;
+ EFI_HANDLE *HandleBuffer;
+ EFI_STATUS Status;
+ UINTN HandleCount;
+ UINTN HandleBufferSize;
+ UINTN Index;
+ UINTN SourceCount = 0;
+ UINTN SourceLength = 0;
+ VOID *ErrorSourcePtr;
+ UINTN TotalSourceLength = 0;
+ UINTN TotalSourceCount = 0;
+
+ if (*CommBufferSize < HEST_ERROR_SOURCE_DESC_INFO_SIZE) {
+ //
+ // Ensures that the communication buffer has enough space to atleast hold
+ // the ErrSourceDescCount and ErrSourceDescSize elements of the
+ // HEST_ERROR_SOURCE_DESC_INFO structure.
+ //
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Invalid CommBufferSize parameter\n",
+ __FUNCTION__
+ ));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Get all handles that implement the HEST error source descriptor protocol.
+ // Get the buffer size required to store list of handles for the protocol.
+ //
+ HandleBuffer = NULL;
+ HandleBufferSize = 0;
+ Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
+ if ((Status == EFI_NOT_FOUND) ||
+ (HandleBufferSize == 0))
+ {
+ return Status;
+ }
+
+ // Allocate memory for HandleBuffer of size HandleBufferSize.
+ HandleBuffer = AllocatePool (HandleBufferSize);
[SAMI] AllocateZeroPool() ?

Ack.

+ if (HandleBuffer == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for HandleBuffer\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ // Get the list of handles.
+ Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
+ if ((EFI_ERROR (Status)) ||
+ (HandleBuffer == NULL))
[SAMI] Is check for HandleBuffer == NULL right here?

Ack.

- Omkar

+ {
+ FreePool (HandleBuffer);
+ return Status;
+ }
+
+ // Count of handles for the protocol.
+ HandleCount = HandleBufferSize / sizeof (EFI_HANDLE);
+
+ //
+ // Loop to get the count and length of the error source descriptors.
+ //
+ // This loop collects and adds the length of error source descriptors and
+ // its count from all the the MM drivers implementing HEST error source.
+ // descriptor protocol. The total length and count values retrieved help
+ // to determine weather the CommBuffer is big enough to hold the descriptor
+ // information.
+ // As mentioned in the HEST error source descriptor protocol definition,
+ // Buffer parameter set to NULL ensures only length and the count values
+ // are returned from the driver and no error source information is copied to
+ // Buffer.
+ //
+ for (Index = 0; Index < HandleCount; ++Index) {
+ Status = mMmst->MmHandleProtocol (
+ HandleBuffer[Index],
+ &gMmHestErrorSourceDescProtocolGuid,
+ (VOID **)&HestErrSourceDescProtocolHandle
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ //
+ // Protocol called with Buffer parameter passed as NULL, must return
+ // error source length and error count for that driver.
+ //
+ Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
+ HestErrSourceDescProtocolHandle,
+ NULL,
+ &SourceLength,
+ &SourceCount
+ );
+ if (Status == EFI_INVALID_PARAMETER) {
[SAMI] I think the error handling in this function and the error return implementation in GetHestErrorSourceDescriptors() could be improved.
e.g. GetHestErrorSourceDescriptors() could first check for the SourceLength & SourceCount and if it is less than what is required, it returns EFI_BUFFER_TOO_SMALL.
The next check would be to check ErrorSourcePtr and return EFI_INVALID_PARAMETER if it is NULL.
 [/SAMI]

This also considers the case where there are multiple MM drivers that can return error source descriptors. In case of multiple implementations for MmHestErrorSourceDescProtocol protocol by MM drivers this can lead to unnecessary calls between secure and non-secure world via MM Communicate. Instead as mentioned by the protocol definition for MmHestErrorSourceDescProtocol, first get the count and length of descriptors from all the MM drivers implementing protocol MmHestErrorSourceDescProtocol by passing “Buffer” param as NULL. This call fails by returning EFI_INVALID_PARAMETER but returns count and size of the error source descriptors it supports.

- Omkar

+ TotalSourceLength += SourceLength;
+ TotalSourceCount += SourceCount;
+ }
+ }
+
+ // Set the count and length in the error source descriptor.
+ ErrorSourceInfoList = (HEST_ERROR_SOURCE_DESC_INFO *)(CommBuffer);
+ ErrorSourceInfoList->ErrSourceDescCount = TotalSourceCount;
+ ErrorSourceInfoList->ErrSourceDescSize = TotalSourceLength;
+
+ //
+ // Check the size of CommBuffer, it should atleast be of size
+ // TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE.
+ //
+ TotalSourceLength = TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
+ if ((*CommBufferSize) < TotalSourceLength) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Invalid CommBufferSize parameter\n",
+ __FUNCTION__
+ ));
+ FreePool (HandleBuffer);
+ return EFI_BAD_BUFFER_SIZE;
[SAMI] Should the return code be EFI_BUFFER_TOO_SMALL? The difference being, the caller can attempt to call again with a larger buffer if EFI_BUFFER_TOO_SMALL is returned.
CommBufferSize is declared as an OUT paramter, was the intent to return the required buffer size?
[/SAMI]

Ack. ErrSourceDescSize is used to pass the required buffer size.

- Omkar

+ }
+
+ //
+ // CommBuffer size is adequate to return all the error source descriptors.
+ // So go ahead and populate it with the error source descriptor information.
+ //
+
+ // Buffer pointer to append the Error Descriptors data.
+ ErrorSourcePtr = ErrorSourceInfoList->ErrSourceDescList;
+
+ //
+ // Loop to retrieve error source descriptors information.
+ //
+ // Calls into each MM driver that implement the HEST error source descriptor
+ // protocol. Here the Buffer parameter passed to the protocol service is
+ // valid. So the MM driver when called copies the descriptor information.
+ //
+ for (Index = 0; Index < HandleCount; ++Index) {
+ Status = mMmst->MmHandleProtocol (
+ HandleBuffer[Index],
+ &gMmHestErrorSourceDescProtocolGuid,
+ (VOID **)&HestErrSourceDescProtocolHandle
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
+ HestErrSourceDescProtocolHandle,
+ (VOID **)&ErrorSourcePtr,
+ &SourceLength,
+ &SourceCount
+ );
+ if (Status == EFI_SUCCESS) {
+ ErrorSourcePtr += SourceLength;
+ }
+ }
+
+ // Free the buffer holding all the protocol handles.
+ FreePool (HandleBuffer);
+
+ return EFI_SUCCESS;
[SAMI] return Status of last operation.

Ack.

- Omkar

+}
+
+/**
+ Entry point for this Stanalone MM driver.
+
+ Registers an Mmi handler that retrieves the error source descriptors from all
+ the MM drivers implementing the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL.
+
+ @param[in] ImageHandle The firmware allocated handle for the EFI image.
+ @param[in] SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The entry point registered handler successfully.
+ @retval Other Some error occurred when executing this entry point.
+**/
+EFI_STATUS
+EFIAPI
+StandaloneMmHestErrorSourceInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_MM_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_HANDLE DispatchHandle;
+ EFI_STATUS Status;
+
+ ASSERT (SystemTable != NULL);
+ mMmst = SystemTable;
+
+ Status = mMmst->MmiHandlerRegister (
+ HestErrorSourcesInfoMmiHandler,
+ &gMmHestGetErrorSourceInfoGuid,
+ &DispatchHandle
+ );
+ if (EFI_ERROR(Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Mmi handler registration failed with status : %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ return EFI_SUCCESS;
[SAMI] return Status of last operation.

Ack.

- Omkar

+}


Re: [PATCH v2 2/4] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL

Omkar Anand Kulkarni
 

Hi Sami,

Thanks for reviewing this patch. Please find my response inline.

Regards,
Omkar

Hi Omkar,
Please find my response below marked [SAMI]
Regards,

Sami Mujawar

On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Add the protocol definition of the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
protocol. This protocol can be implemented by MM drivers to publish
error source descriptors that have to be populated into HEST table.

Co-authored-by: Thomas Abraham mailto:thomas.abraham@...
Signed-off-by: Omkar Anand Kulkarni mailto:omkar.kulkarni@...
---
ArmPlatformPkg/ArmPlatformPkg.dec | 1 +
ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h | 64 ++++++++++++++++++++
2 files changed, 65 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index e4afe5da8e11..4f062292663b 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -131,3 +131,4 @@
[Protocols.common]
## Arm Platform HEST table generation protocol
gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
+ gMmHestErrorSourceDescProtocolGuid = { 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
diff --git a/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
new file mode 100644
index 000000000000..95afd4dffe9c
--- /dev/null
+++ b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
@@ -0,0 +1,64 @@
+/** @file
+ MM protocol to get the secure error source descriptor information.
+
+ MM Drivers must implement this protocol in order to publish secure side
+ error source descriptor information to OSPM through the HEST ACPI table.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MM_HEST_ERROR_SOURCE_DESC_
+#define MM_HEST_ERROR_SOURCE_DESC_
+
+#define MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_GUID \
+ { \
+ 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } \
+ }
+
+typedef struct MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_
[SAMI] Not sure if a trailing underscore would be right to use for the name tag. Can MmHestErrorSourceDescProtocol be used as the name tag?
Also see https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/56_declarations_and_types#5-6-3-2-structure-declaration-with-forward-reference-or-self-reference
+ MM_HEST_ERROR_SOURCE_DESC_PROTOCOL;
+

Ack.

- Omkar

+/**
+ Get HEST Secure Error Source Descriptors.
+
+ The MM drivers implementing this protocol must convey the total count and
+ total length of the error sources the driver has along with the actual error
+ source descriptor(s).
+
+ Passing NULL as Buffer parameter shall return EFI_INVALID_PARAMETR with the
+ total length and count of the error source descriptor(s) it supports.
+
+ @param[in] This MM_HEST_ERROR_SOURCE_DESC_PROTOCOL instance.
+ @param[out] Buffer Buffer to be appended with the error
+ source descriptors information.
+ @param[out] ErrorSourcesLength Total length of all the error source
+ descriptors.
+ @param[out] ErrorSourceCount Count of total error source descriptors
+ supported by the driver.
+
+ retval EFI_SUCCESS If the Buffer is valid and is filled with valid
+ Error Source descriptor data.
+ retval EFI_INVALID_PARAMTER Buffer is NULL.
+ retval Other If no error source descriptor information is
+ available.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS) (
+ IN MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *This,
+ OUT VOID **Buffer,
+ OUT UINTN *ErrorSourcesLength,
+ OUT UINTN *ErrorSourcesCount
+ );
+
+//
+// Protocol declaration
+//
+struct MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_ {
+ MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS GetHestErrorSourceDescriptors;
+};
+
+extern EFI_GUID gMmHestErrorSourceDescProtocolGuid;
+
+#endif // MM_HEST_ERROR_SOURCE_DESC_


Re: [PATCH v2 1/4] ArmPlatformPkg: Allow dynamic generation of HEST ACPI table

Omkar Anand Kulkarni
 

Hi Sami,

Thanks for reviewing this patch. Please find my response inline.

Regards,
Omkar

Hi Omkar,
Please find my response marked inline as [SAMI].
Regards,
Sami Mujawar

On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Introduce the HEST table generation protocol that allows platforms to
build the table with multiple error source descriptors and install the
table. The protocol provides two interfaces. The first interface allows
for adding multiple error source descriptors into the HEST table. The
second interface can then be used to dynamically install the fully
populated HEST table. This allows multiple drivers and/or libraries to
dynamically register error source descriptors into the HEST table.

Co-authored-by: Thomas Abraham mailto:thomas.abraham@...
Signed-off-by: Omkar Anand Kulkarni mailto:omkar.kulkarni@...
---
ArmPlatformPkg/ArmPlatformPkg.dec | 4 +
ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf | 49 +++
ArmPlatformPkg/Include/Protocol/HestTable.h | 71 ++++
ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c | 354 ++++++++++++++++++++
4 files changed, 478 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 3a25ddcdc8ca..e4afe5da8e11 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -127,3 +127,7 @@
gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|300000000|UINT32|0x00000022

gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
+
+[Protocols.common]
+ ## Arm Platform HEST table generation protocol
+ gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
diff --git a/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf
new file mode 100644
index 000000000000..91c7385bf7ff
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf
@@ -0,0 +1,49 @@
+## @file
+# Dxe driver that creates and publishes the HEST table.
+#
+# This driver creates HEST header and provides protocol service to append
+# and install the HEST table.
+#
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = HestDxe
+ FILE_GUID = 933099a2-ef71-4e00-82aa-a79b1e0a3b38
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = HestInitialize
+
+[Sources.Common]
+ HestDxe.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+ ArmLib
+ BaseMemoryLib
+ DebugLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[Protocols]
+ gEfiAcpiTableProtocolGuid ## PROTOCOL ALWAYS_CONSUMED
+ gHestTableProtocolGuid ## PRODUCES
+
+[FixedPcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
+
+[Depex]
+ TRUE
diff --git a/ArmPlatformPkg/Include/Protocol/HestTable.h b/ArmPlatformPkg/Include/Protocol/HestTable.h
new file mode 100644
index 000000000000..3b2e1f7d9203
--- /dev/null
+++ b/ArmPlatformPkg/Include/Protocol/HestTable.h
@@ -0,0 +1,71 @@
+/** @file
+ Builds and installs the HEST ACPI table.
+
+ Define the protocol interface that allows HEST ACPI table to be created,
+ populated with error record descriptions and installation of the HEST ACPI
+ table.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef HEST_TABLE_H_
+#define HEST_TABLE_H_
+
+#define HEST_TABLE_PROTOCOL_GUID \
+ { \
+ 0x705bdcd9, 0x8c47, 0x457e, \
+ { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } \
+ }
+
+/**
+ Append HEST error source descriptor protocol service.
+
+ Protocol service used to append newly collected error source descriptors to
+ to an already created HEST table.
+
+ @param[in] ErrorSourceDescriptorList List of Error Source Descriptors.
+ @param[in] ErrorSourceDescriptorListSize Total Size of Error Source
+ Descriptors.
+ @param[in] ErrorSourceDescriptorCount Total count of error source
+ descriptors.
+
+ @retval EFI_SUCCESS Appending the error source descriptors
+ successful.
+ @retval EFI_OUT_OF_RESOURCES Buffer reallocation failed for the Hest
+ table.
+ @retval EFI_INVALID_PARAMETER Null ErrorSourceDescriptorList param or
+**/
+typedef
+EFI_STATUS
+(EFIAPI *APPEND_ERROR_SOURCE_DESCRIPTOR) (
+ IN CONST VOID *ErrorSourceDescriptorList,
+ IN UINTN ErrorSourceDescriptorListSize,
+ IN UINTN ErrorSourceDescriptorCount
+ );
+
+/**
+ Install HEST table protocol service.
+
+ Installs the HEST table that has been appended with the error source
+ descriptors. The checksum of this table is calculated and updated in
+ the table header. The HEST table is then installed.
+
+ @retval EFI_SUCCESS HEST table is installed successfully.
+ @retval EFI_ABORTED HEST table is NULL.
+ @retval Other Install service call failed.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *INSTALL_HEST_TABLE) (VOID);
+
+//
+// HEST_TABLE_PROTOCOL enables creation and installation of HEST table
+//
+typedef struct {
+ APPEND_ERROR_SOURCE_DESCRIPTOR AppendErrorSourceDescriptors;
+ INSTALL_HEST_TABLE InstallHestTable;
+} HEST_TABLE_PROTOCOL;
+
+extern EFI_GUID gHestTableProtocolGuid;
+#endif // HEST_TABLE_H_
diff --git a/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c
new file mode 100644
index 000000000000..b68e1a0c4e48
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c
@@ -0,0 +1,354 @@
+/** @file
+ Builds and installs the HEST ACPI table.
+
+ This driver installs a protocol that can be used to create and install a HEST
+ ACPI table. The protocol allows one or more error source producers to add the
+ error source descriptors into the HEST table. It also allows the resulting
+ HEST table to be installed.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - ACPI 6.3, Table 18-382, Hardware Error Source Table
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/AcpiSystemDescriptionTable.h>
+#include <Protocol/AcpiTable.h>
+#include <Protocol/HestTable.h>
+
+typedef struct {
+ VOID *HestTable; /// Pointer to HEST table.
+ UINT32 CurrentTableSize; /// Current size of HEST table.
+} HEST_DXE_DRIVER_DATA;
+
+STATIC EFI_ACPI_TABLE_PROTOCOL *mAcpiTableProtocol = NULL;
+STATIC HEST_DXE_DRIVER_DATA mHestDriverData;
+
+/**
+ Helper function to the driver.
+
+ Function that reallocates memory for every new error source descriptor info
+ added.
+
+ @param[in] OldTableSize Old memory pool size.
+ @param[in] NewTableSize Required memory pool size.
+ @param[in, out] OldBuffer Current memory buffer pointer holding the Hest
+ table data.
+
+ @return New pointer to reallocated memory pool.
+**/
+STATIC
+VOID*
+ReallocateHestTableMemory (
+ IN UINT32 OldTableSize,
+ IN UINT32 NewTableSize,
+ IN OUT VOID *OldBuffer
+ )
+{
[SAMI] Have you considered maintaining a linked list of the error sources and serialising the list once InstallHestTable() is called?

This is a simple implementation which collects all the error source descriptors as buffers. Linked list will add complexity to the code. Are there any particular benefits of using linked list over this method. For the v3, linked list has not been used. Let me know you opinion.

- Omkar

+ return ReallocateReservedPool (
+ OldTableSize,
+ NewTableSize,
+ OldBuffer
+ );
[SAMI] Is this wrapper function required? Can ReallocateReservedPool() be used directly instead?

Ack.

- Omkar

+}
+
+/**
+ Allocate memory for the HEST table header and populate it.
+
+ Helper function for this driver, populates the HEST table header. Called once
+ in the beginning on first invocation of append error source descriptor
+ protocol service.
+
+ @retval EFI_SUCCESS On successful creation of HEST header.
+ @retval EFI_OUT_OF_RESOURCES If system is out of memory recources.
+**/
+STATIC
+EFI_STATUS
+BuildHestHeader (VOID)
[SAMI] VOID and closing parenthesis should be on a separate line.

Ack.

- Omkar

+{
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER HestHeader;
+ UINT64 TempOemTableId;
+
+ //
+ // Allocate memory for the HEST table header.
+ //
+ mHestDriverData.HestTable =
+ ReallocateHestTableMemory (
+ 0,
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER),
+ NULL
+ );
[SAMI] Is the Relocate version of the function required here, maybe the Alloc version could be used.
- Should EfiReservedMemoryType be used?
    Please see extract from section 2.3.6 AArch64 Platforms of the UEFI 2.9 specification below:
    "Note:Previous EFI specifications allowed ACPI tables loaded at runtime to be in the
     EfiReservedMemoryType and there was no guidance provided for other EFI Configuration
    Tables. EfiReservedMemoryType is not intended to be used by firmware. UEFI 2.0 clarified
    the situation moving forward. "

Ack.

- Omkar

+ if (mHestDriverData.HestTable == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ mHestDriverData.CurrentTableSize =
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER);
+
+ //
+ // Populate the values of the HEST table header.
+ //
+ HestHeader.Header.Signature =
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_SIGNATURE;
+ HestHeader.Header.Revision =
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_REVISION;
+ CopyMem (
+ &HestHeader.Header.OemId,
+ FixedPcdGetPtr (PcdAcpiDefaultOemId),
+ sizeof (HestHeader.Header.OemId)
+ );
+ TempOemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId);
+ CopyMem (
+ &HestHeader.Header.OemTableId,
+ &TempOemTableId,
+ sizeof (HestHeader.Header.OemTableId)
+ );
[SAMI] HestHeader.Header.OemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId); ?
+ HestHeader.Header.OemRevision = PcdGet32 (PcdAcpiDefaultOemRevision);
+ HestHeader.Header.CreatorId = PcdGet32 (PcdAcpiDefaultCreatorId);
+ HestHeader.Header.CreatorRevision = PcdGet32 (PcdAcpiDefaultCreatorRevision);
+ HestHeader.ErrorSourceCount = 0;
+ CopyMem (mHestDriverData.HestTable, &HestHeader, sizeof (HestHeader));
[SAMI] Is it possible to use a local HEST table pointer to initalise the values instead of initialising a HEST structure and then doing memcopy?

Ack.

- Omkar

+
+ return EFI_SUCCESS;
+}
+
+/**
+ Append HEST error source descriptor protocol service.
+
+ Protocol service used to append newly collected error source descriptors to
+ to an already created HEST table.
+
+ @param[in] ErrorSourceDescriptorList List of Error Source Descriptors.
+ @param[in] ErrorSourceDescriptorListSize Total Size of Error Source
+ Descriptors.
+ @param[in] ErrorSourceDescriptorCount Total count of error source
+ descriptors.
+
+ @retval EFI_SUCCESS Appending the error source descriptors
+ successful.
+ @retval EFI_OUT_OF_RESOURCES Buffer reallocation failed for the Hest
+ table.
+ @retval EFI_INVALID_PARAMETER Null ErrorSourceDescriptorList param or
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AppendErrorSourceDescriptor (
+ IN CONST VOID *ErrorSourceDescriptorList,
+ IN UINTN ErrorSourceDescriptorListSize,
+ IN UINTN ErrorSourceDescriptorCount
+ )
+{
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeaderPtr;
+ EFI_STATUS Status;
+ UINT32 NewTableSize;
+ VOID *ErrorDescriptorPtr;
+
+ if ((ErrorSourceDescriptorList == NULL) ||
+ (ErrorSourceDescriptorListSize == 0)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Create a HEST table header if not already created.
+ //
+ if (mHestDriverData.HestTable == NULL) {
+ Status = BuildHestHeader ();
+ if (Status == EFI_OUT_OF_RESOURCES) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to build HEST header, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
+
+ //
+ // Resize the existing HEST table buffer to accommodate the incoming error
+ // source descriptors.
+ //
+ NewTableSize = mHestDriverData.CurrentTableSize +
+ ErrorSourceDescriptorListSize;
+ mHestDriverData.HestTable = ReallocateHestTableMemory (
+ mHestDriverData.CurrentTableSize,
+ NewTableSize,
+ mHestDriverData.HestTable
+ );
+ if (mHestDriverData.HestTable == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to reallocate memory for HEST table\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
[SAMI] As mentioned earlier, would it be possible to maintain a link list instead of relocating the memory?

This implementation is simpler over the linked list implementation. Same as the comment above, for v3, this implementation will be maintained. Let me know your opinion.

- Omkar

+ //
+ // Copy the incoming error source descriptors into HEST table.
+ //
+ ErrorDescriptorPtr = (VOID *)mHestDriverData.HestTable +
+ mHestDriverData.CurrentTableSize;
+ HestHeaderPtr = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+ mHestDriverData.HestTable;
+ CopyMem (
+ ErrorDescriptorPtr,
+ ErrorSourceDescriptorList,
+ ErrorSourceDescriptorListSize
+ );
+ mHestDriverData.CurrentTableSize = NewTableSize;
+ HestHeaderPtr->Header.Length = mHestDriverData.CurrentTableSize;
+ HestHeaderPtr->ErrorSourceCount += ErrorSourceDescriptorCount;
+
+ DEBUG ((
+ DEBUG_INFO,
+ "HestDxe: %d Error source descriptor(s) added \n",
+ ErrorSourceDescriptorCount
+ ));
+ return EFI_SUCCESS;
+}
+
+/**
+ Install HEST table protocol service.
+
+ Installs the HEST table that has been appended with the error source
+ descriptors. The checksum of this table is calculated and updated in
+ the table header. The HEST table is then installed.
+
+ @retval EFI_SUCCESS HEST table is installed successfully.
+ @retval EFI_ABORTED HEST table is NULL.
+ @retval Other Install service call failed.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InstallHestAcpiTable (VOID)
[SAMI] Please update according to coding standard.

Ack.

- Omkar

+{
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeader;
+ EFI_STATUS Status;
+ UINTN AcpiTableHandle;
+
+ //
+ // Check if we have valid HEST table data. If not, there no hardware error
+ // sources supported by the platform and no HEST table to publish, return.
+ //
+ if (mHestDriverData.HestTable == NULL) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HestDxe: No data available to generate HEST table\n"
+ ));
+ return EFI_SUCCESS;
[SAMI] Can a suitable error code be returned here? EFI_NOT_FOUND?

Ack.

- Omkar

+ }
+
+ //
+ // Valid data for HEST table found. Update the header checksum and install the
+ // HEST table.
+ //
+ HestHeader = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+ mHestDriverData.HestTable;
+ HestHeader->Header.Checksum = CalculateCheckSum8 (
+ (UINT8 *)HestHeader,
+ HestHeader->Header.Length
+ );
[SAMI] Checksum calculation is not needed as ACPITableProtocol->InstallAcpiTable() does this internally.

Ack.

- Omkar

+
+ Status = mAcpiTableProtocol->InstallAcpiTable (
+ mAcpiTableProtocol,
+ HestHeader,
+ HestHeader->Header.Length,
+ &AcpiTableHandle
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: HEST table installation failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ //
+ // Free the HEST table buffer.
+ //
+ FreePool (mHestDriverData.HestTable);
+ DEBUG ((
+ DEBUG_INFO,
+ "HestDxe: Installed HEST table \n"
+ ));
+ return EFI_SUCCESS;
[SAMI] return Status;

Ack.

- Omkar

+}
+
+//
+// HEST table generation protocol instance.
+//
+STATIC HEST_TABLE_PROTOCOL mHestProtocol = {
+ AppendErrorSourceDescriptor,
+ InstallHestAcpiTable
+};
[SAMI] HEST is platform and processor architecture independent. Threfore, can this implementation be paced in a common location? MdeModulePkg?

Ack.

- Omkar

+
+/**
+ The Entry Point for HEST Dxe driver.
+
+ This function installs the HEST table creation and installation protocol
+ services.
+
+ @param[in] ImageHandle Handle to the Efi image.
+ @param[in] SystemTable A pointer to the Efi System Table.
+
+ @retval EFI_SUCCESS On successful installation of protocol services and
+ location the ACPI table protocol.
+ @retval Other On Failure to locate ACPI table protocol or install
+ of HEST table generation protocol.
+**/
+EFI_STATUS
+EFIAPI
+HestInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_HANDLE Handle = NULL;
+ EFI_STATUS Status;
+
+ Status = gBS->LocateProtocol (
+ &gEfiAcpiTableProtocolGuid,
+ NULL,
+ (VOID **)&mAcpiTableProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate ACPI table protocol, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gBS->InstallProtocolInterface (
+ &Handle,
+ &gHestTableProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mHestProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to install HEST table generation protocol status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ return EFI_SUCCESS;
[SAMI] return Status;

Ack.

- Omkar

+}


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

Omkar Anand Kulkarni
 

Hi Sami,

Thanks for reviewing this patch series. Please find my response inline.

Regards,
Omkar

Hi Omkar,

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

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

Regards,

Sami Mujawar
Yes, we will follow up on the HEST parser patch and post it soon.

- Omkar


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

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

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

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

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

Or

2. in a Readme.md file

Regards,

Sami Mujawar
Ack.

- Omkar


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

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

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

NS | S
+--------------------------------------+--------------------------------------+

| | |
|+-------------------------------------+---------------------+ |
|| +---------------------+--------------------+| |
|| | | || |
|| +-----------+ |+------------------+ | +-----------------+||
+-------------+|
|| |HestTable | || HestErrorSource | | | HestErrorSource ||| |
DMC-620 ||
|| | DXE | || DXE | | | StandaloneMM |||
|Standalone MM||
|| +-----------+ |+------------------+ | +-----------------+||
+-------------+|
|| |GHESv2 | || |
|| +---------------------+--------------------+| |
|| +--------------------+ | | |
|| |PlatformErrorHandler| | | |
|| | DXE | | | |
|| +--------------------+ | | |
||FF FWK | | |
|+-------------------------------------+---------------------+ |
| | |
+--------------------------------------+--------------------------------------+

|
Figure: Firmware First Error Handling approach.

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

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

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

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

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

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

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

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


Re: [PATCH] UefiPayloadPkg: Dump hob info from gEdkiiBootManagerMenuFileGuid

Ni, Ray
 

+ ASSERT (HobLength >= sizeof (*BootManagerMenuFile));
+ ASSERT (HobLength >= BootManagerMenuFile->Header.Length);
Are the two assertions duplicated?

Thanks,
Ray


[PATCH] SecurityPkg/Tcg: remove TcgMorLockSmm driver

Qi Zhang
 

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

TcgMorLockSmm is only for secure MOR V1.
VariableSmm covers secure MOR V1 and V2.

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Qi Zhang <qi1.zhang@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Ray Ni <ray.ni@...>
---
SecurityPkg/SecurityPkg.dsc | 1 -
.../TcgMorLock.c | 191 ------------------
.../TcgMorLock.h | 131 ------------
.../TcgMorLock.uni | 16 --
.../TcgMorLockExtra.uni | 14 --
.../TcgMorLockSmm.c | 152 --------------
.../TcgMorLockSmm.inf | 65 ------
7 files changed, 570 deletions(-)
delete mode 100644 SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMo=
rLock.c
delete mode 100644 SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMo=
rLock.h
delete mode 100644 SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMo=
rLock.uni
delete mode 100644 SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMo=
rLockExtra.uni
delete mode 100644 SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMo=
rLockSmm.c
delete mode 100644 SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMo=
rLockSmm.inf

diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 64157e20f9..7898fe4282 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -338,7 +338,6 @@
=0D
[Components.IA32, Components.X64]=0D
=0D
- SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSmm.inf=0D
SecurityPkg/Tcg/TcgSmm/TcgSmm.inf=0D
SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf=0D
SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf=0D
diff --git a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.c=
b/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.c
deleted file mode 100644
index aa230eeefa..0000000000
--- a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/** @file=0D
- TCG MOR (Memory Overwrite Request) Lock Control Driver.=0D
-=0D
- This driver initializes MemoryOverwriteRequestControlLock variable.=0D
- This module will add Variable Hook and allow MemoryOverwriteRequestContr=
olLock variable set only once.=0D
-=0D
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>=0D
-SPDX-License-Identifier: BSD-2-Clause-Patent=0D
-=0D
-**/=0D
-=0D
-#include <PiDxe.h>=0D
-#include <Guid/MemoryOverwriteControl.h>=0D
-#include <IndustryStandard/MemoryOverwriteRequestControlLock.h>=0D
-#include <Library/DebugLib.h>=0D
-#include <Library/BaseLib.h>=0D
-#include <Library/BaseMemoryLib.h>=0D
-#include "TcgMorLock.h"=0D
-=0D
-typedef struct {=0D
- CHAR16 *VariableName;=0D
- EFI_GUID *VendorGuid;=0D
-} VARIABLE_TYPE;=0D
-=0D
-VARIABLE_TYPE mMorVariableType[] =3D {=0D
- {MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, &gEfiMemoryOverwriteContro=
lDataGuid},=0D
- {MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, &gEfiMemoryOverwriteReques=
tControlLockGuid},=0D
-};=0D
-=0D
-/**=0D
- Returns if this is MOR related variable.=0D
-=0D
- @param VariableName the name of the vendor's variable, it's a Null-Term=
inated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
-=0D
- @retval TRUE The variable is MOR related.=0D
- @retval FALSE The variable is NOT MOR related.=0D
-**/=0D
-BOOLEAN=0D
-IsAnyMorVariable (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid=0D
- )=0D
-{=0D
- UINTN Index;=0D
-=0D
- for (Index =3D 0; Index < sizeof(mMorVariableType)/sizeof(mMorVariableTy=
pe[0]); Index++) {=0D
- if ((StrCmp (VariableName, mMorVariableType[Index].VariableName) =3D=
=3D 0) &&=0D
- (CompareGuid (VendorGuid, mMorVariableType[Index].VendorGuid))) {=
=0D
- return TRUE;=0D
- }=0D
- }=0D
- return FALSE;=0D
-}=0D
-=0D
-/**=0D
- Returns if this is MOR lock variable.=0D
-=0D
- @param VariableName the name of the vendor's variable, it's a Null-Term=
inated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
-=0D
- @retval TRUE The variable is MOR lock variable.=0D
- @retval FALSE The variable is NOT MOR lock variable.=0D
-**/=0D
-BOOLEAN=0D
-IsMorLockVariable (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid=0D
- )=0D
-{=0D
- if ((StrCmp (VariableName, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME) =
=3D=3D 0) &&=0D
- (CompareGuid (VendorGuid, &gEfiMemoryOverwriteRequestControlLockGuid=
))) {=0D
- return TRUE;=0D
- }=0D
- return FALSE;=0D
-}=0D
-=0D
-/**=0D
- This service is a checker handler for the UEFI Runtime Service SetVariab=
le()=0D
-=0D
- @param VariableName the name of the vendor's variable, as a=0D
- Null-Terminated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
- @param Attributes Point to memory location to return the attributes o=
f variable. If the point=0D
- is NULL, the parameter would be ignored.=0D
- @param DataSize The size in bytes of Data-Buffer.=0D
- @param Data Point to the content of the variable.=0D
-=0D
- @retval EFI_SUCCESS The firmware has successfully stored the=
variable and its data as=0D
- defined by the Attributes.=0D
- @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits=
was supplied, or the=0D
- DataSize exceeds the maximum allowed.=0D
- @retval EFI_INVALID_PARAMETER VariableName is an empty Unicode string.=
=0D
- @retval EFI_OUT_OF_RESOURCES Not enough storage is available to hold =
the variable and its data.=0D
- @retval EFI_DEVICE_ERROR The variable could not be saved due to a=
hardware failure.=0D
- @retval EFI_WRITE_PROTECTED The variable in question is read-only.=0D
- @retval EFI_WRITE_PROTECTED The variable in question cannot be delet=
ed.=0D
- @retval EFI_SECURITY_VIOLATION The variable could not be written due to=
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS=0D
- set but the AuthInfo does NOT pass the v=
alidation check carried=0D
- out by the firmware.=0D
- @retval EFI_NOT_FOUND The variable trying to be updated or del=
eted was not found.=0D
-=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-SetVariableCheckHandlerMor (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid,=0D
- IN UINT32 Attributes,=0D
- IN UINTN DataSize,=0D
- IN VOID *Data=0D
- )=0D
-{=0D
- UINTN MorLockDataSize;=0D
- BOOLEAN MorLock;=0D
- EFI_STATUS Status;=0D
-=0D
- //=0D
- // do not handle non-MOR variable=0D
- //=0D
- if (!IsAnyMorVariable (VariableName, VendorGuid)) {=0D
- return EFI_SUCCESS;=0D
- }=0D
-=0D
- MorLockDataSize =3D sizeof(MorLock);=0D
- Status =3D InternalGetVariable (=0D
- MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,=0D
- &gEfiMemoryOverwriteRequestControlLockGuid,=0D
- NULL,=0D
- &MorLockDataSize,=0D
- &MorLock=0D
- );=0D
- if (!EFI_ERROR (Status) && MorLock) {=0D
- //=0D
- // If lock, deny access=0D
- //=0D
- return EFI_INVALID_PARAMETER;=0D
- }=0D
-=0D
- //=0D
- // Delete not OK=0D
- //=0D
- if ((DataSize !=3D sizeof(UINT8)) || (Data =3D=3D NULL) || (Attributes =
=3D=3D 0)) {=0D
- return EFI_INVALID_PARAMETER;=0D
- }=0D
-=0D
- //=0D
- // check format=0D
- //=0D
- if (IsMorLockVariable(VariableName, VendorGuid)) {=0D
- //=0D
- // set to any other value not OK=0D
- //=0D
- if ((*(UINT8 *)Data !=3D 1) && (*(UINT8 *)Data !=3D 0)) {=0D
- return EFI_INVALID_PARAMETER;=0D
- }=0D
- }=0D
- //=0D
- // Or grant access=0D
- //=0D
- return EFI_SUCCESS;=0D
-}=0D
-=0D
-/**=0D
- Entry Point for MOR Lock Control driver.=0D
-=0D
- @param[in] ImageHandle Image handle of this driver.=0D
- @param[in] SystemTable A Pointer to the EFI System Table.=0D
-=0D
- @retval EFI_SUCCESS=0D
- @return Others Some error occurs.=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-MorLockDriverInit (=0D
- VOID=0D
- )=0D
-{=0D
- EFI_STATUS Status;=0D
- UINT8 Data;=0D
-=0D
- Data =3D 0;=0D
- Status =3D InternalSetVariable (=0D
- MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,=0D
- &gEfiMemoryOverwriteRequestControlLockGuid,=0D
- EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS |=
EFI_VARIABLE_RUNTIME_ACCESS,=0D
- 1,=0D
- &Data=0D
- );=0D
- return Status;=0D
-}=0D
diff --git a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.h=
b/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.h
deleted file mode 100644
index 5a6658c158..0000000000
--- a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.h
+++ /dev/null
@@ -1,131 +0,0 @@
-/** @file=0D
- TCG MOR (Memory Overwrite Request) Lock Control Driver header file.=0D
-=0D
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>=0D
-SPDX-License-Identifier: BSD-2-Clause-Patent=0D
-=0D
-**/=0D
-=0D
-#ifndef _EFI_TCG_MOR_LOCK_H_=0D
-#define _EFI_TCG_MOR_LOCK_H_=0D
-=0D
-/**=0D
- This service is a wrapper for the UEFI Runtime Service GetVariable().=0D
-=0D
- @param VariableName the name of the vendor's variable, it's a Null-Term=
inated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
- @param Attributes Point to memory location to return the attributes o=
f variable. If the point=0D
- is NULL, the parameter would be ignored.=0D
- @param DataSize As input, point to the maximum size of return Data-=
Buffer.=0D
- As output, point to the actual size of the returned=
Data-Buffer.=0D
- @param Data Point to return Data-Buffer.=0D
-=0D
- @retval EFI_SUCCESS The function completed successfully.=0D
- @retval EFI_NOT_FOUND The variable was not found.=0D
- @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the result=
. DataSize has=0D
- been updated with the size needed to com=
plete the request.=0D
- @retval EFI_INVALID_PARAMETER VariableName is NULL.=0D
- @retval EFI_INVALID_PARAMETER VendorGuid is NULL.=0D
- @retval EFI_INVALID_PARAMETER DataSize is NULL.=0D
- @retval EFI_INVALID_PARAMETER The DataSize is not too small and Data i=
s NULL.=0D
- @retval EFI_DEVICE_ERROR The variable could not be retrieved due =
to a hardware error.=0D
- @retval EFI_SECURITY_VIOLATION The variable could not be retrieved due =
to an authentication failure.=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-InternalGetVariable (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid,=0D
- OUT UINT32 *Attributes OPTIONAL,=0D
- IN OUT UINTN *DataSize,=0D
- OUT VOID *Data=0D
- );=0D
-=0D
-/**=0D
- This service is a wrapper for the UEFI Runtime Service SetVariable()=0D
-=0D
- @param VariableName the name of the vendor's variable, as a=0D
- Null-Terminated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
- @param Attributes Point to memory location to return the attributes o=
f variable. If the point=0D
- is NULL, the parameter would be ignored.=0D
- @param DataSize The size in bytes of Data-Buffer.=0D
- @param Data Point to the content of the variable.=0D
-=0D
- @retval EFI_SUCCESS The firmware has successfully stored the=
variable and its data as=0D
- defined by the Attributes.=0D
- @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits=
was supplied, or the=0D
- DataSize exceeds the maximum allowed.=0D
- @retval EFI_INVALID_PARAMETER VariableName is an empty Unicode string.=
=0D
- @retval EFI_OUT_OF_RESOURCES Not enough storage is available to hold =
the variable and its data.=0D
- @retval EFI_DEVICE_ERROR The variable could not be saved due to a=
hardware failure.=0D
- @retval EFI_WRITE_PROTECTED The variable in question is read-only.=0D
- @retval EFI_WRITE_PROTECTED The variable in question cannot be delet=
ed.=0D
- @retval EFI_SECURITY_VIOLATION The variable could not be written due to=
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS=0D
- set but the AuthInfo does NOT pass the v=
alidation check carried=0D
- out by the firmware.=0D
- @retval EFI_NOT_FOUND The variable trying to be updated or del=
eted was not found.=0D
-=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-InternalSetVariable (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid,=0D
- IN UINT32 Attributes,=0D
- IN UINTN DataSize,=0D
- IN VOID *Data=0D
- );=0D
-=0D
-/**=0D
- This service is a checker handler for the UEFI Runtime Service SetVariab=
le()=0D
-=0D
- @param VariableName the name of the vendor's variable, as a=0D
- Null-Terminated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
- @param Attributes Point to memory location to return the attributes o=
f variable. If the point=0D
- is NULL, the parameter would be ignored.=0D
- @param DataSize The size in bytes of Data-Buffer.=0D
- @param Data Point to the content of the variable.=0D
-=0D
- @retval EFI_SUCCESS The firmware has successfully stored the=
variable and its data as=0D
- defined by the Attributes.=0D
- @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits=
was supplied, or the=0D
- DataSize exceeds the maximum allowed.=0D
- @retval EFI_INVALID_PARAMETER VariableName is an empty Unicode string.=
=0D
- @retval EFI_OUT_OF_RESOURCES Not enough storage is available to hold =
the variable and its data.=0D
- @retval EFI_DEVICE_ERROR The variable could not be saved due to a=
hardware failure.=0D
- @retval EFI_WRITE_PROTECTED The variable in question is read-only.=0D
- @retval EFI_WRITE_PROTECTED The variable in question cannot be delet=
ed.=0D
- @retval EFI_SECURITY_VIOLATION The variable could not be written due to=
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS=0D
- set but the AuthInfo does NOT pass the v=
alidation check carried=0D
- out by the firmware.=0D
- @retval EFI_NOT_FOUND The variable trying to be updated or del=
eted was not found.=0D
-=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-SetVariableCheckHandlerMor (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid,=0D
- IN UINT32 Attributes,=0D
- IN UINTN DataSize,=0D
- IN VOID *Data=0D
- );=0D
-=0D
-/**=0D
- Entry Point for MOR Lock Control driver.=0D
-=0D
- @param[in] ImageHandle Image handle of this driver.=0D
- @param[in] SystemTable A Pointer to the EFI System Table.=0D
-=0D
- @retval EFI_SUCCESS=0D
- @return Others Some error occurs.=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-MorLockDriverInit (=0D
- VOID=0D
- );=0D
-=0D
-#endif=0D
diff --git a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.u=
ni b/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.uni
deleted file mode 100644
index 711b37d866..0000000000
--- a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLock.uni
+++ /dev/null
@@ -1,16 +0,0 @@
-// /** @file=0D
-// Initializes MemoryOverwriteRequestControlLock variable=0D
-//=0D
-// This module will add Variable Hook and allow MemoryOverwriteRequestCont=
rolLock variable set only once.=0D
-//=0D
-// Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>=0D
-//=0D
-// SPDX-License-Identifier: BSD-2-Clause-Patent=0D
-//=0D
-// **/=0D
-=0D
-=0D
-#string STR_MODULE_ABSTRACT #language en-US "Initializes Memor=
yOverwriteRequestControlLock variable"=0D
-=0D
-#string STR_MODULE_DESCRIPTION #language en-US "This module will =
add Variable Hook and allow MemoryOverwriteRequestControlLock variable set =
only once."=0D
-=0D
diff --git a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockEx=
tra.uni b/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockExtra=
.uni
deleted file mode 100644
index 2679c08c86..0000000000
--- a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockExtra.uni
+++ /dev/null
@@ -1,14 +0,0 @@
-// /** @file=0D
-// TcgMorLock Localized Strings and Content=0D
-//=0D
-// Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>=
=0D
-//=0D
-// SPDX-License-Identifier: BSD-2-Clause-Patent=0D
-//=0D
-// **/=0D
-=0D
-#string STR_PROPERTIES_MODULE_NAME=0D
-#language en-US=0D
-"TCG (Trusted Computing Group) MOR Lock"=0D
-=0D
-=0D
diff --git a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSm=
m.c b/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSmm.c
deleted file mode 100644
index 8c92317313..0000000000
--- a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSmm.c
+++ /dev/null
@@ -1,152 +0,0 @@
-/** @file=0D
- TCG MOR (Memory Overwrite Request) Lock Control Driver SMM wrapper.=0D
-=0D
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>=0D
-SPDX-License-Identifier: BSD-2-Clause-Patent=0D
-=0D
-**/=0D
-=0D
-#include <PiSmm.h>=0D
-#include <Library/SmmServicesTableLib.h>=0D
-#include <Library/DebugLib.h>=0D
-#include <Protocol/SmmVarCheck.h>=0D
-#include <Protocol/SmmVariable.h>=0D
-#include "TcgMorLock.h"=0D
-=0D
-EFI_SMM_VARIABLE_PROTOCOL *mSmmVariable;=0D
-=0D
-/**=0D
- This service is a wrapper for the UEFI Runtime Service GetVariable().=0D
-=0D
- @param VariableName the name of the vendor's variable, it's a Null-Term=
inated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
- @param Attributes Point to memory location to return the attributes o=
f variable. If the point=0D
- is NULL, the parameter would be ignored.=0D
- @param DataSize As input, point to the maximum size of return Data-=
Buffer.=0D
- As output, point to the actual size of the returned=
Data-Buffer.=0D
- @param Data Point to return Data-Buffer.=0D
-=0D
- @retval EFI_SUCCESS The function completed successfully.=0D
- @retval EFI_NOT_FOUND The variable was not found.=0D
- @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the result=
. DataSize has=0D
- been updated with the size needed to com=
plete the request.=0D
- @retval EFI_INVALID_PARAMETER VariableName is NULL.=0D
- @retval EFI_INVALID_PARAMETER VendorGuid is NULL.=0D
- @retval EFI_INVALID_PARAMETER DataSize is NULL.=0D
- @retval EFI_INVALID_PARAMETER The DataSize is not too small and Data i=
s NULL.=0D
- @retval EFI_DEVICE_ERROR The variable could not be retrieved due =
to a hardware error.=0D
- @retval EFI_SECURITY_VIOLATION The variable could not be retrieved due =
to an authentication failure.=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-InternalGetVariable (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid,=0D
- OUT UINT32 *Attributes OPTIONAL,=0D
- IN OUT UINTN *DataSize,=0D
- OUT VOID *Data=0D
- )=0D
-{=0D
- return mSmmVariable->SmmGetVariable (=0D
- VariableName,=0D
- VendorGuid,=0D
- Attributes,=0D
- DataSize,=0D
- Data=0D
- );=0D
-}=0D
-=0D
-/**=0D
- This service is a wrapper for the UEFI Runtime Service SetVariable()=0D
-=0D
- @param VariableName the name of the vendor's variable, as a=0D
- Null-Terminated Unicode String=0D
- @param VendorGuid Unify identifier for vendor.=0D
- @param Attributes Point to memory location to return the attributes o=
f variable. If the point=0D
- is NULL, the parameter would be ignored.=0D
- @param DataSize The size in bytes of Data-Buffer.=0D
- @param Data Point to the content of the variable.=0D
-=0D
- @retval EFI_SUCCESS The firmware has successfully stored the=
variable and its data as=0D
- defined by the Attributes.=0D
- @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits=
was supplied, or the=0D
- DataSize exceeds the maximum allowed.=0D
- @retval EFI_INVALID_PARAMETER VariableName is an empty Unicode string.=
=0D
- @retval EFI_OUT_OF_RESOURCES Not enough storage is available to hold =
the variable and its data.=0D
- @retval EFI_DEVICE_ERROR The variable could not be saved due to a=
hardware failure.=0D
- @retval EFI_WRITE_PROTECTED The variable in question is read-only.=0D
- @retval EFI_WRITE_PROTECTED The variable in question cannot be delet=
ed.=0D
- @retval EFI_SECURITY_VIOLATION The variable could not be written due to=
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS=0D
- set but the AuthInfo does NOT pass the v=
alidation check carried=0D
- out by the firmware.=0D
- @retval EFI_NOT_FOUND The variable trying to be updated or del=
eted was not found.=0D
-=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-InternalSetVariable (=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid,=0D
- IN UINT32 Attributes,=0D
- IN UINTN DataSize,=0D
- IN VOID *Data=0D
- )=0D
-{=0D
- return mSmmVariable->SmmSetVariable (=0D
- VariableName,=0D
- VendorGuid,=0D
- Attributes,=0D
- DataSize,=0D
- Data=0D
- );=0D
-}=0D
-=0D
-/**=0D
- Entry Point for MOR Lock Control driver.=0D
-=0D
- @param[in] ImageHandle The firmware allocated handle for the EFI imag=
e.=0D
- @param[in] SystemTable A pointer to the EFI System Table.=0D
-=0D
- @retval EFI_SUCCESS EntryPoint runs successfully.=0D
-=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-MorLockDriverEntryPointSmm (=0D
- IN EFI_HANDLE ImageHandle,=0D
- IN EFI_SYSTEM_TABLE *SystemTable=0D
- )=0D
-{=0D
- EFI_STATUS Status;=0D
- EDKII_SMM_VAR_CHECK_PROTOCOL *SmmVarCheck;=0D
-=0D
- //=0D
- // This driver link to Smm Variable driver=0D
- //=0D
- DEBUG ((EFI_D_INFO, "MorLockDriverEntryPointSmm\n"));=0D
-=0D
- Status =3D gSmst->SmmLocateProtocol (=0D
- &gEfiSmmVariableProtocolGuid,=0D
- NULL,=0D
- (VOID **) &mSmmVariable=0D
- );=0D
- ASSERT_EFI_ERROR (Status);=0D
-=0D
- Status =3D gSmst->SmmLocateProtocol (=0D
- &gEdkiiSmmVarCheckProtocolGuid,=0D
- NULL,=0D
- (VOID **) &SmmVarCheck=0D
- );=0D
- ASSERT_EFI_ERROR (Status);=0D
-=0D
- Status =3D MorLockDriverInit ();=0D
- if (EFI_ERROR (Status)) {=0D
- return Status;=0D
- }=0D
-=0D
- Status =3D SmmVarCheck->SmmRegisterSetVariableCheckHandler (SetVariableC=
heckHandlerMor);=0D
- ASSERT_EFI_ERROR (Status);=0D
-=0D
- return Status;=0D
-}=0D
-=0D
diff --git a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSm=
m.inf b/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSmm.inf
deleted file mode 100644
index 875c1e5f3a..0000000000
--- a/SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSmm.inf
+++ /dev/null
@@ -1,65 +0,0 @@
-## @file=0D
-# Initializes MemoryOverwriteRequestControlLock variable=0D
-#=0D
-# This module will add Variable Hook and allow MemoryOverwriteRequestCont=
rolLock variable set only once.=0D
-#=0D
-# NOTE: This module only handles secure MOR V1 and is deprecated.=0D
-# The secure MOR V2 is handled inside of variable driver.=0D
-#=0D
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>=0D
-# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
-#=0D
-##=0D
-=0D
-[Defines]=0D
- INF_VERSION =3D 0x00010005=0D
- BASE_NAME =3D TcgMorLockSmm=0D
- MODULE_UNI_FILE =3D TcgMorLock.uni=0D
- FILE_GUID =3D E2EA6F47-E678-47FA-8C1B-02A03E825C6E=
=0D
- MODULE_TYPE =3D DXE_SMM_DRIVER=0D
- VERSION_STRING =3D 1.0=0D
- PI_SPECIFICATION_VERSION =3D 0x0001000A=0D
- ENTRY_POINT =3D MorLockDriverEntryPointSmm=0D
-=0D
-#=0D
-# The following information is for reference only and not required by the =
build tools.=0D
-#=0D
-# VALID_ARCHITECTURES =3D IA32 X64 EBC=0D
-#=0D
-=0D
-[Sources]=0D
- TcgMorLock.h=0D
- TcgMorLock.c=0D
- TcgMorLockSmm.c=0D
-=0D
-[Packages]=0D
- MdePkg/MdePkg.dec=0D
- MdeModulePkg/MdeModulePkg.dec=0D
- SecurityPkg/SecurityPkg.dec=0D
-=0D
-[LibraryClasses]=0D
- UefiDriverEntryPoint=0D
- SmmServicesTableLib=0D
- DebugLib=0D
- BaseLib=0D
- BaseMemoryLib=0D
-=0D
-[Guids]=0D
- ## SOMETIMES_CONSUMES ## Variable:L"MemoryOverwriteRequestControl"=
=0D
- gEfiMemoryOverwriteControlDataGuid=0D
-=0D
- ## SOMETIMES_CONSUMES ## Variable:L"MemoryOverwriteRequestControlLo=
ck"=0D
- ## PRODUCES ## Variable:L"MemoryOverwriteRequestControlLo=
ck"=0D
- gEfiMemoryOverwriteRequestControlLockGuid=0D
-=0D
-[Protocols]=0D
- gEdkiiSmmVarCheckProtocolGuid ## CONSUMES=0D
- gEfiSmmVariableProtocolGuid ## CONSUMES=0D
-=0D
-[Depex]=0D
- gEfiSmmVariableProtocolGuid AND=0D
- gSmmVariableWriteGuid AND=0D
- ( gEfiTcgProtocolGuid OR gEfiTcg2ProtocolGuid )=0D
-=0D
-[UserExtensions.TianoCore."ExtraFiles"]=0D
- TcgMorLockExtra.uni=0D
--=20
2.26.2.windows.1


Re: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

Michael D Kinney
 

Pushed as ff31f8f683..75899d2a8f

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Monday, August 23, 2021 6:58 PM
To: Pedro Falcato <pedro.falcato@...>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>
Cc: Leif Lindholm <leif@...>; Bret Barkelew <Bret.Barkelew@...>
Subject: RE: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

Hi Pedro,

Found one VS compat issue with signed/unsigned comparison in last patch.

It was a very simple fix for force an unsigned compare.

Change:
if (Len < EXT4_MIN_DIR_ENTRY_LEN + Entry.name_len) {

To:
if (Len < (UINTN)(EXT4_MIN_DIR_ENTRY_LEN + Entry.name_len)) {


With this one change: Series Reviewed-by: Michael D Kinney <michael.d.kinney@...>

I will do push with this change

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Monday, August 23, 2021 6:42 PM
To: Pedro Falcato <pedro.falcato@...>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>
Cc: Leif Lindholm <leif@...>; Bret Barkelew <Bret.Barkelew@...>
Subject: RE: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

Series

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

Mike

-----Original Message-----
From: Pedro Falcato <pedro.falcato@...>
Sent: Saturday, August 21, 2021 7:47 AM
To: devel@edk2.groups.io
Cc: Pedro Falcato <pedro.falcato@...>; Leif Lindholm <leif@...>; Kinney, Michael D
<michael.d.kinney@...>; Bret Barkelew <Bret.Barkelew@...>
Subject: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

This patch-series addresses bugs found when testing the filesystem with
more complex usage of the file protocol, particularly through the shell
itself.

This is version 2 of the patch series and addresses feedback received
from the community. This version also adds two new patches to further
improve Ext4Dxe and make it more resilient and ready to be used.

Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Bret Barkelew <Bret.Barkelew@...>

Pedro Falcato (5):
Ext4Pkg: Fix incorrect usage of Ext4InitExtentsMap.
Ext4Pkg: Hide "." and ".." entries from Read() callers.
Ext4Pkg: Add a directory entry tree.
Ext4Pkg: Add handling of EFI_FILE_SYSTEM_VOLUME_LABEL GetInfo().
Ext4Pkg: Sanity check more EXT4_DIR_ENTRY values.

Features/Ext4Pkg/Ext4Dxe/Directory.c | 343 ++++++++++++++++++++------
Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 3 +
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 1 -
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 88 ++++++-
Features/Ext4Pkg/Ext4Dxe/File.c | 202 +++++++++++----
Features/Ext4Pkg/Ext4Dxe/Inode.c | 3 +-
Features/Ext4Pkg/Ext4Dxe/Partition.c | 7 +
Features/Ext4Pkg/Ext4Dxe/Superblock.c | 23 +-
8 files changed, 534 insertions(+), 136 deletions(-)

--
2.33.0


Re: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

Michael D Kinney
 

Hi Pedro,

Found one VS compat issue with signed/unsigned comparison in last patch.

It was a very simple fix for force an unsigned compare.

Change:
if (Len < EXT4_MIN_DIR_ENTRY_LEN + Entry.name_len) {

To:
if (Len < (UINTN)(EXT4_MIN_DIR_ENTRY_LEN + Entry.name_len)) {


With this one change: Series Reviewed-by: Michael D Kinney <michael.d.kinney@...>

I will do push with this change

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Monday, August 23, 2021 6:42 PM
To: Pedro Falcato <pedro.falcato@...>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>
Cc: Leif Lindholm <leif@...>; Bret Barkelew <Bret.Barkelew@...>
Subject: RE: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

Series

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

Mike

-----Original Message-----
From: Pedro Falcato <pedro.falcato@...>
Sent: Saturday, August 21, 2021 7:47 AM
To: devel@edk2.groups.io
Cc: Pedro Falcato <pedro.falcato@...>; Leif Lindholm <leif@...>; Kinney, Michael D
<michael.d.kinney@...>; Bret Barkelew <Bret.Barkelew@...>
Subject: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

This patch-series addresses bugs found when testing the filesystem with
more complex usage of the file protocol, particularly through the shell
itself.

This is version 2 of the patch series and addresses feedback received
from the community. This version also adds two new patches to further
improve Ext4Dxe and make it more resilient and ready to be used.

Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Bret Barkelew <Bret.Barkelew@...>

Pedro Falcato (5):
Ext4Pkg: Fix incorrect usage of Ext4InitExtentsMap.
Ext4Pkg: Hide "." and ".." entries from Read() callers.
Ext4Pkg: Add a directory entry tree.
Ext4Pkg: Add handling of EFI_FILE_SYSTEM_VOLUME_LABEL GetInfo().
Ext4Pkg: Sanity check more EXT4_DIR_ENTRY values.

Features/Ext4Pkg/Ext4Dxe/Directory.c | 343 ++++++++++++++++++++------
Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 3 +
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 1 -
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 88 ++++++-
Features/Ext4Pkg/Ext4Dxe/File.c | 202 +++++++++++----
Features/Ext4Pkg/Ext4Dxe/Inode.c | 3 +-
Features/Ext4Pkg/Ext4Dxe/Partition.c | 7 +
Features/Ext4Pkg/Ext4Dxe/Superblock.c | 23 +-
8 files changed, 534 insertions(+), 136 deletions(-)

--
2.33.0


Re: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

Michael D Kinney
 

Series

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

Mike

-----Original Message-----
From: Pedro Falcato <pedro.falcato@...>
Sent: Saturday, August 21, 2021 7:47 AM
To: devel@edk2.groups.io
Cc: Pedro Falcato <pedro.falcato@...>; Leif Lindholm <leif@...>; Kinney, Michael D
<michael.d.kinney@...>; Bret Barkelew <Bret.Barkelew@...>
Subject: [edk2-platforms PATCH v2 0/5] Ext4Pkg: Fix bugs

This patch-series addresses bugs found when testing the filesystem with
more complex usage of the file protocol, particularly through the shell
itself.

This is version 2 of the patch series and addresses feedback received
from the community. This version also adds two new patches to further
improve Ext4Dxe and make it more resilient and ready to be used.

Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Bret Barkelew <Bret.Barkelew@...>

Pedro Falcato (5):
Ext4Pkg: Fix incorrect usage of Ext4InitExtentsMap.
Ext4Pkg: Hide "." and ".." entries from Read() callers.
Ext4Pkg: Add a directory entry tree.
Ext4Pkg: Add handling of EFI_FILE_SYSTEM_VOLUME_LABEL GetInfo().
Ext4Pkg: Sanity check more EXT4_DIR_ENTRY values.

Features/Ext4Pkg/Ext4Dxe/Directory.c | 343 ++++++++++++++++++++------
Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 3 +
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 1 -
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 88 ++++++-
Features/Ext4Pkg/Ext4Dxe/File.c | 202 +++++++++++----
Features/Ext4Pkg/Ext4Dxe/Inode.c | 3 +-
Features/Ext4Pkg/Ext4Dxe/Partition.c | 7 +
Features/Ext4Pkg/Ext4Dxe/Superblock.c | 23 +-
8 files changed, 534 insertions(+), 136 deletions(-)

--
2.33.0


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

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

Reminder: TianoCore Bug Triage - APAC / NAMO

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

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

Organizer: Liming Gao gaoliming@...

View Event

Description:

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao

 

________________________________________________________________________________

Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions

Or call in (audio only)

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

Phone Conference ID: 774 638 21#

Find a local number | Reset PIN

Learn More | Meeting options


Re: [PATCH] UefiPayloadPkg: Add FV Guid for DXEFV and PLDFV

King Sumo
 

Hi All,

This patch broke the coreboot payload loading. Tested with:
build -a IA32 -a X64 -p UefiPayloadPkg/UefiPayloadPkg.dsc -b RELEASE -t GCC5 -D BOOTLOADER=COREBOOT

Basically the coreboot cbfstool reports the following error when creating the CBFS / flash image:
"Not a usable UEFI firmware volume"

Trying to boot coreboot results in an exception and the following error message:
"Payload not loaded"

Probably it broke the interface.

commit 4bac086e8e007c7143e33f87bb96238326d1d6ba
Author: Zhiguang Liu <zhiguang.liu@...>
Date:   Wed Jul 14 14:24:45 2021 +0800

    UefiPayloadPkg: Add FV Guid for DXEFV and PLDFV

    Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
    Reviewed-by: Ray Ni <ray.ni@...>
    Reviewed-by: Guo Dong <guo.dong@...>


Kind regards,
Sumo

On Wed, Jul 14, 2021 at 1:08 PM Guo Dong <guo.dong@...> wrote:

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

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Zhiguang Liu
> Sent: Tuesday, July 13, 2021 11:25 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH] UefiPayloadPkg: Add FV Guid for DXEFV and
> PLDFV
>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
> ---
>  UefiPayloadPkg/UefiPayloadPkg.fdf | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf
> b/UefiPayloadPkg/UefiPayloadPkg.fdf
> index 2d51fdbacb..041fed842c 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.fdf
> +++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
> @@ -34,6 +34,7 @@ FV = PLDFV
>
>
>
> ##########################################################
> ######################
>
>  [FV.PLDFV]
>
> +FvNameGuid         = 96E75986-6FDD-491E-9FD5-35E21AC45B45
>
>  BlockSize          = $(FD_BLOCK_SIZE)
>
>  FvAlignment        = 16
>
>  ERASE_POLARITY     = 1
>
> @@ -62,6 +63,7 @@ FILE FV_IMAGE = 4E35FD93-9C72-4c15-8C4B-
> E77F1DB2D793 {
>
> ##########################################################
> ######################
>
>
>
>  [FV.DXEFV]
>
> +FvNameGuid         = 8063C21A-8E58-4576-95CE-089E87975D23
>
>  BlockSize          = $(FD_BLOCK_SIZE)
>
>  FvForceRebase      = FALSE
>
>  FvAlignment        = 16
>
> --
> 2.30.0.windows.2
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#77762): https://edk2.groups.io/g/devel/message/77762
> Mute This Topic: https://groups.io/mt/84196221/1781375
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [guo.dong@...]
> -=-=-=-=-=-=
>






12641 - 12660 of 92312