Re: [PATCH 11/67] KVM: SVM: Delete IRTE link from previous vCPU irrespective of new routing

[Sean Christopherson]

On Tue, Apr 15, 2025, Sairaj Kodilkar wrote:
> On 4/5/2025 1:08 AM, Sean Christopherson wrote:
> > Delete the IRTE link from the previous vCPU irrespective of the new
> > routing state.  This is a glorified nop (only the ordering changes), as
> > both the "posting" and "remapped" mode paths pre-delete the link.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> >   arch/x86/kvm/svm/avic.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> > index 02b6f0007436..e9ded2488a0b 100644
> > --- a/arch/x86/kvm/svm/avic.c
> > +++ b/arch/x86/kvm/svm/avic.c
> > @@ -870,6 +870,12 @@ int avic_pi_update_irte(struct kvm_kernel_irqfd *irqfd, struct kvm *kvm,
> >   	if (!kvm_arch_has_assigned_device(kvm) || !kvm_arch_has_irq_bypass())
> >   		return 0;
> > +	/*
> > +	 * If the IRQ was affined to a different vCPU, remove the IRTE metadata
> > +	 * from the *previous* vCPU's list.
> > +	 */
> > +	svm_ir_list_del(irqfd);
> > +
> >   	pr_debug("SVM: %s: host_irq=%#x, guest_irq=%#x, set=%#x\n",
> >   		 __func__, host_irq, guest_irq, set);
> > @@ -892,8 +898,6 @@ int avic_pi_update_irte(struct kvm_kernel_irqfd *irqfd, struct kvm *kvm,
> >   		WARN_ON_ONCE(new && memcmp(e, new, sizeof(*new)));
> > -		svm_ir_list_del(irqfd);
> > -
> >   		/**
> >   		 * Here, we setup with legacy mode in the following cases:
> >   		 * 1. When cannot target interrupt to a specific vcpu.
> 
> Hi sean,
> Why not combine patch 10 and patch 11 ? Is there a reason to separate
> the changes ?

To provide distinct bisection points if one (or both) changes introduces a bug.

Patch 10, "Delete IRTE link from previous vCPU before setting new IRTE", is a
non-trivial change in how KVM tracks per-vCPU IRTEs.

This patch is also a somewhat non-trivial change, in that removes IRTEs from the
per-vCPU list even when the new routing isn't an MSI.

Ah, but the changelog for this patch is wrong (I wrote a number of the changelogs
several months after I wrote the code, ugh).  Either that or I've now confused
myself.  I'll stare at this a bit more and rewrite the changelog unless current
me is the one that's confused.