Re: [PATCH v5 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Sat, 23 Jan 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Refactor gic-v3 related functions into dt and generic parts. This will be
> helpful when adding acpi support for gic-v3.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: none
> v4: Use INVALID_PADDR and move ioremap to common init function
> ---
>  xen/arch/arm/gic-v3.c | 114 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 61 insertions(+), 53 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index a245b56..65a4de6 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1138,41 +1138,14 @@ static int __init cmp_rdist(const void *a, const void *b)
>      return ( l->base < r->base) ? -1 : 0;
>  }
>  
> +static paddr_t __initdata dbase = INVALID_PADDR, vbase = INVALID_PADDR;
> +static paddr_t __initdata cbase = INVALID_PADDR, csize = INVALID_PADDR;
> +
>  /* If the GICv3 supports GICv2, initialize it */
> -static void __init gicv3_init_v2(const struct dt_device_node *node,
> -                                 paddr_t dbase)
> +static void __init gicv3_init_v2(void)
>  {
> -    int res;
> -    paddr_t cbase, csize;
> -    paddr_t vbase, vsize;
> -
> -    /*
> -     * For GICv3 supporting GICv2, GICC and GICV base address will be
> -     * provided.
> -     */
> -    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> -                                &cbase, &csize);
> -    if ( res )
> -        return;
> -
> -    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> -                                &vbase, &vsize);
> -    if ( res )
> -        return;
> -
> -    /*
> -     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
> -     * So only support GICv2 on GICv3 when the virtual CPU interface is
> -     * at least GUEST_GICC_SIZE.
> -     */
> -    if ( vsize < GUEST_GICC_SIZE )
> -    {
> -        printk(XENLOG_WARNING
> -               "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
> -               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
> -               vsize, GUEST_GICC_SIZE);

The vsize < GUEST_GICC_SIZE check needs to remain here because ... 


> +    if ( cbase == INVALID_PADDR || vbase == INVALID_PADDR )
>          return;
> -    }
>  
>      printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase %#"PRIpaddr"\n",
>             cbase, vbase);
> @@ -1180,20 +1153,12 @@ static void __init gicv3_init_v2(const struct dt_device_node *node,
>      vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
>  }
>  
> -/* Set up the GIC */
> -static int __init gicv3_init(void)
> +static void __init gicv3_dt_init(void)
>  {
>      struct rdist_region *rdist_regs;
>      int res, i;
> -    uint32_t reg;
>      const struct dt_device_node *node = gicv3_info.node;
> -    paddr_t dbase;
> -
> -    if ( !cpu_has_gicv3 )
> -    {
> -        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
> -        return -ENODEV;
> -    }
> +    paddr_t vsize;
>  
>      res = dt_device_get_address(node, 0, &dbase, NULL);
>      if ( res )
> @@ -1203,14 +1168,6 @@ static int __init gicv3_init(void)
>          panic("GICv3:  Found unaligned distributor address %"PRIpaddr"",
>                dbase);
>  
> -    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
> -    if ( !gicv3.map_dbase )
> -        panic("GICv3: Failed to ioremap for GIC distributor\n");
> -
> -    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> -    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
> -         panic("GICv3: no distributor detected\n");
> -
>      if ( !dt_property_read_u32(node, "#redistributor-regions",
>                  &gicv3.rdist_count) )
>          gicv3.rdist_count = 1;
> @@ -1248,6 +1205,57 @@ static int __init gicv3_init(void)
>          panic("GICv3: Cannot find the maintenance IRQ");
>      gicv3_info.maintenance_irq = res;
>  
> +    /*
> +     * For GICv3 supporting GICv2, GICC and GICV base address will be
> +     * provided.
> +     */
> +    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> +                                &cbase, &csize);
> +    if ( res )
> +        return;
> +
> +    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> +                                &vbase, &vsize);
> +    if ( res )
> +        return;
> +
> +    /*
> +     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
> +     * So only support GICv2 on GICv3 when the virtual CPU interface is
> +     * at least GUEST_GICC_SIZE.
> +     */
> +    if ( vsize < GUEST_GICC_SIZE )
> +    {
> +        printk(XENLOG_WARNING
> +               "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
> +               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
> +               vsize, GUEST_GICC_SIZE);
> +        return;

... here it is completely ineffectual.


> +    }
> +}
> +
> +/* Set up the GIC */
> +static int __init gicv3_init(void)
> +{
> +    int res, i;
> +    uint32_t reg;
> +
> +    if ( !cpu_has_gicv3 )
> +    {
> +        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
> +        return -ENODEV;
> +    }
> +
> +    gicv3_dt_init();
> +
> +    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
> +    if ( !gicv3.map_dbase )
> +        panic("GICv3: Failed to ioremap for GIC distributor\n");
> +
> +    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> +    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
> +         panic("GICv3: no distributor detected\n");
> +
>      for ( i = 0; i < gicv3.rdist_count; i++ )
>      {
>          /* map dbase & rdist regions */
> @@ -1277,7 +1285,7 @@ static int __init gicv3_init(void)
>  
>      vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
>                       gicv3.rdist_stride);
> -    gicv3_init_v2(node, dbase);
> +    gicv3_init_v2();
>  
>      spin_lock_init(&gicv3.lock);
>  
> @@ -1317,7 +1325,7 @@ static const struct gic_hw_operations gicv3_ops = {
>      .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
>  };
>  
> -static int __init gicv3_preinit(struct dt_device_node *node, const void *data)
> +static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
>  {
>      gicv3_info.hw_version = GIC_V3;
>      gicv3_info.node = node;
> @@ -1335,7 +1343,7 @@ static const struct dt_device_match gicv3_dt_match[] __initconst =
>  
>  DT_DEVICE_START(gicv3, "GICv3", DEVICE_GIC)
>          .dt_match = gicv3_dt_match,
> -        .init = gicv3_preinit,
> +        .init = gicv3_dt_preinit,
>  DT_DEVICE_END
>  
>  /*
> -- 
> 2.0.4
> 
>