diff mbox

[Xen-devel,V5,12/15] Add efi_arch_use_config_file() function to control use of config file

Message ID 1411080607-32365-13-git-send-email-roy.franz@linaro.org
State New
Headers show

Commit Message

Roy Franz Sept. 18, 2014, 10:50 p.m. UTC 
The x86 EFI build of Xen always uses a configuration file to load modules, but
the ARM version can either use a config file to specify the modules, or be
loaded by GRUB in which case GRUB loads the modules and adds them to the DTB
that is passed to Xen.  Add the efi_arch_use_config_file() to indicate if a
configuration file is required.  For x86, this will always be true.  ARM will
examine the DTB passed via EFI configuration table (if any), and if it contains
module information will use that that not use the configuration file at all.
Add Emacs footer to efi-boot.h and boot.c

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

Comments

Jan Beulich Sept. 22, 2014, 12:48 p.m. UTC | #1 
>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
> The x86 EFI build of Xen always uses a configuration file to load modules, 
> but
> the ARM version can either use a config file to specify the modules, or be
> loaded by GRUB in which case GRUB loads the modules and adds them to the DTB
> that is passed to Xen.  Add the efi_arch_use_config_file() to indicate if a
> configuration file is required.  For x86, this will always be true.  ARM 
> will
> examine the DTB passed via EFI configuration table (if any), and if it 
> contains
> module information will use that that not use the configuration file at all.
> Add Emacs footer to efi-boot.h and boot.c
> 
> Signed-off-by: Roy Franz <roy.franz@linaro.org>

Acked-by: Jan Beulich <jbeulich@suse.com>
with two formatting change requests:

> @@ -809,109 +805,119 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>      if ( EFI_ERROR(status) )
>          gop = NULL;
>  
> -    /* Read and parse the config file. */
> -    if ( !cfg_file_name.w )
> +    if ( efi_arch_use_config_file(SystemTable) )
>      {
> -        CHAR16 *tail;
> +        EFI_FILE_HANDLE dir_handle;
> +        /* Get the file system interface. */
> +        dir_handle = get_parent_handle(loaded_image, &file_name.w);

Blank line between declarations and statements please.

> --- a/xen/include/asm-x86/efi-boot.h
> +++ b/xen/include/asm-x86/efi-boot.h
> @@ -628,3 +628,17 @@ static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
>          blexit(L"Xen must be loaded at a 2Mb boundary.");
>      trampoline_xen_phys_start = xen_phys_start;
>  }
> +
> +static __init bool_t efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)

static <type> <attributes> <name>

please (i.e. __init and bool_t ought to switch places).
Roy Franz Sept. 23, 2014, 1:59 a.m. UTC | #2 
On Mon, Sep 22, 2014 at 5:48 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>> The x86 EFI build of Xen always uses a configuration file to load modules,
>> but
>> the ARM version can either use a config file to specify the modules, or be
>> loaded by GRUB in which case GRUB loads the modules and adds them to the DTB
>> that is passed to Xen.  Add the efi_arch_use_config_file() to indicate if a
>> configuration file is required.  For x86, this will always be true.  ARM
>> will
>> examine the DTB passed via EFI configuration table (if any), and if it
>> contains
>> module information will use that that not use the configuration file at all.
>> Add Emacs footer to efi-boot.h and boot.c
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with two formatting change requests:
>
>> @@ -809,109 +805,119 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>>      if ( EFI_ERROR(status) )
>>          gop = NULL;
>>
>> -    /* Read and parse the config file. */
>> -    if ( !cfg_file_name.w )
>> +    if ( efi_arch_use_config_file(SystemTable) )
>>      {
>> -        CHAR16 *tail;
>> +        EFI_FILE_HANDLE dir_handle;
>> +        /* Get the file system interface. */
>> +        dir_handle = get_parent_handle(loaded_image, &file_name.w);
>
> Blank line between declarations and statements please.
I reviewed all the functions - forgot to check all the other blocks....

>
>> --- a/xen/include/asm-x86/efi-boot.h
>> +++ b/xen/include/asm-x86/efi-boot.h
>> @@ -628,3 +628,17 @@ static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
>>          blexit(L"Xen must be loaded at a 2Mb boundary.");
>>      trampoline_xen_phys_start = xen_phys_start;
>>  }
>> +
>> +static __init bool_t efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
>
> static <type> <attributes> <name>
>
> please (i.e. __init and bool_t ought to switch places).
>
I'll fix this..

Thanks,
Roy
diff mbox

Patch

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index f4ef308..006a73c 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -684,7 +684,6 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_SHIM_LOCK_PROTOCOL *shim_lock;
     EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
     EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info;
-    EFI_FILE_HANDLE dir_handle;
     union string section = { NULL }, name, file_name, cfg_file_name = {NULL};
     bool_t base_video = 0;
     UINT32 mmap_desc_ver = 0;
@@ -711,9 +710,6 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
     efi_arch_load_addr_check(loaded_image);
 
-    /* Get the file system interface. */
-    dir_handle = get_parent_handle(loaded_image, &file_name.w);
-
     argc = get_argv(0, NULL, loaded_image->LoadOptions,
                     loaded_image->LoadOptionsSize, &options);
     if ( argc > 0 &&
@@ -809,109 +805,119 @@  efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     if ( EFI_ERROR(status) )
         gop = NULL;
 
-    /* Read and parse the config file. */
-    if ( !cfg_file_name.w )
+    if ( efi_arch_use_config_file(SystemTable) )
     {
-        CHAR16 *tail;
+        EFI_FILE_HANDLE dir_handle;
+        /* Get the file system interface. */
+        dir_handle = get_parent_handle(loaded_image, &file_name.w);
 
-        while ( (tail = point_tail(file_name.w)) != NULL )
+        /* Read and parse the config file. */
+        if ( !cfg_file_name.w )
         {
-            wstrcpy(tail, L".cfg");
-            if ( read_file(dir_handle, &cfg, file_name.w, NULL) )
-                break;
-            *tail = 0;
+            CHAR16 *tail;
+
+            while ( (tail = point_tail(file_name.w)) != NULL )
+            {
+                wstrcpy(tail, L".cfg");
+                if ( read_file(dir_handle, &cfg, file_name.w, NULL) )
+                    break;
+                *tail = 0;
+            }
+            if ( !tail )
+                blexit(L"No configuration file found.");
+            PrintStr(L"Using configuration file '");
+            PrintStr(file_name.w);
+            PrintStr(L"'\r\n");
         }
-        if ( !tail )
-            blexit(L"No configuration file found.");
-        PrintStr(L"Using configuration file '");
-        PrintStr(file_name.w);
-        PrintStr(L"'\r\n");
-    }
-    else if ( !read_file(dir_handle, &cfg, cfg_file_name.w, NULL) )
-        blexit(L"Configuration file not found.");
-    pre_parse(&cfg);
+        else if ( !read_file(dir_handle, &cfg, cfg_file_name.w, NULL) )
+            blexit(L"Configuration file not found.");
+        pre_parse(&cfg);
 
-    if ( section.w )
-        w2s(&section);
-    else
-        section.s = get_value(&cfg, "global", "default");
+        if ( section.w )
+            w2s(&section);
+        else
+            section.s = get_value(&cfg, "global", "default");
 
-    for ( ; ; )
-    {
-        name.s = get_value(&cfg, section.s, "kernel");
-        if ( name.s )
-            break;
-        name.s = get_value(&cfg, "global", "chain");
-        if ( !name.s )
-            break;
-        efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
-        cfg.addr = 0;
-        if ( !read_file(dir_handle, &cfg, s2w(&name), NULL) )
+        for ( ; ; )
         {
-            PrintStr(L"Chained configuration file '");
-            PrintStr(name.w);
+            name.s = get_value(&cfg, section.s, "kernel");
+            if ( name.s )
+                break;
+            name.s = get_value(&cfg, "global", "chain");
+            if ( !name.s )
+                break;
+            efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
+            cfg.addr = 0;
+            if ( !read_file(dir_handle, &cfg, s2w(&name), NULL) )
+            {
+                PrintStr(L"Chained configuration file '");
+                PrintStr(name.w);
+                efi_bs->FreePool(name.w);
+                blexit(L"'not found.");
+            }
+            pre_parse(&cfg);
             efi_bs->FreePool(name.w);
-            blexit(L"'not found.");
         }
-        pre_parse(&cfg);
-        efi_bs->FreePool(name.w);
-    }
-    if ( !name.s )
-        blexit(L"No Dom0 kernel image specified.");
-
-    efi_arch_cfg_file_early(dir_handle, section.s);
 
-    option_str = split_string(name.s);
-    read_file(dir_handle, &kernel, s2w(&name), option_str);
-    efi_bs->FreePool(name.w);
+        if ( !name.s )
+            blexit(L"No Dom0 kernel image specified.");
 
-    if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
-                    (void **)&shim_lock)) &&
-         (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
-        PrintErrMesg(L"Dom0 kernel image could not be verified", status);
+        efi_arch_cfg_file_early(dir_handle, section.s);
 
-    name.s = get_value(&cfg, section.s, "ramdisk");
-    if ( name.s )
-    {
         option_str = split_string(name.s);
-        read_file(dir_handle, &ramdisk, s2w(&name), option_str);
+        read_file(dir_handle, &kernel, s2w(&name), option_str);
         efi_bs->FreePool(name.w);
-    }
 
-    name.s = get_value(&cfg, section.s, "xsm");
-    if ( name.s )
-    {
-        option_str = split_string(name.s);
-        read_file(dir_handle, &xsm, s2w(&name), option_str);
-        efi_bs->FreePool(name.w);
-    }
+        if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
+                        (void **)&shim_lock)) &&
+             (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
+            PrintErrMesg(L"Dom0 kernel image could not be verified", status);
 
-    name.s = get_value(&cfg, section.s, "options");
-    efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
+        name.s = get_value(&cfg, section.s, "ramdisk");
+        if ( name.s )
+        {
+            option_str = split_string(name.s);
+            read_file(dir_handle, &ramdisk, s2w(&name), option_str);
+            efi_bs->FreePool(name.w);
+        }
 
-    cols = rows = depth = 0;
-    if ( !base_video )
-    {
-        name.cs = get_value(&cfg, section.s, "video");
-        if ( !name.cs )
-            name.cs = get_value(&cfg, "global", "video");
-        if ( name.cs && !strncmp(name.cs, "gfx-", 4) )
+        name.s = get_value(&cfg, section.s, "xsm");
+        if ( name.s )
         {
-            cols = simple_strtoul(name.cs + 4, &name.cs, 10);
-            if ( *name.cs == 'x' )
-                rows = simple_strtoul(name.cs + 1, &name.cs, 10);
-            if ( *name.cs == 'x' )
-                depth = simple_strtoul(name.cs + 1, &name.cs, 10);
-            if ( *name.cs )
-                cols = rows = depth = 0;
+            option_str = split_string(name.s);
+            read_file(dir_handle, &xsm, s2w(&name), option_str);
+            efi_bs->FreePool(name.w);
         }
-    }
-    efi_arch_cfg_file_late(dir_handle, section.s);
 
-    efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
-    cfg.addr = 0;
+        name.s = get_value(&cfg, section.s, "options");
+        efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
+
+        cols = rows = depth = 0;
+        if ( !base_video )
+        {
+            name.cs = get_value(&cfg, section.s, "video");
+            if ( !name.cs )
+                name.cs = get_value(&cfg, "global", "video");
+            if ( name.cs && !strncmp(name.cs, "gfx-", 4) )
+            {
+                cols = simple_strtoul(name.cs + 4, &name.cs, 10);
+                if ( *name.cs == 'x' )
+                    rows = simple_strtoul(name.cs + 1, &name.cs, 10);
+                if ( *name.cs == 'x' )
+                    depth = simple_strtoul(name.cs + 1, &name.cs, 10);
+                if ( *name.cs )
+                    cols = rows = depth = 0;
+            }
+        }
 
-    dir_handle->Close(dir_handle);
+        efi_arch_cfg_file_late(dir_handle, section.s);
+
+        efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
+        cfg.addr = 0;
+
+        dir_handle->Close(dir_handle);
+
+    }
 
     if ( gop && !base_video )
     {
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 2a3a2b6..3ff593d 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -628,3 +628,17 @@  static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
         blexit(L"Xen must be loaded at a 2Mb boundary.");
     trampoline_xen_phys_start = xen_phys_start;
 }
+
+static __init bool_t efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
+{
+    return 1; /* x86 always uses a config file */
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */