Re: [PATCH v7 22/28] xen/arm: ITS: Allocate irq descriptors for LPIs

[Julien Grall <julien.grall@citrix.com>]

Hi Vijay,

On 18/09/15 14:09, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Allocate dynamically irq descriptors for LPIs

This patch is the best example why we should only use nr_lpis to setup
the number of ID bits in the GICv3.

Here you are using gic_is_lpi to know if the IRQ is an LPIs. This
function is using the number of IRQ (gic_nr_irq_ids()).

Although, you allocate the LPIs using nr_lpis...

> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
> v6: - Add separate patch for irq_pending structures
>     - renamed and moved is_domain_lpi to vgic
>     - Updated __irq_to_domain
> ---
>  xen/arch/arm/gic-v3-its.c |    4 ++++
>  xen/arch/arm/irq.c        |   34 +++++++++++++++++++++++++++++++++-
>  xen/include/asm-arm/irq.h |    1 +
>  3 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index f3346d3..4875b0f 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -1490,6 +1490,10 @@ int __init its_init(struct rdist_prop *rdists)
>      its_alloc_lpi_tables();
>      its_lpi_init(rdists->id_bits);
>  
> +    /* Allocate irq descriptors for LPIs */
> +    if ( init_lpi() )
> +        return -ENOMEM;
> +
>      its = list_first_entry(&its_nodes, struct its_node, entry);
>      /*
>       * As per vITS design spec, Xen exposes only one virtual ITS per domain.
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 3a01f46..32c3b53 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -31,6 +31,8 @@
>  static unsigned int local_irqs_type[NR_LOCAL_IRQS];
>  static DEFINE_SPINLOCK(local_irqs_type_lock);
>  
> +static irq_desc_t *irq_desc_lpi;
> +
>  /* Number of LPIs supported by Xen.
>   *
>   * The LPI identifier starts from 8192. Given that the hardware is
> @@ -73,7 +75,15 @@ static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
>  irq_desc_t *__irq_to_desc(int irq)
>  {
>      if (irq < NR_LOCAL_IRQS) return &this_cpu(local_irq_desc)[irq];
> -    return &irq_desc[irq-NR_LOCAL_IRQS];
> +    else if ( gic_is_lpi(irq) )
> +    {
> +        ASSERT(irq_desc_lpi != NULL);
> +        return &irq_desc_lpi[irq - FIRST_GIC_LPI];
> +    }
> +    else
> +        return &irq_desc[irq - NR_LOCAL_IRQS];
> +
> +    return NULL;
>  }
>  
>  int __init arch_init_one_irq_desc(struct irq_desc *desc)
> @@ -123,6 +133,28 @@ static int __cpuinit init_local_irq_data(void)
>      return 0;
>  }
>  
> +int init_lpi(void)
> +{
> +    struct irq_desc *desc;
> +    unsigned int i;
> +
> +    /* Allocate LPI irq descriptors */
> +    irq_desc_lpi = xzalloc_array(struct irq_desc, nr_lpis);

Please use gic_nr_irq_ids()

> +    if ( !irq_desc_lpi )
> +        return -ENOSPC;

We can't allocate memory so the proper errno is ENOMEM.

> +
> +    for ( i = 0; i < nr_lpis; i++ )

Same here.

> +    {
> +       desc = &irq_desc_lpi[i];
> +       init_one_irq_desc(desc);
> +       desc->irq = FIRST_GIC_LPI + i;
> +       desc->arch.type = DT_IRQ_TYPE_EDGE_BOTH;
> +       desc->action = NULL;
> +    }
> +
> +    return 0;
> +}
> +
>  void __init init_IRQ(void)
>  {
>      int irq;

Regards,

-- 
Julien Grall