* [PATCH v4 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf
2022-08-10 22:28 [PATCH v4 0/6] Enhance DynamicTablesPkg modules Kun Qin
@ 2022-08-10 22:28 ` Kun Qin
2022-08-10 22:28 ` [PATCH v4 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing Kun Qin
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Kun Qin @ 2022-08-10 22:28 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez, Sami Mujawar,
Pierre Gondois
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>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
Notes:
v2:
- Added Reviewed-by tag [Sami]
- Added Reviewed-by tag [Pierre]
v3:
- No change.
v4:
- No change.
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.37.1.windows.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing
2022-08-10 22:28 [PATCH v4 0/6] Enhance DynamicTablesPkg modules Kun Qin
2022-08-10 22:28 ` [PATCH v4 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
@ 2022-08-10 22:28 ` Kun Qin
2022-08-10 22:28 ` [PATCH v4 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers Kun Qin
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Kun Qin @ 2022-08-10 22:28 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez, Sami Mujawar,
Pierre Gondois
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>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
Notes:
v2:
- Added Reviewed-by tag [Sami]
- Added Reviewed-by tag [Pierre]
v3:
- No change.
v4:
- No change.
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.37.1.windows.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers
2022-08-10 22:28 [PATCH v4 0/6] Enhance DynamicTablesPkg modules Kun Qin
2022-08-10 22:28 ` [PATCH v4 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
2022-08-10 22:28 ` [PATCH v4 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing Kun Qin
@ 2022-08-10 22:28 ` Kun Qin
2022-08-10 22:28 ` [PATCH v4 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Kun Qin
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Kun Qin @ 2022-08-10 22:28 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez, Sami Mujawar,
Pierre Gondois
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>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
Notes:
v2:
- Added Reviewed-by tag [Sami]
- Added Reviewed-by tag [Pierre]
v3:
- No change.
v4:
- No change.
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.37.1.windows.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
2022-08-10 22:28 [PATCH v4 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (2 preceding siblings ...)
2022-08-10 22:28 ` [PATCH v4 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers Kun Qin
@ 2022-08-10 22:28 ` Kun Qin
2022-08-10 22:28 ` [PATCH v4 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Kun Qin @ 2022-08-10 22:28 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez, Pierre Gondois
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, DBG2 and SPCR 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>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
Notes:
v2:
- Function description updates [Sami]
- Refactorized the table verification [Pierre]
v3:
- Added descriptions for new structures [Pierre]
- Added check for SDT protocol PCD before using it [Pierre]
v4:
- Reset return status before inspecting the table presence [Sami]
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 216 ++++++++++++--------
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 4 +
2 files changed, 140 insertions(+), 80 deletions(-)
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index ed62299f9bbd..1e9b811c4017 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
@@ -10,6 +10,7 @@
#include <Library/DebugLib.h>
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/AcpiSystemDescriptionTable.h>
#include <Protocol/AcpiTable.h>
// Module specific include files.
@@ -22,6 +23,58 @@
#include <Protocol/DynamicTableFactoryProtocol.h>
#include <SmbiosTableGenerator.h>
+///
+/// Bit definitions for acceptable ACPI table presence formats.
+/// Currently only ACPI tables present in the ACPI info list and
+/// already installed will count towards "Table Present" during
+/// verification routine.
+///
+#define ACPI_TABLE_PRESENT_INFO_LIST BIT0
+#define ACPI_TABLE_PRESENT_INSTALLED BIT1
+
+///
+/// Order of ACPI table being verified during presence inspection.
+///
+#define ACPI_TABLE_VERIFY_FADT 0
+#define ACPI_TABLE_VERIFY_MADT 1
+#define ACPI_TABLE_VERIFY_GTDT 2
+#define ACPI_TABLE_VERIFY_DSDT 3
+#define ACPI_TABLE_VERIFY_DBG2 4
+#define ACPI_TABLE_VERIFY_SPCR 5
+#define ACPI_TABLE_VERIFY_COUNT 6
+
+///
+/// Private data structure to verify the presence of mandatory
+/// or optional ACPI tables.
+///
+typedef struct {
+ /// ESTD ID for the ACPI table of interest.
+ ESTD_ACPI_TABLE_ID EstdTableId;
+ /// Standard UINT32 ACPI signature.
+ UINT32 AcpiTableSignature;
+ /// 4 character ACPI table name (the 5th char8 is for null terminator).
+ CHAR8 AcpiTableName[sizeof (UINT32) + 1];
+ /// Indicator on whether the ACPI table is required.
+ BOOLEAN IsMandatory;
+ /// Formats of verified presences, as defined by ACPI_TABLE_PRESENT_*
+ /// This field should be initialized to 0 and will be populated during
+ /// verification routine.
+ UINT16 Presence;
+} ACPI_TABLE_PRESENCE_INFO;
+
+///
+/// We require the FADT, MADT, GTDT and the DSDT tables to boot.
+/// This list also include optional ACPI tables: DBG2, SPCR.
+///
+ACPI_TABLE_PRESENCE_INFO mAcpiVerifyTables[ACPI_TABLE_VERIFY_COUNT] = {
+ { EStdAcpiTableIdFadt, EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, "FADT", TRUE, 0 },
+ { EStdAcpiTableIdMadt, EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, "MADT", TRUE, 0 },
+ { EStdAcpiTableIdGtdt, EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, "GTDT", TRUE, 0 },
+ { EStdAcpiTableIdDsdt, EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE, "DSDT", TRUE, 0 },
+ { EStdAcpiTableIdDbg2, EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE, "DBG2", FALSE, 0 },
+ { EStdAcpiTableIdSpcr, EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE, "SPCR", FALSE, 0 },
+};
+
/** This macro expands to a function that retrieves the ACPI Table
List from the Configuration Manager.
*/
@@ -395,6 +448,7 @@ BuildAndInstallAcpiTable (
@retval EFI_SUCCESS Success.
@retval EFI_NOT_FOUND If mandatory table is not found.
+ @retval EFI_ALREADY_STARTED If mandatory table found in AcpiTableInfo is already installed.
**/
STATIC
EFI_STATUS
@@ -404,75 +458,73 @@ VerifyMandatoryTablesArePresent (
IN UINT32 AcpiTableCount
)
{
- EFI_STATUS Status;
- BOOLEAN FadtFound;
- BOOLEAN MadtFound;
- BOOLEAN GtdtFound;
- BOOLEAN DsdtFound;
- BOOLEAN Dbg2Found;
- BOOLEAN SpcrFound;
+ EFI_STATUS Status;
+ UINTN Handle;
+ UINTN Index;
+ UINTN InstalledTableIndex;
+ EFI_ACPI_DESCRIPTION_HEADER *DescHeader;
+ EFI_ACPI_TABLE_VERSION Version;
+ EFI_ACPI_SDT_PROTOCOL *AcpiSdt;
- Status = EFI_SUCCESS;
- FadtFound = FALSE;
- MadtFound = FALSE;
- GtdtFound = FALSE;
- DsdtFound = FALSE;
- Dbg2Found = FALSE;
- SpcrFound = FALSE;
ASSERT (AcpiTableInfo != NULL);
+ Status = EFI_SUCCESS;
+
+ // Check against the statically initialized ACPI tables to see if they are in ACPI info list
while (AcpiTableCount-- != 0) {
- switch (AcpiTableInfo[AcpiTableCount].AcpiTableSignature) {
- case EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE:
- FadtFound = TRUE;
- break;
- case EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE:
- MadtFound = TRUE;
- break;
- case EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE:
- GtdtFound = TRUE;
- break;
- case EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE:
- DsdtFound = TRUE;
- break;
- case EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE:
- Dbg2Found = TRUE;
- break;
- case EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE:
- SpcrFound = TRUE;
- break;
- default:
+ for (Index = 0; Index < ACPI_TABLE_VERIFY_COUNT; Index++) {
+ if (AcpiTableInfo[AcpiTableCount].AcpiTableSignature == mAcpiVerifyTables[Index].AcpiTableSignature) {
+ mAcpiVerifyTables[Index].Presence |= ACPI_TABLE_PRESENT_INFO_LIST;
+ // Found this table, skip the rest.
break;
+ }
}
}
- // 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"));
- Status = EFI_NOT_FOUND;
- }
+ // They also might be published already, so we can search from there
+ if (FeaturePcdGet (PcdInstallAcpiSdtProtocol)) {
+ AcpiSdt = NULL;
+ Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL, (VOID **)&AcpiSdt);
- if (!MadtFound) {
- DEBUG ((DEBUG_ERROR, "ERROR: MADT Table not found.\n"));
- Status = EFI_NOT_FOUND;
- }
+ if (EFI_ERROR (Status) || (AcpiSdt == NULL)) {
+ DEBUG ((DEBUG_ERROR, "ERROR: Failed to locate ACPI SDT protocol (0x%p) - %r\n", AcpiSdt, Status));
+ return Status;
+ }
- if (!GtdtFound) {
- DEBUG ((DEBUG_ERROR, "ERROR: GTDT Table not found.\n"));
- Status = EFI_NOT_FOUND;
- }
+ for (Index = 0; Index < ACPI_TABLE_VERIFY_COUNT; Index++) {
+ Handle = 0;
+ InstalledTableIndex = 0;
+ do {
+ Status = AcpiSdt->GetAcpiTable (InstalledTableIndex, (EFI_ACPI_SDT_HEADER **)&DescHeader, &Version, &Handle);
+ if (EFI_ERROR (Status)) {
+ break;
+ }
- if (!DsdtFound) {
- DEBUG ((DEBUG_ERROR, "ERROR: DSDT Table not found.\n"));
- Status = EFI_NOT_FOUND;
- }
+ InstalledTableIndex++;
+ } while (DescHeader->Signature != mAcpiVerifyTables[Index].AcpiTableSignature);
- if (!Dbg2Found) {
- DEBUG ((DEBUG_WARN, "WARNING: DBG2 Table not found.\n"));
+ if (!EFI_ERROR (Status)) {
+ mAcpiVerifyTables[Index].Presence |= ACPI_TABLE_PRESENT_INSTALLED;
+ }
+ }
}
- if (!SpcrFound) {
- DEBUG ((DEBUG_WARN, "WARNING: SPCR Table not found.\n"));
+ // Reset the return Status value to EFI_SUCCESS. We do not fully care if the table look up has failed.
+ Status = EFI_SUCCESS;
+ for (Index = 0; Index < ACPI_TABLE_VERIFY_COUNT; Index++) {
+ if (mAcpiVerifyTables[Index].Presence == 0) {
+ if (mAcpiVerifyTables[Index].IsMandatory) {
+ DEBUG ((DEBUG_ERROR, "ERROR: %a Table not found.\n", mAcpiVerifyTables[Index].AcpiTableName));
+ Status = EFI_NOT_FOUND;
+ } else {
+ DEBUG ((DEBUG_WARN, "WARNING: %a Table not found.\n", mAcpiVerifyTables[Index].AcpiTableName));
+ }
+ } else if (mAcpiVerifyTables[Index].Presence ==
+ (ACPI_TABLE_PRESENT_INFO_LIST | ACPI_TABLE_PRESENT_INSTALLED))
+ {
+ DEBUG ((DEBUG_ERROR, "ERROR: %a Table found while already published.\n", mAcpiVerifyTables[Index].AcpiTableName));
+ Status = EFI_ALREADY_STARTED;
+ }
}
return Status;
@@ -489,8 +541,9 @@ VerifyMandatoryTablesArePresent (
@param [in] CfgMgrProtocol Pointer to the Configuration Manager
Protocol Interface.
- @retval EFI_SUCCESS Success.
- @retval EFI_NOT_FOUND If a mandatory table or a generator is not found.
+ @retval EFI_SUCCESS Success.
+ @retval EFI_NOT_FOUND If a mandatory table or a generator is not found.
+ @retval EFI_ALREADY_STARTED If mandatory table found in AcpiTableInfo is already installed.
**/
STATIC
EFI_STATUS
@@ -562,7 +615,7 @@ ProcessAcpiTables (
if (EFI_ERROR (Status)) {
DEBUG ((
DEBUG_ERROR,
- "ERROR: Failed to find mandatory ACPI Table(s)."
+ "ERROR: Failed to verify mandatory ACPI Table(s) presence."
" Status = %r\n",
Status
));
@@ -570,29 +623,32 @@ 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;
- }
+ if ((mAcpiVerifyTables[ACPI_TABLE_VERIFY_FADT].Presence & ACPI_TABLE_PRESENT_INSTALLED) == 0) {
+ // 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;
- }
- } // for
+ break;
+ }
+ } // 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..ad8b3d037c16 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
@@ -34,8 +34,12 @@ [LibraryClasses]
UefiBootServicesTableLib
UefiDriverEntryPoint
+[FeaturePcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol ## CONSUMES
+
[Protocols]
gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+ gEfiAcpiSdtProtocolGuid # PROTOCOL ALWAYS_CONSUMED
gEdkiiConfigurationManagerProtocolGuid # PROTOCOL ALWAYS_CONSUMED
gEdkiiDynamicTableFactoryProtocolGuid # PROTOCOL ALWAYS_CONSUMED
--
2.37.1.windows.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space
2022-08-10 22:28 [PATCH v4 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (3 preceding siblings ...)
2022-08-10 22:28 ` [PATCH v4 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Kun Qin
@ 2022-08-10 22:28 ` Kun Qin
2022-08-17 12:02 ` [edk2-devel] " Sami Mujawar
2022-08-10 22:28 ` [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config Kun Qin
2022-09-01 10:17 ` [PATCH v4 0/6] Enhance DynamicTablesPkg modules Sami Mujawar
6 siblings, 1 reply; 17+ messages in thread
From: Kun Qin @ 2022-08-10 22:28 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>
---
Notes:
v2:
- Only create RES0 after config space checking [Pierre]
v3:
- Updated function names and descriptions [Pierre]
- Moved translation calculation to CONFIG case [Pierre]
v4:
- Used CM_ARM_PCI_CONFIG_SPACE_INFO for ECAM region calculation [Sami, Pierre]
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 135 ++++++++++++++++++++
1 file changed, 135 insertions(+)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index ceffe2838c03..dd75fc27e60e 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -34,6 +34,9 @@
#include "SsdtPcieGenerator.h"
+#define PCI_MAX_DEVICE_COUNT_PER_BUS 32
+#define PCI_MAX_FUNCTION_COUNT_PER_DEVICE 8
+
/** ARM standard SSDT Pcie Table Generator.
Requirements:
@@ -616,6 +619,130 @@ GeneratePciCrs (
return Status;
}
+/** Generate a RES0 device node to reserve PNP motherboard resources
+ for a given PCI node.
+
+ @param [in] PciNode Parent PCI node handle of the generated
+ resource object.
+ @param [out] CrsNode CRS node of the AML tree to populate.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid input parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GenerateMotherboardDevice (
+ IN AML_OBJECT_NODE_HANDLE PciNode,
+ OUT AML_OBJECT_NODE_HANDLE *CrsNode
+ )
+{
+ EFI_STATUS Status;
+ UINT32 EisaId;
+ AML_OBJECT_NODE_HANDLE ResNode;
+
+ if (CrsNode == NULL) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // ASL: Device (RES0) {}
+ 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;
+ }
+
+ return Status;
+}
+
+/** Reserves ECAM space for PCI config space
+
+ @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
+ReserveEcamSpace (
+ 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;
+ AML_OBJECT_NODE_HANDLE CrsNode;
+ UINT64 AddressMinimum;
+ UINT64 AddressMaximum;
+
+ Status = GenerateMotherboardDevice (PciNode, &CrsNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ AddressMinimum = PciInfo->BaseAddress + (PciInfo->StartBusNumber *
+ PCI_MAX_DEVICE_COUNT_PER_BUS * PCI_MAX_FUNCTION_COUNT_PER_DEVICE * SIZE_4KB);
+ AddressMaximum = PciInfo->BaseAddress + ((PciInfo->EndBusNumber + 1) *
+ PCI_MAX_DEVICE_COUNT_PER_BUS * PCI_MAX_FUNCTION_COUNT_PER_DEVICE * SIZE_4KB) - 1;
+
+ Status = AmlCodeGenRdQWordMemory (
+ FALSE,
+ TRUE,
+ TRUE,
+ TRUE,
+ FALSE, // non-cacheable
+ TRUE,
+ 0,
+ AddressMinimum,
+ AddressMaximum,
+ 0, // no translation
+ AddressMaximum - AddressMinimum + 1,
+ 0,
+ NULL,
+ 0,
+ TRUE,
+ CrsNode,
+ NULL
+ );
+
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ return Status;
+}
+
/** Generate a Pci device.
@param [in] Generator The SSDT Pci generator.
@@ -702,9 +829,17 @@ GeneratePciDevice (
return Status;
}
+ // Add the PNP Motherboard Resources Device to reserve ECAM space
+ Status = ReserveEcamSpace (Generator, CfgMgrProtocol, PciInfo, PciNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
// Add the template _OSC method.
Status = AddOscMethod (PciInfo, PciNode);
ASSERT_EFI_ERROR (Status);
+
return Status;
}
--
2.37.1.windows.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [edk2-devel] [PATCH v4 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space
2022-08-10 22:28 ` [PATCH v4 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
@ 2022-08-17 12:02 ` Sami Mujawar
0 siblings, 0 replies; 17+ messages in thread
From: Sami Mujawar @ 2022-08-17 12:02 UTC (permalink / raw)
To: devel, kuqin12; +Cc: Joe Lopez, nd@arm.com
Hi Kun,
Thank you for the updated patch.
These changes look good to me.
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 10/08/2022 11:28 pm, 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>
> ---
>
> Notes:
> v2:
> - Only create RES0 after config space checking [Pierre]
>
> v3:
> - Updated function names and descriptions [Pierre]
> - Moved translation calculation to CONFIG case [Pierre]
>
> v4:
> - Used CM_ARM_PCI_CONFIG_SPACE_INFO for ECAM region calculation [Sami, Pierre]
>
> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 135 ++++++++++++++++++++
> 1 file changed, 135 insertions(+)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> index ceffe2838c03..dd75fc27e60e 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> @@ -34,6 +34,9 @@
>
>
> #include "SsdtPcieGenerator.h"
>
>
>
> +#define PCI_MAX_DEVICE_COUNT_PER_BUS 32
>
> +#define PCI_MAX_FUNCTION_COUNT_PER_DEVICE 8
>
> +
>
> /** ARM standard SSDT Pcie Table Generator.
>
>
>
> Requirements:
>
> @@ -616,6 +619,130 @@ GeneratePciCrs (
> return Status;
>
> }
>
>
>
> +/** Generate a RES0 device node to reserve PNP motherboard resources
>
> + for a given PCI node.
>
> +
>
> + @param [in] PciNode Parent PCI node handle of the generated
>
> + resource object.
>
> + @param [out] CrsNode CRS node of the AML tree to populate.
>
> +
>
> + @retval EFI_SUCCESS The function completed successfully.
>
> + @retval EFI_INVALID_PARAMETER Invalid input parameter.
>
> + @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
>
> +**/
>
> +STATIC
>
> +EFI_STATUS
>
> +EFIAPI
>
> +GenerateMotherboardDevice (
>
> + IN AML_OBJECT_NODE_HANDLE PciNode,
>
> + OUT AML_OBJECT_NODE_HANDLE *CrsNode
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + UINT32 EisaId;
>
> + AML_OBJECT_NODE_HANDLE ResNode;
>
> +
>
> + if (CrsNode == NULL) {
>
> + ASSERT (0);
>
> + return EFI_INVALID_PARAMETER;
>
> + }
>
> +
>
> + // ASL: Device (RES0) {}
>
> + 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;
>
> + }
>
> +
>
> + return Status;
>
> +}
>
> +
>
> +/** Reserves ECAM space for PCI config space
>
> +
>
> + @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
>
> +ReserveEcamSpace (
>
> + 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;
>
> + AML_OBJECT_NODE_HANDLE CrsNode;
>
> + UINT64 AddressMinimum;
>
> + UINT64 AddressMaximum;
>
> +
>
> + Status = GenerateMotherboardDevice (PciNode, &CrsNode);
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT (0);
>
> + return Status;
>
> + }
>
> +
>
> + AddressMinimum = PciInfo->BaseAddress + (PciInfo->StartBusNumber *
>
> + PCI_MAX_DEVICE_COUNT_PER_BUS * PCI_MAX_FUNCTION_COUNT_PER_DEVICE * SIZE_4KB);
>
> + AddressMaximum = PciInfo->BaseAddress + ((PciInfo->EndBusNumber + 1) *
>
> + PCI_MAX_DEVICE_COUNT_PER_BUS * PCI_MAX_FUNCTION_COUNT_PER_DEVICE * SIZE_4KB) - 1;
>
> +
>
> + Status = AmlCodeGenRdQWordMemory (
>
> + FALSE,
>
> + TRUE,
>
> + TRUE,
>
> + TRUE,
>
> + FALSE, // non-cacheable
>
> + TRUE,
>
> + 0,
>
> + AddressMinimum,
>
> + AddressMaximum,
>
> + 0, // no translation
>
> + AddressMaximum - AddressMinimum + 1,
>
> + 0,
>
> + NULL,
>
> + 0,
>
> + TRUE,
>
> + CrsNode,
>
> + NULL
>
> + );
>
> +
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT (0);
>
> + return Status;
>
> + }
>
> +
>
> + return Status;
>
> +}
>
> +
>
> /** Generate a Pci device.
>
>
>
> @param [in] Generator The SSDT Pci generator.
>
> @@ -702,9 +829,17 @@ GeneratePciDevice (
> return Status;
>
> }
>
>
>
> + // Add the PNP Motherboard Resources Device to reserve ECAM space
>
> + Status = ReserveEcamSpace (Generator, CfgMgrProtocol, PciInfo, PciNode);
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT (0);
>
> + return Status;
>
> + }
>
> +
>
> // Add the template _OSC method.
>
> Status = AddOscMethod (PciInfo, PciNode);
>
> ASSERT_EFI_ERROR (Status);
>
> +
>
> return Status;
>
> }
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-08-10 22:28 [PATCH v4 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (4 preceding siblings ...)
2022-08-10 22:28 ` [PATCH v4 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
@ 2022-08-10 22:28 ` Kun Qin
2022-08-16 15:33 ` PierreGondois
2022-09-01 10:17 ` [PATCH v4 0/6] Enhance DynamicTablesPkg modules Sami Mujawar
6 siblings, 1 reply; 17+ messages in thread
From: Kun Qin @ 2022-08-10 22:28 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez, Pierre Gondois,
Sami Mujawar
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>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- Added Reviewed-by tag [Pierre]
v3:
- No change
v4:
- Added Reviewed-by tag [Sami]
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 dd75fc27e60e..c6fbd09c43f8 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -606,6 +606,11 @@ GeneratePciCrs (
);
break;
+ case PCI_SS_CONFIG:
+ // Do nothing
+ Status = EFI_SUCCESS;
+ break;
+
default:
Status = EFI_INVALID_PARAMETER;
} // switch
--
2.37.1.windows.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-08-10 22:28 ` [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config Kun Qin
@ 2022-08-16 15:33 ` PierreGondois
2022-08-17 0:17 ` Kun Qin
0 siblings, 1 reply; 17+ messages in thread
From: PierreGondois @ 2022-08-16 15:33 UTC (permalink / raw)
To: Kun Qin, devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
Hello Kun,
Is this patch still required ?
Cf: https://edk2.groups.io/g/devel/message/92204
The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
PCI_SS_CONFIG should be an invalid case.
If not I don't think a v5 should be necessary.
Also I ran the patchset on KvmTool and everything was working.
Regards,
Pierre
On 8/11/22 00:28, Kun Qin wrote:
> 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>
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
> v2:
> - Added Reviewed-by tag [Pierre]
>
> v3:
> - No change
>
> v4:
> - Added Reviewed-by tag [Sami]
>
> 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 dd75fc27e60e..c6fbd09c43f8 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> @@ -606,6 +606,11 @@ GeneratePciCrs (
> );
> break;
>
> + case PCI_SS_CONFIG:
> + // Do nothing
> + Status = EFI_SUCCESS;
> + break;
> +
> default:
> Status = EFI_INVALID_PARAMETER;
> } // switch
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-08-16 15:33 ` PierreGondois
@ 2022-08-17 0:17 ` Kun Qin
2022-08-17 8:53 ` PierreGondois
0 siblings, 1 reply; 17+ messages in thread
From: Kun Qin @ 2022-08-17 0:17 UTC (permalink / raw)
To: Pierre Gondois, devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
[-- Attachment #1: Type: text/plain, Size: 2512 bytes --]
Hi Pierre,
You are correct that if CM_ARM_PCI_ADDRESS_MAP_INFO.PCI_SS_CONFIG
is no longer being used, this patch is not needed. Thanks for catching this.
On the other hand, just for my learning purpose, could you please let me
know
what the use case for "PCI_SS_CONFIG" is? It does not seem to be used at
all.
Thanks again for testing these patches!
Regards,
Kun
On 8/16/2022 8:33 AM, Pierre Gondois wrote:
> Hello Kun,
>
> Is this patch still required ?
> Cf: https://edk2.groups.io/g/devel/message/92204
>
> The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
> the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
> PCI_SS_CONFIG should be an invalid case.
> If not I don't think a v5 should be necessary.
>
> Also I ran the patchset on KvmTool and everything was working.
>
> Regards,
> Pierre
>
> On 8/11/22 00:28, Kun Qin wrote:
>> 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>
>> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>>
>> Notes:
>> v2:
>> - Added Reviewed-by tag [Pierre]
>> v3:
>> - No change
>> v4:
>> - Added Reviewed-by tag [Sami]
>>
>> 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 dd75fc27e60e..c6fbd09c43f8 100644
>> ---
>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>> +++
>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>> @@ -606,6 +606,11 @@ GeneratePciCrs (
>> );
>> break;
>> + case PCI_SS_CONFIG:
>> + // Do nothing
>> + Status = EFI_SUCCESS;
>> + break;
>> +
>> default:
>> Status = EFI_INVALID_PARAMETER;
>> } // switch
[-- Attachment #2: Type: text/html, Size: 5327 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-08-17 0:17 ` Kun Qin
@ 2022-08-17 8:53 ` PierreGondois
2022-08-17 12:06 ` Sami Mujawar
0 siblings, 1 reply; 17+ messages in thread
From: PierreGondois @ 2022-08-17 8:53 UTC (permalink / raw)
To: Kun Qin, devel; +Cc: Sami Mujawar, Alexei Fedorov, Joe Lopez
On 8/17/22 02:17, Kun Qin wrote:
> Hi Pierre,
>
> You are correct that if CM_ARM_PCI_ADDRESS_MAP_INFO.PCI_SS_CONFIG
> is no longer being used, this patch is not needed. Thanks for catching this.
>
> On the other hand, just for my learning purpose, could you please let me know
> what the use case for "PCI_SS_CONFIG" is? It does not seem to be used at all.
I haven't seen any usecase neither so far, but it is to be used to reference a
PCI bus/device/function/register location to identify/configure a device
from what I understood.
>
> Thanks again for testing these patches!
>
> Regards,
> Kun
>
> On 8/16/2022 8:33 AM, Pierre Gondois wrote:
>> Hello Kun,
>>
>> Is this patch still required ?
>> Cf: https://edk2.groups.io/g/devel/message/92204
>>
>> The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
>> the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
>> PCI_SS_CONFIG should be an invalid case.
>> If not I don't think a v5 should be necessary.
>>
>> Also I ran the patchset on KvmTool and everything was working.
>>
>> Regards,
>> Pierre
>>
>> On 8/11/22 00:28, Kun Qin wrote:
>>> 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>
>>> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>>> ---
>>>
>>> Notes:
>>> v2:
>>> - Added Reviewed-by tag [Pierre]
>>> v3:
>>> - No change
>>> v4:
>>> - Added Reviewed-by tag [Sami]
>>>
>>> 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 dd75fc27e60e..c6fbd09c43f8 100644
>>> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>> @@ -606,6 +606,11 @@ GeneratePciCrs (
>>> );
>>> break;
>>> + case PCI_SS_CONFIG:
>>> + // Do nothing
>>> + Status = EFI_SUCCESS;
>>> + break;
>>> +
>>> default:
>>> Status = EFI_INVALID_PARAMETER;
>>> } // switch
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-08-17 8:53 ` PierreGondois
@ 2022-08-17 12:06 ` Sami Mujawar
2022-08-17 17:42 ` [edk2-devel] " Kun Qin
0 siblings, 1 reply; 17+ messages in thread
From: Sami Mujawar @ 2022-08-17 12:06 UTC (permalink / raw)
To: Pierre Gondois, Kun Qin, devel; +Cc: Alexei Fedorov, Joe Lopez, nd@arm.com
Hi Kun,
I plan to get this series merged when the merge window opens.
If you agree, I will drop this patch before merging. Please let me know
if that is ok.
Regards,
Sami Mujawar
On 17/08/2022 09:53 am, Pierre Gondois wrote:
>
>
> On 8/17/22 02:17, Kun Qin wrote:
>> Hi Pierre,
>>
>> You are correct that if CM_ARM_PCI_ADDRESS_MAP_INFO.PCI_SS_CONFIG
>> is no longer being used, this patch is not needed. Thanks for
>> catching this.
>>
>> On the other hand, just for my learning purpose, could you please let
>> me know
>> what the use case for "PCI_SS_CONFIG" is? It does not seem to be used
>> at all.
>
> I haven't seen any usecase neither so far, but it is to be used to
> reference a
> PCI bus/device/function/register location to identify/configure a device
> from what I understood.
>
>>
>> Thanks again for testing these patches!
>>
>> Regards,
>> Kun
>>
>> On 8/16/2022 8:33 AM, Pierre Gondois wrote:
>>> Hello Kun,
>>>
>>> Is this patch still required ?
>>> Cf: https://edk2.groups.io/g/devel/message/92204
>>>
>>> The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
>>> the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
>>> PCI_SS_CONFIG should be an invalid case.
>>> If not I don't think a v5 should be necessary.
>>>
>>> Also I ran the patchset on KvmTool and everything was working.
>>>
>>> Regards,
>>> Pierre
>>>
>>> On 8/11/22 00:28, Kun Qin wrote:
>>>> 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>
>>>> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
>>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>>>> ---
>>>>
>>>> Notes:
>>>> v2:
>>>> - Added Reviewed-by tag [Pierre]
>>>> v3:
>>>> - No change
>>>> v4:
>>>> - Added Reviewed-by tag [Sami]
>>>>
>>>> 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 dd75fc27e60e..c6fbd09c43f8 100644
>>>> ---
>>>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>> +++
>>>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>> @@ -606,6 +606,11 @@ GeneratePciCrs (
>>>> );
>>>> break;
>>>> + case PCI_SS_CONFIG:
>>>> + // Do nothing
>>>> + Status = EFI_SUCCESS;
>>>> + break;
>>>> +
>>>> default:
>>>> Status = EFI_INVALID_PARAMETER;
>>>> } // switch
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-08-17 12:06 ` Sami Mujawar
@ 2022-08-17 17:42 ` Kun Qin
2022-08-30 17:55 ` Kun Qin
0 siblings, 1 reply; 17+ messages in thread
From: Kun Qin @ 2022-08-17 17:42 UTC (permalink / raw)
To: devel, sami.mujawar, Pierre Gondois; +Cc: Alexei Fedorov, Joe Lopez, nd@arm.com
Hi Sami,
Thank you for the help! I agree that we can drop this patch and merge
the rest when the window is open.
Pierre,
Thanks for your input on the usage as well!
Regards,
Kun
On 8/17/2022 5:06 AM, Sami Mujawar wrote:
> Hi Kun,
>
> I plan to get this series merged when the merge window opens.
>
> If you agree, I will drop this patch before merging. Please let me
> know if that is ok.
>
> Regards,
>
> Sami Mujawar
>
> On 17/08/2022 09:53 am, Pierre Gondois wrote:
>>
>>
>> On 8/17/22 02:17, Kun Qin wrote:
>>> Hi Pierre,
>>>
>>> You are correct that if CM_ARM_PCI_ADDRESS_MAP_INFO.PCI_SS_CONFIG
>>> is no longer being used, this patch is not needed. Thanks for
>>> catching this.
>>>
>>> On the other hand, just for my learning purpose, could you please
>>> let me know
>>> what the use case for "PCI_SS_CONFIG" is? It does not seem to be
>>> used at all.
>>
>> I haven't seen any usecase neither so far, but it is to be used to
>> reference a
>> PCI bus/device/function/register location to identify/configure a device
>> from what I understood.
>>
>>>
>>> Thanks again for testing these patches!
>>>
>>> Regards,
>>> Kun
>>>
>>> On 8/16/2022 8:33 AM, Pierre Gondois wrote:
>>>> Hello Kun,
>>>>
>>>> Is this patch still required ?
>>>> Cf: https://edk2.groups.io/g/devel/message/92204
>>>>
>>>> The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
>>>> the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
>>>> PCI_SS_CONFIG should be an invalid case.
>>>> If not I don't think a v5 should be necessary.
>>>>
>>>> Also I ran the patchset on KvmTool and everything was working.
>>>>
>>>> Regards,
>>>> Pierre
>>>>
>>>> On 8/11/22 00:28, Kun Qin wrote:
>>>>> 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>
>>>>> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
>>>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>> v2:
>>>>> - Added Reviewed-by tag [Pierre]
>>>>> v3:
>>>>> - No change
>>>>> v4:
>>>>> - Added Reviewed-by tag [Sami]
>>>>>
>>>>> 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 dd75fc27e60e..c6fbd09c43f8 100644
>>>>> ---
>>>>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>>> +++
>>>>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>>> @@ -606,6 +606,11 @@ GeneratePciCrs (
>>>>> );
>>>>> break;
>>>>> + case PCI_SS_CONFIG:
>>>>> + // Do nothing
>>>>> + Status = EFI_SUCCESS;
>>>>> + break;
>>>>> +
>>>>> default:
>>>>> Status = EFI_INVALID_PARAMETER;
>>>>> } // switch
>
>
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
2022-08-17 17:42 ` [edk2-devel] " Kun Qin
@ 2022-08-30 17:55 ` Kun Qin
0 siblings, 0 replies; 17+ messages in thread
From: Kun Qin @ 2022-08-30 17:55 UTC (permalink / raw)
To: devel, sami.mujawar, Pierre Gondois; +Cc: Alexei Fedorov, Joe Lopez, nd@arm.com
Hi Sami,
Now that the merge window opens, could you please help us to merge in
these patch series (after dropping this specific patch) when you have a
chance?
Please let me know if you ran into issues when doing that. Thanks in
advance!
Regards,
Kun
On 8/17/2022 10:42 AM, Kun Qin wrote:
> Hi Sami,
>
> Thank you for the help! I agree that we can drop this patch and merge
> the rest when the window is open.
>
> Pierre,
>
> Thanks for your input on the usage as well!
>
> Regards,
> Kun
>
> On 8/17/2022 5:06 AM, Sami Mujawar wrote:
>> Hi Kun,
>>
>> I plan to get this series merged when the merge window opens.
>>
>> If you agree, I will drop this patch before merging. Please let me
>> know if that is ok.
>>
>> Regards,
>>
>> Sami Mujawar
>>
>> On 17/08/2022 09:53 am, Pierre Gondois wrote:
>>>
>>>
>>> On 8/17/22 02:17, Kun Qin wrote:
>>>> Hi Pierre,
>>>>
>>>> You are correct that if CM_ARM_PCI_ADDRESS_MAP_INFO.PCI_SS_CONFIG
>>>> is no longer being used, this patch is not needed. Thanks for
>>>> catching this.
>>>>
>>>> On the other hand, just for my learning purpose, could you please
>>>> let me know
>>>> what the use case for "PCI_SS_CONFIG" is? It does not seem to be
>>>> used at all.
>>>
>>> I haven't seen any usecase neither so far, but it is to be used to
>>> reference a
>>> PCI bus/device/function/register location to identify/configure a
>>> device
>>> from what I understood.
>>>
>>>>
>>>> Thanks again for testing these patches!
>>>>
>>>> Regards,
>>>> Kun
>>>>
>>>> On 8/16/2022 8:33 AM, Pierre Gondois wrote:
>>>>> Hello Kun,
>>>>>
>>>>> Is this patch still required ?
>>>>> Cf: https://edk2.groups.io/g/devel/message/92204
>>>>>
>>>>> The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
>>>>> the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
>>>>> PCI_SS_CONFIG should be an invalid case.
>>>>> If not I don't think a v5 should be necessary.
>>>>>
>>>>> Also I ran the patchset on KvmTool and everything was working.
>>>>>
>>>>> Regards,
>>>>> Pierre
>>>>>
>>>>> On 8/11/22 00:28, Kun Qin wrote:
>>>>>> 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>
>>>>>> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
>>>>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>>>>>> ---
>>>>>>
>>>>>> Notes:
>>>>>> v2:
>>>>>> - Added Reviewed-by tag [Pierre]
>>>>>> v3:
>>>>>> - No change
>>>>>> v4:
>>>>>> - Added Reviewed-by tag [Sami]
>>>>>>
>>>>>> 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 dd75fc27e60e..c6fbd09c43f8 100644
>>>>>> ---
>>>>>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>>>> +++
>>>>>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>>>> @@ -606,6 +606,11 @@ GeneratePciCrs (
>>>>>> );
>>>>>> break;
>>>>>> + case PCI_SS_CONFIG:
>>>>>> + // Do nothing
>>>>>> + Status = EFI_SUCCESS;
>>>>>> + break;
>>>>>> +
>>>>>> default:
>>>>>> Status = EFI_INVALID_PARAMETER;
>>>>>> } // switch
>>
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/6] Enhance DynamicTablesPkg modules
2022-08-10 22:28 [PATCH v4 0/6] Enhance DynamicTablesPkg modules Kun Qin
` (5 preceding siblings ...)
2022-08-10 22:28 ` [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config Kun Qin
@ 2022-09-01 10:17 ` Sami Mujawar
2022-09-01 11:17 ` [edk2-devel] " Sami Mujawar
2022-09-01 17:38 ` Kun Qin
6 siblings, 2 replies; 17+ messages in thread
From: Sami Mujawar @ 2022-09-01 10:17 UTC (permalink / raw)
To: Kun Qin, devel; +Cc: Joe Lopez, Alexei Fedorov, Pierre Gondois, nd@arm.com
Dropped last patch in this series as discussed at
https://edk2.groups.io/g/devel/topic/92947269#92984 and tested using
guest firmware for Kvmtool.
For this series.
Tested-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 10/08/2022 11:28 pm, Kun Qin 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
>
> This patch series is a follow-up of previous submission:
> https://edk2.groups.io/g/devel/message/91995
>
> The main changes between v3 and v4 patches are:
> - Added reviewed-by collected from previous iteration
> - Fixed an error where the ACPI verification will fail incorrectly
> - Updated ECAM reservation routine with config space based look-up
>
> 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 v4 branch: https://github.com/kuqin12/edk2/tree/dynamic_update_v4
>
> Cc: Joe Lopez <joelopez@microsoft.com>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
> Cc: Pierre Gondois <pierre.gondois@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 | 216 ++++++++++++--------
> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 140 +++++++++++++
> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c | 80 +++++++-
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 4 +
> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf | 1 +
> 5 files changed, 357 insertions(+), 84 deletions(-)
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [edk2-devel] [PATCH v4 0/6] Enhance DynamicTablesPkg modules
2022-09-01 10:17 ` [PATCH v4 0/6] Enhance DynamicTablesPkg modules Sami Mujawar
@ 2022-09-01 11:17 ` Sami Mujawar
2022-09-01 17:38 ` Kun Qin
1 sibling, 0 replies; 17+ messages in thread
From: Sami Mujawar @ 2022-09-01 11:17 UTC (permalink / raw)
To: Sami Mujawar, devel
[-- Attachment #1: Type: text/plain, Size: 77 bytes --]
Merged as 9ca7ece8b3b1..033ba8bb2976
Thanks.
Regards,
Sami Mujawar
[-- Attachment #2: Type: text/html, Size: 101 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/6] Enhance DynamicTablesPkg modules
2022-09-01 10:17 ` [PATCH v4 0/6] Enhance DynamicTablesPkg modules Sami Mujawar
2022-09-01 11:17 ` [edk2-devel] " Sami Mujawar
@ 2022-09-01 17:38 ` Kun Qin
1 sibling, 0 replies; 17+ messages in thread
From: Kun Qin @ 2022-09-01 17:38 UTC (permalink / raw)
To: Sami Mujawar, devel; +Cc: Joe Lopez, Alexei Fedorov, Pierre Gondois, nd@arm.com
Hi Sami,
Thank you so much the test and help!
Regards,
Kun
On 9/1/2022 3:17 AM, Sami Mujawar wrote:
> Dropped last patch in this series as discussed at
> https://edk2.groups.io/g/devel/topic/92947269#92984 and tested using
> guest firmware for Kvmtool.
>
> For this series.
>
> Tested-by: Sami Mujawar <sami.mujawar@arm.com>
>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>
> Regards,
>
> Sami Mujawar
>
> On 10/08/2022 11:28 pm, Kun Qin 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
>>
>> This patch series is a follow-up of previous submission:
>> https://edk2.groups.io/g/devel/message/91995
>>
>> The main changes between v3 and v4 patches are:
>> - Added reviewed-by collected from previous iteration
>> - Fixed an error where the ACPI verification will fail incorrectly
>> - Updated ECAM reservation routine with config space based look-up
>>
>> 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 v4 branch: https://github.com/kuqin12/edk2/tree/dynamic_update_v4
>>
>> Cc: Joe Lopez <joelopez@microsoft.com>
>> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
>> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>> Cc: Pierre Gondois <pierre.gondois@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
>> | 216 ++++++++++++--------
>> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>> | 140 +++++++++++++
>> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/CmObjectTokenFixer.c
>> | 80 +++++++-
>> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
>> | 4 +
>> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
>> | 1 +
>> 5 files changed, 357 insertions(+), 84 deletions(-)
>>
^ permalink raw reply [flat|nested] 17+ messages in thread