From mboxrd@z Thu Jan  1 00:00:00 1970
From: Roy Franz <roy.franz@linaro.org>
Subject: Re: [PATCH V5 12/15] Add efi_arch_use_config_file()
 function to control use of config file
Date: Mon, 22 Sep 2014 18:59:41 -0700
Message-ID: <CAFECyb-9mWGm4ACOj-VOSULShim5Rfd93vcwR9_7rpnrwFCp2A@mail.gmail.com>
References: <1411080607-32365-1-git-send-email-roy.franz@linaro.org>
	<1411080607-32365-13-git-send-email-roy.franz@linaro.org>
	<542036B70200007800036EF2@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <542036B70200007800036EF2@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: keir <keir@xen.org>, Ian Campbell <ian.campbell@citrix.com>, tim <tim@xen.org>, xen-devel <xen-devel@lists.xen.org>, Stefano Stabellini <stefano.stabellini@citrix.com>, Fu Wei <fu.wei@linaro.org>
List-Id: xen-devel@lists.xenproject.org

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