diff mbox

[Xen-devel,V4,06/15] Add efi_arch_handle_cmdline() for processing commandline

Message ID 1410310325-4509-7-git-send-email-roy.franz@linaro.org
State New
Headers show

Commit Message

Roy Franz Sept. 10, 2014, 12:51 a.m. UTC 
Add arch function for processing the Xen commandline and
updating internal structures.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 xen/common/efi/boot.c          | 41 +++++++++++------------------------------
 xen/include/asm-x86/efi-boot.h | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 30 deletions(-)

Comments

Jan Beulich Sept. 11, 2014, 2:22 p.m. UTC | #1 
>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote:
> @@ -786,19 +786,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>          efi_bs->FreePool(name.w);
>      }
>  
> -    name.s = get_value(&cfg, section.s, "options");
> -    if ( name.s )
> -        place_string(&mbi.cmdline, name.s);
> -    /* Insert image name last, as it gets prefixed to the other options. */
> -    if ( argc )
> -    {
> -        name.w = *argv;
> -        w2s(&name);
> -    }
> -    else
> -        name.s = "xen";
> -    place_string(&mbi.cmdline, name.s);
> -
>      cols = rows = depth = 0;
>      if ( !base_video )
>      {
> @@ -817,6 +804,9 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>          }
>      }
>  
> +    name.s = get_value(&cfg, section.s, "options");
> +    efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
> +
>      efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
>      cfg.addr = 0;
>  

Again - is there a particular reason to move this down?

> --- a/xen/include/asm-x86/efi-boot.h
> +++ b/xen/include/asm-x86/efi-boot.h
> @@ -607,3 +607,37 @@ static void __init efi_arch_cfg_file(EFI_FILE_HANDLE 
> dir_handle, char *section)
>          efi_bs->FreePool(name.w);
>      }
>  }
> +
> +static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
> +                                           CHAR16 *cmdline_options,
> +                                           char *cfgfile_options)
> +{
> +    union string name;
> +
> +    if ( cmdline_options )
> +    {
> +        name.w = cmdline_options;
> +        w2s(&name);
> +        place_string(&mbi.cmdline, name.s);
> +    }
> +    if ( cfgfile_options )
> +        place_string(&mbi.cmdline, cfgfile_options);
> +    /* Insert image name last, as it gets prefixed to the other options. */
> +    if ( image_name )
> +    {
> +        name.w = image_name;
> +        w2s(&name);
> +    }
> +    else
> +        name.s = "xen";
> +    place_string(&mbi.cmdline, name.s);
> +
> +    if ( mbi.cmdline )
> +        mbi.flags |= MBI_CMDLINE;
> +    /*
> +     * These must not be initialized statically, since the value must
> +     * not get relocated when processing base relocations below.
> +     */

The "below" here will need adjustment now that this doesn't live in
the same function anymore.

Jan
Roy Franz Sept. 11, 2014, 6:24 p.m. UTC | #2 
On Thu, Sep 11, 2014 at 7:22 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote:
>> @@ -786,19 +786,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>>          efi_bs->FreePool(name.w);
>>      }
>>
>> -    name.s = get_value(&cfg, section.s, "options");
>> -    if ( name.s )
>> -        place_string(&mbi.cmdline, name.s);
>> -    /* Insert image name last, as it gets prefixed to the other options. */
>> -    if ( argc )
>> -    {
>> -        name.w = *argv;
>> -        w2s(&name);
>> -    }
>> -    else
>> -        name.s = "xen";
>> -    place_string(&mbi.cmdline, name.s);
>> -
>>      cols = rows = depth = 0;
>>      if ( !base_video )
>>      {
>> @@ -817,6 +804,9 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>>          }
>>      }
>>
>> +    name.s = get_value(&cfg, section.s, "options");
>> +    efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
>> +
>>      efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
>>      cfg.addr = 0;
>>
>
> Again - is there a particular reason to move this down?
No, I will move it to the location of the code that was removed.

>
>> --- a/xen/include/asm-x86/efi-boot.h
>> +++ b/xen/include/asm-x86/efi-boot.h
>> @@ -607,3 +607,37 @@ static void __init efi_arch_cfg_file(EFI_FILE_HANDLE
>> dir_handle, char *section)
>>          efi_bs->FreePool(name.w);
>>      }
>>  }
>> +
>> +static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
>> +                                           CHAR16 *cmdline_options,
>> +                                           char *cfgfile_options)
>> +{
>> +    union string name;
>> +
>> +    if ( cmdline_options )
>> +    {
>> +        name.w = cmdline_options;
>> +        w2s(&name);
>> +        place_string(&mbi.cmdline, name.s);
>> +    }
>> +    if ( cfgfile_options )
>> +        place_string(&mbi.cmdline, cfgfile_options);
>> +    /* Insert image name last, as it gets prefixed to the other options. */
>> +    if ( image_name )
>> +    {
>> +        name.w = image_name;
>> +        w2s(&name);
>> +    }
>> +    else
>> +        name.s = "xen";
>> +    place_string(&mbi.cmdline, name.s);
>> +
>> +    if ( mbi.cmdline )
>> +        mbi.flags |= MBI_CMDLINE;
>> +    /*
>> +     * These must not be initialized statically, since the value must
>> +     * not get relocated when processing base relocations below.
>> +     */
>
> The "below" here will need adjustment now that this doesn't live in
> the same function anymore.
I'll fix this.

>
> Jan
>
diff mbox

Patch

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index a33a8f6..fd9d382 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -53,6 +53,7 @@  static char *__init get_value(const struct file *cfg, const char *section,
                               const char *item);
 static void __init split_value(char *s);
 static CHAR16 *__init s2w(union string *str);
+static char *__init w2s(const union string *str);
 static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
                                struct file *file);
 
@@ -251,7 +252,8 @@  static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode)
 }
 
 static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
-                                    CHAR16 *cmdline, UINTN cmdsize)
+                                    CHAR16 *cmdline, UINTN cmdsize,
+                                    CHAR16 **options)
 {
     CHAR16 *ptr = (CHAR16 *)(argv + argc + 1), *prev = NULL;
     bool_t prev_sep = TRUE;
@@ -277,10 +279,8 @@  static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
                 ++argc;
             else if ( prev && wstrcmp(prev, L"--") == 0 )
             {
-                union string rest = { .w = cmdline };
-
-                --argv;
-                place_string(&mbi.cmdline, w2s(&rest));
+                if ( options )
+                    *options = cmdline;
                 break;
             }
             else
@@ -569,7 +569,7 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_LOADED_IMAGE *loaded_image;
     EFI_STATUS status;
     unsigned int i, argc;
-    CHAR16 **argv, *file_name, *cfg_file_name = NULL;
+    CHAR16 **argv, *file_name, *cfg_file_name = NULL, *options = NULL;
     UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
     EFI_HANDLE *handles = NULL;
     EFI_SHIM_LOCK_PROTOCOL *shim_lock;
@@ -610,14 +610,14 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     dir_handle = get_parent_handle(loaded_image, &file_name);
 
     argc = get_argv(0, NULL, loaded_image->LoadOptions,
-                    loaded_image->LoadOptionsSize);
+                    loaded_image->LoadOptionsSize, &options);
     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);
+                 loaded_image->LoadOptionsSize, &options);
     else
         argc = 0;
     for ( i = 1; i < argc; ++i )
@@ -786,19 +786,6 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         efi_bs->FreePool(name.w);
     }
 
-    name.s = get_value(&cfg, section.s, "options");
-    if ( name.s )
-        place_string(&mbi.cmdline, name.s);
-    /* Insert image name last, as it gets prefixed to the other options. */
-    if ( argc )
-    {
-        name.w = *argv;
-        w2s(&name);
-    }
-    else
-        name.s = "xen";
-    place_string(&mbi.cmdline, name.s);
-
     cols = rows = depth = 0;
     if ( !base_video )
     {
@@ -817,6 +804,9 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         }
     }
 
+    name.s = get_value(&cfg, section.s, "options");
+    efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
+
     efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
     cfg.addr = 0;
 
@@ -863,15 +853,6 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         }
     }
 
-    if ( mbi.cmdline )
-        mbi.flags |= MBI_CMDLINE;
-    /*
-     * These must not be initialized statically, since the value must
-     * not get relocated when processing base relocations below.
-     */
-    mbi.boot_loader_name = (long)"EFI";
-    mbi.mods_addr = (long)mb_modules;
-
     /* Collect EDD info. */
     BUILD_BUG_ON(offsetof(struct edd_info, edd_device_params) != EDDEXTSIZE);
     BUILD_BUG_ON(sizeof(struct edd_device_params) != EDDPARMSIZE);
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 3937955..d72b02b 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -607,3 +607,37 @@  static void __init efi_arch_cfg_file(EFI_FILE_HANDLE dir_handle, char *section)
         efi_bs->FreePool(name.w);
     }
 }
+
+static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
+                                           CHAR16 *cmdline_options,
+                                           char *cfgfile_options)
+{
+    union string name;
+
+    if ( cmdline_options )
+    {
+        name.w = cmdline_options;
+        w2s(&name);
+        place_string(&mbi.cmdline, name.s);
+    }
+    if ( cfgfile_options )
+        place_string(&mbi.cmdline, cfgfile_options);
+    /* Insert image name last, as it gets prefixed to the other options. */
+    if ( image_name )
+    {
+        name.w = image_name;
+        w2s(&name);
+    }
+    else
+        name.s = "xen";
+    place_string(&mbi.cmdline, name.s);
+
+    if ( mbi.cmdline )
+        mbi.flags |= MBI_CMDLINE;
+    /*
+     * These must not be initialized statically, since the value must
+     * not get relocated when processing base relocations below.
+     */
+    mbi.boot_loader_name = (long)"EFI";
+    mbi.mods_addr = (long)mb_modules;
+}