Re: [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index

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

Hi Ian,

On 26/06/14 17:45, Ian Campbell wrote:
> This is more natural and better matches how multiboot is actually supposed to
> work.

I don't find any modification of consider_modules in this series. Is it 
normal? This function is badly assuming that MOD_XEN will be always the 
first one. So may end up to forget to exclude some modules during the 
memory allocation.

[..]

> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5eef8a3..c1a54e5 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -161,9 +161,10 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
>       int res = 0;
>       int had_dom0_bootargs = 0;
>
> -    if ( bootinfo.modules.nr_mods >= MOD_KERNEL &&
> -         bootinfo.modules.module[MOD_KERNEL].cmdline[0] )
> -        bootargs = &bootinfo.modules.module[MOD_KERNEL].cmdline[0];
> +    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);

It looks like a bit pointless to search the BOOTMOD_KERNEL everytime we 
try to write properties of a node. But it's not the purpose of this patch.

[..]

>   struct bootmodules {
>       int nr_mods;
>       /* Module 0 is Xen itself, followed by the provided modules-proper */

Does this comment still relevant?

Regards,

-- 
Julien Grall