From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:400e:c00::229; helo=mail-pf0-x229.google.com; envelope-from=haojian.zhuang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf0-x229.google.com (mail-pf0-x229.google.com [IPv6:2607:f8b0:400e:c00::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BB38F2282E592 for ; Tue, 24 Apr 2018 21:59:36 -0700 (PDT) Received: by mail-pf0-x229.google.com with SMTP id o76so9443966pfi.5 for ; Tue, 24 Apr 2018 21:59:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:thread-topic:thread-index:date:message-id :references:in-reply-to:accept-language:content-language :content-transfer-encoding:mime-version; bh=sCt36LFrF61foEmR4AVjiU0szdnaODxH94PGK3s1zvY=; b=XjpjAQnTo+h79BaKzZwmZ/S4nkOFk11RW9e5rO3BvgICFdFKACXglNZnXHE/WGzhuO fpvju9ZYGsx/XtzSeJMsw8/AmbDinUePFc4ko8y+SVsIetxNLZm81PdHQ49rETAfFHJk k7L+wNP4NrGq20IV8jlUNeu0cSNdDIW5HfXPM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:thread-topic:thread-index :date:message-id:references:in-reply-to:accept-language :content-language:content-transfer-encoding:mime-version; bh=sCt36LFrF61foEmR4AVjiU0szdnaODxH94PGK3s1zvY=; b=QLGsy//1ikLmTmBu1fV/PkUfM553D4Xwjrpp/prpEAboTN+3ahZPZ4hG2ZNNJdKuC1 frda7fZ1RmUUhkM1Yk7k/u4slp1r3zixFcj+ONvWL+2S7sxlnizVVWAt0f7qbNnKKtSP 9tud1ikbXLVPXpZKx6MsCImeBS5ggntsf4ZGiKu9V7mtaCRTvPgTmgvIbVi9nafDGqS1 IwctzHJY08dcTVSgX2UfWiFPY34U2igot3UAopVaTMeHD8JJxvvHcQyPNT9uO9z0Q3r7 H54Szh8P0WqbEs+cYDSRKwjMfKGWONdLL6wTuO3aOyzIXfoO0Dg1lMecVrEHa+mvMHby 8qSA== X-Gm-Message-State: ALQs6tC+IFhT3SsvqGWndldGHS90QWEcooFDmsqZEMygEyDynuWQTs4m Nf3GsoXR4jisqUp+RF0WNQwzjg== X-Google-Smtp-Source: AIpwx4/4240fjGiGr/j6sGiMNVD94GbnjLxNOlS6m6z8IAjE9vOC0UiMYliRX3OKKWJh7C3Wwplszw== X-Received: by 2002:a17:902:30a3:: with SMTP id v32-v6mr27923499plb.123.1524632375848; Tue, 24 Apr 2018 21:59:35 -0700 (PDT) Received: from CY1PR15MB0730.namprd15.prod.outlook.com ([132.245.253.237]) by smtp.gmail.com with ESMTPSA id u4sm16465126pfn.3.2018.04.24.21.59.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 24 Apr 2018 21:59:34 -0700 (PDT) From: Haojian Zhuang To: Laszlo Ersek , Haojian Zhuang , "edk2-devel@lists.01.org" CC: Leif Lindholm , Ard Biesheuvel Thread-Topic: [edk2] [PATCH edk2-platforms v4 1/2] Platform/HiKey960: register predefined boot options Thread-Index: AQHT20mKHOeWNSgBYEGQXRJHkZi1oKQQ7aql X-MS-Exchange-MessageSentRepresentingType: 2 Date: Wed, 25 Apr 2018 04:59:32 +0000 Message-ID: References: <1524464559-14549-1-git-send-email-haojian.zhuang@linaro.org> <1524464559-14549-2-git-send-email-haojian.zhuang@linaro.org>, In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-Exchange-Organization-SCL: -1 X-MS-TNEF-Correlator: X-MS-Exchange-Organization-RecordReviewCfmType: 0 MIME-Version: 1.0 Subject: Re: [PATCH edk2-platforms v4 1/2] Platform/HiKey960: register predefined boot options X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Apr 2018 04:59:36 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi Laszlo,=0A= =0A= Thanks a lot. Fix them in v5.=0A= =0A= Best Regards=0A= Haojian=0A= ________________________________________=0A= From: Laszlo Ersek =0A= Sent: Monday, April 23, 2018 9:24 PM=0A= To: Haojian Zhuang; edk2-devel@lists.01.org=0A= Cc: Leif Lindholm; Ard Biesheuvel=0A= Subject: Re: [edk2] [PATCH edk2-platforms v4 1/2] Platform/HiKey960: regist= er predefined boot options=0A= =0A= Hello Haojian,=0A= =0A= On 04/23/18 08:22, Haojian Zhuang wrote:=0A= > Create 4 boot options on HiKey960 platform. They're "Boot from SD",=0A= > "Grub", "Android Boot" and "Android Fastboot".=0A= >=0A= > Cc: Laszlo Ersek =0A= > Cc: Leif Lindholm =0A= > Cc: Ard Biesheuvel =0A= > Contributed-under: TianoCore Contribution Agreement 1.1=0A= > Signed-off-by: Haojian Zhuang =0A= > ---=0A= > Platform/Hisilicon/HiKey960/HiKey960.dec | 35 ++++=0A= > Platform/Hisilicon/HiKey960/HiKey960.dsc | 6 +=0A= > .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c | 182 ++++++++++++= +++++++++=0A= > .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf | 11 ++=0A= > 4 files changed, 234 insertions(+)=0A= > create mode 100644 Platform/Hisilicon/HiKey960/HiKey960.dec=0A= =0A= This patch looks good to me in general, but there are a number of small=0A= warts / improvements that I might as well point out. I will leave it up=0A= to you to address or ignore each of these points, as you see fit.=0A= =0A= > diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dec b/Platform/Hisil= icon/HiKey960/HiKey960.dec=0A= > new file mode 100644=0A= > index 000000000000..922d7199c5a5=0A= > --- /dev/null=0A= > +++ b/Platform/Hisilicon/HiKey960/HiKey960.dec=0A= > @@ -0,0 +1,35 @@=0A= > +#=0A= > +# Copyright (c) 2018, Linaro Limited. All rights reserved.=0A= > +#=0A= > +# This program and the accompanying materials=0A= > +# are licensed and made available under the terms and conditions of = the BSD License=0A= > +# which accompanies this distribution. The full text of the license= may be found at=0A= > +# http://opensource.org/licenses/bsd-license.php=0A= > +#=0A= > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BAS= IS,=0A= > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS = OR IMPLIED.=0A= > +#=0A= > +=0A= > +[Defines]=0A= > + DEC_SPECIFICATION =3D 0x00010019=0A= > + PACKAGE_NAME =3D HiKey960=0A= > + PACKAGE_GUID =3D 1892b5b5-d18d-47a3-8fab-e3ae6b42= 26b0=0A= > + PACKAGE_VERSION =3D 0.1=0A= > +=0A= > +#####################################################################= ###########=0A= > +#=0A= > +# Include Section - list of Include Paths that are provided by this p= ackage.=0A= > +# Comments are used for Keywords and Module Types.= =0A= > +#=0A= > +# Supported Module Types:=0A= > +# BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_= SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION=0A= > +#=0A= > +#####################################################################= ###########=0A= > +[Guids.common]=0A= > + gHiKey960TokenSpaceGuid =3D { 0x99a14446, 0xaad7, 0xe460, {0= xb4, 0xe5, 0x1f, 0x79, 0xaa, 0xa4, 0x93, 0xfd } }=0A= > +=0A= > +[PcdsFixedAtBuild.common]=0A= > + gHiKey960TokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x000000= 01=0A= > + gHiKey960TokenSpaceGuid.PcdAndroidBootFile|{ 0x36, 0x8b, 0x73, 0x3a= , 0xc5, 0xb9, 0x63, 0x47, 0xab, 0xbd, 0x6c, 0xbd, 0x4b, 0x25, 0xf9, 0xff }|= VOID*|0x00000002=0A= > + gHiKey960TokenSpaceGuid.PcdAndroidFastbootFile|{ 0x2a, 0x50, 0x88, = 0x95, 0x70, 0x53, 0xe3, 0x11, 0x86, 0x31, 0xd7, 0xc5, 0x95, 0x13, 0x64, 0xc= 8 }|VOID*|0x00000003=0A= > + gHiKey960TokenSpaceGuid.PcdSdBootDevicePath|L""|VOID*|0x00000004=0A= > diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisil= icon/HiKey960/HiKey960.dsc=0A= > index 859ab84f8415..475d39916262 100644=0A= > --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc=0A= > +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc=0A= > @@ -132,6 +132,12 @@ [PcdsFixedAtBuild.common]=0A= > gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbVendorId|0x18d1=0A= > gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xd00d=0A= >=0A= > + #=0A= > + # Android Loader=0A= > + #=0A= > + gHiKey960TokenSpaceGuid.PcdAndroidBootDevicePath|L"VenHw(0D51905B-B= 77E-452A-A2C0-ECA0CC8D514A,00003BFF0000000000)/UFS(0x0,0x3)/HD(7,GPT,D33406= 96-9B95-4C64-8DF6-E6D4548FBA41,0x12100,0x4000)"=0A= > + gHiKey960TokenSpaceGuid.PcdSdBootDevicePath|L"VenHw(0D51905B-B77E-4= 52A-A2C0-ECA0CC8D514A,00F037FF0000000000)/SD(0x0)"=0A= > +=0A= > #####################################################################= ###########=0A= > #=0A= > # Components Section - list of all EDK II Modules needed by this Plat= form=0A= > diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c b/P= latform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c=0A= > index 473d61ed384e..da5e3d5a80ce 100644=0A= > --- a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c=0A= > +++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c=0A= > @@ -30,10 +30,16 @@=0A= > #include =0A= > #include =0A= > #include =0A= > +#include =0A= > #include =0A= > +#include =0A= > #include =0A= >=0A= > +#include =0A= > +#include =0A= > #include =0A= > +#include =0A= > +#include =0A= > #include =0A= >=0A= > #define ADC_ADCIN0 0=0A= > @@ -86,6 +92,10 @@=0A= >=0A= > #define DETECT_SW_FASTBOOT 68 // GPIO8_4=0A= >=0A= > +#define HIKEY960_BOOT_OPTION_NUM 4=0A= > +=0A= > +#define GRUB_FILE_NAME L"\\EFI\\BOOT\\GRUBAA64.EFI"= =0A= > +=0A= > typedef struct {=0A= > UINT64 Magic;=0A= > UINT64 Data;=0A= > @@ -359,6 +369,168 @@ OnEndOfDxe (=0A= > }=0A= > }=0A= >=0A= > +STATIC=0A= > +EFI_STATUS=0A= > +GetPlatformBootOptionsAndKeys (=0A= > + OUT UINTN *BootCount,=0A= > + OUT EFI_BOOT_MANAGER_LOAD_OPTION **BootOptions,=0A= > + OUT EFI_INPUT_KEY **BootKeys=0A= > + )=0A= > +{=0A= > + EFI_DEVICE_PATH *DevicePath;=0A= > + EFI_DEVICE_PATH *FileDevicePath;=0A= > + EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL *DPProtocol;=0A= > + FILEPATH_DEVICE_PATH *FilePath;=0A= > + EFI_GUID *FileGuid;=0A= > + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;=0A= > + MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;=0A= > + CHAR16 *PathStr;=0A= > + EFI_STATUS Status;=0A= > + UINTN Size;=0A= > +=0A= > + if ((BootCount =3D=3D NULL) || (BootOptions =3D=3D NULL) || (BootKe= ys =3D=3D NULL)) {=0A= > + return EFI_INVALID_PARAMETER;=0A= > + }=0A= =0A= This is a good check. I didn't suggest specifying EFI_INVALID_PARAMETER=0A= at the protocol definition level -- none of the OUT parameters were=0A= marked OPTIONAL, and I expect callers and implementors to honor the=0A= interface contract, otherwise they invoke undefined behavior --, but,=0A= indeed, if extra checking of the parameters for validity isn't hard,=0A= then it's justifiable to do it.=0A= =0A= > + Size =3D sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * HIKEY960_BOOT_OPTI= ON_NUM;=0A= > + *BootOptions =3D (EFI_BOOT_MANAGER_LOAD_OPTION *)AllocateZeroPool (= Size);=0A= > + if (*BootOptions =3D=3D NULL) {=0A= > + DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootOptions\n= "));=0A= > + return EFI_BUFFER_TOO_SMALL;=0A= > + }=0A= =0A= (1) This should return EFI_OUT_OF_RESOURCES, as specified at the=0A= protocol level.=0A= =0A= > + Size =3D sizeof (EFI_INPUT_KEY) * HIKEY960_BOOT_OPTION_NUM;=0A= > + *BootKeys =3D (EFI_INPUT_KEY *)AllocateZeroPool (Size);=0A= > + if (*BootKeys =3D=3D NULL) {=0A= > + DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootKeys\n"))= ;=0A= > + FreePool (*BootOptions);=0A= > + return EFI_BUFFER_TOO_SMALL;=0A= > + }=0A= =0A= (2) Same as (1).=0A= =0A= (3) In general I suggest adding "goto"-style error handler labels,=0A= because given a number of branches that can fail, the FreePool() calls=0A= like the above accumulate too much. Perhaps it's not too bad in this=0A= patch; up to you to evaluate.=0A= =0A= > +=0A= > + PathStr =3D (CHAR16 *)PcdGetPtr (PcdSdBootDevicePath);=0A= > + ASSERT (PathStr !=3D NULL);=0A= > + Status =3D gBS->LocateProtocol (&gEfiDevicePathFromTextProto= colGuid,=0A= > + NULL,=0A= > + (VOID **)&DPProtocol=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + DevicePath =3D (EFI_DEVICE_PATH *)DPProtocol->ConvertTextToDevic= ePath (PathStr);=0A= =0A= (4) You can simplify this code by calling the ConvertTextToDevicePath()=0A= function from the DevicePathLib class.=0A= =0A= The library class header is:=0A= =0A= MdePkg/Include/Library/DevicePathLib.h=0A= =0A= and the library instance that the platform DSC file should select, for a=0A= (presumably) DXE_DRIVER module like this, is=0A= =0A= MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevic= ePathProtocol.inf=0A= =0A= which will base the library function atop the protocol. (See also=0A= which I just= =0A= filed.)=0A= =0A= > + ASSERT (DevicePath !=3D NULL);=0A= > + Status =3D EfiBootManagerInitializeLoadOption (=0A= > + &(*BootOptions)[0],=0A= > + LoadOptionNumberUnassigned,=0A= > + LoadOptionTypeBoot,=0A= > + LOAD_OPTION_ACTIVE,=0A= > + L"Boot from SD",=0A= > + DevicePath,=0A= > + NULL,=0A= > + 0=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + FreePool (DevicePath);=0A= =0A= Yup, good choice, EfiBootManagerInitializeLoadOption() makes deep=0A= copies.=0A= =0A= > +=0A= > + PathStr =3D (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);=0A= > + ASSERT (PathStr !=3D NULL);=0A= > + Status =3D gBS->LocateProtocol (&gEfiDevicePathFromTextProto= colGuid,=0A= > + NULL,=0A= > + (VOID **)&DPProtocol=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + DevicePath =3D (EFI_DEVICE_PATH *)DPProtocol->ConvertTextToDevic= ePath (PathStr);=0A= > + ASSERT (DevicePath !=3D NULL);=0A= =0A= (5) See (4).=0A= =0A= > + Size =3D StrSize (GRUB_FILE_NAME);=0A= > + FileDevicePath =3D AllocatePool (Size + SIZE_OF_FILEPATH_DEVICE_PAT= H + END_DEVICE_PATH_LENGTH);=0A= > + if (FileDevicePath !=3D NULL) {=0A= > + FilePath =3D (FILEPATH_DEVICE_PATH *) FileDevicePath;=0A= > + FilePath->Header.Type =3D MEDIA_DEVICE_PATH;=0A= > + FilePath->Header.SubType =3D MEDIA_FILEPATH_DP;=0A= > + CopyMem (&FilePath->PathName, GRUB_FILE_NAME, Size);=0A= > + SetDevicePathNodeLength (&FilePath->Header, Size + SIZE_OF= _FILEPATH_DEVICE_PATH);=0A= > + SetDevicePathEndNode (NextDevicePathNode (&FilePath->Heade= r));=0A= > +=0A= > + DevicePath =3D AppendDevicePath (DevicePath, FileDevicePath);=0A= > + FreePool (FileDevicePath);=0A= > + }=0A= =0A= (6) This logic can be replaced with a simple call to FileDevicePath().=0A= Please read its documentation in=0A= "MdePkg/Include/Library/DevicePathLib.h". (In general that library class=0A= has very useful features.)=0A= =0A= For that to work however, you first have to kill the local variable=0A= called "FileDevicePath", because it shadows the function :)=0A= =0A= (7) Another important note: whenever you call=0A= =0A= DevicePath =3D AppendDevicePath (DevicePath, FileDevicePath);=0A= =0A= the original DevicePath is forgotten (it is not modified in place). So,=0A= if DevicePath originally pointed to dynamically allocated storage that=0A= is now unreachable otherwise, it's leaked. And that's the case here,=0A= because DevicePath comes from ConvertTextToDevicePath().=0A= =0A= Please read the AppendDevicePath() docs in the lib class header.=0A= =0A= (8) Taking one step back -- is there any particular reason for defining=0A= GRUB_FILE_NAME as a macro, and appending it dynamically to=0A= "PcdAndroidBootDevicePath"? Why not put the full GRUB pathname into=0A= "PcdAndroidBootDevicePath" to begin with?=0A= =0A= > + Status =3D EfiBootManagerInitializeLoadOption (=0A= > + &(*BootOptions)[1],=0A= > + LoadOptionNumberUnassigned,=0A= > + LoadOptionTypeBoot,=0A= > + LOAD_OPTION_ACTIVE,=0A= > + L"Grub",=0A= > + DevicePath,=0A= > + NULL,=0A= > + 0=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + FreePool (DevicePath);=0A= =0A= (9) So, with (8) fixed, I believe that you can introduce a helper=0A= function for the first two boot options:=0A= =0A= - A pointer to the textual device path -- you can fill this in from=0A= PcdGetPtr() calls, at the call sites,=0A= - an EFI_BOOT_MANAGER_LOAD_OPTION to populate (via pointer)=0A= =0A= No need to duplicate the same code for both boot options.=0A= =0A= > +=0A= > + FileGuid =3D PcdGetPtr (PcdAndroidBootFile);=0A= > + ASSERT (FileGuid !=3D NULL);=0A= > + Status =3D gBS->HandleProtocol (=0A= > + gImageHandle,=0A= > + &gEfiLoadedImageProtocolGuid,=0A= > + (VOID **) &LoadedImage=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);=0A= > + DevicePath =3D DevicePathFromHandle (LoadedImage->DeviceHandle);= =0A= > + ASSERT (DevicePath !=3D NULL);=0A= > + DevicePath =3D AppendDevicePathNode (=0A= > + DevicePath,=0A= > + (EFI_DEVICE_PATH_PROTOCOL *) &FileNode=0A= > + );=0A= > + ASSERT (DevicePath !=3D NULL);=0A= > + Status =3D EfiBootManagerInitializeLoadOption (=0A= > + &(*BootOptions)[2],=0A= > + LoadOptionNumberUnassigned,=0A= > + LoadOptionTypeBoot,=0A= > + LOAD_OPTION_ACTIVE,=0A= > + L"Android Boot",=0A= > + DevicePath,=0A= > + NULL,=0A= > + 0=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + FreePool (DevicePath);=0A= =0A= This looks correct. Here the pointer that DevicePathFromHandle() returns=0A= for DevicePath is not an "owner" pointer that we must not forget, but a=0A= "navigation" pointer that we are free to forget.=0A= =0A= > +=0A= > + FileGuid =3D PcdGetPtr (PcdAndroidFastbootFile);=0A= > + ASSERT (FileGuid !=3D NULL);=0A= > + Status =3D gBS->HandleProtocol (=0A= > + gImageHandle,=0A= > + &gEfiLoadedImageProtocolGuid,=0A= > + (VOID **) &LoadedImage=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);=0A= > + DevicePath =3D DevicePathFromHandle (LoadedImage->DeviceHandle);= =0A= > + ASSERT (DevicePath !=3D NULL);=0A= > + DevicePath =3D AppendDevicePathNode (=0A= > + DevicePath,=0A= > + (EFI_DEVICE_PATH_PROTOCOL *) &FileNode=0A= > + );=0A= > + ASSERT (DevicePath !=3D NULL);=0A= > + Status =3D EfiBootManagerInitializeLoadOption (=0A= > + &(*BootOptions)[3],=0A= > + LoadOptionNumberUnassigned,=0A= > + LoadOptionTypeBoot,=0A= > + LOAD_OPTION_ACTIVE,=0A= > + L"Android Fastboot",=0A= > + DevicePath,=0A= > + NULL,=0A= > + 0=0A= > + );=0A= > + ASSERT_EFI_ERROR (Status);=0A= > + FreePool (DevicePath);=0A= =0A= (10) This is an almost identical duplicate of the previous boot option.=0A= Another helper function seems possible, which would take:=0A= =0A= - an FFS (firmware file system) file GUID, populated from PcdGetPtr() at=0A= the call site,=0A= - an EFI_BOOT_MANAGER_LOAD_OPTION to populate (via pointer)=0A= =0A= Thanks,=0A= Laszlo=0A= =0A= > + (*BootKeys)[3].ScanCode =3D SCAN_NULL;=0A= > + (*BootKeys)[3].UnicodeChar =3D 'f';=0A= > +=0A= > + *BootCount =3D 4;=0A= > +=0A= > + return EFI_SUCCESS;=0A= > +}=0A= > +=0A= > +PLATFORM_BOOT_MANAGER_PROTOCOL mPlatformBootManager =3D {=0A= > + GetPlatformBootOptionsAndKeys=0A= > +};=0A= > +=0A= > EFI_STATUS=0A= > EFIAPI=0A= > VirtualKeyboardRegister (=0A= > @@ -487,5 +659,15 @@ HiKey960EntryPoint (=0A= > EFI_NATIVE_INTERFACE,=0A= > &mVirtualKeyboard=0A= > );=0A= > + if (EFI_ERROR (Status)) {=0A= > + return Status;=0A= > + }=0A= > +=0A= > + Status =3D gBS->InstallProtocolInterface (=0A= > + &ImageHandle,=0A= > + &gPlatformBootManagerProtocolGuid,=0A= > + EFI_NATIVE_INTERFACE,=0A= > + &mPlatformBootManager=0A= > + );=0A= > return Status;=0A= > }=0A= > diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf b= /Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf=0A= > index cc517656b340..5adedd79d3e8 100644=0A= > --- a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf=0A= > +++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf=0A= > @@ -25,6 +25,7 @@ [Packages]=0A= > EmbeddedPkg/EmbeddedPkg.dec=0A= > MdePkg/MdePkg.dec=0A= > MdeModulePkg/MdeModulePkg.dec=0A= > + Platform/Hisilicon/HiKey960/HiKey960.dec=0A= >=0A= > [LibraryClasses]=0A= > BaseMemoryLib=0A= > @@ -37,15 +38,25 @@ [LibraryClasses]=0A= > PrintLib=0A= > SerialPortLib=0A= > TimerLib=0A= > + UefiBootManagerLib=0A= > UefiBootServicesTableLib=0A= > UefiDriverEntryPoint=0A= > UefiLib=0A= > UefiRuntimeServicesTableLib=0A= >=0A= > [Protocols]=0A= > + gEfiDevicePathFromTextProtocolGuid=0A= > + gEfiLoadedImageProtocolGuid=0A= > gEmbeddedGpioProtocolGuid=0A= > + gPlatformBootManagerProtocolGuid=0A= > gPlatformVirtualKeyboardProtocolGuid=0A= >=0A= > +[Pcd]=0A= > + gHiKey960TokenSpaceGuid.PcdAndroidBootDevicePath=0A= > + gHiKey960TokenSpaceGuid.PcdAndroidBootFile=0A= > + gHiKey960TokenSpaceGuid.PcdAndroidFastbootFile=0A= > + gHiKey960TokenSpaceGuid.PcdSdBootDevicePath=0A= > +=0A= > [Guids]=0A= > gEfiEndOfDxeEventGroupGuid=0A= > gEfiFileInfoGuid=0A= >=0A= =0A= = =