Re: [PATCH 05/43] KVM: arm64: gic-v5: Extract host IRS caps from IRS config frame

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Tue, 2026-04-28 at 16:20 +0100, Marc Zyngier wrote:
> On Mon, 27 Apr 2026 17:07:44 +0100,
> Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> > 
> > The host irqchip driver provides KVM with a pointer to an IRS's
> > config
> > frame, which allows KVM to directly interact with the host's IRS.
> > The
> > MMIO registers in the config frame are used to configure VMs (in
> > addition to them being used by the host). The IRS's config frame
> > also
> > includes a set of ID registers which describe the capabilities that
> > the IRS has.
> > 
> > Stash the pointer to the config frame, and extract the VM
> > capabilities
> > (from IRS_IDR3 & IRS_IDR4), as well as the IST
> > capabilities/requirements (IRS_IDR2) from the IRS.
> > 
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > ---
> >  arch/arm64/kvm/Makefile              |  2 +-
> >  arch/arm64/kvm/vgic/vgic-v5-tables.c |  8 +++++
> >  arch/arm64/kvm/vgic/vgic-v5-tables.h | 41 ++++++++++++++++++++++
> >  arch/arm64/kvm/vgic/vgic-v5.c        | 52
> > ++++++++++++++++++++++++++++
> >  include/linux/irqchip/arm-gic-v5.h   | 10 ++++++
> >  5 files changed, 112 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/kvm/vgic/vgic-v5-tables.c
> >  create mode 100644 arch/arm64/kvm/vgic/vgic-v5-tables.h
> > 
> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> > index 59612d2f277c1..431de9b145ca1 100644
> > --- a/arch/arm64/kvm/Makefile
> > +++ b/arch/arm64/kvm/Makefile
> > @@ -24,7 +24,7 @@ kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o
> > pvtime.o \
> >  	 vgic/vgic-mmio.o vgic/vgic-mmio-v2.o \
> >  	 vgic/vgic-mmio-v3.o vgic/vgic-kvm-device.o \
> >  	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o \
> > -	 vgic/vgic-v5.o
> > +	 vgic/vgic-v5.o vgic/vgic-v5-tables.o
> >  
> >  kvm-$(CONFIG_HW_PERF_EVENTS)  += pmu-emul.o pmu.o
> >  kvm-$(CONFIG_ARM64_PTR_AUTH)  += pauth.o
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5-tables.c
> > b/arch/arm64/kvm/vgic/vgic-v5-tables.c
> > new file mode 100644
> > index 0000000000000..30e2b108b1aa3
> > --- /dev/null
> > +++ b/arch/arm64/kvm/vgic/vgic-v5-tables.c
> > @@ -0,0 +1,8 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2025, 2026 Arm Ltd.
> > + */
> > +
> > +#include "vgic-v5-tables.h"
> > +
> > +struct vgic_v5_host_ist_caps gicv5_host_ist_caps;
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5-tables.h
> > b/arch/arm64/kvm/vgic/vgic-v5-tables.h
> > new file mode 100644
> > index 0000000000000..cf00a248eabd5
> > --- /dev/null
> > +++ b/arch/arm64/kvm/vgic/vgic-v5-tables.h
> > @@ -0,0 +1,41 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2025, 2026 Arm Ltd.
> > + */
> > +
> > +#ifndef __KVM_ARM_VGICV5_TABLES_H__
> > +#define __KVM_ARM_VGICV5_TABLES_H__
> > +
> > +#include <linux/irqchip/arm-gic-v5.h>
> > +
> > +struct vgic_v5_host_ist_caps {
> > +	/* IST Capabilities */
> > +
> > +	/* Apply to LPIs and SPIs */
> > +	u8	ist_id_bits;
> > +	bool	ist_levels;
> > +	u8	ist_l2sz;
> > +	bool	istmd;
> > +	u8	istmd_sz;
> > +
> > +	/* LPI only */
> > +	u8	min_lpi_id_bits;
> > +
> > +	/* VM Table, VPE Table */
> > +	bool	two_level_vmt_support;
> > +	u32	max_vms;
> > +	u32	max_vpes;
> > +	u16	vmd_size;
> > +	u16	vped_size;
> > +
> > +	/* Is the IRS coherent with us, or not? */
> > +	bool	irs_non_coherent;
> > +};
> > +
> > +extern struct vgic_v5_host_ist_caps gicv5_host_ist_caps;
> > +static inline struct vgic_v5_host_ist_caps
> > *vgic_v5_host_caps(void)
> > +{
> > +	return &gicv5_host_ist_caps;
> > +}
> 
> Err. No. Make gicv5_host_ist_caps static, and move the helper as
> non-inline in vgic-v5-tables.c. It's not like this is anywhere near
> performance-critical stuff, is it?
> 
> But also, if that's global information, we have
> kvm_vgic_global_state.
> Isn't that where these things should live? Then the introduction of
> vgic-v5-tables.[ch] can be moved to the point where it actually
> matters.

I've gone with this latter approach. vgic-v5-tables.[ch] are now
introduced at a later stage in the series. All of these caps are now
part of the global state. Much cleaner for sure!

