Re: [PATCH v10 02/32] ARM: GICv3: setup number of LPI bits for a GICv3 guest

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

Hi Andre,

On 26/05/17 18:35, Andre Przywara wrote:
> The host supports a certain number of LPI identifiers, as stored in
> the GICD_TYPER register.
> Store this number from the hardware register in vgic_v3_hw to allow
> injecting the very same number into a guest (Dom0).
> DomUs get the legacy number of 10 bits here, since for now it only sees
> SPIs, so it does not need more. This should be revisited once we get
> proper DomU ITS support.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  xen/arch/arm/gic-v3.c        |  6 +++++-
>  xen/arch/arm/vgic-v3.c       | 10 +++++++++-
>  xen/include/asm-arm/domain.h |  1 +
>  xen/include/asm-arm/vgic.h   |  3 ++-
>  4 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index a559e5e..29c8964 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1579,6 +1579,7 @@ static int __init gicv3_init(void)
>  {
>      int res, i;
>      uint32_t reg;
> +    unsigned int intid_bits;
>
>      if ( !cpu_has_gicv3 )
>      {
> @@ -1622,8 +1623,11 @@ static int __init gicv3_init(void)
>                 i, r->base, r->base + r->size);
>      }
>
> +    reg = readl_relaxed(GICD + GICD_TYPER);
> +    intid_bits = GICD_TYPE_ID_BITS(reg);
> +
>      vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
> -                     gicv3.rdist_stride);
> +                     gicv3.rdist_stride, intid_bits);
>      gicv3_init_v2();
>
>      spin_lock_init(&gicv3.lock);
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index d10757a..87f5fb3 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -57,18 +57,21 @@ static struct {
>      unsigned int nr_rdist_regions;
>      const struct rdist_region *regions;
>      uint32_t rdist_stride; /* Re-distributor stride */
> +    unsigned int intid_bits;  /* Number of interrupt ID bits */
>  } vgic_v3_hw;
>
>  void vgic_v3_setup_hw(paddr_t dbase,
>                        unsigned int nr_rdist_regions,
>                        const struct rdist_region *regions,
> -                      uint32_t rdist_stride)
> +                      uint32_t rdist_stride,
> +                      unsigned int intid_bits)
>  {
>      vgic_v3_hw.enabled = 1;
>      vgic_v3_hw.dbase = dbase;
>      vgic_v3_hw.nr_rdist_regions = nr_rdist_regions;
>      vgic_v3_hw.regions = regions;
>      vgic_v3_hw.rdist_stride = rdist_stride;
> +    vgic_v3_hw.intid_bits = intid_bits;
>  }
>
>  static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain *d, uint64_t irouter)
> @@ -1482,6 +1485,8 @@ static int vgic_v3_domain_init(struct domain *d)
>
>              first_cpu += size / d->arch.vgic.rdist_stride;
>          }
> +
> +        d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits;
>      }
>      else
>      {
> @@ -1497,6 +1502,9 @@ static int vgic_v3_domain_init(struct domain *d)
>          d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE;
>          d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE;
>          d->arch.vgic.rdist_regions[0].first_cpu = 0;
> +
> +        /* TODO: only SPIs for now, adjust this when guests need LPIs */
> +        d->arch.vgic.intid_bits = 10;

What if the guest support no SPIs? Should not you based this number of 
the number of SPIs supported by the guest?

>      }
>
>      ret = vgic_v3_its_init_domain(d);
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 6de8082..7c3829d 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -111,6 +111,7 @@ struct arch_domain
>          uint32_t rdist_stride;              /* Re-Distributor stride */
>          struct rb_root its_devices;         /* Devices mapped to an ITS */
>          spinlock_t its_devices_lock;        /* Protects the its_devices tree */
> +        unsigned int intid_bits;
>  #endif
>      } vgic;
>
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 544867a..df75064 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -346,7 +346,8 @@ struct rdist_region;
>  void vgic_v3_setup_hw(paddr_t dbase,
>                        unsigned int nr_rdist_regions,
>                        const struct rdist_region *regions,
> -                      uint32_t rdist_stride);
> +                      uint32_t rdist_stride,
> +                      unsigned int intid_bits);
>  #endif
>
>  #endif /* __ASM_ARM_VGIC_H__ */
>

Cheers,

-- 
Julien Grall

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