[v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

[edk2] [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

Posted by Ard Biesheuvel 7 years, 7 months ago

As a follow up to the changes proposed by Laszlo to make ACPI and DT
mutually exclusive on ArmVirtQemu, this patch proposed a DT platform
DXE driver that either installs the NULL protocol PlatformHasAcpiGuid,
or installs the FV embedded DTB binary as a configuration table under
the appropriate GUID, depending on a preference setting recorded as
a UEFI variable, and configurable via a HII screen.

The DTB binary can be embedded in the firmware image by adding the
following to the platform .fdf file:

  FILE FREEFORM = 25462CDA-221F-47DF-AC1D-259CFAA4E326 {
    SECTION RAW = SomePkg/path/to/foo.dtb
  }

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: fixup various errors and rough edges pointed out by Laszlo

 EmbeddedPkg/EmbeddedPkg.dec                         |   6 +
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  65 ++++++
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h   |  31 +++
 EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h |  23 +++
 EmbeddedPkg/Include/Guid/DtPlatformFormSet.h        |  23 +++
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr |  51 +++++
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 216 ++++++++++++++++++++
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni |  27 +++
 8 files changed, 442 insertions(+)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 29736fb4a71d..871fc5ff4016 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -62,6 +62,12 @@ [Guids.common]
   ## Include/Guid/PlatformHasDeviceTree.h
   gEdkiiPlatformHasDeviceTreeGuid = { 0x7ebb920d, 0x1aaf, 0x46d9, { 0xb2, 0xaf, 0x54, 0x1e, 0x1d, 0xce, 0x14, 0x8b } }
 
+  # HII form set GUID for DtPlatformDxe driver
+  gDtPlatformFormSetGuid = { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
+
+  # File GUID for default DTB image embedded in the firmware volume
+  gDtPlatformDefaultDtbFileGuid = { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
+
 [Protocols.common]
   gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
   gEfiDebugSupportPeriodicCallbackProtocolGuid = { 0x9546e07c, 0x2cbb, 0x4c88, { 0x98, 0x6c, 0xcd, 0x34, 0x10, 0x86, 0xf0, 0x44 } }
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
new file mode 100644
index 000000000000..aa1d3abb4012
--- /dev/null
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
@@ -0,0 +1,65 @@
+## @file
+#
+#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
+#
+#  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.
+#
+##
+
+[Defines]
+  INF_VERSION               = 0x00010019
+  BASE_NAME                 = DtPlatformDxe
+  FILE_GUID                 = FC097B3C-2EBD-4A75-A3DA-121DCAB365CC
+  MODULE_TYPE               = DXE_DRIVER
+  VERSION_STRING            = 1.0
+  ENTRY_POINT               = DtPlatformDxeEntryPoint
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES      = IA32 X64 ARM AARCH64
+#
+
+[Sources]
+  DtPlatformDxe.c
+  DtPlatformHii.vfr
+  DtPlatformHii.uni
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  UefiLib
+  UefiDriverEntryPoint
+  UefiBootServicesTableLib
+  UefiRuntimeServicesTableLib
+  UefiHiiServicesLib
+  MemoryAllocationLib
+  HiiLib
+  PcdLib
+  DxeServicesLib
+
+[Guids]
+  gDtPlatformFormSetGuid
+  gEdkiiPlatformHasAcpiGuid
+  gDtPlatformDefaultDtbFileGuid
+  gFdtTableGuid
+
+[Protocols]
+  gEfiHiiConfigAccessProtocolGuid                ## PRODUCES
+
+[Depex]
+  gEfiHiiDatabaseProtocolGuid         AND
+  gEfiVariableArchProtocolGuid        AND
+  gEfiVariableWriteArchProtocolGuid
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
new file mode 100644
index 000000000000..2369367277b0
--- /dev/null
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
@@ -0,0 +1,31 @@
+/** @file
+*
+*  Copyright (c) 2017, 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.
+*
+**/
+
+#ifndef __DT_PLATFORM_DXE_H__
+#define __DT_PLATFORM_DXE_H__
+
+#include <Guid/HiiPlatformSetupFormset.h>
+#include <Guid/DtPlatformFormSet.h>
+
+#define DT_ACPI_SELECT_DT       0x0
+#define DT_ACPI_SELECT_ACPI     0x1
+
+#define DT_ACPI_VARIABLE_NAME   L"DtAcpiPref"
+
+typedef struct {
+  UINT8         Pref;
+  UINT8         Reserved[3];
+} DT_ACPI_VARSTORE_DATA;
+
+#endif
diff --git a/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
new file mode 100644
index 000000000000..c44b4d9522fe
--- /dev/null
+++ b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
@@ -0,0 +1,23 @@
+/** @file
+*
+*  Copyright (c) 2017, 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.
+*
+**/
+
+#ifndef __DT_PLATFORM_DEFAULT_DTB_FILE_H__
+#define __DT_PLATFORM_DEFAULT_DTB_FILE_H__
+
+#define DT_PLATFORM_DEFAULT_DTB_FILE_GUID  \
+  { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
+
+extern EFI_GUID gDtPlatformDefaultDtbFileGuid;
+
+#endif
diff --git a/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
new file mode 100644
index 000000000000..71e3e7ebf7f3
--- /dev/null
+++ b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
@@ -0,0 +1,23 @@
+/** @file
+*
+*  Copyright (c) 2017, 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.
+*
+**/
+
+#ifndef __DT_PLATFORM_FORMSET_H__
+#define __DT_PLATFORM_FORMSET_H__
+
+#define DT_PLATFORM_FORMSET_GUID  \
+  { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
+
+extern EFI_GUID gDtPlatformFormSetGuid;
+
+#endif
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
new file mode 100644
index 000000000000..3516746c4d84
--- /dev/null
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
@@ -0,0 +1,51 @@
+/** @file
+*
+*  Copyright (c) 2017, Linaro, Ltd. 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 "DtPlatformDxe.h"
+
+//
+// EFI Variable attributes
+//
+#define EFI_VARIABLE_NON_VOLATILE       0x00000001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
+#define EFI_VARIABLE_RUNTIME_ACCESS     0x00000004
+#define EFI_VARIABLE_READ_ONLY          0x00000008
+
+formset
+  guid      = DT_PLATFORM_FORMSET_GUID,
+  title     = STRING_TOKEN(STR_FORM_SET_TITLE),
+  help      = STRING_TOKEN(STR_FORM_SET_TITLE_HELP),
+  classguid = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
+
+  efivarstore DT_ACPI_VARSTORE_DATA,
+    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,  // EFI variable attributes
+    name  = DtAcpiPref,
+    guid  = DT_PLATFORM_FORMSET_GUID;
+
+  form formid = 0x1000,
+    title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
+
+    oneof varid = DtAcpiPref.Pref,
+        prompt      = STRING_TOKEN(STR_DT_ACPI_SELECT_PROMPT),
+        help        = STRING_TOKEN(STR_DT_ACPI_SELECT_HELP),
+        flags       = NUMERIC_SIZE_1 | INTERACTIVE | RESET_REQUIRED,
+        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_DT), value = DT_ACPI_SELECT_DT, flags = DEFAULT;
+        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_ACPI), value = DT_ACPI_SELECT_ACPI, flags = 0;
+    endoneof;
+
+    subtitle text = STRING_TOKEN(STR_NULL_STRING);
+
+  endform;
+
+endformset;
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
new file mode 100644
index 000000000000..f714551213c2
--- /dev/null
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
@@ -0,0 +1,216 @@
+/** @file
+*
+*  Copyright (c) 2017, Linaro, Ltd. 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 <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/UefiHiiServicesLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/HiiLib.h>
+#include <Library/PcdLib.h>
+#include <Library/DxeServicesLib.h>
+
+#include "DtPlatformDxe.h"
+
+extern  UINT8                     DtPlatformHiiBin[];
+extern  UINT8                     DtPlatformDxeStrings[];
+
+typedef struct {
+  VENDOR_DEVICE_PATH              VendorDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL        End;
+} HII_VENDOR_DEVICE_PATH;
+
+STATIC HII_VENDOR_DEVICE_PATH     mDtPlatformDxeVendorDevicePath = {
+  {
+    {
+      HARDWARE_DEVICE_PATH,
+      HW_VENDOR_DP,
+      {
+        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
+        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
+      }
+    },
+    DT_PLATFORM_FORMSET_GUID
+  },
+  {
+    END_DEVICE_PATH_TYPE,
+    END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    {
+      (UINT8) (END_DEVICE_PATH_LENGTH),
+      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
+    }
+  }
+};
+
+STATIC
+EFI_STATUS
+InstallHiiPages (
+  VOID
+  )
+{
+  EFI_STATUS                      Status;
+  EFI_HII_HANDLE                  HiiHandle;
+  EFI_HANDLE                      DriverHandle;
+
+  DriverHandle = NULL;
+  Status = gBS->InstallMultipleProtocolInterfaces (&DriverHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &mDtPlatformDxeVendorDevicePath,
+                  NULL);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  HiiHandle = HiiAddPackages (&gDtPlatformFormSetGuid,
+                              DriverHandle,
+                              DtPlatformDxeStrings,
+                              DtPlatformHiiBin,
+                              NULL);
+
+  if (HiiHandle == NULL) {
+    gBS->UninstallMultipleProtocolInterfaces (DriverHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &mDtPlatformDxeVendorDevicePath,
+                  NULL);
+    return EFI_OUT_OF_RESOURCES;
+  }
+  return EFI_SUCCESS;
+}
+
+/**
+  The entry point for DtPlatformDxe driver.
+
+  @param[in] ImageHandle     The image handle of the driver.
+  @param[in] SystemTable     The system table.
+
+  @retval EFI_ALREADY_STARTED     The driver already exists in system.
+  @retval EFI_OUT_OF_RESOURCES    Fail to execute entry point due to lack of
+                                  resources.
+  @retval EFI_SUCCES              All the related protocols are installed on
+                                  the driver.
+
+**/
+EFI_STATUS
+EFIAPI
+DtPlatformDxeEntryPoint (
+  IN EFI_HANDLE                   ImageHandle,
+  IN EFI_SYSTEM_TABLE             *SystemTable
+  )
+{
+  EFI_STATUS                      Status;
+  DT_ACPI_VARSTORE_DATA           DtAcpiPref;
+  UINTN                           BufferSize;
+  VOID                            *Dtb;
+  UINTN                           DtbSize;
+  VOID                            *DtbCopy;
+
+  //
+  // Check whether a DTB blob is included in the firmware image.
+  //
+  Dtb = NULL;
+  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
+             EFI_SECTION_RAW, 0, &Dtb, &DtbSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: no DTB blob found, defaulting to ACPI\n",
+      __FUNCTION__));
+    DtAcpiPref.Pref = DT_ACPI_SELECT_ACPI;
+  } else {
+    //
+    // Get the current DT/ACPI preference from the DtAcpiPref variable.
+    //
+    BufferSize = sizeof (DtAcpiPref);
+    Status = gRT->GetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
+                    NULL, &BufferSize, &DtAcpiPref);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN, "%a: no DT/ACPI preference found, defaulting to DT\n",
+        __FUNCTION__));
+      DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
+    }
+  }
+
+  if (!EFI_ERROR (Status) &&
+      DtAcpiPref.Pref != DT_ACPI_SELECT_ACPI &&
+      DtAcpiPref.Pref != DT_ACPI_SELECT_DT) {
+    DEBUG ((DEBUG_WARN, "%a: invalid value for DtAcpiPref, defaulting to DT\n",
+      __FUNCTION__));
+    DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
+    Status = EFI_INVALID_PARAMETER; // trigger setvar below
+  }
+
+  //
+  // Write the newly selected default value back to the variable store.
+  //
+  if (EFI_ERROR (Status)) {
+    Status = gRT->SetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
+                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+                    sizeof (DtAcpiPref), &DtAcpiPref);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  }
+
+  if (DtAcpiPref.Pref == DT_ACPI_SELECT_ACPI) {
+    //
+    // ACPI was selected: install the gEdkiiPlatformHasAcpiGuid GUID as a
+    // NULL protocol to unlock dispatch of ACPI related drivers.
+    //
+    Status = gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
+                    &gEdkiiPlatformHasAcpiGuid, NULL, NULL);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR,
+        "%a: failed to install gEdkiiPlatformHasAcpiGuid as a protocol\n",
+        __FUNCTION__));
+      return Status;
+    }
+  } else if (DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
+    //
+    // DT was selected: copy the blob into newly allocated memory and install
+    // a reference to it as the FDT configuration table.
+    //
+    DtbCopy = AllocateCopyPool (DtbSize, Dtb);
+    if (DtbCopy == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DtbCopy);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: failed to install FDT configuration table\n",
+        __FUNCTION__));
+      FreePool (DtbCopy);
+      return Status;
+    }
+  } else {
+    ASSERT (FALSE);
+  }
+
+  //
+  // No point in installing the HII pages if ACPI is the only description
+  // we have
+  //
+  if (Dtb == NULL) {
+    return EFI_SUCCESS;
+  }
+
+  //
+  // Note that we don't uninstall the gEdkiiPlatformHasAcpiGuid protocol nor
+  // the FDT configuration table if the following call fails. While that will
+  // cause loading of this driver to fail, proceeding with ACPI and DT both
+  // disabled will guarantee a failed boot, and so it is better to leave them
+  // installed in that case.
+  //
+  return InstallHiiPages ();
+}
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
new file mode 100644
index 000000000000..bc995c1d0fbd
--- /dev/null
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
@@ -0,0 +1,27 @@
+/** @file
+*
+*  Copyright (c) 2017, Linaro, Ltd. 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.
+*
+**/
+
+#langdef en-US  "English"
+
+#string STR_FORM_SET_TITLE             #language en-US "O/S Hardware Description Selection"
+#string STR_FORM_SET_TITLE_HELP        #language en-US "Press <Enter> to choose between ACPI and DT hardware descriptions."
+
+#string STR_MAIN_FORM_TITLE            #language en-US "O/S Hardware Description Selection"
+#string STR_NULL_STRING                #language en-US ""
+
+#string STR_DT_ACPI_SELECT_PROMPT      #language en-US "O/S Hardware Description"
+#string STR_DT_ACPI_SELECT_HELP        #language en-US "Select the hardware description that will be exposed to the O/S."
+
+#string STR_DT_ACPI_SELECT_DT          #language en-US "Device Tree"
+#string STR_DT_ACPI_SELECT_ACPI        #language en-US "ACPI"
-- 
2.9.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

Posted by Ard Biesheuvel 7 years, 7 months ago

(use correct address for Marcin -- please take into account when replying)

On 28 March 2017 at 17:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> As a follow up to the changes proposed by Laszlo to make ACPI and DT
> mutually exclusive on ArmVirtQemu, this patch proposed a DT platform
> DXE driver that either installs the NULL protocol PlatformHasAcpiGuid,
> or installs the FV embedded DTB binary as a configuration table under
> the appropriate GUID, depending on a preference setting recorded as
> a UEFI variable, and configurable via a HII screen.
>
> The DTB binary can be embedded in the firmware image by adding the
> following to the platform .fdf file:
>
>   FILE FREEFORM = 25462CDA-221F-47DF-AC1D-259CFAA4E326 {
>     SECTION RAW = SomePkg/path/to/foo.dtb
>   }
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v2: fixup various errors and rough edges pointed out by Laszlo
>
>  EmbeddedPkg/EmbeddedPkg.dec                         |   6 +
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  65 ++++++
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h   |  31 +++
>  EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h |  23 +++
>  EmbeddedPkg/Include/Guid/DtPlatformFormSet.h        |  23 +++
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr |  51 +++++
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 216 ++++++++++++++++++++
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni |  27 +++
>  8 files changed, 442 insertions(+)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index 29736fb4a71d..871fc5ff4016 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -62,6 +62,12 @@ [Guids.common]
>    ## Include/Guid/PlatformHasDeviceTree.h
>    gEdkiiPlatformHasDeviceTreeGuid = { 0x7ebb920d, 0x1aaf, 0x46d9, { 0xb2, 0xaf, 0x54, 0x1e, 0x1d, 0xce, 0x14, 0x8b } }
>
> +  # HII form set GUID for DtPlatformDxe driver
> +  gDtPlatformFormSetGuid = { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
> +
> +  # File GUID for default DTB image embedded in the firmware volume
> +  gDtPlatformDefaultDtbFileGuid = { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
> +
>  [Protocols.common]
>    gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
>    gEfiDebugSupportPeriodicCallbackProtocolGuid = { 0x9546e07c, 0x2cbb, 0x4c88, { 0x98, 0x6c, 0xcd, 0x34, 0x10, 0x86, 0xf0, 0x44 } }
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> new file mode 100644
> index 000000000000..aa1d3abb4012
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> @@ -0,0 +1,65 @@
> +## @file
> +#
> +#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
> +#
> +#  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.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION               = 0x00010019
> +  BASE_NAME                 = DtPlatformDxe
> +  FILE_GUID                 = FC097B3C-2EBD-4A75-A3DA-121DCAB365CC
> +  MODULE_TYPE               = DXE_DRIVER
> +  VERSION_STRING            = 1.0
> +  ENTRY_POINT               = DtPlatformDxeEntryPoint
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES      = IA32 X64 ARM AARCH64
> +#
> +
> +[Sources]
> +  DtPlatformDxe.c
> +  DtPlatformHii.vfr
> +  DtPlatformHii.uni
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  UefiLib
> +  UefiDriverEntryPoint
> +  UefiBootServicesTableLib
> +  UefiRuntimeServicesTableLib
> +  UefiHiiServicesLib
> +  MemoryAllocationLib
> +  HiiLib
> +  PcdLib
> +  DxeServicesLib
> +
> +[Guids]
> +  gDtPlatformFormSetGuid
> +  gEdkiiPlatformHasAcpiGuid
> +  gDtPlatformDefaultDtbFileGuid
> +  gFdtTableGuid
> +
> +[Protocols]
> +  gEfiHiiConfigAccessProtocolGuid                ## PRODUCES
> +
> +[Depex]
> +  gEfiHiiDatabaseProtocolGuid         AND
> +  gEfiVariableArchProtocolGuid        AND
> +  gEfiVariableWriteArchProtocolGuid
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
> new file mode 100644
> index 000000000000..2369367277b0
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
> @@ -0,0 +1,31 @@
> +/** @file
> +*
> +*  Copyright (c) 2017, 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.
> +*
> +**/
> +
> +#ifndef __DT_PLATFORM_DXE_H__
> +#define __DT_PLATFORM_DXE_H__
> +
> +#include <Guid/HiiPlatformSetupFormset.h>
> +#include <Guid/DtPlatformFormSet.h>
> +
> +#define DT_ACPI_SELECT_DT       0x0
> +#define DT_ACPI_SELECT_ACPI     0x1
> +
> +#define DT_ACPI_VARIABLE_NAME   L"DtAcpiPref"
> +
> +typedef struct {
> +  UINT8         Pref;
> +  UINT8         Reserved[3];
> +} DT_ACPI_VARSTORE_DATA;
> +
> +#endif
> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
> new file mode 100644
> index 000000000000..c44b4d9522fe
> --- /dev/null
> +++ b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
> @@ -0,0 +1,23 @@
> +/** @file
> +*
> +*  Copyright (c) 2017, 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.
> +*
> +**/
> +
> +#ifndef __DT_PLATFORM_DEFAULT_DTB_FILE_H__
> +#define __DT_PLATFORM_DEFAULT_DTB_FILE_H__
> +
> +#define DT_PLATFORM_DEFAULT_DTB_FILE_GUID  \
> +  { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
> +
> +extern EFI_GUID gDtPlatformDefaultDtbFileGuid;
> +
> +#endif
> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
> new file mode 100644
> index 000000000000..71e3e7ebf7f3
> --- /dev/null
> +++ b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
> @@ -0,0 +1,23 @@
> +/** @file
> +*
> +*  Copyright (c) 2017, 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.
> +*
> +**/
> +
> +#ifndef __DT_PLATFORM_FORMSET_H__
> +#define __DT_PLATFORM_FORMSET_H__
> +
> +#define DT_PLATFORM_FORMSET_GUID  \
> +  { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
> +
> +extern EFI_GUID gDtPlatformFormSetGuid;
> +
> +#endif
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
> new file mode 100644
> index 000000000000..3516746c4d84
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
> @@ -0,0 +1,51 @@
> +/** @file
> +*
> +*  Copyright (c) 2017, Linaro, Ltd. 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 "DtPlatformDxe.h"
> +
> +//
> +// EFI Variable attributes
> +//
> +#define EFI_VARIABLE_NON_VOLATILE       0x00000001
> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x00000004
> +#define EFI_VARIABLE_READ_ONLY          0x00000008
> +
> +formset
> +  guid      = DT_PLATFORM_FORMSET_GUID,
> +  title     = STRING_TOKEN(STR_FORM_SET_TITLE),
> +  help      = STRING_TOKEN(STR_FORM_SET_TITLE_HELP),
> +  classguid = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
> +
> +  efivarstore DT_ACPI_VARSTORE_DATA,
> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,  // EFI variable attributes
> +    name  = DtAcpiPref,
> +    guid  = DT_PLATFORM_FORMSET_GUID;
> +
> +  form formid = 0x1000,
> +    title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
> +
> +    oneof varid = DtAcpiPref.Pref,
> +        prompt      = STRING_TOKEN(STR_DT_ACPI_SELECT_PROMPT),
> +        help        = STRING_TOKEN(STR_DT_ACPI_SELECT_HELP),
> +        flags       = NUMERIC_SIZE_1 | INTERACTIVE | RESET_REQUIRED,
> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_DT), value = DT_ACPI_SELECT_DT, flags = DEFAULT;
> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_ACPI), value = DT_ACPI_SELECT_ACPI, flags = 0;
> +    endoneof;
> +
> +    subtitle text = STRING_TOKEN(STR_NULL_STRING);
> +
> +  endform;
> +
> +endformset;
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> new file mode 100644
> index 000000000000..f714551213c2
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> @@ -0,0 +1,216 @@
> +/** @file
> +*
> +*  Copyright (c) 2017, Linaro, Ltd. 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 <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiDriverEntryPoint.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Library/UefiHiiServicesLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/HiiLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/DxeServicesLib.h>
> +
> +#include "DtPlatformDxe.h"
> +
> +extern  UINT8                     DtPlatformHiiBin[];
> +extern  UINT8                     DtPlatformDxeStrings[];
> +
> +typedef struct {
> +  VENDOR_DEVICE_PATH              VendorDevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL        End;
> +} HII_VENDOR_DEVICE_PATH;
> +
> +STATIC HII_VENDOR_DEVICE_PATH     mDtPlatformDxeVendorDevicePath = {
> +  {
> +    {
> +      HARDWARE_DEVICE_PATH,
> +      HW_VENDOR_DP,
> +      {
> +        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    DT_PLATFORM_FORMSET_GUID
> +  },
> +  {
> +    END_DEVICE_PATH_TYPE,
> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    {
> +      (UINT8) (END_DEVICE_PATH_LENGTH),
> +      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
> +    }
> +  }
> +};
> +
> +STATIC
> +EFI_STATUS
> +InstallHiiPages (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  EFI_HII_HANDLE                  HiiHandle;
> +  EFI_HANDLE                      DriverHandle;
> +
> +  DriverHandle = NULL;
> +  Status = gBS->InstallMultipleProtocolInterfaces (&DriverHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  &mDtPlatformDxeVendorDevicePath,
> +                  NULL);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  HiiHandle = HiiAddPackages (&gDtPlatformFormSetGuid,
> +                              DriverHandle,
> +                              DtPlatformDxeStrings,
> +                              DtPlatformHiiBin,
> +                              NULL);
> +
> +  if (HiiHandle == NULL) {
> +    gBS->UninstallMultipleProtocolInterfaces (DriverHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  &mDtPlatformDxeVendorDevicePath,
> +                  NULL);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  The entry point for DtPlatformDxe driver.
> +
> +  @param[in] ImageHandle     The image handle of the driver.
> +  @param[in] SystemTable     The system table.
> +
> +  @retval EFI_ALREADY_STARTED     The driver already exists in system.
> +  @retval EFI_OUT_OF_RESOURCES    Fail to execute entry point due to lack of
> +                                  resources.
> +  @retval EFI_SUCCES              All the related protocols are installed on
> +                                  the driver.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +DtPlatformDxeEntryPoint (
> +  IN EFI_HANDLE                   ImageHandle,
> +  IN EFI_SYSTEM_TABLE             *SystemTable
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  DT_ACPI_VARSTORE_DATA           DtAcpiPref;
> +  UINTN                           BufferSize;
> +  VOID                            *Dtb;
> +  UINTN                           DtbSize;
> +  VOID                            *DtbCopy;
> +
> +  //
> +  // Check whether a DTB blob is included in the firmware image.
> +  //
> +  Dtb = NULL;
> +  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> +             EFI_SECTION_RAW, 0, &Dtb, &DtbSize);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_WARN, "%a: no DTB blob found, defaulting to ACPI\n",
> +      __FUNCTION__));
> +    DtAcpiPref.Pref = DT_ACPI_SELECT_ACPI;
> +  } else {
> +    //
> +    // Get the current DT/ACPI preference from the DtAcpiPref variable.
> +    //
> +    BufferSize = sizeof (DtAcpiPref);
> +    Status = gRT->GetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
> +                    NULL, &BufferSize, &DtAcpiPref);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "%a: no DT/ACPI preference found, defaulting to DT\n",
> +        __FUNCTION__));
> +      DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
> +    }
> +  }
> +
> +  if (!EFI_ERROR (Status) &&
> +      DtAcpiPref.Pref != DT_ACPI_SELECT_ACPI &&
> +      DtAcpiPref.Pref != DT_ACPI_SELECT_DT) {
> +    DEBUG ((DEBUG_WARN, "%a: invalid value for DtAcpiPref, defaulting to DT\n",
> +      __FUNCTION__));
> +    DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
> +    Status = EFI_INVALID_PARAMETER; // trigger setvar below
> +  }
> +
> +  //
> +  // Write the newly selected default value back to the variable store.
> +  //
> +  if (EFI_ERROR (Status)) {
> +    Status = gRT->SetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
> +                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
> +                    sizeof (DtAcpiPref), &DtAcpiPref);
> +    if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
> +  }
> +
> +  if (DtAcpiPref.Pref == DT_ACPI_SELECT_ACPI) {
> +    //
> +    // ACPI was selected: install the gEdkiiPlatformHasAcpiGuid GUID as a
> +    // NULL protocol to unlock dispatch of ACPI related drivers.
> +    //
> +    Status = gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
> +                    &gEdkiiPlatformHasAcpiGuid, NULL, NULL);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR,
> +        "%a: failed to install gEdkiiPlatformHasAcpiGuid as a protocol\n",
> +        __FUNCTION__));
> +      return Status;
> +    }
> +  } else if (DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
> +    //
> +    // DT was selected: copy the blob into newly allocated memory and install
> +    // a reference to it as the FDT configuration table.
> +    //
> +    DtbCopy = AllocateCopyPool (DtbSize, Dtb);
> +    if (DtbCopy == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DtbCopy);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "%a: failed to install FDT configuration table\n",
> +        __FUNCTION__));
> +      FreePool (DtbCopy);
> +      return Status;
> +    }
> +  } else {
> +    ASSERT (FALSE);
> +  }
> +
> +  //
> +  // No point in installing the HII pages if ACPI is the only description
> +  // we have
> +  //
> +  if (Dtb == NULL) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  //
> +  // Note that we don't uninstall the gEdkiiPlatformHasAcpiGuid protocol nor
> +  // the FDT configuration table if the following call fails. While that will
> +  // cause loading of this driver to fail, proceeding with ACPI and DT both
> +  // disabled will guarantee a failed boot, and so it is better to leave them
> +  // installed in that case.
> +  //
> +  return InstallHiiPages ();
> +}
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
> new file mode 100644
> index 000000000000..bc995c1d0fbd
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
> @@ -0,0 +1,27 @@
> +/** @file
> +*
> +*  Copyright (c) 2017, Linaro, Ltd. 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.
> +*
> +**/
> +
> +#langdef en-US  "English"
> +
> +#string STR_FORM_SET_TITLE             #language en-US "O/S Hardware Description Selection"
> +#string STR_FORM_SET_TITLE_HELP        #language en-US "Press <Enter> to choose between ACPI and DT hardware descriptions."
> +
> +#string STR_MAIN_FORM_TITLE            #language en-US "O/S Hardware Description Selection"
> +#string STR_NULL_STRING                #language en-US ""
> +
> +#string STR_DT_ACPI_SELECT_PROMPT      #language en-US "O/S Hardware Description"
> +#string STR_DT_ACPI_SELECT_HELP        #language en-US "Select the hardware description that will be exposed to the O/S."
> +
> +#string STR_DT_ACPI_SELECT_DT          #language en-US "Device Tree"
> +#string STR_DT_ACPI_SELECT_ACPI        #language en-US "ACPI"
> --
> 2.9.3
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

Posted by Ard Biesheuvel 7 years, 7 months ago

On 28 March 2017 at 17:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> (use correct address for Marcin -- please take into account when replying)
>
> On 28 March 2017 at 17:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> As a follow up to the changes proposed by Laszlo to make ACPI and DT
>> mutually exclusive on ArmVirtQemu, this patch proposed a DT platform
>> DXE driver that either installs the NULL protocol PlatformHasAcpiGuid,
>> or installs the FV embedded DTB binary as a configuration table under
>> the appropriate GUID, depending on a preference setting recorded as
>> a UEFI variable, and configurable via a HII screen.
>>
>> The DTB binary can be embedded in the firmware image by adding the
>> following to the platform .fdf file:
>>
>>   FILE FREEFORM = 25462CDA-221F-47DF-AC1D-259CFAA4E326 {
>>     SECTION RAW = SomePkg/path/to/foo.dtb
>>   }
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> v2: fixup various errors and rough edges pointed out by Laszlo
>>
>>  EmbeddedPkg/EmbeddedPkg.dec                         |   6 +
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  65 ++++++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h   |  31 +++
>>  EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h |  23 +++
>>  EmbeddedPkg/Include/Guid/DtPlatformFormSet.h        |  23 +++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr |  51 +++++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 216 ++++++++++++++++++++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni |  27 +++
>>  8 files changed, 442 insertions(+)
>>
>> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
>> index 29736fb4a71d..871fc5ff4016 100644
>> --- a/EmbeddedPkg/EmbeddedPkg.dec
>> +++ b/EmbeddedPkg/EmbeddedPkg.dec
>> @@ -62,6 +62,12 @@ [Guids.common]
>>    ## Include/Guid/PlatformHasDeviceTree.h
>>    gEdkiiPlatformHasDeviceTreeGuid = { 0x7ebb920d, 0x1aaf, 0x46d9, { 0xb2, 0xaf, 0x54, 0x1e, 0x1d, 0xce, 0x14, 0x8b } }
>>
>> +  # HII form set GUID for DtPlatformDxe driver
>> +  gDtPlatformFormSetGuid = { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
>> +
>> +  # File GUID for default DTB image embedded in the firmware volume
>> +  gDtPlatformDefaultDtbFileGuid = { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
>> +
>>  [Protocols.common]
>>    gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
>>    gEfiDebugSupportPeriodicCallbackProtocolGuid = { 0x9546e07c, 0x2cbb, 0x4c88, { 0x98, 0x6c, 0xcd, 0x34, 0x10, 0x86, 0xf0, 0x44 } }
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>> new file mode 100644
>> index 000000000000..aa1d3abb4012
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>> @@ -0,0 +1,65 @@
>> +## @file
>> +#
>> +#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
>> +#
>> +#  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.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION               = 0x00010019
>> +  BASE_NAME                 = DtPlatformDxe
>> +  FILE_GUID                 = FC097B3C-2EBD-4A75-A3DA-121DCAB365CC
>> +  MODULE_TYPE               = DXE_DRIVER
>> +  VERSION_STRING            = 1.0
>> +  ENTRY_POINT               = DtPlatformDxeEntryPoint
>> +
>> +#
>> +# The following information is for reference only and not required by the build tools.
>> +#
>> +#  VALID_ARCHITECTURES      = IA32 X64 ARM AARCH64
>> +#
>> +
>> +[Sources]
>> +  DtPlatformDxe.c
>> +  DtPlatformHii.vfr
>> +  DtPlatformHii.uni
>> +
>> +[Packages]
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  DebugLib
>> +  UefiLib
>> +  UefiDriverEntryPoint
>> +  UefiBootServicesTableLib
>> +  UefiRuntimeServicesTableLib
>> +  UefiHiiServicesLib
>> +  MemoryAllocationLib
>> +  HiiLib
>> +  PcdLib
>> +  DxeServicesLib
>> +
>> +[Guids]
>> +  gDtPlatformFormSetGuid
>> +  gEdkiiPlatformHasAcpiGuid
>> +  gDtPlatformDefaultDtbFileGuid
>> +  gFdtTableGuid
>> +
>> +[Protocols]
>> +  gEfiHiiConfigAccessProtocolGuid                ## PRODUCES
>> +
>> +[Depex]
>> +  gEfiHiiDatabaseProtocolGuid         AND
>> +  gEfiVariableArchProtocolGuid        AND
>> +  gEfiVariableWriteArchProtocolGuid
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
>> new file mode 100644
>> index 000000000000..2369367277b0
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
>> @@ -0,0 +1,31 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, 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.
>> +*
>> +**/
>> +
>> +#ifndef __DT_PLATFORM_DXE_H__
>> +#define __DT_PLATFORM_DXE_H__
>> +
>> +#include <Guid/HiiPlatformSetupFormset.h>
>> +#include <Guid/DtPlatformFormSet.h>
>> +
>> +#define DT_ACPI_SELECT_DT       0x0
>> +#define DT_ACPI_SELECT_ACPI     0x1
>> +
>> +#define DT_ACPI_VARIABLE_NAME   L"DtAcpiPref"
>> +
>> +typedef struct {
>> +  UINT8         Pref;
>> +  UINT8         Reserved[3];
>> +} DT_ACPI_VARSTORE_DATA;
>> +
>> +#endif
>> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
>> new file mode 100644
>> index 000000000000..c44b4d9522fe
>> --- /dev/null
>> +++ b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
>> @@ -0,0 +1,23 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, 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.
>> +*
>> +**/
>> +
>> +#ifndef __DT_PLATFORM_DEFAULT_DTB_FILE_H__
>> +#define __DT_PLATFORM_DEFAULT_DTB_FILE_H__
>> +
>> +#define DT_PLATFORM_DEFAULT_DTB_FILE_GUID  \
>> +  { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
>> +
>> +extern EFI_GUID gDtPlatformDefaultDtbFileGuid;
>> +
>> +#endif
>> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
>> new file mode 100644
>> index 000000000000..71e3e7ebf7f3
>> --- /dev/null
>> +++ b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
>> @@ -0,0 +1,23 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, 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.
>> +*
>> +**/
>> +
>> +#ifndef __DT_PLATFORM_FORMSET_H__
>> +#define __DT_PLATFORM_FORMSET_H__
>> +
>> +#define DT_PLATFORM_FORMSET_GUID  \
>> +  { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
>> +
>> +extern EFI_GUID gDtPlatformFormSetGuid;
>> +
>> +#endif
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
>> new file mode 100644
>> index 000000000000..3516746c4d84
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
>> @@ -0,0 +1,51 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. 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 "DtPlatformDxe.h"
>> +
>> +//
>> +// EFI Variable attributes
>> +//
>> +#define EFI_VARIABLE_NON_VOLATILE       0x00000001
>> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
>> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x00000004
>> +#define EFI_VARIABLE_READ_ONLY          0x00000008
>> +
>> +formset
>> +  guid      = DT_PLATFORM_FORMSET_GUID,
>> +  title     = STRING_TOKEN(STR_FORM_SET_TITLE),
>> +  help      = STRING_TOKEN(STR_FORM_SET_TITLE_HELP),
>> +  classguid = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
>> +
>> +  efivarstore DT_ACPI_VARSTORE_DATA,
>> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,  // EFI variable attributes
>> +    name  = DtAcpiPref,
>> +    guid  = DT_PLATFORM_FORMSET_GUID;
>> +
>> +  form formid = 0x1000,
>> +    title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
>> +
>> +    oneof varid = DtAcpiPref.Pref,
>> +        prompt      = STRING_TOKEN(STR_DT_ACPI_SELECT_PROMPT),
>> +        help        = STRING_TOKEN(STR_DT_ACPI_SELECT_HELP),
>> +        flags       = NUMERIC_SIZE_1 | INTERACTIVE | RESET_REQUIRED,
>> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_DT), value = DT_ACPI_SELECT_DT, flags = DEFAULT;
>> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_ACPI), value = DT_ACPI_SELECT_ACPI, flags = 0;
>> +    endoneof;
>> +
>> +    subtitle text = STRING_TOKEN(STR_NULL_STRING);
>> +
>> +  endform;
>> +
>> +endformset;
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
>> new file mode 100644
>> index 000000000000..f714551213c2
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
>> @@ -0,0 +1,216 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. 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 <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DevicePathLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Library/UefiDriverEntryPoint.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +#include <Library/UefiHiiServicesLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/HiiLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/DxeServicesLib.h>
>> +
>> +#include "DtPlatformDxe.h"
>> +
>> +extern  UINT8                     DtPlatformHiiBin[];
>> +extern  UINT8                     DtPlatformDxeStrings[];
>> +
>> +typedef struct {
>> +  VENDOR_DEVICE_PATH              VendorDevicePath;
>> +  EFI_DEVICE_PATH_PROTOCOL        End;
>> +} HII_VENDOR_DEVICE_PATH;
>> +
>> +STATIC HII_VENDOR_DEVICE_PATH     mDtPlatformDxeVendorDevicePath = {
>> +  {
>> +    {
>> +      HARDWARE_DEVICE_PATH,
>> +      HW_VENDOR_DP,
>> +      {
>> +        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
>> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
>> +      }
>> +    },
>> +    DT_PLATFORM_FORMSET_GUID
>> +  },
>> +  {
>> +    END_DEVICE_PATH_TYPE,
>> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> +    {
>> +      (UINT8) (END_DEVICE_PATH_LENGTH),
>> +      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
>> +    }
>> +  }
>> +};
>> +
>> +STATIC
>> +EFI_STATUS
>> +InstallHiiPages (
>> +  VOID
>> +  )
>> +{
>> +  EFI_STATUS                      Status;
>> +  EFI_HII_HANDLE                  HiiHandle;
>> +  EFI_HANDLE                      DriverHandle;
>> +
>> +  DriverHandle = NULL;
>> +  Status = gBS->InstallMultipleProtocolInterfaces (&DriverHandle,
>> +                  &gEfiDevicePathProtocolGuid,
>> +                  &mDtPlatformDxeVendorDevicePath,
>> +                  NULL);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  HiiHandle = HiiAddPackages (&gDtPlatformFormSetGuid,
>> +                              DriverHandle,
>> +                              DtPlatformDxeStrings,
>> +                              DtPlatformHiiBin,
>> +                              NULL);
>> +
>> +  if (HiiHandle == NULL) {
>> +    gBS->UninstallMultipleProtocolInterfaces (DriverHandle,
>> +                  &gEfiDevicePathProtocolGuid,
>> +                  &mDtPlatformDxeVendorDevicePath,
>> +                  NULL);
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> +  The entry point for DtPlatformDxe driver.
>> +
>> +  @param[in] ImageHandle     The image handle of the driver.
>> +  @param[in] SystemTable     The system table.
>> +
>> +  @retval EFI_ALREADY_STARTED     The driver already exists in system.
>> +  @retval EFI_OUT_OF_RESOURCES    Fail to execute entry point due to lack of
>> +                                  resources.
>> +  @retval EFI_SUCCES              All the related protocols are installed on
>> +                                  the driver.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +DtPlatformDxeEntryPoint (
>> +  IN EFI_HANDLE                   ImageHandle,
>> +  IN EFI_SYSTEM_TABLE             *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS                      Status;
>> +  DT_ACPI_VARSTORE_DATA           DtAcpiPref;
>> +  UINTN                           BufferSize;
>> +  VOID                            *Dtb;
>> +  UINTN                           DtbSize;
>> +  VOID                            *DtbCopy;
>> +
>> +  //
>> +  // Check whether a DTB blob is included in the firmware image.
>> +  //
>> +  Dtb = NULL;
>> +  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
>> +             EFI_SECTION_RAW, 0, &Dtb, &DtbSize);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_WARN, "%a: no DTB blob found, defaulting to ACPI\n",
>> +      __FUNCTION__));
>> +    DtAcpiPref.Pref = DT_ACPI_SELECT_ACPI;
>> +  } else {
>> +    //
>> +    // Get the current DT/ACPI preference from the DtAcpiPref variable.
>> +    //
>> +    BufferSize = sizeof (DtAcpiPref);
>> +    Status = gRT->GetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
>> +                    NULL, &BufferSize, &DtAcpiPref);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_WARN, "%a: no DT/ACPI preference found, defaulting to DT\n",
>> +        __FUNCTION__));
>> +      DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
>> +    }
>> +  }
>> +
>> +  if (!EFI_ERROR (Status) &&
>> +      DtAcpiPref.Pref != DT_ACPI_SELECT_ACPI &&
>> +      DtAcpiPref.Pref != DT_ACPI_SELECT_DT) {
>> +    DEBUG ((DEBUG_WARN, "%a: invalid value for DtAcpiPref, defaulting to DT\n",
>> +      __FUNCTION__));
>> +    DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
>> +    Status = EFI_INVALID_PARAMETER; // trigger setvar below
>> +  }
>> +
>> +  //
>> +  // Write the newly selected default value back to the variable store.
>> +  //
>> +  if (EFI_ERROR (Status)) {
>> +    Status = gRT->SetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
>> +                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
>> +                    sizeof (DtAcpiPref), &DtAcpiPref);
>> +    if (EFI_ERROR (Status)) {
>> +      return Status;
>> +    }
>> +  }
>> +
>> +  if (DtAcpiPref.Pref == DT_ACPI_SELECT_ACPI) {
>> +    //
>> +    // ACPI was selected: install the gEdkiiPlatformHasAcpiGuid GUID as a
>> +    // NULL protocol to unlock dispatch of ACPI related drivers.
>> +    //
>> +    Status = gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
>> +                    &gEdkiiPlatformHasAcpiGuid, NULL, NULL);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_ERROR,
>> +        "%a: failed to install gEdkiiPlatformHasAcpiGuid as a protocol\n",
>> +        __FUNCTION__));
>> +      return Status;
>> +    }
>> +  } else if (DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
>> +    //
>> +    // DT was selected: copy the blob into newly allocated memory and install
>> +    // a reference to it as the FDT configuration table.
>> +    //
>> +    DtbCopy = AllocateCopyPool (DtbSize, Dtb);
>> +    if (DtbCopy == NULL) {
>> +      return EFI_OUT_OF_RESOURCES;
>> +    }
>> +    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DtbCopy);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_ERROR, "%a: failed to install FDT configuration table\n",
>> +        __FUNCTION__));
>> +      FreePool (DtbCopy);
>> +      return Status;
>> +    }
>> +  } else {
>> +    ASSERT (FALSE);
>> +  }
>> +
>> +  //
>> +  // No point in installing the HII pages if ACPI is the only description
>> +  // we have
>> +  //
>> +  if (Dtb == NULL) {
>> +    return EFI_SUCCESS;
>> +  }
>> +
>> +  //
>> +  // Note that we don't uninstall the gEdkiiPlatformHasAcpiGuid protocol nor
>> +  // the FDT configuration table if the following call fails. While that will
>> +  // cause loading of this driver to fail, proceeding with ACPI and DT both
>> +  // disabled will guarantee a failed boot, and so it is better to leave them
>> +  // installed in that case.
>> +  //
>> +  return InstallHiiPages ();
>> +}
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
>> new file mode 100644
>> index 000000000000..bc995c1d0fbd
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
>> @@ -0,0 +1,27 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. 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.
>> +*
>> +**/
>> +
>> +#langdef en-US  "English"
>> +
>> +#string STR_FORM_SET_TITLE             #language en-US "O/S Hardware Description Selection"
>> +#string STR_FORM_SET_TITLE_HELP        #language en-US "Press <Enter> to choose between ACPI and DT hardware descriptions."
>> +
>> +#string STR_MAIN_FORM_TITLE            #language en-US "O/S Hardware Description Selection"
>> +#string STR_NULL_STRING                #language en-US ""
>> +
>> +#string STR_DT_ACPI_SELECT_PROMPT      #language en-US "O/S Hardware Description"
>> +#string STR_DT_ACPI_SELECT_HELP        #language en-US "Select the hardware description that will be exposed to the O/S."
>> +
>> +#string STR_DT_ACPI_SELECT_DT          #language en-US "Device Tree"
>> +#string STR_DT_ACPI_SELECT_ACPI        #language en-US "ACPI"
>> --
>> 2.9.3
>>


As promised in the notes:

diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc
b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc
index 355442bcfca6..460ae36d44f8 100644
--- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc
+++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc
@@ -613,7 +613,12 @@ DEFINE DO_KCS    = 0
   #
   # ACPI Support
   #
-  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf {
+!ifdef $(CELLO_DTB_IMAGE)
+    <LibraryClasses>
+      NULL|EmbeddedPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
+!endif
+  }
   OpenPlatformPkg/Platforms/AMD/Styx/AcpiTables/AcpiAml.inf
   OpenPlatformPkg/Platforms/AMD/Styx/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf

@@ -680,3 +685,7 @@ DEFINE DO_KCS    = 0
 !ifdef $(RENESAS_XHCI_FW_DIR)
   OpenPlatformPkg/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf
 !endif
+
+!ifdef $(CELLO_DTB_IMAGE)
+  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
+!endif
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf
b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf
index 29103531a224..e25eec7baec3 100644
--- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf
+++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf
@@ -231,6 +231,14 @@ READ_LOCK_STATUS   = TRUE
   }
 !endif

+!ifdef $(CELLO_DTB_IMAGE)
+  INF EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
+
+  FILE FREEFORM = 25462CDA-221F-47DF-AC1D-259CFAA4E326 {
+    SECTION RAW = $(CELLO_DTB_IMAGE)
+  }
+!endif
+
 [FV.STYX_EFI]
 FvAlignment        = 16
 ERASE_POLARITY     = 1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

