Re: [PATCH v3 38/62] KVM: SVM: Take and hold ir_list_lock across IRTE updates in IOMMU

Naveen N Rao
2025-06-17T15:50:19Z

On Wed, Jun 11, 2025 at 03:45:41PM -0700, Sean Christopherson wrote:
> Now that svm_ir_list_add() isn't overloaded with all manner of weird
> things, fold it into avic_pi_update_irte(), and more importantly take
> ir_list_lock across the irq_set_vcpu_affinity() calls to ensure the info
> that's shoved into the IRTE is fresh. While preemption (and IRQs) is
> disabled on the task performing the IRTE update, thanks to irqfds.lock,
> that task doesn't hold the vCPU's mutex, i.e. preemption being disabled
> is irrelevant.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/svm/avic.c | 55 +++++++++++++++++------------------------
> 1 file changed, 22 insertions(+), 33 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index f1e9f0dd43e8..4747fb09aca4 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -769,32 +769,6 @@ static void svm_ir_list_del(struct kvm_kernel_irqfd *irqfd)
> spin_unlock_irqrestore(&to_svm(vcpu)->ir_list_lock, flags);
> }
>
> -static void svm_ir_list_add(struct vcpu_svm *svm,
> - struct kvm_kernel_irqfd *irqfd,
> - struct amd_iommu_pi_data *pi)
> -{
> - unsigned long flags;
> - u64 entry;
> -
> - irqfd->irq_bypass_data = pi->ir_data;
> -
> - spin_lock_irqsave(&svm->ir_list_lock, flags);
> -
> - /*
> - * Update the target pCPU for IOMMU doorbells if the vCPU is running.
> - * If the vCPU is NOT running, i.e. is blocking or scheduled out, KVM
> - * will update the pCPU info when the vCPU awkened and/or scheduled in.
> - * See also avic_vcpu_load().
> - */
> - entry = svm->avic_physical_id_entry;
> - if (entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK)
> - amd_iommu_update_ga(entry & AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK,
> - true, pi->ir_data);
> -
> - list_add(&irqfd->vcpu_list, &svm->ir_list);
> - spin_unlock_irqrestore(&svm->ir_list_lock, flags);
> -}
> -

There are a few comments in avic_vcpu_load() and avic_vcpu_put() which
still refer to svm_ir_list_add(). Would be good to update those.

- Naveen