Leonid Komarianskyi <leonid_komarians...@epam.com> writes:

> This change introduces resource management in the VGIC to handle
> extended SPIs introduced in GICv3.1. The pending_irqs and
> allocated_irqs arrays are resized to support the required
> number of eSPIs, based on what is supported by the hardware and
> requested by the guest. A new field, ext_shared_irqs, is added
> to the VGIC structure to store information about eSPIs, similar
> to how shared_irqs is used for regular SPIs.
>
> Since the eSPI range starts at INTID 4096 and INTIDs between 1025
> and 4095 are reserved, helper macros are introduced to simplify the
> transformation of indices and to enable easier access to eSPI-specific
> resources. These changes prepare the VGIC for processing eSPIs as
> required by future functionality.
>
> The initialization and deinitialization paths for vgic have been updated
> to allocate and free these resources appropriately. Additionally,
> updated handling of INTIDs greater than 1024, passed from the toolstack
> during domain creation, and verification logic ensures only valid SPI or
> eSPI INTIDs are used.
>
> The existing SPI behavior remains unaffected when guests do not request
> eSPIs, GIC hardware does not support them, or the CONFIG_GICV3_ESPI
> option is disabled.
>
> Signed-off-by: Leonid Komarianskyi <leonid_komarians...@epam.com>
> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>

Reviewed-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>

