[AMD Official Use Only - General] Ok, I got it. Thanks.
Reviewed-by: Abner Chang <abner.ch...@amd.com> Abner From: Ashish Singhal <ashishsin...@nvidia.com> Sent: Tuesday, November 21, 2023 1:18 AM To: Chang, Abner <abner.ch...@amd.com>; devel@edk2.groups.io; quic_llind...@quicinc.com; ardb+tianoc...@kernel.org; g...@danielschaefer.me; Jeff Brasen <jbra...@nvidia.com> Subject: Re: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command line [AMD Official Use Only - General] Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. ________________________________________ From: Chang, Abner <mailto:abner.ch...@amd.com> Sent: Monday, November 20, 2023 5:30 AM To: Ashish Singhal <mailto:ashishsin...@nvidia.com>; mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>; mailto:quic_llind...@quicinc.com <mailto:quic_llind...@quicinc.com>; mailto:ardb+tianoc...@kernel.org <mailto:ardb+tianoc...@kernel.org>; mailto:g...@danielschaefer.me <mailto:g...@danielschaefer.me>; Jeff Brasen <mailto:jbra...@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 <mailto:ashishsin...@nvidia.com> > Sent: Wednesday, November 15, 2023 11:12 AM > To: mailto:devel@edk2.groups.io; mailto:quic_llind...@quicinc.com; > mailto:ardb+tianoc...@kernel.org; Chang, Abner <mailto:abner.ch...@amd.com>; > mailto:g...@danielschaefer.me; mailto:jbra...@nvidia.com > Cc: Ashish Singhal <mailto:ashishsin...@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 <mailto:ashishsin...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111494): https://edk2.groups.io/g/devel/message/111494 Mute This Topic: https://groups.io/mt/102598723/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