Posted by Laszlo Ersek 7 years, 7 months ago

On 03/28/17 18:16, Ard Biesheuvel wrote:
> (use correct address for Marcin -- please take into account when replying)
> 
> On 28 March 2017 at 17:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> As a follow up to the changes proposed by Laszlo to make ACPI and DT
>> mutually exclusive on ArmVirtQemu, this patch proposed a DT platform

new typo relative to v1: s/proposed/proposes/

>> DXE driver that either installs the NULL protocol PlatformHasAcpiGuid,
>> or installs the FV embedded DTB binary as a configuration table under
>> the appropriate GUID, depending on a preference setting recorded as
>> a UEFI variable, and configurable via a HII screen.
>>
>> The DTB binary can be embedded in the firmware image by adding the
>> following to the platform .fdf file:
>>
>>   FILE FREEFORM = 25462CDA-221F-47DF-AC1D-259CFAA4E326 {
>>     SECTION RAW = SomePkg/path/to/foo.dtb
>>   }
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> v2: fixup various errors and rough edges pointed out by Laszlo
>>
>>  EmbeddedPkg/EmbeddedPkg.dec                         |   6 +
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  65 ++++++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h   |  31 +++
>>  EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h |  23 +++
>>  EmbeddedPkg/Include/Guid/DtPlatformFormSet.h        |  23 +++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr |  51 +++++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 216 ++++++++++++++++++++
>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni |  27 +++
>>  8 files changed, 442 insertions(+)
>>
>> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
>> index 29736fb4a71d..871fc5ff4016 100644
>> --- a/EmbeddedPkg/EmbeddedPkg.dec
>> +++ b/EmbeddedPkg/EmbeddedPkg.dec
>> @@ -62,6 +62,12 @@ [Guids.common]
>>    ## Include/Guid/PlatformHasDeviceTree.h
>>    gEdkiiPlatformHasDeviceTreeGuid = { 0x7ebb920d, 0x1aaf, 0x46d9, { 0xb2, 0xaf, 0x54, 0x1e, 0x1d, 0xce, 0x14, 0x8b } }
>>
>> +  # HII form set GUID for DtPlatformDxe driver
>> +  gDtPlatformFormSetGuid = { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
>> +
>> +  # File GUID for default DTB image embedded in the firmware volume
>> +  gDtPlatformDefaultDtbFileGuid = { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
>> +
>>  [Protocols.common]
>>    gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
>>    gEfiDebugSupportPeriodicCallbackProtocolGuid = { 0x9546e07c, 0x2cbb, 0x4c88, { 0x98, 0x6c, 0xcd, 0x34, 0x10, 0x86, 0xf0, 0x44 } }
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>> new file mode 100644
>> index 000000000000..aa1d3abb4012
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>> @@ -0,0 +1,65 @@
>> +## @file
>> +#
>> +#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
>> +#
>> +#  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.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION               = 0x00010019
>> +  BASE_NAME                 = DtPlatformDxe
>> +  FILE_GUID                 = FC097B3C-2EBD-4A75-A3DA-121DCAB365CC
>> +  MODULE_TYPE               = DXE_DRIVER
>> +  VERSION_STRING            = 1.0
>> +  ENTRY_POINT               = DtPlatformDxeEntryPoint
>> +
>> +#
>> +# The following information is for reference only and not required by the build tools.
>> +#
>> +#  VALID_ARCHITECTURES      = IA32 X64 ARM AARCH64
>> +#
>> +
>> +[Sources]
>> +  DtPlatformDxe.c
>> +  DtPlatformHii.vfr
>> +  DtPlatformHii.uni
>> +
>> +[Packages]
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  DebugLib
>> +  UefiLib
>> +  UefiDriverEntryPoint
>> +  UefiBootServicesTableLib
>> +  UefiRuntimeServicesTableLib
>> +  UefiHiiServicesLib
>> +  MemoryAllocationLib
>> +  HiiLib
>> +  PcdLib
>> +  DxeServicesLib
>> +
>> +[Guids]
>> +  gDtPlatformFormSetGuid
>> +  gEdkiiPlatformHasAcpiGuid
>> +  gDtPlatformDefaultDtbFileGuid
>> +  gFdtTableGuid
>> +
>> +[Protocols]
>> +  gEfiHiiConfigAccessProtocolGuid                ## PRODUCES

I think this should have been dropped as well -- you can remove it when
you commit the patch, if the removal works. (I think it should.)

>> +
>> +[Depex]
>> +  gEfiHiiDatabaseProtocolGuid         AND

IMO this should have been dropped too. You are using UefiHiiServicesLib
(only one instance available:
"MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf"), and
that instance gives you a dependency on gEfiHiiDatabaseProtocolGuid
automatically.

You never use the protocol manually, just through the following chain:

* HiiAddPackages() is from HiiLib (only one instance:
MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf), which relies on
"gHiiDatabase"

* "gHiiDatabase" is defined / initialized in UefiHiiServicesLib
(UefiHiiServicesLibConstructor()), dependent on
gEfiHiiDatabaseProtocolGuid, and the library instance also spells out
the DEPEX for that.

So I think, if you can drop this from the depex while keeping things
functional, you should. No need to resubmit just because of this, again.


Some more things which look useless for this driver:
- PcdLib (class in INF, hdr inclusion in source)
- ... OK I guess that's all.

(In my v1 review I wrote "Basically, please make this INF as minimal as
possible" -- apologies for not spelling it out in more detail.)

>> +  gEfiVariableArchProtocolGuid        AND
>> +  gEfiVariableWriteArchProtocolGuid
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
>> new file mode 100644
>> index 000000000000..2369367277b0
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
>> @@ -0,0 +1,31 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, 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.
>> +*
>> +**/
>> +
>> +#ifndef __DT_PLATFORM_DXE_H__
>> +#define __DT_PLATFORM_DXE_H__
>> +
>> +#include <Guid/HiiPlatformSetupFormset.h>
>> +#include <Guid/DtPlatformFormSet.h>
>> +
>> +#define DT_ACPI_SELECT_DT       0x0
>> +#define DT_ACPI_SELECT_ACPI     0x1
>> +
>> +#define DT_ACPI_VARIABLE_NAME   L"DtAcpiPref"
>> +
>> +typedef struct {
>> +  UINT8         Pref;
>> +  UINT8         Reserved[3];
>> +} DT_ACPI_VARSTORE_DATA;
>> +
>> +#endif
>> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
>> new file mode 100644
>> index 000000000000..c44b4d9522fe
>> --- /dev/null
>> +++ b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
>> @@ -0,0 +1,23 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, 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.
>> +*
>> +**/
>> +
>> +#ifndef __DT_PLATFORM_DEFAULT_DTB_FILE_H__
>> +#define __DT_PLATFORM_DEFAULT_DTB_FILE_H__
>> +
>> +#define DT_PLATFORM_DEFAULT_DTB_FILE_GUID  \
>> +  { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
>> +
>> +extern EFI_GUID gDtPlatformDefaultDtbFileGuid;
>> +
>> +#endif
>> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
>> new file mode 100644
>> index 000000000000..71e3e7ebf7f3
>> --- /dev/null
>> +++ b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
>> @@ -0,0 +1,23 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, 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.
>> +*
>> +**/
>> +
>> +#ifndef __DT_PLATFORM_FORMSET_H__
>> +#define __DT_PLATFORM_FORMSET_H__
>> +
>> +#define DT_PLATFORM_FORMSET_GUID  \
>> +  { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
>> +
>> +extern EFI_GUID gDtPlatformFormSetGuid;
>> +
>> +#endif
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
>> new file mode 100644
>> index 000000000000..3516746c4d84
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
>> @@ -0,0 +1,51 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. 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 "DtPlatformDxe.h"
>> +
>> +//
>> +// EFI Variable attributes
>> +//
>> +#define EFI_VARIABLE_NON_VOLATILE       0x00000001
>> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
>> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x00000004
>> +#define EFI_VARIABLE_READ_ONLY          0x00000008
>> +
>> +formset
>> +  guid      = DT_PLATFORM_FORMSET_GUID,
>> +  title     = STRING_TOKEN(STR_FORM_SET_TITLE),
>> +  help      = STRING_TOKEN(STR_FORM_SET_TITLE_HELP),
>> +  classguid = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
>> +
>> +  efivarstore DT_ACPI_VARSTORE_DATA,
>> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,  // EFI variable attributes
>> +    name  = DtAcpiPref,
>> +    guid  = DT_PLATFORM_FORMSET_GUID;
>> +
>> +  form formid = 0x1000,
>> +    title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
>> +
>> +    oneof varid = DtAcpiPref.Pref,
>> +        prompt      = STRING_TOKEN(STR_DT_ACPI_SELECT_PROMPT),
>> +        help        = STRING_TOKEN(STR_DT_ACPI_SELECT_HELP),
>> +        flags       = NUMERIC_SIZE_1 | INTERACTIVE | RESET_REQUIRED,
>> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_DT), value = DT_ACPI_SELECT_DT, flags = DEFAULT;
>> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_ACPI), value = DT_ACPI_SELECT_ACPI, flags = 0;
>> +    endoneof;
>> +
>> +    subtitle text = STRING_TOKEN(STR_NULL_STRING);
>> +
>> +  endform;
>> +
>> +endformset;
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
>> new file mode 100644
>> index 000000000000..f714551213c2
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
>> @@ -0,0 +1,216 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. 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 <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DevicePathLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Library/UefiDriverEntryPoint.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +#include <Library/UefiHiiServicesLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/HiiLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/DxeServicesLib.h>
>> +
>> +#include "DtPlatformDxe.h"
>> +
>> +extern  UINT8                     DtPlatformHiiBin[];
>> +extern  UINT8                     DtPlatformDxeStrings[];
>> +
>> +typedef struct {
>> +  VENDOR_DEVICE_PATH              VendorDevicePath;
>> +  EFI_DEVICE_PATH_PROTOCOL        End;
>> +} HII_VENDOR_DEVICE_PATH;
>> +
>> +STATIC HII_VENDOR_DEVICE_PATH     mDtPlatformDxeVendorDevicePath = {
>> +  {
>> +    {
>> +      HARDWARE_DEVICE_PATH,
>> +      HW_VENDOR_DP,
>> +      {
>> +        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
>> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
>> +      }
>> +    },
>> +    DT_PLATFORM_FORMSET_GUID
>> +  },
>> +  {
>> +    END_DEVICE_PATH_TYPE,
>> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> +    {
>> +      (UINT8) (END_DEVICE_PATH_LENGTH),
>> +      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
>> +    }
>> +  }
>> +};
>> +
>> +STATIC
>> +EFI_STATUS
>> +InstallHiiPages (
>> +  VOID
>> +  )
>> +{
>> +  EFI_STATUS                      Status;
>> +  EFI_HII_HANDLE                  HiiHandle;
>> +  EFI_HANDLE                      DriverHandle;
>> +
>> +  DriverHandle = NULL;
>> +  Status = gBS->InstallMultipleProtocolInterfaces (&DriverHandle,
>> +                  &gEfiDevicePathProtocolGuid,
>> +                  &mDtPlatformDxeVendorDevicePath,
>> +                  NULL);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  HiiHandle = HiiAddPackages (&gDtPlatformFormSetGuid,
>> +                              DriverHandle,
>> +                              DtPlatformDxeStrings,
>> +                              DtPlatformHiiBin,
>> +                              NULL);
>> +
>> +  if (HiiHandle == NULL) {
>> +    gBS->UninstallMultipleProtocolInterfaces (DriverHandle,
>> +                  &gEfiDevicePathProtocolGuid,
>> +                  &mDtPlatformDxeVendorDevicePath,
>> +                  NULL);
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> +  The entry point for DtPlatformDxe driver.
>> +
>> +  @param[in] ImageHandle     The image handle of the driver.
>> +  @param[in] SystemTable     The system table.
>> +
>> +  @retval EFI_ALREADY_STARTED     The driver already exists in system.
>> +  @retval EFI_OUT_OF_RESOURCES    Fail to execute entry point due to lack of
>> +                                  resources.
>> +  @retval EFI_SUCCES              All the related protocols are installed on
>> +                                  the driver.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +DtPlatformDxeEntryPoint (
>> +  IN EFI_HANDLE                   ImageHandle,
>> +  IN EFI_SYSTEM_TABLE             *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS                      Status;
>> +  DT_ACPI_VARSTORE_DATA           DtAcpiPref;
>> +  UINTN                           BufferSize;
>> +  VOID                            *Dtb;
>> +  UINTN                           DtbSize;
>> +  VOID                            *DtbCopy;
>> +
>> +  //
>> +  // Check whether a DTB blob is included in the firmware image.
>> +  //
>> +  Dtb = NULL;
>> +  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
>> +             EFI_SECTION_RAW, 0, &Dtb, &DtbSize);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_WARN, "%a: no DTB blob found, defaulting to ACPI\n",
>> +      __FUNCTION__));
>> +    DtAcpiPref.Pref = DT_ACPI_SELECT_ACPI;
>> +  } else {
>> +    //
>> +    // Get the current DT/ACPI preference from the DtAcpiPref variable.
>> +    //
>> +    BufferSize = sizeof (DtAcpiPref);
>> +    Status = gRT->GetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
>> +                    NULL, &BufferSize, &DtAcpiPref);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_WARN, "%a: no DT/ACPI preference found, defaulting to DT\n",
>> +        __FUNCTION__));
>> +      DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
>> +    }
>> +  }
>> +
>> +  if (!EFI_ERROR (Status) &&
>> +      DtAcpiPref.Pref != DT_ACPI_SELECT_ACPI &&
>> +      DtAcpiPref.Pref != DT_ACPI_SELECT_DT) {
>> +    DEBUG ((DEBUG_WARN, "%a: invalid value for DtAcpiPref, defaulting to DT\n",
>> +      __FUNCTION__));

Can you replace this instance of "... DtAcpiPref ..." with "... %a ..."
and DT_ACPI_VARIABLE_NAME? No need to resubmit.

With those warts removed:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

>> +    DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
>> +    Status = EFI_INVALID_PARAMETER; // trigger setvar below
>> +  }
>> +
>> +  //
>> +  // Write the newly selected default value back to the variable store.
>> +  //
>> +  if (EFI_ERROR (Status)) {
>> +    Status = gRT->SetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
>> +                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
>> +                    sizeof (DtAcpiPref), &DtAcpiPref);
>> +    if (EFI_ERROR (Status)) {
>> +      return Status;
>> +    }
>> +  }
>> +
>> +  if (DtAcpiPref.Pref == DT_ACPI_SELECT_ACPI) {
>> +    //
>> +    // ACPI was selected: install the gEdkiiPlatformHasAcpiGuid GUID as a
>> +    // NULL protocol to unlock dispatch of ACPI related drivers.
>> +    //
>> +    Status = gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
>> +                    &gEdkiiPlatformHasAcpiGuid, NULL, NULL);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_ERROR,
>> +        "%a: failed to install gEdkiiPlatformHasAcpiGuid as a protocol\n",
>> +        __FUNCTION__));
>> +      return Status;
>> +    }
>> +  } else if (DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
>> +    //
>> +    // DT was selected: copy the blob into newly allocated memory and install
>> +    // a reference to it as the FDT configuration table.
>> +    //
>> +    DtbCopy = AllocateCopyPool (DtbSize, Dtb);
>> +    if (DtbCopy == NULL) {
>> +      return EFI_OUT_OF_RESOURCES;
>> +    }
>> +    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DtbCopy);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_ERROR, "%a: failed to install FDT configuration table\n",
>> +        __FUNCTION__));
>> +      FreePool (DtbCopy);
>> +      return Status;
>> +    }
>> +  } else {
>> +    ASSERT (FALSE);
>> +  }
>> +
>> +  //
>> +  // No point in installing the HII pages if ACPI is the only description
>> +  // we have
>> +  //
>> +  if (Dtb == NULL) {
>> +    return EFI_SUCCESS;
>> +  }
>> +
>> +  //
>> +  // Note that we don't uninstall the gEdkiiPlatformHasAcpiGuid protocol nor
>> +  // the FDT configuration table if the following call fails. While that will
>> +  // cause loading of this driver to fail, proceeding with ACPI and DT both
>> +  // disabled will guarantee a failed boot, and so it is better to leave them
>> +  // installed in that case.
>> +  //
>> +  return InstallHiiPages ();
>> +}
>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
>> new file mode 100644
>> index 000000000000..bc995c1d0fbd
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni
>> @@ -0,0 +1,27 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2017, Linaro, Ltd. 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.
>> +*
>> +**/
>> +
>> +#langdef en-US  "English"
>> +
>> +#string STR_FORM_SET_TITLE             #language en-US "O/S Hardware Description Selection"
>> +#string STR_FORM_SET_TITLE_HELP        #language en-US "Press <Enter> to choose between ACPI and DT hardware descriptions."
>> +
>> +#string STR_MAIN_FORM_TITLE            #language en-US "O/S Hardware Description Selection"
>> +#string STR_NULL_STRING                #language en-US ""
>> +
>> +#string STR_DT_ACPI_SELECT_PROMPT      #language en-US "O/S Hardware Description"
>> +#string STR_DT_ACPI_SELECT_HELP        #language en-US "Select the hardware description that will be exposed to the O/S."
>> +
>> +#string STR_DT_ACPI_SELECT_DT          #language en-US "Device Tree"
>> +#string STR_DT_ACPI_SELECT_ACPI        #language en-US "ACPI"
>> --
>> 2.9.3
>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

Posted by Ard Biesheuvel 7 years, 7 months ago

On 28 March 2017 at 18:26, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/28/17 18:16, Ard Biesheuvel wrote:
>> (use correct address for Marcin -- please take into account when replying)
>>
>> On 28 March 2017 at 17:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> As a follow up to the changes proposed by Laszlo to make ACPI and DT
>>> mutually exclusive on ArmVirtQemu, this patch proposed a DT platform
>
> new typo relative to v1: s/proposed/proposes/
>
>>> DXE driver that either installs the NULL protocol PlatformHasAcpiGuid,
>>> or installs the FV embedded DTB binary as a configuration table under
>>> the appropriate GUID, depending on a preference setting recorded as
>>> a UEFI variable, and configurable via a HII screen.
>>>
>>> The DTB binary can be embedded in the firmware image by adding the
>>> following to the platform .fdf file:
>>>
>>>   FILE FREEFORM = 25462CDA-221F-47DF-AC1D-259CFAA4E326 {
>>>     SECTION RAW = SomePkg/path/to/foo.dtb
>>>   }
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> v2: fixup various errors and rough edges pointed out by Laszlo
>>>
>>>  EmbeddedPkg/EmbeddedPkg.dec                         |   6 +
>>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  65 ++++++
>>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h   |  31 +++
>>>  EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h |  23 +++
>>>  EmbeddedPkg/Include/Guid/DtPlatformFormSet.h        |  23 +++
>>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr |  51 +++++
>>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 216 ++++++++++++++++++++
>>>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.uni |  27 +++
>>>  8 files changed, 442 insertions(+)
>>>
>>> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
>>> index 29736fb4a71d..871fc5ff4016 100644
>>> --- a/EmbeddedPkg/EmbeddedPkg.dec
>>> +++ b/EmbeddedPkg/EmbeddedPkg.dec
>>> @@ -62,6 +62,12 @@ [Guids.common]
>>>    ## Include/Guid/PlatformHasDeviceTree.h
>>>    gEdkiiPlatformHasDeviceTreeGuid = { 0x7ebb920d, 0x1aaf, 0x46d9, { 0xb2, 0xaf, 0x54, 0x1e, 0x1d, 0xce, 0x14, 0x8b } }
>>>
>>> +  # HII form set GUID for DtPlatformDxe driver
>>> +  gDtPlatformFormSetGuid = { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
>>> +
>>> +  # File GUID for default DTB image embedded in the firmware volume
>>> +  gDtPlatformDefaultDtbFileGuid = { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
>>> +
>>>  [Protocols.common]
>>>    gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
>>>    gEfiDebugSupportPeriodicCallbackProtocolGuid = { 0x9546e07c, 0x2cbb, 0x4c88, { 0x98, 0x6c, 0xcd, 0x34, 0x10, 0x86, 0xf0, 0x44 } }
>>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>>> new file mode 100644
>>> index 000000000000..aa1d3abb4012
>>> --- /dev/null
>>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>>> @@ -0,0 +1,65 @@
>>> +## @file
>>> +#
>>> +#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
>>> +#
>>> +#  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.
>>> +#
>>> +##
>>> +
>>> +[Defines]
>>> +  INF_VERSION               = 0x00010019
>>> +  BASE_NAME                 = DtPlatformDxe
>>> +  FILE_GUID                 = FC097B3C-2EBD-4A75-A3DA-121DCAB365CC
>>> +  MODULE_TYPE               = DXE_DRIVER
>>> +  VERSION_STRING            = 1.0
>>> +  ENTRY_POINT               = DtPlatformDxeEntryPoint
>>> +
>>> +#
>>> +# The following information is for reference only and not required by the build tools.
>>> +#
>>> +#  VALID_ARCHITECTURES      = IA32 X64 ARM AARCH64
>>> +#
>>> +
>>> +[Sources]
>>> +  DtPlatformDxe.c
>>> +  DtPlatformHii.vfr
>>> +  DtPlatformHii.uni
>>> +
>>> +[Packages]
>>> +  EmbeddedPkg/EmbeddedPkg.dec
>>> +  MdePkg/MdePkg.dec
>>> +  MdeModulePkg/MdeModulePkg.dec
>>> +
>>> +[LibraryClasses]
>>> +  BaseLib
>>> +  DebugLib
>>> +  UefiLib
>>> +  UefiDriverEntryPoint
>>> +  UefiBootServicesTableLib
>>> +  UefiRuntimeServicesTableLib
>>> +  UefiHiiServicesLib
>>> +  MemoryAllocationLib
>>> +  HiiLib
>>> +  PcdLib
>>> +  DxeServicesLib
>>> +
>>> +[Guids]
>>> +  gDtPlatformFormSetGuid
>>> +  gEdkiiPlatformHasAcpiGuid
>>> +  gDtPlatformDefaultDtbFileGuid
>>> +  gFdtTableGuid
>>> +
>>> +[Protocols]
>>> +  gEfiHiiConfigAccessProtocolGuid                ## PRODUCES
>
> I think this should have been dropped as well -- you can remove it when
> you commit the patch, if the removal works. (I think it should.)
>
>>> +
>>> +[Depex]
>>> +  gEfiHiiDatabaseProtocolGuid         AND
>
> IMO this should have been dropped too. You are using UefiHiiServicesLib
> (only one instance available:
> "MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf"), and
> that instance gives you a dependency on gEfiHiiDatabaseProtocolGuid
> automatically.
>
> You never use the protocol manually, just through the following chain:
>
> * HiiAddPackages() is from HiiLib (only one instance:
> MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf), which relies on
> "gHiiDatabase"
>
> * "gHiiDatabase" is defined / initialized in UefiHiiServicesLib
> (UefiHiiServicesLibConstructor()), dependent on
> gEfiHiiDatabaseProtocolGuid, and the library instance also spells out
> the DEPEX for that.
>
> So I think, if you can drop this from the depex while keeping things
> functional, you should. No need to resubmit just because of this, again.
>
>
> Some more things which look useless for this driver:
> - PcdLib (class in INF, hdr inclusion in source)
> - ... OK I guess that's all.
>
> (In my v1 review I wrote "Basically, please make this INF as minimal as
> possible" -- apologies for not spelling it out in more detail.)
>
>>> +  gEfiVariableArchProtocolGuid        AND
>>> +  gEfiVariableWriteArchProtocolGuid
>>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
>>> new file mode 100644
>>> index 000000000000..2369367277b0
>>> --- /dev/null
>>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.h
>>> @@ -0,0 +1,31 @@
>>> +/** @file
>>> +*
>>> +*  Copyright (c) 2017, 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.
>>> +*
>>> +**/
>>> +
>>> +#ifndef __DT_PLATFORM_DXE_H__
>>> +#define __DT_PLATFORM_DXE_H__
>>> +
>>> +#include <Guid/HiiPlatformSetupFormset.h>
>>> +#include <Guid/DtPlatformFormSet.h>
>>> +
>>> +#define DT_ACPI_SELECT_DT       0x0
>>> +#define DT_ACPI_SELECT_ACPI     0x1
>>> +
>>> +#define DT_ACPI_VARIABLE_NAME   L"DtAcpiPref"
>>> +
>>> +typedef struct {
>>> +  UINT8         Pref;
>>> +  UINT8         Reserved[3];
>>> +} DT_ACPI_VARSTORE_DATA;
>>> +
>>> +#endif
>>> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
>>> new file mode 100644
>>> index 000000000000..c44b4d9522fe
>>> --- /dev/null
>>> +++ b/EmbeddedPkg/Include/Guid/DtPlatformDefaultDtbFile.h
>>> @@ -0,0 +1,23 @@
>>> +/** @file
>>> +*
>>> +*  Copyright (c) 2017, 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.
>>> +*
>>> +**/
>>> +
>>> +#ifndef __DT_PLATFORM_DEFAULT_DTB_FILE_H__
>>> +#define __DT_PLATFORM_DEFAULT_DTB_FILE_H__
>>> +
>>> +#define DT_PLATFORM_DEFAULT_DTB_FILE_GUID  \
>>> +  { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
>>> +
>>> +extern EFI_GUID gDtPlatformDefaultDtbFileGuid;
>>> +
>>> +#endif
>>> diff --git a/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
>>> new file mode 100644
>>> index 000000000000..71e3e7ebf7f3
>>> --- /dev/null
>>> +++ b/EmbeddedPkg/Include/Guid/DtPlatformFormSet.h
>>> @@ -0,0 +1,23 @@
>>> +/** @file
>>> +*
>>> +*  Copyright (c) 2017, 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.
>>> +*
>>> +**/
>>> +
>>> +#ifndef __DT_PLATFORM_FORMSET_H__
>>> +#define __DT_PLATFORM_FORMSET_H__
>>> +
>>> +#define DT_PLATFORM_FORMSET_GUID  \
>>> +  { 0x2b7a240d, 0xd5ad, 0x4fd6, { 0xbe, 0x1c, 0xdf, 0xa4, 0x41, 0x5f, 0x55, 0x26 } }
>>> +
>>> +extern EFI_GUID gDtPlatformFormSetGuid;
>>> +
>>> +#endif
>>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
>>> new file mode 100644
>>> index 000000000000..3516746c4d84
>>> --- /dev/null
>>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
>>> @@ -0,0 +1,51 @@
>>> +/** @file
>>> +*
>>> +*  Copyright (c) 2017, Linaro, Ltd. 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 "DtPlatformDxe.h"
>>> +
>>> +//
>>> +// EFI Variable attributes
>>> +//
>>> +#define EFI_VARIABLE_NON_VOLATILE       0x00000001
>>> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
>>> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x00000004
>>> +#define EFI_VARIABLE_READ_ONLY          0x00000008
>>> +
>>> +formset
>>> +  guid      = DT_PLATFORM_FORMSET_GUID,
>>> +  title     = STRING_TOKEN(STR_FORM_SET_TITLE),
>>> +  help      = STRING_TOKEN(STR_FORM_SET_TITLE_HELP),
>>> +  classguid = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
>>> +
>>> +  efivarstore DT_ACPI_VARSTORE_DATA,
>>> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,  // EFI variable attributes
>>> +    name  = DtAcpiPref,
>>> +    guid  = DT_PLATFORM_FORMSET_GUID;
>>> +
>>> +  form formid = 0x1000,
>>> +    title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
>>> +
>>> +    oneof varid = DtAcpiPref.Pref,
>>> +        prompt      = STRING_TOKEN(STR_DT_ACPI_SELECT_PROMPT),
>>> +        help        = STRING_TOKEN(STR_DT_ACPI_SELECT_HELP),
>>> +        flags       = NUMERIC_SIZE_1 | INTERACTIVE | RESET_REQUIRED,
>>> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_DT), value = DT_ACPI_SELECT_DT, flags = DEFAULT;
>>> +        option text = STRING_TOKEN(STR_DT_ACPI_SELECT_ACPI), value = DT_ACPI_SELECT_ACPI, flags = 0;
>>> +    endoneof;
>>> +
>>> +    subtitle text = STRING_TOKEN(STR_NULL_STRING);
>>> +
>>> +  endform;
>>> +
>>> +endformset;
>>> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
>>> new file mode 100644
>>> index 000000000000..f714551213c2
>>> --- /dev/null
>>> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
>>> @@ -0,0 +1,216 @@
>>> +/** @file
>>> +*
>>> +*  Copyright (c) 2017, Linaro, Ltd. 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 <Library/BaseLib.h>
>>> +#include <Library/DebugLib.h>
>>> +#include <Library/DevicePathLib.h>
>>> +#include <Library/UefiLib.h>
>>> +#include <Library/UefiDriverEntryPoint.h>
>>> +#include <Library/UefiBootServicesTableLib.h>
>>> +#include <Library/UefiRuntimeServicesTableLib.h>
>>> +#include <Library/UefiHiiServicesLib.h>
>>> +#include <Library/MemoryAllocationLib.h>
>>> +#include <Library/HiiLib.h>
>>> +#include <Library/PcdLib.h>
>>> +#include <Library/DxeServicesLib.h>
>>> +
>>> +#include "DtPlatformDxe.h"
>>> +
>>> +extern  UINT8                     DtPlatformHiiBin[];
>>> +extern  UINT8                     DtPlatformDxeStrings[];
>>> +
>>> +typedef struct {
>>> +  VENDOR_DEVICE_PATH              VendorDevicePath;
>>> +  EFI_DEVICE_PATH_PROTOCOL        End;
>>> +} HII_VENDOR_DEVICE_PATH;
>>> +
>>> +STATIC HII_VENDOR_DEVICE_PATH     mDtPlatformDxeVendorDevicePath = {
>>> +  {
>>> +    {
>>> +      HARDWARE_DEVICE_PATH,
>>> +      HW_VENDOR_DP,
>>> +      {
>>> +        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
>>> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
>>> +      }
>>> +    },
>>> +    DT_PLATFORM_FORMSET_GUID
>>> +  },
>>> +  {
>>> +    END_DEVICE_PATH_TYPE,
>>> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
>>> +    {
>>> +      (UINT8) (END_DEVICE_PATH_LENGTH),
>>> +      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
>>> +    }
>>> +  }
>>> +};
>>> +
>>> +STATIC
>>> +EFI_STATUS
>>> +InstallHiiPages (
>>> +  VOID
>>> +  )
>>> +{
>>> +  EFI_STATUS                      Status;
>>> +  EFI_HII_HANDLE                  HiiHandle;
>>> +  EFI_HANDLE                      DriverHandle;
>>> +
>>> +  DriverHandle = NULL;
>>> +  Status = gBS->InstallMultipleProtocolInterfaces (&DriverHandle,
>>> +                  &gEfiDevicePathProtocolGuid,
>>> +                  &mDtPlatformDxeVendorDevicePath,
>>> +                  NULL);
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  HiiHandle = HiiAddPackages (&gDtPlatformFormSetGuid,
>>> +                              DriverHandle,
>>> +                              DtPlatformDxeStrings,
>>> +                              DtPlatformHiiBin,
>>> +                              NULL);
>>> +
>>> +  if (HiiHandle == NULL) {
>>> +    gBS->UninstallMultipleProtocolInterfaces (DriverHandle,
>>> +                  &gEfiDevicePathProtocolGuid,
>>> +                  &mDtPlatformDxeVendorDevicePath,
>>> +                  NULL);
>>> +    return EFI_OUT_OF_RESOURCES;
>>> +  }
>>> +  return EFI_SUCCESS;
>>> +}
>>> +
>>> +/**
>>> +  The entry point for DtPlatformDxe driver.
>>> +
>>> +  @param[in] ImageHandle     The image handle of the driver.
>>> +  @param[in] SystemTable     The system table.
>>> +
>>> +  @retval EFI_ALREADY_STARTED     The driver already exists in system.
>>> +  @retval EFI_OUT_OF_RESOURCES    Fail to execute entry point due to lack of
>>> +                                  resources.
>>> +  @retval EFI_SUCCES              All the related protocols are installed on
>>> +                                  the driver.
>>> +
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +DtPlatformDxeEntryPoint (
>>> +  IN EFI_HANDLE                   ImageHandle,
>>> +  IN EFI_SYSTEM_TABLE             *SystemTable
>>> +  )
>>> +{
>>> +  EFI_STATUS                      Status;
>>> +  DT_ACPI_VARSTORE_DATA           DtAcpiPref;
>>> +  UINTN                           BufferSize;
>>> +  VOID                            *Dtb;
>>> +  UINTN                           DtbSize;
>>> +  VOID                            *DtbCopy;
>>> +
>>> +  //
>>> +  // Check whether a DTB blob is included in the firmware image.
>>> +  //
>>> +  Dtb = NULL;
>>> +  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
>>> +             EFI_SECTION_RAW, 0, &Dtb, &DtbSize);
>>> +  if (EFI_ERROR (Status)) {
>>> +    DEBUG ((DEBUG_WARN, "%a: no DTB blob found, defaulting to ACPI\n",
>>> +      __FUNCTION__));
>>> +    DtAcpiPref.Pref = DT_ACPI_SELECT_ACPI;
>>> +  } else {
>>> +    //
>>> +    // Get the current DT/ACPI preference from the DtAcpiPref variable.
>>> +    //
>>> +    BufferSize = sizeof (DtAcpiPref);
>>> +    Status = gRT->GetVariable(DT_ACPI_VARIABLE_NAME, &gDtPlatformFormSetGuid,
>>> +                    NULL, &BufferSize, &DtAcpiPref);
>>> +    if (EFI_ERROR (Status)) {
>>> +      DEBUG ((DEBUG_WARN, "%a: no DT/ACPI preference found, defaulting to DT\n",
>>> +        __FUNCTION__));
>>> +      DtAcpiPref.Pref = DT_ACPI_SELECT_DT;
>>> +    }
>>> +  }
>>> +
>>> +  if (!EFI_ERROR (Status) &&
>>> +      DtAcpiPref.Pref != DT_ACPI_SELECT_ACPI &&
>>> +      DtAcpiPref.Pref != DT_ACPI_SELECT_DT) {
>>> +    DEBUG ((DEBUG_WARN, "%a: invalid value for DtAcpiPref, defaulting to DT\n",
>>> +      __FUNCTION__));
>
> Can you replace this instance of "... DtAcpiPref ..." with "... %a ..."
> and DT_ACPI_VARIABLE_NAME? No need to resubmit.
>

That should be %s, I suppose?

> With those warts removed:
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>

Thanks!
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

Posted by Laszlo Ersek 7 years, 7 months ago

On 03/28/17 19:30, Ard Biesheuvel wrote:
> On 28 March 2017 at 18:26, Laszlo Ersek <lersek@redhat.com> wrote:

>>>> +#define DT_ACPI_VARIABLE_NAME   L"DtAcpiPref"

>> Can you replace this instance of "... DtAcpiPref ..." with "... %a ..."
>> and DT_ACPI_VARIABLE_NAME? No need to resubmit.
>>
> 
> That should be %s, I suppose?

Heh, sure. Forgot the L by the time I got there :)

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI

Posted by Ard Biesheuvel 7 years, 7 months ago

On 28 March 2017 at 19:00, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/28/17 19:30, Ard Biesheuvel wrote:
>> On 28 March 2017 at 18:26, Laszlo Ersek <lersek@redhat.com> wrote:
>
>>>>> +#define DT_ACPI_VARIABLE_NAME   L"DtAcpiPref"
>
>>> Can you replace this instance of "... DtAcpiPref ..." with "... %a ..."
>>> and DT_ACPI_VARIABLE_NAME? No need to resubmit.
>>>
>>
>> That should be %s, I suppose?
>
> Heh, sure. Forgot the L by the time I got there :)
>


Pushed as 779cc439e881

Thanks,
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel