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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 1 Aug 2018, Julien Grall wrote:
> 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.

OK


> > 
> > 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.

I'll add a flag.


> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > 
> > ---
> > 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.

OK


> >         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;

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