From: Chang, Abner <Abner.Chang@amd.com>
Sent: Monday, November 20, 2023 5:30 AM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; quic_llindhol@quicinc.com <quic_llindhol@quicinc.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; git@danielschaefer.me <git@danielschaefer.me>; Jeff
Brasen <jbrasen@nvidia.com>
Subject: RE: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command line
External email: Use caution opening links or attachments
[AMD Official Use Only - General]
> -----Original Message-----
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Wednesday, November 15, 2023 11:12 AM
> To: devel@edk2.groups.io; quic_llindhol@quicinc.com;
> ardb+tianocore@kernel.org; Chang, Abner <Abner.Chang@amd.com>;
> git@danielschaefer.me; jbrasen@nvidia.com
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command
> line
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> AndroidBootImgLib allows for platforms to append to kernel command
> line but does not allow for the overall kernel command line to go
> beyond the limit set by the image header. Address this limitation
> by adding a pcd where platform can tell how many extra characters
> they expect on their platform in addition to what the image header
> specifies.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> EmbeddedPkg/EmbeddedPkg.dec | 5 +++++
> EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c | 2 +-
> EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf | 3 ++-
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec
> b/EmbeddedPkg/EmbeddedPkg.dec
> index 341ef5e6a6..94dc3c9b76 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -183,3 +183,8 @@
> # Selection between DT and ACPI as a default
> #
> gEmbeddedTokenSpaceGuid.PcdDefaultDtPref|TRUE|BOOLEAN|0x0000059
> +
> + #
> + # Expected Overflow Android Kernel Command Line Characters
> + #
> +
> gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow|0|UI
> NT32|0x000005C
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index f63648e60d..d16929f2bb 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -335,7 +335,7 @@ AndroidBootImgUpdateArgs (
> return Status;
> }
>
> - NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
> + NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE + PcdGet32
> (PcdAndroidKernelCommandLineOverflow);
I don't know the history of ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, however, I am a little bit confused.
Is this PCD introduced for the case the string size may greater than ANDROID_BOOTIMG_KERNEL_ARGS_SIZE? If yes, then why don't we just use StrSize (ImageKernelArgs) * sizeof(UINT16) as the buffer size?
Regards,
Abner
Android style image header can never have more than
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE bytes.
However, a platform may need to add new kernel command line arguments at boot time that may need more buffer space than
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
bytes. To allow for that, we can have a PCD that by default is set to 0 but a user can override that in their platform dsc if they expect more than
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE characters.
This is not a bugfix but a limitation that is being addressed in this patch.
Thanks
Ashish
> *KernelArgs = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
> if (*KernelArgs == NULL) {
> DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> index 8eefeef4f9..9754664df5 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> @@ -45,5 +45,6 @@
> gEfiAcpiTableGuid
> gFdtTableGuid
>
> -[FeaturePcd]
> +[Pcd]
> gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
> + gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow
> --
> 2.17.1