[PATCH] CorebootModulePkg/CbSupportDxe: Enable ACPI via cmd register

[Matt Delco <delco@google.com>]

The existing code sets the SCI_EN bit directly, which is a violation
of the ACPI spec ("It is the responsibility of the hardware to set or
reset this bit. OSPM always preserves this bit position"). The proper
way to cause this bit to bit set is to reference the FADT table and
write the value of ACPI_ENABLE to SMI_CMD.  The SMI will in turn set
the SCI_EN bit.

Prior to this change ACPI events were not being delivered because
ACPI was not properly enabled and the OS also did not attempt
to enable ACPI since it sees that SCI_EN is already set.  After this
change I observed that ACPI events were now being delivered properly.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Matt Delco <delco@google.com>
---
 CorebootModulePkg/CbSupportDxe/CbSupportDxe.c | 79 ++++++++++++++++---
 .../CbSupportDxe/CbSupportDxe.inf             |  1 +
 2 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
index c526c9e871..4c7917ff2a 100755
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
@@ -14,7 +14,10 @@
 **/
 #include "CbSupportDxe.h"

-UINTN mPmCtrlReg = 0;
+BOOLEAN mSmiPortFound = FALSE;
+UINTN mSmiCommandPort = 0;
+UINT8 mAcpiEnable = 0;
+
 /**
   Reserve MMIO/IO resource in GCD

@@ -107,9 +110,63 @@ OnReadyToBoot (
   //
   // Enable SCI
   //
-  IoOr16 (mPmCtrlReg, BIT0);
+  if (!mSmiPortFound) {
+    DEBUG ((DEBUG_ERROR, "SMI port not known so can not enable SCI\n"));
+  } else {
+    IoWrite8 (mSmiCommandPort, mAcpiEnable);
+    DEBUG ((DEBUG_ERROR, "Enable ACPI at 0x%lx with 0x%x before boot\n",
(UINT64)mSmiCommandPort, (UINT32)mAcpiEnable));
+  }
+}
+
+/**
+  Lookup port and value for enabling ACPI
+
+  @param[in] SystemTable    A pointer to the EFI System Table.

-  DEBUG ((EFI_D_ERROR, "Enable SCI bit at 0x%lx before boot\n",
(UINT64)mPmCtrlReg));
+**/
+VOID
+FindAcpiFadtTableInfo (
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
+  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
+  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
+  UINTN                                         Index;
+  Rsdp  = NULL;
+  Rsdt  = NULL;
+  Fadt  = NULL;
+
+  for (Index = 0; Index < SystemTable->NumberOfTableEntries; Index++) {
+    if (CompareGuid (&(SystemTable->ConfigurationTable[Index].VendorGuid),
&gEfiAcpi20TableGuid)) {
+      Rsdp = SystemTable->ConfigurationTable[Index].VendorTable;
+      break;
+    }
+  }
+  if (Rsdp == NULL) {
+    return;
+  }
+
+  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
+  if (Rsdt == NULL || Rsdt->Signature !=
EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
+    return;
+  }
+  for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < Rsdt->Length;
Index = Index + sizeof (UINT32)) {
+    UINT32 Data32  = *(UINT32 *) ((UINT8 *) Rsdt + Index);
+    Fadt    = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) (UINT32 *)
(UINTN) Data32;
+    if (Fadt->Header.Signature ==
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+      break;
+    }
+  }
+
+  if (Fadt == NULL || Fadt->Header.Signature !=
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+    return;
+  }
+
+  mSmiPortFound = TRUE;
+  mSmiCommandPort = Fadt->SmiCmd;
+  mAcpiEnable = Fadt->AcpiEnable;
+  return;
 }

 /**
@@ -133,7 +190,6 @@ CbDxeEntryPoint (
   EFI_EVENT  ReadyToBootEvent;
   EFI_HOB_GUID_TYPE  *GuidHob;
   SYSTEM_TABLE_INFO  *pSystemTableInfo;
-  ACPI_BOARD_INFO    *pAcpiBoardInfo;
   FRAME_BUFFER_INFO  *FbInfo;

   Status = EFI_SUCCESS;
@@ -172,14 +228,14 @@ CbDxeEntryPoint (
   }

   //
-  // Find the acpi board information guid hob
+  // Find the SMI port in the FADT table in acpi
   //
-  GuidHob = GetFirstGuidHob (&gUefiAcpiBoardInfoGuid);
-  ASSERT (GuidHob != NULL);
-  pAcpiBoardInfo = (ACPI_BOARD_INFO *)GET_GUID_HOB_DATA (GuidHob);
-
-  mPmCtrlReg = (UINTN)pAcpiBoardInfo->PmCtrlRegBase;
-  DEBUG ((EFI_D_ERROR, "PmCtrlReg at 0x%lx\n", (UINT64)mPmCtrlReg));
+  FindAcpiFadtTableInfo(SystemTable);
+  if (mSmiPortFound) {
+    DEBUG ((DEBUG_ERROR, "Found ACPI enable info in FADT: 0x%lx 0x%x\n",
(UINT64)mSmiCommandPort, (UINT32)mAcpiEnable));
+  } else {
+    DEBUG ((DEBUG_ERROR, "Failed to find FADT info for enabling ACPI\n"));
+  }

   //
   // Find the frame buffer information and update PCDs
@@ -212,4 +268,3 @@ CbDxeEntryPoint (

   return EFI_SUCCESS;
 }
-
diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
index 99245183ea..e334dafdb7 100644
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
@@ -51,6 +51,7 @@

 [Guids]
   gEfiAcpiTableGuid
+  gEfiAcpi20TableGuid
   gEfiSmbiosTableGuid
   gUefiSystemTableInfoGuid
   gUefiAcpiBoardInfoGuid
-- 
2.17.0.921.gf22659ad46-goog