Re: [Xen-devel] [PATCH v5 06/19] xen: arm: move dom0 gic and timer device tree nodes under /xen/

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 13 Nov 2013, Ian Campbell wrote:
> Julien observed that we were relying on the provided host DTB supplying
> suitable #address-cells and #size-cells values to allow us to represent these
> addresses, which may not reliably be the case. Moving these under our own
> known (somewhat analogous to the use of /soc/ or /motherboard/ on some
> platforms) allows us to control these sizes.
> 
> Since the xen node is created out of thin air it does not have a corresponding
> struct dt_device_node and therefore we cannot use dt_n_addr_cells or
> dt_n_size_cells, we can use hardcoded constants instead. For the same reason
> we define and use set_xen_range instead of dt_set_range.
> 
> The hypervisor, cpus and psci node all either defined #foo-cells for their
> children or do not contain reg properties and therefore can remain at the top
> level.
> 
> The logging in make_gic_node was inconsistent. Fix it.

I think that using the string "xen" as node name is confusing.
I would use something more informative, as "xen-motherboard" or
"xen-virtual-devices", so that people can guess how it differs from
the hypervisor node.



> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
> v5: New patch
> ---
>  xen/arch/arm/domain_build.c |   76 
> ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 61 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 186746c..8645aa1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -48,6 +48,24 @@ custom_param("dom0_mem", parse_dom0_mem);
>   */
>  #define DOM0_FDT_EXTRA_SIZE (128 + sizeof(struct fdt_reserve_entry))
>  
> +/*
> + * Number of cells used for addresses and sizes under the /xen/
> + * node.
> + *
> + * We don't have a struct dt_device_node we can reference as a parent
> + * to get these values, so use these constants instead.
> + */
> +#define XEN_FDT_NODE_ADDRESS_CELLS 2
> +#define XEN_FDT_NODE_SIZE_CELLS 2
> +#define XEN_FDT_NODE_REG_SIZE \
> +    dt_cells_to_size(XEN_FDT_NODE_ADDRESS_CELLS + XEN_FDT_NODE_SIZE_CELLS)
> +
> +static void set_xen_range(__be32 **cellp, u64 address, u64 size)
> +{
> +    dt_set_cell(cellp, XEN_FDT_NODE_ADDRESS_CELLS, address);
> +    dt_set_cell(cellp, XEN_FDT_NODE_SIZE_CELLS, size);
> +}
> +
>  struct vcpu *__init alloc_dom0_vcpu0(void)
>  {
>      if ( opt_dom0_max_vcpus == 0 )
> @@ -477,8 +495,7 @@ static int make_cpus_node(const struct domain *d, void 
> *fdt,
>      return res;
>  }
>  
> -static int make_gic_node(const struct domain *d, void *fdt,
> -                         const struct dt_device_node *parent)
> +static int make_gic_node(const struct domain *d, void *fdt)
>  {
>      const struct dt_device_node *gic = dt_interrupt_controller;
>      const void *compatible = NULL;
> @@ -512,20 +529,19 @@ static int make_gic_node(const struct domain *d, void 
> *fdt,
>      if ( res )
>          return res;
>  
> -    len = dt_cells_to_size(dt_n_addr_cells(parent) + 
> dt_n_size_cells(parent));
> -    len *= 2;
> -    new_cells = xzalloc_bytes(dt_cells_to_size(len));
> +    len = XEN_FDT_NODE_REG_SIZE * 2;
> +    new_cells = xzalloc_bytes(len);
>      if ( new_cells == NULL )
>          return -FDT_ERR_XEN(ENOMEM);
>  
>      tmp = new_cells;
>      DPRINT("  Set Distributor Base 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
>             d->arch.vgic.dbase, d->arch.vgic.dbase + PAGE_SIZE - 1);
> -    dt_set_range(&tmp, parent, d->arch.vgic.dbase, PAGE_SIZE);
> +    set_xen_range(&tmp, d->arch.vgic.dbase, PAGE_SIZE);
>  
> -    DPRINT("  Set Cpu Base 0x%"PRIpaddr" size = 0x%"PRIpaddr"\n",
> +    DPRINT("  Set Cpu Base 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
>             d->arch.vgic.cbase, d->arch.vgic.cbase + (PAGE_SIZE * 2) - 1);
> -    dt_set_range(&tmp, parent, d->arch.vgic.cbase, PAGE_SIZE * 2);
> +    set_xen_range(&tmp, d->arch.vgic.cbase, PAGE_SIZE * 2);
>  
>      res = fdt_property(fdt, "reg", new_cells, len);
>      xfree(new_cells);
> @@ -550,8 +566,7 @@ static int make_gic_node(const struct domain *d, void 
> *fdt,
>      return res;
>  }
>  
> -static int make_timer_node(const struct domain *d, void *fdt,
> -                           const struct dt_device_node *parent)
> +static int make_timer_node(const struct domain *d, void *fdt)
>  {
>      static const struct dt_device_match timer_ids[] __initconst =
>      {
> @@ -611,6 +626,41 @@ static int make_timer_node(const struct domain *d, void 
> *fdt,
>      return res;
>  }
>  
> +static int make_xen_node(const struct domain *d, void *fdt,
> +                         const struct dt_device_node *parent)
> +{
> +    int res;
> +
> +    res = fdt_begin_node(fdt, "xen");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#address-cells",
> +                            XEN_FDT_NODE_ADDRESS_CELLS);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#size-cells",
> +                            XEN_FDT_NODE_SIZE_CELLS);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "ranges", NULL, 0);
> +    if ( res )
> +        return res;
> +
> +    res = make_gic_node(d, fdt);
> +    if ( res )
> +        return res;
> +
> +    res = make_timer_node(d, fdt);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_end_node(fdt);
> +    return res;
> +}
> +
>  /* Map the device in the domain */
>  static int map_device(struct domain *d, const struct dt_device_node *dev)
>  {
> @@ -776,11 +826,7 @@ static int handle_node(struct domain *d, struct 
> kernel_info *kinfo,
>          if ( res )
>              return res;
>  
> -        res = make_gic_node(d, kinfo->fdt, np);
> -        if ( res )
> -            return res;
> -
> -        res = make_timer_node(d, kinfo->fdt, np);
> +        res = make_xen_node(d, kinfo->fdt, np);
>          if ( res )
>              return res;
>      }
> -- 
> 1.7.10.4
> 

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