[PATCH v2] x86/kvm: Use RCU for svm_vm_data_hash

[PATCH v2] x86/kvm: Use RCU for svm_vm_data_hash

[Sairaj Kodilkar]

GALOG notifier iterates svm_vm_data_hash to find the `struct kvm_vcpu`
for the given vm id and vcpu id. Holding spinlock is not ideal here since
it allows only a single reader to iterate over the hash. This degrades
performance when interrupt rate is high and large number of readers are
waiting for the spinlock. Hence replace spinlock with the RCU.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---

Tested on a 2-socket AMD EPYC 9965 host against the same kernel without
this change:

  - 12 VMs, each with 32 vCPUs and one passed-through NVMe device
  - vCPUs pinned NUMA-local, at 1:1, 1:2, 1:3 and 1:4 pCPU:vCPU
  - fio in each guest: randread, bs=4k, iodepth=32, numjobs=32,
    ioengine=libaio, direct=1, 60s

Time spent in the hash lookup in avic_ga_log_notifier(), measured with
a temporary tracepoint around the lookup, averaged over 12 VMs and
3 runs:

             |         mean           |          p99         |
             |------------------------|----------------------|
  pCPU:vCPU  | spinlock  |     RCU    | spinlock  |     RCU  |
-------------|-----------|------------|-----------|----------|
  1:1        | 409.8 ns  |  132.6 ns  |   960 ns  |  750 ns  |
  1:2        | 977.3 ns  |   21.9 ns  |  3530 ns  |   30 ns  |
  1:3        | 747.5 ns  |   23.7 ns  |  2150 ns  |   40 ns  |
  1:4        | 614.8 ns  |   23.5 ns  |  1610 ns  |   40 ns  |

Total IOPS was unchanged at every ratio.

Changes since v1:
================
https://lore.kernel.org/all/a2a4c5e6-8264-47e1-80de-375dc2eac048@amd.com/
- Restore spinlock guards.
- Use synchronize_rcu() in destruction path.

---
 arch/x86/kvm/svm/avic.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 96ca39c0045f..84a0e8d406e4 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -250,7 +250,6 @@ static void avic_deactivate_vmcb(struct vcpu_svm *svm)
  */
 static int avic_ga_log_notifier(u32 ga_tag)
 {
-	unsigned long flags;
 	struct kvm_svm *kvm_svm;
 	struct kvm_vcpu *vcpu = NULL;
 	u32 vm_id = AVIC_GATAG_TO_VMID(ga_tag);
@@ -259,14 +258,13 @@ static int avic_ga_log_notifier(u32 ga_tag)
 	pr_debug("SVM: %s: vm_id=%#x, vcpu_idx=%#x\n", __func__, vm_id, vcpu_idx);
 	trace_kvm_avic_ga_log(vm_id, vcpu_idx);
 
-	spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
-	hash_for_each_possible(svm_vm_data_hash, kvm_svm, hnode, vm_id) {
+	rcu_read_lock();
+	hash_for_each_possible_rcu(svm_vm_data_hash, kvm_svm, hnode, vm_id) {
 		if (kvm_svm->avic_vm_id != vm_id)
 			continue;
 		vcpu = kvm_get_vcpu(&kvm_svm->kvm, vcpu_idx);
 		break;
 	}
-	spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
 
 	/* Note:
 	 * At this point, the IOMMU should have already set the pending
@@ -276,6 +274,8 @@ static int avic_ga_log_notifier(u32 ga_tag)
 	if (vcpu)
 		kvm_vcpu_wake_up(vcpu);
 
+	rcu_read_unlock();
+
 	return 0;
 }
 
@@ -340,7 +340,7 @@ static void avic_add_vm_to_ga_log_list(struct kvm *kvm)
 		}
 	}
 	kvm_svm->avic_vm_id = vm_id;
-	hash_add(svm_vm_data_hash, &kvm_svm->hnode, kvm_svm->avic_vm_id);
+	hash_add_rcu(svm_vm_data_hash, &kvm_svm->hnode, kvm_svm->avic_vm_id);
 }
 
 int avic_vcpu_precreate(struct kvm *kvm)
@@ -373,9 +373,12 @@ void avic_vm_pre_destroy(struct kvm *kvm)
 	if (WARN_ON_ONCE(!enable_apicv) || !kvm_svm->avic_vm_id)
 		return;
 
-	guard(spinlock_irqsave)(&svm_vm_data_hash_lock);
+	{
+		guard(spinlock_irqsave)(&svm_vm_data_hash_lock);
+		hash_del_rcu(&kvm_svm->hnode);
+	}
 
-	hash_del(&kvm_svm->hnode);
+	synchronize_rcu();
 }
 
 void avic_vm_destroy(struct kvm *kvm)
-- 
2.34.1