| Message ID | 1408994367-11143-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 08/25/14 21:19, Ard Biesheuvel wrote: > This adds the possibility to include a DTB blob into the firmware image, and > have it installed as a configuration under the correct GUID at UEFI init time. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > MdeModulePkg/MdeModulePkg.dec | 2 + > MdeModulePkg/MdeModulePkg.dsc | 2 + > .../Universal/Fdt/FdtTableDxe/FdtTableDxe.c | 33 +++++++++++++++ > .../Universal/Fdt/FdtTableDxe/FdtTableDxe.inf | 48 ++++++++++++++++++++++ > MdePkg/Include/Guid/FdtTable.h | 26 ++++++++++++ > MdePkg/MdePkg.dec | 3 ++ > 6 files changed, 114 insertions(+) > create mode 100644 MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c > create mode 100644 MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf > create mode 100644 MdePkg/Include/Guid/FdtTable.h > > diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec > index e04246a2f419..d4d2a423256c 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -692,6 +692,8 @@ > ## Default Creator Revision for ACPI table creation. > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|0x01000013|UINT32|0x30001038 > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFdtImage|{ 0x66,0x0f,0xe1,0x96,0xa5,0x0f,0x43,0x8c,0xa9,0x50,0xbe,0x6a,0x58,0xb9,0x12,0x1b }|VOID*|0x30001041 > + > [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > ## This PCD defines the Console output column and the default value is 25 according to UEFI spec. > # This PCD could be set to 0 then console output could be at max column and max row. > diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc > index f4733253442b..616387a746c1 100644 > --- a/MdeModulePkg/MdeModulePkg.dsc > +++ b/MdeModulePkg/MdeModulePkg.dsc > @@ -322,6 +322,8 @@ > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf > MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf > > + MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf > + > [Components.IA32, Components.X64, Components.IPF] > MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf > MdeModulePkg/Universal/DebugSupportDxe/DebugSupportDxe.inf > diff --git a/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c > new file mode 100644 > index 000000000000..c9b8b4c42464 > --- /dev/null > +++ b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c > @@ -0,0 +1,33 @@ > + > +#include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/UefiLib.h> > +#include <Library/BaseMemoryLib.h> > +#include <Library/UefiDriverEntryPoint.h> > +#include <Library/MemoryAllocationLib.h> > +#include <Library/UefiBootServicesTableLib.h> > +#include <Library/PcdLib.h> > +#include <Library/DxeServicesLib.h> > + > +#include <Guid/FdtTable.h> > + > +EFI_STATUS > +EFIAPI > +InitializeFdtTableDxe ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + VOID *FdtImageData; > + UINTN FdtImageSize; > + > + Status = GetSectionFromAnyFv(PcdGetPtr(PcdFdtImage), EFI_SECTION_RAW, 0, > + &FdtImageData, &FdtImageSize); > + if (EFI_ERROR(Status)) > + return Status; > + > + DEBUG((EFI_D_ERROR, "InitializeFdtTableDxe: DTB @ 0x%08x\n", FdtImageData)); > + > + return gBS->InstallConfigurationTable(&gEfiFdtTableGuid, FdtImageData); > +} > diff --git a/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf > new file mode 100644 > index 000000000000..993b778b8f66 > --- /dev/null > +++ b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf > @@ -0,0 +1,48 @@ > +## @file > +# FDT Table Protocol Driver > +# > +# Copyright (c) 2014, 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 = 0x00010005 > + BASE_NAME = FdtTableDxe > + FILE_GUID = 261F9737-CDF6-46CE-A9E2-5DD16B957413 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + > + ENTRY_POINT = InitializeFdtTableDxe > + > +[Sources] > + FdtTableDxe.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + > +[LibraryClasses] > + BaseLib > + PcdLib > + UefiDriverEntryPoint > + DxeServicesLib > + > +[Guids] > + gEfiFdtTableGuid > + > +[FeaturePcd] > + > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFdtImage > + > +[Protocols] > + > +[Depex] > + TRUE > diff --git a/MdePkg/Include/Guid/FdtTable.h b/MdePkg/Include/Guid/FdtTable.h > new file mode 100644 > index 000000000000..ab2e0bc43940 > --- /dev/null > +++ b/MdePkg/Include/Guid/FdtTable.h > @@ -0,0 +1,26 @@ > +/** @file > + GUIDs used to locate the FDT image in the UEFI 2.x system table. > + > + Copyright (c) 2014 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. > + > +**/ > + > +#ifndef __FDT_TABLE_GUID_H__ > +#define __FDT_TABLE_GUID_H__ > + > +#define FDT_TABLE_GUID \ > + { \ > + 0xb1b621d5, 0xf19c, 0x41a5, { 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 } \ > + } > + > +extern EFI_GUID gEfiFdtTableGuid; > + > +#endif > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec > index 4daf3e6a75ea..574561b9f675 100644 > --- a/MdePkg/MdePkg.dec > +++ b/MdePkg/MdePkg.dec > @@ -615,6 +615,9 @@ > ## Include/Guid/VectorHandoffTable.h > gEfiVectorHandoffTableGuid = { 0x996ec11c, 0x5397, 0x4e73, { 0xb5, 0x8f, 0x82, 0x7e, 0x52, 0x90, 0x6d, 0xef }} > > + ## Include/Guid/FdtTable.h > + gEfiFdtTableGuid = { 0xb1b621d5, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 }} > + > [Guids.IA32, Guids.X64] > ## Include/Guid/Cper.h > gEfiIa32X64ErrorTypeCacheCheckGuid = { 0xA55701F5, 0xE3EF, 0x43de, { 0xAC, 0x72, 0x24, 0x9B, 0x57, 0x3F, 0xAD, 0x2C }} > I'm trying to familiarize myself with the structure of this series. Before I get to the real meat (or what I consider to be the "real meat"), I'd like to ask why we need this patch. I searched the other patches for "FdtTableDxe", and I got no hits. As far as I understand, this functionality is useful for the linaro tree, and some of the physical hardware platforms. In a virtual machine though, the DTB will always come from QEMU, so for aarch64 VM support, this patch doesn't appear to be necessary. Can you drop it from this series? (Along with 02/10, using the FDF change you described before.) I'd like to reduce the things I must understand to a minimum. Additionally, adding anything at all to MdePkg and MdeModulePkg is pretty hard in upstream edk2, so if we don't have to, let's not try. :) If you drop the first two patches, then your series remains in the confines of ArmPkg and ArmPlatformPkg. I can't stress enough how much easier that makes it for your series to get applied. Thanks, Laszlo ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
On 26 August 2014 11:39, Laszlo Ersek <lersek@redhat.com> wrote: > On 08/25/14 21:19, Ard Biesheuvel wrote: >> This adds the possibility to include a DTB blob into the firmware image, and >> have it installed as a configuration under the correct GUID at UEFI init time. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- [...] > > I'm trying to familiarize myself with the structure of this series. > > Before I get to the real meat (or what I consider to be the "real > meat"), I'd like to ask why we need this patch. I searched the other > patches for "FdtTableDxe", and I got no hits. > This patch does two things: - declare a GUID for a device tree configuration table - implement a driver that takes an embedded FV containing a device tree blob and install it as a configuration table under the declared GUID The latter part is not used in this series, but the GUID /is/ used by VirtFdtDxe.c to install the device tree as a config table. This is what allows the kernel to boot straight from UEFI without having to pass -dtb arguments and have the EFI stub perform the load. Granted, this is somewhat of a bonus, and the EFI stub does support dtb=, but we disable it for secure boot because loading unauthenticated data and install it right into the heart of the kernel is considered a security hazard. So i could split it up and move the part we need under ArmPkg, and drop the other half, I suppose. I know Olivier is also working on device tree integration, so perhaps he could chime in as well? @Olivier:? > As far as I understand, this functionality is useful for the linaro > tree, and some of the physical hardware platforms. In a virtual machine > though, the DTB will always come from QEMU, so for aarch64 VM support, > this patch doesn't appear to be necessary. > The point is not where the device tree came from, but where it goes to ... > Can you drop it from this series? (Along with 02/10, using the FDF > change you described before.) I'd like to reduce the things I must > understand to a minimum. > > Additionally, adding anything at all to MdePkg and MdeModulePkg is > pretty hard in upstream edk2, so if we don't have to, let's not try. :) > If you drop the first two patches, then your series remains in the > confines of ArmPkg and ArmPlatformPkg. I can't stress enough how much > easier that makes it for your series to get applied. > Agreed.
The GUID has already been declared in EmbeddedPkg/Include/Guid/Fdt.h. I have just pushed some helper functions to load FDT from Firmware Volume (svn rev15905) or File System (svn rev15908). These functions could be used by VirtFdtDxe.c. > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: 26 August 2014 11:12 > To: Laszlo Ersek > Cc: Olivier Martin; edk2-devel@lists.sourceforge.net; Peter Maydell; > Christoffer Dall; Andrew Jones; Ilias Biris; Leif Lindholm > Subject: Re: [PATCH 01/10] Add minimal support for passing a device > tree image > > On 26 August 2014 11:39, Laszlo Ersek <lersek@redhat.com> wrote: > > On 08/25/14 21:19, Ard Biesheuvel wrote: > >> This adds the possibility to include a DTB blob into the firmware > image, and > >> have it installed as a configuration under the correct GUID at UEFI > init time. > >> > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > [...] > > > > I'm trying to familiarize myself with the structure of this series. > > > > Before I get to the real meat (or what I consider to be the "real > > meat"), I'd like to ask why we need this patch. I searched the other > > patches for "FdtTableDxe", and I got no hits. > > > > This patch does two things: > - declare a GUID for a device tree configuration table > - implement a driver that takes an embedded FV containing a device > tree blob and install it as a configuration table under the declared > GUID > > The latter part is not used in this series, but the GUID /is/ used by > VirtFdtDxe.c to install the device tree as a config table. This is > what allows the kernel to boot straight from UEFI without having to > pass -dtb arguments and have the EFI stub perform the load. Granted, > this is somewhat of a bonus, and the EFI stub does support dtb=, but > we disable it for secure boot because loading unauthenticated data and > install it right into the heart of the kernel is considered a security > hazard. > > So i could split it up and move the part we need under ArmPkg, and > drop the other half, I suppose. > I know Olivier is also working on device tree integration, so perhaps > he could chime in as well? > > @Olivier:? > > > As far as I understand, this functionality is useful for the linaro > > tree, and some of the physical hardware platforms. In a virtual > machine > > though, the DTB will always come from QEMU, so for aarch64 VM > support, > > this patch doesn't appear to be necessary. > > > > The point is not where the device tree came from, but where it goes to > ... > > > Can you drop it from this series? (Along with 02/10, using the FDF > > change you described before.) I'd like to reduce the things I must > > understand to a minimum. > > > > Additionally, adding anything at all to MdePkg and MdeModulePkg is > > pretty hard in upstream edk2, so if we don't have to, let's not try. > :) > > If you drop the first two patches, then your series remains in the > > confines of ArmPkg and ArmPlatformPkg. I can't stress enough how much > > easier that makes it for your series to get applied. > > > > Agreed. > > -- > Ard. ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index e04246a2f419..d4d2a423256c 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -692,6 +692,8 @@ ## Default Creator Revision for ACPI table creation. gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|0x01000013|UINT32|0x30001038 + gEfiMdeModulePkgTokenSpaceGuid.PcdFdtImage|{ 0x66,0x0f,0xe1,0x96,0xa5,0x0f,0x43,0x8c,0xa9,0x50,0xbe,0x6a,0x58,0xb9,0x12,0x1b }|VOID*|0x30001041 + [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## This PCD defines the Console output column and the default value is 25 according to UEFI spec. # This PCD could be set to 0 then console output could be at max column and max row. diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc index f4733253442b..616387a746c1 100644 --- a/MdeModulePkg/MdeModulePkg.dsc +++ b/MdeModulePkg/MdeModulePkg.dsc @@ -322,6 +322,8 @@ MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf + MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf + [Components.IA32, Components.X64, Components.IPF] MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf MdeModulePkg/Universal/DebugSupportDxe/DebugSupportDxe.inf diff --git a/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c new file mode 100644 index 000000000000..c9b8b4c42464 --- /dev/null +++ b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c @@ -0,0 +1,33 @@ + +#include <Library/BaseLib.h> +#include <Library/DebugLib.h> +#include <Library/UefiLib.h> +#include <Library/BaseMemoryLib.h> +#include <Library/UefiDriverEntryPoint.h> +#include <Library/MemoryAllocationLib.h> +#include <Library/UefiBootServicesTableLib.h> +#include <Library/PcdLib.h> +#include <Library/DxeServicesLib.h> + +#include <Guid/FdtTable.h> + +EFI_STATUS +EFIAPI +InitializeFdtTableDxe ( + IN EFI_HANDLE ImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + EFI_STATUS Status; + VOID *FdtImageData; + UINTN FdtImageSize; + + Status = GetSectionFromAnyFv(PcdGetPtr(PcdFdtImage), EFI_SECTION_RAW, 0, + &FdtImageData, &FdtImageSize); + if (EFI_ERROR(Status)) + return Status; + + DEBUG((EFI_D_ERROR, "InitializeFdtTableDxe: DTB @ 0x%08x\n", FdtImageData)); + + return gBS->InstallConfigurationTable(&gEfiFdtTableGuid, FdtImageData); +} diff --git a/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf new file mode 100644 index 000000000000..993b778b8f66 --- /dev/null +++ b/MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf @@ -0,0 +1,48 @@ +## @file +# FDT Table Protocol Driver +# +# Copyright (c) 2014, 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 = 0x00010005 + BASE_NAME = FdtTableDxe + FILE_GUID = 261F9737-CDF6-46CE-A9E2-5DD16B957413 + MODULE_TYPE = DXE_DRIVER + VERSION_STRING = 1.0 + + ENTRY_POINT = InitializeFdtTableDxe + +[Sources] + FdtTableDxe.c + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + +[LibraryClasses] + BaseLib + PcdLib + UefiDriverEntryPoint + DxeServicesLib + +[Guids] + gEfiFdtTableGuid + +[FeaturePcd] + +[Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdFdtImage + +[Protocols] + +[Depex] + TRUE diff --git a/MdePkg/Include/Guid/FdtTable.h b/MdePkg/Include/Guid/FdtTable.h new file mode 100644 index 000000000000..ab2e0bc43940 --- /dev/null +++ b/MdePkg/Include/Guid/FdtTable.h @@ -0,0 +1,26 @@ +/** @file + GUIDs used to locate the FDT image in the UEFI 2.x system table. + + Copyright (c) 2014 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. + +**/ + +#ifndef __FDT_TABLE_GUID_H__ +#define __FDT_TABLE_GUID_H__ + +#define FDT_TABLE_GUID \ + { \ + 0xb1b621d5, 0xf19c, 0x41a5, { 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 } \ + } + +extern EFI_GUID gEfiFdtTableGuid; + +#endif diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index 4daf3e6a75ea..574561b9f675 100644 --- a/MdePkg/MdePkg.dec +++ b/MdePkg/MdePkg.dec @@ -615,6 +615,9 @@ ## Include/Guid/VectorHandoffTable.h gEfiVectorHandoffTableGuid = { 0x996ec11c, 0x5397, 0x4e73, { 0xb5, 0x8f, 0x82, 0x7e, 0x52, 0x90, 0x6d, 0xef }} + ## Include/Guid/FdtTable.h + gEfiFdtTableGuid = { 0xb1b621d5, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 }} + [Guids.IA32, Guids.X64] ## Include/Guid/Cper.h gEfiIa32X64ErrorTypeCacheCheckGuid = { 0xA55701F5, 0xE3EF, 0x43de, { 0xAC, 0x72, 0x24, 0x9B, 0x57, 0x3F, 0xAD, 0x2C }}
This adds the possibility to include a DTB blob into the firmware image, and have it installed as a configuration under the correct GUID at UEFI init time. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- MdeModulePkg/MdeModulePkg.dec | 2 + MdeModulePkg/MdeModulePkg.dsc | 2 + .../Universal/Fdt/FdtTableDxe/FdtTableDxe.c | 33 +++++++++++++++ .../Universal/Fdt/FdtTableDxe/FdtTableDxe.inf | 48 ++++++++++++++++++++++ MdePkg/Include/Guid/FdtTable.h | 26 ++++++++++++ MdePkg/MdePkg.dec | 3 ++ 6 files changed, 114 insertions(+) create mode 100644 MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.c create mode 100644 MdeModulePkg/Universal/Fdt/FdtTableDxe/FdtTableDxe.inf create mode 100644 MdePkg/Include/Guid/FdtTable.h