> 
> > +
> > +#endif
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c
> > b/arch/arm64/kvm/vgic/vgic-v5.c
> > index d4789ff3e7402..fd3d6299a2baa 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v5.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/irqchip/arm-vgic-info.h>
> >  
> >  #include "vgic.h"
> > +#include "vgic-v5-tables.h"
> >  
> >  #define ppi_caps	kvm_vgic_global_state.vgic_v5_ppi_caps
> >  
> > @@ -34,6 +35,54 @@ static void vgic_v5_get_implemented_ppis(void)
> >  	__assign_bit(GICV5_ARCH_PPI_PMUIRQ,
> > ppi_caps.impl_ppi_mask, system_supports_pmuv3());
> >  }
> >  
> > +static void __iomem *irs_base;
> 
> Global state?

Yup, done.

> 
> > +
> > +static u32 irs_readl_relaxed(const u32 reg_offset)
> > +{
> > +	return readl_relaxed(irs_base + reg_offset);
> > +}
> > +
> > +static int gicv5_irs_extract_vm_caps(const struct gic_kvm_info
> > *info)
> > +{
> > +	u64 idr;
> > +
> > +	irs_base = info->gicv5_irs.base;
> > +	if (!irs_base) {
> > +		kvm_info("No GICv5 MMIO IRS address; no GICv5
> > support\n");
> > +		return -ENODEV;
> > +	}
> 
> Should you instead bail out early by not registering the gic_kvm_info
> when the IRS base is unknown, making this sort of checks irrelevant?
> 
> Also, it's not like we can make it very far without an IRS...

In fact, we won't even make it this far (or even as far as registering
gic_kvm_info) if we fail to init the host's IRS. Have dropped this.

> 
> > +
> > +	vgic_v5_host_caps()->irs_non_coherent = info-
> > >gicv5_irs.non_coherent;
> > +
> > +	idr = irs_readl_relaxed(GICV5_IRS_IDR2);
> > +
> > +	/* We skip the LPI field as it only applies to physical
> > LPIs */
> > +	vgic_v5_host_caps()->ist_id_bits =
> > FIELD_GET(GICV5_IRS_IDR2_ID_BITS, idr);
> > +	vgic_v5_host_caps()->min_lpi_id_bits =
> > FIELD_GET(GICV5_IRS_IDR2_MIN_LPI_ID_BITS, idr);
> > +	vgic_v5_host_caps()->ist_levels =
> > !!FIELD_GET(GICV5_IRS_IDR2_IST_LEVELS, idr);
> > +	vgic_v5_host_caps()->ist_l2sz =
> > FIELD_GET(GICV5_IRS_IDR2_IST_L2SZ, idr);
> > +	vgic_v5_host_caps()->istmd =
> > !!FIELD_GET(GICV5_IRS_IDR2_ISTMD, idr);
> > +	vgic_v5_host_caps()->istmd_sz =
> > FIELD_GET(GICV5_IRS_IDR2_ISTMD_SZ, idr);
> > +
> > +	idr = irs_readl_relaxed(GICV5_IRS_IDR3);
> > +
> > +	vgic_v5_host_caps()->max_vms =
> > BIT(FIELD_GET(GICV5_IRS_IDR3_VM_ID_BITS, idr));
> > +	vgic_v5_host_caps()->two_level_vmt_support =
> > !!FIELD_GET(GICV5_IRS_IDR3_VMT_LEVELS, idr);
> > +
> > +	if (FIELD_GET(GICV5_IRS_IDR3_VMD, idr))
> 
> The constant (ab)use of FIELD_GET() for fields that are single bit
> wide is very hard to read. I'd like to see:
> 
> 	vgic_v5_host_caps()->ist_levels = (idr &
> GICV5_IRS_IDR2_IST_LEVELS);
> [...]
> 	vgic_v5_host_caps()->istmd = (idr & GICV5_IRS_IDR2_ISTMD);
> [...]
> 	if (idr & GICV5_IRS_IDR3_VMD)
> [...]
> 
> which is infinitely more readable.

Done.

> 
> > +		vgic_v5_host_caps()->vmd_size =
> > BIT(FIELD_GET(GICV5_IRS_IDR3_VMD_SZ, idr));
> > +	else
> > +		vgic_v5_host_caps()->vmd_size = 0;
> > +
> > +	idr = irs_readl_relaxed(GICV5_IRS_IDR4);
> > +
> > +	vgic_v5_host_caps()->vped_size =
> > BIT(FIELD_GET(GICV5_IRS_IDR4_VPED_SZ, idr));
> > +	/* Field stores VPE_ID_BITS - 1 */
> > +	vgic_v5_host_caps()->max_vpes =
> > BIT(FIELD_GET(GICV5_IRS_IDR4_VPE_ID_BITS, idr) + 1);
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Probe for a vGICv5 compatible interrupt controller, returning 0
> > on success.
> >   */
> > @@ -61,6 +110,9 @@ int vgic_v5_probe(const struct gic_kvm_info
> > *info)
> >  		goto skip_v5;
> >  	}
> >  
> > +	if (gicv5_irs_extract_vm_caps(info))
> > +		goto skip_v5;
> > +
> 
> We shouldn't "skip_v5" anymore. If we can't initialise KVM with
> GICv5,
> we're done, and we should not even try to register v3.

Agreed, with the exception of pKVM. In that case, we only support
vGICv3 on (compatible) GICv5 hardware. In all other cases, it makes
sense to fail outright if we can't correctly init vGICv5.

I'll update it to error out on all cases aside from that one.

> 
> Thanks,
> 
> 	M.
>