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@google.com>
> > ---
> > 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.