Re: [PATCH v3 10/25] xen/arm: don't add duplicate boot modules

[Julien Grall <julien.grall@arm.com>]

Hi Stefano,

On 01/08/18 00:27, Stefano Stabellini wrote:
> Don't add duplicate boot modules (same kind and same start address).

Please explain why you don't want to duplicate it.

> 
> Mark kernels and ramdisks of "xen,domain" nodes as BOOTMOD_KERNEL_DOMAIN
> and BOOTMOD_RAMDISK_DOMAIN respectively, to avoid getting confused in
> kernel_probe, where we try to guess which is the dom0 kernel and initrd
> to be compatible with older versions of the multiboot spec.

In patch #9, you wrote we don't want duplication because of the free 
function. But it would be possible to have all the domains (including 
Dom0) to use the same kernel.

It would be better to find a different way to find Dom0 information. One 
way would be to add a flag in the boot module.

> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> 
> ---
> Changes in v2:
> - new patch
> ---
>   xen/arch/arm/bootfdt.c      | 7 +++++++
>   xen/arch/arm/setup.c        | 9 +++++++++
>   xen/include/asm-arm/setup.h | 2 ++
>   3 files changed, 18 insertions(+)
> 
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 6f44022..dbaa8f4 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -256,6 +256,13 @@ static void __init process_multiboot_node(const void *fdt, int node,
>               kind = BOOTMOD_XSM;
>       }
>   
> +    if ( fdt_node_check_compatible(fdt, parent_node, "xen,domain") == 0 )
> +    {
> +        if ( kind == BOOTMOD_KERNEL )
> +            kind = BOOTMOD_KERNEL_DOMAIN;
> +        if ( kind == BOOTMOD_RAMDISK )
> +            kind = BOOTMOD_RAMDISK_DOMAIN;
> +    }
>       add_boot_module(kind, start, size);
>       add_boot_cmdline(fdt, node, fdt_get_name(fdt, parent_node, &len), kind);
>   }
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 67ab1fd..b1a117f 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -204,6 +204,7 @@ struct bootmodule __init *add_boot_module(bootmodule_kind kind,
>   {
>       struct bootmodules *mods = &bootinfo.modules;
>       struct bootmodule *mod;
> +    int i;

unsigned please.

>   
>       if ( mods->nr_mods == MAX_MODULES )
>       {
> @@ -211,6 +212,12 @@ struct bootmodule __init *add_boot_module(bootmodule_kind kind,
>                  boot_module_kind_as_string(kind), start, start + size);
>           return NULL;
>       }
> +    for ( i = 0 ; i < mods->nr_mods ; i++ )
> +    {
> +        mod = &mods->module[i];
> +        if ( mod->kind == kind && mod->start == start )
> +            return mod;
> +    }
>   
>       mod = &mods->module[mods->nr_mods++];
>       mod->kind = kind;
> @@ -257,6 +264,8 @@ const char * __init boot_module_kind_as_string(bootmodule_kind kind)
>       case BOOTMOD_KERNEL:  return "Kernel";
>       case BOOTMOD_RAMDISK: return "Ramdisk";
>       case BOOTMOD_XSM:     return "XSM";
> +    case BOOTMOD_KERNEL_DOMAIN:  return "DomU Kernel";
> +    case BOOTMOD_RAMDISK_DOMAIN: return "DomU Ramdisk";
>       case BOOTMOD_UNKNOWN: return "Unknown";
>       default: BUG();
>       }
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index cb7da51..353c32a 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -16,6 +16,8 @@ typedef enum {
>       BOOTMOD_KERNEL,
>       BOOTMOD_RAMDISK,
>       BOOTMOD_XSM,
> +    BOOTMOD_KERNEL_DOMAIN,
> +    BOOTMOD_RAMDISK_DOMAIN,
>       BOOTMOD_UNKNOWN
>   }  bootmodule_kind;
>   
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel