Re: [PATCH v3 26/40] KVM: arm64: gic-v5: Add GIC VDPEND and GIC VDRCFG hyp calls

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-07-03 at 16:31 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] Missing Context Synchronization Event (ISB) after writing to
> the VDPEND system register.
> --
> 
> commit 3d6631fff5db53c3f7f6517ea282e7e87b142f9a
> Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
> 
> KVM: arm64: gic-v5: Add GIC VDPEND and GIC VDRCFG hyp calls
> 
> This commit introduces hypercalls to manage SPI and LPI pending
> states for a
> VM using the GIC VDPEND and GIC VDRCFG system instructions. It wraps
> these
> EL2-only instructions for NVHE/hVHE configurations to allow the
> hypervisor
> to inject interrupts and query deactivated states.
> 
> [ ... ]
> 
> > diff --git a/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > index 46992a6c2cacb..c50e6ae93ba3f 100644
> > --- a/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > +++ b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> > @@ -149,3 +149,23 @@ void __vgic_v5_restore_state(struct
> > vgic_v5_cpu_if *cpu_if)
> >  {
> >  	write_sysreg_s(cpu_if->vgic_icsr, SYS_ICC_ICSR_EL1);
> >  }
> > +
> > +void __vgic_v5_vdpend(u32 intid, bool pending, u16 vm)
> > +{
> > +	u64 value;
> > +
> > +	value = intid & (GICV5_GIC_VDPEND_ID_MASK |
> > GICV5_GIC_VDPEND_TYPE_MASK);
> > +	value |= FIELD_PREP(GICV5_GIC_VDPEND_PENDING_MASK,
> > pending);
> > +	value |= FIELD_PREP(GICV5_GIC_VDPEND_VM_MASK, vm);
> > +	gic_insn(value, VDPEND);
> > +}
> 
> [Severity: High]
> Is a context synchronization event needed immediately after this
> system register
> write?

No. We're making an SPI (or in the future LPI) pending(which goes out
over GICStream to the IRS so the last thing we want to do is to wait
around for that to complete. The hardware will signal the interrupt to
the guest (or generate a doorbell) at some point in the future when the
interrupt is selected as the HPPI for the guest.

> 
> When __vgic_v5_vdpend() completes the write via gic_insn(), it
> returns directly.
> Without an explicit isb() before returning, could the CPU pipeline be
> left in
> an undefined architectural state for subsequent instructions?

Please stop this "undefined architectural state" stuff for the CPU
pipeline. It doesn't make any sense.

> 
> For comparison, the __vgic_v5_vdrcfg() implementation in this same
> patch
> includes an isb() immediately after its gic_insn() call.

Correct, it does. There's a good reason for it too. We need to be sure
that the VDRCFG has completed before we read from the register that it
populates (ICC_ICSR_EL1). Else, we'd read stale state. This one example
doesn't mean that all GIC instructions need to be treated like that!

> 
> [ ... ]
> 
Thanks,
Sascha