linux-kernel - Re: [PATCH v3 38/62] KVM: SVM: Take and hold ir_list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <qhvns5twcxwzrz2fhp7njmyqb5x5icgiz4iszwvwmeoxhw7ycv@3we3xqqaqeib>
Date: Tue, 17 Jun 2025 21:12:58 +0530
From: Naveen N Rao <naveen@...nel.org>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>, 
	Paolo Bonzini <pbonzini@...hat.com>, Joerg Roedel <joro@...tes.org>, 
	David Woodhouse <dwmw2@...radead.org>, Lu Baolu <baolu.lu@...ux.intel.com>, 
	linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev, kvm@...r.kernel.org, 
	iommu@...ts.linux.dev, linux-kernel@...r.kernel.org, Sairaj Kodilkar <sarunkod@....com>, 
	Vasant Hegde <vasant.hegde@....com>, Maxim Levitsky <mlevitsk@...hat.com>, 
	Joao Martins <joao.m.martins@...cle.com>, Francesco Lavra <francescolavra.fl@...il.com>, 
	David Matlack <dmatlack@...gle.com>
Subject: Re: [PATCH v3 38/62] KVM: SVM: Take and hold ir_list_lock across
 IRTE updates in IOMMU

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@...gle.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