Re: [PATCH v4] xen: arm: Only lookup kernel/initrd bootmodule once while building dom0.

[Julien Grall <julien.grall@linaro.org>]

On 07/21/2014 01:09 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v4: Call the new field kernel_bootmodule for clarity
>     Use the new field throughout kernel.c as well as in domain_build.c
>     Add and use initrd_bootmodule too.
>     Const up the uses
> v3: New patch
> ---
>  xen/arch/arm/domain_build.c |    6 +++---
>  xen/arch/arm/kernel.c       |    2 ++
>  xen/arch/arm/kernel.h       |    1 +
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 154367e..23261e4 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -405,7 +405,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
>      int res = 0;
>      int had_dom0_bootargs = 0;
>  
> -    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
> +    const struct bootmodule *mod = kinfo->kernel_bootmodule;
>  
>      if ( mod && mod->cmdline[0] )
>          bootargs = &mod->cmdline[0];
> @@ -455,7 +455,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
>  
>      if ( dt_node_path_is_equal(node, "/chosen") )
>      {
> -        struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_RAMDISK);
> +        const struct bootmodule *mod = kinfo->initrd_bootmodule;

Technically speaking, we only execute once this part of the code. So
having a new field in kernel_info doesn't seem useful.

BTW, the function already have a variable "mod" defined at the begining.
I would rename this variable to modinitrd or smth different to shadow
the previous variable.

Regards,

-- 
Julien Grall