Re: [PATCH v3 19/36] KVM: arm64: gic-v5: Implement PPI interrupt injection

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Fri, 9 Jan 2026 17:04:45 +0000
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:

> This change introduces interrupt injection for PPIs for GICv5-based
> guests.
> 
> The lifecycle of PPIs is largely managed by the hardware for a GICv5
> system. The hypervisor injects pending state into the guest by using
> the ICH_PPI_PENDRx_EL2 registers. These are used by the hardware to
> pick a Highest Priority Pending Interrupt (HPPI) for the guest based
> on the enable state of each individual interrupt. The enable state and
> priority for each interrupt are provided by the guest itself (through
> writes to the PPI registers).
> 
> When Direct Virtual Interrupt (DVI) is set for a particular PPI, the
> hypervisor is even able to skip the injection of the pending state
> altogether - it all happens in hardware.
> 
> The result of the above is that no AP lists are required for GICv5,
> unlike for older GICs. Instead, for PPIs the ICH_PPI_* registers
> fulfil the same purpose for all 128 PPIs. Hence, as long as the
> ICH_PPI_* registers are populated prior to guest entry, and merged
> back into the KVM shadow state on exit, the PPI state is preserved,
> and interrupts can be injected.
> 
> When injecting the state of a PPI the state is merged into the KVM's
> shadow state using the set_pending_state irq_op. The directly sets the
> relevant bit in the shadow ICH_PPI_PENDRx_EL2, which is presented to
> the guest (and GICv5 hardware) on next guest entry. The
> queue_irq_unlock irq_op is required to kick the vCPU to ensure that it
> seems the new state. The result is that no AP lists are used for
> private interrupts on GICv5.
> 
> Prior to entering the guest, vgic_v5_flush_ppi_state is called from
> kvm_vgic_flush_hwstate. The effectively snapshots the shadow PPI
> pending state (twice - an entry and an exit copy) in order to track
> any changes. These changes can come from a guest consuming an
> interrupt or from a guest making an Edge-triggered interrupt pending.
> 
> When returning from running a guest, the guest's PPI state is merged
> back into KVM's shadow state in vgic_v5_merge_ppi_state from
> kvm_vgic_sync_hwstate. The Enable and Active state is synced back for
> all PPIs, and the pending state is synced back for Edge PPIs (Level is
> driven directly by the devices generating said levels). The incoming
> pending state from the guest is merged with KVM's shadow state to
> avoid losing any incoming interrupts.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
Trivial naming thing inline. Either way
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  arch/arm64/kvm/vgic/vgic-v5.c | 160 ++++++++++++++++++++++++++++++++++
>  arch/arm64/kvm/vgic/vgic.c    |  40 +++++++--
>  arch/arm64/kvm/vgic/vgic.h    |  25 ++++--
>  3 files changed, 209 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index bf2c77bafa1d3..c1899add8f5c3 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -139,6 +139,166 @@ void vgic_v5_get_implemented_ppis(void)
>  		ppi_caps->impl_ppi_mask[0] |= BIT_ULL(GICV5_ARCH_PPI_PMUIRQ);
>  }
>  
> +static 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 = FIELD_GET(GICV5_HWIRQ_ID, irq->intid);
See below. This seems like reasonable naming choice where I suggest
adding a local variable. 
> +	unsigned long *p;
> +
> +	if (!vcpu || !irq)
> +		return false;
> +
> +	/*
> +	 * For DVI'd interrupts, the state is directly driven by the host
> +	 * hardware connected to the interrupt line. There is nothing for us to
> +	 * do here. Moreover, this is just broken!
> +	 */
> +	if (WARN_ON(irq->directly_injected))
> +		return true;
> +
> +	cpu_if = &vcpu->arch.vgic_cpu.vgic_v5;
> +
> +	p = (unsigned long *)&cpu_if->vgic_ppi_pendr[id / 64];
> +	__assign_bit(id % 64, p, irq_is_pending(irq));
> +
> +	return true;
> +}

>  /*
>   * Sets/clears the corresponding bit in the ICH_PPI_DVIR register.
>   */
> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index c465ff51cb073..1cdfa5224ead5 100644
> --- a/arch/arm64/kvm/vgic/vgic.c
> +++ b/arch/arm64/kvm/vgic/vgic.c
> @@ -105,6 +105,18 @@ struct vgic_irq *vgic_get_vcpu_irq(struct kvm_vcpu *vcpu, u32 intid)
>  	if (WARN_ON(!vcpu))
>  		return NULL;
>  
> +	if (vgic_is_v5(vcpu->kvm)) {
> +		u32 int_num;
> +
> +		if (!__irq_is_ppi(KVM_DEV_TYPE_ARM_VGIC_V5, intid))
> +			return NULL;
> +
> +		int_num = FIELD_GET(GICV5_HWIRQ_ID, intid);

I'd use an extra local variable to avoid int_num changing meaning like this.
Perhaps 
		u32 int_num, hwirq_id;

		hwirq_id = FIELD_GET(GICV5_HWIRQ_ID, intid);
		int_num = array_index_nospec(hwirq_id, VGIC_V5_NR_PRIVATE_IRQS);

or just id = FIELD_GET() given use above.


> +		int_num = array_index_nospec(int_num, VGIC_V5_NR_PRIVATE_IRQS);
> +
> +		return &vcpu->arch.vgic_cpu.private_irqs[int_num];
> +	}
> +