Re: [RFC PATCH v3 17/18] xen/arm: ITS: Generate ITS node for Dom0

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-06-22 at 17:31 +0530, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Parse host dt and generate ITS node for Dom0.
> ITS node resides inside GIC node so when GIC node
> is encountered look for ITS node.
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
>  xen/arch/arm/domain_build.c   |   50 +++++++++++++++++++++++++++++++++++-
>  xen/arch/arm/gic-v3-its.c     |   57 +++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/gic-its.h |    2 ++
>  3 files changed, 108 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e9cb8a9..0de5a8b 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -20,6 +20,7 @@
>  #include <asm/cpufeature.h>
>  
>  #include <asm/gic.h>
> +#include <asm/gic-its.h>
>  #include <xen/irq.h>
>  #include "kernel.h"
>  
> @@ -803,6 +804,34 @@ static int make_cpus_node(const struct domain *d, void *fdt,
>      return res;
>  }
>  
> +static int make_its_node(const struct domain *d, void *fdt,
> +                         const struct dt_device_node *node)
> +{
> +    int res = 0;
> +
> +    DPRINT("Create GIC ITS node\n");
> +
> +    res = its_make_dt_node(d, node, fdt);
> +    if ( res )
> +        return res;
> +
> +    /*
> +     * The value of the property "phandle" in the property "interrupts"
> +     * to know on which interrupt controller the interrupt is wired.
> +     */
> +    if ( node->phandle )
> +    {
> +        DPRINT("  Set phandle = 0x%x\n", node->phandle);
> +        res = fdt_property_cell(fdt, "phandle", node->phandle);
> +        if ( res )
> +            return res;
> +    }
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}
> +
>  static int make_gic_node(const struct domain *d, void *fdt,
>                           const struct dt_device_node *node)
>  {
> @@ -1119,7 +1148,13 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>          DT_MATCH_TIMER,
>          { /* sentinel */ },
>      };
> +    static const struct dt_device_match gits_matches[] __initconst =
> +    {
> +        DT_MATCH_GIC_ITS,
> +        { /* sentinel */ },
> +    };
>      struct dt_device_node *child;
> +    struct dt_device_node *gic_child;
>      int res;
>      const char *name;
>      const char *path;
> @@ -1143,7 +1178,20 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>      /* Replace these nodes with our own. Note that the original may be
>       * used_by DOMID_XEN so this check comes first. */
>      if ( device_get_class(node) == DEVICE_GIC )
> -        return make_gic_node(d, kinfo->fdt, node);
> +    {
> +        if ( !make_gic_node(d, kinfo->fdt, node) )
> +        {
> +            dt_for_each_child_node(node, gic_child)
> +            {
> +                if ( gic_child != NULL )
> +                {
> +                    if ( dt_match_node(gits_matches, gic_child) )
> +                        return make_its_node(d, kinfo->fdt, gic_child);

This will create multiple ITS nodes, we only want one and we need the
msi-parent properties everywhere else to be changed to point to it.


> +                }
> +            }
> +        }
> +        return 0;
> +    }
>      if ( dt_match_node(timer_matches, node) )
>          return make_timer_node(d, kinfo->fdt, node);
>  
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 8aa1ec5..fc853d4 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -27,6 +27,8 @@
>  #include <xen/sched.h>
>  #include <xen/errno.h>
>  #include <xen/delay.h>
> +#include <xen/device_tree.h>
> +#include <xen/libfdt/libfdt.h>
>  #include <xen/list.h>
>  #include <xen/sizes.h>
>  #include <xen/vmap.h>
> @@ -1205,6 +1207,61 @@ static void its_cpu_init_collection(void)
>      spin_unlock(&its_lock);
>  }
>  
> +int its_make_dt_node(const struct domain *d,
> +                     const struct dt_device_node *node, void *fdt)
> +{
> +    struct its_node *its;
> +    const struct dt_device_node *gic;
> +    const void *compatible = NULL;
> +    u32 len;
> +    __be32 *new_cells, *tmp;
> +    int res = 0;
> +
> +    /* Will pass only first ITS node info */
> +    /* TODO: Handle multi node */
> +    its = list_first_entry(&its_nodes, struct its_node, entry);

I think this should be done at the top level by not walking all children
and by blacklisting all ITS nodes to be replaced by our own.

It's also not clear why list_first_entry should be NULL for all but the
first, but nevermind.

> +    if ( !its )
> +    {
> +        dprintk(XENLOG_ERR, "ITS node not found\n");
> +        return -FDT_ERR_XEN(ENOENT);
> +    }
> +
> +    gic = its->dt_node;
> +
> +    compatible = dt_get_property(gic, "compatible", &len);
> +    if ( !compatible )
> +    {
> +        dprintk(XENLOG_ERR, "Can't find compatible property for the its node\n");
> +        return -FDT_ERR_XEN(ENOENT);
> +    }
> +
> +    res = fdt_begin_node(fdt, "gic-its");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "compatible", compatible, len);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "msi-controller", NULL, 0);
> +    if ( res )
> +        return res;
> +
> +    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
> +
> +    new_cells = xzalloc_bytes(len);
> +    if ( new_cells == NULL )
> +        return -FDT_ERR_XEN(ENOMEM);
> +    tmp = new_cells;
> +
> +    dt_set_range(&tmp, node, its->phys_base, its->phys_size);
> +
> +    res = fdt_property(fdt, "reg", new_cells, len);
> +    xfree(new_cells);
> +
> +    return res;
> +}
> +
>  static int its_force_quiescent(void __iomem *base)
>  {
>      u32 count = 1000000;   /* 1s */
> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
> index 7e25f5b..c4c2a07 100644
> --- a/xen/include/asm-arm/gic-its.h
> +++ b/xen/include/asm-arm/gic-its.h
> @@ -274,6 +274,8 @@ static inline uint32_t its_decode_devid(struct domain *d, its_cmd_block *cmd)
>      return (cmd->raw_cmd[0] >> 32);  
>  }
>  
> +int its_make_dt_node(const struct domain *d,
> +                     const struct dt_device_node *node, void *fdt);
>  int its_cpu_init(void);
>  int its_init(struct rdist_prop *rdist);
>  void its_domain_init(struct domain *d);