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 2F909209603E6 for ; Tue, 15 May 2018 06:52:45 -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 1430D81536A0; Tue, 15 May 2018 13:52:45 +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 904326352A; Tue, 15 May 2018 13:52:35 +0000 (UTC) To: =?UTF-8?Q?Marvin_H=c3=a4user?= , "edk2-devel@lists.01.org" Cc: "eric.dong@intel.com" , "star.zeng@intel.com" , "jordan.l.justen@intel.com" , "ard.biesheuvel@linaro.org" , "ruiyu.ni@intel.com" , "Gabriel L. Somlo (GMail)" , Gerd Hoffmann References: <12b0e557-4f3b-3766-1e52-c069c02b692e@redhat.com> From: Laszlo Ersek Message-ID: <9d7aa3e0-d342-ab5a-f54b-2a853a5fcf57@redhat.com> Date: Tue, 15 May 2018 15:52:34 +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.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.8]); Tue, 15 May 2018 13:52:45 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 15 May 2018 13:52:45 +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: 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 13:52:46 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 05/15/18 15:02, Marvin Häuser wrote: >> -----Original Message----- >> From: Laszlo Ersek >> Sent: Tuesday, May 15, 2018 10:22 AM >> To: Marvin Häuser ; edk2- >> devel@lists.01.org >> Cc: eric.dong@intel.com; star.zeng@intel.com >> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook. >> >> On 05/14/18 21:00, Marvin Häuser wrote: >>> [...] 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() > > I have misunderstood the current edk2 logic to be what I expected > (partially), however I have just discovered that scanned boot options > are added to NV and to BootOrder. This is not a coincidence. That's what the EfiBootManagerRefreshAllBootOption() API does, by design, and that's the functionality for which platforms call the API. > Furthermore, I did not mention another usage case for the proposed > hook as I thought it was practically irrelevant to the edk2 community: > Supporting Apple's bless boot concept. (For those who wonder, > virtualizing macOS on Apple hardware is perfectly legal). So, I will > have to explain a bit more in detail, in two parts. > > UEFI BOOT > > What I imagined to be the boot flow, by the specification and by my > personal taste, is the following: > 1. Boot BootOrder[] one-by-one. This connects the referenced drive and > attempts to start the specified file. This is the case today. > 2. Boot *Recovery[]. This is the case today. > 3. There are two alternatives here, based on platform preference. > 3.1. Boot enumerated drives (via the algorithm defined in the UEFI > spec). I think this is not the case today. > 3.2. Enumerate the drives (via the algorithm defined in the UEFI spec) > and show them in BootManagerMenuApp. I think this is done today > and is what I would prefer too. > > What I basically do not agree with in the list above are two nuances. > 3.1 and 3.2: I would like to see support for a platform-defined logic > of enumerating in addition to the one defined in the UEFI spec. This > can be used for UEFI Shell, platform diagnostic apps and Apple bless. I think platform-specific boot option generation -- enumeration of various protocols that stand for different kinds of devices, and generating boot options from them -- is already possible today. Platforms call EfiBootManagerRefreshAllBootOption() because (a) it's easy to call, (b) it generally does the right thing. (Note that this API does not connect devices; it just scans devices and turns whatever it finds into boot options.) Nothing stops a platform from *not* calling EfiBootManagerRefreshAllBootOption() -- the platform can simply skip that and generate other options. Or the platform may generate options in addition to those that are generated by the API. > 3.2: I think adding them as volatile variables is the better approach, > except for platform-specific code, which should be capable of adding > NV options. The reasoning behind this is that usually the found > volumes are removable volumes (such as USB Flash Drives) and will be > cleaned away soon again anyway. "Extra wishes" for boot options should > be added via the UEFI Setup menu or UEFI Shell. I can't really comment on this -- the fact that Boot#### options are non-volatile comes from the UEFI spec. I'm not immediately sure what I think of their suggested volatility, but this is likely something for the USWG to discuss. > The reason why I still wish for platform code to be able to add NV > options as part of enumeration is that FV-embedded tools are indeed NV > (Shell etc.) and will not need to be cleaned up, as well as that we're > not in an utopia. Sometimes an unwanted purge of boot option happens > ("CMOS reset" on boards, etc) and consumer platform provides do not > want to leave their users with a constant boot to BootMenu or no > option at all. Platform code often does currently and probably should > add a NV Windows boot option in the proposed hook (this should of > course not be part of edk2). Furthermore, if an OS boot option is > rendered invalid, usually there is no OsRecovery option available as > proposed in the specification. > > Considering the boot protocol I just described, I think that the call > in PlatformBootManagerAfterConsole() is not a good idea, because it > refreshes boot options unconditionally. When the first element of > BootOrder[] can be booted, there is no reason to do so and it costs > time. Such a refresh makes sense when booting the first option fails > though (to make sure it is purged to not delay boot again). I think there are two possibilities for disagreement here. First, if the first boot option cares about the full set of Boot#### options (for whatever reasons), then this is an incompatible change. Second, just because a boot option fails, it should not be removed. For example, a netboot option could fail, or a USB drive might not be connected (temporarily). I don't think that's grounds enough for summarily removing a boot option, in shared reference code. > The function is also called by BootManagerMenuApp in the entry point, > which makes a lot of sense. In the scenario described above, "scanned" > boot options are added only when the NV ones cannot be booted and > MenuApp is rendered in consequence. It basically moves the scanning > delay from the "NV phase" to the following "scanning phase", which is > not supposed to be entered when booting a "sane" platform. For OVMF specifically, calling EfiBootManagerRefreshAllBootOption() at every boot is necessary. The set of bootable devices and their desired boot order may change at every boot, and the boot order spec that comes from QEMU isn't expressive enough for the *generation* of boot options; it's expressive enough only for the *filtering* and reordering of boot options. > APPLE BLESS > > This might be interesting for the OVMF maintainers. > I did not want to mention this concept at first, because I don't think > there is a terrible huge demand or interest, however I would like to > be able to implement Apple bless support for OVMF without having to > fork and modify edk2 drivers. > I did not check about a concrete way of implementation, however I will > shortly describe what needs to be involved. > > Secondary partitions: Supporting this will be easy when accepting the > hook proposed above and considering my comments regarding NV vs > volatile variables. No boot options are proactively added for those > installs and they are scanned on demand, which can be done entirely in > the proposed hook function. I think it could also be done in AfterConsole(). I realize it might incur more pflash traffic -- like any other Boot#### option generation -- than what you might consider optimal, but functionally that shouldn't be a barrier. > Primary partition: The so-called "Startup Volume" unfortunately is a > bit trickier. For it, a practically invalid Boot Option is added, > which is an expanded device path to the volume to be booted, however > without having a File Device Path Node appended. This doesn't immediately seem invalid -- if memory serves, you can have \EFI\BOOT\BOOT.efi on fixed media as well, and if a boot option only names the HD() in question, that default boot path will be launched off of it. > I had hoped to be able to use EFI_LOAD_FILE_PROTOCOL, however > obviously EFI_SIMPLE_FILE_SYSTEM_PROTOCOL will attach to the mentioned > DevicePath, which means LoadFile will not be called. Support for this > would need to be done via a platform-specific error handler for when > the DevicePath is determined to be invalid, which can then either fix > it up and return success or fail as well. I have not looked into this > in detail and I can understand if you are not interested in supporting > such a scenario. However if you do, I will look into it as soon as > possible and probably perform a few tests in OVMF. I have a more general comment in the end: I doubt I could legally test an OSX guest on non-Apple hardware, so I won't try (and I'll most likely not buy or otherwise procure OSX, let alone Apple hardware, just for this). That means, if you post patches, my main focus will be on keeping the current behavior regression-free, and (secondarily) the implementation preferably simple & separated. I've added Gabriel and Gerd to the CC list because I believe they might be interested in OSX guests (I seem to remember that a sizeable out-of-tree patchset is necessary for OSX guests anyway). Thanks, Laszlo