[PATCH 0/7] Make ASIDs static for SVM

[PATCH 0/7] Make ASIDs static for SVM

[Yosry Ahmed]

This series changes SVM to use a single ASID per-VM, instead of using
dynamic generation-based ASIDs per-vCPU. Dynamic ASIDs were added for
CPUs without FLUSHBYASID to avoid full TLB flushes, but as Sean said,
FLUSHBYASID was added in 2010, and the case for this is no longer as
strong [1].

Furthermore, having different ASIDs for different vCPUs is not required.
ASIDs are local to physical CPUs. The only requirement is to make sure
the ASID is flushed before a differnet vCPU runs on the same physical
CPU (see below). Furthermore, SEV VMs have been using with a single ASID
per-VM anyway (required for different reasons).

A new ASID is currently allocated in 3 cases:
(a) Once when the vCPU is initialized.
(b) When the vCPU moves to a new physical CPU.
(c) On TLB flushes when FLUSHBYASID is not available.

Case (a) is trivial, instead the ASID is allocated for VM creation.
Case (b) is handled by flushing the ASID instead of assigning a new one.
Case (c) is handled by doing a full TLB flush (i.e.
TLB_CONTROL_FLUSH_ALL_ASID) instead of assinging a new ASID. This is
a bit aggressive, but FLUSHBYASID is available in all modern CPUs.

The series is organized as follows:
- Patch 1 generalizes the VPID allocation code in VMX to be
  vendor-neutral, to reuse for SVM.
- Patches 2-3 do some refactoring and cleanups.
- Patches 4-5 address cases (b) and (c) above.
- Patch 6 moves to single ASID per-VM.
- Patch 7 performs some minimal unification between SVM and SEV code.
  More unification can be done. In particular, SEV can use the
  generalized kvm_tlb_tags to allocate ASIDs, and can stop tracking the
  ASID separately in struct kvm_sev_info. However, I didn't have enough
  SEV knowledge (or testability) to do this.

The performance impact does not seem to be that bad. To test this
series, I ran 3 benchmarks in an SVM guest on a Milan machine:
- netperf
- cpuid_rate [2]
- A simple program doing mmap() and munmap() of 100M for 100 iterations,
  to trigger MMU syncs and TLB flushes when using the shadow MMU.

The benchmarks were ran with and without the patches for 5 iterations
each, and also with and without NPT and FLUSBYASID to emulate old
hardware. In all cases, there was either no difference or a 1-2%
performance hit for the old hardware case. The performance hit could be
larger for specific workloads, but niche performance-sensitive workloads
should not be running on very old hardware.

[1] https://lore.kernel.org/lkml/Z8JOvMx6iLexT3pK@google.com/
[2] https://lore.kernel.org/kvm/20231109180646.2963718-1-khorenko@virtuozzo.com/

Yosry Ahmed (7):
  KVM: VMX: Generalize VPID allocation to be vendor-neutral
  KVM: SVM: Use cached local variable in init_vmcb()
  KVM: SVM: Add helpers to set/clear ASID flush
  KVM: SVM: Flush everything if FLUSHBYASID is not available
  KVM: SVM: Flush the ASID when running on a new CPU
  KVM: SVM: Use a single ASID per VM
  KVM: SVM: Share more code between pre_sev_run() and pre_svm_run()

 arch/x86/include/asm/svm.h |  5 ---
 arch/x86/kvm/svm/nested.c  |  4 +-
 arch/x86/kvm/svm/sev.c     | 26 +++++-------
 arch/x86/kvm/svm/svm.c     | 87 ++++++++++++++++++++------------------
 arch/x86/kvm/svm/svm.h     | 28 ++++++++----
 arch/x86/kvm/vmx/nested.c  |  4 +-
 arch/x86/kvm/vmx/vmx.c     | 38 +++--------------
 arch/x86/kvm/vmx/vmx.h     |  4 +-
 arch/x86/kvm/x86.c         | 58 +++++++++++++++++++++++++
 arch/x86/kvm/x86.h         | 13 ++++++
 10 files changed, 161 insertions(+), 106 deletions(-)

-- 
2.49.0.rc1.451.g8f38331e32-goog

[PATCH 1/7] KVM: VMX: Generalize VPID allocation to be vendor-neutral

[Yosry Ahmed]

Generalize the VMX VPID allocation code and make move it to common code
in preparation for sharing with SVM. Create a generic struct
kvm_tlb_tags, representing a factory for VPIDs (or ASIDs later), and use
one for VPIDs.

Most of the functionality remains the same, with the following
differences:
- The enable_vpid checks are moved to the callers for allocate_vpid()
  and free_vpid(), as they are specific to VMX.
- The bitmap allocation is now dynamic (which will be required for SVM),
  so it is initialized and cleaned up in vmx_hardware_{setup/unsetup}().
- The range of valid TLB tags is expressed in terms of min/max instead
  of the number of tags to support SVM use cases.

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/vmx/nested.c |  4 +--
 arch/x86/kvm/vmx/vmx.c    | 38 +++++--------------------
 arch/x86/kvm/vmx/vmx.h    |  4 +--
 arch/x86/kvm/x86.c        | 58 +++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.h        | 13 +++++++++
 5 files changed, 82 insertions(+), 35 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index d06e50d9c0e79..b017bd2eb2382 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -343,7 +343,7 @@ static void free_nested(struct kvm_vcpu *vcpu)
 	vmx->nested.vmxon = false;
 	vmx->nested.smm.vmxon = false;
 	vmx->nested.vmxon_ptr = INVALID_GPA;
-	free_vpid(vmx->nested.vpid02);
+	kvm_tlb_tags_free(&vmx_vpids, vmx->nested.vpid02);
 	vmx->nested.posted_intr_nv = -1;
 	vmx->nested.current_vmptr = INVALID_GPA;
 	if (enable_shadow_vmcs) {
@@ -5333,7 +5333,7 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu)
 		     HRTIMER_MODE_ABS_PINNED);
 	vmx->nested.preemption_timer.function = vmx_preemption_timer_fn;
 
-	vmx->nested.vpid02 = allocate_vpid();
+	vmx->nested.vpid02 = enable_vpid ? kvm_tlb_tags_alloc(&vmx_vpids) : 0;
 
 	vmx->nested.vmcs02_initialized = false;
 	vmx->nested.vmxon = true;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index b70ed72c1783d..f7ce75842fa26 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -496,8 +496,7 @@ DEFINE_PER_CPU(struct vmcs *, current_vmcs);
  */
 static DEFINE_PER_CPU(struct list_head, loaded_vmcss_on_cpu);
 
-static DECLARE_BITMAP(vmx_vpid_bitmap, VMX_NR_VPIDS);
-static DEFINE_SPINLOCK(vmx_vpid_lock);
+struct kvm_tlb_tags vmx_vpids;
 
 struct vmcs_config vmcs_config __ro_after_init;
 struct vmx_capability vmx_capability __ro_after_init;
@@ -3972,31 +3971,6 @@ static void seg_setup(int seg)
 	vmcs_write32(sf->ar_bytes, ar);
 }
 
-int allocate_vpid(void)
-{
-	int vpid;
-
-	if (!enable_vpid)
-		return 0;
-	spin_lock(&vmx_vpid_lock);
-	vpid = find_first_zero_bit(vmx_vpid_bitmap, VMX_NR_VPIDS);
-	if (vpid < VMX_NR_VPIDS)
-		__set_bit(vpid, vmx_vpid_bitmap);
-	else
-		vpid = 0;
-	spin_unlock(&vmx_vpid_lock);
-	return vpid;
-}
-
-void free_vpid(int vpid)
-{
-	if (!enable_vpid || vpid == 0)
-		return;
-	spin_lock(&vmx_vpid_lock);
-	__clear_bit(vpid, vmx_vpid_bitmap);
-	spin_unlock(&vmx_vpid_lock);
-}
-
 static void vmx_msr_bitmap_l01_changed(struct vcpu_vmx *vmx)
 {
 	/*
@@ -7559,7 +7533,7 @@ void vmx_vcpu_free(struct kvm_vcpu *vcpu)
 
 	if (enable_pml)
 		vmx_destroy_pml_buffer(vmx);
-	free_vpid(vmx->vpid);
+	kvm_tlb_tags_free(&vmx_vpids, vmx->vpid);
 	nested_vmx_free_vcpu(vcpu);
 	free_loaded_vmcs(vmx->loaded_vmcs);
 	free_page((unsigned long)vmx->ve_info);
@@ -7578,7 +7552,7 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
 
 	err = -ENOMEM;
 
-	vmx->vpid = allocate_vpid();
+	vmx->vpid = enable_vpid ? kvm_tlb_tags_alloc(&vmx_vpids) : 0;
 
 	/*
 	 * If PML is turned on, failure on enabling PML just results in failure
@@ -7681,7 +7655,7 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
 free_pml:
 	vmx_destroy_pml_buffer(vmx);
 free_vpid:
-	free_vpid(vmx->vpid);
+	kvm_tlb_tags_free(&vmx_vpids, vmx->vpid);
 	return err;
 }
 
@@ -8373,6 +8347,7 @@ void vmx_hardware_unsetup(void)
 		nested_vmx_hardware_unsetup();
 
 	free_kvm_area();
+	kvm_tlb_tags_destroy(&vmx_vpids);
 }
 
 void vmx_vm_destroy(struct kvm *kvm)
@@ -8591,7 +8566,8 @@ __init int vmx_hardware_setup(void)
 	kvm_caps.has_bus_lock_exit = cpu_has_vmx_bus_lock_detection();
 	kvm_caps.has_notify_vmexit = cpu_has_notify_vmexit();
 
-	set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */
+	/* VPID 0 is reserved for host, so min=1  */
+	kvm_tlb_tags_init(&vmx_vpids, 1, VMX_NR_VPIDS - 1);
 
 	if (enable_ept)
 		kvm_mmu_set_ept_masks(enable_ept_ad_bits,
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 951e44dc9d0ea..9bece3ea63eaa 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -376,10 +376,10 @@ struct kvm_vmx {
 	u64 *pid_table;
 };
 
+extern struct kvm_tlb_tags vmx_vpids;
+
 void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu,
 			struct loaded_vmcs *buddy);
-int allocate_vpid(void);
-void free_vpid(int vpid);
 void vmx_set_constant_host_state(struct vcpu_vmx *vmx);
 void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu);
 void vmx_set_host_fs_gs(struct vmcs_host_state *host, u16 fs_sel, u16 gs_sel,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 69c20a68a3f01..182f18ebc62f3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13992,6 +13992,64 @@ int kvm_sev_es_string_io(struct kvm_vcpu *vcpu, unsigned int size,
 }
 EXPORT_SYMBOL_GPL(kvm_sev_es_string_io);
 
+int kvm_tlb_tags_init(struct kvm_tlb_tags *tlb_tags, unsigned int min,
+		      unsigned int max)
+{
+	/*
+	 * 0 is assumed to be the host's TLB tag and is returned on failed
+	 * allocations.
+	 */
+	if (WARN_ON_ONCE(min == 0))
+		return -1;
+
+	/*
+	 * Allocate enough bits to index the bitmap directly by the tag,
+	 * potentially wasting a bit of memory.
+	 */
+	tlb_tags->bitmap = bitmap_zalloc(max + 1, GFP_KERNEL);
+	if (!tlb_tags->bitmap)
+		return -1;
+
+	tlb_tags->min = min;
+	tlb_tags->max = max;
+	spin_lock_init(&tlb_tags->lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_tlb_tags_init);
+
+void kvm_tlb_tags_destroy(struct kvm_tlb_tags *tlb_tags)
+{
+	bitmap_free(tlb_tags->bitmap);
+}
+EXPORT_SYMBOL_GPL(kvm_tlb_tags_destroy);
+
+unsigned int kvm_tlb_tags_alloc(struct kvm_tlb_tags *tlb_tags)
+{
+	unsigned int tag;
+
+	spin_lock(&tlb_tags->lock);
+	tag = find_next_zero_bit(tlb_tags->bitmap, tlb_tags->max + 1,
+				 tlb_tags->min);
+	if (tag <= tlb_tags->max)
+		__set_bit(tag, tlb_tags->bitmap);
+	else
+		tag = 0;
+	spin_unlock(&tlb_tags->lock);
+	return tag;
+}
+EXPORT_SYMBOL_GPL(kvm_tlb_tags_alloc);
+
+void kvm_tlb_tags_free(struct kvm_tlb_tags *tlb_tags, unsigned int tag)
+{
+	if (tag < tlb_tags->min || WARN_ON_ONCE(tag > tlb_tags->max))
+		return;
+
+	spin_lock(&tlb_tags->lock);
+	__clear_bit(tag, tlb_tags->bitmap);
+	spin_unlock(&tlb_tags->lock);
+}
+EXPORT_SYMBOL_GPL(kvm_tlb_tags_free);
+
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_entry);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 9dc32a4090761..9f84e933d189b 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -652,4 +652,17 @@ int ____kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr,
 
 int kvm_emulate_hypercall(struct kvm_vcpu *vcpu);
 
+struct kvm_tlb_tags {
+	spinlock_t	lock;
+	unsigned long	*bitmap;
+	unsigned int	min;
+	unsigned int	max;
+};
+
+int kvm_tlb_tags_init(struct kvm_tlb_tags *tlb_tags, unsigned int min,
+		      unsigned int max);
+void kvm_tlb_tags_destroy(struct kvm_tlb_tags *tlb_tags);
+unsigned int kvm_tlb_tags_alloc(struct kvm_tlb_tags *tlb_tags);
+void kvm_tlb_tags_free(struct kvm_tlb_tags *tlb_tags, unsigned int tag);
+
 #endif
-- 
2.49.0.rc1.451.g8f38331e32-goog

[PATCH 2/7] KVM: SVM: Use cached local variable in init_vmcb()

[Yosry Ahmed]

svm->vmcb->control is already cached in the 'control' local variable, so
use that.

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/svm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 8abeab91d329d..28a6d2c0f250f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1367,12 +1367,12 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
 		avic_init_vmcb(svm, vmcb);
 
 	if (vnmi)
-		svm->vmcb->control.int_ctl |= V_NMI_ENABLE_MASK;
+		control->int_ctl |= V_NMI_ENABLE_MASK;
 
 	if (vgif) {
 		svm_clr_intercept(svm, INTERCEPT_STGI);
 		svm_clr_intercept(svm, INTERCEPT_CLGI);
-		svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK;
+		control->int_ctl |= V_GIF_ENABLE_MASK;
 	}
 
 	if (sev_guest(vcpu->kvm))
-- 
2.49.0.rc1.451.g8f38331e32-goog

[PATCH 3/7] KVM: SVM: Add helpers to set/clear ASID flush

[Yosry Ahmed]

Incoming changes will add more code paths that set tlb_ctl to
TLB_CONTROL_FLUSH_ASID, and will eliminate the use of
TLB_CONTROL_FLUSH_ALL_ASID except as fallback when FLUSHBYASID is not
available. Introduce set/clear helpers to set tlb_ctl to
TLB_CONTROL_FLUSH_ASID or TLB_CONTROL_DO_NOTHING.

Opportunistically move the TLB_CONTROL_* definitions to
arch/x86/kvm/svm/svm.h as they are not used outside of arch/x86/kvm/svm/.

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/include/asm/svm.h |  5 -----
 arch/x86/kvm/svm/nested.c  |  2 +-
 arch/x86/kvm/svm/sev.c     |  2 +-
 arch/x86/kvm/svm/svm.c     |  4 ++--
 arch/x86/kvm/svm/svm.h     | 15 +++++++++++++++
 5 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 9b7fa99ae9513..a97da63562eb3 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -171,11 +171,6 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 };
 
 
