[PATCH edk2-platforms v2 0/2] enable Secure96 mezzanine on ACPI systems

[PATCH edk2-platforms v2 0/2] enable Secure96 mezzanine on ACPI systems

[Ard Biesheuvel]

This series plumbs in the support for describing 96boards mezzanines
via ACPI, and uses it to add a description of the Secure96. Note that
the TPM is still missing at the moment, but the I2C crypto accelerators
and the GPIO LEDs are covered.

Changes since v1:
- Don't rely on a ACPI device in the host platform's namespace to describe
  the GPIO lines - this prevents the mezzanine from using GPIOs as interrupts,
  and is generally far too restrictive. Since we need a hack (see below) for
  I2C anyway, just use the same method for the GPIO controller.
- I2C resources are described with a reference to the I2C controller via a
  string argument, which cannot be parameterized using PCDs like we do with
  GPIO interrupt numbers. (This is due to the fact that string PCDs are not
  expanded to true string literals by EDK2's AutoGen.h CPP magic). So add the
  default names (e.g., \_SB.I2C0) to Secure96.h, and permit a platform to
  override them if necessary by setting a scoped build option with a -D
  argument.

Ard Biesheuvel (2):
  Platform/96Boards: add ACPI support to mezzanine/LS connector driver
  Platform/Secure96Dxe: add ACPI description of the LEDs and I2C
    peripherals

 Platform/96Boards/Include/Protocol/Mezzanine.h      |  21 ++++
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c   |  35 ++++--
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf |   1 +
 Platform/96Boards/Secure96Dxe/Secure96.asl          | 116 ++++++++++++++++++++
 Platform/96Boards/Secure96Dxe/Secure96.h            |   8 ++
 Platform/96Boards/Secure96Dxe/Secure96Dxe.c         |  59 +++++++++-
 Platform/96Boards/Secure96Dxe/Secure96Dxe.inf       |   1 +
 7 files changed, 225 insertions(+), 16 deletions(-)
 create mode 100644 Platform/96Boards/Secure96Dxe/Secure96.asl

-- 
2.20.1

[PATCH edk2-platforms v2 1/2] Platform/96Boards: add ACPI support to mezzanine/LS connector driver

[Ard Biesheuvel]

Make the LS connector mezzanine support code ACPI aware, and invoke
the appropriate hook in the driver code to install a SSDT instead of
a DT overlay when running on a system that is booting in ACPI mode.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/96Boards/Include/Protocol/Mezzanine.h      | 21 ++++++++++++
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c   | 35 ++++++++++++++------
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf |  1 +
 3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/Platform/96Boards/Include/Protocol/Mezzanine.h b/Platform/96Boards/Include/Protocol/Mezzanine.h
index 9847649d2ac3..97f43e9a920f 100644
--- a/Platform/96Boards/Include/Protocol/Mezzanine.h
+++ b/Platform/96Boards/Include/Protocol/Mezzanine.h
@@ -16,6 +16,7 @@
 #define _96BOARDS_MEZZANINE_H_
 
 #include <Pi/PiI2c.h>
+#include <Protocol/AcpiTable.h>
 #include <Protocol/SpiConfiguration.h>
 
 #define MEZZANINE_PROTOCOL_GUID \
@@ -39,12 +40,32 @@ EFI_STATUS
   IN  OUT VOID                  *Dtb
   );
 
