Re: [PATCH v3 1/3] xen/efi: Handle cases where file didn't come from ESP

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Thu, Jun 26, 2025 at 1:53 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 25.06.2025 09:34, Frediano Ziglio wrote:
> > A boot loader can load files from outside ESP.
>
> I think it would have helped if you said somewhere what ESP is. People may
> think of this as UEFI System Partition (or some such), which doesn't collapse
> to the acronym used.
>

The acronym is EFI System Partition (ESP).
Most files and prefixes use "efi" more than "uefi".

> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -443,6 +443,18 @@ static EFI_FILE_HANDLE __init get_parent_handle(const 
> > EFI_LOADED_IMAGE *loaded_i
> >      CHAR16 *pathend, *ptr;
> >      EFI_STATUS ret;
> >
> > +    /*
> > +     * In some cases the image could not come from a specific device.
> > +     * For instance this can happen if Xen was loaded using GRUB2 "linux"
> > +     * command.
> > +     */
>
> I consider this bogus: Why in the world would one use "linux" for loading Xen?
>

The question could be why "linux" for efi is very different from
no-efi "linux" not having much to do with Linux kernel.
One answer is that EFI is dominated by PE format and so "linux"
command just use PE standard entry, relocations and "LoadOptions" and
use specific protocols (like LoadFile2 here) instead of Linux standard
ways.

> Yet I'll commit this as-is anyway, as Marek has acked it, just with ...
>
> > @@ -1404,7 +1427,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE 
> > ImageHandle,
> >          /* Read and parse the config file. */
> >          if ( read_section(loaded_image, L"config", &cfg, NULL) )
> >              PrintStr(L"Using builtin config file\r\n");
> > -        else if ( !cfg_file_name )
> > +        else if ( !cfg_file_name && file_name)
>
> ... the style issue addressed here.
>

Done, sending v4

> Jan

Frediano