-#define TLB_CONTROL_DO_NOTHING 0
-#define TLB_CONTROL_FLUSH_ALL_ASID 1
-#define TLB_CONTROL_FLUSH_ASID 3
-#define TLB_CONTROL_FLUSH_ASID_LOCAL 7
-
 #define V_TPR_MASK 0x0f
 
 #define V_IRQ_SHIFT 8
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 834b67672d50f..3bff948bc5752 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -681,7 +681,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
 	/* Done at vmrun: asid.  */
 
 	/* Also overwritten later if necessary.  */
-	vmcb02->control.tlb_ctl = TLB_CONTROL_DO_NOTHING;
+	svm_vmcb_clear_flush_asid(vmcb02);
 
 	/* nested_cr3.  */
 	if (nested_npt_enabled(svm))
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 0bc708ee27887..b393674733969 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3479,7 +3479,7 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu)
 		return 0;
 
 	sd->sev_vmcbs[asid] = svm->vmcb;
-	svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
+	svm_vmcb_set_flush_asid(svm->vmcb);
 	vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
 	return 0;
 }
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 28a6d2c0f250f..8c90686a33f44 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4006,7 +4006,7 @@ static void svm_flush_tlb_asid(struct kvm_vcpu *vcpu)
 	 * VM-Exit (via kvm_mmu_reset_context()).
 	 */
 	if (static_cpu_has(X86_FEATURE_FLUSHBYASID))
-		svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
+		svm_vmcb_set_flush_asid(svm->vmcb);
 	else
 		svm->current_vmcb->asid_generation--;
 }
@@ -4373,7 +4373,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu,
 		svm->nested.nested_run_pending = 0;
 	}
 
-	svm->vmcb->control.tlb_ctl = TLB_CONTROL_DO_NOTHING;
+	svm_vmcb_clear_flush_asid(svm->vmcb);
 	vmcb_mark_all_clean(svm->vmcb);
 
 	/* if exit due to PF check for async PF */
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index d4490eaed55dd..9fd5b249b9c19 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -638,6 +638,21 @@ void svm_set_x2apic_msr_interception(struct vcpu_svm *svm, bool disable);
 void svm_complete_interrupt_delivery(struct kvm_vcpu *vcpu, int delivery_mode,
 				     int trig_mode, int vec);
 
+#define TLB_CONTROL_DO_NOTHING 0
+#define TLB_CONTROL_FLUSH_ALL_ASID 1
+#define TLB_CONTROL_FLUSH_ASID 3
+#define TLB_CONTROL_FLUSH_ASID_LOCAL 7
+
+static inline void svm_vmcb_set_flush_asid(struct vmcb *vmcb)
+{
+	vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
+}
+
+static inline void svm_vmcb_clear_flush_asid(struct vmcb *vmcb)
+{
+	vmcb->control.tlb_ctl = TLB_CONTROL_DO_NOTHING;
+}
+
 /* nested.c */
 
 #define NESTED_EXIT_HOST	0	/* Exit handled on host level */
-- 
2.49.0.rc1.451.g8f38331e32-goog

[PATCH 4/7] KVM: SVM: Flush everything if FLUSHBYASID is not available

[Yosry Ahmed]

Currently, if FLUSHBYASID is not available when performing a TLB flush,
the fallback is decrementing the ASID generation to trigger allocating a
new ASID. In preparation for using a static ASID per VM, just fallback
to flushing everything if FLUSHBYASID is not available. This is probably
worse from a performance perspective, but FLUSHBYASID has been around
for ~15 years and it's not worth carrying the complexity.

The fallback logic is moved within svm_vmcb_set_flush_asid(), as more
callers will be added and will need the fallback as well.

Suggested-by: Sean Christopherson <seanjc@google.com>

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/svm.c | 5 +----
 arch/x86/kvm/svm/svm.h | 5 ++++-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 8c90686a33f44..e5064fbefb822 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4005,10 +4005,7 @@ static void svm_flush_tlb_asid(struct kvm_vcpu *vcpu)
 	 * unconditionally does a TLB flush on both nested VM-Enter and nested
 	 * VM-Exit (via kvm_mmu_reset_context()).
 	 */
