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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.18 at 17:00, <tamas@xxxxxxxxxxxxx> 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?

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 <tamas@xxxxxxxxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel