diff mbox series

[edk2,v2,2/3] EmbeddedPkg: add base DtPlatformDtbLoaderLib implementation

Message ID 20170331105607.3477-3-ard.biesheuvel@linaro.org
State New
Headers show
Series EmbeddedPkg: revert DTB loading to platform lib | expand

Commit Message

Ard Biesheuvel March 31, 2017, 10:56 a.m. UTC 
Introduce an implementation of the new DtPlatformDtbLoaderLib library
class that simply retrieves the first raw section for an FV file named
gDtPlatformDefaultDtbFileGuid.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 EmbeddedPkg/EmbeddedPkg.dsc                                                   |  4 ++
 EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c   | 60 ++++++++++++++++++++
 EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf | 36 ++++++++++++
 3 files changed, 100 insertions(+)

-- 
2.9.3

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

Comments

Laszlo Ersek March 31, 2017, 11:29 a.m. UTC | #1 
On 03/31/17 12:56, Ard Biesheuvel wrote:
> Introduce an implementation of the new DtPlatformDtbLoaderLib library

> class that simply retrieves the first raw section for an FV file named


s/for/of/ ?

> gDtPlatformDefaultDtbFileGuid.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  EmbeddedPkg/EmbeddedPkg.dsc                                                   |  4 ++

>  EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c   | 60 ++++++++++++++++++++

>  EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf | 36 ++++++++++++

>  3 files changed, 100 insertions(+)

> 

> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc

> index ba4f1ea0f004..0d5db68631bb 100644

> --- a/EmbeddedPkg/EmbeddedPkg.dsc

> +++ b/EmbeddedPkg/EmbeddedPkg.dsc

> @@ -109,6 +109,9 @@ [LibraryClasses.common]

>    HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf

>    UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf

>  

> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf

> +  DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf

> +

>  [LibraryClasses.common.DXE_DRIVER]

>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf

>    ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf

> @@ -247,6 +250,7 @@ [Components.common]

>    EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf

>    EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf

>    EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf

> +  EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf

>  

>    EmbeddedPkg/Ebl/Ebl.inf

>  ####  EmbeddedPkg/EblExternCmd/EblExternCmd.inf

> diff --git a/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c

> new file mode 100644

> index 000000000000..313d0b104681

> --- /dev/null

> +++ b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c

> @@ -0,0 +1,60 @@

> +/** @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 <PiDxe.h>

> +

> +#include <Library/BaseLib.h>

> +#include <Library/DxeServicesLib.h>

> +#include <Library/MemoryAllocationLib.h>

> +

> +/**

> +  Return a pool allocated copy of the DTB image that is appropriate for

> +  booting the current platform via DT.

> +

> +  @param[out]   Dtb                   Pointer to the DTB copy

> +  @param[out]   DtbSize               Size of the DTB copy

> +

> +  @retval       EFI_SUCCESS           Operation completed successfully

> +  @retval       EFI_NOT_FOUND         No suitable DTB image could be located

> +  @retval       EFI_OUT_OF_RESOURCES  No pool memory available

> +

> +**/

> +EFI_STATUS

> +EFIAPI

> +DtPlatformLoadDtb (

> +  OUT   VOID        **Dtb,

> +  OUT   UINTN       *DtbSize

> +  )

> +{

> +  EFI_STATUS      Status;

> +  VOID            *OrigDtb;

> +  VOID            *CopyDtb;

> +  UINTN           OrigDtbSize;

> +

> +  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,

> +             EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);

> +  if (EFI_ERROR (Status)) {

> +    return EFI_NOT_FOUND;

> +  }

> +

> +  CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);

> +  if (CopyDtb == NULL) {

> +    return EFI_OUT_OF_RESOURCES;

> +  }

> +

> +  *Dtb = CopyDtb;

> +  *DtbSize = OrigDtbSize;

> +

> +  return EFI_SUCCESS;

> +}

> diff --git a/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf

> new file mode 100644

> index 000000000000..7c337aace3ef

> --- /dev/null

> +++ b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf

> @@ -0,0 +1,36 @@

> +/** @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.

> +*

> +**/

> +

> +[Defines]

> +  INF_VERSION                    = 0x00010019

> +  BASE_NAME                      = DtPlatformDtbLoaderBaseLib

> +  FILE_GUID                      = 419a1910-70da-4c99-8696-ba81a57be508

> +  MODULE_TYPE                    = DXE_DRIVER

> +  VERSION_STRING                 = 1.0

> +  LIBRARY_CLASS                  = DtPlatformDtbLoaderLib|DXE_DRIVER

> +

> +[Sources]

> +  DtPlatformDtbLoaderBaseLib.c

> +

> +[Packages]

> +  MdePkg/MdePkg.dec

> +  EmbeddedPkg/EmbeddedPkg.dec

> +

> +[LibraryClasses]

> +  BaseLib

> +  DxeServicesLib

> +  MemoryAllocationLib

> +

> +[Guids]

> +  gDtPlatformDefaultDtbFileGuid

> 


This patch looks good, but I think the naming isn't right. Generally we
form the names of library instances in the following pattern:

<phase or module type> + <library class name> + <implementation details>

For example, if you have a library class called FooBarLib, then the
do-nothing instance is usually:

  Base + FooBarLib + Null,

that is, BaseFoobarLibNull. Examples:

$ ls -1d Mde{,Module}Pkg/Library/Base*LibNull

MdeModulePkg/Library/BaseIpmiLibNull
MdeModulePkg/Library/BasePlatformHookLibNull
MdeModulePkg/Library/BaseResetSystemLibNull
MdePkg/Library/BaseDebugLibNull
MdePkg/Library/BasePalLibNull
MdePkg/Library/BasePcdLibNull
MdePkg/Library/BasePeCoffExtraActionLibNull
MdePkg/Library/BasePerformanceLibNull
MdePkg/Library/BaseReportStatusCodeLibNull
MdePkg/Library/BaseS3BootScriptLibNull
MdePkg/Library/BaseSerialPortLibNull
MdePkg/Library/BaseSmbusLibNull

The first component, <phase or module type> can be Base (suitable for
all modules), Pei, Dxe, Uefi, or a combination thereof.

$ ls -1d Mde{,Module}Pkg/Library/Pei*Lib*
$ ls -1d Mde{,Module}Pkg/Library/Dxe*Lib*
$ ls -1d Mde{,Module}Pkg/Library/Uefi*Lib*

Finally, the last component of the lib instance name is usually a hint
to the core implementation trait of the library instance, the one that
basically sets it apart from all other lib instances of the same class.

Therefore, I suggest the following name for this instance:

  DxeDtPlatformDtbLoaderLibFfsFileSection0

Feel free to replace "FfsFileSection0" with something less gross, if you
like.

With the commit message and the lib instance name updated (please locate
all occurrences -- file names, file contents, and commit message too):

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


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

Patch

diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index ba4f1ea0f004..0d5db68631bb 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -109,6 +109,9 @@  [LibraryClasses.common]
   HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
   UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
 
+  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+  DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf
+
 [LibraryClasses.common.DXE_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
   ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
@@ -247,6 +250,7 @@  [Components.common]
   EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf
   EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
   EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
+  EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf
 
   EmbeddedPkg/Ebl/Ebl.inf
 ####  EmbeddedPkg/EblExternCmd/EblExternCmd.inf
diff --git a/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c
new file mode 100644
index 000000000000..313d0b104681
--- /dev/null
+++ b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.c
@@ -0,0 +1,60 @@ 
+/** @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 <PiDxe.h>
+
+#include <Library/BaseLib.h>
+#include <Library/DxeServicesLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+/**
+  Return a pool allocated copy of the DTB image that is appropriate for
+  booting the current platform via DT.
+
+  @param[out]   Dtb                   Pointer to the DTB copy
+  @param[out]   DtbSize               Size of the DTB copy
+
+  @retval       EFI_SUCCESS           Operation completed successfully
+  @retval       EFI_NOT_FOUND         No suitable DTB image could be located
+  @retval       EFI_OUT_OF_RESOURCES  No pool memory available
+
+**/
+EFI_STATUS
+EFIAPI
+DtPlatformLoadDtb (
+  OUT   VOID        **Dtb,
+  OUT   UINTN       *DtbSize
+  )
+{
+  EFI_STATUS      Status;
+  VOID            *OrigDtb;
+  VOID            *CopyDtb;
+  UINTN           OrigDtbSize;
+
+  Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
+             EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
+  if (EFI_ERROR (Status)) {
+    return EFI_NOT_FOUND;
+  }
+
+  CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
+  if (CopyDtb == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  *Dtb = CopyDtb;
+  *DtbSize = OrigDtbSize;
+
+  return EFI_SUCCESS;
+}
diff --git a/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf
new file mode 100644
index 000000000000..7c337aace3ef
--- /dev/null
+++ b/EmbeddedPkg/Library/DtPlatformDtbLoaderBaseLib/DtPlatformDtbLoaderBaseLib.inf
@@ -0,0 +1,36 @@ 
+/** @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.
+*
+**/
+
+[Defines]
+  INF_VERSION                    = 0x00010019
+  BASE_NAME                      = DtPlatformDtbLoaderBaseLib
+  FILE_GUID                      = 419a1910-70da-4c99-8696-ba81a57be508
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = DtPlatformDtbLoaderLib|DXE_DRIVER
+
+[Sources]
+  DtPlatformDtbLoaderBaseLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DxeServicesLib
+  MemoryAllocationLib
+
+[Guids]
+  gDtPlatformDefaultDtbFileGuid