* [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI
@ 2017-03-28 16:15 Ard Biesheuvel
2017-03-28 16:16 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2017-03-28 16:15 UTC (permalink / raw)
To: edk2-devel, lersek; +Cc: mwojtas, leif.lindholm, Ard Biesheuvel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI
2017-03-28 16:15 [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI Ard Biesheuvel
@ 2017-03-28 16:16 ` Ard Biesheuvel
2017-03-28 16:26 ` Ard Biesheuvel
2017-03-28 17:26 ` Laszlo Ersek
0 siblings, 2 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-03-28 16:16 UTC (permalink / raw)
To: edk2-devel@lists.01.org, Laszlo Ersek, Marcin Wojtas
Cc: Leif Lindholm, Ard Biesheuvel
(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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI
2017-03-28 16:16 ` Ard Biesheuvel
@ 2017-03-28 16:26 ` Ard Biesheuvel
2017-03-28 17:26 ` Laszlo Ersek
1 sibling, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-03-28 16:26 UTC (permalink / raw)
To: edk2-devel@lists.01.org, Laszlo Ersek, Marcin Wojtas
Cc: Leif Lindholm, Ard Biesheuvel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI
2017-03-28 16:16 ` Ard Biesheuvel
2017-03-28 16:26 ` Ard Biesheuvel
@ 2017-03-28 17:26 ` Laszlo Ersek
2017-03-28 17:30 ` Ard Biesheuvel
1 sibling, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2017-03-28 17:26 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org, Marcin Wojtas; +Cc: Leif Lindholm
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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI
2017-03-28 17:26 ` Laszlo Ersek
@ 2017-03-28 17:30 ` Ard Biesheuvel
2017-03-28 18:00 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2017-03-28 17:30 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Marcin Wojtas, Leif Lindholm
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!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI
2017-03-28 17:30 ` Ard Biesheuvel
@ 2017-03-28 18:00 ` Laszlo Ersek
2017-03-28 18:01 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2017-03-28 18:00 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Marcin Wojtas, Leif Lindholm
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 :)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI
2017-03-28 18:00 ` Laszlo Ersek
@ 2017-03-28 18:01 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-03-28 18:01 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Marcin Wojtas, Leif Lindholm
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,
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-28 18:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-28 16:15 [PATCH v2] EmbeddedPkg: add DT platform driver to select between DT and ACPI Ard Biesheuvel
2017-03-28 16:16 ` Ard Biesheuvel
2017-03-28 16:26 ` Ard Biesheuvel
2017-03-28 17:26 ` Laszlo Ersek
2017-03-28 17:30 ` Ard Biesheuvel
2017-03-28 18:00 ` Laszlo Ersek
2017-03-28 18:01 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox