On Thu, May 17, 2018 at 2:03 AM, Jan Beulich <jbeul...@suse.com> wrote:
>>>> On 07.02.18 at 17:00, <ta...@tklengyel.com> wrote:
>> This patch as-is correctly tells the two possible formats apart. I
>> tested and Xen boots correctly both from the Shell and from the
>> firmware boot menu. I would not like to start addressing hypothetical
>> scenarios that I have no reasonable way to test against. If you are
>> inclined to do that, that's your call but I'll just leave this patch
>> here for now and I hope you would consider merging it.
>
> Would you mind giving the tentative v4 (below) a try?
Unfortunately this does not seem to work as intended:
# cat /boot/efi/EFI/xen/xen.cfg
[global]
default=old
[old]
options=console=vga
kernel=vmlinuz-4.9.0-6-amd64
root=UUID=19f184db-23a8-42c6-8dfa-67816c822573 ro quiet
ramdisk=initrd.img-4.9.0-6-amd64
[new]
options=console=vga,com1 com1=115200,8n1,amt loglvl=all
guest_loglvl=all altp2m=1
kernel=vmlinuz-4.9.0-6-amd64
root=UUID=19f184db-23a8-42c6-8dfa-67816c822573 ro quiet
ramdisk=initrd.img-4.9.0-6-amd64
# efibootmgr -v
BootCurrent: 0001
Timeout: 0 seconds
BootOrder: 0001,0000,0003,0004,0005,0006,0007
Boot0000* Xen
HD(1,GPT,ffc5e29b-fa57-483d-a5de-0053f87abdc4,0x800,0x100000)/File(\EFI\xen\xen.efi)
Boot0001* Xen altp2m
HD(1,GPT,ffc5e29b-fa57-483d-a5de-0053f87abdc4,0x800,0x100000)/File(\EFI\xen\xen.efi)n.e.w.
# xl info
...
xen_commandline : console=vga
As you can see boot option 1 (Xen altp2m) was used for booting but Xen
still used the default global option from the config file instead of
the one specified by the OptionalData.
Tamas
>
> Jan
>
> EFI: add EFI_LOAD_OPTION support
>
> 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 the buffer contains a
> string. UEFI provides a different standard to pass optional arguments
> to an application, and in this patch we make Xen properly parse this
> buffer, thus making it possible to have separate EFI boot options
> present for the different config sections.
>
> Signed-off-by: Tamas K Lengyel <ta...@tklengyel.com>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
> ---
> v4: Address my own review comments.
>
> --- unstable.orig/xen/common/efi/boot.c
> +++ unstable/xen/common/efi/boot.c
> @@ -88,6 +88,14 @@ typedef struct _EFI_APPLE_PROPERTIES {
> EFI_APPLE_PROPERTIES_GETALL GetAll;
> } EFI_APPLE_PROPERTIES;
>
> +typedef struct _EFI_LOAD_OPTION {
> + UINT32 Attributes;
> + UINT16 FilePathListLength;
> + CHAR16 Description[];
> +} EFI_LOAD_OPTION;
> +
> +#define LOAD_OPTION_ACTIVE 0x00000001
> +
> union string {
> CHAR16 *w;
> char *s;
> @@ -275,6 +283,16 @@ static int __init wstrncmp(const CHAR16
> return n ? *s1 - *s2 : 0;
> }
>
> +static const CHAR16 *__init wmemchr(const CHAR16 *s, CHAR16 c, UINTN n)
> +{
> + while ( n && *s != c )
> + {
> + --n;
> + ++s;
> + }
> + return n ? s : NULL;
> +}
> +
> static CHAR16 *__init s2w(union string *str)
> {
> const char *s = str->s;
> @@ -374,14 +392,49 @@ static void __init PrintErrMesg(const CH
> }
>
> static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
> - CHAR16 *cmdline, UINTN cmdsize,
> + VOID *data, UINTN size, UINTN *offset,
> CHAR16 **options)
> {
> - CHAR16 *ptr = (CHAR16 *)(argv + argc + 1), *prev = NULL;
> + CHAR16 *ptr = (CHAR16 *)(argv + argc + 1), *prev = NULL, *cmdline = NULL;
> bool prev_sep = true;
>
> - for ( ; cmdsize > sizeof(*cmdline) && *cmdline;
> - cmdsize -= sizeof(*cmdline), ++cmdline )
> + if ( *offset < size )
> + cmdline = data + *offset;
> + else if ( size > sizeof(*cmdline) && !(size % sizeof(*cmdline)) &&
> + (wmemchr(data, 0, size / sizeof(*cmdline)) ==
> + data + size - sizeof(*cmdline)) )
> + {
> + *offset = 0;
> + cmdline = data;
> + }
> + else if ( size > sizeof(EFI_LOAD_OPTION) )
> + {
> + const EFI_LOAD_OPTION *elo = data;
> + /* The minimum size the buffer needs to be. */
> + size_t elo_min = offsetof(EFI_LOAD_OPTION, Description[1]) +
> + elo->FilePathListLength;
> +
> + if ( (elo->Attributes & LOAD_OPTION_ACTIVE) && size > elo_min &&
> + !((size - elo_min) % sizeof(*cmdline)) )
> + {
> + const CHAR16 *desc = elo->Description;
> + const CHAR16 *end = wmemchr(desc, 0,
> + (size - elo_min) / sizeof(*desc) +
> 1);
> +
> + if ( end )
> + {
> + *offset = elo_min + (end - desc) * sizeof(*desc);
> + if ( (size -= *offset) > sizeof(*cmdline) )
> + cmdline = data + *offset;
> + }
> + }
> + }
> +
> + if ( !cmdline )
> + return 0;
> +
> + for ( ; size > sizeof(*cmdline) && *cmdline;
> + size -= sizeof(*cmdline), ++cmdline )
> {
> bool cur_sep = *cmdline == L' ' || *cmdline == L'\t';
>
> @@ -1095,15 +1148,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY
>
> if ( use_cfg_file )
> {
> + UINTN offset = ~(UINTN)0;
> +
> argc = get_argv(0, NULL, loaded_image->LoadOptions,
> - loaded_image->LoadOptionsSize, NULL);
> + loaded_image->LoadOptionsSize, &offset, NULL);
> if ( argc > 0 &&
> efi_bs->AllocatePool(EfiLoaderData,
> (argc + 1) * sizeof(*argv) +
> loaded_image->LoadOptionsSize,
> (void **)&argv) == EFI_SUCCESS )
> get_argv(argc, argv, loaded_image->LoadOptions,
> - loaded_image->LoadOptionsSize, &options);
> + loaded_image->LoadOptionsSize, &offset, &options);
> else
> argc = 0;
> for ( i = 1; i < argc; ++i )
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
