Re: [Xen-devel] [PATCH V2 15/33] xen/arm: Use hierarchical device tree to retrieve GIC information

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-05-08 at 03:33 +0100, Julien Grall wrote:
> - Remove early parsing for GIC addresses
> - Remove hard coded maintenance IRQ number
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> Changes in v2:
>     - use the new function request_dt_irq
> ---
>  xen/arch/arm/gic.c            |   63 
> ++++++++++++++++++++++++++++-------------
>  xen/common/device_tree.c      |   42 ---------------------------
>  xen/include/xen/device_tree.h |    8 ------
>  3 files changed, 43 insertions(+), 70 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 1efa9a3..34304b3 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -45,6 +45,7 @@ static struct {
>      paddr_t hbase;       /* Address of virtual interface registers */
>      paddr_t vbase;       /* Address of virtual cpu interface registers */
>      unsigned int lines;  /* Number of interrupts (SPIs + PPIs + SGIs) */
> +    struct dt_irq maintenance; /* IRQ maintenance */
>      unsigned int cpus;
>      spinlock_t lock;
>  } gic;
> @@ -352,28 +353,49 @@ int gic_irq_xlate(const u32 *intspec, unsigned int 
> intsize,
>  /* Set up the GIC */
>  void __init gic_init(void)
>  {
> +    struct dt_device_node *node;
> +    int res;
> +
> +    node = dt_find_interrupt_controller("arm,cortex-a15-gic");
> +    if ( !node )
> +        panic("Unable to find compatible GIC in the device tree\n");
> +
> +    dt_device_set_used_by(node, DT_USED_BY_XEN);
> +
> +    res = dt_device_get_address(node, 0, &gic.dbase, NULL);
> +    if ( res || !gic.dbase || (gic.dbase & ~PAGE_MASK) )
> +        panic("GIC: Cannot find a valid address for the distributor\n");
> +
> +    res = dt_device_get_address(node, 1, &gic.cbase, NULL);
> +    if ( res || !gic.cbase || (gic.cbase & ~PAGE_MASK) )
> +        panic("GIC: Cannot find a valid address for the CPU\n");
> +
> +    res = dt_device_get_address(node, 2, &gic.hbase, NULL);
> +    if ( res || !gic.hbase || (gic.hbase & ~PAGE_MASK) )
> +        panic("GIC: Cannot find a valid address for the hypervisor\n");
> +
> +    res = dt_device_get_address(node, 3, &gic.vbase, NULL);
> +    if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) )
> +        panic("GIC: Cannot find a valid address for the virtual CPU\n");
> +
> +    res = dt_device_get_irq(node, 0, &gic.maintenance);
> +    if ( res )
> +        panic("GIC: Cannot find the maintenance IRQ\n");
> +
> +    /* Set the GIC as the primary interrupt controller */
> +    dt_interrupt_controller = node;
> +
> +    /* TODO: Add check on distributor, cpu size */
> +
>      printk("GIC initialization:\n"
>                "        gic_dist_addr=%"PRIpaddr"\n"
>                "        gic_cpu_addr=%"PRIpaddr"\n"
>                "        gic_hyp_addr=%"PRIpaddr"\n"
> -              "        gic_vcpu_addr=%"PRIpaddr"\n",
> -              early_info.gic.gic_dist_addr, early_info.gic.gic_cpu_addr,
> -              early_info.gic.gic_hyp_addr, early_info.gic.gic_vcpu_addr);
> -    if ( !early_info.gic.gic_dist_addr ||
> -         !early_info.gic.gic_cpu_addr ||
> -         !early_info.gic.gic_hyp_addr ||
> -         !early_info.gic.gic_vcpu_addr )
> -        panic("the physical address of one of the GIC interfaces is 
> missing\n");
> -    if ( (early_info.gic.gic_dist_addr & ~PAGE_MASK) ||
> -         (early_info.gic.gic_cpu_addr & ~PAGE_MASK) ||
> -         (early_info.gic.gic_hyp_addr & ~PAGE_MASK) ||
> -         (early_info.gic.gic_vcpu_addr & ~PAGE_MASK) )
> -        panic("GIC interfaces not page aligned.\n");

Please can we keep this last check separate and after the print, this
way we get to see why it failed (not present vs invalid).

I'm ambivalent about keeping the !gic.Xbase separate or including it in
the res check.

Ian.



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