Re: [PATCH v5 17/36] KVM: arm64: gic-v5: Finalize GICv5 PPIs and generate mask

Sascha Bischoff
2026-03-04T17:39:47Z

On Wed, 2026-03-04 at 10:50 +0000, Marc Zyngier wrote:
> On Thu, 26 Feb 2026 15:59:48 +0000,
> Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> >
> > We only want to expose a subset of the PPIs to a guest. If a PPI
> > does
> > not have an owner, it is not being actively driven by a device. The
> > SW_PPI is a special case, as it is likely for userspace to wish to
> > inject that.
> >
> > Therefore, just prior to running the guest for the first time, we
> > need
> > to finalize the PPIs. A mask is generated which, when combined with
> > trapping a guest's PPI accesses, allows for the guest's view of the
> > PPI to be filtered. This mask is global to the VM as all VCPUs PPI
> > configurations must match.
> >
> > In addition, the PPI HMR is calculated.
> >
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > ---
> >  arch/arm64/kvm/arm.c               |  4 +++
> >  arch/arm64/kvm/vgic/vgic-v5.c      | 46
> > ++++++++++++++++++++++++++++++
> >  include/kvm/arm_vgic.h             |  9 ++++++
> >  include/linux/irqchip/arm-gic-v5.h | 17 +++++++++++
> >  4 files changed, 76 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index eb2ca65dc7297..8290c5df0616e 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -935,6 +935,10 @@ int kvm_arch_vcpu_run_pid_change(struct
> > kvm_vcpu *vcpu)
> >   return ret;
> >   }
> >
> > + ret = vgic_v5_finalize_ppi_state(kvm);
> > + if (ret)
> > + return ret;
> > +
> >   if (is_protected_kvm_enabled()) {
> >   ret = pkvm_create_hyp_vm(kvm);
> >   if (ret)
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c
> > b/arch/arm64/kvm/vgic/vgic-v5.c
> > index f5cd9decfc26e..db2225aefb130 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v5.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> > @@ -86,6 +86,52 @@ int vgic_v5_probe(const struct gic_kvm_info
> > *info)
> >   return 0;
> >  }
> >
> > +int vgic_v5_finalize_ppi_state(struct kvm *kvm)
> > +{
> > + struct kvm_vcpu *vcpu;
> > +
> > + if (!vgic_is_v5(kvm))
> > + return 0;
> > +
> > + /* The PPI state for all VCPUs should be the same. Pick
> > the first. */
> > + vcpu = kvm_get_vcpu(kvm, 0);
> > +
> > + vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_mask[0] = 0;
> > + vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_mask[1] = 0;
> > + vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_hmr[0] = 0;
> > + vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_hmr[1] = 0;
>
> vcpu->kvm == kvm. You don't need the indirection (same in most of the
> function).

Ah, that was rather silly of me... Fixed, thanks!
>
> > +
> > + for (int i = 0; i < VGIC_V5_NR_PRIVATE_IRQS; i++) {
> > + int reg = i / 64;
> > + u64 bit = BIT_ULL(i % 64);
> > + struct vgic_irq *irq = &vcpu-
> > >arch.vgic_cpu.private_irqs[i];
>
> vgic_get_vcpu_irq()?

I've changed it to this (it is safer to say the least), but we do the
lookup with the full GICv5 IntID, so we have to build that for the
purposes of the lookup itself which feels a tad silly. Still better to
use the proper interface though.

>
> > +
> > + guard(raw_spinlock_irqsave)(&irq->irq_lock);
> > +
> > + /*
> > + * We only expose PPIs with an owner or the SW_PPI
> > to the
> > + * guest.
> > + */
> > + if (!irq->owner &&
> > +     FIELD_GET(GICV5_HWIRQ_ID, irq->intid) !=
> > GICV5_ARCH_PPI_SW_PPI)
> > + continue;
>
> This sort of construct is rather cumbersome, and I see it replicated
> in quite a few places. How about introducing a couple of basic
> accessors:
>
> #define vgic_v5_get_hwirq_id(x) FIELD_GET(GICV5_HWIRQ_ID, (x))
> #define vgic_v5_set_hwirq_id(x) FIELD_PREP(GICV5_HWIRQ_ID, (x))
>
> which is a bit easier on the eye?

I've added those, and some to make raw IDs into PPIs, SPIs, LPIs (so,
setting the top bits to the correct type, effectively) to "KVM: arm64:
gic: Introduce interrupt type helpers". Will work them into the
relevant places in the series.

>
> > +
> > + /*
> > + * If the PPI isn't implemented, we can't pass it
> > through to a
> > + * guest anyhow.
> > + */
> > + if (!(ppi_caps.impl_ppi_mask[reg] & bit))
> > + continue;
> > +
> > + vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_mask[reg]
> > |= bit;
> > +
> > + if (irq->config == VGIC_CONFIG_LEVEL)
> > + vcpu->kvm-
> > >arch.vgic.gicv5_vm.vgic_ppi_hmr[reg] |= bit;
> > + }
> > +
> > + return 0;
> > +}
> > +
> >  /*
> >   * Sets/clears the corresponding bit in the ICH_PPI_DVIR register.
> >   */
> > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> > index d828861f8298a..a4416afca5efc 100644
> > --- a/include/kvm/arm_vgic.h
> > +++ b/include/kvm/arm_vgic.h
> > @@ -32,6 +32,8 @@
> >  #define VGIC_MIN_LPI 8192
> >  #define KVM_IRQCHIP_NUM_PINS (1020 - 32)
> >
> > +#define VGIC_V5_NR_PRIVATE_IRQS 128
> > +
> >  #define is_v5_type(t, i) (FIELD_GET(GICV5_HWIRQ_TYPE, (i))
> > == (t))
> >
> >  #define __irq_is_sgi(t,
> > i) \
> > @@ -381,6 +383,11 @@ struct vgic_dist {
> >   * else.
> >   */
> >   struct its_vm its_vm;
> > +
> > + /*
> > + * GICv5 per-VM data.
> > + */
> > + struct gicv5_vm gicv5_vm;
>
> Depending how this grows, we may have to move that as part of a union
> with the previous member (which is obviously v4 specific).

Makes sense. I'll leave this for the time being, but it feels like
something we should definitely consider going forward.

Thanks,
Sascha

>
> >  };
> >
> >  struct vgic_v2_cpu_if {
> > @@ -567,6 +574,8 @@ int vgic_v4_load(struct kvm_vcpu *vcpu);
> >  void vgic_v4_commit(struct kvm_vcpu *vcpu);
> >  int vgic_v4_put(struct kvm_vcpu *vcpu);
> >
> > +int vgic_v5_finalize_ppi_state(struct kvm *kvm);
> > +
> >  bool vgic_state_is_nested(struct kvm_vcpu *vcpu);
> >
> >  /* CPU HP callbacks */
> > diff --git a/include/linux/irqchip/arm-gic-v5.h
> > b/include/linux/irqchip/arm-gic-v5.h
> > index 3e838a3058861..30a1b656daa35 100644
> > --- a/include/linux/irqchip/arm-gic-v5.h
> > +++ b/include/linux/irqchip/arm-gic-v5.h
> > @@ -380,6 +380,23 @@ struct gicv5_vpe {
> >   bool resident;
> >  };
> >
> > +struct gicv5_vm {
> > + /*
> > + * We only expose a subset of PPIs to the guest. This
> > subset
> > + * is a combination of the PPIs that are actually
> > implemented
> > + * and what we actually choose to expose.
> > + */
> > + u64 vgic_ppi_mask[2];
> > +
> > + /*
> > + * The HMR itself is handled by the hardware, but we still
> > need to have
> > + * a mask that we can use when merging in pending state
> > (only the state
> > + * of Edge PPIs is merged back in from the guest an the
> > HMR provides a
> > + * convenient way to do that).
> > + */
> > + u64 vgic_ppi_hmr[2];
> > +};
> > +
> >  struct gicv5_its_devtab_cfg {
> >   union {
> >   struct {
>
> Thanks,
>
> M.
>