Re: [PATCH v2 12/45] KVM: arm64: GICv3: Extract LR folding primitive

[Marc Zyngier <maz@kernel.org>]

On Mon, 10 Nov 2025 09:01:21 +0000,
Yao Yuan <yaoyuan@linux.alibaba.com> wrote:
> 
> On Sun, Nov 09, 2025 at 05:15:46PM +0800, Marc Zyngier wrote:
> > As we are going to need to handle deactivation for interrupts that
> > are not in the LRs, split vgic_v3_fold_lr_state() into a helper
> > that deals with a single interrupt, and the function that loops
> > over the used LRs.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/vgic/vgic-v3.c | 88 +++++++++++++++++------------------
> >  1 file changed, 43 insertions(+), 45 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index 3ede79e381513..0fccfe9e3e8dd 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -33,78 +33,76 @@ static bool lr_signals_eoi_mi(u64 lr_val)
> >  	       !(lr_val & ICH_LR_HW);
> >  }
> >
> > -void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
> > +static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
> >  {
> > -	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> > -	struct vgic_v3_cpu_if *cpuif = &vgic_cpu->vgic_v3;
> > -	u32 model = vcpu->kvm->arch.vgic.vgic_model;
> > -	int lr;
> > -
> > -	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
> > -
> > -	cpuif->vgic_hcr &= ~ICH_HCR_EL2_UIE;
> > -
> > -	for (lr = 0; lr < cpuif->used_lrs; lr++) {
> > -		u64 val = cpuif->vgic_lr[lr];
> > -		u32 intid, cpuid;
> > -		struct vgic_irq *irq;
> > -		bool is_v2_sgi = false;
> > -		bool deactivated;
> > -
> > -		cpuid = val & GICH_LR_PHYSID_CPUID;
> > -		cpuid >>= GICH_LR_PHYSID_CPUID_SHIFT;
> > -
> > -		if (model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> > -			intid = val & ICH_LR_VIRTUAL_ID_MASK;
> > -		} else {
> > -			intid = val & GICH_LR_VIRTUALID;
> > -			is_v2_sgi = vgic_irq_is_sgi(intid);
> > -		}
> > +	struct vgic_irq *irq;
> > +	bool is_v2_sgi = false;
> > +	bool deactivated;
> > +	u32 intid;
> >
> > -		/* Notify fds when the guest EOI'ed a level-triggered IRQ */
> > -		if (lr_signals_eoi_mi(val) && vgic_valid_spi(vcpu->kvm, intid))
> > -			kvm_notify_acked_irq(vcpu->kvm, 0,
> > -					     intid - VGIC_NR_PRIVATE_IRQS);
> > +	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> > +		intid = val & ICH_LR_VIRTUAL_ID_MASK;
> > +	} else {
> > +		intid = val & GICH_LR_VIRTUALID;
> > +		is_v2_sgi = vgic_irq_is_sgi(intid);
> > +	}
> >
> > -		irq = vgic_get_vcpu_irq(vcpu, intid);
> > -		if (!irq)	/* An LPI could have been unmapped. */
> > -			continue;
> > +	irq = vgic_get_vcpu_irq(vcpu, intid);
> > +	if (!irq)	/* An LPI could have been unmapped. */
> > +		return;
> >
> > -		raw_spin_lock(&irq->irq_lock);
> > +	/* Notify fds when the guest EOI'ed a level-triggered IRQ */
> > +	if (lr_signals_eoi_mi(val) && vgic_valid_spi(vcpu->kvm, intid))
> > +		kvm_notify_acked_irq(vcpu->kvm, 0,
> > +				     intid - VGIC_NR_PRIVATE_IRQS);
> 
> The fds notifiy happens before checking irq's mapping before
> this patch, and now in reversal order w/ above change. It's
> fine for vLPI, and for vSPI no necessary call
> kvm_notify_acked_irq() if the it has been remapped, no
> gsi<->pin mapping there. Is above understanding correct ?

We can only notify an irqfd for an SPI, never for an LPI. Given that
only looking up an LPI can result in a NULL pointer (if it has been
concurrently removed), this change is immaterial

This results in something that is easier to understand, as I find it
more logical to weed out the error cases first before taking any
significant action.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.