+/**
+  Install the mezzanine's SSDT table
+
+  @param[in]      This      Pointer to the MEZZANINE_PROTOCOL instance.
+  @param[in]      Dtb       Pointer to the device tree blob
+
+  @return   EFI_SUCCESS     Operation succeeded.
+  @return   other           An error has occurred.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *INSTALL_SSDT_TABLE) (
+  IN      MEZZANINE_PROTOCOL        *This,
+  IN      EFI_ACPI_TABLE_PROTOCOL   *AcpiProtocol
+  );
+
 struct _MEZZANINE_PROTOCOL {
   //
   // Get the device tree overlay for this mezzanine board
   //
   APPLY_DEVICE_TREE_OVERLAY   ApplyDeviceTreeOverlay;
   //
+  // Install the mezzanine's SSDT table
+  //
+  INSTALL_SSDT_TABLE          InstallSsdtTable;
+  //
   // The number of devices on LS connector I2C bus #0
   //
   UINT32                      I2c0NumDevices;
diff --git a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c
index f19d95635056..27044c5da699 100644
--- a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c
+++ b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c
@@ -21,6 +21,7 @@
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/AcpiTable.h>
 #include <Protocol/LsConnector.h>
 #include <Protocol/Mezzanine.h>
 
@@ -97,7 +98,7 @@ InstallHiiPages (
 STATIC
 VOID
 EFIAPI
-ApplyDeviceTreeOverlay (
+PublishOsDescription (
   EFI_EVENT           Event,
   VOID                *Context
   )
@@ -105,11 +106,30 @@ ApplyDeviceTreeOverlay (
   VOID                    *Dtb;
   MEZZANINE_PROTOCOL      *Mezzanine;
   EFI_STATUS              Status;
+  EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol;
+
+  Status = gBS->LocateProtocol (&g96BoardsMezzanineProtocolGuid, NULL,
+                  (VOID **)&Mezzanine);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "%a: no mezzanine driver active\n", __FUNCTION__));
+    return;
+  }
+
+  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
+                  (VOID **)&AcpiProtocol);
+  if (!EFI_ERROR (Status)) {
+    Status = Mezzanine->InstallSsdtTable (Mezzanine, AcpiProtocol);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table - %r\n",
+        __FUNCTION__, Status));
+    }
+    return;
+  }
 
   //
   // Find the DTB in the configuration table array. If it isn't there, just
-  // bail without an error: we may be running on an ACPI platform even if
-  // this driver does not support it [yet].
+  // bail without an error: the system may be able to proceed even without
+  // ACPI or DT description, so it isn't up to us to complain about this.
   //
   Status = EfiGetSystemConfigurationTable (&gFdtTableGuid, &Dtb);
   if (Status == EFI_NOT_FOUND) {
@@ -117,13 +137,6 @@ ApplyDeviceTreeOverlay (
   }
   ASSERT_EFI_ERROR (Status);
 
-  Status = gBS->LocateProtocol (&g96BoardsMezzanineProtocolGuid, NULL,
-                  (VOID **)&Mezzanine);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_INFO, "%a: no mezzanine driver active\n", __FUNCTION__));
-    return;
-  }
-
   Status = Mezzanine->ApplyDeviceTreeOverlay (Mezzanine, Dtb);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_WARN, "%a: failed to apply DT overlay - %r\n", __FUNCTION__,
@@ -211,7 +224,7 @@ EntryPoint (
   Status = gBS->CreateEventEx (
                   EVT_NOTIFY_SIGNAL,
                   TPL_NOTIFY,
-                  ApplyDeviceTreeOverlay,
+                  PublishOsDescription,
                   NULL,
                   &gEfiEndOfDxeEventGroupGuid,
                   &EndOfDxeEvent);
diff --git a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf
index 1bf528ceaa84..20b9637c1923 100644
--- a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf
+++ b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
 [Protocols]
   g96BoardsLsConnectorProtocolGuid     ## PRODUCES
   g96BoardsMezzanineProtocolGuid       ## CONSUMES
+  gEfiAcpiTableProtocolGuid            ## SOMETIMES_CONSUMES
 
 [Guids]
   gEfiEndOfDxeEventGroupGuid
-- 
2.20.1

[PATCH edk2-platforms v2 2/2] Platform/Secure96Dxe: add ACPI description of the LEDs and I2C peripherals

[Ard Biesheuvel]

Wire up the new 96boards mezzanine SSDT loading support, and use it
to describe the four GPIO LEDs on the Secure96 mezzanine board, as
well as the two ATmel crypto accelerators residing on the I2C bus.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/96Boards/Secure96Dxe/Secure96.asl    | 116 ++++++++++++++++++++
 Platform/96Boards/Secure96Dxe/Secure96.h      |   8 ++
 Platform/96Boards/Secure96Dxe/Secure96Dxe.c   |  59 +++++++++-
 Platform/96Boards/Secure96Dxe/Secure96Dxe.inf |   1 +
 4 files changed, 179 insertions(+), 5 deletions(-)

diff --git a/Platform/96Boards/Secure96Dxe/Secure96.asl b/Platform/96Boards/Secure96Dxe/Secure96.asl
new file mode 100644
index 000000000000..f458929fffb6
--- /dev/null
+++ b/Platform/96Boards/Secure96Dxe/Secure96.asl
@@ -0,0 +1,116 @@
+/** @file
+ * Copyright (c) 2019, Linaro Limited. All rights reserved.
+ *
+ * This program and the accompanying materials are licensed and made
+ * available under the terms and conditions of the BSD License which
+ * accompanies this distribution.  The full text of the license may be
+ * found at http://opensource.org/licenses/bsd-license.php
+ *
+ * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+ * IMPLIED.
+ */
+
+#include "Secure96.h"
+
+DefinitionBlock ("Secure96.aml", "SSDT", 2, "96BRDS", "SECURE96", 1)
+{
+    Scope (_SB)
+    {
+        Device (LD96)
+        {
+            Name (_HID, "LNRO9601")
+            Name (_UID, 0x0)
+            Name (_CID, "PRP0001")
+            Name (_DSD, Package () {
+                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                Package () { Package () { "compatible", "gpio-leds" }, }
+            })
+            Name (_CRS, ResourceTemplate() {
+                GpioIo (Exclusive, PullDefault, 0, 0, IoRestrictionOutputOnly, SECURE96_ACPI_GPIO)
+                {
+                    FixedPcdGet32 (PcdGpioPinG),
+                    FixedPcdGet32 (PcdGpioPinF),
+                    FixedPcdGet32 (PcdGpioPinI),
+                    FixedPcdGet32 (PcdGpioPinH),
+                }
+            })
+
+            Device (LDU1)
+            {
+                Name (_ADR, 0x1)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u1" },
+                        Package () { "gpios", Package () { ^^LD96, 0, 0, 0 }, },
+                    }
+                })
+            }
+
+            Device (LDU2)
+            {
+                Name (_ADR, 0x2)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u2" },
+                        Package () { "gpios", Package () { ^^LD96, 0, 1, 0 }, },
+                    }
+                })
+            }
+
+            Device (LDU3)
+            {
+                Name (_ADR, 0x3)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u3" },
+                        Package () { "gpios", Package () { ^^LD96, 0, 2, 0 }, },
+                    }
+                })
+            }
+
+            Device (LDU4)
+            {
+                Name (_ADR, 0x4)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u4" },
+                        Package () { "gpios", Package () { ^^LD96, 0, 3, 0 }, },
+                    }
+                })
+            }
+        }
+
+        Device (SH96)
+        {
+            Name (_HID, "LNRO9602")
+            Name (_UID, 0x0)
+            Name (_CID, "PRP0001")
+            Name (_DSD, Package () {
+                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                Package () { Package () { "compatible", "atmel,atsha204a" }, }
+            })
+            Name (_CRS, ResourceTemplate() {
+                I2CSerialBus (ATSHA204A_SLAVE_ADDRESS,, 100000,, SECURE96_ACPI_I2C0,,,,)
+            })
+        }
+
+        Device (EC96)
+        {
+            Name (_HID, "LNRO9603")
+            Name (_UID, 0x0)
+            Name (_CID, "PRP0001")
+            Name (_DSD, Package () {
+                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                Package () { Package () { "compatible", "atmel,atecc508a" }, }
+            })
+            Name (_CRS, ResourceTemplate() {
+                I2CSerialBus (ATECC508A_SLAVE_ADDRESS,, 100000,, SECURE96_ACPI_I2C0,,,,)
+            })
+        }
+    }
+}
diff --git a/Platform/96Boards/Secure96Dxe/Secure96.h b/Platform/96Boards/Secure96Dxe/Secure96.h
index 84b8aed13d1e..a44dc4804f92 100644
--- a/Platform/96Boards/Secure96Dxe/Secure96.h
+++ b/Platform/96Boards/Secure96Dxe/Secure96.h
@@ -23,4 +23,12 @@
 #define INFINEON_SLB9670_SPI_CS         0x0
 #define INFINEON_SLB9670_DT_NODENAME    tpm@0
 
