Re: [PATCH v4 07/48] KVM: arm64: gic-v5: Extract host IRS caps from IRS config frame

On Sat, 2026-07-25 at 11:40 +0100, Marc Zyngier wrote:
> On Fri, 24 Jul 2026 11:50:13 +0100,
> Sascha Bischoff <[email protected]> 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 <[email protected]>
> > ---
> >  arch/arm64/kvm/vgic/vgic-v5.c | 46
> > +++++++++++++++++++++++++++++++++--
> >  include/kvm/arm_vgic.h        | 26 ++++++++++++++++++++
> >  2 files changed, 70 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c
> > b/arch/arm64/kvm/vgic/vgic-v5.c
> > index d4789ff3e7402..3f7b132110114 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v5.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> > @@ -11,6 +11,7 @@
> >  #include "vgic.h"
> >  
> >  #define ppi_caps	kvm_vgic_global_state.vgic_v5_ppi_caps
> > +#define irs_caps	kvm_vgic_global_state.vgic_v5_irs_caps
> >  
> >  /*
> >   * Not all PPIs are guaranteed to be implemented for GICv5.
> > Deterermine which
> > @@ -34,6 +35,45 @@ static void vgic_v5_get_implemented_ppis(void)
> >  	__assign_bit(GICV5_ARCH_PPI_PMUIRQ,
> > ppi_caps.impl_ppi_mask, system_supports_pmuv3());
> >  }
> >  
> > +static u32 irs_readl_relaxed(const u32 reg_offset)
> > +{
> > +	return readl_relaxed(irs_caps.irs_base + reg_offset);
> > +}
> > +
> > +static void vgic_v5_irs_extract_vm_caps(const struct gic_kvm_info
> > *info)
> > +{
> > +	u64 idr;
> > +
> > +	irs_caps.irs_base = info->gicv5_irs.base;
> > +	irs_caps.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 */
> > +	irs_caps.ist_id_bits = FIELD_GET(GICV5_IRS_IDR2_ID_BITS,
> > idr);
> > +	irs_caps.min_lpi_id_bits =
> > FIELD_GET(GICV5_IRS_IDR2_MIN_LPI_ID_BITS, idr);
> > +	irs_caps.ist_levels = (idr & GICV5_IRS_IDR2_IST_LEVELS);
> > +	irs_caps.ist_l2sz = FIELD_GET(GICV5_IRS_IDR2_IST_L2SZ,
> > idr);
> > +	irs_caps.istmd = (idr & GICV5_IRS_IDR2_ISTMD);
> > +	irs_caps.istmd_sz = FIELD_GET(GICV5_IRS_IDR2_ISTMD_SZ,
> > idr);
> > +
> > +	idr = irs_readl_relaxed(GICV5_IRS_IDR3);
> > +
> > +	irs_caps.max_vms =
> > BIT(FIELD_GET(GICV5_IRS_IDR3_VM_ID_BITS, idr));
> > +	irs_caps.two_level_vmt_support = (idr &
> > GICV5_IRS_IDR3_VMT_LEVELS);
> > +
> > +	if (idr & GICV5_IRS_IDR3_VMD)
> > +		irs_caps.vmd_size =
> > BIT(FIELD_GET(GICV5_IRS_IDR3_VMD_SZ, idr));
> > +	else
> > +		irs_caps.vmd_size = 0;
> > +
> > +	idr = irs_readl_relaxed(GICV5_IRS_IDR4);
> > +
> > +	irs_caps.vped_size = BIT(FIELD_GET(GICV5_IRS_IDR4_VPED_SZ,
> > idr));
> > +	/* Field stores VPE_ID_BITS - 1 */
> > +	irs_caps.max_vpes =
> > BIT(FIELD_GET(GICV5_IRS_IDR4_VPE_ID_BITS, idr) + 1);
> 
> Not a big deal, but I'm a bit over this split of ID regs in
> individual
> fields. It looks appealing at first, but ends-up being problematic.
> 
> The reason for this is that EL2 doesn't map kvm_vgic_global_state,
> which will eventually force pKVM to either duplicate the structure,
> or
> access the ID reg directly (the latter resulting in traps under NV).
> 
> My preference would be to only cache the raw ID reg values, and have
> inline accessors for the individual fields. Once this is in place, we
> can patch the ID reg values in the code directly, ICH_VTR_EL2-style.

Given that I need to post a v5 in either case, I've gone ahead and made
this change. We now cache the values of IDR2/3/4 in
kvm_vgic_global_state and have a set of helpers to extract their values
at the points where we need them.

> 
> Anyway, something to think about.
> 
> Thanks,
> 
> 	M.
> 
Thanks,
Sascha