[PATCH 15/32] KVM: arm64: gic-v5: Implement direct injection of PPIs
Sascha Bischoff
Sascha.Bischoff at arm.com
Wed Jan 7 06:50:38 PST 2026
On Wed, 2025-12-17 at 11:40 +0000, Marc Zyngier wrote:
> On Fri, 12 Dec 2025 15:22:40 +0000,
> Sascha Bischoff <Sascha.Bischoff at arm.com> wrote:
> >
> > GICv5 is able to directly inject PPI pending state into a guest
> > using
> > a mechanism called DVI whereby the pending bit for a paticular PPI
> > is
> > driven directly by the physically-connected hardware. This
> > mechanism
> > itself doesn't allow for any ID translation, so the host interrupt
> > is
> > directly mapped into a guest with the same interrupt ID.
> >
> > When mapping a virtual interrupt to a physical interrupt via
> > kvm_vgic_map_irq for a GICv5 guest, check if the interrupt itself
> > is a
> > PPI or not. If it is, and the host's interrupt ID matches that used
> > for the guest DVI is enabled, and the interrupt itself is marked as
> > directly_injected.
> >
> > When the interrupt is unmapped again, this process is reversed, and
> > DVI is disabled for the interrupt again.
> >
> > Note: the expectation is that a directly injected PPI is disabled
> > on
> > the host while the guest state is loaded. The reason is that
> > although
> > DVI is enabled to drive the guest's pending state directly, the
> > host
> > pending state also remains driven. In order to avoid the same PPI
> > firing on both the host and the guest, the host's interrupt must be
> > disabled (masked). This is left up to the code that owns the device
> > generating the PPI as this needs to be handled on a per-VM basis.
> > One
> > VM might use DVI, while another might not, in which case the
> > physical
> > PPI should be enabled for the latter.
> >
> > Co-authored-by: Timothy Hayes <timothy.hayes at arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes at arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff at arm.com>
> > ---
> > arch/arm64/kvm/vgic/vgic-v5.c | 22 ++++++++++++++++++++++
> > arch/arm64/kvm/vgic/vgic.c | 16 ++++++++++++++++
> > arch/arm64/kvm/vgic/vgic.h | 1 +
> > include/kvm/arm_vgic.h | 1 +
> > 4 files changed, 40 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c
> > b/arch/arm64/kvm/vgic/vgic-v5.c
> > index 2fb2db23ed39a..22558080711eb 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v5.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> > @@ -54,6 +54,28 @@ int vgic_v5_probe(const struct gic_kvm_info
> > *info)
> > return 0;
> > }
> >
> > +/*
> > + * Sets/clears the corresponding bit in the ICH_PPI_DVIR register.
> > + */
> > +int vgic_v5_set_ppi_dvi(struct kvm_vcpu *vcpu, u32 irq, bool dvi)
> > +{
> > + struct vgic_v5_cpu_if *cpu_if = &vcpu-
> > >arch.vgic_cpu.vgic_v5;
> > + u32 ppi = FIELD_GET(GICV5_HWIRQ_ID, irq);
> > +
> > + if (ppi >= 128)
> > + return -EINVAL;
>
> Surely this is bad. *very* bad. How can we get here the first place?
Yeah, this would absolutely be bad. I don't believe that we can even
hit this anymore, so I've dropped it.
>
> > +
> > + if (dvi) {
> > + /* Set the bit */
> > + cpu_if->vgic_ppi_dvir[ppi / 64] |= 1UL << (ppi %
> > 64);
> > + } else {
> > + /* Clear the bit */
> > + cpu_if->vgic_ppi_dvir[ppi / 64] &= ~(1UL << (ppi %
> > 64));
> > + }
>
> This should be simplified:
>
> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c
> b/arch/arm64/kvm/vgic/vgic-v5.c
> index d74cc3543b9a4..f434ee85f7e1a 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -191,8 +191,8 @@ bool vgic_v5_ppi_set_pending_state(struct
> kvm_vcpu *vcpu,
> struct vgic_irq *irq)
> {
> struct vgic_v5_cpu_if *cpu_if;
> - const u32 id_bit = BIT_ULL(irq->intid % 64);
> const u32 reg = FIELD_GET(GICV5_HWIRQ_ID, irq->intid) / 64;
> + unsigned long *p;
>
> if (!vcpu || !irq)
> return false;
> @@ -203,10 +203,8 @@ bool vgic_v5_ppi_set_pending_state(struct
> kvm_vcpu *vcpu,
>
> cpu_if = &vcpu->arch.vgic_cpu.vgic_v5;
>
> - if (irq_is_pending(irq))
> - cpu_if->vgic_ppi_pendr[reg] |= id_bit;
> - else
> - cpu_if->vgic_ppi_pendr[reg] &= ~id_bit;
> + p = (unsigned long *)&cpu_if->vgic_ppi_pendr[reg];
> + __assign_bit(irq->intid % 64, p, irq_is_pending(irq));
>
> return true;
> }
I've made this change in the corresponding commit (impl PPI injection)
- thanks.
> @@ -449,17 +447,13 @@ int vgic_v5_set_ppi_dvi(struct kvm_vcpu *vcpu,
> u32 irq, bool dvi)
> {
> struct vgic_v5_cpu_if *cpu_if = &vcpu-
> >arch.vgic_cpu.vgic_v5;
> u32 ppi = FIELD_GET(GICV5_HWIRQ_ID, irq);
> + unsigned long *p;
>
> if (ppi >= 128)
> return -EINVAL;
>
> - if (dvi) {
> - /* Set the bit */
> - cpu_if->vgic_ppi_dvir[ppi / 64] |= 1UL << (ppi %
> 64);
> - } else {
> - /* Clear the bit */
> - cpu_if->vgic_ppi_dvir[ppi / 64] &= ~(1UL << (ppi %
> 64));
> - }
> + p = (unsigned long *)&cpu_if->vgic_ppi_dvir[ppi / 64];
> + __assign_bit(ppi % 64, p, dvi);
>
> return 0;
> }
>
> (yes, unsigned long and u64 are the same thing on any sane
> architecture).
Done, thanks!
>
> > +
> > + return 0;
> > +}
> > +
> > void vgic_v5_load(struct kvm_vcpu *vcpu)
> > {
> > struct vgic_v5_cpu_if *cpu_if = &vcpu-
> > >arch.vgic_cpu.vgic_v5;
> > diff --git a/arch/arm64/kvm/vgic/vgic.c
> > b/arch/arm64/kvm/vgic/vgic.c
> > index 1005ff5f36235..1fe3dcc997860 100644
> > --- a/arch/arm64/kvm/vgic/vgic.c
> > +++ b/arch/arm64/kvm/vgic/vgic.c
> > @@ -577,12 +577,28 @@ static int kvm_vgic_map_irq(struct kvm_vcpu
> > *vcpu, struct vgic_irq *irq,
> > irq->host_irq = host_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 (FIELD_GET(GICV5_HWIRQ_ID, irq->intid) == irq-
> > >hwintid) {
> > + if (!vgic_v5_set_ppi_dvi(vcpu, irq-
> > >hwintid, true))
> > + irq->directly_injected = true;
>
> The error handling gives me the creeps. If we can end-up at this
> stage
> with the wrong INTID, we're screwed.
Yeah, valid point, it would indeed be rather broken if we ever made it
here. I've dropped this check as I don't think it actually makes sense
anymore anyhow.
This is used for the arch timer. For a non-nested guest, there should
be a 1:1 relationship between the host and guest timer PPIs. If that
isn't the case, the arch timer code has gone awry by not using the
architected ID. For a nested guest, there's actually an explicit PPI ID
change (e.g. host PPI 28 is directly injected as virtual PPI 28) which
would incorrectly fail this check.
Thanks,
Sascha
>
> > + }
> > + }
> > +
> > return 0;
> > }
> >
> > /* @irq->irq_lock must be held */
> > static inline void kvm_vgic_unmap_irq(struct vgic_irq *irq)
> > {
> > + if (irq->directly_injected && vgic_is_v5(irq->target_vcpu-
> > >kvm))
> > + WARN_ON(vgic_v5_set_ppi_dvi(irq->target_vcpu, irq-
> > >hwintid, false));
> > +
> > + irq->directly_injected = false;
> > irq->hw = false;
> > irq->hwintid = 0;
> > irq->ops = NULL;
> > diff --git a/arch/arm64/kvm/vgic/vgic.h
> > b/arch/arm64/kvm/vgic/vgic.h
> > index 6e1f386dffade..b6e3f5e3aba18 100644
> > --- a/arch/arm64/kvm/vgic/vgic.h
> > +++ b/arch/arm64/kvm/vgic/vgic.h
> > @@ -363,6 +363,7 @@ void vgic_debug_init(struct kvm *kvm);
> > void vgic_debug_destroy(struct kvm *kvm);
> >
> > int vgic_v5_probe(const struct gic_kvm_info *info);
> > +int vgic_v5_set_ppi_dvi(struct kvm_vcpu *vcpu, u32 irq, bool dvi);
> > void vgic_v5_load(struct kvm_vcpu *vcpu);
> > void vgic_v5_put(struct kvm_vcpu *vcpu);
> > void vgic_v5_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr
> > *vmcr);
> > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> > index 45d83f45b065d..ce9e149b85a58 100644
> > --- a/include/kvm/arm_vgic.h
> > +++ b/include/kvm/arm_vgic.h
> > @@ -163,6 +163,7 @@ struct vgic_irq {
> > bool enabled:1;
> > bool active:1;
> > bool hw:1; /* Tied to HW IRQ */
> > + bool directly_injected:1; /* A directly injected HW
> > IRQ */
> > bool on_lr:1; /* Present in a CPU LR */
> > refcount_t refcount; /* Used for LPIs */
> > u32 hwintid; /* HW INTID number */
>
> Thanks,
>
> M.
>
More information about the linux-arm-kernel
mailing list