+#ifndef SECURE96_ACPI_GPIO
+#define SECURE96_ACPI_GPIO              "\\_SB.GPIO"
+#endif
+
+#ifndef SECURE96_ACPI_I2C0
+#define SECURE96_ACPI_I2C0              "\\_SB.I2C0"
+#endif
+
 #endif // _SECURE96_H_
diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
index 6c48d7c0b024..68f8ec812b52 100644
--- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
+++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
@@ -24,6 +24,8 @@
 
 #include "Secure96.h"
 
+#define SECURE96_SSDT_OEM_TABLE_ID SIGNATURE_64('S','E','C','U','R','E','9','6')
+
 STATIC CONST UINT32 mI2cAtmelSha204aSlaveAddress[] = {
   ATSHA204A_SLAVE_ADDRESS,
 
@@ -148,15 +150,20 @@ ApplyDeviceTreeOverlay (
   UINTN           OverlaySize;
   EFI_STATUS      Status;
   INT32           Err;
+  UINTN           Index;
 
   //
   // Load the raw overlay DTB image from the raw section of this FFS file.
   //
-  Status = GetSectionFromFv (&gEfiCallerIdGuid,
-             EFI_SECTION_RAW, 0, &Overlay, &OverlaySize);
-  ASSERT_EFI_ERROR (Status);
-  if (EFI_ERROR (Status)) {
-    return EFI_NOT_FOUND;
+  for (Index = 0;; Index++) {
+    Status = GetSectionFromFv (&gEfiCallerIdGuid,
+               EFI_SECTION_RAW, Index, &Overlay, &OverlaySize);
+    if (EFI_ERROR (Status)) {
+      return EFI_NOT_FOUND;
+    }
+    if (!fdt_check_header (Overlay)) {
+      break;
+    }
   }
 
   //
@@ -177,8 +184,50 @@ ApplyDeviceTreeOverlay (
   return EFI_SUCCESS;
 }
 
+/**
+  Install the mezzanine's SSDT table
+
+  @param[in]      This      Pointer to the MEZZANINE_PROTOCOL instance.
+  @param[in]      Dtb       Pointer to the device tree blob
+
+  @return   EFI_SUCCESS     Operation succeeded.
+  @return   other           An error has occurred.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InstallSsdtTable (
+  IN      MEZZANINE_PROTOCOL        *This,
+  IN      EFI_ACPI_TABLE_PROTOCOL   *AcpiProtocol
+  )
+{
+  EFI_ACPI_DESCRIPTION_HEADER   *Ssdt;
+  UINTN                         SsdtSize;
+  EFI_STATUS                    Status;
+  UINTN                         Index;
+  UINTN                         TableKey;
+
+  //
+  // Load SSDT table from the raw section of this FFS file.
+  //
+  for (Index = 0;; Index++) {
+    Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
+               (VOID **)&Ssdt, &SsdtSize);
+    if (EFI_ERROR (Status)) {
+      return EFI_NOT_FOUND;
+    }
+    if (SsdtSize >= sizeof (EFI_ACPI_DESCRIPTION_HEADER) &&
+        Ssdt->OemTableId == SECURE96_SSDT_OEM_TABLE_ID) {
+      break;
+    }
+  }
+  return AcpiProtocol->InstallAcpiTable (AcpiProtocol, Ssdt, SsdtSize,
+                         &TableKey);
+}
+
 STATIC MEZZANINE_PROTOCOL mMezzanine = {
   ApplyDeviceTreeOverlay,
+  InstallSsdtTable,
   ARRAY_SIZE (mI2c0Devices),
   0,
   mI2c0Devices,
diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
index 72dbf1314c15..ce4c8b5f8fa5 100644
--- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
+++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
@@ -21,6 +21,7 @@ [Defines]
   ENTRY_POINT                    = Secure96DxeEntryPoint
 
 [Sources]
+  Secure96.asl
   Secure96.dts
   Secure96.h
   Secure96Dxe.c
-- 
2.20.1

Re: [PATCH edk2-platforms v2 0/2] enable Secure96 mezzanine on ACPI systems

[Leif Lindholm]

On Thu, May 02, 2019 at 11:58:52AM +0200, Ard Biesheuvel wrote:
> This series plumbs in the support for describing 96boards mezzanines
> via ACPI, and uses it to add a description of the Secure96. Note that
> the TPM is still missing at the moment, but the I2C crypto accelerators
> and the GPIO LEDs are covered.
> 
> Changes since v1:
> - Don't rely on a ACPI device in the host platform's namespace to describe
>   the GPIO lines - this prevents the mezzanine from using GPIOs as interrupts,
>   and is generally far too restrictive. Since we need a hack (see below) for
>   I2C anyway, just use the same method for the GPIO controller.
> - I2C resources are described with a reference to the I2C controller via a
>   string argument, which cannot be parameterized using PCDs like we do with
>   GPIO interrupt numbers. (This is due to the fact that string PCDs are not
>   expanded to true string literals by EDK2's AutoGen.h CPP magic). So add the
>   default names (e.g., \_SB.I2C0) to Secure96.h, and permit a platform to
>   override them if necessary by setting a scoped build option with a -D
>   argument.
> 
> Ard Biesheuvel (2):
>   Platform/96Boards: add ACPI support to mezzanine/LS connector driver
>   Platform/Secure96Dxe: add ACPI description of the LEDs and I2C
>     peripherals

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

>  Platform/96Boards/Include/Protocol/Mezzanine.h      |  21 ++++
>  Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c   |  35 ++++--
>  Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf |   1 +
>  Platform/96Boards/Secure96Dxe/Secure96.asl          | 116 ++++++++++++++++++++
>  Platform/96Boards/Secure96Dxe/Secure96.h            |   8 ++
>  Platform/96Boards/Secure96Dxe/Secure96Dxe.c         |  59 +++++++++-
>  Platform/96Boards/Secure96Dxe/Secure96Dxe.inf       |   1 +
>  7 files changed, 225 insertions(+), 16 deletions(-)
>  create mode 100644 Platform/96Boards/Secure96Dxe/Secure96.asl
> 
> -- 
> 2.20.1
> 

Re: [PATCH edk2-platforms v2 0/2] enable Secure96 mezzanine on ACPI systems

[Ard Biesheuvel]

On Thu, 2 May 2019 at 17:40, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, May 02, 2019 at 11:58:52AM +0200, Ard Biesheuvel wrote:
> > This series plumbs in the support for describing 96boards mezzanines
> > via ACPI, and uses it to add a description of the Secure96. Note that
> > the TPM is still missing at the moment, but the I2C crypto accelerators
> > and the GPIO LEDs are covered.
> >
> > Changes since v1:
> > - Don't rely on a ACPI device in the host platform's namespace to describe
> >   the GPIO lines - this prevents the mezzanine from using GPIOs as interrupts,
> >   and is generally far too restrictive. Since we need a hack (see below) for
> >   I2C anyway, just use the same method for the GPIO controller.
> > - I2C resources are described with a reference to the I2C controller via a
> >   string argument, which cannot be parameterized using PCDs like we do with
> >   GPIO interrupt numbers. (This is due to the fact that string PCDs are not
> >   expanded to true string literals by EDK2's AutoGen.h CPP magic). So add the
> >   default names (e.g., \_SB.I2C0) to Secure96.h, and permit a platform to
> >   override them if necessary by setting a scoped build option with a -D
> >   argument.
> >
> > Ard Biesheuvel (2):
> >   Platform/96Boards: add ACPI support to mezzanine/LS connector driver
> >   Platform/Secure96Dxe: add ACPI description of the LEDs and I2C
> >     peripherals
>
> For the series:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks

Pushed as 639a2e38ca66..747e75cc3e67