-	if (static_cpu_has(X86_FEATURE_FLUSHBYASID))
-		svm_vmcb_set_flush_asid(svm->vmcb);
-	else
-		svm->current_vmcb->asid_generation--;
+	svm_vmcb_set_flush_asid(svm->vmcb);
 }
 
 static void svm_flush_tlb_current(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 9fd5b249b9c19..0f6426809e1b9 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -645,7 +645,10 @@ void svm_complete_interrupt_delivery(struct kvm_vcpu *vcpu, int delivery_mode,
 
 static inline void svm_vmcb_set_flush_asid(struct vmcb *vmcb)
 {
-	vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
+	if (static_cpu_has(X86_FEATURE_FLUSHBYASID))
+		vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
+	else
+		vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;
 }
 
 static inline void svm_vmcb_clear_flush_asid(struct vmcb *vmcb)
-- 
2.49.0.rc1.451.g8f38331e32-goog

[PATCH 5/7] KVM: SVM: Flush the ASID when running on a new CPU

[Yosry Ahmed]

Currently, when a vCPU is migrated to a new physical CPU, the ASID
generation is reset to trigger allocating a new ASID. In preparation for
using a static ASID per VM, just flush the ASID in this case (falling
back to flushing everything if FLUSBYASID is not available).

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/svm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e5064fbefb822..b8a3fc81fc9c8 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3626,12 +3626,12 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	/*
-	 * If the previous vmrun of the vmcb occurred on a different physical
-	 * cpu, then mark the vmcb dirty and assign a new asid.  Hardware's
-	 * vmcb clean bits are per logical CPU, as are KVM's asid assignments.
+	 * If the previous VMRUN of the VMCB occurred on a different physical
+	 * CPU, then mark the VMCB dirty and flush the ASID.  Hardware's
+	 * VMCB clean bits are per logical CPU, as are KVM's ASID assignments.
 	 */
 	if (unlikely(svm->current_vmcb->cpu != vcpu->cpu)) {
-		svm->current_vmcb->asid_generation = 0;
+		svm_vmcb_set_flush_asid(svm->vmcb);
 		vmcb_mark_all_dirty(svm->vmcb);
 		svm->current_vmcb->cpu = vcpu->cpu;
         }
-- 
2.49.0.rc1.451.g8f38331e32-goog

[PATCH 6/7] KVM: SVM: Use a single ASID per VM

[Yosry Ahmed]

The ASID generation and dynamic ASID allocation logic is now only used
by initialization the generation to 0 to trigger a new ASID allocation
per-vCPU on the first VMRUN, so the ASID is more-or-less static
per-vCPU.

Moreover, having a unique ASID per-vCPU is not required. ASIDs are local
to each physical CPU, and the ASID is flushed when a vCPU is migrated to
a new physical CPU anyway. SEV VMs have been using a single ASID per VM
already for other reasons.

Use a static ASID per VM and drop the dynamic ASID allocation logic. The
ASID is allocated during vCPU reset (SEV allocates its own ASID), and
the ASID is always flushed on first use in case it was used by another
VM previously.

On VMRUN, WARN if the ASID in the VMCB does not match the VM's ASID, and
update it accordingly. Also, flush the ASID on every VMRUN if the VM
failed to allocate a unique ASID. This would probably wreck performance
if it happens, but it should be an edge case as most AMD CPUs have over
32k ASIDs.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/nested.c |  2 ++
 arch/x86/kvm/svm/sev.c    |  7 +++--
 arch/x86/kvm/svm/svm.c    | 60 +++++++++++++++++++++------------------
 arch/x86/kvm/svm/svm.h    |  8 +-----
 4 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 3bff948bc5752..d6a07644c8734 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -643,6 +643,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
 	struct kvm_vcpu *vcpu = &svm->vcpu;
 	struct vmcb *vmcb01 = svm->vmcb01.ptr;
 	struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
+	struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
 	u32 pause_count12;
 	u32 pause_thresh12;
 
@@ -677,6 +678,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
 	vmcb02->control.nested_ctl = vmcb01->control.nested_ctl;
 	vmcb02->control.iopm_base_pa = vmcb01->control.iopm_base_pa;
 	vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
+	vmcb02->control.asid = kvm_svm->asid;
 
 	/* Done at vmrun: asid.  */
 
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b393674733969..1ee04d6b9356b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3465,8 +3465,10 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu)
 	if (sev_es_guest(kvm) && !VALID_PAGE(svm->vmcb->control.vmsa_pa))
 		return -EINVAL;
 
-	/* Assign the asid allocated with this SEV guest */
-	svm->asid = asid;
+	if (WARN_ON_ONCE(svm->vmcb->control.asid != asid)) {
+		svm->vmcb->control.asid = asid;
+		vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
+	}
 
 	/*
 	 * Flush guest TLB:
@@ -4509,6 +4511,7 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
 void sev_init_vmcb(struct vcpu_svm *svm)
 {
 	svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ENABLE;
+	svm->vmcb->control.asid = sev_get_asid(svm->vcpu.kvm);
 	clr_exception_intercept(svm, UD_VECTOR);
 
 	/*
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index b8a3fc81fc9c8..c5e2733fb856d 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -249,6 +249,8 @@ static unsigned long iopm_base;
 
 DEFINE_PER_CPU(struct svm_cpu_data, svm_data);
 
+static struct kvm_tlb_tags svm_asids;
+
 /*
  * Only MSR_TSC_AUX is switched via the user return hook.  EFER is switched via
  * the VMCB, and the SYSCALL/SYSENTER MSRs are handled by VMLOAD/VMSAVE.
@@ -621,10 +623,6 @@ static int svm_enable_virtualization_cpu(void)
 		return -EBUSY;
 
 	sd = per_cpu_ptr(&svm_data, me);
-	sd->asid_generation = 1;
-	sd->max_asid = cpuid_ebx(SVM_CPUID_FUNC) - 1;
-	sd->next_asid = sd->max_asid + 1;
-	sd->min_asid = max_sev_asid + 1;
 
 	wrmsrl(MSR_EFER, efer | EFER_SVME);
 
@@ -1126,6 +1124,7 @@ static void svm_hardware_unsetup(void)
 
 	__free_pages(__sme_pa_to_page(iopm_base), get_order(IOPM_SIZE));
 	iopm_base = 0;
+	kvm_tlb_tags_destroy(&svm_asids);
 }
 
 static void init_seg(struct vmcb_seg *seg)
@@ -1234,6 +1233,7 @@ static inline void init_vmcb_after_set_cpuid(struct kvm_vcpu *vcpu)
 
 static void init_vmcb(struct kvm_vcpu *vcpu)
 {
+	struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
 	struct vcpu_svm *svm = to_svm(vcpu);
 	struct vmcb *vmcb = svm->vmcb01.ptr;
 	struct vmcb_control_area *control = &vmcb->control;
@@ -1339,8 +1339,6 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
 		save->g_pat = vcpu->arch.pat;
 		save->cr3 = 0;
 	}
-	svm->current_vmcb->asid_generation = 0;
-	svm->asid = 0;
 
 	svm->nested.vmcb12_gpa = INVALID_GPA;
 	svm->nested.last_vmcb12_gpa = INVALID_GPA;
@@ -1375,8 +1373,14 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
 		control->int_ctl |= V_GIF_ENABLE_MASK;
 	}
 
-	if (sev_guest(vcpu->kvm))
+	/* sev_init_vmcb() will assign its own ASID */
+	if (sev_guest(vcpu->kvm)) {
 		sev_init_vmcb(svm);
+		WARN_ON_ONCE(!control->asid);
+	} else {
+		control->asid = kvm_svm->asid;
+		svm_vmcb_set_flush_asid(svm->vmcb);
+	}
 
 	svm_hv_init_vmcb(vmcb);
 	init_vmcb_after_set_cpuid(vcpu);
@@ -1982,19 +1986,6 @@ static void svm_update_exception_bitmap(struct kvm_vcpu *vcpu)
 	}
 }
 
-static void new_asid(struct vcpu_svm *svm, struct svm_cpu_data *sd)
-{
-	if (sd->next_asid > sd->max_asid) {
-		++sd->asid_generation;
-		sd->next_asid = sd->min_asid;
-		svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;
-		vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
-	}
-
-	svm->current_vmcb->asid_generation = sd->asid_generation;
-	svm->asid = sd->next_asid++;
-}
-
 static void svm_set_dr6(struct kvm_vcpu *vcpu, unsigned long value)
 {
 	struct vmcb *vmcb = to_svm(vcpu)->vmcb;
@@ -3622,7 +3613,7 @@ static int svm_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
 
 static int pre_svm_run(struct kvm_vcpu *vcpu)
 {
-	struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, vcpu->cpu);
+	struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	/*
@@ -3639,9 +3630,15 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
 	if (sev_guest(vcpu->kvm))
 		return pre_sev_run(svm, vcpu->cpu);
 
-	/* FIXME: handle wraparound of asid_generation */
-	if (svm->current_vmcb->asid_generation != sd->asid_generation)
-		new_asid(svm, sd);
+	/* Flush the ASID on every VMRUN if kvm_svm->asid allocation failed */
+	if (unlikely(!kvm_svm->asid))
+		svm_vmcb_set_flush_asid(svm->vmcb);
+
+	if (WARN_ON_ONCE(svm->vmcb->control.asid != kvm_svm->asid)) {
+		svm_vmcb_set_flush_asid(svm->vmcb);
+		svm->vmcb->control.asid = kvm_svm->asid;
+		vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
+	}
 
 	return 0;
 }
@@ -4289,10 +4286,6 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu,
 
 	sync_lapic_to_cr8(vcpu);
 
