From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7E6392251212F for ; Fri, 20 Apr 2018 04:33:12 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B1067EAEBB; Fri, 20 Apr 2018 11:33:11 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-11.rdu2.redhat.com [10.10.120.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id CC3FBAFD52; Fri, 20 Apr 2018 11:33:10 +0000 (UTC) To: Haojian Zhuang , edk2-devel@lists.01.org Cc: Leif Lindholm , Ard Biesheuvel References: <1524134560-1245-1-git-send-email-haojian.zhuang@linaro.org> <1524134560-1245-2-git-send-email-haojian.zhuang@linaro.org> From: Laszlo Ersek Message-ID: <22875fc6-2c8c-481c-c4a1-3ecd27f2aee1@redhat.com> Date: Fri, 20 Apr 2018 13:33:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <1524134560-1245-2-git-send-email-haojian.zhuang@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 20 Apr 2018 11:33:11 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 20 Apr 2018 11:33:11 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH v3 1/2] EmbeddedPkg: add platform boot manager protocol 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: Fri, 20 Apr 2018 11:33:12 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hello Haojian, On 04/19/18 12:42, Haojian Zhuang wrote: > Create the PlatformBootManagerProtocol that is used to add > predefined boot options in platform driver. This interface > will be used in ArmPkg/PlatformBootManagerLib. > > Cc: Laszlo Ersek > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Haojian Zhuang > --- > EmbeddedPkg/EmbeddedPkg.dec | 1 + > EmbeddedPkg/Include/Protocol/PlatformBootManager.h | 46 ++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h > > diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec > index eb135340b173..9cd50738363b 100644 > --- a/EmbeddedPkg/EmbeddedPkg.dec > +++ b/EmbeddedPkg/EmbeddedPkg.dec > @@ -81,6 +81,7 @@ [Protocols.common] > gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 0x08, 0x12, 0x54, 0x7a, 0xc2 }} > gAndroidBootImgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 0xf8, 0xda, 0x65, 0x65, 0xf4, 0xa5 }} > gPlatformVirtualKeyboardProtocolGuid = { 0x0e3606d2, 0x1dc3, 0x4e6f, { 0xbe, 0x65, 0x39, 0x49, 0x82, 0xa2, 0x65, 0x47 }} > + gPlatformBootManagerProtocolGuid = { 0x7197c8a7, 0x6559, 0x4c93, { 0x93, 0xd5, 0x8a, 0x84, 0xf9, 0x88, 0x79, 0x8b }} > > [Ppis] > gEdkiiEmbeddedGpioPpiGuid = { 0x21c3b115, 0x4e0b, 0x470c, { 0x85, 0xc7, 0xe1, 0x05, 0xa5, 0x75, 0xc9, 0x7b }} > diff --git a/EmbeddedPkg/Include/Protocol/PlatformBootManager.h b/EmbeddedPkg/Include/Protocol/PlatformBootManager.h > new file mode 100644 > index 000000000000..8fd63c8b45d6 > --- /dev/null > +++ b/EmbeddedPkg/Include/Protocol/PlatformBootManager.h > @@ -0,0 +1,46 @@ > +/** @file > + > + Copyright (c) 2018, Linaro. All rights reserved.
> + > + This program and the accompanying materials > + are licensed and made available under the terms and conditions of the BSD License > + which accompanies this distribution. The full text of the license may be found at > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > + > +**/ > + > +#ifndef __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ > +#define __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ > + > +// > +// Protocol interface structure > +// > +typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL PLATFORM_BOOT_MANAGER_PROTOCOL; > + > +// > +// Function Prototypes > +// > + > +/* > + Get predefined boot options for platform. > + > + @param[out] BootOptions An array of predefined boot options. > + @param[out] BootKeys An array of predefined keys. > +*/ > +typedef > +EFI_STATUS > +(EFIAPI *GET_PLATFORM_BOOT_OPTIONS_AND_KEYS) ( > + OUT EFI_BOOT_MANAGER_LOAD_OPTION **BootOptions, > + OUT EFI_INPUT_KEY **BootKeys > + ); > + > +struct _PLATFORM_BOOT_MANAGER_PROTOCOL { > + GET_PLATFORM_BOOT_OPTIONS_AND_KEYS GetPlatformBootOptionsAndKeys; > +}; > + > +extern EFI_GUID gPlatformBootManagerProtocolGuid; > + > +#endif /* __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ */ > this looks good generally, but the protocol documentation is still seriously lacking. The documentation should enable anyone to call the member and know everything about the intended use and lifecycle of the output parameters. Also, because the two output arrays are supposed to be used in tandem, I suggest that we replace the terminator zero element with an explicit element count (a UINTN output parameter). For example: @param[out] Count The number of elements in each of BootOptions and BootKeys. On successful return, Count is at least one. @param[out] BootOptions An array of platform boot options. BootOptions is pool-allocated by GET_PLATFORM_BOOT_OPTIONS_AND_KEYS, and GET_PLATFORM_BOOT_OPTIONS_AND_KEYS populates every element in BootOptions with EfiBootManagerInitializeLoadOption(). BootOptions shall not contain duplicate entries. The caller is responsible for releasing BootOptions after use with EfiBootManagerFreeLoadOptions(). @param[out] BootKeys A pool-allocated array of platform boot hotkeys. For every 0 <= Index < Count, If BootOptions[Index] is not to be associated with a hotkey, then BootKeys[Index] is zero-filled. Otherwise, BootKeys[Index] specifies the boot hotkey for BootOptions[Index]. BootKeys shall not contain duplicate entries (other than zero-filled entries). The caller is responsible for releasing BootKeys with FreePool() after use. @retval EFI_SUCCESS Count, BootOptions and BootKeys have been set. @retval EFI_OUT_OF_RESOURCES Memory allocation failed. @retval EFI_UNSUPPORTED The platform doesn't provide boot options as a feature. @retval EFI_NOT_FOUND The platform could provide boot options as a feature, but none have been configured. @return Error codes propagated from underlying functions. Thanks Laszlo