Re: [PATCH 07/32] KVM: arm64: gic: Introduce interrupt type helpers

[Marc Zyngier <maz@kernel.org>]

On Mon, 15 Dec 2025 13:32:04 +0000,
Marc Zyngier <maz@kernel.org> wrote:
> 
> I'd rather you have something that denotes the non-GICv5-ness of the
> implementation. irq_is_nv5_ppi()?

Actually, this is just as bad. I spent the past 30 minutes hacking on
this, and came up with this hack (which compiles, but probably doesn't
run). It is significantly more code, but I like that it treats all GIC
implementations more or less the same way.

It also clears the who [v]gic_is_v5() situation that made little
sense.

Let me know what you think.

	M.

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index b0a5a6c6bf8da..c908d5ac4d678 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -62,11 +62,6 @@ static struct irq_ops arch_timer_irq_ops_vgic_v5 = {
 	.queue_irq_unlock = vgic_v5_ppi_queue_irq_unlock,
 };
 
-static bool vgic_is_v5(struct kvm_vcpu *vcpu)
-{
-	return vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V5;
-}
-
 static int nr_timers(struct kvm_vcpu *vcpu)
 {
 	if (!vcpu_has_nv(vcpu))
@@ -708,7 +703,7 @@ static void kvm_timer_vcpu_load_gic(struct arch_timer_context *ctx)
 
 	phys_active |= ctx->irq.level;
 
-	if (!vgic_is_v5(vcpu))
+	if (!vgic_is_v5(vcpu->kvm))
 		set_timer_irq_phys_active(ctx, phys_active);
 	else
 		set_timer_irq_phys_masked(ctx, true);
@@ -760,7 +755,7 @@ static void kvm_timer_vcpu_load_nested_switch(struct kvm_vcpu *vcpu,
 	if (!irqchip_in_kernel(vcpu->kvm))
 		return;
 
-	ops = vgic_is_v5(vcpu) ? &arch_timer_irq_ops_vgic_v5 :
+	ops = vgic_is_v5(vcpu->kvm) ? &arch_timer_irq_ops_vgic_v5 :
 				 &arch_timer_irq_ops;
 
 	/*
@@ -905,7 +900,7 @@ void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)
 
 	if (static_branch_likely(&has_gic_active_state)) {
 		/* We don't do NV on GICv5, yet */
-		if (vcpu_has_nv(vcpu) && !vgic_is_v5(vcpu))
+		if (vcpu_has_nv(vcpu) && !vgic_is_v5(vcpu->kvm))
 			kvm_timer_vcpu_load_nested_switch(vcpu, &map);
 
 		kvm_timer_vcpu_load_gic(map.direct_vtimer);
@@ -977,7 +972,7 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
 		kvm_timer_blocking(vcpu);
 
 	/* Unmask again on GICV5 */
-	if (vgic_is_v5(vcpu)) {
+	if (vgic_is_v5(vcpu->kvm)) {
 		set_timer_irq_phys_masked(map.direct_vtimer, false);
 
 		if (map.direct_ptimer)
@@ -1623,7 +1618,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
 		return -EINVAL;
 	}
 
-	ops = vgic_is_v5(vcpu) ? &arch_timer_irq_ops_vgic_v5 :
+	ops = vgic_is_v5(vcpu->kvm) ? &arch_timer_irq_ops_vgic_v5 :
 				 &arch_timer_irq_ops;
 
 	get_timer_map(vcpu, &map);
@@ -1700,7 +1695,7 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
 	 * The PPIs for the Arch Timers arch architecturally defined for
 	 * GICv5. Reject anything that changes them from the specified value.
 	 */
-	if (vgic_is_v5(vcpu) && vcpu->kvm->arch.timer_data.ppi[idx] != irq) {
+	if (vgic_is_v5(vcpu->kvm) && vcpu->kvm->arch.timer_data.ppi[idx] != irq) {
 		ret = -EINVAL;
 		goto out;
 	}
diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
index d74cc3543b9a4..19d8bf90f8f6c 100644
--- a/arch/arm64/kvm/vgic/vgic-v5.c
+++ b/arch/arm64/kvm/vgic/vgic-v5.c
@@ -225,7 +225,7 @@ bool vgic_v5_ppi_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
 
 	lockdep_assert_held(&irq->irq_lock);
 
-	if (WARN_ON_ONCE(!irq_is_ppi_v5(irq->intid)))
+	if (WARN_ON_ONCE(!__irq_is_ppi(KVM_DEV_TYPE_ARM_VGIC_V5, irq->intid)))
 		return false;
 
 	vcpu = irq->target_vcpu;
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 62d7d4c5650e4..6d8e4cd661734 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -109,10 +109,10 @@ struct vgic_irq *vgic_get_vcpu_irq(struct kvm_vcpu *vcpu, u32 intid)
 	if (WARN_ON(!vcpu))
 		return NULL;
 
-	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V5) {
+	if (vgic_is_v5(vcpu->kvm)) {
 		u32 int_num = FIELD_GET(GICV5_HWIRQ_ID, intid);
 
-		if (irq_is_ppi_v5(intid)) {
+		if (__irq_is_ppi(KVM_DEV_TYPE_ARM_VGIC_V5, intid)) {
 			int_num = array_index_nospec(int_num, VGIC_V5_NR_PRIVATE_IRQS);
 			return &vcpu->arch.vgic_cpu.private_irqs[int_num];
 		}
@@ -600,15 +600,13 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
 	irq->hwintid = data->hwirq;
 	irq->ops = ops;
 
-	if (vgic_is_v5(vcpu->kvm)) {
-		/* Nothing for us to do */
-		if (!irq_is_ppi_v5(irq->intid))
-			return 0;
+	if (vgic_is_v5(vcpu->kvm) &&
+	    !__irq_is_ppi(KVM_DEV_TYPE_ARM_VGIC_V5, irq->intid))
+		return 0;
 
-		if (FIELD_GET(GICV5_HWIRQ_ID, irq->intid) == irq->hwintid) {
-			if (!vgic_v5_set_ppi_dvi(vcpu, irq->hwintid, true))
-				irq->directly_injected = true;
-		}
+	if (FIELD_GET(GICV5_HWIRQ_ID, irq->intid) == irq->hwintid) {
+		if (!vgic_v5_set_ppi_dvi(vcpu, irq->hwintid, true))
+			irq->directly_injected = true;
 	}
 
 	return 0;
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 91969b3b80d04..d8a947a7eb941 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -500,11 +500,6 @@ static inline bool vgic_is_v3(struct kvm *kvm)
 	return kvm_vgic_global_state.type == VGIC_V3 || vgic_is_v3_compat(kvm);
 }
 
-static inline bool vgic_is_v5(struct kvm *kvm)
-{
-	return kvm_vgic_global_state.type == VGIC_V5 && !vgic_is_v3_compat(kvm);
-}
-
 bool system_supports_direct_sgis(void);
 bool vgic_supports_direct_msis(struct kvm *kvm);
 bool vgic_supports_direct_sgis(struct kvm *kvm);
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 6863e19d6eeb7..ae2897c539af7 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -36,22 +36,78 @@
 /* GICv5 constants */
 #define VGIC_V5_NR_PRIVATE_IRQS	128
 
-#define irq_is_ppi_legacy(irq) ((irq) >= VGIC_NR_SGIS && (irq) < VGIC_NR_PRIVATE_IRQS)
-#define irq_is_spi_legacy(irq) ((irq) >= VGIC_NR_PRIVATE_IRQS && \
-					(irq) <= VGIC_MAX_SPI)
-#define irq_is_lpi_legacy(irq) ((irq) > VGIC_MAX_SPI)
-
-#define irq_is_ppi_v5(irq) (FIELD_GET(GICV5_HWIRQ_TYPE, irq) == GICV5_HWIRQ_TYPE_PPI)
-#define irq_is_spi_v5(irq) (FIELD_GET(GICV5_HWIRQ_TYPE, irq) == GICV5_HWIRQ_TYPE_SPI)
-#define irq_is_lpi_v5(irq) (FIELD_GET(GICV5_HWIRQ_TYPE, irq) == GICV5_HWIRQ_TYPE_LPI)
-
-#define gic_is_v5(k) ((k)->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V5)
-
-#define irq_is_ppi(k, i) (gic_is_v5(k) ? irq_is_ppi_v5(i) : irq_is_ppi_legacy(i))
-#define irq_is_spi(k, i) (gic_is_v5(k) ? irq_is_spi_v5(i) : irq_is_spi_legacy(i))
-#define irq_is_lpi(k, i) (gic_is_v5(k) ? irq_is_lpi_v5(i) : irq_is_lpi_legacy(i))
-
-#define irq_is_private(k, i) (gic_is_v5(k) ? irq_is_ppi_v5(i) : i < VGIC_NR_PRIVATE_IRQS)
+#define is_v5_type(t, i)	(FIELD_GET(GICV5_HWIRQ_TYPE, (i)) == (t))
+
+#define __irq_is_sgi(t, i)						\
+	({								\
+		bool __ret;						\
+									\
+		switch (t) {						\
+		case KVM_DEV_TYPE_ARM_VGIC_V5:				\
+			__ret = false;					\
+			break;						\
+		default:						\
+			__ret  = (i) < VGIC_NR_SGIS;			\
+		}							\
+									\
+		__ret;							\
+	})
+
+#define __irq_is_ppi(t, i)						\
+	({								\
+		bool __ret;						\
+									\
+		switch (t) {						\
+		case KVM_DEV_TYPE_ARM_VGIC_V5:				\
+			__ret = is_v5_type(GICV5_HWIRQ_TYPE_PPI, (i));	\
+			break;						\
+		default:						\
+			__ret  = (i) >= VGIC_NR_SGIS;			\
+			__ret &= (i) < VGIC_NR_PRIVATE_IRQS;		\
+		}							\
+									\
+		__ret;							\
+	})
+
+#define __irq_is_spi(t, i)						\
+	({								\
+		bool __ret;						\
+									\
+		switch (t) {						\
+		case KVM_DEV_TYPE_ARM_VGIC_V5:				\
+			__ret = is_v5_type(GICV5_HWIRQ_TYPE_SPI, (i));	\
+			break;						\
+		default:						\
+			__ret  = (i) <= VGIC_MAX_SPI;			\
+			__ret &= (i) >= VGIC_NR_PRIVATE_IRQS;		\
+		}							\
+									\
+		__ret;							\
+	})
+
+#define __irq_is_lpi(t, i)						\
+	({								\
+		bool __ret;						\
+									\
+		switch (t) {						\
+		case KVM_DEV_TYPE_ARM_VGIC_V5:				\
+			__ret = is_v5_type(GICV5_HWIRQ_TYPE_LPI, (i));	\
+			break;						\
+		default:						\
+			__ret  = (i) >= 8192;				\
+		}							\
+									\
+		__ret;							\
+	})
+
+#define irq_is_sgi(k, i) __irq_is_sgi((k)->arch.vgic.vgic_model, i)
+#define irq_is_ppi(k, i) __irq_is_ppi((k)->arch.vgic.vgic_model, i)
+#define irq_is_spi(k, i) __irq_is_spi((k)->arch.vgic.vgic_model, i)
+#define irq_is_lpi(k, i) __irq_is_lpi((k)->arch.vgic.vgic_model, i)
+
+#define irq_is_private(k, i) (irq_is_ppi(k, i) || irq_is_sgi(k, i))
+
+#define vgic_is_v5(k) ((k)->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V5)
 
 enum vgic_type {
 	VGIC_V2,		/* Good ol' GICv2 */

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