>
> ---
> Changes in V7:
> - minor: changed hardcoded value of 32 to NR_LOCAL_IRQS
> - minor: used local variable idx in spi_to_pending() and vgic_reserve_virq()
> - minor: added a comment for allocated_irqs and pending_irqs with index
>   mappings
> - added reviewed-by from Oleksandr Tyshchenko
>
> Changes for V6:
> - introduced a new function for index to virq conversion, idx_to_virq.
>   This allows the removal of eSPI-specific functions and enables the
>   reuse of existing code for regular SPIs
> - fixed the return value of vgic_allocate_virq in the case of eSPI
> - updated the error message in route_irq_to_guest(). To simplify it and
>   avoid overcomplicating with INTID ranges, propose removing the
>   information about the max number of IRQs
> - fixed eSPI rank initialization to avoid using EXT_RANK_IDX2NUM for
>   converting the eSPI rank to the extended range
> - updated the recalculation logic to avoid the nr_spis > 1020 -
>   NR_LOCAL_IRQS check when nr_spis is reassigned in the case of eSPI
> - fixed formatting issues for macros
> - minor change: changed the macros ESPI_INTID2IDX and ESPI_IDX2INTID
>   into appropriate inline functions introduced in the previous patch
> - minor change: changed int to unsigned int for nr_espis
>
> Changes in V5:
> - removed the has_espi field because it can be determined by checking
>   whether domain->arch.vgic.nr_espis is zero or not
> - since vgic_ext_rank_offset is not used in this patch, it has been
>   moved to the appropriate patch in the patch series, which implements
>   vgic eSPI registers emulation and requires this function
> - removed ifdefs for eSPI-specific macros to reduce the number of ifdefs
>   and code duplication in further changes
> - fixed minor nit: used %pd for printing domain with its ID
>
> Changes in V4:
> - added has_espi field to simplify determining whether a domain is able
>   to operate with eSPI
> - fixed formatting issues and misspellings
>
> Changes in V3:
> - fixed formatting for lines with more than 80 symbols
> - introduced helper functions to be able to use stubs in case of
>   CONFIG_GICV3_ESPI disabled, and as a result, reduce the number of
>   #ifdefs
> - fixed checks for nr_spis in domain_vgic_init
> - updated comment about nr_spis adjustments with dom0less mention
> - moved comment with additional explanations before checks
> - used unsigned int for indexes since they cannot be negative
> - removed unnecessary parentheses
> - move vgic_ext_rank_offset to the below ifdef guard, to reduce the
>   number of ifdefs
>
> Changes in V2:
> - change is_espi_rank to is_valid_espi_rank to verify whether the array
>   element ext_shared_irqs exists. The previous version, is_espi_rank,
>   only checked if the rank index was less than the maximum possible eSPI
>   rank index, but this could potentially result in accessing a
>   non-existing array element. To address this, is_valid_espi_rank was
>   introduced, which ensures that the required eSPI rank exists
> - move gic_number_espis to
>   xen/arm: gicv3: implement handling of GICv3.1 eSPI
> - update vgic_is_valid_irq checks to allow operating with eSPIs
> - remove redundant newline in vgic_allocate_virq
> ---
>  xen/arch/arm/include/asm/vgic.h |  26 ++++-
>  xen/arch/arm/irq.c              |   3 +-
>  xen/arch/arm/vgic.c             | 180 +++++++++++++++++++++++++++++---
>  3 files changed, 193 insertions(+), 16 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h
> index 3e7cbbb196..caffea092b 100644
> --- a/xen/arch/arm/include/asm/vgic.h
> +++ b/xen/arch/arm/include/asm/vgic.h
> @@ -144,11 +144,25 @@ struct vgic_dist {
>      spinlock_t lock;
>      uint32_t ctlr;
>      int nr_spis; /* Number of SPIs */
> -    unsigned long *allocated_irqs; /* bitmap of IRQs allocated */
> +    /*
> +     * Bitmap of allocated IRQs with the following index mapping:
> +     * Local IRQs [0..31]
> +     * Regular SPIs [32..nr_spis + 31]
> +     * Optional, if supported:
> +     * Extended SPIs [nr_spis + 32..nr_spis + nr_espis + 31]
> +     */
> +    unsigned long *allocated_irqs;
>      struct vgic_irq_rank *shared_irqs;
> +#ifdef CONFIG_GICV3_ESPI
> +    struct vgic_irq_rank *ext_shared_irqs;
> +    unsigned int nr_espis; /* Number of extended SPIs */
> +#endif
>      /*
>       * SPIs are domain global, SGIs and PPIs are per-VCPU and stored in
> -     * struct arch_vcpu.
> +     * struct arch_vcpu. The index mapping is as follows:
> +     * Regular SPIs [0..nr_spis - 1]
> +     * Optional, if supported:
> +     * eSPIs [nr_spis..nr_spis + nr_espis - 1]
>       */
>      struct pending_irq *pending_irqs;
>      /* Base address for guest GIC */
> @@ -243,6 +257,14 @@ struct vgic_ops {
>  /* Number of ranks of interrupt registers for a domain */
>  #define DOMAIN_NR_RANKS(d) (((d)->arch.vgic.nr_spis+31)/32)
>  
> +#ifdef CONFIG_GICV3_ESPI
> +#define DOMAIN_NR_EXT_RANKS(d) (((d)->arch.vgic.nr_espis + 31) / 32)
> +#endif
> +#define EXT_RANK_MIN (ESPI_BASE_INTID / 32)
> +#define EXT_RANK_MAX ((ESPI_MAX_INTID + 31) / 32)
> +#define EXT_RANK_NUM2IDX(num) ((num) - EXT_RANK_MIN)
> +#define EXT_RANK_IDX2NUM(idx) ((idx) + EXT_RANK_MIN)
> +
>  #define vgic_lock(v)   spin_lock_irq(&(v)->domain->arch.vgic.lock)
>  #define vgic_unlock(v) spin_unlock_irq(&(v)->domain->arch.vgic.lock)
>  
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index c934d39bf6..c2f1ceb91f 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -494,8 +494,7 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>      if ( !vgic_is_valid_line(d, virq) )
>      {
>          printk(XENLOG_G_ERR
> -               "the vIRQ number %u is too high for domain %u (max = %u)\n",
> -               irq, d->domain_id, vgic_num_irqs(d));
> +               "invalid vIRQ number %u for domain %pd\n", irq, d);
>          return -EINVAL;
>      }
>  
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 2bbf4d99aa..878daa71d4 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -25,11 +25,61 @@
>  #include <asm/vgic.h>
>  
>  
> +static inline unsigned int idx_to_virq(struct domain *d, unsigned int idx)
> +{
> +    if ( idx >= vgic_num_irqs(d) )
> +        return espi_idx_to_intid(idx - vgic_num_irqs(d));
> +
> +    return idx;
> +}
> +
>  bool vgic_is_valid_line(struct domain *d, unsigned int virq)
>  {
> +#ifdef CONFIG_GICV3_ESPI
> +    if ( virq >= ESPI_BASE_INTID &&
> +         virq < espi_idx_to_intid(d->arch.vgic.nr_espis) )
> +        return true;
> +#endif
> +
>      return virq < vgic_num_irqs(d);
>  }
>  
> +#ifdef CONFIG_GICV3_ESPI
> +/*
> + * Since eSPI indexes start from 4096 and numbers from 1024 to
> + * 4095 are forbidden, we need to check both lower and upper
> + * limits for ranks.
> + */
> +static inline bool is_valid_espi_rank(struct domain *d, unsigned int rank)
> +{
> +    return rank >= EXT_RANK_MIN &&
> +           EXT_RANK_NUM2IDX(rank) < DOMAIN_NR_EXT_RANKS(d);
> +}
> +
> +static inline struct vgic_irq_rank *vgic_get_espi_rank(struct vcpu *v,
> +                                                       unsigned int rank)
> +{
> +    return &v->domain->arch.vgic.ext_shared_irqs[EXT_RANK_NUM2IDX(rank)];
> +}
> +
> +#else
> +static inline bool is_valid_espi_rank(struct domain *d, unsigned int rank)
> +{
> +    return false;
> +}
> +
> +/*
> + * This function is stub and will not be called if CONFIG_GICV3_ESPI=n,
> + * because in this case, is_valid_espi_rank will always return false.
> + */
> +static inline struct vgic_irq_rank *vgic_get_espi_rank(struct vcpu *v,
> +                                                       unsigned int rank)
> +{
> +    ASSERT_UNREACHABLE();
> +    return NULL;
> +}
> +#endif
> +
>  static inline struct vgic_irq_rank *vgic_get_rank(struct vcpu *v,
>                                                    unsigned int rank)
>  {
> @@ -37,6 +87,8 @@ static inline struct vgic_irq_rank *vgic_get_rank(struct 
> vcpu *v,
>          return v->arch.vgic.private_irqs;
>      else if ( rank <= DOMAIN_NR_RANKS(v->domain) )
>          return &v->domain->arch.vgic.shared_irqs[rank - 1];
> +    else if ( is_valid_espi_rank(v->domain, rank) )
> +        return vgic_get_espi_rank(v, rank);
>      else
>          return NULL;
>  }
> @@ -117,6 +169,54 @@ int domain_vgic_register(struct domain *d, unsigned int 
> *mmio_count)
>      return 0;
>  }
>  
> +#ifdef CONFIG_GICV3_ESPI
> +static unsigned int vgic_num_spi_lines(struct domain *d)
> +{
> +    return d->arch.vgic.nr_spis + d->arch.vgic.nr_espis;
> +}
> +
> +static int init_vgic_espi(struct domain *d)
> +{
> +    unsigned int i, idx;
> +
> +    if ( d->arch.vgic.nr_espis == 0 )
> +        return 0;
> +
> +    d->arch.vgic.ext_shared_irqs =
> +        xzalloc_array(struct vgic_irq_rank, DOMAIN_NR_EXT_RANKS(d));
> +    if ( d->arch.vgic.ext_shared_irqs == NULL )
> +        return -ENOMEM;
> +
> +    for ( i = d->arch.vgic.nr_spis, idx = 0;
> +          i < vgic_num_spi_lines(d); i++, idx++ )
> +        vgic_init_pending_irq(&d->arch.vgic.pending_irqs[i],
> +                              espi_idx_to_intid(idx));
> +
> +    for ( i = 0; i < DOMAIN_NR_EXT_RANKS(d); i++ )
> +        vgic_rank_init(&d->arch.vgic.ext_shared_irqs[i],
> +                       EXT_RANK_IDX2NUM(i), 0);
> +
> +    return 0;
> +}
> +
> +#else
> +static int init_vgic_espi(struct domain *d)
> +{
> +    return 0;
> +}
> +
> +static unsigned int vgic_num_spi_lines(struct domain *d)
> +{
> +    return d->arch.vgic.nr_spis;
> +}
> +
> +#endif
> +
> +static unsigned int vgic_num_alloc_irqs(struct domain *d)
> +{
> +    return vgic_num_spi_lines(d) + NR_LOCAL_IRQS;
> +}
> +
>  int domain_vgic_init(struct domain *d, unsigned int nr_spis)
>  {
>      int i;
> @@ -133,7 +233,39 @@ int domain_vgic_init(struct domain *d, unsigned int 
> nr_spis)
>  
>      /* Limit the number of virtual SPIs supported to (1020 - 32) = 988  */
>      if ( nr_spis > (1020 - NR_LOCAL_IRQS) )
> +#ifndef CONFIG_GICV3_ESPI
>          return -EINVAL;
> +#else
> +    {
> +        /*
> +         * During domain creation, the dom0less DomUs code or toolstack
> +         * specifies the maximum INTID, which is defined in the domain
> +         * config subtracted by 32 to cover the local IRQs (please see
> +         * the comment to VGIC_DEF_NR_SPIS). To compute the actual number
> +         * of eSPI that will be usable for, add back 32 (NR_LOCAL_IRQS).
> +         */
> +        nr_spis += NR_LOCAL_IRQS;
> +        if ( nr_spis > espi_idx_to_intid(NR_ESPI_IRQS) )
> +            return -EINVAL;
> +
> +        if ( nr_spis >= ESPI_BASE_INTID )
> +        {
> +            unsigned int nr_espis = min(nr_spis - ESPI_BASE_INTID, 1024U);
> +
> +            /* Verify if GIC HW can handle provided INTID */
> +            if ( nr_espis > gic_number_espis() )
> +                return -EINVAL;
> +
> +            d->arch.vgic.nr_espis = nr_espis;
> +            /* Set the maximum available number for regular SPIs */
> +            nr_spis = VGIC_DEF_NR_SPIS;
> +        }
> +        else
> +        {
> +            return -EINVAL;
> +        }
> +    }
> +#endif
>  
>      d->arch.vgic.nr_spis = nr_spis;
>  
> @@ -145,7 +277,7 @@ int domain_vgic_init(struct domain *d, unsigned int 
> nr_spis)
>          return -ENOMEM;
>  
>      d->arch.vgic.pending_irqs =
> -        xzalloc_array(struct pending_irq, d->arch.vgic.nr_spis);
> +        xzalloc_array(struct pending_irq, vgic_num_spi_lines(d));
>      if ( d->arch.vgic.pending_irqs == NULL )
>          return -ENOMEM;
>  
> @@ -156,12 +288,16 @@ int domain_vgic_init(struct domain *d, unsigned int 
> nr_spis)
>      for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ )
>          vgic_rank_init(&d->arch.vgic.shared_irqs[i], i + 1, 0);
>  
> +    ret = init_vgic_espi(d);
> +    if ( ret )
> +        return ret;
> +
>      ret = d->arch.vgic.handler->domain_init(d);
>      if ( ret )
>          return ret;
>  
>      d->arch.vgic.allocated_irqs =
> -        xzalloc_array(unsigned long, BITS_TO_LONGS(vgic_num_irqs(d)));
> +        xzalloc_array(unsigned long, BITS_TO_LONGS(vgic_num_alloc_irqs(d)));
>      if ( !d->arch.vgic.allocated_irqs )
>          return -ENOMEM;
>  
> @@ -182,9 +318,12 @@ void domain_vgic_free(struct domain *d)
>      int i;
>      int ret;
>  
> -    for ( i = 0; i < (d->arch.vgic.nr_spis); i++ )
> +    for ( i = NR_LOCAL_IRQS; i < vgic_num_alloc_irqs(d); i++ )
>      {
> -        struct pending_irq *p = spi_to_pending(d, i + 32);
> +        struct pending_irq *p;
> +        unsigned int virq = idx_to_virq(d, i);
> +
> +        p = spi_to_pending(d, virq);
>  
>          if ( p->desc )
>          {
> @@ -198,6 +337,9 @@ void domain_vgic_free(struct domain *d)
>      if ( d->arch.vgic.handler )
>          d->arch.vgic.handler->domain_free(d);
>      xfree(d->arch.vgic.shared_irqs);
> +#ifdef CONFIG_GICV3_ESPI
> +    xfree(d->arch.vgic.ext_shared_irqs);
> +#endif
>      xfree(d->arch.vgic.pending_irqs);
>      xfree(d->arch.vgic.allocated_irqs);
>  }
> @@ -323,10 +465,12 @@ void arch_move_irqs(struct vcpu *v)
>       */
>      ASSERT(!is_lpi(vgic_num_irqs(d) - 1));
>  
> -    for ( i = 32; i < vgic_num_irqs(d); i++ )
> +    for ( i = NR_LOCAL_IRQS; i < vgic_num_alloc_irqs(d); i++ )
>      {
> -        v_target = vgic_get_target_vcpu(v, i);
> -        p = irq_to_pending(v_target, i);
> +        unsigned int virq = idx_to_virq(d, i);
> +
> +        v_target = vgic_get_target_vcpu(v, virq);
> +        p = irq_to_pending(v_target, virq);
>  
>          if ( v_target == v && !test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) 
> )
>              irq_set_affinity(p->desc, cpu_mask);
> @@ -539,15 +683,22 @@ struct pending_irq *irq_to_pending(struct vcpu *v, 
> unsigned int irq)
>      else if ( is_lpi(irq) )
>          n = v->domain->arch.vgic.handler->lpi_to_pending(v->domain, irq);
>      else
> -        n = &v->domain->arch.vgic.pending_irqs[irq - 32];
> +        n = spi_to_pending(v->domain, irq);
>      return n;
>  }
>  
>  struct pending_irq *spi_to_pending(struct domain *d, unsigned int irq)
>  {
> +    unsigned int idx;
> +
>      ASSERT(irq >= NR_LOCAL_IRQS);
>  
> -    return &d->arch.vgic.pending_irqs[irq - 32];
> +    if ( is_espi(irq) )
> +        idx = espi_intid_to_idx(irq) + d->arch.vgic.nr_spis;
> +    else
> +        idx = irq - NR_LOCAL_IRQS;
> +
> +    return &d->arch.vgic.pending_irqs[idx];
>  }
>  
>  void vgic_clear_pending_irqs(struct vcpu *v)
> @@ -665,10 +816,15 @@ bool vgic_emulate(struct cpu_user_regs *regs, union hsr 
> hsr)
>  
>  bool vgic_reserve_virq(struct domain *d, unsigned int virq)
>  {
> +    unsigned int idx = virq;
> +
>      if ( !vgic_is_valid_line(d, virq) )
>          return false;
>  
> -    return !test_and_set_bit(virq, d->arch.vgic.allocated_irqs);
> +    if ( is_espi(virq) )
> +        idx = espi_intid_to_idx(virq) + vgic_num_irqs(d);
> +
> +    return !test_and_set_bit(idx, d->arch.vgic.allocated_irqs);
>  }
>  
>  int vgic_allocate_virq(struct domain *d, bool spi)
> @@ -685,7 +841,7 @@ int vgic_allocate_virq(struct domain *d, bool spi)
>      else
>      {
>          first = 32;
> -        end = vgic_num_irqs(d);
> +        end = vgic_num_alloc_irqs(d);
>      }
>  
>      /*
> @@ -700,7 +856,7 @@ int vgic_allocate_virq(struct domain *d, bool spi)
>      }
>      while ( test_and_set_bit(virq, d->arch.vgic.allocated_irqs) );
>  
> -    return virq;
> +    return idx_to_virq(d, virq);
>  }
>  
>  void vgic_free_virq(struct domain *d, unsigned int virq)

-- 
WBR, Volodymyr

Reply via email to