Re: [Xen-devel] [PATCH v3 15/16] xen/arm: Update Dom0 GIC dt node with GICv3 information

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-04-15 at 16:47 +0530, vijay.kilari@xxxxxxxxx wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> 
> Update GIC device tree node for DOM0 with GICv3
> information. GIC hw specfic device tree information
> is moved to respective GIC driver.

Please add a reference to the relevant bindings document. NB this
probably does mean that much of this series is blocked on that binding
being formalised (AKA committed to
linux/Docuemtnation/devicetree/bindings at the moment), so please let us
know when that has happened.

> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 1e62dd9..cacb9e4 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -487,6 +487,14 @@ static unsigned int gicv2_read_vmcr_priority(void)
>     return (GICH[GICH_VMCR] >> GICH_VMCR_PRIORITY_SHIFT) & 
> GICH_VMCR_PRIORITY_MASK;
>  }
>  
> +int static gicv3_make_dt_node(const struct domain *d,
> +              const struct dt_device_node *node, void *fdt, __be32 *cells)
> +{
> +    dt_set_range(&cells, node, d->arch.vgic.dbase, PAGE_SIZE);
> +    dt_set_range(&cells, node, d->arch.vgic.cbase, PAGE_SIZE * 2);

No rdist? 

Oh wait, this is gic-v2.c, so your function name is just wrong.

> +    return 0;
> +}
> +
>  static hw_irq_controller irq_ops = {
>      .enable              = gicv2_enable_irq,
>      .disable             = gicv2_disable_irq,
> @@ -512,6 +520,7 @@ static struct gic_hw_operations gic_ops = {
>      .read_lr             = gicv2_read_lr,
>      .write_lr            = gicv2_write_lr,
>      .read_vmcr_priority  = gicv2_read_vmcr_priority,
> +    .make_dt_node        = gicv3_make_dt_node,
>  };
>  
>  static const char * const gicv2_dt_compat[] __initconst =
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 8625e0c..e27b094 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -30,6 +30,7 @@
>  #include <xen/softirq.h>
>  #include <xen/list.h>
>  #include <xen/device_tree.h>
> +#include <xen/libfdt/libfdt.h>
>  #include <xen/delay.h>
>  #include <asm/p2m.h>
>  #include <asm/domain.h>
> @@ -825,6 +826,37 @@ static unsigned int gicv3_read_vmcr_priority(void)
>              GICH_VMCR_PRIORITY_MASK);
>  }
>  
> +int static gicv3_make_dt_node(const struct domain *d,
> +              const struct dt_device_node *node, void *fdt, __be32 *cells)
> +{
> +    uint32_t rd_stride = 0;
> +    uint32_t rd_count = 0;
> +    int res, i;
> +
> +    const struct dt_device_node *gic = dt_interrupt_controller;
> +    res = dt_property_read_u32(gic, "redistributor-stride", &rd_stride);
> +    if ( !res )
> +        rd_stride = 0;
> +
> +    res = dt_property_read_u32(gic, "#redistributor-regions", &rd_count);
> +    if ( !res )
> +        rd_count = 1;

This is passing on the physical values, which is only correct for dom0.
For domU you need to get these values from d->arch.vgic, which is where
they should come from for dom0 too.

That might mean you can drop the fdt param? If you do keep it then
please make the prototype of the function match the similar functions
throughout domain_build.c., which is (d, fdt, node)

Which also means you should drop the cells argument and handle it within
each callback, the existing one isn't big enough for gicv3 anyway. 

Ian.


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