Re: [Xen-devel] [PATCH] xen: Add EFI_LOAD_OPTION support

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Jan 3, 2018 at 4:20 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 02.01.18 at 16:56, <tamas@xxxxxxxxxxxxx> wrote:
>> When booting Xen via UEFI the Xen config file can contain multiple sections
>> each describing different boot options. It is currently only possible to 
>> choose
>> which section to boot with if Xen is started through an EFI Shell.
>
> Is this true? I thought that EFI Boot Manager entries can very well
> have command line options. And other boot loaders (e.g. grub2)
> should provide their own means to hand over options.

Maybe the comment is inaccurate in that if you use grub or some other
bootloader it can also specify which section to use from the Xen
config. I only tried it with the Shell though so I can't speak for
other bootloaders. If Xen is being launched directly by UEFI (or by
the shim) the only way to specify the section is with
EFI_LOAD_OPTION's OptionalData. When you do that though the current
command line parsing code won't work because it's not a string:

# efibootmgr -w -L "Xen" -l "\EFI\xen\xen.efi" -c -d /dev/nvme0n1
--part 1 -e 3 -u "section5"
# efivar -p --name 8be4df61-93ca-11d2-aa0d-00e098032b8c-Boot0000
GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c
Name: "Boot0000"
Attributes:
    Non-Volatile
    Boot Service Access
    Runtime Service Access
Value:
00000000  01 00 00 00 7c 00 58 00  65 00 6e 00 00 00 02 01  |....|.X.e.n.....|
00000010  0c 00 d0 41 03 0a 00 00  00 00 01 01 06 00 00 1d  |...A............|
00000020  01 01 06 00 00 00 03 17  10 00 01 00 00 00 00 08  |................|
00000030  0d 03 00 0b a8 e0 04 01  2a 00 01 00 00 00 00 08  |........*.......|
00000040  00 00 00 00 00 00 00 00  10 00 00 00 00 00 9b e2  |................|
00000050  c5 ff 57 fa 3d 48 a5 de  00 53 f8 7a bd c4 02 02  |..W.=H...S.z....|
00000060  04 04 26 00 5c 00 45 00  46 00 49 00 5c 00 78 00  |..&.\.E.F.I.\.x.|
00000070  65 00 6e 00 5c 00 78 00  65 00 6e 00 2e 00 65 00  |e.n.\.x.e.n...e.|
00000080  66 00 69 00 00 00 7f ff  04 00 73 00 65 00 63 00  |f.i.......s.e.c.|
00000090  74 00 69 00 6f 00 6e 00  35 00                    |t.i.o.n.5.      |

>> +        if ( elo->Attributes & LOAD_OPTION_ACTIVE )
>
> Without any other (earlier) check, how can you reliably tell this
> being a pointer to EFI_LOAD_OPTION from it being the
> currently used one to CHAR16? Is the distinction perhaps UEFI
> version dependent? In the 2.3 spec I can't find any information
> on the layout of what ->LoadOptions points to.

AFAICT there is no clear cut way to distinguish what is being passed
here. When launched via UEFI Xen receives the EFI_LOAD_OPTION buffer
here already, which it tries to parse as a string and fails. Take a
look at the same problem in the shim:
https://github.com/rhboot/shim/blob/master/shim.c#L2501. If there is a
clearer way to distinguish what is being passed here or more checks
that can be done I would be open for suggestions.

>> --- a/xen/include/efi/efiapi.h
>> +++ b/xen/include/efi/efiapi.h
>
> Generally additions to this file are expected to come from the gnu-efi
> package, which it was originally cloned from. I've just checked and
> see that 3.0.6 doesn't appear to have any of this (yet). In such a
> case at the very least you should match pre-existing style (e.g.
> indentation). Commonly, however, we introduce such private
> (and temporary) definitions into the source file that needs them (see
> e.g. the Apple Properties interface).

This part I took as-is from
tools/firmware/etherboot/ipxe/src/include/ipxe/efi/Uefi/UefiSpec.h as
found in the Xen source tree.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel