* [PATCH v1 0/6] Enhance DynamicTablesPkg modules
@ 2022-07-19 0:22 Kun Qin
2022-07-19 0:22 ` [PATCH v1 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: Kun Qin @ 2022-07-19 0:22 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
Current DynamicTablesPkg provide great support for creating dynamic ACPI
tables during boot time.
However, there are some modules needs minor tweaks to expand support and
compatibility for OS requirements and platform needs.
This patch series proposes a few fixes to resolve minor issues discovered
in DynamicPlatRepoLib, AcpiSsdtPcieLibArm and DynamicTableManagerDxe.
Patch v1 branch: https://github.com/kuqin12/edk2/tree/dynamic_update
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Kun Qin (6):
DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf
DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing
DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers
DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed
tables
DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM
space
DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI
config
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 200 ++++++++++++++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 135 +++++++++++++
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c | 80 +++++++-
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 1 +
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf | 1 +
5 files changed, 379 insertions(+), 38 deletions(-)
--
2.36.0.windows.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
@ 2022-07-19 0:22 ` Kun Qin
2022-07-20 10:03 ` Sami Mujawar
2022-07-19 0:22 ` [PATCH v1 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing Kun Qin
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Kun Qin @ 2022-07-19 0:22 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
The DynamicPlatRepoLib has multiple reference to MemoryAllocationLib,
such as DynamicPlatRepo.c and TokenMapper.c. Not including it in the
library inf file could lead to potential build break.
This change added the MemoryAllocationLib into this inf file.
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf | 1 +
1 file changed, 1 insertion(+)
diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
index 9a3cc87fd91d..8423352550c2 100644
--- a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
+++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
@@ -31,3 +31,4 @@ [Packages]
[LibraryClasses]
AcpiHelperLib
BaseLib
+ MemoryAllocationLib
--
2.36.0.windows.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
2022-07-19 0:22 ` [PATCH v1 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
@ 2022-07-19 0:22 ` Kun Qin
2022-07-20 10:06 ` Sami Mujawar
2022-07-19 0:22 ` [PATCH v1 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers Kun Qin
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Kun Qin @ 2022-07-19 0:22 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
The content of token should be derived from the data section of the
`CmObject` instead of the object itself.
This change fixed the issue by dereferencing the token value from the
data buffer of input CmObject.
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
index 80d0aa17bc1a..84e4bb7e3bc8 100644
--- a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
+++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
@@ -60,7 +60,7 @@ TokenFixerItsGroup (
)
{
ASSERT (CmObject != NULL);
- ((CM_ARM_ITS_GROUP_NODE *)CmObject)->Token = Token;
+ ((CM_ARM_ITS_GROUP_NODE *)CmObject->Data)->Token = Token;
return EFI_SUCCESS;
}
--
2.36.0.windows.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
2022-07-19 0:22 ` [PATCH v1 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
2022-07-19 0:22 ` [PATCH v1 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing Kun Qin
@ 2022-07-19 0:22 ` Kun Qin
2022-07-20 10:39 ` Sami Mujawar
2022-07-19 0:22 ` [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Kun Qin
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Kun Qin @ 2022-07-19 0:22 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
This change added more token fixers for other node types, including
NamedComponentNode, RootComplexNode, and SmmuV3Node.
The corresponding entries for tokenFixer functions table is also updated.
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c | 78 +++++++++++++++++++-
1 file changed, 75 insertions(+), 3 deletions(-)
diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
index 84e4bb7e3bc8..345acab53f74 100644
--- a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
+++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
@@ -64,6 +64,78 @@ TokenFixerItsGroup (
return EFI_SUCCESS;
}
+/** EArmObjNamedComponent token fixer.
+
+ CmObjectToken fixer function that updates the Tokens in the CmObjects.
+
+ @param [in] CmObject Pointer to the Configuration Manager Object.
+ @param [in] Token Token to be updated in the CmObject.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_UNSUPPORTED Not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+TokenFixerNamedComponentNode (
+ IN CM_OBJ_DESCRIPTOR *CmObject,
+ IN CM_OBJECT_TOKEN Token
+ )
+{
+ ASSERT (CmObject != NULL);
+ ((CM_ARM_NAMED_COMPONENT_NODE *)CmObject->Data)->Token = Token;
+ return EFI_SUCCESS;
+}
+
+/** EArmObjRootComplex token fixer.
+
+ CmObjectToken fixer function that updates the Tokens in the CmObjects.
+
+ @param [in] CmObject Pointer to the Configuration Manager Object.
+ @param [in] Token Token to be updated in the CmObject.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_UNSUPPORTED Not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+TokenFixerRootComplexNode (
+ IN CM_OBJ_DESCRIPTOR *CmObject,
+ IN CM_OBJECT_TOKEN Token
+ )
+{
+ ASSERT (CmObject != NULL);
+ ((CM_ARM_ROOT_COMPLEX_NODE *)CmObject->Data)->Token = Token;
+ return EFI_SUCCESS;
+}
+
+/** EArmObjSmmuV3 token fixer.
+
+ CmObjectToken fixer function that updates the Tokens in the CmObjects.
+
+ @param [in] CmObject Pointer to the Configuration Manager Object.
+ @param [in] Token Token to be updated in the CmObject.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_UNSUPPORTED Not supported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+TokenFixerSmmuV3Node (
+ IN CM_OBJ_DESCRIPTOR *CmObject,
+ IN CM_OBJECT_TOKEN Token
+ )
+{
+ ASSERT (CmObject != NULL);
+ ((CM_ARM_SMMUV3_NODE *)CmObject->Data)->Token = Token;
+ return EFI_SUCCESS;
+}
+
/** TokenFixer functions table.
A CmObj having a CM_OBJECT_TOKEN field might need to have its
@@ -90,10 +162,10 @@ CM_OBJECT_TOKEN_FIXER TokenFixer[EArmObjMax] = {
NULL, ///< 16 - Hypervisor Vendor Id
NULL, ///< 17 - Fixed feature flags for FADT
TokenFixerItsGroup, ///< 18 - ITS Group
- TokenFixerNotImplemented, ///< 19 - Named Component
- TokenFixerNotImplemented, ///< 20 - Root Complex
+ TokenFixerNamedComponentNode, ///< 19 - Named Component
+ TokenFixerRootComplexNode, ///< 20 - Root Complex
TokenFixerNotImplemented, ///< 21 - SMMUv1 or SMMUv2
- TokenFixerNotImplemented, ///< 22 - SMMUv3
+ TokenFixerSmmuV3Node, ///< 22 - SMMUv3
TokenFixerNotImplemented, ///< 23 - PMCG
NULL, ///< 24 - GIC ITS Identifier Array
NULL, ///< 25 - ID Mapping Array
--
2.36.0.windows.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (2 preceding siblings ...)
2022-07-19 0:22 ` [PATCH v1 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers Kun Qin
@ 2022-07-19 0:22 ` Kun Qin
2022-07-20 11:00 ` Sami Mujawar
2022-07-20 13:36 ` [edk2-devel] " PierreGondois
2022-07-19 0:22 ` [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
` (2 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: Kun Qin @ 2022-07-19 0:22 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
This change added an extra step to allow check for installed ACPI tables.
For FADT, MADT, GTDT, DSDT and DBG2 tables, either pre-installed or
supplied through AcpiTableInfo can be accepted.
An extra check for FADT ACPI table existence during installation step is
also added.
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 200 ++++++++++++++++----
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 1 +
2 files changed, 167 insertions(+), 34 deletions(-)
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index ed62299f9bbd..ac5fe0bed91b 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
@@ -11,6 +11,7 @@
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/AcpiTable.h>
+#include <Protocol/AcpiSystemDescriptionTable.h>
// Module specific include files.
#include <AcpiTableGenerator.h>
@@ -387,6 +388,57 @@ BuildAndInstallAcpiTable (
return Status;
}
+/**
+ This function uses the ACPI SDT protocol to locate an ACPI table.
+ It is really only useful for finding tables that only have a single instance,
+ e.g. FADT, FACS, MADT, etc. It is not good for locating SSDT, etc.
+
+ @param[in] Signature - Pointer to an ASCII string containing the OEM Table ID from the ACPI table header
+ @param[in, out] Table - Updated with a pointer to the table
+ @param[in, out] Handle - AcpiSupport protocol table handle for the table found
+
+ @retval EFI_SUCCESS - The function completed successfully.
+**/
+STATIC
+EFI_STATUS
+LocateAcpiTableBySignature (
+ IN UINT32 Signature,
+ IN OUT EFI_ACPI_DESCRIPTION_HEADER **Table,
+ IN OUT UINTN *Handle
+ )
+{
+ EFI_STATUS Status;
+ INTN Index;
+ EFI_ACPI_TABLE_VERSION Version;
+ EFI_ACPI_SDT_PROTOCOL *AcpiSdt;
+
+ AcpiSdt = NULL;
+ Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL, (VOID **)&AcpiSdt);
+
+ if (EFI_ERROR (Status) || (AcpiSdt == NULL)) {
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // Locate table with matching ID
+ //
+ Version = 0;
+ Index = 0;
+ do {
+ Status = AcpiSdt->GetAcpiTable (Index, (EFI_ACPI_SDT_HEADER **)Table, &Version, Handle);
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ Index++;
+ } while ((*Table)->Signature != Signature);
+
+ //
+ // If we found the table, there will be no error.
+ //
+ return Status;
+}
+
/** The function checks if the Configuration Manager has provided the
mandatory ACPI tables for installation.
@@ -411,6 +463,9 @@ VerifyMandatoryTablesArePresent (
BOOLEAN DsdtFound;
BOOLEAN Dbg2Found;
BOOLEAN SpcrFound;
+ UINTN Handle;
+
+ EFI_ACPI_DESCRIPTION_HEADER *DummyHeader;
Status = EFI_SUCCESS;
FadtFound = FALSE;
@@ -447,32 +502,99 @@ VerifyMandatoryTablesArePresent (
}
// We need at least the FADT, MADT, GTDT and the DSDT tables to boot
- if (!FadtFound) {
- DEBUG ((DEBUG_ERROR, "ERROR: FADT Table not found\n"));
+ // But they also might be published already, so we can search from there
+ Handle = 0;
+ Status = LocateAcpiTableBySignature (
+ EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ &DummyHeader,
+ &Handle
+ );
+ if (EFI_ERROR (Status) && !FadtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: FADT Table not found.\n"));
Status = EFI_NOT_FOUND;
+ } else if (!EFI_ERROR (Status) && FadtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: FADT Table found while already published.\n"));
+ Status = EFI_ALREADY_STARTED;
+ } else {
+ FadtFound = TRUE;
}
- if (!MadtFound) {
+ Handle = 0;
+ Status = LocateAcpiTableBySignature (
+ EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+ &DummyHeader,
+ &Handle
+ );
+ if (EFI_ERROR (Status) && !MadtFound) {
DEBUG ((DEBUG_ERROR, "ERROR: MADT Table not found.\n"));
Status = EFI_NOT_FOUND;
+ } else if (!EFI_ERROR (Status) && MadtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: MADT Table found while already published.\n"));
+ Status = EFI_ALREADY_STARTED;
+ } else {
+ MadtFound = TRUE;
}
- if (!GtdtFound) {
+ Handle = 0;
+ Status = LocateAcpiTableBySignature (
+ EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
+ &DummyHeader,
+ &Handle
+ );
+ if (EFI_ERROR (Status) && !GtdtFound) {
DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table not found.\n"));
Status = EFI_NOT_FOUND;
+ } else if (!EFI_ERROR (Status) && GtdtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table found while already published.\n"));
+ Status = EFI_ALREADY_STARTED;
+ } else {
+ GtdtFound = TRUE;
}
- if (!DsdtFound) {
+ Handle = 0;
+ Status = LocateAcpiTableBySignature (
+ EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+ &DummyHeader,
+ &Handle
+ );
+ if (EFI_ERROR (Status) && !DsdtFound) {
DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table not found.\n"));
Status = EFI_NOT_FOUND;
+ } else if (!EFI_ERROR (Status) && DsdtFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table found while already published.\n"));
+ Status = EFI_ALREADY_STARTED;
+ } else {
+ DsdtFound = TRUE;
}
- if (!Dbg2Found) {
+ Handle = 0;
+ Status = LocateAcpiTableBySignature (
+ EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
+ &DummyHeader,
+ &Handle
+ );
+ if (EFI_ERROR (Status) && !Dbg2Found) {
DEBUG ((DEBUG_WARN, "WARNING: DBG2 Table not found.\n"));
+ } else if (!EFI_ERROR (Status) && Dbg2Found) {
+ DEBUG ((DEBUG_ERROR, "ERROR: DBG2 Table found while already published.\n"));
+ Status = EFI_ALREADY_STARTED;
+ } else {
+ Dbg2Found = TRUE;
}
- if (!SpcrFound) {
+ Handle = 0;
+ Status = LocateAcpiTableBySignature (
+ EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+ &DummyHeader,
+ &Handle
+ );
+ if (EFI_ERROR (Status) && !SpcrFound) {
DEBUG ((DEBUG_WARN, "WARNING: SPCR Table not found.\n"));
+ } else if (!EFI_ERROR (Status) && SpcrFound) {
+ DEBUG ((DEBUG_ERROR, "ERROR: SPCR Table found while already published.\n"));
+ Status = EFI_ALREADY_STARTED;
+ } else {
+ SpcrFound = TRUE;
}
return Status;
@@ -500,11 +622,13 @@ ProcessAcpiTables (
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol
)
{
- EFI_STATUS Status;
- EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
- CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
- UINT32 AcpiTableCount;
- UINT32 Idx;
+ EFI_STATUS Status;
+ EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
+ CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
+ UINT32 AcpiTableCount;
+ UINT32 Idx;
+ UINTN Handle;
+ EFI_ACPI_DESCRIPTION_HEADER *DummyHeader;
ASSERT (TableFactoryProtocol != NULL);
ASSERT (CfgMgrProtocol != NULL);
@@ -570,29 +694,37 @@ ProcessAcpiTables (
}
// Add the FADT Table first.
- for (Idx = 0; Idx < AcpiTableCount; Idx++) {
- if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
- AcpiTableInfo[Idx].TableGeneratorId)
- {
- Status = BuildAndInstallAcpiTable (
- TableFactoryProtocol,
- CfgMgrProtocol,
- AcpiTableProtocol,
- &AcpiTableInfo[Idx]
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: Failed to find build and install ACPI FADT Table." \
- " Status = %r\n",
- Status
- ));
- return Status;
+ Status = LocateAcpiTableBySignature (
+ EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ &DummyHeader,
+ &Handle
+ );
+ if (EFI_ERROR (Status)) {
+ // FADT is not yet installed
+ for (Idx = 0; Idx < AcpiTableCount; Idx++) {
+ if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
+ AcpiTableInfo[Idx].TableGeneratorId)
+ {
+ Status = BuildAndInstallAcpiTable (
+ TableFactoryProtocol,
+ CfgMgrProtocol,
+ AcpiTableProtocol,
+ &AcpiTableInfo[Idx]
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: Failed to find build and install ACPI FADT Table." \
+ " Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ break;
}
-
- break;
- }
- } // for
+ } // for
+ }
// Add remaining ACPI Tables
for (Idx = 0; Idx < AcpiTableCount; Idx++) {
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
index 028c3d413cf8..5ca98c8b4895 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
@@ -36,6 +36,7 @@ [LibraryClasses]
[Protocols]
gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+ gEfiAcpiSdtProtocolGuid # PROTOCOL ALWAYS_CONSUMED
gEdkiiConfigurationManagerProtocolGuid # PROTOCOL ALWAYS_CONSUMED
gEdkiiDynamicTableFactoryProtocolGuid # PROTOCOL ALWAYS_CONSUMED
--
2.36.0.windows.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (3 preceding siblings ...)
2022-07-19 0:22 ` [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Kun Qin
@ 2022-07-19 0:22 ` Kun Qin
2022-07-20 13:36 ` [edk2-devel] " PierreGondois
2022-07-19 0:22 ` [PATCH v1 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config Kun Qin
2022-07-20 13:38 ` [edk2-devel] [PATCH v1 0/6] Enhance DynamicTablesPkg modules PierreGondois
6 siblings, 1 reply; 16+ messages in thread
From: Kun Qin @ 2022-07-19 0:22 UTC (permalink / raw)
To: devel; +Cc: Joe Lopez
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
Certain OSes will complain if the ECAM config space is not reserved in
the ACPI namespace.
This change adds a function to reserve PNP motherboard resources for a
given PCI node.
Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 130 ++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index 626e53d70205..d9ed513a2ee3 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -772,6 +772,128 @@ error_handler:
return Status;
}
+/** Generate a Pci Resource Template to hold Address Space Info
+
+ @param [in] Generator The SSDT Pci generator.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol interface.
+ @param [in] PciInfo Pci device information.
+ @param [in, out] PciNode RootNode of the AML tree to populate.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GeneratePciRes (
+ IN ACPI_PCI_GENERATOR *Generator,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST CM_ARM_PCI_CONFIG_SPACE_INFO *PciInfo,
+ IN OUT AML_OBJECT_NODE_HANDLE PciNode
+ )
+{
+ EFI_STATUS Status;
+ UINT32 EisaId;
+ AML_OBJECT_NODE_HANDLE ResNode;
+ AML_OBJECT_NODE_HANDLE CrsNode;
+ BOOLEAN Translation;
+ UINT32 Index;
+ CM_ARM_OBJ_REF *RefInfo;
+ UINT32 RefCount;
+ CM_ARM_PCI_ADDRESS_MAP_INFO *AddrMapInfo;
+
+ // ASL: Device (PCIx) {}
+ Status = AmlCodeGenDevice ("RES0", PciNode, &ResNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // ASL: Name (_HID, EISAID ("PNP0C02"))
+ Status = AmlGetEisaIdFromString ("PNP0C02", &EisaId); /* PNP Motherboard Resources */
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // ASL: Name (_CRS, ResourceTemplate () {})
+ Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, &CrsNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // Get the array of CM_ARM_OBJ_REF referencing the
+ // CM_ARM_PCI_ADDRESS_MAP_INFO objects.
+ Status = GetEArmObjCmRef (
+ CfgMgrProtocol,
+ PciInfo->AddressMapToken,
+ &RefInfo,
+ &RefCount
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ for (Index = 0; Index < RefCount; Index++) {
+ // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one.
+ Status = GetEArmObjPciAddressMapInfo (
+ CfgMgrProtocol,
+ RefInfo[Index].ReferenceToken,
+ &AddrMapInfo,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);
+
+ switch (AddrMapInfo->SpaceCode) {
+ case PCI_SS_CONFIG:
+ Status = AmlCodeGenRdQWordMemory (
+ FALSE,
+ TRUE,
+ TRUE,
+ TRUE,
+ FALSE, // non-cacheable
+ TRUE,
+ 0,
+ AddrMapInfo->PciAddress,
+ AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,
+ Translation ? AddrMapInfo->CpuAddress : 0,
+ AddrMapInfo->AddressSize,
+ 0,
+ NULL,
+ 0,
+ TRUE,
+ CrsNode,
+ NULL
+ );
+ break;
+ default:
+ break;
+ } // switch
+
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+ }
+
+ return Status;
+}
+
/** Generate a Pci device.
@param [in] Generator The SSDT Pci generator.
@@ -855,9 +977,17 @@ GeneratePciDevice (
return Status;
}
+ // Add the PNP Motherboard Resources Device to reserve ECAM space
+ Status = GeneratePciRes (Generator, CfgMgrProtocol, PciInfo, PciNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
// Add the template _OSC method.
Status = AddOscMethod (PciNode);
ASSERT_EFI_ERROR (Status);
+
return Status;
}
--
2.36.0.windows.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (4 preceding siblings ...)
2022-07-19 0:22 ` [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
@ 2022-07-19 0:22 ` Kun Qin
2022-07-20 13:38 ` [edk2-devel] [PATCH v1 0/6] Enhance DynamicTablesPkg modules PierreGondois
6 siblings, 0 replies; 16+ messages in thread
From: Kun Qin @ 2022-07-19 0:22 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
This change added a switch case handling for PCI_SS_CONFIG during SSDT
generation. This will allow PCI config case return EFI_SUCCESS instead of
EFI_INVALID_PARAMETER.
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index d9ed513a2ee3..f3e153ef5309 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -676,6 +676,11 @@ GeneratePciCrs (
);
break;
+ case PCI_SS_CONFIG:
+ // Do nothing
+ Status = EFI_SUCCESS;
+ break;
+
default:
Status = EFI_INVALID_PARAMETER;
} // switch
--
2.36.0.windows.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf
2022-07-19 0:22 ` [PATCH v1 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
@ 2022-07-20 10:03 ` Sami Mujawar
0 siblings, 0 replies; 16+ messages in thread
From: Sami Mujawar @ 2022-07-20 10:03 UTC (permalink / raw)
To: Kun Qin, devel; +Cc: Alexei Fedorov, Joe Lopez, nd
Hi Kun,
Thank you for this patch.
This change looks good to me.
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 19/07/2022 01:22 am, Kun Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
>
> The DynamicPlatRepoLib has multiple reference to MemoryAllocationLib,
> such as DynamicPlatRepo.c and TokenMapper.c. Not including it in the
> library inf file could lead to potential build break.
>
> This change added the MemoryAllocationLib into this inf file.
>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>
> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
> index 9a3cc87fd91d..8423352550c2 100644
> --- a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
> +++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
> @@ -31,3 +31,4 @@ [Packages]
> [LibraryClasses]
>
> AcpiHelperLib
>
> BaseLib
>
> + MemoryAllocationLib
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing
2022-07-19 0:22 ` [PATCH v1 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing Kun Qin
@ 2022-07-20 10:06 ` Sami Mujawar
0 siblings, 0 replies; 16+ messages in thread
From: Sami Mujawar @ 2022-07-20 10:06 UTC (permalink / raw)
To: Kun Qin, devel; +Cc: Alexei Fedorov, Joe Lopez, nd
Hi Kun,
Thank you for this fix.
This patch looks good to me.
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 19/07/2022 01:22 am, Kun Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
>
> The content of token should be derived from the data section of the
> `CmObject` instead of the object itself.
>
> This change fixed the issue by dereferencing the token value from the
> data buffer of input CmObject.
>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>
> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
> index 80d0aa17bc1a..84e4bb7e3bc8 100644
> --- a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
> +++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
> @@ -60,7 +60,7 @@ TokenFixerItsGroup (
> )
>
> {
>
> ASSERT (CmObject != NULL);
>
> - ((CM_ARM_ITS_GROUP_NODE *)CmObject)->Token = Token;
>
> + ((CM_ARM_ITS_GROUP_NODE *)CmObject->Data)->Token = Token;
>
> return EFI_SUCCESS;
>
> }
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers
2022-07-19 0:22 ` [PATCH v1 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers Kun Qin
@ 2022-07-20 10:39 ` Sami Mujawar
0 siblings, 0 replies; 16+ messages in thread
From: Sami Mujawar @ 2022-07-20 10:39 UTC (permalink / raw)
To: Kun Qin, devel; +Cc: Alexei Fedorov, Joe Lopez, nd
Hi Kun,
Thank you for this patch.
These changes look good to me.
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 19/07/2022 01:22 am, Kun Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
>
> This change added more token fixers for other node types, including
> NamedComponentNode, RootComplexNode, and SmmuV3Node.
>
> The corresponding entries for tokenFixer functions table is also updated.
>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>
> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c | 78 +++++++++++++++++++-
> 1 file changed, 75 insertions(+), 3 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
> index 84e4bb7e3bc8..345acab53f74 100644
> --- a/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
> +++ b/DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
> @@ -64,6 +64,78 @@ TokenFixerItsGroup (
> return EFI_SUCCESS;
>
> }
>
>
>
> +/** EArmObjNamedComponent token fixer.
>
> +
>
> + CmObjectToken fixer function that updates the Tokens in the CmObjects.
>
> +
>
> + @param [in] CmObject Pointer to the Configuration Manager Object.
>
> + @param [in] Token Token to be updated in the CmObject.
>
> +
>
> + @retval EFI_SUCCESS Success.
>
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
>
> + @retval EFI_UNSUPPORTED Not supported.
>
> +**/
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +TokenFixerNamedComponentNode (
>
> + IN CM_OBJ_DESCRIPTOR *CmObject,
>
> + IN CM_OBJECT_TOKEN Token
>
> + )
>
> +{
>
> + ASSERT (CmObject != NULL);
>
> + ((CM_ARM_NAMED_COMPONENT_NODE *)CmObject->Data)->Token = Token;
>
> + return EFI_SUCCESS;
>
> +}
>
> +
>
> +/** EArmObjRootComplex token fixer.
>
> +
>
> + CmObjectToken fixer function that updates the Tokens in the CmObjects.
>
> +
>
> + @param [in] CmObject Pointer to the Configuration Manager Object.
>
> + @param [in] Token Token to be updated in the CmObject.
>
> +
>
> + @retval EFI_SUCCESS Success.
>
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
>
> + @retval EFI_UNSUPPORTED Not supported.
>
> +**/
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +TokenFixerRootComplexNode (
>
> + IN CM_OBJ_DESCRIPTOR *CmObject,
>
> + IN CM_OBJECT_TOKEN Token
>
> + )
>
> +{
>
> + ASSERT (CmObject != NULL);
>
> + ((CM_ARM_ROOT_COMPLEX_NODE *)CmObject->Data)->Token = Token;
>
> + return EFI_SUCCESS;
>
> +}
>
> +
>
> +/** EArmObjSmmuV3 token fixer.
>
> +
>
> + CmObjectToken fixer function that updates the Tokens in the CmObjects.
>
> +
>
> + @param [in] CmObject Pointer to the Configuration Manager Object.
>
> + @param [in] Token Token to be updated in the CmObject.
>
> +
>
> + @retval EFI_SUCCESS Success.
>
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
>
> + @retval EFI_UNSUPPORTED Not supported.
>
> +**/
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +TokenFixerSmmuV3Node (
>
> + IN CM_OBJ_DESCRIPTOR *CmObject,
>
> + IN CM_OBJECT_TOKEN Token
>
> + )
>
> +{
>
> + ASSERT (CmObject != NULL);
>
> + ((CM_ARM_SMMUV3_NODE *)CmObject->Data)->Token = Token;
>
> + return EFI_SUCCESS;
>
> +}
>
> +
>
> /** TokenFixer functions table.
>
>
>
> A CmObj having a CM_OBJECT_TOKEN field might need to have its
>
> @@ -90,10 +162,10 @@ CM_OBJECT_TOKEN_FIXER TokenFixer[EArmObjMax] = {
> NULL, ///< 16 - Hypervisor Vendor Id
>
> NULL, ///< 17 - Fixed feature flags for FADT
>
> TokenFixerItsGroup, ///< 18 - ITS Group
>
> - TokenFixerNotImplemented, ///< 19 - Named Component
>
> - TokenFixerNotImplemented, ///< 20 - Root Complex
>
> + TokenFixerNamedComponentNode, ///< 19 - Named Component
>
> + TokenFixerRootComplexNode, ///< 20 - Root Complex
>
> TokenFixerNotImplemented, ///< 21 - SMMUv1 or SMMUv2
>
> - TokenFixerNotImplemented, ///< 22 - SMMUv3
>
> + TokenFixerSmmuV3Node, ///< 22 - SMMUv3
>
> TokenFixerNotImplemented, ///< 23 - PMCG
>
> NULL, ///< 24 - GIC ITS Identifier Array
>
> NULL, ///< 25 - ID Mapping Array
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
2022-07-19 0:22 ` [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Kun Qin
@ 2022-07-20 11:00 ` Sami Mujawar
2022-07-21 1:48 ` Kun Qin
2022-07-20 13:36 ` [edk2-devel] " PierreGondois
1 sibling, 1 reply; 16+ messages in thread
From: Sami Mujawar @ 2022-07-20 11:00 UTC (permalink / raw)
To: Kun Qin, devel; +Cc: Alexei Fedorov, Joe Lopez, nd
Hi Kun,
Thank you for this patch.
I have some minor suggestions marked inline as [SAMI], otherwise this
patch looks good to me.
With that updated.
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 19/07/2022 01:22 am, Kun Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
>
> This change added an extra step to allow check for installed ACPI tables.
>
> For FADT, MADT, GTDT, DSDT and DBG2 tables, either pre-installed or
> supplied through AcpiTableInfo can be accepted.
>
> An extra check for FADT ACPI table existence during installation step is
> also added.
>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>
> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 200 ++++++++++++++++----
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 1 +
> 2 files changed, 167 insertions(+), 34 deletions(-)
>
> diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
> index ed62299f9bbd..ac5fe0bed91b 100644
> --- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
> +++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
> @@ -11,6 +11,7 @@
> #include <Library/PcdLib.h>
>
> #include <Library/UefiBootServicesTableLib.h>
>
> #include <Protocol/AcpiTable.h>
>
> +#include <Protocol/AcpiSystemDescriptionTable.h>
[SAMI] Can thie include statement above be alphabetically ordered, please?
>
>
>
> // Module specific include files.
>
> #include <AcpiTableGenerator.h>
>
> @@ -387,6 +388,57 @@ BuildAndInstallAcpiTable (
> return Status;
>
> }
>
>
>
> +/**
>
> + This function uses the ACPI SDT protocol to locate an ACPI table.
>
> + It is really only useful for finding tables that only have a single instance,
>
> + e.g. FADT, FACS, MADT, etc. It is not good for locating SSDT, etc.
>
> +
>
> + @param[in] Signature - Pointer to an ASCII string containing the OEM Table ID from the ACPI table header
>
> + @param[in, out] Table - Updated with a pointer to the table
>
> + @param[in, out] Handle - AcpiSupport protocol table handle for the table found
>
> +
>
> + @retval EFI_SUCCESS - The function completed successfully.
[SAMI] Please add EFI_NOT_FOUND as a return type if an ACPI table with
the requested signature is not found or if the ACPI SDT protocol is not
installed.
>
> +**/
>
> +STATIC
>
> +EFI_STATUS
>
> +LocateAcpiTableBySignature (
>
> + IN UINT32 Signature,
>
> + IN OUT EFI_ACPI_DESCRIPTION_HEADER **Table,
>
> + IN OUT UINTN *Handle
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + INTN Index;
>
> + EFI_ACPI_TABLE_VERSION Version;
>
> + EFI_ACPI_SDT_PROTOCOL *AcpiSdt;
>
> +
>
> + AcpiSdt = NULL;
>
> + Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL, (VOID **)&AcpiSdt);
>
> +
>
> + if (EFI_ERROR (Status) || (AcpiSdt == NULL)) {
>
> + return EFI_NOT_FOUND;
>
> + }
>
> +
>
> + //
>
> + // Locate table with matching ID
>
> + //
>
> + Version = 0;
>
> + Index = 0;
>
> + do {
>
> + Status = AcpiSdt->GetAcpiTable (Index, (EFI_ACPI_SDT_HEADER **)Table, &Version, Handle);
>
> + if (EFI_ERROR (Status)) {
>
> + break;
>
> + }
>
> +
>
> + Index++;
>
> + } while ((*Table)->Signature != Signature);
>
> +
>
> + //
>
> + // If we found the table, there will be no error.
>
> + //
>
> + return Status;
>
> +}
>
> +
>
> /** The function checks if the Configuration Manager has provided the
>
> mandatory ACPI tables for installation.
>
>
>
> @@ -411,6 +463,9 @@ VerifyMandatoryTablesArePresent (
> BOOLEAN DsdtFound;
>
> BOOLEAN Dbg2Found;
>
> BOOLEAN SpcrFound;
>
> + UINTN Handle;
>
> +
>
> + EFI_ACPI_DESCRIPTION_HEADER *DummyHeader;
>
>
>
> Status = EFI_SUCCESS;
>
> FadtFound = FALSE;
>
> @@ -447,32 +502,99 @@ VerifyMandatoryTablesArePresent (
> }
>
>
>
> // We need at least the FADT, MADT, GTDT and the DSDT tables to boot
>
> - if (!FadtFound) {
>
> - DEBUG ((DEBUG_ERROR, "ERROR: FADT Table not found\n"));
>
> + // But they also might be published already, so we can search from there
>
> + Handle = 0;
>
> + Status = LocateAcpiTableBySignature (
>
> + EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>
> + &DummyHeader,
>
> + &Handle
>
> + );
>
> + if (EFI_ERROR (Status) && !FadtFound) {
>
> + DEBUG ((DEBUG_ERROR, "ERROR: FADT Table not found.\n"));
>
> Status = EFI_NOT_FOUND;
>
> + } else if (!EFI_ERROR (Status) && FadtFound) {
>
> + DEBUG ((DEBUG_ERROR, "ERROR: FADT Table found while already published.\n"));
>
> + Status = EFI_ALREADY_STARTED;
[SAMI] Please update the function documentation header to reflect the
EFI_ALREADY_STARTED error code.
>
> + } else {
>
> + FadtFound = TRUE;
>
> }
>
>
>
> - if (!MadtFound) {
>
> + Handle = 0;
>
> + Status = LocateAcpiTableBySignature (
>
> + EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
>
> + &DummyHeader,
>
> + &Handle
>
> + );
>
> + if (EFI_ERROR (Status) && !MadtFound) {
>
> DEBUG ((DEBUG_ERROR, "ERROR: MADT Table not found.\n"));
>
> Status = EFI_NOT_FOUND;
>
> + } else if (!EFI_ERROR (Status) && MadtFound) {
>
> + DEBUG ((DEBUG_ERROR, "ERROR: MADT Table found while already published.\n"));
>
> + Status = EFI_ALREADY_STARTED;
>
> + } else {
>
> + MadtFound = TRUE;
>
> }
>
>
>
> - if (!GtdtFound) {
>
> + Handle = 0;
>
> + Status = LocateAcpiTableBySignature (
>
> + EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
>
> + &DummyHeader,
>
> + &Handle
>
> + );
>
> + if (EFI_ERROR (Status) && !GtdtFound) {
>
> DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table not found.\n"));
>
> Status = EFI_NOT_FOUND;
>
> + } else if (!EFI_ERROR (Status) && GtdtFound) {
>
> + DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table found while already published.\n"));
>
> + Status = EFI_ALREADY_STARTED;
>
> + } else {
>
> + GtdtFound = TRUE;
>
> }
>
>
>
> - if (!DsdtFound) {
>
> + Handle = 0;
>
> + Status = LocateAcpiTableBySignature (
>
> + EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
>
> + &DummyHeader,
>
> + &Handle
>
> + );
>
> + if (EFI_ERROR (Status) && !DsdtFound) {
>
> DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table not found.\n"));
>
> Status = EFI_NOT_FOUND;
>
> + } else if (!EFI_ERROR (Status) && DsdtFound) {
>
> + DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table found while already published.\n"));
>
> + Status = EFI_ALREADY_STARTED;
>
> + } else {
>
> + DsdtFound = TRUE;
>
> }
>
>
>
> - if (!Dbg2Found) {
>
> + Handle = 0;
>
> + Status = LocateAcpiTableBySignature (
>
> + EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
>
> + &DummyHeader,
>
> + &Handle
>
> + );
>
> + if (EFI_ERROR (Status) && !Dbg2Found) {
>
> DEBUG ((DEBUG_WARN, "WARNING: DBG2 Table not found.\n"));
>
> + } else if (!EFI_ERROR (Status) && Dbg2Found) {
>
> + DEBUG ((DEBUG_ERROR, "ERROR: DBG2 Table found while already published.\n"));
>
> + Status = EFI_ALREADY_STARTED;
>
> + } else {
>
> + Dbg2Found = TRUE;
>
> }
>
>
>
> - if (!SpcrFound) {
>
> + Handle = 0;
>
> + Status = LocateAcpiTableBySignature (
>
> + EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
>
> + &DummyHeader,
>
> + &Handle
>
> + );
>
> + if (EFI_ERROR (Status) && !SpcrFound) {
>
> DEBUG ((DEBUG_WARN, "WARNING: SPCR Table not found.\n"));
>
> + } else if (!EFI_ERROR (Status) && SpcrFound) {
>
> + DEBUG ((DEBUG_ERROR, "ERROR: SPCR Table found while already published.\n"));
>
> + Status = EFI_ALREADY_STARTED;
>
> + } else {
>
> + SpcrFound = TRUE;
>
> }
>
>
>
> return Status;
>
> @@ -500,11 +622,13 @@ ProcessAcpiTables (
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol
>
> )
>
> {
>
> - EFI_STATUS Status;
>
> - EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
>
> - CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
>
> - UINT32 AcpiTableCount;
>
> - UINT32 Idx;
>
> + EFI_STATUS Status;
>
> + EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
>
> + CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
>
> + UINT32 AcpiTableCount;
>
> + UINT32 Idx;
>
> + UINTN Handle;
>
> + EFI_ACPI_DESCRIPTION_HEADER *DummyHeader;
>
>
>
> ASSERT (TableFactoryProtocol != NULL);
>
> ASSERT (CfgMgrProtocol != NULL);
>
> @@ -570,29 +694,37 @@ ProcessAcpiTables (
> }
> [snip]
// Check if mandatory ACPI tables are present.
Status = VerifyMandatoryTablesArePresent (
AcpiTableInfo,
AcpiTableCount
);
if (EFI_ERROR (Status)) {
DEBUG ((
DEBUG_ERROR,
"ERROR: Failed to find mandatory ACPI Table(s)."
" Status = %r\n",
Status
));
[SAMI] Is it possible to update the error reporting to reflect the
EFI_ALREADY_STARTED error type, please? Please also update the function
documentation header for ProcessAcpiTables().
return Status;
}
[/snip]
>
>
> // Add the FADT Table first.
>
> - for (Idx = 0; Idx < AcpiTableCount; Idx++) {
>
> - if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
>
> - AcpiTableInfo[Idx].TableGeneratorId)
>
> - {
>
> - Status = BuildAndInstallAcpiTable (
>
> - TableFactoryProtocol,
>
> - CfgMgrProtocol,
>
> - AcpiTableProtocol,
>
> - &AcpiTableInfo[Idx]
>
> - );
>
> - if (EFI_ERROR (Status)) {
>
> - DEBUG ((
>
> - DEBUG_ERROR,
>
> - "ERROR: Failed to find build and install ACPI FADT Table." \
>
> - " Status = %r\n",
>
> - Status
>
> - ));
>
> - return Status;
>
> + Status = LocateAcpiTableBySignature (
>
> + EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>
> + &DummyHeader,
>
> + &Handle
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> + // FADT is not yet installed
>
> + for (Idx = 0; Idx < AcpiTableCount; Idx++) {
>
> + if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
>
> + AcpiTableInfo[Idx].TableGeneratorId)
>
> + {
>
> + Status = BuildAndInstallAcpiTable (
>
> + TableFactoryProtocol,
>
> + CfgMgrProtocol,
>
> + AcpiTableProtocol,
>
> + &AcpiTableInfo[Idx]
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> + DEBUG ((
>
> + DEBUG_ERROR,
>
> + "ERROR: Failed to find build and install ACPI FADT Table." \
>
> + " Status = %r\n",
>
> + Status
>
> + ));
>
> + return Status;
>
> + }
>
> +
>
> + break;
>
> }
>
> -
>
> - break;
>
> - }
>
> - } // for
>
> + } // for
>
> + }
>
>
>
> // Add remaining ACPI Tables
>
> for (Idx = 0; Idx < AcpiTableCount; Idx++) {
>
> diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
> index 028c3d413cf8..5ca98c8b4895 100644
> --- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
> +++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
> @@ -36,6 +36,7 @@ [LibraryClasses]
>
>
> [Protocols]
>
> gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>
> + gEfiAcpiSdtProtocolGuid # PROTOCOL ALWAYS_CONSUMED
[SAMI] Should the DEPEX section be updated to relect the dependency on
the SDT protocol?
>
>
>
> gEdkiiConfigurationManagerProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>
> gEdkiiDynamicTableFactoryProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-devel] [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space
2022-07-19 0:22 ` [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
@ 2022-07-20 13:36 ` PierreGondois
0 siblings, 0 replies; 16+ messages in thread
From: PierreGondois @ 2022-07-20 13:36 UTC (permalink / raw)
To: devel, kuqin12, Sami Mujawar; +Cc: Joe Lopez
Hello Kun,
On 7/19/22 02:22, Kun Qin via groups.io wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
>
> Certain OSes will complain if the ECAM config space is not reserved in
> the ACPI namespace.
>
> This change adds a function to reserve PNP motherboard resources for a
> given PCI node.
>
> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 130 ++++++++++++++++++++
> 1 file changed, 130 insertions(+)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> index 626e53d70205..d9ed513a2ee3 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> @@ -772,6 +772,128 @@ error_handler:
> return Status;
> }
>
> +/** Generate a Pci Resource Template to hold Address Space Info
> +
> + @param [in] Generator The SSDT Pci generator.
> + @param [in] CfgMgrProtocol Pointer to the Configuration Manager
> + Protocol interface.
> + @param [in] PciInfo Pci device information.
> + @param [in, out] PciNode RootNode of the AML tree to populate.
> +
> + @retval EFI_SUCCESS The function completed successfully.
> + @retval EFI_INVALID_PARAMETER Invalid parameter.
> + @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GeneratePciRes (
> + IN ACPI_PCI_GENERATOR *Generator,
> + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
> + IN CONST CM_ARM_PCI_CONFIG_SPACE_INFO *PciInfo,
> + IN OUT AML_OBJECT_NODE_HANDLE PciNode
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 EisaId;
> + AML_OBJECT_NODE_HANDLE ResNode;
> + AML_OBJECT_NODE_HANDLE CrsNode;
> + BOOLEAN Translation;
> + UINT32 Index;
> + CM_ARM_OBJ_REF *RefInfo;
> + UINT32 RefCount;
> + CM_ARM_PCI_ADDRESS_MAP_INFO *AddrMapInfo;
> +
> + // ASL: Device (PCIx) {}
> + Status = AmlCodeGenDevice ("RES0", PciNode, &ResNode);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + // ASL: Name (_HID, EISAID ("PNP0C02"))
> + Status = AmlGetEisaIdFromString ("PNP0C02", &EisaId); /* PNP Motherboard Resources */
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + // ASL: Name (_CRS, ResourceTemplate () {})
> + Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, &CrsNode);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + // Get the array of CM_ARM_OBJ_REF referencing the
> + // CM_ARM_PCI_ADDRESS_MAP_INFO objects.
> + Status = GetEArmObjCmRef (
> + CfgMgrProtocol,
> + PciInfo->AddressMapToken,
> + &RefInfo,
> + &RefCount
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + for (Index = 0; Index < RefCount; Index++) {
> + // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one.
> + Status = GetEArmObjPciAddressMapInfo (
> + CfgMgrProtocol,
> + RefInfo[Index].ReferenceToken,
> + &AddrMapInfo,
> + NULL
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
> + Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);
> +
> + switch (AddrMapInfo->SpaceCode) {
> + case PCI_SS_CONFIG:
> + Status = AmlCodeGenRdQWordMemory (
> + FALSE,
> + TRUE,
> + TRUE,
> + TRUE,
> + FALSE, // non-cacheable
> + TRUE,
> + 0,
> + AddrMapInfo->PciAddress,
> + AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,
> + Translation ? AddrMapInfo->CpuAddress : 0,
> + AddrMapInfo->AddressSize,
> + 0,
> + NULL,
> + 0,
> + TRUE,
> + CrsNode,
> + NULL
> + );
> + break;
> + default:
> + break;
> + } // switch
> +
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> + }
> +
> + return Status;
> +}
> +
> /** Generate a Pci device.
>
> @param [in] Generator The SSDT Pci generator.
> @@ -855,9 +977,17 @@ GeneratePciDevice (
> return Status;
> }
>
> + // Add the PNP Motherboard Resources Device to reserve ECAM space
> + Status = GeneratePciRes (Generator, CfgMgrProtocol, PciInfo, PciNode);
> + if (EFI_ERROR (Status)) {
> + ASSERT (0);
> + return Status;
> + }
> +
(Just a remark for Sami)
It seems the RES0 device will be generated under each PCI device:
\_SB.PCIX.RES0
So there would be multiple devices with the PNP0C02 Eisaid.
PCI Firmware 3.2, sec 4.1.2:
If the operating system does not natively comprehend reserving the
MMCFG region, the MMCFG region must be reserved by firmware. The
address range reported in the MCFG table or by _CBA method (see Section
4.1.3) must be reserved by declaring a motherboard resource. For most
systems, the motherboard resource would appear at the root of the ACPI
namespace (under \_SB) in a node with a _HID of EISAID (PNP0C02), and
the resources in this case should not be claimed in the root PCI bus’s
_CRS. The resources can optionally be returned in Int15 E820 or
EFIGetMemoryMap as reserved memory but must always be reported through
ACPI as a motherboard resource.
There are many examples devices describing the configuration space at other places
than under \_SB, so it should be ok to place it here.
----
Also it seems that a RES0 device will be generated even when no Address Space Info
is available. I think it should be checked that there is a configuration space
to describe first.
Regards,
Pierre
> // Add the template _OSC method.
> Status = AddOscMethod (PciNode);
> ASSERT_EFI_ERROR (Status);
> +
> return Status;
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-devel] [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
2022-07-19 0:22 ` [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Kun Qin
2022-07-20 11:00 ` Sami Mujawar
@ 2022-07-20 13:36 ` PierreGondois
1 sibling, 0 replies; 16+ messages in thread
From: PierreGondois @ 2022-07-20 13:36 UTC (permalink / raw)
To: devel, kuqin12; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
Hello Kun,
On 7/19/22 02:22, Kun Qin via groups.io wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
>
> This change added an extra step to allow check for installed ACPI tables.
>
> For FADT, MADT, GTDT, DSDT and DBG2 tables, either pre-installed or
> supplied through AcpiTableInfo can be accepted.
>
> An extra check for FADT ACPI table existence during installation step is
> also added.
>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>
> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 200 ++++++++++++++++----
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 1 +
> 2 files changed, 167 insertions(+), 34 deletions(-)
>
> diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
> index ed62299f9bbd..ac5fe0bed91b 100644
> --- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
> +++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
> @@ -11,6 +11,7 @@
> #include <Library/PcdLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> #include <Protocol/AcpiTable.h>
> +#include <Protocol/AcpiSystemDescriptionTable.h>
>
> // Module specific include files.
> #include <AcpiTableGenerator.h>
> @@ -387,6 +388,57 @@ BuildAndInstallAcpiTable (
> return Status;
> }
>
[snip]
> - if (!Dbg2Found) {
> + Handle = 0;
> + Status = LocateAcpiTableBySignature (
> + EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
> + &DummyHeader,
> + &Handle
> + );
> + if (EFI_ERROR (Status) && !Dbg2Found) {
> DEBUG ((DEBUG_WARN, "WARNING: DBG2 Table not found.\n"));
> + } else if (!EFI_ERROR (Status) && Dbg2Found) {
> + DEBUG ((DEBUG_ERROR, "ERROR: DBG2 Table found while already published.\n"));
> + Status = EFI_ALREADY_STARTED;
> + } else {
> + Dbg2Found = TRUE;
> }
>
> - if (!SpcrFound) {
> + Handle = 0;
> + Status = LocateAcpiTableBySignature (
> + EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> + &DummyHeader,
> + &Handle
> + );
> + if (EFI_ERROR (Status) && !SpcrFound) {
> DEBUG ((DEBUG_WARN, "WARNING: SPCR Table not found.\n"));
> + } else if (!EFI_ERROR (Status) && SpcrFound) {
> + DEBUG ((DEBUG_ERROR, "ERROR: SPCR Table found while already published.\n"));
> + Status = EFI_ALREADY_STARTED;
> + } else {
> + SpcrFound = TRUE;
> }
Since there are many tables, maybe it would be good to factorize the code and
create a static array containing all the tables that are mandatory, and containing:
1. the ESTD_ACPI_TABLE_ID of the table
2. the table signature (*_DESCRIPTION_TABLE_SIGNATURE)
3. the table name (for printing)
4. whether the table was found (dynamically populated)
(maybe other fields would be required)
Like this we could have 2 loops in VerifyMandatoryTablesArePresent(),
one going through AcpiTableInfo[AcpiTableCount].AcpiTableSignature,
and a second one going through the already installed tables (AcpiSdt->GetAcpiTable (Index, ...))
This should also allow to simplify the code for installing the FADT aswell
and code of LocateAcpiTableBySignature() would be included in VerifyMandatoryTablesArePresent().
Also, I think you will have to rebase your patches on the latest master and
do the same thing as
6cda306da1dd DynamicTablesPkg: AcpiSsdtPcieLibArm: Correct translation value
does.
Regards,
Pierre
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-devel] [PATCH v1 0/6] Enhance DynamicTablesPkg modules
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (5 preceding siblings ...)
2022-07-19 0:22 ` [PATCH v1 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config Kun Qin
@ 2022-07-20 13:38 ` PierreGondois
2022-07-21 1:45 ` Kun Qin
6 siblings, 1 reply; 16+ messages in thread
From: PierreGondois @ 2022-07-20 13:38 UTC (permalink / raw)
To: devel, kuqin12; +Cc: Sami Mujawar, Alexei Fedorov
Except for:
[PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
[PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space
where I had some remarks,
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
Thanks!
On 7/19/22 02:22, Kun Qin via groups.io wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
>
> Current DynamicTablesPkg provide great support for creating dynamic ACPI
> tables during boot time.
>
> However, there are some modules needs minor tweaks to expand support and
> compatibility for OS requirements and platform needs.
>
> This patch series proposes a few fixes to resolve minor issues discovered
> in DynamicPlatRepoLib, AcpiSsdtPcieLibArm and DynamicTableManagerDxe.
>
> Patch v1 branch: https://github.com/kuqin12/edk2/tree/dynamic_update
>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>
> Kun Qin (6):
> DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf
> DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing
> DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers
> DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed
> tables
> DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM
> space
> DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI
> config
>
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 200 ++++++++++++++++----
> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 135 +++++++++++++
> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c | 80 +++++++-
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 1 +
> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf | 1 +
> 5 files changed, 379 insertions(+), 38 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-devel] [PATCH v1 0/6] Enhance DynamicTablesPkg modules
2022-07-20 13:38 ` [edk2-devel] [PATCH v1 0/6] Enhance DynamicTablesPkg modules PierreGondois
@ 2022-07-21 1:45 ` Kun Qin
0 siblings, 0 replies; 16+ messages in thread
From: Kun Qin @ 2022-07-21 1:45 UTC (permalink / raw)
To: devel, pierre.gondois; +Cc: Sami Mujawar, Alexei Fedorov
Thank you for the review, Pierre! We will update the patches per your
feedback and send v2 after validation.
Regards,
Kun
On 7/20/2022 6:38 AM, PierreGondois wrote:
> Except for:
> [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check
> for installed tables
> [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to
> reserve ECAM space
> where I had some remarks,
>
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
>
> Thanks!
>
> On 7/19/22 02:22, Kun Qin via groups.io wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3996
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
>>
>> Current DynamicTablesPkg provide great support for creating dynamic ACPI
>> tables during boot time.
>>
>> However, there are some modules needs minor tweaks to expand support and
>> compatibility for OS requirements and platform needs.
>>
>> This patch series proposes a few fixes to resolve minor issues
>> discovered
>> in DynamicPlatRepoLib, AcpiSsdtPcieLibArm and DynamicTableManagerDxe.
>>
>> Patch v1 branch: https://github.com/kuqin12/edk2/tree/dynamic_update
>>
>> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
>> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>>
>> Kun Qin (6):
>> DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to
>> inf
>> DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing
>> DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers
>> DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed
>> tables
>> DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM
>> space
>> DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI
>> config
>>
>> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
>> | 200 ++++++++++++++++----
>> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>> | 135 +++++++++++++
>> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
>> | 80 +++++++-
>> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
>> | 1 +
>> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
>> | 1 +
>> 5 files changed, 379 insertions(+), 38 deletions(-)
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
2022-07-20 11:00 ` Sami Mujawar
@ 2022-07-21 1:48 ` Kun Qin
0 siblings, 0 replies; 16+ messages in thread
From: Kun Qin @ 2022-07-21 1:48 UTC (permalink / raw)
To: Sami Mujawar, devel; +Cc: Alexei Fedorov, Joe Lopez, nd
Thank you for your reviews, Sami! I will incorporate your suggestions in
v2 patch and send for review after validation.
Regards,
Kun
On 7/20/2022 4:00 AM, Sami Mujawar wrote:
> Hi Kun,
>
> Thank you for this patch.
>
> I have some minor suggestions marked inline as [SAMI], otherwise this
> patch looks good to me.
>
> With that updated.
>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>
> Regards,
>
> Sami Mujawar
>
> On 19/07/2022 01:22 am, Kun Qin wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
>>
>> This change added an extra step to allow check for installed ACPI
>> tables.
>>
>> For FADT, MADT, GTDT, DSDT and DBG2 tables, either pre-installed or
>> supplied through AcpiTableInfo can be accepted.
>>
>> An extra check for FADT ACPI table existence during installation step is
>> also added.
>>
>> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
>> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>>
>> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
>> Signed-off-by: Kun Qin <kuqin12@gmail.com>
>> ---
>> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
>> | 200 ++++++++++++++++----
>> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
>> | 1 +
>> 2 files changed, 167 insertions(+), 34 deletions(-)
>>
>> diff --git
>> a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
>> b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
>>
>> index ed62299f9bbd..ac5fe0bed91b 100644
>> ---
>> a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
>> +++
>> b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
>> @@ -11,6 +11,7 @@
>> #include <Library/PcdLib.h>
>>
>> #include <Library/UefiBootServicesTableLib.h>
>>
>> #include <Protocol/AcpiTable.h>
>>
>> +#include <Protocol/AcpiSystemDescriptionTable.h>
> [SAMI] Can thie include statement above be alphabetically ordered,
> please?
>>
>>
>> // Module specific include files.
>>
>> #include <AcpiTableGenerator.h>
>>
>> @@ -387,6 +388,57 @@ BuildAndInstallAcpiTable (
>> return Status;
>>
>> }
>>
>>
>> +/**
>>
>> + This function uses the ACPI SDT protocol to locate an ACPI table.
>>
>> + It is really only useful for finding tables that only have a
>> single instance,
>>
>> + e.g. FADT, FACS, MADT, etc. It is not good for locating SSDT, etc.
>>
>> +
>>
>> + @param[in] Signature - Pointer to an ASCII string
>> containing the OEM Table ID from the ACPI table header
>>
>> + @param[in, out] Table - Updated with a pointer to the table
>>
>> + @param[in, out] Handle - AcpiSupport protocol table handle
>> for the table found
>>
>> +
>>
>> + @retval EFI_SUCCESS - The function completed successfully.
>
> [SAMI] Please add EFI_NOT_FOUND as a return type if an ACPI table with
> the requested signature is not found or if the ACPI SDT protocol is
> not installed.
>
>>
>> +**/
>>
>> +STATIC
>>
>> +EFI_STATUS
>>
>> +LocateAcpiTableBySignature (
>>
>> + IN UINT32 Signature,
>>
>> + IN OUT EFI_ACPI_DESCRIPTION_HEADER **Table,
>>
>> + IN OUT UINTN *Handle
>>
>> + )
>>
>> +{
>>
>> + EFI_STATUS Status;
>>
>> + INTN Index;
>>
>> + EFI_ACPI_TABLE_VERSION Version;
>>
>> + EFI_ACPI_SDT_PROTOCOL *AcpiSdt;
>>
>> +
>>
>> + AcpiSdt = NULL;
>>
>> + Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL,
>> (VOID **)&AcpiSdt);
>>
>> +
>>
>> + if (EFI_ERROR (Status) || (AcpiSdt == NULL)) {
>>
>> + return EFI_NOT_FOUND;
>>
>> + }
>>
>> +
>>
>> + //
>>
>> + // Locate table with matching ID
>>
>> + //
>>
>> + Version = 0;
>>
>> + Index = 0;
>>
>> + do {
>>
>> + Status = AcpiSdt->GetAcpiTable (Index, (EFI_ACPI_SDT_HEADER
>> **)Table, &Version, Handle);
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + break;
>>
>> + }
>>
>> +
>>
>> + Index++;
>>
>> + } while ((*Table)->Signature != Signature);
>>
>> +
>>
>> + //
>>
>> + // If we found the table, there will be no error.
>>
>> + //
>>
>> + return Status;
>>
>> +}
>>
>> +
>>
>> /** The function checks if the Configuration Manager has provided the
>>
>> mandatory ACPI tables for installation.
>>
>>
>> @@ -411,6 +463,9 @@ VerifyMandatoryTablesArePresent (
>> BOOLEAN DsdtFound;
>>
>> BOOLEAN Dbg2Found;
>>
>> BOOLEAN SpcrFound;
>>
>> + UINTN Handle;
>>
>> +
>>
>> + EFI_ACPI_DESCRIPTION_HEADER *DummyHeader;
>>
>>
>> Status = EFI_SUCCESS;
>>
>> FadtFound = FALSE;
>>
>> @@ -447,32 +502,99 @@ VerifyMandatoryTablesArePresent (
>> }
>>
>>
>> // We need at least the FADT, MADT, GTDT and the DSDT tables to boot
>>
>> - if (!FadtFound) {
>>
>> - DEBUG ((DEBUG_ERROR, "ERROR: FADT Table not found\n"));
>>
>> + // But they also might be published already, so we can search from
>> there
>>
>> + Handle = 0;
>>
>> + Status = LocateAcpiTableBySignature (
>>
>> + EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>>
>> + &DummyHeader,
>>
>> + &Handle
>>
>> + );
>>
>> + if (EFI_ERROR (Status) && !FadtFound) {
>>
>> + DEBUG ((DEBUG_ERROR, "ERROR: FADT Table not found.\n"));
>>
>> Status = EFI_NOT_FOUND;
>>
>> + } else if (!EFI_ERROR (Status) && FadtFound) {
>>
>> + DEBUG ((DEBUG_ERROR, "ERROR: FADT Table found while already
>> published.\n"));
>>
>> + Status = EFI_ALREADY_STARTED;
> [SAMI] Please update the function documentation header to reflect the
> EFI_ALREADY_STARTED error code.
>>
>> + } else {
>>
>> + FadtFound = TRUE;
>>
>> }
>>
>>
>> - if (!MadtFound) {
>>
>> + Handle = 0;
>>
>> + Status = LocateAcpiTableBySignature (
>>
>> + EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
>>
>> + &DummyHeader,
>>
>> + &Handle
>>
>> + );
>>
>> + if (EFI_ERROR (Status) && !MadtFound) {
>>
>> DEBUG ((DEBUG_ERROR, "ERROR: MADT Table not found.\n"));
>>
>> Status = EFI_NOT_FOUND;
>>
>> + } else if (!EFI_ERROR (Status) && MadtFound) {
>>
>> + DEBUG ((DEBUG_ERROR, "ERROR: MADT Table found while already
>> published.\n"));
>>
>> + Status = EFI_ALREADY_STARTED;
>>
>> + } else {
>>
>> + MadtFound = TRUE;
>>
>> }
>>
>>
>> - if (!GtdtFound) {
>>
>> + Handle = 0;
>>
>> + Status = LocateAcpiTableBySignature (
>>
>> + EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
>>
>> + &DummyHeader,
>>
>> + &Handle
>>
>> + );
>>
>> + if (EFI_ERROR (Status) && !GtdtFound) {
>>
>> DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table not found.\n"));
>>
>> Status = EFI_NOT_FOUND;
>>
>> + } else if (!EFI_ERROR (Status) && GtdtFound) {
>>
>> + DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table found while already
>> published.\n"));
>>
>> + Status = EFI_ALREADY_STARTED;
>>
>> + } else {
>>
>> + GtdtFound = TRUE;
>>
>> }
>>
>>
>> - if (!DsdtFound) {
>>
>> + Handle = 0;
>>
>> + Status = LocateAcpiTableBySignature (
>>
>> + EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
>>
>> + &DummyHeader,
>>
>> + &Handle
>>
>> + );
>>
>> + if (EFI_ERROR (Status) && !DsdtFound) {
>>
>> DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table not found.\n"));
>>
>> Status = EFI_NOT_FOUND;
>>
>> + } else if (!EFI_ERROR (Status) && DsdtFound) {
>>
>> + DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table found while already
>> published.\n"));
>>
>> + Status = EFI_ALREADY_STARTED;
>>
>> + } else {
>>
>> + DsdtFound = TRUE;
>>
>> }
>>
>>
>> - if (!Dbg2Found) {
>>
>> + Handle = 0;
>>
>> + Status = LocateAcpiTableBySignature (
>>
>> + EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
>>
>> + &DummyHeader,
>>
>> + &Handle
>>
>> + );
>>
>> + if (EFI_ERROR (Status) && !Dbg2Found) {
>>
>> DEBUG ((DEBUG_WARN, "WARNING: DBG2 Table not found.\n"));
>>
>> + } else if (!EFI_ERROR (Status) && Dbg2Found) {
>>
>> + DEBUG ((DEBUG_ERROR, "ERROR: DBG2 Table found while already
>> published.\n"));
>>
>> + Status = EFI_ALREADY_STARTED;
>>
>> + } else {
>>
>> + Dbg2Found = TRUE;
>>
>> }
>>
>>
>> - if (!SpcrFound) {
>>
>> + Handle = 0;
>>
>> + Status = LocateAcpiTableBySignature (
>>
>> + EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
>>
>> + &DummyHeader,
>>
>> + &Handle
>>
>> + );
>>
>> + if (EFI_ERROR (Status) && !SpcrFound) {
>>
>> DEBUG ((DEBUG_WARN, "WARNING: SPCR Table not found.\n"));
>>
>> + } else if (!EFI_ERROR (Status) && SpcrFound) {
>>
>> + DEBUG ((DEBUG_ERROR, "ERROR: SPCR Table found while already
>> published.\n"));
>>
>> + Status = EFI_ALREADY_STARTED;
>>
>> + } else {
>>
>> + SpcrFound = TRUE;
>>
>> }
>>
>>
>> return Status;
>>
>> @@ -500,11 +622,13 @@ ProcessAcpiTables (
>> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol
>>
>> )
>>
>> {
>>
>> - EFI_STATUS Status;
>>
>> - EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
>>
>> - CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
>>
>> - UINT32 AcpiTableCount;
>>
>> - UINT32 Idx;
>>
>> + EFI_STATUS Status;
>>
>> + EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol;
>>
>> + CM_STD_OBJ_ACPI_TABLE_INFO *AcpiTableInfo;
>>
>> + UINT32 AcpiTableCount;
>>
>> + UINT32 Idx;
>>
>> + UINTN Handle;
>>
>> + EFI_ACPI_DESCRIPTION_HEADER *DummyHeader;
>>
>>
>> ASSERT (TableFactoryProtocol != NULL);
>>
>> ASSERT (CfgMgrProtocol != NULL);
>>
>> @@ -570,29 +694,37 @@ ProcessAcpiTables (
>> }
>
>> [snip]
>
> // Check if mandatory ACPI tables are present.
> Status = VerifyMandatoryTablesArePresent (
> AcpiTableInfo,
> AcpiTableCount
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((
> DEBUG_ERROR,
> "ERROR: Failed to find mandatory ACPI Table(s)."
> " Status = %r\n",
> Status
>
> ));
>
>
> [SAMI] Is it possible to update the error reporting to reflect the
> EFI_ALREADY_STARTED error type, please? Please also update the
> function documentation header for ProcessAcpiTables().
>
>
> return Status;
>
> }
>
>
> [/snip]
>
>
>>
>> // Add the FADT Table first.
>>
>> - for (Idx = 0; Idx < AcpiTableCount; Idx++) {
>>
>> - if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
>>
>> - AcpiTableInfo[Idx].TableGeneratorId)
>>
>> - {
>>
>> - Status = BuildAndInstallAcpiTable (
>>
>> - TableFactoryProtocol,
>>
>> - CfgMgrProtocol,
>>
>> - AcpiTableProtocol,
>>
>> - &AcpiTableInfo[Idx]
>>
>> - );
>>
>> - if (EFI_ERROR (Status)) {
>>
>> - DEBUG ((
>>
>> - DEBUG_ERROR,
>>
>> - "ERROR: Failed to find build and install ACPI FADT Table." \
>>
>> - " Status = %r\n",
>>
>> - Status
>>
>> - ));
>>
>> - return Status;
>>
>> + Status = LocateAcpiTableBySignature (
>>
>> + EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>>
>> + &DummyHeader,
>>
>> + &Handle
>>
>> + );
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + // FADT is not yet installed
>>
>> + for (Idx = 0; Idx < AcpiTableCount; Idx++) {
>>
>> + if (CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt) ==
>>
>> + AcpiTableInfo[Idx].TableGeneratorId)
>>
>> + {
>>
>> + Status = BuildAndInstallAcpiTable (
>>
>> + TableFactoryProtocol,
>>
>> + CfgMgrProtocol,
>>
>> + AcpiTableProtocol,
>>
>> + &AcpiTableInfo[Idx]
>>
>> + );
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + DEBUG ((
>>
>> + DEBUG_ERROR,
>>
>> + "ERROR: Failed to find build and install ACPI FADT
>> Table." \
>>
>> + " Status = %r\n",
>>
>> + Status
>>
>> + ));
>>
>> + return Status;
>>
>> + }
>>
>> +
>>
>> + break;
>>
>> }
>>
>> -
>>
>> - break;
>>
>> - }
>>
>> - } // for
>>
>> + } // for
>>
>> + }
>>
>>
>> // Add remaining ACPI Tables
>>
>> for (Idx = 0; Idx < AcpiTableCount; Idx++) {
>>
>> diff --git
>> a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
>> b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
>>
>> index 028c3d413cf8..5ca98c8b4895 100644
>> ---
>> a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
>> +++
>> b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
>> @@ -36,6 +36,7 @@ [LibraryClasses]
>>
>> [Protocols]
>>
>> gEfiAcpiTableProtocolGuid # PROTOCOL
>> ALWAYS_CONSUMED
>>
>> + gEfiAcpiSdtProtocolGuid # PROTOCOL
>> ALWAYS_CONSUMED
>
> [SAMI] Should the DEPEX section be updated to relect the dependency on
> the SDT protocol?
>
>>
>>
>> gEdkiiConfigurationManagerProtocolGuid # PROTOCOL
>> ALWAYS_CONSUMED
>>
>> gEdkiiDynamicTableFactoryProtocolGuid # PROTOCOL
>> ALWAYS_CONSUMED
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-07-21 1:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-19 0:22 [PATCH v1 0/6] Enhance DynamicTablesPkg modules Kun Qin
2022-07-19 0:22 ` [PATCH v1 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
2022-07-20 10:03 ` Sami Mujawar
2022-07-19 0:22 ` [PATCH v1 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing Kun Qin
2022-07-20 10:06 ` Sami Mujawar
2022-07-19 0:22 ` [PATCH v1 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers Kun Qin
2022-07-20 10:39 ` Sami Mujawar
2022-07-19 0:22 ` [PATCH v1 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Kun Qin
2022-07-20 11:00 ` Sami Mujawar
2022-07-21 1:48 ` Kun Qin
2022-07-20 13:36 ` [edk2-devel] " PierreGondois
2022-07-19 0:22 ` [PATCH v1 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
2022-07-20 13:36 ` [edk2-devel] " PierreGondois
2022-07-19 0:22 ` [PATCH v1 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config Kun Qin
2022-07-20 13:38 ` [edk2-devel] [PATCH v1 0/6] Enhance DynamicTablesPkg modules PierreGondois
2022-07-21 1:45 ` Kun Qin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox