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

Marc Zyngier
2026-04-28T15:20:36Z

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.

> +
> +#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?

> +
> +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...

> +
> + 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.

> + 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.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.