public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
@ 2021-09-25  3:59 Guo Dong
  2021-09-26  2:47 ` Ni, Ray
  0 siblings, 1 reply; 6+ messages in thread
From: Guo Dong @ 2021-09-25  3:59 UTC (permalink / raw)
  To: devel; +Cc: Guo Dong, Ray Ni, Maurice Ma, Benjamin You

From: Guo Dong <guo.dong@intel.com>

For universal UEFI payload, build a HOB from the ACPI table, so that
other modules could use this info from HOB at very early DXE phase.
This code are shared by universal payload and non universal payload.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Signed-off-by: Guo Dong <guo.dong@intel.com>
---
 UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c               | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c        | 192 ++++++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h        |  14 ++++++++++++++
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf      |   1 +
 UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c   |  12 ++++++++++++
 UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf |   2 +-
 6 files changed, 244 insertions(+), 179 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c b/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c
new file mode 100644
index 0000000000..6547fd8ab1
--- /dev/null
+++ b/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c
@@ -0,0 +1,202 @@
+/** @file
+
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "UefiPayloadEntry.h"
+
+
+/**
+  Find the board related info from ACPI table
+
+  @param  AcpiTableBase          ACPI table start address in memory
+  @param  AcpiBoardInfo          Pointer to the acpi board info strucutre
+
+  @retval RETURN_SUCCESS     Successfully find out all the required information.
+  @retval RETURN_NOT_FOUND   Failed to find the required info.
+
+**/
+RETURN_STATUS
+ParseAcpiInfo (
+  IN   UINT64                                   AcpiTableBase,
+  OUT  ACPI_BOARD_INFO                          *AcpiBoardInfo
+  )
+{
+  EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
+  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
+  UINT32                                        *Entry32;
+  UINTN                                         Entry32Num;
+  EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
+  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
+  UINT64                                        *Entry64;
+  UINTN                                         Entry64Num;
+  UINTN                                         Idx;
+  UINT32                                        *Signature;
+  EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *MmCfgHdr;
+  EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *MmCfgBase;
+
+  Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)(UINTN)AcpiTableBase;
+  DEBUG ((DEBUG_INFO, "Rsdp at 0x%p\n", Rsdp));
+  DEBUG ((DEBUG_INFO, "Rsdt at 0x%x, Xsdt at 0x%lx\n", Rsdp->RsdtAddress, Rsdp->XsdtAddress));
+
+  //
+  // Search Rsdt First
+  //
+  Fadt     = NULL;
+  MmCfgHdr = NULL;
+  Rsdt     = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)(Rsdp->RsdtAddress);
+  if (Rsdt != NULL) {
+    Entry32  = (UINT32 *)(Rsdt + 1);
+    Entry32Num = (Rsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) >> 2;
+    for (Idx = 0; Idx < Entry32Num; Idx++) {
+      Signature = (UINT32 *)(UINTN)Entry32[Idx];
+      if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+        Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
+        DEBUG ((DEBUG_INFO, "Found Fadt in Rsdt\n"));
+      }
+
+      if (*Signature == EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) {
+        MmCfgHdr = (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature;
+        DEBUG ((DEBUG_INFO, "Found MM config address in Rsdt\n"));
+      }
+
+      if ((Fadt != NULL) && (MmCfgHdr != NULL)) {
+        goto Done;
+      }
+    }
+  }
+
+  //
+  // Search Xsdt Second
+  //
+  Xsdt     = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)(Rsdp->XsdtAddress);
+  if (Xsdt != NULL) {
+    Entry64  = (UINT64 *)(Xsdt + 1);
+    Entry64Num = (Xsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) >> 3;
+    for (Idx = 0; Idx < Entry64Num; Idx++) {
+      Signature = (UINT32 *)(UINTN)Entry64[Idx];
+      if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+        Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
+        DEBUG ((DEBUG_INFO, "Found Fadt in Xsdt\n"));
+      }
+
+      if (*Signature == EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) {
+        MmCfgHdr = (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature;
+        DEBUG ((DEBUG_INFO, "Found MM config address in Xsdt\n"));
+      }
+
+      if ((Fadt != NULL) && (MmCfgHdr != NULL)) {
+        goto Done;
+      }
+    }
+  }
+
+  if (Fadt == NULL) {
+    return RETURN_NOT_FOUND;
+  }
+
+Done:
+
+  AcpiBoardInfo->PmCtrlRegBase   = Fadt->Pm1aCntBlk;
+  AcpiBoardInfo->PmTimerRegBase  = Fadt->PmTmrBlk;
+  AcpiBoardInfo->ResetRegAddress = Fadt->ResetReg.Address;
+  AcpiBoardInfo->ResetValue      = Fadt->ResetValue;
+  AcpiBoardInfo->PmEvtBase       = Fadt->Pm1aEvtBlk;
+  AcpiBoardInfo->PmGpeEnBase     = Fadt->Gpe0Blk + Fadt->Gpe0BlkLen / 2;
+
+  if (MmCfgHdr != NULL) {
+    MmCfgBase = (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *)((UINT8*) MmCfgHdr + sizeof (*MmCfgHdr));
+    AcpiBoardInfo->PcieBaseAddress = MmCfgBase->BaseAddress;
+    AcpiBoardInfo->PcieBaseSize = (MmCfgBase->EndBusNumber + 1 - MmCfgBase->StartBusNumber) * 4096 * 32 * 8;
+  } else {
+    AcpiBoardInfo->PcieBaseAddress = 0;
+    AcpiBoardInfo->PcieBaseSize = 0;
+  }
+  DEBUG ((DEBUG_INFO, "PmCtrl  Reg 0x%lx\n",  AcpiBoardInfo->PmCtrlRegBase));
+  DEBUG ((DEBUG_INFO, "PmTimer Reg 0x%lx\n",  AcpiBoardInfo->PmTimerRegBase));
+  DEBUG ((DEBUG_INFO, "Reset   Reg 0x%lx\n",  AcpiBoardInfo->ResetRegAddress));
+  DEBUG ((DEBUG_INFO, "Reset   Value 0x%x\n", AcpiBoardInfo->ResetValue));
+  DEBUG ((DEBUG_INFO, "PmEvt   Reg 0x%lx\n",  AcpiBoardInfo->PmEvtBase));
+  DEBUG ((DEBUG_INFO, "PmGpeEn Reg 0x%lx\n",  AcpiBoardInfo->PmGpeEnBase));
+  DEBUG ((DEBUG_INFO, "PcieBaseAddr 0x%lx\n", AcpiBoardInfo->PcieBaseAddress));
+  DEBUG ((DEBUG_INFO, "PcieBaseSize 0x%lx\n", AcpiBoardInfo->PcieBaseSize));
+
+  //
+  // Verify values for proper operation
+  //
+  ASSERT(Fadt->Pm1aCntBlk != 0);
+  ASSERT(Fadt->PmTmrBlk != 0);
+  ASSERT(Fadt->ResetReg.Address != 0);
+  ASSERT(Fadt->Pm1aEvtBlk != 0);
+  ASSERT(Fadt->Gpe0Blk != 0);
+
+  DEBUG_CODE_BEGIN ();
+    BOOLEAN    SciEnabled;
+
+    //
+    // Check the consistency of SCI enabling
+    //
+
+    //
+    // Get SCI_EN value
+    //
+   if (Fadt->Pm1CntLen == 4) {
+      SciEnabled = (IoRead32 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+    } else {
+      //
+      // if (Pm1CntLen == 2), use 16 bit IO read;
+      // if (Pm1CntLen != 2 && Pm1CntLen != 4), use 16 bit IO read as a fallback
+      //
+      SciEnabled = (IoRead16 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+    }
+
+    if (!(Fadt->Flags & EFI_ACPI_5_0_HW_REDUCED_ACPI) &&
+        (Fadt->SmiCmd == 0) &&
+       !SciEnabled) {
+      //
+      // The ACPI enabling status is inconsistent: SCI is not enabled but ACPI
+      // table does not provide a means to enable it through FADT->SmiCmd
+      //
+      DEBUG ((DEBUG_ERROR, "ERROR: The ACPI enabling status is inconsistent: SCI is not"
+        " enabled but the ACPI table does not provide a means to enable it through FADT->SmiCmd."
+        " This may cause issues in OS.\n"));
+    }
+  DEBUG_CODE_END ();
+
+  return RETURN_SUCCESS;
+}
+
+
+/**
+  Build ACPI board info HOB using infomation from ACPI table
+
+  @param  AcpiTableBase      ACPI table start address in memory
+
+  @retval RETURN_SUCCESS     Successfully build HOB from ACPI table.
+  @retval RETURN_NOT_FOUND   Failed to build the HOB.
+**/
+EFI_STATUS
+BuildHobFromAcpi (
+  IN   UINT64                           AcpiTableBase
+  )
+{
+  EFI_STATUS                            Status;
+  ACPI_BOARD_INFO                       AcpiBoardInfo;
+  ACPI_BOARD_INFO                       *NewAcpiBoardInfo;
+
+  Status = ParseAcpiInfo (AcpiTableBase, &AcpiBoardInfo);
+  ASSERT_EFI_ERROR (Status);
+  if (!EFI_ERROR (Status)) {
+    NewAcpiBoardInfo = BuildGuidHob (&gUefiAcpiBoardInfoGuid, sizeof (ACPI_BOARD_INFO));
+    ASSERT (NewAcpiBoardInfo != NULL);
+    CopyMem (NewAcpiBoardInfo, &AcpiBoardInfo, sizeof (ACPI_BOARD_INFO));
+    DEBUG ((DEBUG_INFO, "Create acpi board info guid hob\n"));
+  }
+  return Status;
+}
+
+
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c..d8e745bf32 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -7,7 +7,8 @@
 
 #include "UefiPayloadEntry.h"
 
-STATIC UINT32 mTopOfLowerUsableDram = 0;
+STATIC UINT32            mTopOfLowerUsableDram = 0;
+STATIC ACPI_BOARD_INFO   *mAcpiBoardInfo       = NULL;
 
 /**
    Callback function to build resource descriptor HOB
@@ -32,11 +33,16 @@ MemInfoCallbackMmio (
   EFI_RESOURCE_TYPE            Type;
   UINT64                       Size;
   EFI_RESOURCE_ATTRIBUTE_TYPE  Attribue;
-  ACPI_BOARD_INFO              *AcpiBoardInfo;
+  EFI_HOB_GUID_TYPE            *GuidHob;
 
-  AcpiBoardInfo = (ACPI_BOARD_INFO *)Params;
-  if (AcpiBoardInfo == NULL) {
-    return EFI_INVALID_PARAMETER;
+
+  //
+  // Find the acpi board information guid hob
+  //
+  if (mAcpiBoardInfo == NULL) {
+    GuidHob = GetFirstGuidHob (&gUefiAcpiBoardInfoGuid);
+    ASSERT (GuidHob != NULL);
+    mAcpiBoardInfo = (ACPI_BOARD_INFO *)GET_GUID_HOB_DATA (GuidHob);
   }
 
   //
@@ -46,7 +52,7 @@ MemInfoCallbackMmio (
     return EFI_SUCCESS;
   }
 
-  if (MemoryMapEntry->Base == AcpiBoardInfo->PcieBaseAddress) {
+  if (MemoryMapEntry->Base == mAcpiBoardInfo->PcieBaseAddress) {
     //
     // MMCONF is always MMIO
     //
@@ -211,168 +217,6 @@ MemInfoCallback (
 }
 
 
-
-/**
-  Find the board related info from ACPI table
-
-  @param  AcpiTableBase          ACPI table start address in memory
-  @param  AcpiBoardInfo          Pointer to the acpi board info strucutre
-
-  @retval RETURN_SUCCESS     Successfully find out all the required information.
-  @retval RETURN_NOT_FOUND   Failed to find the required info.
-
-**/
-RETURN_STATUS
-ParseAcpiInfo (
-  IN   UINT64                                   AcpiTableBase,
-  OUT  ACPI_BOARD_INFO                          *AcpiBoardInfo
-  )
-{
-  EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
-  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
-  UINT32                                        *Entry32;
-  UINTN                                         Entry32Num;
-  EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
-  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
-  UINT64                                        *Entry64;
-  UINTN                                         Entry64Num;
-  UINTN                                         Idx;
-  UINT32                                        *Signature;
-  EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *MmCfgHdr;
-  EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *MmCfgBase;
-
-  Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)(UINTN)AcpiTableBase;
-  DEBUG ((DEBUG_INFO, "Rsdp at 0x%p\n", Rsdp));
-  DEBUG ((DEBUG_INFO, "Rsdt at 0x%x, Xsdt at 0x%lx\n", Rsdp->RsdtAddress, Rsdp->XsdtAddress));
-
-  //
-  // Search Rsdt First
-  //
-  Fadt     = NULL;
-  MmCfgHdr = NULL;
-  Rsdt     = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)(Rsdp->RsdtAddress);
-  if (Rsdt != NULL) {
-    Entry32  = (UINT32 *)(Rsdt + 1);
-    Entry32Num = (Rsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) >> 2;
-    for (Idx = 0; Idx < Entry32Num; Idx++) {
-      Signature = (UINT32 *)(UINTN)Entry32[Idx];
-      if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
-        Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
-        DEBUG ((DEBUG_INFO, "Found Fadt in Rsdt\n"));
-      }
-
-      if (*Signature == EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) {
-        MmCfgHdr = (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature;
-        DEBUG ((DEBUG_INFO, "Found MM config address in Rsdt\n"));
-      }
-
-      if ((Fadt != NULL) && (MmCfgHdr != NULL)) {
-        goto Done;
-      }
-    }
-  }
-
-  //
-  // Search Xsdt Second
-  //
-  Xsdt     = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)(Rsdp->XsdtAddress);
-  if (Xsdt != NULL) {
-    Entry64  = (UINT64 *)(Xsdt + 1);
-    Entry64Num = (Xsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) >> 3;
-    for (Idx = 0; Idx < Entry64Num; Idx++) {
-      Signature = (UINT32 *)(UINTN)Entry64[Idx];
-      if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
-        Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
-        DEBUG ((DEBUG_INFO, "Found Fadt in Xsdt\n"));
-      }
-
-      if (*Signature == EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) {
-        MmCfgHdr = (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature;
-        DEBUG ((DEBUG_INFO, "Found MM config address in Xsdt\n"));
-      }
-
-      if ((Fadt != NULL) && (MmCfgHdr != NULL)) {
-        goto Done;
-      }
-    }
-  }
-
-  if (Fadt == NULL) {
-    return RETURN_NOT_FOUND;
-  }
-
-Done:
-
-  AcpiBoardInfo->PmCtrlRegBase   = Fadt->Pm1aCntBlk;
-  AcpiBoardInfo->PmTimerRegBase  = Fadt->PmTmrBlk;
-  AcpiBoardInfo->ResetRegAddress = Fadt->ResetReg.Address;
-  AcpiBoardInfo->ResetValue      = Fadt->ResetValue;
-  AcpiBoardInfo->PmEvtBase       = Fadt->Pm1aEvtBlk;
-  AcpiBoardInfo->PmGpeEnBase     = Fadt->Gpe0Blk + Fadt->Gpe0BlkLen / 2;
-
-  if (MmCfgHdr != NULL) {
-    MmCfgBase = (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *)((UINT8*) MmCfgHdr + sizeof (*MmCfgHdr));
-    AcpiBoardInfo->PcieBaseAddress = MmCfgBase->BaseAddress;
-    AcpiBoardInfo->PcieBaseSize = (MmCfgBase->EndBusNumber + 1 - MmCfgBase->StartBusNumber) * 4096 * 32 * 8;
-  } else {
-    AcpiBoardInfo->PcieBaseAddress = 0;
-    AcpiBoardInfo->PcieBaseSize = 0;
-  }
-  DEBUG ((DEBUG_INFO, "PmCtrl  Reg 0x%lx\n",  AcpiBoardInfo->PmCtrlRegBase));
-  DEBUG ((DEBUG_INFO, "PmTimer Reg 0x%lx\n",  AcpiBoardInfo->PmTimerRegBase));
-  DEBUG ((DEBUG_INFO, "Reset   Reg 0x%lx\n",  AcpiBoardInfo->ResetRegAddress));
-  DEBUG ((DEBUG_INFO, "Reset   Value 0x%x\n", AcpiBoardInfo->ResetValue));
-  DEBUG ((DEBUG_INFO, "PmEvt   Reg 0x%lx\n",  AcpiBoardInfo->PmEvtBase));
-  DEBUG ((DEBUG_INFO, "PmGpeEn Reg 0x%lx\n",  AcpiBoardInfo->PmGpeEnBase));
-  DEBUG ((DEBUG_INFO, "PcieBaseAddr 0x%lx\n", AcpiBoardInfo->PcieBaseAddress));
-  DEBUG ((DEBUG_INFO, "PcieBaseSize 0x%lx\n", AcpiBoardInfo->PcieBaseSize));
-
-  //
-  // Verify values for proper operation
-  //
-  ASSERT(Fadt->Pm1aCntBlk != 0);
-  ASSERT(Fadt->PmTmrBlk != 0);
-  ASSERT(Fadt->ResetReg.Address != 0);
-  ASSERT(Fadt->Pm1aEvtBlk != 0);
-  ASSERT(Fadt->Gpe0Blk != 0);
-
-  DEBUG_CODE_BEGIN ();
-    BOOLEAN    SciEnabled;
-
-    //
-    // Check the consistency of SCI enabling
-    //
-
-    //
-    // Get SCI_EN value
-    //
-   if (Fadt->Pm1CntLen == 4) {
-      SciEnabled = (IoRead32 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
-    } else {
-      //
-      // if (Pm1CntLen == 2), use 16 bit IO read;
-      // if (Pm1CntLen != 2 && Pm1CntLen != 4), use 16 bit IO read as a fallback
-      //
-      SciEnabled = (IoRead16 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
-    }
-
-    if (!(Fadt->Flags & EFI_ACPI_5_0_HW_REDUCED_ACPI) &&
-        (Fadt->SmiCmd == 0) &&
-       !SciEnabled) {
-      //
-      // The ACPI enabling status is inconsistent: SCI is not enabled but ACPI
-      // table does not provide a means to enable it through FADT->SmiCmd
-      //
-      DEBUG ((DEBUG_ERROR, "ERROR: The ACPI enabling status is inconsistent: SCI is not"
-        " enabled but the ACPI table does not provide a means to enable it through FADT->SmiCmd."
-        " This may cause issues in OS.\n"));
-    }
-  DEBUG_CODE_END ();
-
-  return RETURN_SUCCESS;
-}
-
-
 /**
   It will build HOBs based on information from bootloaders.
 
@@ -387,8 +231,6 @@ BuildHobFromBl (
   EFI_STATUS                       Status;
   SYSTEM_TABLE_INFO                SysTableInfo;
   SYSTEM_TABLE_INFO                *NewSysTableInfo;
-  ACPI_BOARD_INFO                  AcpiBoardInfo;
-  ACPI_BOARD_INFO                  *NewAcpiBoardInfo;
   EFI_PEI_GRAPHICS_INFO_HOB        GfxInfo;
   EFI_PEI_GRAPHICS_INFO_HOB        *NewGfxInfo;
   EFI_PEI_GRAPHICS_DEVICE_INFO_HOB GfxDeviceInfo;
@@ -471,20 +313,14 @@ BuildHobFromBl (
   //
   // Create guid hob for acpi board information
   //
-  Status = ParseAcpiInfo (SysTableInfo.AcpiTableBase, &AcpiBoardInfo);
+  Status = BuildHobFromAcpi (SysTableInfo.AcpiTableBase);
   ASSERT_EFI_ERROR (Status);
-  if (!EFI_ERROR (Status)) {
-    NewAcpiBoardInfo = BuildGuidHob (&gUefiAcpiBoardInfoGuid, sizeof (ACPI_BOARD_INFO));
-    ASSERT (NewAcpiBoardInfo != NULL);
-    CopyMem (NewAcpiBoardInfo, &AcpiBoardInfo, sizeof (ACPI_BOARD_INFO));
-    DEBUG ((DEBUG_INFO, "Create acpi board info guid hob\n"));
-  }
 
   //
   // Parse memory info and build memory HOBs for reserved DRAM and MMIO
   //
   DEBUG ((DEBUG_INFO , "Building ResourceDescriptorHobs for reserved memory:\n"));
-  Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo);
+  Status = ParseMemoryInfo (MemInfoCallbackMmio, NULL);
   if (EFI_ERROR(Status)) {
     return Status;
   }
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
index 331724c687..00e9e34933 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
@@ -204,4 +204,18 @@ FvFindFileByTypeGuid (
   IN  EFI_GUID                    *Guid           OPTIONAL,
   OUT EFI_FFS_FILE_HEADER         **FileHeader
   );
+
+/**
+  Build ACPI board info HOB using infomation from ACPI table
+
+  @param  AcpiTableBase      ACPI table start address in memory
+
+  @retval RETURN_SUCCESS     Successfully build HOB from ACPI table.
+  @retval RETURN_NOT_FOUND   Failed to build the HOB.
+**/
+EFI_STATUS
+BuildHobFromAcpi (
+  IN   UINT64                           AcpiTableBase
+  );
+
 #endif
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
index 8d42925fcd..4c5170d9cc 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
@@ -25,6 +25,7 @@
   UefiPayloadEntry.c
   LoadDxeCore.c
   MemoryAllocation.c
+  AcpiTable.c
 
 [Sources.Ia32]
   X64/VirtualMemory.h
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
index 03ad9c457b..b64b6529c3 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
@@ -260,6 +260,8 @@ BuildHobs (
   UNIVERSAL_PAYLOAD_EXTRA_DATA     *ExtraData;
   UINT8                            *GuidHob;
   EFI_HOB_FIRMWARE_VOLUME          *FvHob;
+  EFI_STATUS                       Status;
+  UNIVERSAL_PAYLOAD_ACPI_TABLE     *AcpiTable;
 
   Hob.Raw = (UINT8 *) BootloaderParameter;
   MinimalNeededSize = FixedPcdGet32 (PcdSystemMemoryUefiRegionSize);
@@ -351,6 +353,16 @@ BuildHobs (
   *DxeFv = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) ExtraData->Entry[0].Base;
   ASSERT ((*DxeFv)->FvLength == ExtraData->Entry[0].Size);
 
+  //
+  // Create guid hob for acpi board information
+  //
+  GuidHob = GetFirstGuidHob(&gUniversalPayloadAcpiTableGuid);
+  if (GuidHob != NULL) {
+    AcpiTable = (UNIVERSAL_PAYLOAD_ACPI_TABLE *) GET_GUID_HOB_DATA (GuidHob);
+    Status = BuildHobFromAcpi ((UINT64)AcpiTable->Rsdp);
+    ASSERT_EFI_ERROR (Status);
+  }
+
   //
   // Update DXE FV information to first fv hob in the hob list, which
   // is the empty FvHob created before.
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
index 3ee449219d..e7e05b744a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
@@ -25,6 +25,7 @@
   LoadDxeCore.c
   MemoryAllocation.c
   PrintHob.c
+  AcpiTable.c
 
 [Sources.Ia32]
   X64/VirtualMemory.h
@@ -61,7 +62,6 @@
   gEfiGraphicsDeviceInfoHobGuid
   gUefiAcpiBoardInfoGuid
   gEfiSmbiosTableGuid
-  gEfiAcpiTableGuid
   gUefiSerialPortInfoGuid
   gUniversalPayloadExtraDataGuid
   gPcdDataBaseHobGuid
-- 
2.32.0.windows.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
  2021-09-25  3:59 [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table Guo Dong
@ 2021-09-26  2:47 ` Ni, Ray
  2021-09-26 18:32   ` Guo Dong
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ray @ 2021-09-26  2:47 UTC (permalink / raw)
  To: Dong, Guo, devel@edk2.groups.io; +Cc: Ma, Maurice, You, Benjamin


-  Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo);

+  Status = ParseMemoryInfo (MemInfoCallbackMmio, NULL);

Guo,
I am curious why you changed this part.
Without this change, MemInfoCallbackMmio() can get the AcpiBoardInfo from the parameter.
With the change, it has to locate the Guided HOB itself.


Other parts look good to me.

Thanks,
Ray


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
  2021-09-26  2:47 ` Ni, Ray
@ 2021-09-26 18:32   ` Guo Dong
  2021-09-28  2:10     ` Ni, Ray
  0 siblings, 1 reply; 6+ messages in thread
From: Guo Dong @ 2021-09-26 18:32 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io; +Cc: Ma, Maurice, You, Benjamin


Hi Ray,

In this patch, we added a shared file AcpiTable.c for both universal payload and non-universal payload.
The exposed API from this file is:   EFI_STATUS  BuildHobFromAcpi ( IN   UINT64 AcpiTableBase);
This function will build an ACPI board HOB based on the information from ACPI table.

For universal payload, it calls this function to build a hob for other modules. The main function is very simple and clear.

For non-universal payload, ACPI board HOB is used in the ParseMemoryInfo() callback for PCIE base info.
So we could get this HOB from the caller, or get this HOB inside the callback. I select to do it inside the callback.

Thanks,
Guo

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Saturday, September 25, 2021 7:48 PM
To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table


-  Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo);

+  Status = ParseMemoryInfo (MemInfoCallbackMmio, NULL);

Guo,
I am curious why you changed this part.
Without this change, MemInfoCallbackMmio() can get the AcpiBoardInfo from the parameter.
With the change, it has to locate the Guided HOB itself.


Other parts look good to me.

Thanks,
Ray


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
  2021-09-26 18:32   ` Guo Dong
@ 2021-09-28  2:10     ` Ni, Ray
  2021-09-28 17:19       ` Guo Dong
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ray @ 2021-09-28  2:10 UTC (permalink / raw)
  To: Dong, Guo, devel@edk2.groups.io; +Cc: Ma, Maurice, You, Benjamin

I prefer we still let caller to provide the HOB pointer.
This also eliminates a global variable "mAcpiBoardInfo".

You could change the BuildHobFromAcpi() to return the HOB pointer.
So that the pointer can be directly passed to ParseMemoryInfo().

Thanks,
Ray

> -----Original Message-----
> From: Dong, Guo <guo.dong@intel.com>
> Sent: Monday, September 27, 2021 2:32 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> Hi Ray,
> 
> In this patch, we added a shared file AcpiTable.c for both universal payload and non-universal payload.
> The exposed API from this file is:   EFI_STATUS  BuildHobFromAcpi ( IN   UINT64 AcpiTableBase);
> This function will build an ACPI board HOB based on the information from ACPI table.
> 
> For universal payload, it calls this function to build a hob for other modules. The main function is very simple and clear.
> 
> For non-universal payload, ACPI board HOB is used in the ParseMemoryInfo() callback for PCIE base info.
> So we could get this HOB from the caller, or get this HOB inside the callback. I select to do it inside the callback.
> 
> Thanks,
> Guo
> 
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Saturday, September 25, 2021 7:48 PM
> To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> -  Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo);
> 
> +  Status = ParseMemoryInfo (MemInfoCallbackMmio, NULL);
> 
> Guo,
> I am curious why you changed this part.
> Without this change, MemInfoCallbackMmio() can get the AcpiBoardInfo from the parameter.
> With the change, it has to locate the Guided HOB itself.
> 
> 
> Other parts look good to me.
> 
> Thanks,
> Ray


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
  2021-09-28  2:10     ` Ni, Ray