-	if (unlikely(svm->asid != svm->vmcb->control.asid)) {
-		svm->vmcb->control.asid = svm->asid;
-		vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
-	}
 	svm->vmcb->save.cr2 = vcpu->arch.cr2;
 
 	svm_hv_update_vp_id(svm->vmcb, vcpu);
@@ -5024,12 +5017,16 @@ static void svm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
 
 static void svm_vm_destroy(struct kvm *kvm)
 {
+	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
+
 	avic_vm_destroy(kvm);
 	sev_vm_destroy(kvm);
+	kvm_tlb_tags_free(&svm_asids, kvm_svm->asid);
 }
 
 static int svm_vm_init(struct kvm *kvm)
 {
+	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
 	int type = kvm->arch.vm_type;
 
 	if (type != KVM_X86_DEFAULT_VM &&
@@ -5051,6 +5048,7 @@ static int svm_vm_init(struct kvm *kvm)
 			return ret;
 	}
 
+	kvm_svm->asid = kvm_tlb_tags_alloc(&svm_asids);
 	return 0;
 }
 
@@ -5332,6 +5330,7 @@ static __init int svm_hardware_setup(void)
 	void *iopm_va;
 	int r;
 	unsigned int order = get_order(IOPM_SIZE);
+	unsigned int min_asid, max_asid;
 
 	/*
 	 * NX is required for shadow paging and for NPT if the NX huge pages
@@ -5424,6 +5423,11 @@ static __init int svm_hardware_setup(void)
 	 */
 	sev_hardware_setup();
 
