[PATCH 07/32] KVM: arm64: gic: Introduce interrupt type helpers
Sascha Bischoff
Sascha.Bischoff at arm.com
Tue Dec 16 00:57:02 PST 2025
On Mon, 2025-12-15 at 16:05 +0000, Marc Zyngier wrote:
> On Mon, 15 Dec 2025 13:32:04 +0000,
> Marc Zyngier <maz at 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.
Thanks, Marc!
I do think that this is better overall. It removes the naming issues as
you say, and makes the whole situation more readable (and adds in SGI
support in those checks).
It does mean that we're still passing a non-vcpu pointer for the
private irqs (PPIs, SGIs) but else it gets quite inconsistent. Going
through the code we do actually have one place where we check for a PPI
without having a vcpu - setting the GICv3 maint IRQ. I guess that it is
worth paying the price of having vcpu->kvm in a few places for the sake
of consistency.
Will work these changes into the patch series (including cleaning up
the gic type checks).
Thanks,
Sascha
>
> 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 */
>
More information about the linux-arm-kernel
mailing list