public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kun Qin" <kuqin12@gmail.com>
To: devel@edk2.groups.io
Cc: Sami Mujawar <Sami.Mujawar@arm.com>,
	Alexei Fedorov <Alexei.Fedorov@arm.com>,
	Joe Lopez <joelopez@microsoft.com>,
	Pierre Gondois <pierre.gondois@arm.com>
Subject: [PATCH v3 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
Date: Sat, 30 Jul 2022 22:37:25 -0700	[thread overview]
Message-ID: <20220731053727.536-5-kuqin12@gmail.com> (raw)
In-Reply-To: <20220731053727.536-1-kuqin12@gmail.com>

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]

 DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c   | 214 ++++++++++++--------
 DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf |   4 +
 2 files changed, 138 insertions(+), 80 deletions(-)

diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index ed62299f9bbd..7f3deef08a66 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,71 @@ 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"));
+  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 +539,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 +613,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 +621,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


  parent reply	other threads:[~2022-07-31  5:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-31  5:37 [PATCH v3 0/6] Enhance DynamicTablesPkg modules Kun Qin
2022-07-31  5:37 ` [PATCH v3 1/6] DynamicTablesPkg: DynamicPlatRepoLib: Added MemoryAllocationLib to inf Kun Qin
2022-07-31  5:37 ` [PATCH v3 2/6] DynamicTablesPkg: DynamicPlatRepoLib: Fix incorrect dereferencing Kun Qin
2022-07-31  5:37 ` [PATCH v3 3/6] DynamicTablesPkg: DynamicPlatRepoLib: Adding more token fixers Kun Qin
2022-07-31  5:37 ` Kun Qin [this message]
2022-08-08 13:05   ` [PATCH v3 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables Sami Mujawar
2022-08-08 15:39     ` Sami Mujawar
2022-08-08 23:43       ` Kun Qin
2022-08-09  9:01         ` Sami Mujawar
2022-08-10 22:31           ` Kun Qin
2022-07-31  5:37 ` [PATCH v3 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space Kun Qin
2022-08-08 13:22   ` [edk2-devel] " Sami Mujawar
2022-08-08 15:39     ` Sami Mujawar
2022-08-10  8:51       ` PierreGondois
2022-08-10 22:37         ` Kun Qin
2022-07-31  5:37 ` [PATCH v3 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config Kun Qin
2022-08-08 13:22   ` Sami Mujawar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220731053727.536-5-kuqin12@gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox