From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web11.5227.1630486583042685858 for ; Wed, 01 Sep 2021 01:56:23 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Qv4iZedM; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 6D65B6108B for ; Wed, 1 Sep 2021 08:56:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1630486582; bh=McyuOk2P1GTJaRdlBBS50K9yvuXZBzGiUPoyJfvf8Rg=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=Qv4iZedMEA5ewAfbhIxEESPRnTl69jyuIQ1c3FOlAbyb7D8NG2EtjG5rmwG/fUGtb lqEcCnallxZq0mNiqIGFpU55uRIdLT2/3kkFlJZiSRSysHTMw4/ryMAbjOsG4gykXK KfhHidxawhUY3ulLgunMlNATExtlWgH81MHO2qOjh4oV6fdliDznrxSQV4HsF6Hk8a oRj18GiDLxj5d3DqqvaFuaCMbubprAHv7I8i8zwSnAMydX8BYnjS5R0WMd6+gBT9Js IZD4feRcmnc4FcUe8D7gMhXF7MfiQpK7l3ZFIUq5rf039aFOYMXXqlOjxh9WO4aSWa 0hgIPNQQiTrIw== Received: by mail-ot1-f41.google.com with SMTP id i8-20020a056830402800b0051afc3e373aso2600898ots.5 for ; Wed, 01 Sep 2021 01:56:22 -0700 (PDT) X-Gm-Message-State: AOAM530Er9OzAqTurnnd/fWXJjmeXT9VigbqDS0Q3WEa5o+G5ozTKY6U Y5BJrywHHjxvS/i0l9xFlnjCnoeCHMIU7N+Zhg8= X-Google-Smtp-Source: ABdhPJwdJAs6+sjdFvo1wKJ8v2KYxa4f2Rpmwpur5Sg8Vl1btAV8Ou8VnjjXFzNZbv304O34MDyMS2wOEP2WWMq6jUE= X-Received: by 2002:a05:6830:719:: with SMTP id y25mr21358108ots.77.1630486581735; Wed, 01 Sep 2021 01:56:21 -0700 (PDT) MIME-Version: 1.0 References: <747e079edeaca7c1864005ec711b7689e107c34f.1630449811.git.jbrasen@nvidia.com> In-Reply-To: <747e079edeaca7c1864005ec711b7689e107c34f.1630449811.git.jbrasen@nvidia.com> From: "Ard Biesheuvel" Date: Wed, 1 Sep 2021 10:56:10 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/1] EmbeddedPkg: Add LoadFile2 for linux initrd To: Jeff Brasen Cc: edk2-devel-groups-io , Leif Lindholm , Ard Biesheuvel , Abner Chang , Daniel Schaefer Content-Type: text/plain; charset="UTF-8" Hi Jeff, On Wed, 1 Sept 2021 at 00:48, 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 > --- > EmbeddedPkg/EmbeddedPkg.dec | 1 + > .../AndroidBootImgLib/AndroidBootImgLib.inf | 3 + > .../AndroidBootImgLib/AndroidBootImgLib.c | 147 ++++++++++++++++-- > 3 files changed, 142 insertions(+), 9 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 bbc240c3632a..255996e65bdc 100644 > --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > @@ -10,11 +10,15 @@ > #include > #include > #include > +#include > #include > #include > > #include > #include > +#include > + > +#include > > #include > > @@ -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 EFI_PHYSICAL_ADDRESS mRamdiskData = 0; > +STATIC UINT64 mRamdiskSize = 0; > > STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate = > { > @@ -48,6 +59,96 @@ STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate = > } // End > }; > > +STATIC RAMDISK_DEVICE_PATH mRamdiskDevicePath = Could this be CONST? > +{ > + { > + { > + 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)) { Please wrap new code to 80 columns > + return EFI_INVALID_PARAMETER; > + } > + > + if (BootPolicy) { > + return EFI_UNSUPPORTED; > + } > + > + // Check if the given buffer size is big enough > + // EFI_BUFFER_TOO_SMALL gets boot manager 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 > + gBS->CopyMem (Buffer, (VOID *)mRamdiskData, mRamdiskSize); > + > + return EFI_SUCCESS; > +} > + > +/// > +/// Load File Protocol instance > +/// > +GLOBAL_REMOVE_IF_UNREFERENCED > +EFI_LOAD_FILE2_PROTOCOL mAndroidBootImgLoadFile2 = { > + AndroidBootImgLoadFile2 > +}; > + > EFI_STATUS > AndroidBootImgGetImgSize ( > IN VOID *BootImg, > @@ -383,6 +484,7 @@ AndroidBootImgBoot ( > VOID *RamdiskData; > UINTN RamdiskSize; > IN VOID *FdtBase; > + EFI_HANDLE RamdiskHandle; > > Status = gBS->LocateProtocol (&gAndroidBootImgProtocolGuid, NULL, > (VOID **) &mAndroidBootImg); > @@ -420,16 +522,32 @@ AndroidBootImgBoot ( > return Status; > } > > - Status = AndroidBootImgLocateFdt (Buffer, &FdtBase); > - if (EFI_ERROR (Status)) { > - FreePool (NewKernelArg); > - return Status; > - } > + if (FeaturePcdGet (PcdAndroidBootLoadFile2)) { > + RamdiskHandle = 0; > + mRamdiskData = (EFI_PHYSICAL_ADDRESS)RamdiskData; Why is this being converted from a VOID* to EFI_PHYSICAL_ADDRESS and then back again? > + mRamdiskSize = RamdiskSize; > + Status = gBS->InstallMultipleProtocolInterfaces (&RamdiskHandle, > + &gEfiLoadFile2ProtocolGuid, > + &mAndroidBootImgLoadFile2, > + &gEfiDevicePathProtocolGuid, > + &mRamdiskDevicePath, > + NULL); > + if (EFI_ERROR (Status)) { > + FreePool (NewKernelArg); There are now 3 occurrences of this FreePool() so better use goto with a label at the end of the function. > + return Status; > + } > + } else { > + Status = AndroidBootImgLocateFdt (Buffer, &FdtBase); > + if (EFI_ERROR (Status)) { > + FreePool (NewKernelArg); > + return Status; > + } > > - Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize); > - if (EFI_ERROR (Status)) { > - FreePool (NewKernelArg); > - return Status; > + Status = AndroidBootImgUpdateFdt (Buffer, FdtBase, RamdiskData, RamdiskSize); > + if (EFI_ERROR (Status)) { > + FreePool (NewKernelArg); > + return Status; > + } > } > > KernelDevicePath = mMemoryDevicePathTemplate; > @@ -466,5 +584,16 @@ AndroidBootImgBoot ( > Status = gBS->StartImage (ImageHandle, NULL, NULL); > // Clear the Watchdog Timer if the image returns > gBS->SetWatchdogTimer (0, 0x10000, 0, NULL); > + > + if (FeaturePcdGet (PcdAndroidBootLoadFile2)) { > + mRamdiskData = 0; > + mRamdiskSize = 0; > + gBS->UninstallMultipleProtocolInterfaces (RamdiskHandle, > + &gEfiLoadFile2ProtocolGuid, > + &mAndroidBootImgLoadFile2, > + &gEfiDevicePathProtocolGuid, > + &mRamdiskDevicePath, > + NULL); > + } > return EFI_SUCCESS; > } > -- > 2.17.1 >