@ 2021-09-28 17:19       ` Guo Dong
  2021-09-29  2:41         ` Ni, Ray
  0 siblings, 1 reply; 6+ messages in thread
From: Guo Dong @ 2021-09-28 17:19 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io; +Cc: Ma, Maurice, You, Benjamin


Sure. I updated the patch per comments.

Thanks,
Guo

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, September 27, 2021 7:11 PM
To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table

I prefer we still let caller to provide the HOB pointer.
This also eliminates a global variable "mAcpiBoardInfo".

You could change the BuildHobFromAcpi() to return the HOB pointer.
So that the pointer can be directly passed to ParseMemoryInfo().

Thanks,
Ray

> -----Original Message-----
> From: Dong, Guo <guo.dong@intel.com>
> Sent: Monday, September 27, 2021 2:32 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> Hi Ray,
> 
> In this patch, we added a shared file AcpiTable.c for both universal payload and non-universal payload.
> The exposed API from this file is:   EFI_STATUS  BuildHobFromAcpi ( IN   UINT64 AcpiTableBase);
> This function will build an ACPI board HOB based on the information from ACPI table.
> 
> For universal payload, it calls this function to build a hob for other modules. The main function is very simple and clear.
> 
> For non-universal payload, ACPI board HOB is used in the ParseMemoryInfo() callback for PCIE base info.
> So we could get this HOB from the caller, or get this HOB inside the callback. I select to do it inside the callback.
> 
> Thanks,
> Guo
> 
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Saturday, September 25, 2021 7:48 PM
> To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> -  Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo);
> 
> +  Status = ParseMemoryInfo (MemInfoCallbackMmio, NULL);
> 
> Guo,
> I am curious why you changed this part.
> Without this change, MemInfoCallbackMmio() can get the AcpiBoardInfo from the parameter.
> With the change, it has to locate the Guided HOB itself.
> 
> 
> Other parts look good to me.
> 
> Thanks,
> Ray


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
  2021-09-28 17:19       ` Guo Dong
@ 2021-09-29  2:41         ` Ni, Ray
  0 siblings, 0 replies; 6+ messages in thread
