On Tue, Nov 23, 2021 at 03:06:08PM +0000, Marc Zyngier wrote:
> On Tue, 09 Nov 2021 02:38:51 +0000,
> Ricardo Koller <ricarkol@xxxxxxxxxx> wrote:
> >
> > Add a generic library function for reading and writing GICv3 distributor
> > and redistributor registers. Then adapt some functions to use it; more
> > will come and use it in the next commit.
> >
> > Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx>
> > ---
> > .../selftests/kvm/lib/aarch64/gic_v3.c | 124 ++++++++++++++----
> > 1 file changed, 101 insertions(+), 23 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
> > index 2dbf3339b62e..00e944fd8148 100644
> > --- a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
> > +++ b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
> > @@ -19,7 +19,8 @@ struct gicv3_data {
> > unsigned int nr_spis;
> > };
> >
> > -#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K)
> > +#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K)
> > +#define DIST_BIT (1U << 31)
> >
> > enum gicv3_intid_range {
> > SGI_RANGE,
> > @@ -50,6 +51,14 @@ static void gicv3_gicr_wait_for_rwp(void *redist_base)
> > }
> > }
> >
> > +static void gicv3_wait_for_rwp(uint32_t cpu_or_dist)
> > +{
> > + if (cpu_or_dist & DIST_BIT)
> > + gicv3_gicd_wait_for_rwp();
> > + else
> > + gicv3_gicr_wait_for_rwp(gicv3_data.redist_base[cpu_or_dist]);
> > +}
> > +
> > static enum gicv3_intid_range get_intid_range(unsigned int intid)
> > {
> > switch (intid) {
> > @@ -81,39 +90,108 @@ static void gicv3_write_eoir(uint32_t irq)
> > isb();
> > }
> >
> > -static void
> > -gicv3_config_irq(unsigned int intid, unsigned int offset)
> > +uint32_t gicv3_reg_readl(uint32_t cpu_or_dist, uint64_t offset)
> > +{
> > + void *base = cpu_or_dist & DIST_BIT ? gicv3_data.dist_base
> > + : sgi_base_from_redist(gicv3_data.redist_base[cpu_or_dist]);
> > + return readl(base + offset);
> > +}
> > +
> > +void gicv3_reg_writel(uint32_t cpu_or_dist, uint64_t offset, uint32_t reg_val)
> > +{
> > + void *base = cpu_or_dist & DIST_BIT ? gicv3_data.dist_base
> > + : sgi_base_from_redist(gicv3_data.redist_base[cpu_or_dist]);
> > + writel(reg_val, base + offset);
> > +}
> > +
> > +uint32_t gicv3_getl_fields(uint32_t cpu_or_dist, uint64_t offset, uint32_t mask)
> > +{
> > + return gicv3_reg_readl(cpu_or_dist, offset) & mask;
> > +}
> > +
> > +void gicv3_setl_fields(uint32_t cpu_or_dist, uint64_t offset,
> > + uint32_t mask, uint32_t reg_val)
> > +{
> > + uint32_t tmp = gicv3_reg_readl(cpu_or_dist, offset) & ~mask;
> > +
> > + tmp |= (reg_val & mask);
> > + gicv3_reg_writel(cpu_or_dist, offset, tmp);
> > +}
> > +
> > +/*
> > + * We use a single offset for the distributor and redistributor maps as they
> > + * have the same value in both. The only exceptions are registers that only
> > + * exist in one and not the other, like GICR_WAKER that doesn't exist in the
> > + * distributor map. Such registers are conveniently marked as reserved in the
> > + * map that doesn't implement it; like GICR_WAKER's offset of 0x0014 being
> > + * marked as "Reserved" in the Distributor map.
> > + */
> > +static void gicv3_access_reg(uint32_t intid, uint64_t offset,
> > + uint32_t reg_bits, uint32_t bits_per_field,
> > + bool write, uint32_t *val)
> > {
> > uint32_t cpu = guest_get_vcpuid();
> > - uint32_t mask = 1 << (intid % 32);
> > enum gicv3_intid_range intid_range = get_intid_range(intid);
> > - void *reg;
> > -
> > - /* We care about 'cpu' only for SGIs or PPIs */
> > - if (intid_range == SGI_RANGE || intid_range == PPI_RANGE) {
> > - GUEST_ASSERT(cpu < gicv3_data.nr_cpus);
> > -
> > - reg = sgi_base_from_redist(gicv3_data.redist_base[cpu]) +
> > - offset;
> > - writel(mask, reg);
> > - gicv3_gicr_wait_for_rwp(gicv3_data.redist_base[cpu]);
> > - } else if (intid_range == SPI_RANGE) {
> > - reg = gicv3_data.dist_base + offset + (intid / 32) * 4;
> > - writel(mask, reg);
> > - gicv3_gicd_wait_for_rwp();
> > - } else {
> > - GUEST_ASSERT(0);
> > - }
> > + uint32_t fields_per_reg, index, mask, shift;
> > + uint32_t cpu_or_dist;
> > +
> > + GUEST_ASSERT(bits_per_field <= reg_bits);
> > + GUEST_ASSERT(*val < (1U << bits_per_field));
> > + /* Some registers like IROUTER are 64 bit long. Those are currently not
> > + * supported by readl nor writel, so just asserting here until then.
> > + */
> > + GUEST_ASSERT(reg_bits == 32);
>
> IROUTER *does* support 32bit accesses. There are no 64bit MMIO
> registers in the GIC architecture that do not support 32bit accesses,
> if only because there is no guarantee about the width of the MMIO bus
> itself (not to mention the existence of 32bit CPUs!).
>
> See 12.1.3 ("GIC memory-mapped register access") in the GICv3 arch
> spec.
I see, thanks for the pointer. Will update the comment in v2. Although I
might keep the assert as this function doesn't support 64bit accesses
(yet).
Thanks,
Ricardo
> M.
>
> --
> Without deviation from the norm, progress is not possible.
