linux-kernel - Re: [RFC PATCH 23/24] KVM: nSVM: Allocate a new ASID for nested guests

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: <5f714d7fb68aef92f1bea58a10deb4de1a10a5b8.camel@redhat.com>
Date: Thu, 03 Apr 2025 16:11:47 -0400
From: Maxim Levitsky <mlevitsk@...hat.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>, Sean Christopherson
 <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Jim Mattson <jmattson@...gle.com>, 
 Vitaly Kuznetsov <vkuznets@...hat.com>, Rik van Riel <riel@...riel.com>,
 Tom Lendacky <thomas.lendacky@....com>,  x86@...nel.org,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 23/24] KVM: nSVM: Allocate a new ASID for nested
 guests

On Wed, 2025-03-26 at 19:44 +0000, Yosry Ahmed wrote:
> Now that nested TLB flushes are properly tracked, start allocating a
> separate ASID for nested guests. This allows dropping the unconditional
> TLB flushes on nested transitions and doing finer grained TLB flushing
> when necessary.
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@...ux.dev>
> ---
>  arch/x86/kvm/svm/nested.c | 11 +++++++++--
>  arch/x86/kvm/svm/svm.c    |  5 +++--
>  arch/x86/kvm/svm/svm.h    |  3 +++
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 544913461693c..0c887c91bd50d 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1204,6 +1204,7 @@ int svm_allocate_nested(struct vcpu_svm *svm)
>  {
>  	struct kvm_svm *kvm_svm = to_kvm_svm(svm->vcpu.kvm);
>  	struct page *vmcb02_page;
> +	unsigned int asid;
>  
>  	if (svm->nested.initialized)
>  		return 0;
> @@ -1221,8 +1222,14 @@ int svm_allocate_nested(struct vcpu_svm *svm)
>  
>  	svm->nested.initialized = true;
>  
> -	if (!kvm_svm->nested_asid)
> -		kvm_svm->nested_asid = kvm_svm->asid;
> +	if (!kvm_svm->nested_asid) {
> +		asid = kvm_tlb_tags_alloc(&svm_asids);
> +		if (asid && !svm_register_asid(asid)) {
> +			kvm_tlb_tags_free(&svm_asids, asid);
> +			asid = 0;
> +		}
> +		kvm_svm->nested_asid = asid ?: fallback_asid;
> +	}

Nitpick: AFAIK at least nested KVM doesn't enable EFER.SVME,
unless it actually runs a guest thus most of the time we will waste a ASID on a VM
which once did run a VM nested and since then doesn't run anything else.

So maybe we want to free the nested ASID in the svm_free_nested?

>  
>  	return 0;
>  
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 4b95fd6b501e6..196f5bca57a0e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -249,8 +249,8 @@ static unsigned long iopm_base;
>  
>  DEFINE_PER_CPU(struct svm_cpu_data, svm_data);
>  
> -static struct kvm_tlb_tags svm_asids;
> -static unsigned int fallback_asid;
> +struct kvm_tlb_tags svm_asids;
> +unsigned int fallback_asid;
>  
>  /*
>   * Only MSR_TSC_AUX is switched via the user return hook.  EFER is switched via
> @@ -5127,6 +5127,7 @@ static void svm_vm_destroy(struct kvm *kvm)
>  	avic_vm_destroy(kvm);
>  	sev_vm_destroy(kvm);
>  	kvm_tlb_tags_free(&svm_asids, kvm_svm->asid);
> +	kvm_tlb_tags_free(&svm_asids, kvm_svm->nested_asid);
>  }
>  
>  static int svm_vm_init(struct kvm *kvm)
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 0c44133bc05ca..220d10d2b1a5c 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -630,6 +630,9 @@ static inline void svm_vmgexit_no_action(struct vcpu_svm *svm, u64 data)
>  
>  extern bool dump_invalid_vmcb;
>  
> +extern struct kvm_tlb_tags svm_asids;
> +extern unsigned int fallback_asid;
> +
>  u32 svm_msrpm_offset(u32 msr);
>  u32 *svm_vcpu_alloc_msrpm(void);
>  void svm_vcpu_init_msrpm(struct kvm_vcpu *vcpu, u32 *msrpm);


Best regards,
	Maxim Levitsky