[PATCH v2 07/36] KVM: arm64: gic: Introduce interrupt type helpers

[Sascha Bischoff]

On Tue, 2026-01-06 at 18:43 +0000, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 15:52:38 +0000
> Sascha Bischoff <Sascha.Bischoff at arm.com> wrote:
> 
> > GICv5 has moved from using interrupt ranges for different interrupt
> > types to using some of the upper bits of the interrupt ID to denote
> > the interrupt type. This is not compatible with older GICs (which
> > rely
> > on ranges of interrupts to determine the type), and hence a set of
> > helpers is introduced. These helpers take a struct kvm*, and use
> > the
> > vgic model to determine how to interpret the interrupt ID.
> > 
> > Helpers are introduced for PPIs, SPIs, and LPIs. Additionally, a
> > helper is introduced to determine if an interrupt is private - SGIs
> > and PPIs for older GICs, and PPIs only for GICv5.
> > 
> > The helpers are plumbed into the core vgic code, as well as the
> > Arch
> > Timer and PMU code.
> > 
> > There should be no functional changes as part of this change.
> > 
> > Signed-off-by: Sascha Bischoff <sascha.bischoff at arm.com>
> Hi Sascha,
> 
> A bit of bikeshedding / 'valuable' naming feedback to end the day.
> Otherwise LGTM.
> 
> > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> > index b261fb3968d03..6778f676eaf08 100644
> > --- a/include/kvm/arm_vgic.h
> > +++ b/include/kvm/arm_vgic.h
> ...
> 
> >  enum vgic_type {
> >  	VGIC_V2,		/* Good ol' GICv2 */
> > @@ -418,8 +488,12 @@ u64 vgic_v3_get_misr(struct kvm_vcpu *vcpu);
> >  
> >  #define irqchip_in_kernel(k)	(!!((k)->arch.vgic.in_kernel))
> >  #define vgic_initialized(k)	((k)->arch.vgic.initialized)
> > -#define vgic_valid_spi(k, i)	(((i) >= VGIC_NR_PRIVATE_IRQS) &&
> > \
> > +#define vgic_valid_nv5_spi(k, i)	(((i) >=
> > VGIC_NR_PRIVATE_IRQS) && \
> >  			((i) < (k)->arch.vgic.nr_spis +
> > VGIC_NR_PRIVATE_IRQS))
> > +#define vgic_valid_v5_spi(k, i)	(irq_is_spi(k, i) && \
> > +				 (FIELD_GET(GICV5_HWIRQ_ID, i) <
> > (k)->arch.vgic.nr_spis))
> > +#define vgic_valid_spi(k, i) (vgic_is_v5(k)
> > ?				\
> > +			      vgic_valid_v5_spi(k, i) :
> > vgic_valid_nv5_spi(k, i))
> 
> nv is a little awkward as a name as immediately makes me thinking
> nested virtualization instead of not v5 (which I guess is the
> thinking behind that?)
> 
> Probably just me and naming it v23 will break if we get to GIC
> version 23 :)
> nv5 breaks when we get GICv6 ;)

Absolutely agreed here. The v5 and nv5 macros were not used anywhere,
so I've re-worked this a bit to be more in the style of those added
earlier:

-#define vgic_valid_nv5_spi(k, i)       (((i) >= VGIC_NR_PRIVATE_IRQS) && \
-                       ((i) < (k)->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS))
-#define vgic_valid_v5_spi(k, i)        (irq_is_spi(k, i) && \
-                                (FIELD_GET(GICV5_HWIRQ_ID, i) < (k)->arch.vgic.nr_spis))
-#define vgic_valid_spi(k, i) (vgic_is_v5(k) ?                          \
-                             vgic_valid_v5_spi(k, i) : vgic_valid_nv5_spi(k, i))
+#define vgic_valid_spi(k, i)                                           \
+       ({                                                              \
+               bool __ret = irq_is_spi(k, i);                          \
+                                                                       \
+               switch ((k)->arch.vgic.vgic_model) {                    \
+               case KVM_DEV_TYPE_ARM_VGIC_V5:                          \
+                       __ret &= FIELD_GET(GICV5_HWIRQ_ID, i) < (k)->arch.vgic.nr_spis; \
+                       break;                                          \
+               default:                                                \
+                       __ret &= (i) < ((k)->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS); \
+               }                                                       \
+                                                                       \
+               __ret;                                                  \
+       })

More verbose (with annoying line lengths), but certainly more scalable
and removes the naming issue altogether. Personally, I prefer it
because it is more aligned with the related macros above.

Is this preferable/acceptable?

Thanks,
Sascha

> 
> 
> >  
> >  bool kvm_vcpu_has_pending_irqs(struct kvm_vcpu *vcpu);
> >  void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu);
>