From: Ni, Ray @ 2021-09-29  2:41 UTC (permalink / raw)
  To: Dong, Guo, devel@edk2.groups.io; +Cc: Ma, Maurice, You, Benjamin

Guo,
Thank you!:)

-----Original Message-----
From: Dong, Guo <guo.dong@intel.com> 
Sent: Wednesday, September 29, 2021 1:20 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table


Sure. I updated the patch per comments.

Thanks,
Guo

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, September 27, 2021 7:11 PM
To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table

I prefer we still let caller to provide the HOB pointer.
This also eliminates a global variable "mAcpiBoardInfo".

You could change the BuildHobFromAcpi() to return the HOB pointer.
So that the pointer can be directly passed to ParseMemoryInfo().

Thanks,
Ray

> -----Original Message-----
> From: Dong, Guo <guo.dong@intel.com>
> Sent: Monday, September 27, 2021 2:32 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> Hi Ray,
> 
> In this patch, we added a shared file AcpiTable.c for both universal payload and non-universal payload.
> The exposed API from this file is:   EFI_STATUS  BuildHobFromAcpi ( IN   UINT64 AcpiTableBase);
> This function will build an ACPI board HOB based on the information from ACPI table.
> 
> For universal payload, it calls this function to build a hob for other modules. The main function is very simple and clear.
> 
> For non-universal payload, ACPI board HOB is used in the ParseMemoryInfo() callback for PCIE base info.
> So we could get this HOB from the caller, or get this HOB inside the callback. I select to do it inside the callback.
> 
> Thanks,
> Guo
> 
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Saturday, September 25, 2021 7:48 PM
> To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> -  Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo);
> 
> +  Status = ParseMemoryInfo (MemInfoCallbackMmio, NULL);
> 
> Guo,
> I am curious why you changed this part.
> Without this change, MemInfoCallbackMmio() can get the AcpiBoardInfo from the parameter.
> With the change, it has to locate the Guided HOB itself.
> 
> 
> Other parts look good to me.
> 
> Thanks,
> Ray


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-29  2:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-25  3:59 [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table Guo Dong
2021-09-26  2:47 ` Ni, Ray
2021-09-26 18:32   ` Guo Dong
2021-09-28  2:10     ` Ni, Ray
2021-09-28 17:19       ` Guo Dong
2021-09-29  2:41         ` Ni, Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox