Re: [PATCH 2/5] device tree, arm: supply a flat device tree to dom0

[David Vrabel <david.vrabel@citrix.com>]

On 02/04/12 10:30, Ian Campbell wrote:
> On Mon, 2012-04-02 at 10:21 +0100, David Vrabel wrote:
>> On 30/03/12 16:59, Ian Campbell wrote:
>>> On Thu, 2012-03-22 at 19:17 +0000, David Vrabel wrote:
>>>> From: David Vrabel <david.vrabel@citrix.com>
>>>>
>>>> Build a flat device tree for dom0 based on the one supplied to Xen.
>>>> The following changes are made:
>>>>
>>>>   * In the /chosen node, the xen,dom0-bootargs parameter is renamed to
>>>>     bootargs.
>>>>
>>>>   * In all memory nodes, the reg parameters are adjusted to reflect
>>>>     the amount of memory dom0 can use.  The p2m is updated using this
>>>>     info.
>>>>
>>>> Support for passing ATAGS to dom0 is removed.
>>>>
>>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>>> ---
>>>>  xen/arch/arm/domain_build.c         |  257 +++++++++++++++++++++++++++++------
>>>>  xen/arch/arm/kernel.c               |    2 +-
>>>>  xen/arch/arm/kernel.h               |    8 +-
>>>>  xen/common/device_tree.c            |   47 ++++--
>>>>  xen/include/xen/device_tree.h       |    8 +
>>>>  xen/include/xen/libfdt/libfdt_env.h |    3 +
>>>>  6 files changed, 265 insertions(+), 60 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>> index 15632f7..b4c0452 100644
>>>> --- a/xen/arch/arm/domain_build.c
>>>> +++ b/xen/arch/arm/domain_build.c
>>>> @@ -6,6 +6,10 @@
>>>>  #include <xen/sched.h>
>>>>  #include <asm/irq.h>
>>>>  #include <asm/regs.h>
>>>> +#include <xen/errno.h>
>>>> +#include <xen/device_tree.h>
>>>> +#include <xen/libfdt/libfdt.h>
>>>> +#include <xen/guest_access.h>
>>>>
>>>>  #include "gic.h"
>>>>  #include "kernel.h"
>>>> @@ -13,6 +17,13 @@
>>>>  static unsigned int __initdata opt_dom0_max_vcpus;
>>>>  integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
>>>>
>>>> +/*
>>>> + * Amount of extra space required to dom0's device tree.  No new nodes
>>>> + * are added (yet) but one terminating reserve map entry (16 bytes) is
>>>> + * added.
>>>> + */
>>>> +#define DOM0_FDT_EXTRA_SIZE (sizeof(struct fdt_reserve_entry))
>>>> +
>>>>  struct vcpu *__init alloc_dom0_vcpu0(void)
>>>>  {
>>>>      if ( opt_dom0_max_vcpus == 0 )
>>>> @@ -28,43 +39,210 @@ struct vcpu *__init alloc_dom0_vcpu0(void)
>>>>      return alloc_vcpu(dom0, 0, 0);
>>>>  }
>>>>
>>>> -extern void guest_mode_entry(void);
>>>> +static void set_memory_reg(struct domain *d, struct kernel_info *kinfo,
>>>> +                           const void *fdt,
>>>> +                           const u32 *cell, int address_cells, int size_cells,
>>>> +                           u32 *new_cell, int *len)
>>>> +{
>>>> +    int reg_size = (address_cells + size_cells) * sizeof(*cell);
>>>> +    int l;
>>>> +    u64 start;
>>>> +    u64 size;
>>>> +
>>>> +    l = *len;
>>>
>>>> +
>>>> +    while ( kinfo->unassigned_mem > 0 && l >= reg_size
>>>> +            && kinfo->mem.nr_banks < NR_MEM_BANKS )
>>>> +    {
>>>> +        device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
>>>> +        if ( size > kinfo->unassigned_mem )
>>>> +            size = kinfo->unassigned_mem;
>>>> +
>>>> +        device_tree_set_reg(&new_cell, address_cells, size_cells, start, size);
>>>
>>> This assumes/requires that address_cells and size_cells a 1, right? If
>>> they end up being 2 or more then device_tree_get_reg will trample on the
>>> stack via &start and &size.
>>
>> No.  address_cells and size_cells can be 1 or 2.
> 
> Where is that checked or enforced? What happens if someone feeds us a
> DTB with 3? Or are you saying that this isn't possible (perhaps due to
> some constraint in DTB itself)?

I just tested with #address-cells == 3 and it works fine.

We trust the DTB is a valid description of the hardware so assume that
all addresses are below 2^64.

David