+	/* Consumes max_sev_asid initialized by sev_hardware_setup() */
+	min_asid = max_sev_asid + 1;
+	max_asid = cpuid_ebx(SVM_CPUID_FUNC) - 1;
+	kvm_tlb_tags_init(&svm_asids, min_asid, max_asid);
+
 	svm_hv_hardware_setup();
 
 	for_each_possible_cpu(cpu) {
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 0f6426809e1b9..4c6664ba4048d 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -119,6 +119,7 @@ struct kvm_svm {
 
 	/* Struct members for AVIC */
 	u32 avic_vm_id;
+	unsigned int asid;
 	struct page *avic_logical_id_table_page;
 	struct page *avic_physical_id_table_page;
 	struct hlist_node hnode;
@@ -132,7 +133,6 @@ struct kvm_vmcb_info {
 	struct vmcb *ptr;
 	unsigned long pa;
 	int cpu;
-	uint64_t asid_generation;
 };
 
 struct vmcb_save_area_cached {
@@ -247,7 +247,6 @@ struct vcpu_svm {
 	struct vmcb *vmcb;
 	struct kvm_vmcb_info vmcb01;
 	struct kvm_vmcb_info *current_vmcb;
-	u32 asid;
 	u32 sysenter_esp_hi;
 	u32 sysenter_eip_hi;
 	uint64_t tsc_aux;
@@ -330,11 +329,6 @@ struct vcpu_svm {
 };
 
 struct svm_cpu_data {
-	u64 asid_generation;
-	u32 max_asid;
-	u32 next_asid;
-	u32 min_asid;
-
 	struct vmcb *save_area;
 	unsigned long save_area_pa;
 
-- 
2.49.0.rc1.451.g8f38331e32-goog

[PATCH 7/7] KVM: SVM: Share more code between pre_sev_run() and pre_svm_run()

[Yosry Ahmed]

pre_svm_run() and pre_sev_run() now do some redundant work, and the
control flow is not super clear. Specifically:
- Both functions check if the ASID in the VMCB is the expected one.
- Both functions check if the vCPU moved to a different physical CPU.
- Both functions issue an ASID TLB flush if needed.

Pass the ASID and whether or not SEV requires a TLB flush from
pre_sev_run() to pre_svm_run(), and use the logic there instead.
pre_sev_run() now only performs SEV-specific checks.

Note that pre_sev_run() used svm->vcpu.arch.last_vmentry_cpu to check if
the vCPU moved to a different physical CPU, while pre_svm_run uses
svm->current_vmcb->cpu. The former tracks the CPU per vCPU, while the
latter tracks it per VMCB. For SEV, they both should be equivalent since
there is a single VMCB per-vCPU (nested is not supported).

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/sev.c | 27 ++++++++++-----------------
 arch/x86/kvm/svm/svm.c | 10 ++++++----
 arch/x86/kvm/svm/svm.h |  2 +-
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 1ee04d6b9356b..607139757f8ff 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3451,11 +3451,11 @@ void sev_es_unmap_ghcb(struct vcpu_svm *svm)
 	svm->sev_es.ghcb = NULL;
 }
 
-int pre_sev_run(struct vcpu_svm *svm, int cpu)
+int pre_sev_run(struct vcpu_svm *svm, unsigned int *asid, bool *need_flush)
 {
+	int cpu = svm->vcpu.cpu;
 	struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, cpu);
 	struct kvm *kvm = svm->vcpu.kvm;
-	unsigned int asid = sev_get_asid(kvm);
 
 	/*
 	 * Reject KVM_RUN if userspace attempts to run the vCPU with an invalid
@@ -3465,24 +3465,17 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu)
 	if (sev_es_guest(kvm) && !VALID_PAGE(svm->vmcb->control.vmsa_pa))
 		return -EINVAL;
 
-	if (WARN_ON_ONCE(svm->vmcb->control.asid != asid)) {
-		svm->vmcb->control.asid = asid;
-		vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
-	}
-
 	/*
-	 * Flush guest TLB:
-	 *
-	 * 1) when different VMCB for the same ASID is to be run on the same host CPU.
-	 * 2) or this VMCB was executed on different host CPU in previous VMRUNs.
+	 * Flush the guest TLB when a difference VMCB for the same ASID is to be
+	 * run on the same host CPU. The caller will also flush the TLB if the
+	 * VMCB was executed on a different host CPU in previous VMRUNs.
 	 */
-	if (sd->sev_vmcbs[asid] == svm->vmcb &&
-	    svm->vcpu.arch.last_vmentry_cpu == cpu)
-		return 0;
+	*asid = sev_get_asid(kvm);
+	if (sd->sev_vmcbs[*asid] != svm->vmcb) {
+		sd->sev_vmcbs[*asid] = svm->vmcb;
+		*need_flush = true;
+	}
 
-	sd->sev_vmcbs[asid] = svm->vmcb;
-	svm_vmcb_set_flush_asid(svm->vmcb);
-	vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
 	return 0;
 }
 
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index c5e2733fb856d..6b338d84e7b93 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3615,21 +3615,23 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
 {
 	struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
 	struct vcpu_svm *svm = to_svm(vcpu);
+	unsigned int asid = kvm_svm->asid;
+	bool sev_need_flush = false;
+
+	if (sev_guest(vcpu->kvm) && pre_sev_run(svm, &asid, &sev_need_flush))
+		return -1;
 
 	/*
 	 * If the previous VMRUN of the VMCB occurred on a different physical
 	 * CPU, then mark the VMCB dirty and flush the ASID.  Hardware's
 	 * VMCB clean bits are per logical CPU, as are KVM's ASID assignments.
 	 */
-	if (unlikely(svm->current_vmcb->cpu != vcpu->cpu)) {
+	if (unlikely(sev_need_flush || svm->current_vmcb->cpu != vcpu->cpu)) {
 		svm_vmcb_set_flush_asid(svm->vmcb);
 		vmcb_mark_all_dirty(svm->vmcb);
 		svm->current_vmcb->cpu = vcpu->cpu;
         }
 
-	if (sev_guest(vcpu->kvm))
-		return pre_sev_run(svm, vcpu->cpu);
-
 	/* Flush the ASID on every VMRUN if kvm_svm->asid allocation failed */
 	if (unlikely(!kvm_svm->asid))
 		svm_vmcb_set_flush_asid(svm->vmcb);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 4c6664ba4048d..f25e99c79d07d 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -754,7 +754,7 @@ void avic_refresh_virtual_apic_mode(struct kvm_vcpu *vcpu);
 
 /* sev.c */
 
-int pre_sev_run(struct vcpu_svm *svm, int cpu);
+int pre_sev_run(struct vcpu_svm *svm, unsigned int *asid, bool *need_flush);
 void sev_init_vmcb(struct vcpu_svm *svm);
 void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm);
 int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
-- 
2.49.0.rc1.451.g8f38331e32-goog

Re: [PATCH 6/7] KVM: SVM: Use a single ASID per VM

[Yosry Ahmed]

> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 0f6426809e1b9..4c6664ba4048d 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -119,6 +119,7 @@ struct kvm_svm {
>  
>  	/* Struct members for AVIC */
>  	u32 avic_vm_id;
> +	unsigned int asid;

I couldn't have put this in a worse place if I was trying.

Re: [PATCH 7/7] KVM: SVM: Share more code between pre_sev_run() and pre_svm_run()

[Yosry Ahmed]

On Thu, Mar 13, 2025 at 09:55:40PM +0000, Yosry Ahmed wrote:
> pre_svm_run() and pre_sev_run() now do some redundant work, and the
> control flow is not super clear. Specifically:
> - Both functions check if the ASID in the VMCB is the expected one.
> - Both functions check if the vCPU moved to a different physical CPU.
> - Both functions issue an ASID TLB flush if needed.
> 
> Pass the ASID and whether or not SEV requires a TLB flush from
> pre_sev_run() to pre_svm_run(), and use the logic there instead.
> pre_sev_run() now only performs SEV-specific checks.
> 
> Note that pre_sev_run() used svm->vcpu.arch.last_vmentry_cpu to check if
> the vCPU moved to a different physical CPU, while pre_svm_run uses
> svm->current_vmcb->cpu. The former tracks the CPU per vCPU, while the
> latter tracks it per VMCB. For SEV, they both should be equivalent since
> there is a single VMCB per-vCPU (nested is not supported).
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>  arch/x86/kvm/svm/sev.c | 27 ++++++++++-----------------
>  arch/x86/kvm/svm/svm.c | 10 ++++++----
>  arch/x86/kvm/svm/svm.h |  2 +-
>  3 files changed, 17 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 1ee04d6b9356b..607139757f8ff 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3451,11 +3451,11 @@ void sev_es_unmap_ghcb(struct vcpu_svm *svm)
>  	svm->sev_es.ghcb = NULL;
>  }
>  
> -int pre_sev_run(struct vcpu_svm *svm, int cpu)
> +int pre_sev_run(struct vcpu_svm *svm, unsigned int *asid, bool *need_flush)
>  {
> +	int cpu = svm->vcpu.cpu;
>  	struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, cpu);
>  	struct kvm *kvm = svm->vcpu.kvm;
> -	unsigned int asid = sev_get_asid(kvm);
>  
>  	/*
>  	 * Reject KVM_RUN if userspace attempts to run the vCPU with an invalid
> @@ -3465,24 +3465,17 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu)
>  	if (sev_es_guest(kvm) && !VALID_PAGE(svm->vmcb->control.vmsa_pa))
>  		return -EINVAL;
>  
> -	if (WARN_ON_ONCE(svm->vmcb->control.asid != asid)) {
> -		svm->vmcb->control.asid = asid;
> -		vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
> -	}
> -
>  	/*
> -	 * Flush guest TLB:
> -	 *
> -	 * 1) when different VMCB for the same ASID is to be run on the same host CPU.
> -	 * 2) or this VMCB was executed on different host CPU in previous VMRUNs.
> +	 * Flush the guest TLB when a difference VMCB for the same ASID is to be
> +	 * run on the same host CPU. The caller will also flush the TLB if the
> +	 * VMCB was executed on a different host CPU in previous VMRUNs.
>  	 */
> -	if (sd->sev_vmcbs[asid] == svm->vmcb &&
> -	    svm->vcpu.arch.last_vmentry_cpu == cpu)
> -		return 0;
> +	*asid = sev_get_asid(kvm);
> +	if (sd->sev_vmcbs[*asid] != svm->vmcb) {
> +		sd->sev_vmcbs[*asid] = svm->vmcb;
> +		*need_flush = true;
> +	}
>  
> -	sd->sev_vmcbs[asid] = svm->vmcb;
> -	svm_vmcb_set_flush_asid(svm->vmcb);
> -	vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
>  	return 0;
>  }
>  
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index c5e2733fb856d..6b338d84e7b93 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3615,21 +3615,23 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
>  	struct vcpu_svm *svm = to_svm(vcpu);
> +	unsigned int asid = kvm_svm->asid;
> +	bool sev_need_flush = false;
> +
> +	if (sev_guest(vcpu->kvm) && pre_sev_run(svm, &asid, &sev_need_flush))
> +		return -1;
>  
>  	/*
>  	 * If the previous VMRUN of the VMCB occurred on a different physical
>  	 * CPU, then mark the VMCB dirty and flush the ASID.  Hardware's
>  	 * VMCB clean bits are per logical CPU, as are KVM's ASID assignments.
>  	 */
> -	if (unlikely(svm->current_vmcb->cpu != vcpu->cpu)) {
> +	if (unlikely(sev_need_flush || svm->current_vmcb->cpu != vcpu->cpu)) {
>  		svm_vmcb_set_flush_asid(svm->vmcb);
>  		vmcb_mark_all_dirty(svm->vmcb);
>  		svm->current_vmcb->cpu = vcpu->cpu;
>          }
>  
> -	if (sev_guest(vcpu->kvm))
> -		return pre_sev_run(svm, vcpu->cpu);
> -
>  	/* Flush the ASID on every VMRUN if kvm_svm->asid allocation failed */
>  	if (unlikely(!kvm_svm->asid))

This should now check 'asid' instead of 'kvm_svm->asid'.

Same for the WARN below.

>  		svm_vmcb_set_flush_asid(svm->vmcb);
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 4c6664ba4048d..f25e99c79d07d 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -754,7 +754,7 @@ void avic_refresh_virtual_apic_mode(struct kvm_vcpu *vcpu);
>  
>  /* sev.c */
>  
> -int pre_sev_run(struct vcpu_svm *svm, int cpu);
> +int pre_sev_run(struct vcpu_svm *svm, unsigned int *asid, bool *need_flush);
>  void sev_init_vmcb(struct vcpu_svm *svm);
>  void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm);
>  int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
> -- 
> 2.49.0.rc1.451.g8f38331e32-goog
> 

Re: [PATCH 6/7] KVM: SVM: Use a single ASID per VM

[Yosry Ahmed]

On Thu, Mar 13, 2025 at 09:55:39PM +0000, Yosry Ahmed wrote:
> The ASID generation and dynamic ASID allocation logic is now only used
> by initialization the generation to 0 to trigger a new ASID allocation
> per-vCPU on the first VMRUN, so the ASID is more-or-less static
> per-vCPU.
> 
> Moreover, having a unique ASID per-vCPU is not required. ASIDs are local
> to each physical CPU, and the ASID is flushed when a vCPU is migrated to
> a new physical CPU anyway. SEV VMs have been using a single ASID per VM
> already for other reasons.
> 
> Use a static ASID per VM and drop the dynamic ASID allocation logic. The
> ASID is allocated during vCPU reset (SEV allocates its own ASID), and
> the ASID is always flushed on first use in case it was used by another
> VM previously.
> 
> On VMRUN, WARN if the ASID in the VMCB does not match the VM's ASID, and
> update it accordingly. Also, flush the ASID on every VMRUN if the VM
> failed to allocate a unique ASID. This would probably wreck performance
> if it happens, but it should be an edge case as most AMD CPUs have over
> 32k ASIDs.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
[..]
> @@ -3622,7 +3613,7 @@ static int svm_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
>  
>  static int pre_svm_run(struct kvm_vcpu *vcpu)
>  {
> -	struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, vcpu->cpu);
> +	struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
>  	/*
> @@ -3639,9 +3630,15 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
>  	if (sev_guest(vcpu->kvm))
>  		return pre_sev_run(svm, vcpu->cpu);
>  
> -	/* FIXME: handle wraparound of asid_generation */
> -	if (svm->current_vmcb->asid_generation != sd->asid_generation)
> -		new_asid(svm, sd);
> +	/* Flush the ASID on every VMRUN if kvm_svm->asid allocation failed */
> +	if (unlikely(!kvm_svm->asid))
> +		svm_vmcb_set_flush_asid(svm->vmcb);

This is wrong. I thought we can handle ASID allocation failures by just
reusing ASID=0 and flushing it on every VMRUN, but using ASID=0 is
illegal according to the APM. Also, in this case we also need to flush
the ASID on every VM-exit, which I failed to do here.

There are two ways to handle running out of ASIDs:

(a) Failing to create the VM. This will impose a virtual limit on the
number of VMs that can be run concurrently. The number of ASIDs was
quite high on the CPUs I checked (2^15 IIRC), so it's probably not
an issue, but I am not sure if this is considered breaking userspace.

(b) Designating a specific ASID value as the "fallback ASID". This value
would be used by any VMs created after running out of ASIDs, and we
flush it on every VMRUN, similar to what I am trying to do here for
ASID=0.

Any thoughts on which way we should take? (a) is simpler if we can get
away with it and all AMD CPUs have a sufficiently large number of ASIDs.

Re: [PATCH 6/7] KVM: SVM: Use a single ASID per VM

[Jim Mattson]

On Mon, Mar 17, 2025 at 2:12 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
>
> On Thu, Mar 13, 2025 at 09:55:39PM +0000, Yosry Ahmed wrote:
> > The ASID generation and dynamic ASID allocation logic is now only used
> > by initialization the generation to 0 to trigger a new ASID allocation
> > per-vCPU on the first VMRUN, so the ASID is more-or-less static
> > per-vCPU.
> >
> > Moreover, having a unique ASID per-vCPU is not required. ASIDs are local
> > to each physical CPU, and the ASID is flushed when a vCPU is migrated to
> > a new physical CPU anyway. SEV VMs have been using a single ASID per VM
> > already for other reasons.
> >
> > Use a static ASID per VM and drop the dynamic ASID allocation logic. The
> > ASID is allocated during vCPU reset (SEV allocates its own ASID), and
> > the ASID is always flushed on first use in case it was used by another
> > VM previously.
> >
> > On VMRUN, WARN if the ASID in the VMCB does not match the VM's ASID, and
> > update it accordingly. Also, flush the ASID on every VMRUN if the VM
> > failed to allocate a unique ASID. This would probably wreck performance
> > if it happens, but it should be an edge case as most AMD CPUs have over
> > 32k ASIDs.
> >
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> > ---
> [..]
> > @@ -3622,7 +3613,7 @@ static int svm_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
> >
> >  static int pre_svm_run(struct kvm_vcpu *vcpu)
> >  {
> > -     struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, vcpu->cpu);
> > +     struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
> >       struct vcpu_svm *svm = to_svm(vcpu);
> >
> >       /*
> > @@ -3639,9 +3630,15 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> >       if (sev_guest(vcpu->kvm))
> >               return pre_sev_run(svm, vcpu->cpu);
> >
> > -     /* FIXME: handle wraparound of asid_generation */
> > -     if (svm->current_vmcb->asid_generation != sd->asid_generation)
> > -             new_asid(svm, sd);
> > +     /* Flush the ASID on every VMRUN if kvm_svm->asid allocation failed */
> > +     if (unlikely(!kvm_svm->asid))
> > +             svm_vmcb_set_flush_asid(svm->vmcb);
>
> This is wrong. I thought we can handle ASID allocation failures by just
> reusing ASID=0 and flushing it on every VMRUN, but using ASID=0 is
> illegal according to the APM. Also, in this case we also need to flush
> the ASID on every VM-exit, which I failed to do here.
>
> There are two ways to handle running out of ASIDs:
>
> (a) Failing to create the VM. This will impose a virtual limit on the
> number of VMs that can be run concurrently. The number of ASIDs was
> quite high on the CPUs I checked (2^15 IIRC), so it's probably not
> an issue, but I am not sure if this is considered breaking userspace.

I'm pretty sure AMD had only 6 bits of ASID through at least Family
12H. At some point, VMCB ASID bits must have become decoupled from
physical TLB tag bits. 15 TLB tag bits is inconceivable!

> (b) Designating a specific ASID value as the "fallback ASID". This value
> would be used by any VMs created after running out of ASIDs, and we
> flush it on every VMRUN, similar to what I am trying to do here for
> ASID=0.
>
> Any thoughts on which way we should take? (a) is simpler if we can get
> away with it and all AMD CPUs have a sufficiently large number of ASIDs.
>

Re: [PATCH 6/7] KVM: SVM: Use a single ASID per VM

[Yosry Ahmed]

On Mon, Mar 17, 2025 at 02:44:54PM -0700, Jim Mattson wrote:
> On Mon, Mar 17, 2025 at 2:12 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
> >
> > On Thu, Mar 13, 2025 at 09:55:39PM +0000, Yosry Ahmed wrote:
> > > The ASID generation and dynamic ASID allocation logic is now only used
> > > by initialization the generation to 0 to trigger a new ASID allocation
> > > per-vCPU on the first VMRUN, so the ASID is more-or-less static
> > > per-vCPU.
> > >
> > > Moreover, having a unique ASID per-vCPU is not required. ASIDs are local
> > > to each physical CPU, and the ASID is flushed when a vCPU is migrated to
> > > a new physical CPU anyway. SEV VMs have been using a single ASID per VM
> > > already for other reasons.
> > >
> > > Use a static ASID per VM and drop the dynamic ASID allocation logic. The
> > > ASID is allocated during vCPU reset (SEV allocates its own ASID), and
> > > the ASID is always flushed on first use in case it was used by another
> > > VM previously.
> > >
> > > On VMRUN, WARN if the ASID in the VMCB does not match the VM's ASID, and
> > > update it accordingly. Also, flush the ASID on every VMRUN if the VM
> > > failed to allocate a unique ASID. This would probably wreck performance
> > > if it happens, but it should be an edge case as most AMD CPUs have over
> > > 32k ASIDs.
> > >
> > > Suggested-by: Sean Christopherson <seanjc@google.com>
> > > Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> > > ---
> > [..]
> > > @@ -3622,7 +3613,7 @@ static int svm_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
> > >
> > >  static int pre_svm_run(struct kvm_vcpu *vcpu)
> > >  {
> > > -     struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, vcpu->cpu);
> > > +     struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
> > >       struct vcpu_svm *svm = to_svm(vcpu);
> > >
> > >       /*
> > > @@ -3639,9 +3630,15 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> > >       if (sev_guest(vcpu->kvm))
> > >               return pre_sev_run(svm, vcpu->cpu);
> > >
> > > -     /* FIXME: handle wraparound of asid_generation */
> > > -     if (svm->current_vmcb->asid_generation != sd->asid_generation)
> > > -             new_asid(svm, sd);
> > > +     /* Flush the ASID on every VMRUN if kvm_svm->asid allocation failed */
> > > +     if (unlikely(!kvm_svm->asid))
> > > +             svm_vmcb_set_flush_asid(svm->vmcb);
> >
> > This is wrong. I thought we can handle ASID allocation failures by just
> > reusing ASID=0 and flushing it on every VMRUN, but using ASID=0 is
> > illegal according to the APM. Also, in this case we also need to flush
> > the ASID on every VM-exit, which I failed to do here.
> >
> > There are two ways to handle running out of ASIDs:
> >
> > (a) Failing to create the VM. This will impose a virtual limit on the
> > number of VMs that can be run concurrently. The number of ASIDs was
> > quite high on the CPUs I checked (2^15 IIRC), so it's probably not
> > an issue, but I am not sure if this is considered breaking userspace.
> 
> I'm pretty sure AMD had only 6 bits of ASID through at least Family
> 12H. At some point, VMCB ASID bits must have become decoupled from
> physical TLB tag bits. 15 TLB tag bits is inconceivable!

I checked on Rome, Milan, Genoa, and Turin. For leaf 8000000a, the value
of EBX is 00008000 for all of them. That's 32767 ASIDs for VMs (1 for
host, and ignoring SEV). I don't have access to older hardware to check,
but 6 bits would be 63 ASIDs for VMs. I imagine such a constraint may be
less acceptable, so having a single fallback ASID may be the way to go?

> 
> > (b) Designating a specific ASID value as the "fallback ASID". This value
> > would be used by any VMs created after running out of ASIDs, and we
> > flush it on every VMRUN, similar to what I am trying to do here for
> > ASID=0.
> >
> > Any thoughts on which way we should take? (a) is simpler if we can get
> > away with it and all AMD CPUs have a sufficiently large number of ASIDs.
> >

Re: [PATCH 0/7] Make ASIDs static for SVM

[Yosry Ahmed]

On Thu, Mar 13, 2025 at 09:55:33PM +0000, Yosry Ahmed wrote:
> This series changes SVM to use a single ASID per-VM, instead of using
> dynamic generation-based ASIDs per-vCPU. Dynamic ASIDs were added for
> CPUs without FLUSHBYASID to avoid full TLB flushes, but as Sean said,
> FLUSHBYASID was added in 2010, and the case for this is no longer as
> strong [1].
> 
> Furthermore, having different ASIDs for different vCPUs is not required.
> ASIDs are local to physical CPUs. The only requirement is to make sure
> the ASID is flushed before a differnet vCPU runs on the same physical
> CPU (see below). Furthermore, SEV VMs have been using with a single ASID
> per-VM anyway (required for different reasons).
> 
> A new ASID is currently allocated in 3 cases:
> (a) Once when the vCPU is initialized.
> (b) When the vCPU moves to a new physical CPU.
> (c) On TLB flushes when FLUSHBYASID is not available.
> 
> Case (a) is trivial, instead the ASID is allocated for VM creation.
> Case (b) is handled by flushing the ASID instead of assigning a new one.
> Case (c) is handled by doing a full TLB flush (i.e.
> TLB_CONTROL_FLUSH_ALL_ASID) instead of assinging a new ASID. This is
> a bit aggressive, but FLUSHBYASID is available in all modern CPUs.
> 
> The series is organized as follows:
> - Patch 1 generalizes the VPID allocation code in VMX to be
>   vendor-neutral, to reuse for SVM.
> - Patches 2-3 do some refactoring and cleanups.
> - Patches 4-5 address cases (b) and (c) above.
> - Patch 6 moves to single ASID per-VM.
> - Patch 7 performs some minimal unification between SVM and SEV code.
>   More unification can be done. In particular, SEV can use the
>   generalized kvm_tlb_tags to allocate ASIDs, and can stop tracking the
>   ASID separately in struct kvm_sev_info. However, I didn't have enough
>   SEV knowledge (or testability) to do this.
> 
> The performance impact does not seem to be that bad. To test this
> series, I ran 3 benchmarks in an SVM guest on a Milan machine:
> - netperf
> - cpuid_rate [2]
> - A simple program doing mmap() and munmap() of 100M for 100 iterations,
>   to trigger MMU syncs and TLB flushes when using the shadow MMU.
> 
> The benchmarks were ran with and without the patches for 5 iterations
> each, and also with and without NPT and FLUSBYASID to emulate old
> hardware. In all cases, there was either no difference or a 1-2%
> performance hit for the old hardware case. The performance hit could be
> larger for specific workloads, but niche performance-sensitive workloads
> should not be running on very old hardware.

This series has several bugs. It allows a VM to use ASID 0 if we run out
of space (which is not allowed by VMRUN), and it does not handle the
case where multiple vCPUs of the same VM with the same ASID run on the
same CPU (handled by SEV through svm_cpu_data.sev_vmcbs).

I also think it's useful to see how the nSVM TLB flushing looks like on
top of this. So please hold off on reviewing this series, I will send a
new combined series.

> 
> [1] https://lore.kernel.org/lkml/Z8JOvMx6iLexT3pK@google.com/
> [2] https://lore.kernel.org/kvm/20231109180646.2963718-1-khorenko@virtuozzo.com/
> 
> Yosry Ahmed (7):
>   KVM: VMX: Generalize VPID allocation to be vendor-neutral
>   KVM: SVM: Use cached local variable in init_vmcb()
>   KVM: SVM: Add helpers to set/clear ASID flush
>   KVM: SVM: Flush everything if FLUSHBYASID is not available
>   KVM: SVM: Flush the ASID when running on a new CPU
>   KVM: SVM: Use a single ASID per VM
>   KVM: SVM: Share more code between pre_sev_run() and pre_svm_run()
> 
>  arch/x86/include/asm/svm.h |  5 ---
>  arch/x86/kvm/svm/nested.c  |  4 +-
>  arch/x86/kvm/svm/sev.c     | 26 +++++-------
>  arch/x86/kvm/svm/svm.c     | 87 ++++++++++++++++++++------------------
>  arch/x86/kvm/svm/svm.h     | 28 ++++++++----
>  arch/x86/kvm/vmx/nested.c  |  4 +-
>  arch/x86/kvm/vmx/vmx.c     | 38 +++--------------
>  arch/x86/kvm/vmx/vmx.h     |  4 +-
>  arch/x86/kvm/x86.c         | 58 +++++++++++++++++++++++++
>  arch/x86/kvm/x86.h         | 13 ++++++
>  10 files changed, 161 insertions(+), 106 deletions(-)
> 
> -- 
> 2.49.0.rc1.451.g8f38331e32-goog
>