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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91501): https://edk2.groups.io/g/devel/message/91501
Mute This Topic: https://groups.io/mt/92473114/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91581): https://edk2.groups.io/g/devel/message/91581
Mute This Topic: https://groups.io/mt/92473114/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91577): https://edk2.groups.io/g/devel/message/91577
Mute This Topic: https://groups.io/mt/92473114/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91612): https://edk2.groups.io/g/devel/message/91612
Mute This Topic: https://groups.io/mt/92473114/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-