public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode
@ 2019-11-29 10:47 Ard Biesheuvel
  2019-11-29 10:47 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2019-11-29 10:47 UTC (permalink / raw)
  To: devel; +Cc: leif.lindholm, Ard Biesheuvel

Refactor the platform DXE a bit in patch #1 so we can seamlessly drop in
the code in patch #2 to expose a SSDT with a device node describing
OP-TEE when booting in ACPI mode and OP-TEE is present.

Ard Biesheuvel (2):
  Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine
  Silicon/SynQuacer/PlatformDxe: add ACPI device node for OP-TEE if
    present

 .../SynQuacerEvalBoard/SynQuacerEvalBoard.dsc |  1 +
 .../SynQuacer/Drivers/PlatformDxe/Emmc.c      | 55 ------------
 .../SynQuacer/Drivers/PlatformDxe/Optee.asl   | 23 +++++
 .../Drivers/PlatformDxe/PlatformDxe.c         | 83 +++++++++++++++++++
 .../Drivers/PlatformDxe/PlatformDxe.h         |  2 +
 .../Drivers/PlatformDxe/PlatformDxe.inf       |  2 +
 6 files changed, 111 insertions(+), 55 deletions(-)
 create mode 100644 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Optee.asl

-- 
2.17.1


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

* [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine
  2019-11-29 10:47 [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Ard Biesheuvel
@ 2019-11-29 10:47 ` Ard Biesheuvel
  2019-12-02 10:26   ` [edk2-devel] " Philippe Mathieu-Daudé
  2019-11-29 10:47 ` [PATCH edk2-platforms 2/2] Silicon/SynQuacer/PlatformDxe: add ACPI device node for OP-TEE if present Ard Biesheuvel
  2019-11-29 11:29 ` [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Leif Lindholm
  2 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2019-11-29 10:47 UTC (permalink / raw)
  To: devel; +Cc: leif.lindholm, Ard Biesheuvel

In preparation of adding support for describing the presence of OP-TEE
via a SSDT ACPI table, refactor the existing code so we will be able to
reuse it more easily.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c        | 55 -----------------
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 65 ++++++++++++++++++++
 2 files changed, 65 insertions(+), 55 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
index 0d0e5edad901..90b67152c768 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
@@ -53,10 +53,6 @@
 
 STATIC EFI_HANDLE mSdMmcControllerHandle;
 
-STATIC EFI_ACPI_DESCRIPTION_HEADER      *mSsdt;
-STATIC UINTN                            mSsdtSize;
-STATIC VOID                             *mEventRegistration;
-
 /**
 
   Override function for SDHCI capability bits
@@ -185,31 +181,6 @@ STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride = {
   SynQuacerSdMmcNotifyPhase,
 };
 
-STATIC
-VOID
-EFIAPI
-InstallAcpiTable (
-  IN EFI_EVENT                      Event,
-  IN VOID*                          Context
-  )
-{
-  UINTN                             TableKey;
-  EFI_STATUS                        Status;
-  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
-
-  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
-                  (VOID **)&AcpiTable);
-  if (EFI_ERROR (Status)) {
-    return;
-  }
-
-  Status = AcpiTable->InstallAcpiTable (AcpiTable, mSsdt, mSsdtSize, &TableKey);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
-      __FUNCTION__, Status));
-  }
-}
-
 EFI_STATUS
 EFIAPI
 RegisterEmmc (
@@ -218,32 +189,6 @@ RegisterEmmc (
 {
   EFI_STATUS                      Status;
   EFI_HANDLE                      Handle;
-  UINTN                           Index;
-
-  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
-    //
-    // Load the SSDT table from a raw section in this FFS file.
-    //
-    for (Index = 0;; Index++) {
-      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
-                 (VOID **)&mSsdt, &mSsdtSize);
-      if (EFI_ERROR (Status)) {
-        break;
-      }
-
-      if (mSsdt->OemTableId != EMMC_TABLE_ID) {
-        continue;
-      }
-
-      //
-      // Register for the ACPI table protocol
-      //
-      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
-        InstallAcpiTable, NULL, &mEventRegistration);
-
-      break;
-    }
-  }
 
   Status = RegisterNonDiscoverableMmioDevice (
              NonDiscoverableDeviceTypeSdhci,
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
index 73cc560fa8d8..c9cc37dd2478 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
@@ -113,6 +113,11 @@ STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mI2c1Desc[] = {
   }
 };
 
+STATIC EFI_ACPI_DESCRIPTION_HEADER      *mEmmcSsdt;
+STATIC UINTN                            mEmmcSsdtSize;
+
+STATIC VOID                             *mAcpiTableEventRegistration;
+
 STATIC
 EFI_STATUS
 RegisterDevice (
@@ -256,6 +261,32 @@ EnableSettingsForm (
   return InstallHiiPages ();
 }
 
+STATIC
+VOID
+EFIAPI
+InstallAcpiTables (
+  IN EFI_EVENT                      Event,
+  IN VOID*                          Context
+  )
+{
+  UINTN                             TableKey;
+  EFI_STATUS                        Status;
+  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
+
+  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
+                  (VOID **)&AcpiTable);
+  if (EFI_ERROR (Status)) {
+    return;
+  }
+
+  Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
+                        &TableKey);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
+      __FUNCTION__, Status));
+  }
+}
+
 EFI_STATUS
 EFIAPI
 PlatformDxeEntryPoint (
@@ -267,6 +298,9 @@ PlatformDxeEntryPoint (
   VOID                            *Dtb;
   UINTN                           DtbSize;
   EFI_HANDLE                      Handle;
+  EFI_ACPI_DESCRIPTION_HEADER     *Ssdt;
+  UINTN                           SsdtSize;
+  UINTN                           Index;
 
   mHiiSettingsVal = PcdGet64 (PcdPlatformSettings);
   mHiiSettings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&mHiiSettingsVal;
@@ -344,5 +378,36 @@ PlatformDxeEntryPoint (
     ASSERT_EFI_ERROR (Status);
   }
 
+  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
+    //
+    // Load the SSDT tables from a raw section in this FFS file.
+    //
+    for (Index = 0;; Index++) {
+      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
+                 (VOID **)&Ssdt, &SsdtSize);
+      if (EFI_ERROR (Status)) {
+        break;
+      }
+
+      switch (Ssdt->OemTableId) {
+      case EMMC_TABLE_ID:
+        if (mHiiSettings->EnableEmmc != EMMC_ENABLED) {
+          break;
+        }
+        mEmmcSsdt = Ssdt;
+        mEmmcSsdtSize = SsdtSize;
+        break;
+      }
+    }
+
+    if (mEmmcSsdtSize > 0) {
+      //
+      // Register for the ACPI table protocol if we found any SSDTs to install
+      //
+      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
+        InstallAcpiTables, NULL, &mAcpiTableEventRegistration);
+    }
+  }
+
   return EFI_SUCCESS;
 }
-- 
2.17.1


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

* [PATCH edk2-platforms 2/2] Silicon/SynQuacer/PlatformDxe: add ACPI device node for OP-TEE if present
  2019-11-29 10:47 [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Ard Biesheuvel
  2019-11-29 10:47 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine Ard Biesheuvel
@ 2019-11-29 10:47 ` Ard Biesheuvel
  2019-11-29 11:29 ` [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Leif Lindholm
  2 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2019-11-29 10:47 UTC (permalink / raw)
  To: devel; +Cc: leif.lindholm, Ard Biesheuvel

In order to allow the OS to discover the presence of OP-TEE also when
booting via ACPI, expose a PRP0001 ACPI device node in this case with
OP-TEE's DT compatible string attached.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc    |  1 +
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Optee.asl       | 23 ++++++++++++++++++++
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c   | 20 ++++++++++++++++-
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h   |  2 ++
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf |  2 ++
 5 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
index ab1ab6f2de60..968378d5ee5b 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
+++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
@@ -68,6 +68,7 @@ [LibraryClasses.common]
   ArmPlatformStackLib|ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
   ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
   ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
+  OpteeLib|ArmPkg/Library/OpteeLib/OpteeLib.inf
 
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Optee.asl b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Optee.asl
new file mode 100644
index 000000000000..cd506211ab1e
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Optee.asl
@@ -0,0 +1,23 @@
+/** @file
+  Copyright (c) 2019, Linaro Ltd. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+DefinitionBlock ("SsdtTOS0.aml", "SSDT", 1, "SNI", "SynQTOS0",
+                 FixedPcdGet32 (PcdAcpiDefaultOemRevision)) {
+  Scope (_SB) {
+    Device (TOS0) {
+      Name (_HID, "PRP0001")
+      Name (_UID, 0x0)
+      Name (_DSD, Package () {
+        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+        Package () {
+          Package (2) { "compatible", "linaro,optee-tz" },
+          Package (2) { "method", "smc" },
+        }
+      })
+    }
+  } // Scope (_SB)
+}
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
index c9cc37dd2478..138030135986 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
@@ -116,6 +116,9 @@ STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mI2c1Desc[] = {
 STATIC EFI_ACPI_DESCRIPTION_HEADER      *mEmmcSsdt;
 STATIC UINTN                            mEmmcSsdtSize;
 
+STATIC EFI_ACPI_DESCRIPTION_HEADER      *mTos0Ssdt;
+STATIC UINTN                            mTos0SsdtSize;
+
 STATIC VOID                             *mAcpiTableEventRegistration;
 
 STATIC
@@ -285,6 +288,13 @@ InstallAcpiTables (
     DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
       __FUNCTION__, Status));
   }
+
+  Status = AcpiTable->InstallAcpiTable (AcpiTable, mTos0Ssdt, mTos0SsdtSize,
+                        &TableKey);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for OP-TEE - %r\n",
+      __FUNCTION__, Status));
+  }
 }
 
 EFI_STATUS
@@ -397,10 +407,18 @@ PlatformDxeEntryPoint (
         mEmmcSsdt = Ssdt;
         mEmmcSsdtSize = SsdtSize;
         break;
+
+      case TOS0_TABLE_ID:
+        if (!IsOpteePresent ()) {
+          break;
+        }
+        mTos0Ssdt = Ssdt;
+        mTos0SsdtSize = SsdtSize;
+        break;
       }
     }
 
-    if (mEmmcSsdtSize > 0) {
+    if (mEmmcSsdtSize > 0 || mTos0SsdtSize > 0) {
       //
       // Register for the ACPI table protocol if we found any SSDTs to install
       //
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
index c08659f7a796..692654687869 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
@@ -23,6 +23,7 @@
 #include <Library/IoLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/NonDiscoverableDeviceRegistrationLib.h>
+#include <Library/OpteeLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
@@ -36,6 +37,7 @@
 #include <Protocol/SdMmcOverride.h>
 
 #define EMMC_TABLE_ID     SIGNATURE_64('S','y','n','Q','e','M','M','C')
+#define TOS0_TABLE_ID     SIGNATURE_64('S','y','n','Q','T','O','S','0')
 
 extern UINT8                             PlatformDxeHiiBin[];
 extern UINT8                             PlatformDxeStrings[];
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
index ae9f8712f0d2..57f2d071c14e 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
@@ -19,6 +19,7 @@ [Defines]
 [Sources]
   Emmc.asl
   Emmc.c
+  Optee.asl
   Pci.c
   PlatformDxe.c
   PlatformDxeHii.uni
@@ -46,6 +47,7 @@ [LibraryClasses]
   IoLib
   MemoryAllocationLib
   NonDiscoverableDeviceRegistrationLib
+  OpteeLib
   PcdLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
-- 
2.17.1


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

* Re: [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode
  2019-11-29 10:47 [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Ard Biesheuvel
  2019-11-29 10:47 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine Ard Biesheuvel
  2019-11-29 10:47 ` [PATCH edk2-platforms 2/2] Silicon/SynQuacer/PlatformDxe: add ACPI device node for OP-TEE if present Ard Biesheuvel
@ 2019-11-29 11:29 ` Leif Lindholm
  2019-11-29 12:13   ` Ard Biesheuvel
  2 siblings, 1 reply; 11+ messages in thread
From: Leif Lindholm @ 2019-11-29 11:29 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel

On Fri, Nov 29, 2019 at 11:47:14 +0100, Ard Biesheuvel wrote:
> Refactor the platform DXE a bit in patch #1 so we can seamlessly drop in
> the code in patch #2 to expose a SSDT with a device node describing
> OP-TEE when booting in ACPI mode and OP-TEE is present.

If we need any more SSDTs for this platform in future, I'll probably
start grumbling about refactoring away the per-table global variables,
but I guess it would be a bit overkill at this point...

For the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> Ard Biesheuvel (2):
>   Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine
>   Silicon/SynQuacer/PlatformDxe: add ACPI device node for OP-TEE if
>     present
> 
>  .../SynQuacerEvalBoard/SynQuacerEvalBoard.dsc |  1 +
>  .../SynQuacer/Drivers/PlatformDxe/Emmc.c      | 55 ------------
>  .../SynQuacer/Drivers/PlatformDxe/Optee.asl   | 23 +++++
>  .../Drivers/PlatformDxe/PlatformDxe.c         | 83 +++++++++++++++++++
>  .../Drivers/PlatformDxe/PlatformDxe.h         |  2 +
>  .../Drivers/PlatformDxe/PlatformDxe.inf       |  2 +
>  6 files changed, 111 insertions(+), 55 deletions(-)
>  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Optee.asl
> 
> -- 
> 2.17.1
> 

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

* Re: [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode
  2019-11-29 11:29 ` [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Leif Lindholm
@ 2019-11-29 12:13   ` Ard Biesheuvel
  2019-12-02  9:58     ` Ard Biesheuvel
  0 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2019-11-29 12:13 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel-groups-io

On Fri, 29 Nov 2019 at 12:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Nov 29, 2019 at 11:47:14 +0100, Ard Biesheuvel wrote:
> > Refactor the platform DXE a bit in patch #1 so we can seamlessly drop in
> > the code in patch #2 to expose a SSDT with a device node describing
> > OP-TEE when booting in ACPI mode and OP-TEE is present.
>
> If we need any more SSDTs for this platform in future, I'll probably
> start grumbling about refactoring away the per-table global variables,
> but I guess it would be a bit overkill at this point...
>

Yeah, the thought crossed my mind as well, but let's defer that to the
next time.

> For the series:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks,

> > Ard Biesheuvel (2):
> >   Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine
> >   Silicon/SynQuacer/PlatformDxe: add ACPI device node for OP-TEE if
> >     present
> >
> >  .../SynQuacerEvalBoard/SynQuacerEvalBoard.dsc |  1 +
> >  .../SynQuacer/Drivers/PlatformDxe/Emmc.c      | 55 ------------
> >  .../SynQuacer/Drivers/PlatformDxe/Optee.asl   | 23 +++++
> >  .../Drivers/PlatformDxe/PlatformDxe.c         | 83 +++++++++++++++++++
> >  .../Drivers/PlatformDxe/PlatformDxe.h         |  2 +
> >  .../Drivers/PlatformDxe/PlatformDxe.inf       |  2 +
> >  6 files changed, 111 insertions(+), 55 deletions(-)
> >  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Optee.asl
> >
> > --
> > 2.17.1
> >

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

* Re: [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode
  2019-11-29 12:13   ` Ard Biesheuvel
@ 2019-12-02  9:58     ` Ard Biesheuvel
  2019-12-02 12:02       ` Leif Lindholm
  0 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2019-12-02  9:58 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel-groups-io

On Fri, 29 Nov 2019 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Fri, 29 Nov 2019 at 12:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Fri, Nov 29, 2019 at 11:47:14 +0100, Ard Biesheuvel wrote:
> > > Refactor the platform DXE a bit in patch #1 so we can seamlessly drop in
> > > the code in patch #2 to expose a SSDT with a device node describing
> > > OP-TEE when booting in ACPI mode and OP-TEE is present.
> >
> > If we need any more SSDTs for this platform in future, I'll probably
> > start grumbling about refactoring away the per-table global variables,
> > but I guess it would be a bit overkill at this point...
> >
>
> Yeah, the thought crossed my mind as well, but let's defer that to the
> next time.
>
> > For the series:
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
>

Actually, I need to apply this on top

--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
@@ -282,13 +282,16 @@ InstallAcpiTables (
     return;
   }

+  if (mEmmcSsdtSize > 0) {
     Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
                           &TableKey);
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
         __FUNCTION__, Status));
     }
+  }

+  if (mTos0SsdtSize > 0) {
     Status = AcpiTable->InstallAcpiTable (AcpiTable, mTos0Ssdt, mTos0SsdtSize,
                           &TableKey);
     if (EFI_ERROR (Status)) {
@@ -296,6 +299,7 @@ InstallAcpiTables (
         __FUNCTION__, Status));
     }
   }
+}

 EFI_STATUS
 EFIAPI

or we will call InstallAcpiTable() with a zero size if we're only
installing one of the tables.

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

* Re: [edk2-devel] [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine
  2019-11-29 10:47 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine Ard Biesheuvel
@ 2019-12-02 10:26   ` Philippe Mathieu-Daudé
  2019-12-02 16:04     ` Ard Biesheuvel
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-02 10:26 UTC (permalink / raw)
  To: devel, ard.biesheuvel; +Cc: leif.lindholm

On 11/29/19 11:47 AM, Ard Biesheuvel via Groups.Io wrote:
> In preparation of adding support for describing the presence of OP-TEE
> via a SSDT ACPI table, refactor the existing code so we will be able to
> reuse it more easily.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>   Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c        | 55 -----------------
>   Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 65 ++++++++++++++++++++
>   2 files changed, 65 insertions(+), 55 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> index 0d0e5edad901..90b67152c768 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> @@ -53,10 +53,6 @@
>   
>   STATIC EFI_HANDLE mSdMmcControllerHandle;
>   
> -STATIC EFI_ACPI_DESCRIPTION_HEADER      *mSsdt;
> -STATIC UINTN                            mSsdtSize;
> -STATIC VOID                             *mEventRegistration;
> -
>   /**
>   
>     Override function for SDHCI capability bits
> @@ -185,31 +181,6 @@ STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride = {
>     SynQuacerSdMmcNotifyPhase,
>   };
>   
> -STATIC
> -VOID
> -EFIAPI
> -InstallAcpiTable (
> -  IN EFI_EVENT                      Event,
> -  IN VOID*                          Context
> -  )
> -{
> -  UINTN                             TableKey;
> -  EFI_STATUS                        Status;
> -  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
> -
> -  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
> -                  (VOID **)&AcpiTable);
> -  if (EFI_ERROR (Status)) {
> -    return;
> -  }
> -
> -  Status = AcpiTable->InstallAcpiTable (AcpiTable, mSsdt, mSsdtSize, &TableKey);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> -      __FUNCTION__, Status));
> -  }
> -}
> -
>   EFI_STATUS
>   EFIAPI
>   RegisterEmmc (
> @@ -218,32 +189,6 @@ RegisterEmmc (
>   {
>     EFI_STATUS                      Status;
>     EFI_HANDLE                      Handle;
> -  UINTN                           Index;
> -
> -  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
> -    //
> -    // Load the SSDT table from a raw section in this FFS file.
> -    //
> -    for (Index = 0;; Index++) {
> -      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> -                 (VOID **)&mSsdt, &mSsdtSize);
> -      if (EFI_ERROR (Status)) {
> -        break;
> -      }
> -
> -      if (mSsdt->OemTableId != EMMC_TABLE_ID) {
> -        continue;
> -      }
> -
> -      //
> -      // Register for the ACPI table protocol
> -      //
> -      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
> -        InstallAcpiTable, NULL, &mEventRegistration);
> -
> -      break;
> -    }
> -  }
>   
>     Status = RegisterNonDiscoverableMmioDevice (
>                NonDiscoverableDeviceTypeSdhci,
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> index 73cc560fa8d8..c9cc37dd2478 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> @@ -113,6 +113,11 @@ STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mI2c1Desc[] = {
>     }
>   };
>   
> +STATIC EFI_ACPI_DESCRIPTION_HEADER      *mEmmcSsdt;
> +STATIC UINTN                            mEmmcSsdtSize;
> +
> +STATIC VOID                             *mAcpiTableEventRegistration;
> +
>   STATIC
>   EFI_STATUS
>   RegisterDevice (
> @@ -256,6 +261,32 @@ EnableSettingsForm (
>     return InstallHiiPages ();
>   }
>   
> +STATIC
> +VOID
> +EFIAPI
> +InstallAcpiTables (

Maybe rename as InstallEmmcSsdtAcpiTables?

Regardless the name chosen:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> +  IN EFI_EVENT                      Event,
> +  IN VOID*                          Context
> +  )
> +{
> +  UINTN                             TableKey;
> +  EFI_STATUS                        Status;
> +  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
> +
> +  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
> +                  (VOID **)&AcpiTable);
> +  if (EFI_ERROR (Status)) {
> +    return;
> +  }
> +
> +  Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
> +                        &TableKey);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> +      __FUNCTION__, Status));
> +  }
> +}
> +
>   EFI_STATUS
>   EFIAPI
>   PlatformDxeEntryPoint (
> @@ -267,6 +298,9 @@ PlatformDxeEntryPoint (
>     VOID                            *Dtb;
>     UINTN                           DtbSize;
>     EFI_HANDLE                      Handle;
> +  EFI_ACPI_DESCRIPTION_HEADER     *Ssdt;
> +  UINTN                           SsdtSize;
> +  UINTN                           Index;
>   
>     mHiiSettingsVal = PcdGet64 (PcdPlatformSettings);
>     mHiiSettings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&mHiiSettingsVal;
> @@ -344,5 +378,36 @@ PlatformDxeEntryPoint (
>       ASSERT_EFI_ERROR (Status);
>     }
>   
> +  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
> +    //
> +    // Load the SSDT tables from a raw section in this FFS file.
> +    //
> +    for (Index = 0;; Index++) {
> +      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> +                 (VOID **)&Ssdt, &SsdtSize);
> +      if (EFI_ERROR (Status)) {
> +        break;
> +      }
> +
> +      switch (Ssdt->OemTableId) {
> +      case EMMC_TABLE_ID:
> +        if (mHiiSettings->EnableEmmc != EMMC_ENABLED) {
> +          break;
> +        }
> +        mEmmcSsdt = Ssdt;
> +        mEmmcSsdtSize = SsdtSize;
> +        break;
> +      }
> +    }
> +
> +    if (mEmmcSsdtSize > 0) {
> +      //
> +      // Register for the ACPI table protocol if we found any SSDTs to install
> +      //
> +      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
> +        InstallAcpiTables, NULL, &mAcpiTableEventRegistration);
> +    }
> +  }
> +
>     return EFI_SUCCESS;
>   }
> 


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

* Re: [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode
  2019-12-02  9:58     ` Ard Biesheuvel
@ 2019-12-02 12:02       ` Leif Lindholm
  2019-12-02 14:08         ` Ard Biesheuvel
  0 siblings, 1 reply; 11+ messages in thread
From: Leif Lindholm @ 2019-12-02 12:02 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel-groups-io

On Mon, Dec 02, 2019 at 10:58:45 +0100, Ard Biesheuvel wrote:
> On Fri, 29 Nov 2019 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > On Fri, 29 Nov 2019 at 12:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > On Fri, Nov 29, 2019 at 11:47:14 +0100, Ard Biesheuvel wrote:
> > > > Refactor the platform DXE a bit in patch #1 so we can seamlessly drop in
> > > > the code in patch #2 to expose a SSDT with a device node describing
> > > > OP-TEE when booting in ACPI mode and OP-TEE is present.
> > >
> > > If we need any more SSDTs for this platform in future, I'll probably
> > > start grumbling about refactoring away the per-table global variables,
> > > but I guess it would be a bit overkill at this point...
> > >
> >
> > Yeah, the thought crossed my mind as well, but let's defer that to the
> > next time.
> >
> > > For the series:
> > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > >
> >
> 
> Actually, I need to apply this on top
> 
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> @@ -282,13 +282,16 @@ InstallAcpiTables (
>      return;
>    }
> 
> +  if (mEmmcSsdtSize > 0) {
>      Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
>                            &TableKey);

Indentation presumably added for clarity?

>      if (EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
>          __FUNCTION__, Status));
>      }
> +  }
> 
> +  if (mTos0SsdtSize > 0) {
>      Status = AcpiTable->InstallAcpiTable (AcpiTable, mTos0Ssdt, mTos0SsdtSize,
>                            &TableKey);
>      if (EFI_ERROR (Status)) {
> @@ -296,6 +299,7 @@ InstallAcpiTables (
>          __FUNCTION__, Status));
>      }
>    }
> +}

LGTM.
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>  EFI_STATUS
>  EFIAPI
> 
> or we will call InstallAcpiTable() with a zero size if we're only
> installing one of the tables.

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

* Re: [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode
  2019-12-02 12:02       ` Leif Lindholm
@ 2019-12-02 14:08         ` Ard Biesheuvel
  2019-12-10  8:20           ` Ard Biesheuvel
  0 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2019-12-02 14:08 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel-groups-io

On Mon, 2 Dec 2019 at 13:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Mon, Dec 02, 2019 at 10:58:45 +0100, Ard Biesheuvel wrote:
> > On Fri, 29 Nov 2019 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > >
> > > On Fri, 29 Nov 2019 at 12:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > >
> > > > On Fri, Nov 29, 2019 at 11:47:14 +0100, Ard Biesheuvel wrote:
> > > > > Refactor the platform DXE a bit in patch #1 so we can seamlessly drop in
> > > > > the code in patch #2 to expose a SSDT with a device node describing
> > > > > OP-TEE when booting in ACPI mode and OP-TEE is present.
> > > >
> > > > If we need any more SSDTs for this platform in future, I'll probably
> > > > start grumbling about refactoring away the per-table global variables,
> > > > but I guess it would be a bit overkill at this point...
> > > >
> > >
> > > Yeah, the thought crossed my mind as well, but let's defer that to the
> > > next time.
> > >
> > > > For the series:
> > > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > >
> > >
> >
> > Actually, I need to apply this on top
> >
> > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> > @@ -282,13 +282,16 @@ InstallAcpiTables (
> >      return;
> >    }
> >
> > +  if (mEmmcSsdtSize > 0) {
> >      Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
> >                            &TableKey);
>
> Indentation presumably added for clarity?
>

Yep

> >      if (EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> >          __FUNCTION__, Status));
> >      }
> > +  }
> >
> > +  if (mTos0SsdtSize > 0) {
> >      Status = AcpiTable->InstallAcpiTable (AcpiTable, mTos0Ssdt, mTos0SsdtSize,
> >                            &TableKey);
> >      if (EFI_ERROR (Status)) {
> > @@ -296,6 +299,7 @@ InstallAcpiTables (
> >          __FUNCTION__, Status));
> >      }
> >    }
> > +}
>
> LGTM.
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks,

> >  EFI_STATUS
> >  EFIAPI
> >
> > or we will call InstallAcpiTable() with a zero size if we're only
> > installing one of the tables.

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

* Re: [edk2-devel] [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine
  2019-12-02 10:26   ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2019-12-02 16:04     ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2019-12-02 16:04 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: edk2-devel-groups-io, Leif Lindholm

On Mon, 2 Dec 2019 at 10:26, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 11/29/19 11:47 AM, Ard Biesheuvel via Groups.Io wrote:
> > In preparation of adding support for describing the presence of OP-TEE
> > via a SSDT ACPI table, refactor the existing code so we will be able to
> > reuse it more easily.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >   Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c        | 55 -----------------
> >   Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 65 ++++++++++++++++++++
> >   2 files changed, 65 insertions(+), 55 deletions(-)
> >
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> > index 0d0e5edad901..90b67152c768 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> > @@ -53,10 +53,6 @@
> >
> >   STATIC EFI_HANDLE mSdMmcControllerHandle;
> >
> > -STATIC EFI_ACPI_DESCRIPTION_HEADER      *mSsdt;
> > -STATIC UINTN                            mSsdtSize;
> > -STATIC VOID                             *mEventRegistration;
> > -
> >   /**
> >
> >     Override function for SDHCI capability bits
> > @@ -185,31 +181,6 @@ STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride = {
> >     SynQuacerSdMmcNotifyPhase,
> >   };
> >
> > -STATIC
> > -VOID
> > -EFIAPI
> > -InstallAcpiTable (
> > -  IN EFI_EVENT                      Event,
> > -  IN VOID*                          Context
> > -  )
> > -{
> > -  UINTN                             TableKey;
> > -  EFI_STATUS                        Status;
> > -  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
> > -
> > -  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
> > -                  (VOID **)&AcpiTable);
> > -  if (EFI_ERROR (Status)) {
> > -    return;
> > -  }
> > -
> > -  Status = AcpiTable->InstallAcpiTable (AcpiTable, mSsdt, mSsdtSize, &TableKey);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> > -      __FUNCTION__, Status));
> > -  }
> > -}
> > -
> >   EFI_STATUS
> >   EFIAPI
> >   RegisterEmmc (
> > @@ -218,32 +189,6 @@ RegisterEmmc (
> >   {
> >     EFI_STATUS                      Status;
> >     EFI_HANDLE                      Handle;
> > -  UINTN                           Index;
> > -
> > -  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
> > -    //
> > -    // Load the SSDT table from a raw section in this FFS file.
> > -    //
> > -    for (Index = 0;; Index++) {
> > -      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> > -                 (VOID **)&mSsdt, &mSsdtSize);
> > -      if (EFI_ERROR (Status)) {
> > -        break;
> > -      }
> > -
> > -      if (mSsdt->OemTableId != EMMC_TABLE_ID) {
> > -        continue;
> > -      }
> > -
> > -      //
> > -      // Register for the ACPI table protocol
> > -      //
> > -      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
> > -        InstallAcpiTable, NULL, &mEventRegistration);
> > -
> > -      break;
> > -    }
> > -  }
> >
> >     Status = RegisterNonDiscoverableMmioDevice (
> >                NonDiscoverableDeviceTypeSdhci,
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> > index 73cc560fa8d8..c9cc37dd2478 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> > @@ -113,6 +113,11 @@ STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mI2c1Desc[] = {
> >     }
> >   };
> >
> > +STATIC EFI_ACPI_DESCRIPTION_HEADER      *mEmmcSsdt;
> > +STATIC UINTN                            mEmmcSsdtSize;
> > +
> > +STATIC VOID                             *mAcpiTableEventRegistration;
> > +
> >   STATIC
> >   EFI_STATUS
> >   RegisterDevice (
> > @@ -256,6 +261,32 @@ EnableSettingsForm (
> >     return InstallHiiPages ();
> >   }
> >
> > +STATIC
> > +VOID
> > +EFIAPI
> > +InstallAcpiTables (
>
> Maybe rename as InstallEmmcSsdtAcpiTables?
>

The next patch adds a caller that uses it for another SSDT, but I take
your point - it could be more precise.

> Regardless the name chosen:
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>

Thanks,

> > +  IN EFI_EVENT                      Event,
> > +  IN VOID*                          Context
> > +  )
> > +{
> > +  UINTN                             TableKey;
> > +  EFI_STATUS                        Status;
> > +  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
> > +
> > +  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
> > +                  (VOID **)&AcpiTable);
> > +  if (EFI_ERROR (Status)) {
> > +    return;
> > +  }
> > +
> > +  Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
> > +                        &TableKey);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> > +      __FUNCTION__, Status));
> > +  }
> > +}
> > +
> >   EFI_STATUS
> >   EFIAPI
> >   PlatformDxeEntryPoint (
> > @@ -267,6 +298,9 @@ PlatformDxeEntryPoint (
> >     VOID                            *Dtb;
> >     UINTN                           DtbSize;
> >     EFI_HANDLE                      Handle;
> > +  EFI_ACPI_DESCRIPTION_HEADER     *Ssdt;
> > +  UINTN                           SsdtSize;
> > +  UINTN                           Index;
> >
> >     mHiiSettingsVal = PcdGet64 (PcdPlatformSettings);
> >     mHiiSettings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&mHiiSettingsVal;
> > @@ -344,5 +378,36 @@ PlatformDxeEntryPoint (
> >       ASSERT_EFI_ERROR (Status);
> >     }
> >
> > +  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
> > +    //
> > +    // Load the SSDT tables from a raw section in this FFS file.
> > +    //
> > +    for (Index = 0;; Index++) {
> > +      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> > +                 (VOID **)&Ssdt, &SsdtSize);
> > +      if (EFI_ERROR (Status)) {
> > +        break;
> > +      }
> > +
> > +      switch (Ssdt->OemTableId) {
> > +      case EMMC_TABLE_ID:
> > +        if (mHiiSettings->EnableEmmc != EMMC_ENABLED) {
> > +          break;
> > +        }
> > +        mEmmcSsdt = Ssdt;
> > +        mEmmcSsdtSize = SsdtSize;
> > +        break;
> > +      }
> > +    }
> > +
> > +    if (mEmmcSsdtSize > 0) {
> > +      //
> > +      // Register for the ACPI table protocol if we found any SSDTs to install
> > +      //
> > +      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
> > +        InstallAcpiTables, NULL, &mAcpiTableEventRegistration);
> > +    }
> > +  }
> > +
> >     return EFI_SUCCESS;
> >   }
> >
>

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

* Re: [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode
  2019-12-02 14:08         ` Ard Biesheuvel
@ 2019-12-10  8:20           ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2019-12-10  8:20 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel-groups-io

On Mon, 2 Dec 2019 at 15:08, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Mon, 2 Dec 2019 at 13:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Mon, Dec 02, 2019 at 10:58:45 +0100, Ard Biesheuvel wrote:
> > > On Fri, 29 Nov 2019 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > > >
> > > > On Fri, 29 Nov 2019 at 12:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > > >
> > > > > On Fri, Nov 29, 2019 at 11:47:14 +0100, Ard Biesheuvel wrote:
> > > > > > Refactor the platform DXE a bit in patch #1 so we can seamlessly drop in
> > > > > > the code in patch #2 to expose a SSDT with a device node describing
> > > > > > OP-TEE when booting in ACPI mode and OP-TEE is present.
> > > > >
> > > > > If we need any more SSDTs for this platform in future, I'll probably
> > > > > start grumbling about refactoring away the per-table global variables,
> > > > > but I guess it would be a bit overkill at this point...
> > > > >
> > > >
> > > > Yeah, the thought crossed my mind as well, but let's defer that to the
> > > > next time.
> > > >
> > > > > For the series:
> > > > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > > >
> > > >
> > >
> > > Actually, I need to apply this on top
> > >
> > > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> > > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> > > @@ -282,13 +282,16 @@ InstallAcpiTables (
> > >      return;
> > >    }
> > >
> > > +  if (mEmmcSsdtSize > 0) {
> > >      Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
> > >                            &TableKey);
> >
> > Indentation presumably added for clarity?
> >
>
> Yep
>
> > >      if (EFI_ERROR (Status)) {
> > >        DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> > >          __FUNCTION__, Status));
> > >      }
> > > +  }
> > >
> > > +  if (mTos0SsdtSize > 0) {
> > >      Status = AcpiTable->InstallAcpiTable (AcpiTable, mTos0Ssdt, mTos0SsdtSize,
> > >                            &TableKey);
> > >      if (EFI_ERROR (Status)) {
> > > @@ -296,6 +299,7 @@ InstallAcpiTables (
> > >          __FUNCTION__, Status));
> > >      }
> > >    }
> > > +}
> >
> > LGTM.
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
>
> Thanks,
>


Pushed as 3688ab6500e2..c5a2065ee8c2

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

end of thread, other threads:[~2019-12-10  8:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-29 10:47 [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Ard Biesheuvel
2019-11-29 10:47 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine Ard Biesheuvel
2019-12-02 10:26   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-12-02 16:04     ` Ard Biesheuvel
2019-11-29 10:47 ` [PATCH edk2-platforms 2/2] Silicon/SynQuacer/PlatformDxe: add ACPI device node for OP-TEE if present Ard Biesheuvel
2019-11-29 11:29 ` [PATCH edk2-platforms 0/2] SynQuacer: expose OP-TEE in ACPI mode Leif Lindholm
2019-11-29 12:13   ` Ard Biesheuvel
2019-12-02  9:58     ` Ard Biesheuvel
2019-12-02 12:02       ` Leif Lindholm
2019-12-02 14:08         ` Ard Biesheuvel
2019-12-10  8:20           ` Ard Biesheuvel

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