[PATCH v7 4/9] efi_loader: create default file boot option

[Heinrich Schuchardt]

On 10/18/23 11:12, Ilias Apalodimas wrote:
> Hi Kojima-san
>
> On Wed, 18 Oct 2023 at 12:07, Masahisa Kojima
> <masahisa.kojima at linaro.org> wrote:
>>
>> Hi Ilias,
>>
>> On Wed, 18 Oct 2023 at 04:47, Ilias Apalodimas
>> <ilias.apalodimas at linaro.org> wrote:
>>>
>>> Hi all,
>>>
>>> On Tue, 17 Oct 2023 at 08:33, Masahisa Kojima
>>> <masahisa.kojima at linaro.org> wrote:
>>>>
>>>> Hi Heinrich,
>>>>
>>>> On Mon, 16 Oct 2023 at 23:52, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>>
>>>>> On 16.10.23 15:00, Masahisa Kojima wrote:
>>>>>> On Mon, 16 Oct 2023 at 21:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>>>>
>>>>>>> On 16.10.23 14:31, Ilias Apalodimas wrote:
>>>>>>>> Hi Heinrich,
>>>>>>>>
>>>>>>>> On Mon, 16 Oct 2023 at 10:06, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Am 16. Oktober 2023 08:45:21 MESZ schrieb Masahisa Kojima <masahisa.kojima at linaro.org>:
>>>>>>>>>> Current efibootmgr automatically creates the
>>>>>>>>>> boot options of all disks and partitions installing
>>>>>>>>>> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
>>>>>>>>>> Some of the automatically created boot options are
>>>>>>>>>> useless if the disk and partition does not have
>>>>>>>>>> the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
>>>>>>>>>>
>>>>>>>>>> This commit only creates the boot option if the disk and
>>>>>>>>>> partition have the default file so that system can directly
>>>>>>>>>> boot from it.
>>>>>>>>>
>>>>>>>>> I don't directly see the user benefit.
>>>>>>>>
>>>>>>>> The user can add an HTTP boot option now and the installer will
>>>>>>>> automatically start.  That would allow products to ship with a single
>>>>>>>> boot option provisioned and run an installer on first boot
>>>>>>>
>>>>>>> This commit is not about HTTP. It changes how boot options for block
>>>>>>> devices are created.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Reading all file systems will increase the boot time. Shouldn't we avoid this?
>>>>>>>>
>>>>>>>> Any idea what would be an alternative?  But when we added the
>>>>>>>> automatic boot options we said we should avoid dynamic scanning and
>>>>>>>> store results in a file.  This is doing a similar thing.  The only
>>>>>>>> difference is that it mounts the iso image before adding the boot
>>>>>>>> option.
>>>>>>>
>>>>>>> The alternative is to keep showing boot options for block devices even
>>>>>>> if there is no BOOTxxxx.EFI file.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> What does EDK II do?
>>>>>>>>
>>>>>>>> No Idea :)
>>>>>>>
>>>>>>>
>>>>>>> On my workstation I get generated boot options
>>>>>>>
>>>>>>> Boot0001* UEFI:CD/DVD Drive     BBS(129,,0x0)
>>>>>>> Boot0003* UEFI:Removable Device BBS(130,,0x0)
>>>>>>> Boot0004* UEFI:Network Device   BBS(131,,0x0)
>>>>>>>
>>>>>>> without any media inserted and without any PXE server available.
>>>>>>
>>>>>> It is just information about how the EDK2 works.
>>>>>> When I attach the Fedora installation media on EDK2(OVMF),
>>>>>> the automatically generated boot option is as follows.
>>>>>>
>>>>>> UEFI QEMU HARDDISK QM00001 : PciRoot(0x0)/Pci(0x1,0x1)/Ata(Primary,Master,0x0)
>>>>>
>>>>> An ATAPI drive typically is not removable. So I wonder why it is listed.
>>>>> Did you set the removable flag on the command line?
>>>>
>>>> I guess it is not removable(actually I don't know how to set the
>>>> device as removable).
>>>> I just attached the iso image to QEMU with something like '-hda
>>>> Fedora_netinst.iso".
>>>>
>>>> According to the EDK II implementation[1], the boot option is
>>>> enumerated with the following order.
>>>>    1. Removable BlockIo
>>>>    2. Fixed BlockIo
>>>>    3. Non-BlockIo SimpleFileSystem
>>>>    4. LoadFile
>>>> So boot option for the fixed device such as HDD is also automatically created.
>>>>
>>>> [1] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c#L2150
>>>>
>>>>>
>>>>>>
>>>>>> When this boot option is selected, Fedora installer automatically starts.
>>>>>> So EDK II is searching the default file on the fly.
>>>>>
>>>>> What is shown if you attach a medium without Bootaa64.efi?
>>>>
>>>> The same boot option is created.
>>>> UEFI QEMU HARDDISK QM00001 : PciRoot(0x0)/Pci(0x1,0x1)/Ata(Primary,Master,0x0)
>>>
>>> I went back to reading the spec and I think Heinrich is right.  We
>>> don't need that check at all. Going through [0] paragraph 4 says
>>> " This search occurs when the device path of the boot image listed in
>>> any boot option points directly to an EFI_SIMPLE_FILE_SYSTEM_PROTOCOL
>>> device and does not specify the exact file to load"
>>>
>>> So we should *only* add an automatic variable without the default
>>> application.  Our code in try_load_entry() will search for that
>>>
>>
>> Thank you for checking the UEFI specification and sorry for
>> overlooking the above.
>> So we will go back to the previous on the fly default application search.
>
> Yes, I was about to suggest that.
> The problem as I understand it that the current patch not only
> disregards disks and partitions that dont have a default (i.e
> bootaa64.efi) file. It also *changes* the default boot option we add,
> and instead of the disk it adds a file.  That is the part that's
> against the spec.  On top of that it changes the behavior of efi
> bootmgr and we never call the expand_media_path.
>
> So my suggestion would be
> - Drop #4
> - Adjust patch 5 and instead of loading the boot entry directly, scan
> for the special autogenerated boot option and look for that file there
>
> Heinrich would that work for you?

That is fine with me. This allows us to handle any deviations of U-Boot
from the spec concerning generated boot options in a separate patch series.

We still need to look into adding the deletion of the blkmap resource in
case of error or when the EFI application returns.

Best regards

Heinrich

>
> Thanks
> /Ilias
>>
>> Thanks,
>> Masahisa Kojima
>>
>>> [0] https://uefi.org/specs/UEFI/2.10/03_Boot_Manager.html#load-option-processing
>>>
>>> Regards
>>> /Ilias
>>>>
>>>> Thanks,
>>>> Masahisa Kojima
>>>>
>>>>>