[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC 08/15] xen/arm: probe domU kernels and initrds



On Thu, 14 Jun 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 13/06/18 23:15, Stefano Stabellini wrote:
> > Find addresses and sizes on device tree.
> > Introduce a new boot_module_find_by_addr_and_kind function to match not
> > just on boot module kind, but also by address so that we can support
> > multiple domUs.
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > ---
> >   xen/arch/arm/kernel.c       | 54
> > +++++++++++++++++++++++++++++++++++++++++++++
> >   xen/arch/arm/kernel.h       |  2 ++
> >   xen/arch/arm/setup.c        | 15 +++++++++++++
> >   xen/include/asm-arm/setup.h |  2 ++
> >   4 files changed, 73 insertions(+)
> > 
> > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > index 8fdfd91..c41092e 100644
> > --- a/xen/arch/arm/kernel.c
> > +++ b/xen/arch/arm/kernel.c
> > @@ -496,6 +496,60 @@ err:
> >       return rc;
> >   }
> >   +int kernel_probe_domU(struct kernel_info *info, struct dt_device_node
> > *domU)
> > +{
> > +    struct bootmodule *mod;
> > +    struct dt_device_node *node;
> > +    u64 kernel_addr, initrd_addr, size;
> > +    int rc;
> > +
> > +    dt_for_each_child_node(domU, node)
> > +    {
> > +        if ( dt_device_is_compatible(node, "multiboot,domU-kernel") )
> > +        {
> > +            u32 len;
> > +            const __be32 *val;
> > +            val = dt_get_property(node, "reg", &len);
> > +            dt_get_range(&val, node, &kernel_addr, &size);
> > +        }
> > +        else if ( dt_device_is_compatible(node, "multiboot,domU-ramdisk") )
> > +        {
> > +            u32 len;
> > +            const __be32 *val;
> > +            val = dt_get_property(node, "reg", &len);
> > +            dt_get_range(&val, node, &initrd_addr, &size);
> > +        }
> > +        else
> > +            continue;
> > +    }
> > +    info->kernel_bootmodule = mod = boot_module_find_by_addr_and_kind(
> > +
> > BOOTMOD_DOMU_KERNEL, kernel_addr);
> 
> This line contains hard tab.

This patch was screwed for some reason. I fixed it.


> But I don't think this will work as you expect. Imagine the kernel is the same
> for each guest. It would be fine to have the Image loaded once in memory and
> therefore specify the same physical address for all domU-kernel compatible
> node. However, the command line may be different.
> 
> So you would end up to use the wrong module here.

Yes, you are right. In order to fix the issue, I had to rework the way
boot_modules are done. In the next series, it will be possible to use
the same kernel address in multiple modules, and the right cmdline will
get retrieved.


> > +    info->initrd_bootmodule = boot_module_find_by_addr_and_kind(
> > +
> > BOOTMOD_DOMU_RAMDISK, initrd_addr);
> 
> Same here.
> 
> > +    printk("Loading DomU kernel from boot module @ %"PRIpaddr"\n",
> > +              info->kernel_bootmodule->start);
> 
> The indentation is wrong here.
> 
> > +    if ( info->initrd_bootmodule )
> > +        printk("Loading ramdisk from boot module @ %"PRIpaddr"\n",
> > +               info->initrd_bootmodule->start);
> > +
> > +    /* if it is a gzip'ed image, 32bit or 64bit, uncompress it */
> > +    rc = kernel_decompress(mod);
> > +    if (rc < 0 && rc != -EINVAL)
> > +        return rc;
> > +
> > +#ifdef CONFIG_ARM_64
> > +    rc = kernel_zimage64_probe(info, mod->start, mod->size);
> > +    if (rc < 0)
> > +#endif
> > +        rc = kernel_uimage_probe(info, mod->start, mod->size);
> > +    if (rc < 0)
> > +        rc = kernel_zimage32_probe(info, mod->start, mod->size);
> > +    if (rc < 0)
> > +        rc = kernel_elf_probe(info, mod->start, mod->size);
> 
> Most of this code is the same as kernel_probe. How about reworking
> kernel_probe to handle any domain?

Yes, I can make it common.


> > +
> > +    return rc;
> > +}
> > +
> >   int kernel_probe(struct kernel_info *info)
> >   {
> >       struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
> > diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> > index 6d69509..8e1614b 100644
> > --- a/xen/arch/arm/kernel.h
> > +++ b/xen/arch/arm/kernel.h
> > @@ -64,6 +64,8 @@ struct kernel_info {
> >    */
> >   int kernel_probe(struct kernel_info *info);
> >   +int kernel_probe_domU(struct kernel_info *info, struct dt_device_node
> > *node);
> > +
> >   /*
> >    * Loads the kernel into guest RAM.
> >    *
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 82593c8..98bdb24 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -241,6 +241,21 @@ struct bootmodule * __init
> > boot_module_find_by_kind(bootmodule_kind kind)
> >       return NULL;
> >   }
> >   +struct bootmodule * __init
> > boot_module_find_by_addr_and_kind(bootmodule_kind kind,
> > +                                                             paddr_t start)
> > +{
> > +    struct bootmodules *mods = &bootinfo.modules;
> > +    struct bootmodule *mod;
> > +    int i;
> > +    for (i = 0 ; i < mods->nr_mods ; i++ )
> > +    {
> > +        mod = &mods->module[i];
> > +        if ( mod->kind == kind && mod->start == start )
> > +            return mod;
> > +    }
> > +    return NULL;
> > +}
> > +
> >   const char * __init boot_module_kind_as_string(bootmodule_kind kind)
> >   {
> >       switch ( kind )
> > diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> > index 86aac0e..903782f 100644
> > --- a/xen/include/asm-arm/setup.h
> > +++ b/xen/include/asm-arm/setup.h
> > @@ -83,6 +83,8 @@ struct bootmodule *add_boot_module(bootmodule_kind kind,
> >                                      paddr_t start, paddr_t size,
> >                                      const char *cmdline);
> >   struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
> > +struct bootmodule * __init
> > boot_module_find_by_addr_and_kind(bootmodule_kind kind,
> > +                                                             paddr_t
> > start);
> >   const char * __init boot_module_kind_as_string(bootmodule_kind kind);
> >     #endif
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.