Re: [Xen-devel] [PATCH v3 03/14] xen: arm: allocate dom0 memory separately from preparing the dtb

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-11-07 at 23:18 -0800, Julien Grall wrote:
> 
> On 11/07/2013 08:44 AM, Ian Campbell wrote:
> > Mixing these two together is a pain, it forces us to prepare the dtb before
> > processing the kernel which means we don't know whether the guest is 32- or
> > 64-bit while we construct its DTB.
> >
> > Instead split out the memory allocation (including 1:1 workaround handling)
> > and p2m setup into a separate phase and then create a memory node in the DTB
> > based on the result.
> 
> Your solution to create the memory node won't work in some case. From 
> the EPAR, memory nodes can be everywhere. So we can have a device tree 
> like that:
> 
> / {
>    motherboard
>    {
>       #address-cells = 2
>       #size-cells = 2
>       memory {
>          device_type = "memory";
>          reg = < ... >
>       }
>    }
> }
> 
> Here, the root (/) has #address-cells = 2 and #size-cells = 1, that is 
> the default value. As you will create the memory node in slash, you will 
> loose 1 cell of the size.

Urk yes.

I think
        / {
                "memory" {
                        #address-cells = 2;
                        #size-cells = 2;
                        device_type = "memory";
                        reg = <...>;
                }
        }

Won't work because the #foo-cells only applies to children.

I could do
        / {
                "memory" {
                        #address-cells = 2;
                        #size-cells = 2;
                        "memory@foo" {
                                device_type = "memory"
                                reg = <...>;
                        }
                }
        }

which puts the size under my control.

Or I could just remember the root sizes and hope the ram addresses fit,
but that would be fragile (so not an option IMHO).

Or I could leave a placeholder in the original location which I try to
find again later and fill in, but that would suck in practice I think.

> > This allows us to move kernel parsing before DTB setup.
> 
> Why do you want to move the kernel parsing earlier? Xen don't use 
> d->arch.type during dom0 building.

In this series prepare_dtb needs to know which kind of guest it is,
which requires us to have parsed the kernel.

I also think it is an independently worthwhile change to separate
prepping the dtb from the memory allocation, since it makes things
cleaner overall (i.e. we can drop the overlap_check() which is quite a
hacky way to do things which came about because of the intertwining of
the dtb and memory alloc.

> >   static void kernel_elf_load(struct kernel_info *info)
> >   {
> > +    place_modules(info,
> > +                  info->elf.parms.virt_kstart,
> > +                  info->elf.parms.virt_kend);
> > +
> >       printk("Loading ELF image into guest memory\n");
> >       info->elf.elf.dest_base = (void*)(unsigned 
> > long)info->elf.parms.virt_kstart;
> >       info->elf.elf.dest_size =
> >            info->elf.parms.virt_kend - info->elf.parms.virt_kstart;
> > +
> 
> spurious line?

Just aesthetics I think. Could drop it I guess.

Ian.


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