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

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 11/08/2013 09:36 AM, Ian Campbell wrote:
> On Thu, 2013-11-07 at 23:18 -0800, Julien Grall wrote:
> 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.

I have noticed that we have the same issue on the other nodes (gic,
timer,...). Perhaps we can create a node "xen" which contains all the
devices?

"xen" {
   #address-cells = 2;
   #size-cells = 2;
   memory {
     ...
   }
   timer {
     ...
   }
}

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

Thanks, I didn't pay attention that patch #4 uses the kernel type via
is_pv64_domain.

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

Ok.


-- 
Julien Grall

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