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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-06-27 at 11:43 +0100, Julien Grall wrote:
> 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.

Cr*p, yes, I suspect you are right. I'll check.

In an early (unpublished) version of the patch I jumped through some
hoops to make sure that module 0 was always Xen. Looks like I shouldn't
have been so eager to clean that up!

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

Yeah, I should wrap it in a "dt_node_path_is_equal(node,
"/chosen")" (and I'll make the two existing ones into a single bool_t
foo = too while I'm there.

> [..]
> 
> >   struct bootmodules {
> >       int nr_mods;
> >       /* Module 0 is Xen itself, followed by the provided modules-proper */
> 
> Does this comment still relevant?

I suspect not.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel