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 822CC203B859E for ; Tue, 15 May 2018 01:22:33 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 21F78406F132; Tue, 15 May 2018 08:22:32 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-147.rdu2.redhat.com [10.10.120.147]) by smtp.corp.redhat.com (Postfix) with ESMTP id 951C72166BAD; Tue, 15 May 2018 08:22:29 +0000 (UTC) To: Marvin H?user , "edk2-devel@lists.01.org" Cc: "eric.dong@intel.com" , "star.zeng@intel.com" References: From: Laszlo Ersek Message-ID: <12b0e557-4f3b-3766-1e52-c069c02b692e@redhat.com> Date: Tue, 15 May 2018 10:22:28 +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: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 15 May 2018 08:22:32 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 15 May 2018 08:22:32 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: Proposition of a BmEnumerateBootOptions() hook. 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: Tue, 15 May 2018 08:22:33 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 05/14/18 21:00, Marvin H?user wrote: > Hey Star, Eric and everyone else, > > I have seen that some platforms add a Boot Option for the UEFI Shell > in "PlatformBootManagerBeforeConsole()", which is called as part of > the regular boot flow. This is surely beneficial for development > platforms that are supposed to boot to UEFI Shell by default when no > other option has been registered, (Side point: I sure wish *all* platforms included the UEFI shell, one way or another. Debugging issues is the *default* state of any computing system (all software has bugs), so debug tools must be a first class citizen. Forums are full of end-users asking for the UEFI shell on their physical platforms; frequently because their platform firmware gives them an extremely limited interface to managing boot and driver options, and the shell is seen as a way out of that, justifiedly.) > however for retail platforms it usually makes more sense to show the > UEFI Boot Menu, Note that this is already solved in BdsDxe (if I understand your point anyway); please refer to commit d1de487dd2e7 ("MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before hanging", 2017-11-27). > which renders adding the Shell Boot Option as part of the regular boot > flow obsolete and just adds up to the boot time. Meanwhile, there is a > function in the UefiBootManagerLib, "BmEnumerateBootOptions()", which > is called prior to entering the Boot Menu and, in my opinion, would be > the perfect place to introduce another PlatformBootManagerLib hook, > which retrieves platform-specific boot options, such as an UEFI Shell > or other utilities like a Memory Test application. Hmmm, I'm not sure. The only function that calls BmEnumerateBootOptions() is EfiBootManagerRefreshAllBootOption(). The latter is a public UefiBootManagerLib API that several PlatformBootManagerLib instances call, usually from PlatformBootManagerAfterConsole(). PlatformBootManagerAfterConsole() is already a PlatformBootManagerLib hook, so the suggestion would result in one hook calling back into another hook, of the same library instance: BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c] PlatformBootManagerAfterConsole() [SomePlatformPkg/Library/PlatformBootManagerLib/...] EfiBootManagerRefreshAllBootOption() [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c] BmEnumerateBootOptions() [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c] PlatformBootManagerAnotherHook() [SomePlatformPkg/Library/PlatformBootManagerLib/...] If I'm being honest, I'd like to avoid this -- if I'm in PlatformBootManagerBeforeConsole() or PlatformBootManagerAfterConsole(), I'd just like to do whatever's necessary by directly calling either public UefiBootManagerLib APIs, or STATIC functions from the same PlatformBootManagerLib instance that I'm already inside of. Now, what I could see as useful is a public helper function in UefiBootManagerLib that registers the shell boot option. This functionality is usually duplicated across several PlatformBootManagerLib instances. I'll also mention another interface that the edk2-platforms project uses -- several platforms there use the same PlatformBootManagerLib instance, and they abstract out just the default set of platform boot options. Please see [PATCH v4 0/2] add platform boot manager protocol http://mid.mail-archive.com/1524464514-14454-1-git-send-email-haojian.zhuang@linaro.org Thanks Laszlo