Re: [PATCH v1 1/1] DynamicTablesPkg: Add ACPI 6.3 SPE support to MADT generator


Sami Mujawar
 

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

-----Original Message-----
From: Krzysztof Koch <krzysztof.koch@...>
Sent: 16 May 2019 03:53 PM
To: devel@edk2.groups.io
Cc: leif.lindholm@...; Sami Mujawar <Sami.Mujawar@...>; Alexei Fedorov <Alexei.Fedorov@...>; Matteo Carlini <Matteo.Carlini@...>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@...>; nd <nd@...>
Subject: [PATCH v1 1/1] DynamicTablesPkg: Add ACPI 6.3 SPE support to MADT generator

The Dynamic Tables Framework now supports generating Multiple APIC Description Table (MADT) revision 5 for ARM platforms while maintaining backward-compatibility with ACPI 6.2.

The relevant change is the enablement of the Statistical Profiling Extension (SPE).

Signed-off-by: Krzysztof Koch <krzysztof.koch@...>
---

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/476_madt_add_spe_v1

Notes:
v1:
- Add support for SPE in MADT generator [Krzysztof]

DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 7 ++
DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c | 105 +++++++++++---------
2 files changed, 66 insertions(+), 46 deletions(-)

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index bf70dc76cea044b9a8e8eb529d2ddbf892bafd58..d9dcca12db68d16d3ddb73553ecd2383358832e7 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -155,6 +155,13 @@ typedef struct CmArmGicCInfo {
ACPI Specification.
*/
UINT8 ProcessorPowerEfficiencyClass;
+
+ /** Statistical Profiling Extension buffer overflow GSIV. Zero if
+ unsupported by this processor. This field was introduced in
+ ACPI 6.3 (MADT revision 5) and is therefore ignored when
+ generating MADT revision 4 or lower.
+ */
+ UINT16 SpeOverflowInterrupt;
} CM_ARM_GICC_INFO;

/** A structure that describes the
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
index ab9734fb31480f1b653227d1d56abf60bb04f98a..3945d18d76915938081a5ef36fc9e50261ea3a0a 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
@@ -5,7 +5,7 @@
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Reference(s):
- - ACPI 6.2 Specification - Errata A, September 2017
+ - ACPI 6.3 Specification - January 2019

**/

@@ -82,25 +82,27 @@ GET_OBJECT_LIST (
);

/** This function updates the GIC CPU Interface Information in the
- EFI_ACPI_6_2_GIC_STRUCTURE structure.
+ EFI_ACPI_6_3_GIC_STRUCTURE structure.

- @param [in] Gicc Pointer to GIC CPU Interface structure.
- @param [in] GicCInfo Pointer to the GIC CPU Interface Information.
+ @param [in] Gicc Pointer to GIC CPU Interface structure.
+ @param [in] GicCInfo Pointer to the GIC CPU Interface Information.
+ @param [in] MadtRev MADT table revision.
**/
STATIC
VOID
AddGICC (
- IN EFI_ACPI_6_2_GIC_STRUCTURE * CONST Gicc,
- IN CONST CM_ARM_GICC_INFO * CONST GicCInfo
+ IN EFI_ACPI_6_3_GIC_STRUCTURE * CONST Gicc,
+ IN CONST CM_ARM_GICC_INFO * CONST GicCInfo,
+ IN CONST UINT8 MadtRev
)
{
ASSERT (Gicc != NULL);
ASSERT (GicCInfo != NULL);

// UINT8 Type
- Gicc->Type = EFI_ACPI_6_2_GIC;
+ Gicc->Type = EFI_ACPI_6_3_GIC;
// UINT8 Length
- Gicc->Length = sizeof (EFI_ACPI_6_2_GIC_STRUCTURE);
+ Gicc->Length = sizeof (EFI_ACPI_6_3_GIC_STRUCTURE);
// UINT16 Reserved
Gicc->Reserved = EFI_ACPI_RESERVED_WORD;

@@ -134,10 +136,18 @@ AddGICC (
// UINT8 ProcessorPowerEfficiencyClass
Gicc->ProcessorPowerEfficiencyClass =
GicCInfo->ProcessorPowerEfficiencyClass;
- // UINT8 Reserved2[3]
- Gicc->Reserved2[0] = EFI_ACPI_RESERVED_BYTE;
- Gicc->Reserved2[1] = EFI_ACPI_RESERVED_BYTE;
- Gicc->Reserved2[2] = EFI_ACPI_RESERVED_BYTE;
+ // UINT8 Reserved2
+ Gicc->Reserved2 = EFI_ACPI_RESERVED_BYTE;
+
+ // UINT16 SpeOverflowInterrupt
+ if (MadtRev > EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION) {
+ Gicc->SpeOverflowInterrupt = GicCInfo->SpeOverflowInterrupt; }
+ else {
+ // Setting SpeOverflowInterrupt to 0 ensures backward compatibility with
+ // ACPI 6.2 by also clearing the Reserved2[1] and Reserved2[2] fields
+ // in EFI_ACPI_6_2_GIC_STRUCTURE.
+ Gicc->SpeOverflowInterrupt = 0;
+ }
}

/** Add the GIC CPU Interface Information to the MADT Table.
@@ -147,20 +157,22 @@ AddGICC (
@param [in] GicCInfo Pointer to the GIC CPU
Information list.
@param [in] GicCCount Count of GIC CPU Interfaces.
+ @param [in] MadtRev MADT table revision.
**/
STATIC
VOID
AddGICCList (
- IN EFI_ACPI_6_2_GIC_STRUCTURE * Gicc,
+ IN EFI_ACPI_6_3_GIC_STRUCTURE * Gicc,
IN CONST CM_ARM_GICC_INFO * GicCInfo,
- IN UINT32 GicCCount
+ IN UINT32 GicCCount,
+ IN CONST UINT8 MadtRev
)
{
ASSERT (Gicc != NULL);
ASSERT (GicCInfo != NULL);

while (GicCCount-- != 0) {
- AddGICC (Gicc++, GicCInfo++);
+ AddGICC (Gicc++, GicCInfo++, MadtRev);
}
}

@@ -172,7 +184,7 @@ AddGICCList (
STATIC
VOID
AddGICD (
- EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTURE * CONST Gicd,
+ EFI_ACPI_6_3_GIC_DISTRIBUTOR_STRUCTURE * CONST Gicd,
CONST CM_ARM_GICD_INFO * CONST GicDInfo
)
{
@@ -180,9 +192,9 @@ AddGICD (
ASSERT (GicDInfo != NULL);

// UINT8 Type
- Gicd->Type = EFI_ACPI_6_2_GICD;
+ Gicd->Type = EFI_ACPI_6_3_GICD;
// UINT8 Length
- Gicd->Length = sizeof (EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTURE);
+ Gicd->Length = sizeof (EFI_ACPI_6_3_GIC_DISTRIBUTOR_STRUCTURE);
// UINT16 Reserved
Gicd->Reserved1 = EFI_ACPI_RESERVED_WORD;
// UINT32 Identifier
@@ -209,15 +221,15 @@ AddGICD (
STATIC
VOID
AddGICMsiFrame (
- IN EFI_ACPI_6_2_GIC_MSI_FRAME_STRUCTURE * CONST GicMsiFrame,
+ IN EFI_ACPI_6_3_GIC_MSI_FRAME_STRUCTURE * CONST GicMsiFrame,
IN CONST CM_ARM_GIC_MSI_FRAME_INFO * CONST GicMsiFrameInfo
)
{
ASSERT (GicMsiFrame != NULL);
ASSERT (GicMsiFrameInfo != NULL);

- GicMsiFrame->Type = EFI_ACPI_6_2_GIC_MSI_FRAME;
- GicMsiFrame->Length = sizeof (EFI_ACPI_6_2_GIC_MSI_FRAME_STRUCTURE);
+ GicMsiFrame->Type = EFI_ACPI_6_3_GIC_MSI_FRAME; GicMsiFrame->Length
+ = sizeof (EFI_ACPI_6_3_GIC_MSI_FRAME_STRUCTURE);
GicMsiFrame->Reserved1 = EFI_ACPI_RESERVED_WORD;
GicMsiFrame->GicMsiFrameId = GicMsiFrameInfo->GicMsiFrameId;
GicMsiFrame->PhysicalBaseAddress = GicMsiFrameInfo->PhysicalBaseAddress;
@@ -236,7 +248,7 @@ AddGICMsiFrame (
STATIC
VOID
AddGICMsiFrameInfoList (
- IN EFI_ACPI_6_2_GIC_MSI_FRAME_STRUCTURE * GicMsiFrame,
+ IN EFI_ACPI_6_3_GIC_MSI_FRAME_STRUCTURE * GicMsiFrame,
IN CONST CM_ARM_GIC_MSI_FRAME_INFO * GicMsiFrameInfo,
IN UINT32 GicMsiFrameCount
)
@@ -257,15 +269,15 @@ AddGICMsiFrameInfoList ( STATIC VOID AddGICRedistributor (
- IN EFI_ACPI_6_2_GICR_STRUCTURE * CONST Gicr,
+ IN EFI_ACPI_6_3_GICR_STRUCTURE * CONST Gicr,
IN CONST CM_ARM_GIC_REDIST_INFO * CONST GicRedisributorInfo
)
{
ASSERT (Gicr != NULL);
ASSERT (GicRedisributorInfo != NULL);

- Gicr->Type = EFI_ACPI_6_2_GICR;
- Gicr->Length = sizeof (EFI_ACPI_6_2_GICR_STRUCTURE);
+ Gicr->Type = EFI_ACPI_6_3_GICR;
+ Gicr->Length = sizeof (EFI_ACPI_6_3_GICR_STRUCTURE);
Gicr->Reserved = EFI_ACPI_RESERVED_WORD;
Gicr->DiscoveryRangeBaseAddress =
GicRedisributorInfo->DiscoveryRangeBaseAddress;
@@ -281,7 +293,7 @@ AddGICRedistributor ( STATIC VOID AddGICRedistributorList (
- IN EFI_ACPI_6_2_GICR_STRUCTURE * Gicr,
+ IN EFI_ACPI_6_3_GICR_STRUCTURE * Gicr,
IN CONST CM_ARM_GIC_REDIST_INFO * GicRInfo,
IN UINT32 GicRCount
)
@@ -302,15 +314,15 @@ AddGICRedistributorList ( STATIC VOID AddGICInterruptTranslationService (
- IN EFI_ACPI_6_2_GIC_ITS_STRUCTURE * CONST GicIts,
+ IN EFI_ACPI_6_3_GIC_ITS_STRUCTURE * CONST GicIts,
IN CONST CM_ARM_GIC_ITS_INFO * CONST GicItsInfo
)
{
ASSERT (GicIts != NULL);
ASSERT (GicItsInfo != NULL);

- GicIts->Type = EFI_ACPI_6_2_GIC_ITS;
- GicIts->Length = sizeof (EFI_ACPI_6_2_GIC_ITS_STRUCTURE);
+ GicIts->Type = EFI_ACPI_6_3_GIC_ITS;
+ GicIts->Length = sizeof (EFI_ACPI_6_3_GIC_ITS_STRUCTURE);
GicIts->Reserved = EFI_ACPI_RESERVED_WORD;
GicIts->GicItsId = GicItsInfo->GicItsId;
GicIts->PhysicalBaseAddress = GicItsInfo->PhysicalBaseAddress; @@ -327,7 +339,7 @@ AddGICInterruptTranslationService ( STATIC VOID AddGICItsList (
- IN EFI_ACPI_6_2_GIC_ITS_STRUCTURE * GicIts,
+ IN EFI_ACPI_6_3_GIC_ITS_STRUCTURE * GicIts,
IN CONST CM_ARM_GIC_ITS_INFO * GicItsInfo,
IN UINT32 GicItsCount
)
@@ -390,7 +402,7 @@ BuildMadtTable (
UINT32 GicRedistOffset;
UINT32 GicItsOffset;

- EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER * Madt;
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER * Madt;

ASSERT (This != NULL);
ASSERT (AcpiTableInfo != NULL);
@@ -521,22 +533,22 @@ BuildMadtTable (
goto error_handler;
}

- TableSize = sizeof (EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
+ TableSize = sizeof
+ (EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);

GicCOffset = TableSize;
- TableSize += (sizeof (EFI_ACPI_6_2_GIC_STRUCTURE) * GicCCount);
+ TableSize += (sizeof (EFI_ACPI_6_3_GIC_STRUCTURE) * GicCCount);

GicDOffset = TableSize;
- TableSize += (sizeof (EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTURE) * GicDCount);
+ TableSize += (sizeof (EFI_ACPI_6_3_GIC_DISTRIBUTOR_STRUCTURE) *
+ GicDCount);

GicMSIOffset = TableSize;
- TableSize += (sizeof (EFI_ACPI_6_2_GIC_MSI_FRAME_STRUCTURE) * GicMSICount);
+ TableSize += (sizeof (EFI_ACPI_6_3_GIC_MSI_FRAME_STRUCTURE) *
+ GicMSICount);

GicRedistOffset = TableSize;
- TableSize += (sizeof (EFI_ACPI_6_2_GICR_STRUCTURE) * GicRedistCount);
+ TableSize += (sizeof (EFI_ACPI_6_3_GICR_STRUCTURE) * GicRedistCount);

GicItsOffset = TableSize;
- TableSize += (sizeof (EFI_ACPI_6_2_GIC_ITS_STRUCTURE) * GicItsCount);
+ TableSize += (sizeof (EFI_ACPI_6_3_GIC_ITS_STRUCTURE) * GicItsCount);

// Allocate the Buffer for MADT table
*Table = (EFI_ACPI_DESCRIPTION_HEADER*)AllocateZeroPool (TableSize); @@ -552,7 +564,7 @@ BuildMadtTable (
goto error_handler;
}

- Madt = (EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER*)*Table;
+ Madt = (EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER*)*Table;

DEBUG ((
DEBUG_INFO,
@@ -578,19 +590,20 @@ BuildMadtTable (
}

AddGICCList (
- (EFI_ACPI_6_2_GIC_STRUCTURE*)((UINT8*)Madt + GicCOffset),
+ (EFI_ACPI_6_3_GIC_STRUCTURE*)((UINT8*)Madt + GicCOffset),
GicCInfo,
- GicCCount
+ GicCCount,
+ Madt->Header.Revision
);

AddGICD (
- (EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTURE*)((UINT8*)Madt + GicDOffset),
+ (EFI_ACPI_6_3_GIC_DISTRIBUTOR_STRUCTURE*)((UINT8*)Madt +
+ GicDOffset),
GicDInfo
);

if (GicMSICount != 0) {
AddGICMsiFrameInfoList (
- (EFI_ACPI_6_2_GIC_MSI_FRAME_STRUCTURE*)((UINT8*)Madt + GicMSIOffset),
+ (EFI_ACPI_6_3_GIC_MSI_FRAME_STRUCTURE*)((UINT8*)Madt +
+ GicMSIOffset),
GicMSIInfo,
GicMSICount
);
@@ -598,7 +611,7 @@ BuildMadtTable (

if (GicRedistCount != 0) {
AddGICRedistributorList (
- (EFI_ACPI_6_2_GICR_STRUCTURE*)((UINT8*)Madt + GicRedistOffset),
+ (EFI_ACPI_6_3_GICR_STRUCTURE*)((UINT8*)Madt + GicRedistOffset),
GicRedistInfo,
GicRedistCount
);
@@ -606,7 +619,7 @@ BuildMadtTable (

if (GicItsCount != 0) {
AddGICItsList (
- (EFI_ACPI_6_2_GIC_ITS_STRUCTURE*)((UINT8*)Madt + GicItsOffset),
+ (EFI_ACPI_6_3_GIC_ITS_STRUCTURE*)((UINT8*)Madt + GicItsOffset),
GicItsInfo,
GicItsCount
);
@@ -673,9 +686,9 @@ ACPI_TABLE_GENERATOR MadtGenerator = {
// Generator Description
L"ACPI.STD.MADT.GENERATOR",
// ACPI Table Signature
- EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
// ACPI Table Revision supported by this Generator
- EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
// Minimum supported ACPI Table Revision
EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
// Creator ID
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Join devel@edk2.groups.io to automatically receive all group messages.