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

[Masahisa Kojima]

On Thu, 19 Oct 2023 at 16:22, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> [...]
>
> > > > > >
> > > > > > 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.
> >
> > What do you mean by "we never call the expand_media_path"?
> > Do you expect we search the partition whether it has a default file,
> > then load the default file with efi_load_image() only when the
> > partition has a default file?
>
> I mean, that the current patch also changed the behavior of the added
> boot options.
> Instead of adding a device path and using expand_media_path() in the
> efibootmgr to load the image, we explicitly added the file path
> instead

OK, I understand.
You explained what the current patch changed the efibootmgr behavior.

Thanks,
Masahisa Kojima

>
> Regards
> /Ilias
> >
> > Thanks,
> > Masahisa Kojima
> >
> >
> > >
> > > 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?
> > >
> > > 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
> > > > > >
> > > > > > >