* [PATCH v2 0/2] AndroidBootImgLib improvements
@ 2021-09-01 20:28 Jeff Brasen
2021-09-01 20:28 ` [PATCH v2 1/2] EmbeddedPkg: AndroidBootImgBoot error handling updates Jeff Brasen
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jeff Brasen @ 2021-09-01 20:28 UTC (permalink / raw)
To: devel; +Cc: leif, ardb+tianocore, abner.chang, daniel.schaefer, Jeff Brasen
Added support for using loadfile2 approach for passing ramdisk to linux.
Created patch series for general error handling improvments based on
review feedback.
[v2]
-Added review feedback
-General improvements to error handling
[v1]
- Intial revision
Jeff Brasen (2):
EmbeddedPkg: AndroidBootImgBoot error handling updates
EmbeddedPkg: Add LoadFile2 for linux initrd
EmbeddedPkg/EmbeddedPkg.dec | 1 +
.../AndroidBootImgLib/AndroidBootImgLib.inf | 3 +
.../AndroidBootImgLib/AndroidBootImgLib.c | 193 +++++++++++++++---
3 files changed, 171 insertions(+), 26 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] EmbeddedPkg: AndroidBootImgBoot error handling updates
2021-09-01 20:28 [PATCH v2 0/2] AndroidBootImgLib improvements Jeff Brasen
@ 2021-09-01 20:28 ` Jeff Brasen
2021-09-01 20:28 ` [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd Jeff Brasen
2021-09-07 15:44 ` [PATCH v2 0/2] AndroidBootImgLib improvements Jeff Brasen
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Brasen @ 2021-09-01 20:28 UTC (permalink / raw)
To: devel; +Cc: leif, ardb+tianocore, abner.chang, daniel.schaefer, Jeff Brasen
Update AndroidBootImgBoot to use a single return point
Make sure Kernel args are freed and Image is unloaded.
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
.../AndroidBootImgLib/AndroidBootImgLib.c | 50 +++++++++++--------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index bbc240c3632a..3c617649f735 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -384,10 +384,13 @@ AndroidBootImgBoot (
UINTN RamdiskSize;
IN VOID *FdtBase;
+ NewKernelArg = NULL;
+ ImageHandle = NULL;
+
Status = gBS->LocateProtocol (&gAndroidBootImgProtocolGuid, NULL,
(VOID **) &mAndroidBootImg);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Exit;
}
Status = AndroidBootImgGetKernelInfo (
@@ -396,19 +399,19 @@ AndroidBootImgBoot (
&KernelSize
);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Exit;
}
NewKernelArg = AllocateZeroPool (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
if (NewKernelArg == NULL) {
DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
- return EFI_OUT_OF_RESOURCES;
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Exit;
}
Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
if (EFI_ERROR (Status)) {
- FreePool (NewKernelArg);
- return Status;
+ goto Exit;
}
Status = AndroidBootImgGetRamdiskInfo (
@@ -417,19 +420,17 @@ AndroidBootImgBoot (
&RamdiskSize
);
if (EFI_ERROR (Status)) {
- return Status;
+ goto Exit;
}
Status = AndroidBootImgLocateFdt (Buffer, &FdtBase);
if (EFI_ERROR (Status)) {
- FreePool (NewKernelArg);
- return Status;
+ goto Exit;
}
Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize);
if (EFI_ERROR (Status)) {
- FreePool (NewKernelArg);
- return Status;
+ goto Exit;
}
KernelDevicePath = mMemoryDevicePathTemplate;
@@ -442,21 +443,15 @@ AndroidBootImgBoot (
(EFI_DEVICE_PATH *)&KernelDevicePath,
(VOID*)(UINTN)Kernel, KernelSize, &ImageHandle);
if (EFI_ERROR (Status)) {
- //
- // With EFI_SECURITY_VIOLATION retval, the Image was loaded and an ImageHandle was created
- // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not be started right now.
- // If the caller doesn't have the option to defer the execution of an image, we should
- // unload image for the EFI_SECURITY_VIOLATION to avoid resource leak.
- //
- if (Status == EFI_SECURITY_VIOLATION) {
- gBS->UnloadImage (ImageHandle);
- }
- return Status;
+ goto Exit;
}
// Set kernel arguments
Status = gBS->HandleProtocol (ImageHandle, &gEfiLoadedImageProtocolGuid,
(VOID **) &ImageInfo);
+ if (EFI_ERROR (Status)) {
+ goto Exit;
+ }
ImageInfo->LoadOptions = NewKernelArg;
ImageInfo->LoadOptionsSize = StrLen (NewKernelArg) * sizeof (CHAR16);
@@ -466,5 +461,18 @@ AndroidBootImgBoot (
Status = gBS->StartImage (ImageHandle, NULL, NULL);
// Clear the Watchdog Timer if the image returns
gBS->SetWatchdogTimer (0, 0x10000, 0, NULL);
- return EFI_SUCCESS;
+
+Exit:
+ //Unload image as it will not be used anymore
+ if (ImageHandle != NULL) {
+ gBS->UnloadImage (ImageHandle);
+ ImageHandle = NULL;
+ }
+ if (EFI_ERROR (Status)) {
+ if (NewKernelArg != NULL) {
+ FreePool (NewKernelArg);
+ NewKernelArg = NULL;
+ }
+ }
+ return Status;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd
2021-09-01 20:28 [PATCH v2 0/2] AndroidBootImgLib improvements Jeff Brasen
2021-09-01 20:28 ` [PATCH v2 1/2] EmbeddedPkg: AndroidBootImgBoot error handling updates Jeff Brasen
@ 2021-09-01 20:28 ` Jeff Brasen
2021-09-07 17:51 ` Leif Lindholm
2021-09-07 15:44 ` [PATCH v2 0/2] AndroidBootImgLib improvements Jeff Brasen
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Brasen @ 2021-09-01 20:28 UTC (permalink / raw)
To: devel; +Cc: leif, ardb+tianocore, abner.chang, daniel.schaefer, Jeff Brasen
Add support under a pcd feature for using the new interface to pass
initrd to the linux kernel.
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
EmbeddedPkg/EmbeddedPkg.dec | 1 +
.../AndroidBootImgLib/AndroidBootImgLib.inf | 3 +
.../AndroidBootImgLib/AndroidBootImgLib.c | 147 +++++++++++++++++-
3 files changed, 144 insertions(+), 7 deletions(-)
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index c2068ce5a8ce..7638aaaadeb8 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -86,6 +86,7 @@ [Ppis]
[PcdsFeatureFlag.common]
gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob|FALSE|BOOLEAN|0x0000001b
gEmbeddedTokenSpaceGuid.PcdGdbSerial|FALSE|BOOLEAN|0x00000053
+ gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2|FALSE|BOOLEAN|0x0000005b
[PcdsFixedAtBuild.common]
gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0|UINT32|0x0000000b
diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
index bfed4ba9dcd4..39d8abe72cd1 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
@@ -42,3 +42,6 @@ [Protocols]
[Guids]
gFdtTableGuid
+
+[FeaturePcd]
+ gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 3c617649f735..635965690dc0 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -10,11 +10,15 @@
#include <libfdt.h>
#include <Library/AndroidBootImgLib.h>
#include <Library/PrintLib.h>
+#include <Library/DevicePathLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <Protocol/AndroidBootImg.h>
#include <Protocol/LoadedImage.h>
+#include <Protocol/LoadFile2.h>
+
+#include <Guid/LinuxEfiInitrdMedia.h>
#include <libfdt.h>
@@ -25,7 +29,14 @@ typedef struct {
EFI_DEVICE_PATH_PROTOCOL End;
} MEMORY_DEVICE_PATH;
+typedef struct {
+ VENDOR_DEVICE_PATH VenMediaNode;
+ EFI_DEVICE_PATH_PROTOCOL EndNode;
+} RAMDISK_DEVICE_PATH;
+
STATIC ANDROID_BOOTIMG_PROTOCOL *mAndroidBootImg;
+STATIC VOID *mRamdiskData = NULL;
+STATIC UINTN mRamdiskSize = 0;
STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate =
{
@@ -48,6 +59,99 @@ STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate =
} // End
};
+STATIC CONST RAMDISK_DEVICE_PATH mRamdiskDevicePath =
+{
+ {
+ {
+ MEDIA_DEVICE_PATH,
+ MEDIA_VENDOR_DP,
+ { sizeof (VENDOR_DEVICE_PATH), 0 }
+ },
+ LINUX_EFI_INITRD_MEDIA_GUID
+ },
+ {
+ END_DEVICE_PATH_TYPE,
+ END_ENTIRE_DEVICE_PATH_SUBTYPE,
+ { sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }
+ }
+};
+
+/**
+ Causes the driver to load a specified file.
+
+ @param This Protocol instance pointer.
+ @param FilePath The device specific path of the file to load.
+ @param BootPolicy Should always be FALSE.
+ @param BufferSize On input the size of Buffer in bytes. On output with a return
+ code of EFI_SUCCESS, the amount of data transferred to
+ Buffer. On output with a return code of EFI_BUFFER_TOO_SMALL,
+ the size of Buffer required to retrieve the requested file.
+ @param Buffer The memory buffer to transfer the file to. IF Buffer is NULL,
+ then no the size of the requested file is returned in
+ BufferSize.
+
+ @retval EFI_SUCCESS The file was loaded.
+ @retval EFI_UNSUPPORTED BootPolicy is TRUE.
+ @retval EFI_INVALID_PARAMETER FilePath is not a valid device path, or
+ BufferSize is NULL.
+ @retval EFI_NO_MEDIA No medium was present to load the file.
+ @retval EFI_DEVICE_ERROR The file was not loaded due to a device error.
+ @retval EFI_NO_RESPONSE The remote system did not respond.
+ @retval EFI_NOT_FOUND The file was not found
+ @retval EFI_ABORTED The file load process was manually canceled.
+ @retval EFI_BUFFER_TOO_SMALL The BufferSize is too small to read the current
+ directory entry. BufferSize has been updated with
+ the size needed to complete the request.
+
+
+**/
+EFI_STATUS
+EFIAPI
+AndroidBootImgLoadFile2 (
+ IN EFI_LOAD_FILE2_PROTOCOL *This,
+ IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
+ IN BOOLEAN BootPolicy,
+ IN OUT UINTN *BufferSize,
+ IN VOID *Buffer OPTIONAL
+ )
+
+{
+ // Verify if the valid parameters
+ if (This == NULL ||
+ BufferSize == NULL ||
+ FilePath == NULL ||
+ !IsDevicePathValid (FilePath, 0)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (BootPolicy) {
+ return EFI_UNSUPPORTED;
+ }
+
+ // Check if the given buffer size is big enough
+ // EFI_BUFFER_TOO_SMALL to allow caller to allocate a bigger buffer
+ if (mRamdiskSize == 0) {
+ return EFI_NOT_FOUND;
+ }
+ if (Buffer == NULL || *BufferSize < mRamdiskSize) {
+ *BufferSize = mRamdiskSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ // Copy InitRd
+ CopyMem (Buffer, mRamdiskData, mRamdiskSize);
+ *BufferSize = mRamdiskSize;
+
+ return EFI_SUCCESS;
+}
+
+///
+/// Load File Protocol instance
+///
+STATIC EFI_LOAD_FILE2_PROTOCOL mAndroidBootImgLoadFile2 = {
+ AndroidBootImgLoadFile2
+};
+
EFI_STATUS
AndroidBootImgGetImgSize (
IN VOID *BootImg,
@@ -383,6 +487,7 @@ AndroidBootImgBoot (
VOID *RamdiskData;
UINTN RamdiskSize;
IN VOID *FdtBase;
+ EFI_HANDLE RamDiskLoadFileHandle;
NewKernelArg = NULL;
ImageHandle = NULL;
@@ -423,14 +528,29 @@ AndroidBootImgBoot (
goto Exit;
}
- Status = AndroidBootImgLocateFdt (Buffer, &FdtBase);
- if (EFI_ERROR (Status)) {
- goto Exit;
- }
+ if (FeaturePcdGet (PcdAndroidBootLoadFile2)) {
+ RamDiskLoadFileHandle = NULL;
+ mRamdiskData = RamdiskData;
+ mRamdiskSize = RamdiskSize;
+ Status = gBS->InstallMultipleProtocolInterfaces (&RamDiskLoadFileHandle,
+ &gEfiLoadFile2ProtocolGuid,
+ &mAndroidBootImgLoadFile2,
+ &gEfiDevicePathProtocolGuid,
+ &mRamdiskDevicePath,
+ NULL);
+ if (EFI_ERROR (Status)) {
+ goto Exit;
+ }
+ } else {
+ Status = AndroidBootImgLocateFdt (Buffer, &FdtBase);
+ if (EFI_ERROR (Status)) {
+ goto Exit;
+ }
- Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize);
- if (EFI_ERROR (Status)) {
- goto Exit;
+ Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize);
+ if (EFI_ERROR (Status)) {
+ goto Exit;
+ }
}
KernelDevicePath = mMemoryDevicePathTemplate;
@@ -474,5 +594,18 @@ AndroidBootImgBoot (
NewKernelArg = NULL;
}
}
+ if (FeaturePcdGet (PcdAndroidBootLoadFile2)) {
+ mRamdiskData = NULL;
+ mRamdiskSize = 0;
+ if (RamDiskLoadFileHandle != NULL) {
+ gBS->UninstallMultipleProtocolInterfaces (RamDiskLoadFileHandle,
+ &gEfiLoadFile2ProtocolGuid,
+ &mAndroidBootImgLoadFile2,
+ &gEfiDevicePathProtocolGuid,
+ &mRamdiskDevicePath,
+ NULL);
+ RamDiskLoadFileHandle = NULL;
+ }
+ }
return Status;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] AndroidBootImgLib improvements
2021-09-01 20:28 [PATCH v2 0/2] AndroidBootImgLib improvements Jeff Brasen
2021-09-01 20:28 ` [PATCH v2 1/2] EmbeddedPkg: AndroidBootImgBoot error handling updates Jeff Brasen
2021-09-01 20:28 ` [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd Jeff Brasen
@ 2021-09-07 15:44 ` Jeff Brasen
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Brasen @ 2021-09-07 15:44 UTC (permalink / raw)
To: devel@edk2.groups.io
Cc: leif@nuviainc.com, ardb+tianocore@kernel.org, abner.chang@hpe.com,
daniel.schaefer@hpe.com
[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]
Any additional feedback on this patchset?
Thanks,
Jeff
________________________________
From: Jeff Brasen <jbrasen@nvidia.com>
Sent: Wednesday, September 1, 2021 2:28 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@nuviainc.com <leif@nuviainc.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; abner.chang@hpe.com <abner.chang@hpe.com>; daniel.schaefer@hpe.com <daniel.schaefer@hpe.com>; Jeff Brasen <jbrasen@nvidia.com>
Subject: [PATCH v2 0/2] AndroidBootImgLib improvements
Added support for using loadfile2 approach for passing ramdisk to linux.
Created patch series for general error handling improvments based on
review feedback.
[v2]
-Added review feedback
-General improvements to error handling
[v1]
- Intial revision
Jeff Brasen (2):
EmbeddedPkg: AndroidBootImgBoot error handling updates
EmbeddedPkg: Add LoadFile2 for linux initrd
EmbeddedPkg/EmbeddedPkg.dec | 1 +
.../AndroidBootImgLib/AndroidBootImgLib.inf | 3 +
.../AndroidBootImgLib/AndroidBootImgLib.c | 193 +++++++++++++++---
3 files changed, 171 insertions(+), 26 deletions(-)
--
2.17.1
[-- Attachment #2: Type: text/html, Size: 2356 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd
2021-09-01 20:28 ` [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd Jeff Brasen
@ 2021-09-07 17:51 ` Leif Lindholm
2021-09-09 21:01 ` Jeff Brasen
0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2021-09-07 17:51 UTC (permalink / raw)
To: Jeff Brasen; +Cc: devel, ardb+tianocore, abner.chang, daniel.schaefer
Minor style comments below.
On Wed, Sep 01, 2021 at 20:28:27 +0000, Jeff Brasen wrote:
> Add support under a pcd feature for using the new interface to pass
> initrd to the linux kernel.
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
> EmbeddedPkg/EmbeddedPkg.dec | 1 +
> .../AndroidBootImgLib/AndroidBootImgLib.inf | 3 +
> .../AndroidBootImgLib/AndroidBootImgLib.c | 147 +++++++++++++++++-
> 3 files changed, 144 insertions(+), 7 deletions(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index c2068ce5a8ce..7638aaaadeb8 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -86,6 +86,7 @@ [Ppis]
> [PcdsFeatureFlag.common]
> gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob|FALSE|BOOLEAN|0x0000001b
> gEmbeddedTokenSpaceGuid.PcdGdbSerial|FALSE|BOOLEAN|0x00000053
> + gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2|FALSE|BOOLEAN|0x0000005b
>
> [PcdsFixedAtBuild.common]
> gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0|UINT32|0x0000000b
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> index bfed4ba9dcd4..39d8abe72cd1 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> @@ -42,3 +42,6 @@ [Protocols]
>
> [Guids]
> gFdtTableGuid
> +
> +[FeaturePcd]
> + gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index 3c617649f735..635965690dc0 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -10,11 +10,15 @@
> #include <libfdt.h>
> #include <Library/AndroidBootImgLib.h>
> #include <Library/PrintLib.h>
> +#include <Library/DevicePathLib.h>
Move that inserted line up one to maintain alphabetical sorting.
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
>
> #include <Protocol/AndroidBootImg.h>
> #include <Protocol/LoadedImage.h>
> +#include <Protocol/LoadFile2.h>
> +
> +#include <Guid/LinuxEfiInitrdMedia.h>
>
> #include <libfdt.h>
Glad to see we're *really* including libfdt.h (also included above and
completely unrelated to this patch).
> @@ -25,7 +29,14 @@ typedef struct {
> EFI_DEVICE_PATH_PROTOCOL End;
> } MEMORY_DEVICE_PATH;
>
> +typedef struct {
> + VENDOR_DEVICE_PATH VenMediaNode;
VendorMediaNode
> + EFI_DEVICE_PATH_PROTOCOL EndNode;
> +} RAMDISK_DEVICE_PATH;
> +
> STATIC ANDROID_BOOTIMG_PROTOCOL *mAndroidBootImg;
> +STATIC VOID *mRamdiskData = NULL;
> +STATIC UINTN mRamdiskSize = 0;
>
> STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate =
> {
> @@ -48,6 +59,99 @@ STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate =
> } // End
> };
>
> +STATIC CONST RAMDISK_DEVICE_PATH mRamdiskDevicePath =
> +{
> + {
> + {
> + MEDIA_DEVICE_PATH,
> + MEDIA_VENDOR_DP,
> + { sizeof (VENDOR_DEVICE_PATH), 0 }
> + },
> + LINUX_EFI_INITRD_MEDIA_GUID
> + },
> + {
> + END_DEVICE_PATH_TYPE,
> + END_ENTIRE_DEVICE_PATH_SUBTYPE,
> + { sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }
> + }
> +};
> +
> +/**
> + Causes the driver to load a specified file.
> +
> + @param This Protocol instance pointer.
> + @param FilePath The device specific path of the file to load.
> + @param BootPolicy Should always be FALSE.
> + @param BufferSize On input the size of Buffer in bytes. On output with a return
> + code of EFI_SUCCESS, the amount of data transferred to
> + Buffer. On output with a return code of EFI_BUFFER_TOO_SMALL,
> + the size of Buffer required to retrieve the requested file.
> + @param Buffer The memory buffer to transfer the file to. IF Buffer is NULL,
> + then no the size of the requested file is returned in
> + BufferSize.
> +
> + @retval EFI_SUCCESS The file was loaded.
> + @retval EFI_UNSUPPORTED BootPolicy is TRUE.
> + @retval EFI_INVALID_PARAMETER FilePath is not a valid device path, or
> + BufferSize is NULL.
> + @retval EFI_NO_MEDIA No medium was present to load the file.
> + @retval EFI_DEVICE_ERROR The file was not loaded due to a device error.
> + @retval EFI_NO_RESPONSE The remote system did not respond.
> + @retval EFI_NOT_FOUND The file was not found
> + @retval EFI_ABORTED The file load process was manually canceled.
> + @retval EFI_BUFFER_TOO_SMALL The BufferSize is too small to read the current
> + directory entry. BufferSize has been updated with
> + the size needed to complete the request.
> +
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgLoadFile2 (
> + IN EFI_LOAD_FILE2_PROTOCOL *This,
> + IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
> + IN BOOLEAN BootPolicy,
> + IN OUT UINTN *BufferSize,
> + IN VOID *Buffer OPTIONAL
> + )
> +
> +{
> + // Verify if the valid parameters
> + if (This == NULL ||
> + BufferSize == NULL ||
> + FilePath == NULL ||
> + !IsDevicePathValid (FilePath, 0)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (BootPolicy) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + // Check if the given buffer size is big enough
> + // EFI_BUFFER_TOO_SMALL to allow caller to allocate a bigger buffer
> + if (mRamdiskSize == 0) {
> + return EFI_NOT_FOUND;
> + }
> + if (Buffer == NULL || *BufferSize < mRamdiskSize) {
> + *BufferSize = mRamdiskSize;
> + return EFI_BUFFER_TOO_SMALL;
> + }
> +
> + // Copy InitRd
> + CopyMem (Buffer, mRamdiskData, mRamdiskSize);
> + *BufferSize = mRamdiskSize;
> +
> + return EFI_SUCCESS;
> +}
> +
> +///
> +/// Load File Protocol instance
> +///
> +STATIC EFI_LOAD_FILE2_PROTOCOL mAndroidBootImgLoadFile2 = {
> + AndroidBootImgLoadFile2
> +};
> +
> EFI_STATUS
> AndroidBootImgGetImgSize (
> IN VOID *BootImg,
> @@ -383,6 +487,7 @@ AndroidBootImgBoot (
> VOID *RamdiskData;
> UINTN RamdiskSize;
> IN VOID *FdtBase;
> + EFI_HANDLE RamDiskLoadFileHandle;
>
> NewKernelArg = NULL;
> ImageHandle = NULL;
> @@ -423,14 +528,29 @@ AndroidBootImgBoot (
> goto Exit;
> }
>
> - Status = AndroidBootImgLocateFdt (Buffer, &FdtBase);
> - if (EFI_ERROR (Status)) {
> - goto Exit;
> - }
> + if (FeaturePcdGet (PcdAndroidBootLoadFile2)) {
> + RamDiskLoadFileHandle = NULL;
> + mRamdiskData = RamdiskData;
> + mRamdiskSize = RamdiskSize;
> + Status = gBS->InstallMultipleProtocolInterfaces (&RamDiskLoadFileHandle,
> + &gEfiLoadFile2ProtocolGuid,
> + &mAndroidBootImgLoadFile2,
> + &gEfiDevicePathProtocolGuid,
> + &mRamdiskDevicePath,
> + NULL);
> + if (EFI_ERROR (Status)) {
> + goto Exit;
> + }
> + } else {
> + Status = AndroidBootImgLocateFdt (Buffer, &FdtBase);
> + if (EFI_ERROR (Status)) {
> + goto Exit;
> + }
>
> - Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize);
> - if (EFI_ERROR (Status)) {
> - goto Exit;
> + Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize);
> + if (EFI_ERROR (Status)) {
> + goto Exit;
> + }
It looks to me like this changes the functionality so that if
PcdAndroidBootLoadFile2 is set, we now don't locate/update an FDT,
which was not clear to me from the cover letter and commit message.
This may well be intentional and correct, but since this function was
already a bit messy, I'd kind of prefer shunting some of this off into
helper functions now we're adding to the complexity.
/
Leif
> }
>
> KernelDevicePath = mMemoryDevicePathTemplate;
> @@ -474,5 +594,18 @@ AndroidBootImgBoot (
> NewKernelArg = NULL;
> }
> }
> + if (FeaturePcdGet (PcdAndroidBootLoadFile2)) {
> + mRamdiskData = NULL;
> + mRamdiskSize = 0;
> + if (RamDiskLoadFileHandle != NULL) {
> + gBS->UninstallMultipleProtocolInterfaces (RamDiskLoadFileHandle,
> + &gEfiLoadFile2ProtocolGuid,
> + &mAndroidBootImgLoadFile2,
> + &gEfiDevicePathProtocolGuid,
> + &mRamdiskDevicePath,
> + NULL);
> + RamDiskLoadFileHandle = NULL;
> + }
> + }
>
> return Status;
> }
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd
2021-09-07 17:51 ` Leif Lindholm
@ 2021-09-09 21:01 ` Jeff Brasen
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Brasen @ 2021-09-09 21:01 UTC (permalink / raw)
To: Leif Lindholm
Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org,
abner.chang@hpe.com, daniel.schaefer@hpe.com
[-- Attachment #1: Type: text/plain, Size: 11176 bytes --]
Question below
Thanks,
Jeff
________________________________
From: Leif Lindholm <leif@nuviainc.com>
Sent: Tuesday, September 7, 2021 11:51 AM
To: Jeff Brasen <jbrasen@nvidia.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; abner.chang@hpe.com <abner.chang@hpe.com>; daniel.schaefer@hpe.com <daniel.schaefer@hpe.com>
Subject: Re: [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd
External email: Use caution opening links or attachments
Minor style comments below.
On Wed, Sep 01, 2021 at 20:28:27 +0000, Jeff Brasen wrote:
> Add support under a pcd feature for using the new interface to pass
> initrd to the linux kernel.
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
> EmbeddedPkg/EmbeddedPkg.dec | 1 +
> .../AndroidBootImgLib/AndroidBootImgLib.inf | 3 +
> .../AndroidBootImgLib/AndroidBootImgLib.c | 147 +++++++++++++++++-
> 3 files changed, 144 insertions(+), 7 deletions(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index c2068ce5a8ce..7638aaaadeb8 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -86,6 +86,7 @@ [Ppis]
> [PcdsFeatureFlag.common]
> gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob|FALSE|BOOLEAN|0x0000001b
> gEmbeddedTokenSpaceGuid.PcdGdbSerial|FALSE|BOOLEAN|0x00000053
> + gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2|FALSE|BOOLEAN|0x0000005b
>
> [PcdsFixedAtBuild.common]
> gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0|UINT32|0x0000000b
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> index bfed4ba9dcd4..39d8abe72cd1 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> @@ -42,3 +42,6 @@ [Protocols]
>
> [Guids]
> gFdtTableGuid
> +
> +[FeaturePcd]
> + gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index 3c617649f735..635965690dc0 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -10,11 +10,15 @@
> #include <libfdt.h>
> #include <Library/AndroidBootImgLib.h>
> #include <Library/PrintLib.h>
> +#include <Library/DevicePathLib.h>
Move that inserted line up one to maintain alphabetical sorting.
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
>
> #include <Protocol/AndroidBootImg.h>
> #include <Protocol/LoadedImage.h>
> +#include <Protocol/LoadFile2.h>
> +
> +#include <Guid/LinuxEfiInitrdMedia.h>
>
> #include <libfdt.h>
Glad to see we're *really* including libfdt.h (also included above and
completely unrelated to this patch).
> @@ -25,7 +29,14 @@ typedef struct {
> EFI_DEVICE_PATH_PROTOCOL End;
> } MEMORY_DEVICE_PATH;
>
> +typedef struct {
> + VENDOR_DEVICE_PATH VenMediaNode;
VendorMediaNode
> + EFI_DEVICE_PATH_PROTOCOL EndNode;
> +} RAMDISK_DEVICE_PATH;
> +
> STATIC ANDROID_BOOTIMG_PROTOCOL *mAndroidBootImg;
> +STATIC VOID *mRamdiskData = NULL;
> +STATIC UINTN mRamdiskSize = 0;
>
> STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate =
> {
> @@ -48,6 +59,99 @@ STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate =
> } // End
> };
>
> +STATIC CONST RAMDISK_DEVICE_PATH mRamdiskDevicePath =
> +{
> + {
> + {
> + MEDIA_DEVICE_PATH,
> + MEDIA_VENDOR_DP,
> + { sizeof (VENDOR_DEVICE_PATH), 0 }
> + },
> + LINUX_EFI_INITRD_MEDIA_GUID
> + },
> + {
> + END_DEVICE_PATH_TYPE,
> + END_ENTIRE_DEVICE_PATH_SUBTYPE,
> + { sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }
> + }
> +};
> +
> +/**
> + Causes the driver to load a specified file.
> +
> + @param This Protocol instance pointer.
> + @param FilePath The device specific path of the file to load.
> + @param BootPolicy Should always be FALSE.
> + @param BufferSize On input the size of Buffer in bytes. On output with a return
> + code of EFI_SUCCESS, the amount of data transferred to
> + Buffer. On output with a return code of EFI_BUFFER_TOO_SMALL,
> + the size of Buffer required to retrieve the requested file.
> + @param Buffer The memory buffer to transfer the file to. IF Buffer is NULL,
> + then no the size of the requested file is returned in
> + BufferSize.
> +
> + @retval EFI_SUCCESS The file was loaded.
> + @retval EFI_UNSUPPORTED BootPolicy is TRUE.
> + @retval EFI_INVALID_PARAMETER FilePath is not a valid device path, or
> + BufferSize is NULL.
> + @retval EFI_NO_MEDIA No medium was present to load the file.
> + @retval EFI_DEVICE_ERROR The file was not loaded due to a device error.
> + @retval EFI_NO_RESPONSE The remote system did not respond.
> + @retval EFI_NOT_FOUND The file was not found
> + @retval EFI_ABORTED The file load process was manually canceled.
> + @retval EFI_BUFFER_TOO_SMALL The BufferSize is too small to read the current
> + directory entry. BufferSize has been updated with
> + the size needed to complete the request.
> +
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgLoadFile2 (
> + IN EFI_LOAD_FILE2_PROTOCOL *This,
> + IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
> + IN BOOLEAN BootPolicy,
> + IN OUT UINTN *BufferSize,
> + IN VOID *Buffer OPTIONAL
> + )
> +
> +{
> + // Verify if the valid parameters
> + if (This == NULL ||
> + BufferSize == NULL ||
> + FilePath == NULL ||
> + !IsDevicePathValid (FilePath, 0)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (BootPolicy) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + // Check if the given buffer size is big enough
> + // EFI_BUFFER_TOO_SMALL to allow caller to allocate a bigger buffer
> + if (mRamdiskSize == 0) {
> + return EFI_NOT_FOUND;
> + }
> + if (Buffer == NULL || *BufferSize < mRamdiskSize) {
> + *BufferSize = mRamdiskSize;
> + return EFI_BUFFER_TOO_SMALL;
> + }
> +
> + // Copy InitRd
> + CopyMem (Buffer, mRamdiskData, mRamdiskSize);
> + *BufferSize = mRamdiskSize;
> +
> + return EFI_SUCCESS;
> +}
> +
> +///
> +/// Load File Protocol instance
> +///
> +STATIC EFI_LOAD_FILE2_PROTOCOL mAndroidBootImgLoadFile2 = {
> + AndroidBootImgLoadFile2
> +};
> +
> EFI_STATUS
> AndroidBootImgGetImgSize (
> IN VOID *BootImg,
> @@ -383,6 +487,7 @@ AndroidBootImgBoot (
> VOID *RamdiskData;
> UINTN RamdiskSize;
> IN VOID *FdtBase;
> + EFI_HANDLE RamDiskLoadFileHandle;
>
> NewKernelArg = NULL;
> ImageHandle = NULL;
> @@ -423,14 +528,29 @@ AndroidBootImgBoot (
> goto Exit;
> }
>
> - Status = AndroidBootImgLocateFdt (Buffer, &FdtBase);
> - if (EFI_ERROR (Status)) {
> - goto Exit;
> - }
> + if (FeaturePcdGet (PcdAndroidBootLoadFile2)) {
> + RamDiskLoadFileHandle = NULL;
> + mRamdiskData = RamdiskData;
> + mRamdiskSize = RamdiskSize;
> + Status = gBS->InstallMultipleProtocolInterfaces (&RamDiskLoadFileHandle,
> + &gEfiLoadFile2ProtocolGuid,
> + &mAndroidBootImgLoadFile2,
> + &gEfiDevicePathProtocolGuid,
> + &mRamdiskDevicePath,
> + NULL);
> + if (EFI_ERROR (Status)) {
> + goto Exit;
> + }
> + } else {
> + Status = AndroidBootImgLocateFdt (Buffer, &FdtBase);
> + if (EFI_ERROR (Status)) {
> + goto Exit;
> + }
>
> - Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize);
> - if (EFI_ERROR (Status)) {
> - goto Exit;
> + Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize);
> + if (EFI_ERROR (Status)) {
> + goto Exit;
> + }
It looks to me like this changes the functionality so that if
PcdAndroidBootLoadFile2 is set, we now don't locate/update an FDT,
which was not clear to me from the cover letter and commit message.
This may well be intentional and correct, but since this function was
already a bit messy, I'd kind of prefer shunting some of this off into
helper functions now we're adding to the complexity.
/
Leif
[JB] Actually do we know how this is supposed to work in all cases, I was adding this
to mainly support the case of ACPI boot with this format and none of my images have
FDT in them.
Current behavior seems a bit odd
1. If we don't have an fdt in either the system config table or file we exit with an error
2. If mAndroidBootImg->UpdateDtb is NULL then we never install a new FDT either loaded from file or with initrd content in it
I can restructure this but want to make sure it meets peoples needs (The tooling I use to create the boot image format I am not sure if it can embedded a device tree so testing might be hard)
Does this seem right
1. If ACPI is installed and Feature PCD is not enabled ASSERT and return failure
2. If ACPI is installed use LoadFile2 methods to install
3. If FDT based allow PCD to dictate if it passed via LoadFile2 or initrd entries in dt
4. If mAndroidBootImg->UpdateDtb is NULL allow update of initrd if feature pcd is not enabled
One other question if there is a fdt in boot image and also in the system config table which should take priority
-Jeff
> }
>
> KernelDevicePath = mMemoryDevicePathTemplate;
> @@ -474,5 +594,18 @@ AndroidBootImgBoot (
> NewKernelArg = NULL;
> }
> }
> + if (FeaturePcdGet (PcdAndroidBootLoadFile2)) {
> + mRamdiskData = NULL;
> + mRamdiskSize = 0;
> + if (RamDiskLoadFileHandle != NULL) {
> + gBS->UninstallMultipleProtocolInterfaces (RamDiskLoadFileHandle,
> + &gEfiLoadFile2ProtocolGuid,
> + &mAndroidBootImgLoadFile2,
> + &gEfiDevicePathProtocolGuid,
> + &mRamdiskDevicePath,
> + NULL);
> + RamDiskLoadFileHandle = NULL;
> + }
> + }
>
> return Status;
> }
> --
> 2.17.1
>
[-- Attachment #2: Type: text/html, Size: 22026 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-09 21:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-01 20:28 [PATCH v2 0/2] AndroidBootImgLib improvements Jeff Brasen
2021-09-01 20:28 ` [PATCH v2 1/2] EmbeddedPkg: AndroidBootImgBoot error handling updates Jeff Brasen
2021-09-01 20:28 ` [PATCH v2 2/2] EmbeddedPkg: Add LoadFile2 for linux initrd Jeff Brasen
2021-09-07 17:51 ` Leif Lindholm
2021-09-09 21:01 ` Jeff Brasen
2021-09-07 15:44 ` [PATCH v2 0/2] AndroidBootImgLib improvements Jeff Brasen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox