[PATCH 0/6] Improve gfn-to-memslot performance during page faults

[PATCH 0/6] Improve gfn-to-memslot performance during page faults

[David Matlack]

This series improves the performance of gfn-to-memslot lookups during
page faults. Ben Gardon originally identified this performance gap and
sufficiently addressed it in Google's kernel by reading the memslot once
at the beginning of the page fault and passing around the pointer.

This series takes an alternative approach by introducing a per-vCPU
cache of the least recently used memslot index. This avoids needing to
binary search the existing memslots multiple times during a page fault.
Unlike passing around the pointer, the LRU cache has an additional
benefit in that it speeds up gfn-to-memslot lookups *across* faults and
during spte prefetching where the gfn changes.

This difference can be seen clearly when looking at the performance of
fast_page_fault when multiple slots are in play:

Metric                        | Baseline     | Pass*    | LRU**
----------------------------- | ------------ | -------- | ----------
Iteration 2 dirty memory time | 2.8s         | 1.6s     | 0.30s

* Pass: Lookup the memslot once per fault and pass it around.
** LRU: Cache the LRU slot per vCPU (i.e. this series).

(Collected via ./dirty_log_perf_test -v64 -x64)

I plan to also send a follow-up series with a version of Ben's patches
to pass the pointer to the memslot through the page fault handling code
rather than looking it up multiple times. Even when applied on top of
the LRU series it has some performance improvements by avoiding a few
extra memory accesses (mainly kvm->memslots[as_id] and
slots->used_slots). But it will be a judgement call whether or not it's
worth the code churn and complexity.

Here is a break down of this series:

Patches 1-2 introduce a per-vCPU cache of the least recently memslot
index.

Patches 3-5 convert existing gfn-to-memslot lookups to use
kvm_vcpu_gfn_to_memslot so that they can leverage the new LRU cache.

Patch 6 adds support for multiple slots to dirty_log_perf_test which is
used to generate the performance data in this series.

David Matlack (6):
  KVM: Cache the least recently used slot index per vCPU
  KVM: Avoid VM-wide lru_slot lookup in kvm_vcpu_gfn_to_memslot
  KVM: x86/mmu: Speed up dirty logging in
    tdp_mmu_map_handle_target_level
  KVM: x86/mmu: Leverage vcpu->lru_slot_index for rmap_add and
    rmap_recycle
  KVM: x86/mmu: Rename __gfn_to_rmap to gfn_to_rmap
  KVM: selftests: Support multiple slots in dirty_log_perf_test

 arch/x86/kvm/mmu/mmu.c                        | 54 +++++++------
 arch/x86/kvm/mmu/tdp_mmu.c                    | 15 +++-
 include/linux/kvm_host.h                      | 73 +++++++++++++-----
 .../selftests/kvm/access_tracking_perf_test.c |  2 +-
 .../selftests/kvm/demand_paging_test.c        |  2 +-
 .../selftests/kvm/dirty_log_perf_test.c       | 76 ++++++++++++++++---
 .../selftests/kvm/include/perf_test_util.h    |  2 +-
 .../selftests/kvm/lib/perf_test_util.c        | 20 +++--
 .../kvm/memslot_modification_stress_test.c    |  2 +-
 virt/kvm/kvm_main.c                           | 21 ++++-
 10 files changed, 198 insertions(+), 69 deletions(-)

-- 
2.32.0.554.ge1b32706d8-goog

[PATCH 1/6] KVM: Cache the least recently used slot index per vCPU

[David Matlack]

The memslot for a given gfn is looked up multiple times during page
fault handling. Avoid binary searching for it multiple times by caching
the least recently used slot. There is an existing VM-wide LRU slot but
that does not work well for cases where vCPUs are accessing memory in
different slots (see performance data below).

Another benefit of caching the least recently use slot (versus looking
up the slot once and passing around a pointer) is speeding up memslot
lookups *across* faults and during spte prefetching.

To measure the performance of this change I ran dirty_log_perf_test with
64 vCPUs and 64 memslots and measured "Populate memory time" and
"Iteration 2 dirty memory time".  Tests were ran with eptad=N to force
dirty logging to use fast_page_fault so its performance could be
measured.

Config     | Metric                        | Before | After
---------- | ----------------------------- | ------ | ------
tdp_mmu=Y  | Populate memory time          | 6.76s  | 5.47s
tdp_mmu=Y  | Iteration 2 dirty memory time | 2.83s  | 0.31s
tdp_mmu=N  | Populate memory time          | 20.4s  | 18.7s
tdp_mmu=N  | Iteration 2 dirty memory time | 2.65s  | 0.30s

The "Iteration 2 dirty memory time" results are especially compelling
because they are equivalent to running the same test with a single
memslot. In other words, fast_page_fault performance no longer scales
with the number of memslots.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 include/linux/kvm_host.h | 61 ++++++++++++++++++++++++++++++----------
 virt/kvm/kvm_main.c      | 21 +++++++++++++-
 2 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9d6b4ad407b8..320090d5a124 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -354,6 +354,13 @@ struct kvm_vcpu {
 	struct kvm_vcpu_stat stat;
 	char stats_id[KVM_STATS_NAME_SIZE];
 	struct kvm_dirty_ring dirty_ring;
+
+	/*
+	 * The index of the least recently used memslot by this vCPU. It's ok
+	 * if this becomes stale due to memslot changes since we always check
+	 * it is a valid slot.
+	 */
+	int lru_slot_index;
 };
 
 /* must be called with irqs disabled */
@@ -1189,27 +1196,38 @@ int kvm_request_irq_source_id(struct kvm *kvm);
 void kvm_free_irq_source_id(struct kvm *kvm, int irq_source_id);
 bool kvm_arch_irqfd_allowed(struct kvm *kvm, struct kvm_irqfd *args);
 
+static inline struct kvm_memory_slot *get_slot(struct kvm_memslots *slots, int slot_index)
+{
+	if (slot_index < 0 || slot_index >= slots->used_slots)
+		return NULL;
+
+	return &slots->memslots[slot_index];
+}
+
+static inline bool slot_contains_gfn(struct kvm_memslots *slots, int slot_index, gfn_t gfn)
+{
+	struct kvm_memory_slot *memslot = get_slot(slots, slot_index);
+
+	if (!memslot)
+		return false;
+
+	return gfn >= memslot->base_gfn && gfn < memslot->base_gfn + memslot->npages;
+}
+
 /*
- * search_memslots() and __gfn_to_memslot() are here because they are
- * used in non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c.
- * gfn_to_memslot() itself isn't here as an inline because that would
- * bloat other code too much.
- *
  * IMPORTANT: Slots are sorted from highest GFN to lowest GFN!
  */
-static inline struct kvm_memory_slot *
-search_memslots(struct kvm_memslots *slots, gfn_t gfn)
+static inline int __search_memslots(struct kvm_memslots *slots, gfn_t gfn)
 {
 	int start = 0, end = slots->used_slots;
 	int slot = atomic_read(&slots->lru_slot);
 	struct kvm_memory_slot *memslots = slots->memslots;
 
 	if (unlikely(!slots->used_slots))
-		return NULL;
+		return -1;
 
-	if (gfn >= memslots[slot].base_gfn &&
-	    gfn < memslots[slot].base_gfn + memslots[slot].npages)
-		return &memslots[slot];
+	if (slot_contains_gfn(slots, slot, gfn))
+		return slot;
 
 	while (start < end) {
 		slot = start + (end - start) / 2;
@@ -1220,13 +1238,26 @@ search_memslots(struct kvm_memslots *slots, gfn_t gfn)
 			start = slot + 1;
 	}
 
-	if (start < slots->used_slots && gfn >= memslots[start].base_gfn &&
-	    gfn < memslots[start].base_gfn + memslots[start].npages) {
+	if (slot_contains_gfn(slots, start, gfn)) {
 		atomic_set(&slots->lru_slot, start);
-		return &memslots[start];
+		return start;
 	}
 
-	return NULL;
+	return -1;
+}
+
+/*
+ * search_memslots() and __gfn_to_memslot() are here because they are
+ * used in non-modular code in arch/powerpc/kvm/book3s_hv_rm_mmu.c.
+ * gfn_to_memslot() itself isn't here as an inline because that would
+ * bloat other code too much.
+ */
+static inline struct kvm_memory_slot *
+search_memslots(struct kvm_memslots *slots, gfn_t gfn)
+{
+	int slot_index = __search_memslots(slots, gfn);
+
+	return get_slot(slots, slot_index);
 }
 
 static inline struct kvm_memory_slot *
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a96cbe24c688..9307594bda0c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -415,6 +415,7 @@ static void kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
 	vcpu->preempted = false;
 	vcpu->ready = false;
 	preempt_notifier_init(&vcpu->preempt_notifier, &kvm_preempt_ops);
+	vcpu->lru_slot_index = 0;
 }
 
 void kvm_vcpu_destroy(struct kvm_vcpu *vcpu)
@@ -2024,7 +2025,25 @@ EXPORT_SYMBOL_GPL(gfn_to_memslot);
 
 struct kvm_memory_slot *kvm_vcpu_gfn_to_memslot(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
-	return __gfn_to_memslot(kvm_vcpu_memslots(vcpu), gfn);
+	struct kvm_memslots *slots = kvm_vcpu_memslots(vcpu);
+	int slot_index = vcpu->lru_slot_index;
+	struct kvm_memory_slot *slot;
+
+	if (!slot_contains_gfn(slots, slot_index, gfn))
+		slot_index = __search_memslots(slots, gfn);
+
+	slot = get_slot(slots, slot_index);
+
+	/*
+	 * Purposely avoid updating vcpu->lru_slot_index if the gfn is not
+	 * backed by memslot as that will guarantee a cache miss on the next
+	 * try. By leaving vcpu->lru_slot_index untouched we have a chance of
+	 * a hit on the next lookup.
+	 */
+	if (slot)
+		vcpu->lru_slot_index = slot_index;
+
+	return slot;
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_gfn_to_memslot);
 
-- 
2.32.0.554.ge1b32706d8-goog

[PATCH 2/6] KVM: Avoid VM-wide lru_slot lookup in kvm_vcpu_gfn_to_memslot

[David Matlack]

Now that vCPUs keep track of their own LRU slot, there's no good reason
to have them check and update the VM-wide LRU slot. There's no
performance data to motivate this change however there are two
rationals:

1. Now that vCPUs have their own LRU slot, there's a potential for a
   double miss (miss the vCPU LRU slot and then miss the VM-wide LRU slot).
   By avoiding the VM-wide LRU slot check we keep the worst case to a
   single miss.

2. Large VMs are likely to have multiple memslots and vCPUs accessing
   different slots. Intuitively, vCPUs will end up thrashing the VM-wide
   LRU slot, decreasing the LRU hit rate for VM-wide operations such as
   mmu notifiers and VM ioctls.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 include/linux/kvm_host.h | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 320090d5a124..870e1e6fb771 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1220,17 +1220,13 @@ static inline bool slot_contains_gfn(struct kvm_memslots *slots, int slot_index,
 static inline int __search_memslots(struct kvm_memslots *slots, gfn_t gfn)
 {
 	int start = 0, end = slots->used_slots;
-	int slot = atomic_read(&slots->lru_slot);
 	struct kvm_memory_slot *memslots = slots->memslots;
 
 	if (unlikely(!slots->used_slots))
 		return -1;
 
-	if (slot_contains_gfn(slots, slot, gfn))
-		return slot;
-
 	while (start < end) {
-		slot = start + (end - start) / 2;
+		int slot = start + (end - start) / 2;
 
 		if (gfn >= memslots[slot].base_gfn)
 			end = slot;
@@ -1238,10 +1234,8 @@ static inline int __search_memslots(struct kvm_memslots *slots, gfn_t gfn)
 			start = slot + 1;
 	}
 
-	if (slot_contains_gfn(slots, start, gfn)) {
-		atomic_set(&slots->lru_slot, start);
+	if (slot_contains_gfn(slots, start, gfn))
 		return start;
-	}
 
 	return -1;
 }
@@ -1255,8 +1249,16 @@ static inline int __search_memslots(struct kvm_memslots *slots, gfn_t gfn)
 static inline struct kvm_memory_slot *
 search_memslots(struct kvm_memslots *slots, gfn_t gfn)
 {
-	int slot_index = __search_memslots(slots, gfn);
+	int slot_index = atomic_read(&slots->lru_slot);
+
+	if (slot_contains_gfn(slots, slot_index, gfn))
+		return get_slot(slots, slot_index);
+
+	slot_index = __search_memslots(slots, gfn);
+	if (slot_index < 0)
+		return NULL;
 
+	atomic_set(&slots->lru_slot, slot_index);
 	return get_slot(slots, slot_index);
 }
 
-- 
2.32.0.554.ge1b32706d8-goog

[PATCH 3/6] KVM: x86/mmu: Speed up dirty logging in tdp_mmu_map_handle_target_level

[David Matlack]

The existing TDP MMU methods to handle dirty logging are vcpu-agnostic
since they can be driven by MMU notifiers and other non-vcpu-specific
events in addition to page faults. However this means that the TDP MMU
is not benefiting from the new vcpu->lru_slot_index. Fix that by special
casing dirty logging in tdp_mmu_map_handle_target_level.

This improves "Populate memory time" in dirty_log_perf_test by 5%:

Command                         | Before           | After
------------------------------- | ---------------- | -------------
./dirty_log_perf_test -v64 -x64 | 5.472321072s     | 5.169832886s

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 43f12f5d12c0..1467f99c846d 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -929,10 +929,19 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu, int write,
 					 map_writable, !shadow_accessed_mask,
 					 &new_spte);
 
-	if (new_spte == iter->old_spte)
+	if (new_spte == iter->old_spte) {
 		ret = RET_PF_SPURIOUS;
-	else if (!tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte))
-		return RET_PF_RETRY;
+	} else {
+		if (!tdp_mmu_set_spte_atomic_no_dirty_log(vcpu->kvm, iter, new_spte))
+			return RET_PF_RETRY;
+
+		/*
+		 * Mark the gfn dirty here rather that through the vcpu-agnostic
+		 * handle_changed_spte_dirty_log to leverage vcpu->lru_slot_index.
+		 */
+		if (is_writable_pte(new_spte))
+			kvm_vcpu_mark_page_dirty(vcpu, iter->gfn);
+	}
 
 	/*
 	 * If the page fault was caused by a write but the page is write
-- 
2.32.0.554.ge1b32706d8-goog

[PATCH 4/6] KVM: x86/mmu: Leverage vcpu->lru_slot_index for rmap_add and rmap_recycle

[David Matlack]

rmap_add() and rmap_recycle() both run in the context of the vCPU and
thus we can use kvm_vcpu_gfn_to_memslot() to look up the memslot. This
enables rmap_add() and rmap_recycle() to take advantage of
vcpu->lru_slot_index and avoid expensive memslot searching.

This change improves the performance of "Populate memory time" in
dirty_log_perf_test with tdp_mmu=N. In addition to improving the
performance, "Populate memory time" no longer scales with the number
of memslots in the VM.

Command                         | Before           | After
------------------------------- | ---------------- | -------------
./dirty_log_perf_test -v64 -x1  | 15.18001570s     | 14.99469366s
./dirty_log_perf_test -v64 -x64 | 18.71336392s     | 14.98675076s

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index a8cdfd8d45c4..370a6ebc2ede 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1043,17 +1043,6 @@ static struct kvm_rmap_head *__gfn_to_rmap(gfn_t gfn, int level,
 	return &slot->arch.rmap[level - PG_LEVEL_4K][idx];
 }
 
-static struct kvm_rmap_head *gfn_to_rmap(struct kvm *kvm, gfn_t gfn,
-					 struct kvm_mmu_page *sp)
-{
-	struct kvm_memslots *slots;
-	struct kvm_memory_slot *slot;
-
-	slots = kvm_memslots_for_spte_role(kvm, sp->role);
-	slot = __gfn_to_memslot(slots, gfn);
-	return __gfn_to_rmap(gfn, sp->role.level, slot);
-}
-
 static bool rmap_can_add(struct kvm_vcpu *vcpu)
 {
 	struct kvm_mmu_memory_cache *mc;
@@ -1064,24 +1053,39 @@ static bool rmap_can_add(struct kvm_vcpu *vcpu)
 
 static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 {
+	struct kvm_memory_slot *slot;
 	struct kvm_mmu_page *sp;
 	struct kvm_rmap_head *rmap_head;
 
 	sp = sptep_to_sp(spte);
 	kvm_mmu_page_set_gfn(sp, spte - sp->spt, gfn);
-	rmap_head = gfn_to_rmap(vcpu->kvm, gfn, sp);
+	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
+	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
 	return pte_list_add(vcpu, spte, rmap_head);
 }
 
+
 static void rmap_remove(struct kvm *kvm, u64 *spte)
 {
+	struct kvm_memslots *slots;
+	struct kvm_memory_slot *slot;
 	struct kvm_mmu_page *sp;
 	gfn_t gfn;
 	struct kvm_rmap_head *rmap_head;
 
 	sp = sptep_to_sp(spte);
 	gfn = kvm_mmu_page_get_gfn(sp, spte - sp->spt);
-	rmap_head = gfn_to_rmap(kvm, gfn, sp);
+
+	/*
+	 * Unlike rmap_add and rmap_recycle, rmap_remove does not run in the
+	 * context of a vCPU so have to determine which memslots to use based
+	 * on context information in sp->role.
+	 */
+	slots = kvm_memslots_for_spte_role(kvm, sp->role);
+
+	slot = __gfn_to_memslot(slots, gfn);
+	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
+
 	__pte_list_remove(spte, rmap_head);
 }
 
@@ -1628,12 +1632,13 @@ static bool kvm_test_age_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
 
 static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 {
+	struct kvm_memory_slot *slot;
 	struct kvm_rmap_head *rmap_head;
 	struct kvm_mmu_page *sp;
 
 	sp = sptep_to_sp(spte);
-
-	rmap_head = gfn_to_rmap(vcpu->kvm, gfn, sp);
+	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
+	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
 
 	kvm_unmap_rmapp(vcpu->kvm, rmap_head, NULL, gfn, sp->role.level, __pte(0));
 	kvm_flush_remote_tlbs_with_address(vcpu->kvm, sp->gfn,
-- 
2.32.0.554.ge1b32706d8-goog

[PATCH 5/6] KVM: x86/mmu: Rename __gfn_to_rmap to gfn_to_rmap

[David Matlack]

gfn_to_rmap was removed in the previous patch so there is no need to
retain the double underscore on __gfn_to_rmap.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 370a6ebc2ede..df493729d86c 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1034,8 +1034,8 @@ static bool pte_list_destroy(struct kvm_rmap_head *rmap_head,
 	return true;
 }
 
-static struct kvm_rmap_head *__gfn_to_rmap(gfn_t gfn, int level,
-					   const struct kvm_memory_slot *slot)
+static struct kvm_rmap_head *gfn_to_rmap(gfn_t gfn, int level,
+					 const struct kvm_memory_slot *slot)
 {
 	unsigned long idx;
 
@@ -1060,7 +1060,7 @@ static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 	sp = sptep_to_sp(spte);
 	kvm_mmu_page_set_gfn(sp, spte - sp->spt, gfn);
 	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
-	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
+	rmap_head = gfn_to_rmap(gfn, sp->role.level, slot);
 	return pte_list_add(vcpu, spte, rmap_head);
 }
 
@@ -1084,7 +1084,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 	slots = kvm_memslots_for_spte_role(kvm, sp->role);
 
 	slot = __gfn_to_memslot(slots, gfn);
-	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
+	rmap_head = gfn_to_rmap(gfn, sp->role.level, slot);
 
 	__pte_list_remove(spte, rmap_head);
 }
@@ -1306,8 +1306,8 @@ static void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
 		return;
 
 	while (mask) {
-		rmap_head = __gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
-					  PG_LEVEL_4K, slot);
+		rmap_head = gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
+					PG_LEVEL_4K, slot);
 		__rmap_write_protect(kvm, rmap_head, false);
 
 		/* clear the first set bit */
@@ -1339,8 +1339,8 @@ static void kvm_mmu_clear_dirty_pt_masked(struct kvm *kvm,
 		return;
 
 	while (mask) {
-		rmap_head = __gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
-					  PG_LEVEL_4K, slot);
+		rmap_head = gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
+					PG_LEVEL_4K, slot);
 		__rmap_clear_dirty(kvm, rmap_head, slot);
 
 		/* clear the first set bit */
@@ -1406,7 +1406,7 @@ bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
 
 	if (kvm_memslots_have_rmaps(kvm)) {
 		for (i = min_level; i <= KVM_MAX_HUGEPAGE_LEVEL; ++i) {
-			rmap_head = __gfn_to_rmap(gfn, i, slot);
+			rmap_head = gfn_to_rmap(gfn, i, slot);
 			write_protected |= __rmap_write_protect(kvm, rmap_head, true);
 		}
 	}
@@ -1506,9 +1506,8 @@ rmap_walk_init_level(struct slot_rmap_walk_iterator *iterator, int level)
 {
 	iterator->level = level;
 	iterator->gfn = iterator->start_gfn;
-	iterator->rmap = __gfn_to_rmap(iterator->gfn, level, iterator->slot);
-	iterator->end_rmap = __gfn_to_rmap(iterator->end_gfn, level,
-					   iterator->slot);
+	iterator->rmap = gfn_to_rmap(iterator->gfn, level, iterator->slot);
+	iterator->end_rmap = gfn_to_rmap(iterator->end_gfn, level, iterator->slot);
 }
 
 static void
@@ -1638,7 +1637,7 @@ static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 
 	sp = sptep_to_sp(spte);
 	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
-	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
+	rmap_head = gfn_to_rmap(gfn, sp->role.level, slot);
 
 	kvm_unmap_rmapp(vcpu->kvm, rmap_head, NULL, gfn, sp->role.level, __pte(0));
 	kvm_flush_remote_tlbs_with_address(vcpu->kvm, sp->gfn,
-- 
2.32.0.554.ge1b32706d8-goog

[PATCH 6/6] KVM: selftests: Support multiple slots in dirty_log_perf_test

[David Matlack]

Introduce a new option to dirty_log_perf_test: -x number_of_slots. This
causes the test to attempt to split the region of memory into the given
number of slots. If the region cannot be evenly divided, the test will
fail.

This allows testing with more than one slot and therefore measure how
performance scales with the number of memslots.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 .../selftests/kvm/access_tracking_perf_test.c |  2 +-
 .../selftests/kvm/demand_paging_test.c        |  2 +-
 .../selftests/kvm/dirty_log_perf_test.c       | 76 ++++++++++++++++---
 .../selftests/kvm/include/perf_test_util.h    |  2 +-
 .../selftests/kvm/lib/perf_test_util.c        | 20 +++--
 .../kvm/memslot_modification_stress_test.c    |  2 +-
 6 files changed, 84 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index e2baa187a21e..3e23b2105f4b 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -333,7 +333,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	pthread_t *vcpu_threads;
 	int vcpus = params->vcpus;
 
-	vm = perf_test_create_vm(mode, vcpus, params->vcpu_memory_bytes,
+	vm = perf_test_create_vm(mode, vcpus, params->vcpu_memory_bytes, 1,
 				 params->backing_src);
 
 	perf_test_setup_vcpus(vm, vcpus, params->vcpu_memory_bytes,
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index b74704305835..61266a729d88 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -293,7 +293,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	int vcpu_id;
 	int r;
 
-	vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size,
+	vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size, 1,
 				 p->src_type);
 
 	perf_test_args.wr_fract = 1;
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 80cbd3a748c0..034458dd89a2 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -94,8 +94,59 @@ struct test_params {
 	int wr_fract;
 	bool partition_vcpu_memory_access;
 	enum vm_mem_backing_src_type backing_src;
+	int slots;
 };
 
+static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
+{
+	int i;
+
+	for (i = 0; i < slots; i++) {
+		int slot = PERF_TEST_MEM_SLOT_INDEX + i;
+		int flags = enable ? KVM_MEM_LOG_DIRTY_PAGES : 0;
+
+		vm_mem_region_set_flags(vm, slot, flags);
+	}
+}
+
+static inline void enable_dirty_logging(struct kvm_vm *vm, int slots)
+{
+	toggle_dirty_logging(vm, slots, true);
+}
+
+static inline void disable_dirty_logging(struct kvm_vm *vm, int slots)
+{
+	toggle_dirty_logging(vm, slots, false);
+}
+
+static void get_dirty_log(struct kvm_vm *vm, int slots, unsigned long *bitmap,
+			  uint64_t nr_pages)
+{
+	uint64_t slot_pages = nr_pages / slots;
+	int i;
+
+	for (i = 0; i < slots; i++) {
+		int slot = PERF_TEST_MEM_SLOT_INDEX + i;
+		unsigned long *slot_bitmap = bitmap + i * slot_pages;
+
+		kvm_vm_get_dirty_log(vm, slot, slot_bitmap);
+	}
+}
+
+static void clear_dirty_log(struct kvm_vm *vm, int slots, unsigned long *bitmap,
+			    uint64_t nr_pages)
+{
+	uint64_t slot_pages = nr_pages / slots;
+	int i;
+
+	for (i = 0; i < slots; i++) {
+		int slot = PERF_TEST_MEM_SLOT_INDEX + i;
+		unsigned long *slot_bitmap = bitmap + i * slot_pages;
+
+		kvm_vm_clear_dirty_log(vm, slot, slot_bitmap, 0, slot_pages);
+	}
+}
+
 static void run_test(enum vm_guest_mode mode, void *arg)
 {
 	struct test_params *p = arg;
@@ -114,7 +165,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	struct timespec clear_dirty_log_total = (struct timespec){0};
 
 	vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size,
-				 p->backing_src);
+				 p->slots, p->backing_src);
 
 	perf_test_args.wr_fract = p->wr_fract;
 
@@ -163,8 +214,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 	/* Enable dirty logging */
 	clock_gettime(CLOCK_MONOTONIC, &start);
-	vm_mem_region_set_flags(vm, PERF_TEST_MEM_SLOT_INDEX,
-				KVM_MEM_LOG_DIRTY_PAGES);
+	enable_dirty_logging(vm, p->slots);
 	ts_diff = timespec_elapsed(start);
 	pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
 		ts_diff.tv_sec, ts_diff.tv_nsec);
@@ -190,8 +240,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
 
 		clock_gettime(CLOCK_MONOTONIC, &start);
-		kvm_vm_get_dirty_log(vm, PERF_TEST_MEM_SLOT_INDEX, bmap);
-
+		get_dirty_log(vm, p->slots, bmap, host_num_pages);
 		ts_diff = timespec_elapsed(start);
 		get_dirty_log_total = timespec_add(get_dirty_log_total,
 						   ts_diff);
@@ -200,9 +249,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 		if (dirty_log_manual_caps) {
 			clock_gettime(CLOCK_MONOTONIC, &start);
-			kvm_vm_clear_dirty_log(vm, PERF_TEST_MEM_SLOT_INDEX, bmap, 0,
-					       host_num_pages);
-
+			clear_dirty_log(vm, p->slots, bmap, host_num_pages);
 			ts_diff = timespec_elapsed(start);
 			clear_dirty_log_total = timespec_add(clear_dirty_log_total,
 							     ts_diff);
@@ -213,7 +260,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 	/* Disable dirty logging */
 	clock_gettime(CLOCK_MONOTONIC, &start);
-	vm_mem_region_set_flags(vm, PERF_TEST_MEM_SLOT_INDEX, 0);
+	disable_dirty_logging(vm, p->slots);
 	ts_diff = timespec_elapsed(start);
 	pr_info("Disabling dirty logging time: %ld.%.9lds\n",
 		ts_diff.tv_sec, ts_diff.tv_nsec);
@@ -244,7 +291,8 @@ static void help(char *name)
 {
 	puts("");
 	printf("usage: %s [-h] [-i iterations] [-p offset] "
-	       "[-m mode] [-b vcpu bytes] [-v vcpus] [-o] [-s mem type]\n", name);
+	       "[-m mode] [-b vcpu bytes] [-v vcpus] [-o] [-s mem type]"
+	       "[-x memslots]\n", name);
 	puts("");
 	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
 	       TEST_HOST_LOOP_N);
@@ -263,6 +311,8 @@ static void help(char *name)
 	       "     them into a separate region of memory for each vCPU.\n");
 	printf(" -s: specify the type of memory that should be used to\n"
 	       "     back the guest data region.\n\n");
+	printf(" -x: Split the memory region into this number of memslots.\n"
+	       "     (default: 1)");
 	backing_src_help();
 	puts("");
 	exit(0);
@@ -276,6 +326,7 @@ int main(int argc, char *argv[])
 		.wr_fract = 1,
 		.partition_vcpu_memory_access = true,
 		.backing_src = VM_MEM_SRC_ANONYMOUS,
+		.slots = 1,
 	};
 	int opt;
 
@@ -286,7 +337,7 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "hi:p:m:b:f:v:os:")) != -1) {
+	while ((opt = getopt(argc, argv, "hi:p:m:b:f:v:os:x:")) != -1) {
 		switch (opt) {
 		case 'i':
 			p.iterations = atoi(optarg);
@@ -316,6 +367,9 @@ int main(int argc, char *argv[])
 		case 's':
 			p.backing_src = parse_backing_src_type(optarg);
 			break;
+		case 'x':
+			p.slots = atoi(optarg);
+			break;
 		case 'h':
 		default:
 			help(argv[0]);
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index 005f2143adeb..df9f1a3a3ffb 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -44,7 +44,7 @@ extern struct perf_test_args perf_test_args;
 extern uint64_t guest_test_phys_mem;
 
 struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
-				   uint64_t vcpu_memory_bytes,
+				   uint64_t vcpu_memory_bytes, int slots,
 				   enum vm_mem_backing_src_type backing_src);
 void perf_test_destroy_vm(struct kvm_vm *vm);
 void perf_test_setup_vcpus(struct kvm_vm *vm, int vcpus,
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index b488f4aefea8..aebb223d34a7 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -50,11 +50,12 @@ static void guest_code(uint32_t vcpu_id)
 }
 
 struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
-				   uint64_t vcpu_memory_bytes,
+				   uint64_t vcpu_memory_bytes, int slots,
 				   enum vm_mem_backing_src_type backing_src)
 {
 	struct kvm_vm *vm;
 	uint64_t guest_num_pages;
+	int i;
 
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
@@ -68,6 +69,9 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
 		    "Guest memory size is not host page size aligned.");
 	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.guest_page_size == 0,
 		    "Guest memory size is not guest page size aligned.");
+	TEST_ASSERT(guest_num_pages % slots == 0,
+		    "Guest memory cannot be evenly divided into %d slots.",
+		    slots);
 
 	vm = vm_create_with_vcpus(mode, vcpus, DEFAULT_GUEST_PHY_PAGES,
 				  (vcpus * vcpu_memory_bytes) / perf_test_args.guest_page_size,
@@ -95,10 +99,16 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
 #endif
 	pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
 
-	/* Add an extra memory slot for testing */
-	vm_userspace_mem_region_add(vm, backing_src, guest_test_phys_mem,
-				    PERF_TEST_MEM_SLOT_INDEX,
-				    guest_num_pages, 0);
+	/* Add extra memory slots for testing */
+	for (i = 0; i < slots; i++) {
+		uint64_t region_pages = guest_num_pages / slots;
+		vm_paddr_t region_start = guest_test_phys_mem +
+			region_pages * perf_test_args.guest_page_size * i;
+
+		vm_userspace_mem_region_add(vm, backing_src, region_start,
+					    PERF_TEST_MEM_SLOT_INDEX + i,
+					    region_pages, 0);
+	}
 
 	/* Do mapping for the demand paging memory slot */
 	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem, guest_num_pages);
diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
index 98351ba0933c..8a9c6ccce3ca 100644
--- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
+++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
@@ -105,7 +105,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	struct kvm_vm *vm;
 	int vcpu_id;
 
-	vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size,
+	vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size, 1,
 				 VM_MEM_SRC_ANONYMOUS);
 
 	perf_test_args.wr_fract = 1;
-- 
2.32.0.554.ge1b32706d8-goog

Re: [PATCH 5/6] KVM: x86/mmu: Rename __gfn_to_rmap to gfn_to_rmap

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6322 bytes --]

Hi David,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/queue]
[cannot apply to vhost/linux-next v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
config: i386-randconfig-a012-20210730 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/0310eccd630f37e334f797d966bb515ab3c3b3d2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
        git checkout 0310eccd630f37e334f797d966bb515ab3c3b3d2
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/x86/kvm/mmu/mmu.c:1936:
   arch/x86/kvm/mmu/mmu_audit.c: In function 'inspect_spte_has_rmap':
   arch/x86/kvm/mmu/mmu_audit.c:150:14: error: implicit declaration of function '__gfn_to_rmap'; did you mean 'gfn_to_rmap'? [-Werror=implicit-function-declaration]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |              ^~~~~~~~~~~~~
         |              gfn_to_rmap
>> arch/x86/kvm/mmu/mmu_audit.c:150:12: error: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |            ^
   arch/x86/kvm/mmu/mmu_audit.c: In function 'audit_write_protection':
   arch/x86/kvm/mmu/mmu_audit.c:203:12: error: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
     203 |  rmap_head = __gfn_to_rmap(sp->gfn, PG_LEVEL_4K, slot);
         |            ^
   cc1: all warnings being treated as errors


vim +150 arch/x86/kvm/mmu/mmu_audit.c

2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  125  
eb2591865a234c6f arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  126  static void inspect_spte_has_rmap(struct kvm *kvm, u64 *sptep)
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  127  {
bd80158aff71a802 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  128  	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);
018aabb56d6109c8 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  129  	struct kvm_rmap_head *rmap_head;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  130  	struct kvm_mmu_page *rev_sp;
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  131  	struct kvm_memslots *slots;
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  132  	struct kvm_memory_slot *slot;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  133  	gfn_t gfn;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  134  
573546820b792ef6 arch/x86/kvm/mmu/mmu_audit.c Sean Christopherson 2020-06-22  135  	rev_sp = sptep_to_sp(sptep);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  136  	gfn = kvm_mmu_page_get_gfn(rev_sp, sptep - rev_sp->spt);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  137  
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  138  	slots = kvm_memslots_for_spte_role(kvm, rev_sp->role);
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  139  	slot = __gfn_to_memslot(slots, gfn);
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  140  	if (!slot) {
bd80158aff71a802 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  141  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  142  			return;
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  143  		audit_printk(kvm, "no memslot for gfn %llx\n", gfn);
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  144  		audit_printk(kvm, "index %ld of sp (gfn=%llx)\n",
38904e128778c388 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-09-27  145  		       (long int)(sptep - rev_sp->spt), rev_sp->gfn);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  146  		dump_stack();
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  147  		return;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  148  	}
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  149  
018aabb56d6109c8 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20 @150  	rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
018aabb56d6109c8 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  151  	if (!rmap_head->val) {
bd80158aff71a802 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  152  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  153  			return;
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  154  		audit_printk(kvm, "no rmap for writable spte %llx\n",
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  155  			     *sptep);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  156  		dump_stack();
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  157  	}
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  158  }
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  159  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40728 bytes --]

Re: [PATCH 5/6] KVM: x86/mmu: Rename __gfn_to_rmap to gfn_to_rmap

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6408 bytes --]

Hi David,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/queue]
[cannot apply to vhost/linux-next v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
config: i386-randconfig-a012-20210730 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/0310eccd630f37e334f797d966bb515ab3c3b3d2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
        git checkout 0310eccd630f37e334f797d966bb515ab3c3b3d2
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/x86/kvm/mmu/mmu.c:1936:
   arch/x86/kvm/mmu/mmu_audit.c: In function 'inspect_spte_has_rmap':
   arch/x86/kvm/mmu/mmu_audit.c:150:14: error: implicit declaration of function '__gfn_to_rmap'; did you mean 'gfn_to_rmap'? [-Werror=implicit-function-declaration]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |              ^~~~~~~~~~~~~
         |              gfn_to_rmap
>> arch/x86/kvm/mmu/mmu_audit.c:150:12: error: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |            ^
   arch/x86/kvm/mmu/mmu_audit.c: In function 'audit_write_protection':
   arch/x86/kvm/mmu/mmu_audit.c:203:12: error: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
     203 |  rmap_head = __gfn_to_rmap(sp->gfn, PG_LEVEL_4K, slot);
         |            ^
   cc1: all warnings being treated as errors


vim +150 arch/x86/kvm/mmu/mmu_audit.c

2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  125  
eb2591865a234c6f arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  126  static void inspect_spte_has_rmap(struct kvm *kvm, u64 *sptep)
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  127  {
bd80158aff71a802 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  128  	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);
018aabb56d6109c8 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  129  	struct kvm_rmap_head *rmap_head;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  130  	struct kvm_mmu_page *rev_sp;
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  131  	struct kvm_memslots *slots;
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  132  	struct kvm_memory_slot *slot;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  133  	gfn_t gfn;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  134  
573546820b792ef6 arch/x86/kvm/mmu/mmu_audit.c Sean Christopherson 2020-06-22  135  	rev_sp = sptep_to_sp(sptep);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  136  	gfn = kvm_mmu_page_get_gfn(rev_sp, sptep - rev_sp->spt);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  137  
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  138  	slots = kvm_memslots_for_spte_role(kvm, rev_sp->role);
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  139  	slot = __gfn_to_memslot(slots, gfn);
699023e239658e62 arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  140  	if (!slot) {
bd80158aff71a802 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  141  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  142  			return;
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  143  		audit_printk(kvm, "no memslot for gfn %llx\n", gfn);
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  144  		audit_printk(kvm, "index %ld of sp (gfn=%llx)\n",
38904e128778c388 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-09-27  145  		       (long int)(sptep - rev_sp->spt), rev_sp->gfn);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  146  		dump_stack();
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  147  		return;
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  148  	}
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  149  
018aabb56d6109c8 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20 @150  	rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
018aabb56d6109c8 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  151  	if (!rmap_head->val) {
bd80158aff71a802 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  152  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  153  			return;
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  154  		audit_printk(kvm, "no rmap for writable spte %llx\n",
b034cf0105235e65 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  155  			     *sptep);
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  156  		dump_stack();
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  157  	}
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  158  }
2f4f337248cd5660 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  159  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 40728 bytes --]

Re: [PATCH 5/6] KVM: x86/mmu: Rename __gfn_to_rmap to gfn_to_rmap

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 11004 bytes --]

Hi David,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/queue]
[cannot apply to vhost/linux-next v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
config: x86_64-rhel-8.3-kselftests (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/0310eccd630f37e334f797d966bb515ab3c3b3d2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
        git checkout 0310eccd630f37e334f797d966bb515ab3c3b3d2
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from arch/x86/kvm/mmu/mmu.c:1936:
   arch/x86/kvm/mmu/mmu_audit.c: In function 'inspect_spte_has_rmap':
>> arch/x86/kvm/mmu/mmu_audit.c:150:14: error: implicit declaration of function '__gfn_to_rmap'; did you mean 'gfn_to_rmap'? [-Werror=implicit-function-declaration]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |              ^~~~~~~~~~~~~
         |              gfn_to_rmap
>> arch/x86/kvm/mmu/mmu_audit.c:150:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |            ^
   arch/x86/kvm/mmu/mmu_audit.c: In function 'audit_write_protection':
   arch/x86/kvm/mmu/mmu_audit.c:203:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     203 |  rmap_head = __gfn_to_rmap(sp->gfn, PG_LEVEL_4K, slot);
         |            ^
   cc1: some warnings being treated as errors
--
   In file included from arch/x86/kvm/mmu/mmu.c:1936:
   arch/x86/kvm/mmu/mmu_audit.c: In function 'inspect_spte_has_rmap':
>> arch/x86/kvm/mmu/mmu_audit.c:150:14: error: implicit declaration of function '__gfn_to_rmap'; did you mean 'gfn_to_rmap'? [-Werror=implicit-function-declaration]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |              ^~~~~~~~~~~~~
         |              gfn_to_rmap
>> arch/x86/kvm/mmu/mmu_audit.c:150:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |            ^
   arch/x86/kvm/mmu/mmu_audit.c: In function 'audit_write_protection':
   arch/x86/kvm/mmu/mmu_audit.c:203:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     203 |  rmap_head = __gfn_to_rmap(sp->gfn, PG_LEVEL_4K, slot);
         |            ^
   cc1: some warnings being treated as errors
   make[3]: *** [scripts/Makefile.build:271: arch/x86/kvm/mmu/mmu.o] Error 1
   make[3]: Target '__build' not remade because of errors.
   make[2]: *** [scripts/Makefile.build:514: arch/x86/kvm] Error 2
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1842: arch/x86] Error 2
   make[1]: Target 'vmlinux' not remade because of errors.
   make: *** [Makefile:220: __sub-make] Error 2
   make: Target 'vmlinux' not remade because of errors.
   ***
   *** The source tree is not clean, please run 'make ARCH=x86_64 mrproper'
   *** in /kbuild/worktree/build-ktools-consumer
   ***
   make[2]: *** [Makefile:547: outputmakefile] Error 1
   make[2]: Target 'syncconfig' not remade because of errors.
   make[1]: *** [Makefile:710: include/config/auto.conf.cmd] Error 2
   make[1]: *** [include/config/auto.conf.cmd] Deleting file 'include/generated/autoconf.h'
   make[1]: Failed to remake makefile 'include/config/auto.conf.cmd'.
   make[1]: Failed to remake makefile 'include/config/auto.conf'.
   ***
   *** The source tree is not clean, please run 'make ARCH=x86_64 mrproper'
   *** in /kbuild/worktree/build-ktools-consumer
   ***
   make[1]: *** [Makefile:547: outputmakefile] Error 1
   In file included from <command-line>:32:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   In file included from <command-line>:32:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   In file included from <command-line>:32:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   In file included from scripts/selinux/mdp/mdp.c:22:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   make[4]: *** [scripts/Makefile.host:95: scripts/selinux/mdp/mdp] Error 1
   make[4]: Target '__build' not remade because of errors.
   make[3]: *** [scripts/Makefile.build:496: scripts/selinux/mdp] Error 2
   make[3]: Target '__build' not remade because of errors.
   make[2]: *** [scripts/Makefile.build:496: scripts/selinux] Error 2
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1190: scripts] Error 2
   In file included from tools/include/uapi/linux/ethtool.h:19,
                    from xsk.c:18:
   usr/include/linux/if_ether.h:169:1: error: packed attribute is unnecessary for 'ethhdr' [-Werror=packed]
     169 | } __attribute__((packed));
         | ^
   cc1: all warnings being treated as errors
   make[5]: *** [tools/build/Makefile.build:96: tools/bpf/resolve_btfids/staticobjs/xsk.o] Error 1
   make[5]: *** Waiting for unfinished jobs....
   make[4]: *** [Makefile:179: tools/bpf/resolve_btfids/staticobjs/libbpf-in.o] Error 2
   make[3]: *** [Makefile:56: tools/bpf/resolve_btfids//libbpf.a] Error 2
   make[2]: *** [Makefile:71: bpf/resolve_btfids] Error 2
   make[1]: *** [Makefile:1930: tools/bpf/resolve_btfids] Error 2
   make[1]: Target 'vmlinux' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'vmlinux' not remade because of errors.


vim +150 arch/x86/kvm/mmu/mmu_audit.c

2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  125  
eb2591865a234c arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  126  static void inspect_spte_has_rmap(struct kvm *kvm, u64 *sptep)
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  127  {
bd80158aff71a8 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  128  	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);
018aabb56d6109 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  129  	struct kvm_rmap_head *rmap_head;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  130  	struct kvm_mmu_page *rev_sp;
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  131  	struct kvm_memslots *slots;
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  132  	struct kvm_memory_slot *slot;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  133  	gfn_t gfn;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  134  
573546820b792e arch/x86/kvm/mmu/mmu_audit.c Sean Christopherson 2020-06-22  135  	rev_sp = sptep_to_sp(sptep);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  136  	gfn = kvm_mmu_page_get_gfn(rev_sp, sptep - rev_sp->spt);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  137  
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  138  	slots = kvm_memslots_for_spte_role(kvm, rev_sp->role);
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  139  	slot = __gfn_to_memslot(slots, gfn);
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  140  	if (!slot) {
bd80158aff71a8 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  141  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  142  			return;
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  143  		audit_printk(kvm, "no memslot for gfn %llx\n", gfn);
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  144  		audit_printk(kvm, "index %ld of sp (gfn=%llx)\n",
38904e128778c3 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-09-27  145  		       (long int)(sptep - rev_sp->spt), rev_sp->gfn);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  146  		dump_stack();
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  147  		return;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  148  	}
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  149  
018aabb56d6109 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20 @150  	rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
018aabb56d6109 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  151  	if (!rmap_head->val) {
bd80158aff71a8 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  152  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  153  			return;
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  154  		audit_printk(kvm, "no rmap for writable spte %llx\n",
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  155  			     *sptep);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  156  		dump_stack();
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  157  	}
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  158  }
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  159  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42155 bytes --]

Re: [PATCH 5/6] KVM: x86/mmu: Rename __gfn_to_rmap to gfn_to_rmap

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 11172 bytes --]

Hi David,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/queue]
[cannot apply to vhost/linux-next v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
config: x86_64-rhel-8.3-kselftests (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/0310eccd630f37e334f797d966bb515ab3c3b3d2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Matlack/Improve-gfn-to-memslot-performance-during-page-faults/20210731-063919
        git checkout 0310eccd630f37e334f797d966bb515ab3c3b3d2
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from arch/x86/kvm/mmu/mmu.c:1936:
   arch/x86/kvm/mmu/mmu_audit.c: In function 'inspect_spte_has_rmap':
>> arch/x86/kvm/mmu/mmu_audit.c:150:14: error: implicit declaration of function '__gfn_to_rmap'; did you mean 'gfn_to_rmap'? [-Werror=implicit-function-declaration]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |              ^~~~~~~~~~~~~
         |              gfn_to_rmap
>> arch/x86/kvm/mmu/mmu_audit.c:150:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |            ^
   arch/x86/kvm/mmu/mmu_audit.c: In function 'audit_write_protection':
   arch/x86/kvm/mmu/mmu_audit.c:203:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     203 |  rmap_head = __gfn_to_rmap(sp->gfn, PG_LEVEL_4K, slot);
         |            ^
   cc1: some warnings being treated as errors
--
   In file included from arch/x86/kvm/mmu/mmu.c:1936:
   arch/x86/kvm/mmu/mmu_audit.c: In function 'inspect_spte_has_rmap':
>> arch/x86/kvm/mmu/mmu_audit.c:150:14: error: implicit declaration of function '__gfn_to_rmap'; did you mean 'gfn_to_rmap'? [-Werror=implicit-function-declaration]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |              ^~~~~~~~~~~~~
         |              gfn_to_rmap
>> arch/x86/kvm/mmu/mmu_audit.c:150:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     150 |  rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
         |            ^
   arch/x86/kvm/mmu/mmu_audit.c: In function 'audit_write_protection':
   arch/x86/kvm/mmu/mmu_audit.c:203:12: warning: assignment to 'struct kvm_rmap_head *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     203 |  rmap_head = __gfn_to_rmap(sp->gfn, PG_LEVEL_4K, slot);
         |            ^
   cc1: some warnings being treated as errors
   make[3]: *** [scripts/Makefile.build:271: arch/x86/kvm/mmu/mmu.o] Error 1
   make[3]: Target '__build' not remade because of errors.
   make[2]: *** [scripts/Makefile.build:514: arch/x86/kvm] Error 2
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1842: arch/x86] Error 2
   make[1]: Target 'vmlinux' not remade because of errors.
   make: *** [Makefile:220: __sub-make] Error 2
   make: Target 'vmlinux' not remade because of errors.
   ***
   *** The source tree is not clean, please run 'make ARCH=x86_64 mrproper'
   *** in /kbuild/worktree/build-ktools-consumer
   ***
   make[2]: *** [Makefile:547: outputmakefile] Error 1
   make[2]: Target 'syncconfig' not remade because of errors.
   make[1]: *** [Makefile:710: include/config/auto.conf.cmd] Error 2
   make[1]: *** [include/config/auto.conf.cmd] Deleting file 'include/generated/autoconf.h'
   make[1]: Failed to remake makefile 'include/config/auto.conf.cmd'.
   make[1]: Failed to remake makefile 'include/config/auto.conf'.
   ***
   *** The source tree is not clean, please run 'make ARCH=x86_64 mrproper'
   *** in /kbuild/worktree/build-ktools-consumer
   ***
   make[1]: *** [Makefile:547: outputmakefile] Error 1
   In file included from <command-line>:32:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   In file included from <command-line>:32:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   In file included from <command-line>:32:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   In file included from scripts/selinux/mdp/mdp.c:22:
   include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
       7 | #include <generated/autoconf.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   make[4]: *** [scripts/Makefile.host:95: scripts/selinux/mdp/mdp] Error 1
   make[4]: Target '__build' not remade because of errors.
   make[3]: *** [scripts/Makefile.build:496: scripts/selinux/mdp] Error 2
   make[3]: Target '__build' not remade because of errors.
   make[2]: *** [scripts/Makefile.build:496: scripts/selinux] Error 2
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1190: scripts] Error 2
   In file included from tools/include/uapi/linux/ethtool.h:19,
                    from xsk.c:18:
   usr/include/linux/if_ether.h:169:1: error: packed attribute is unnecessary for 'ethhdr' [-Werror=packed]
     169 | } __attribute__((packed));
         | ^
   cc1: all warnings being treated as errors
   make[5]: *** [tools/build/Makefile.build:96: tools/bpf/resolve_btfids/staticobjs/xsk.o] Error 1
   make[5]: *** Waiting for unfinished jobs....
   make[4]: *** [Makefile:179: tools/bpf/resolve_btfids/staticobjs/libbpf-in.o] Error 2
   make[3]: *** [Makefile:56: tools/bpf/resolve_btfids//libbpf.a] Error 2
   make[2]: *** [Makefile:71: bpf/resolve_btfids] Error 2
   make[1]: *** [Makefile:1930: tools/bpf/resolve_btfids] Error 2
   make[1]: Target 'vmlinux' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'vmlinux' not remade because of errors.


vim +150 arch/x86/kvm/mmu/mmu_audit.c

2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  125  
eb2591865a234c arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  126  static void inspect_spte_has_rmap(struct kvm *kvm, u64 *sptep)
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  127  {
bd80158aff71a8 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  128  	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);
018aabb56d6109 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  129  	struct kvm_rmap_head *rmap_head;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  130  	struct kvm_mmu_page *rev_sp;
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  131  	struct kvm_memslots *slots;
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  132  	struct kvm_memory_slot *slot;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  133  	gfn_t gfn;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  134  
573546820b792e arch/x86/kvm/mmu/mmu_audit.c Sean Christopherson 2020-06-22  135  	rev_sp = sptep_to_sp(sptep);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  136  	gfn = kvm_mmu_page_get_gfn(rev_sp, sptep - rev_sp->spt);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  137  
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  138  	slots = kvm_memslots_for_spte_role(kvm, rev_sp->role);
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  139  	slot = __gfn_to_memslot(slots, gfn);
699023e239658e arch/x86/kvm/mmu_audit.c     Paolo Bonzini       2015-05-18  140  	if (!slot) {
bd80158aff71a8 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  141  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  142  			return;
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  143  		audit_printk(kvm, "no memslot for gfn %llx\n", gfn);
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  144  		audit_printk(kvm, "index %ld of sp (gfn=%llx)\n",
38904e128778c3 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-09-27  145  		       (long int)(sptep - rev_sp->spt), rev_sp->gfn);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  146  		dump_stack();
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  147  		return;
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  148  	}
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  149  
018aabb56d6109 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20 @150  	rmap_head = __gfn_to_rmap(gfn, rev_sp->role.level, slot);
018aabb56d6109 arch/x86/kvm/mmu_audit.c     Takuya Yoshikawa    2015-11-20  151  	if (!rmap_head->val) {
bd80158aff71a8 arch/x86/kvm/mmu_audit.c     Jan Kiszka          2011-09-12  152  		if (!__ratelimit(&ratelimit_state))
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  153  			return;
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  154  		audit_printk(kvm, "no rmap for writable spte %llx\n",
b034cf0105235e arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-12-23  155  			     *sptep);
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  156  		dump_stack();
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  157  	}
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  158  }
2f4f337248cd56 arch/x86/kvm/mmu_audit.c     Xiao Guangrong      2010-08-30  159  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 42155 bytes --]

Re: [PATCH 1/6] KVM: Cache the least recently used slot index per vCPU

[Paolo Bonzini]

On 31/07/21 00:37, David Matlack wrote:
> The memslot for a given gfn is looked up multiple times during page
> fault handling. Avoid binary searching for it multiple times by caching
> the least recently used slot. There is an existing VM-wide LRU slot but
> that does not work well for cases where vCPUs are accessing memory in
> different slots (see performance data below).
> 
> Another benefit of caching the least recently use slot (versus looking
> up the slot once and passing around a pointer) is speeding up memslot
> lookups *across* faults and during spte prefetching.
> 
> To measure the performance of this change I ran dirty_log_perf_test with
> 64 vCPUs and 64 memslots and measured "Populate memory time" and
> "Iteration 2 dirty memory time".  Tests were ran with eptad=N to force
> dirty logging to use fast_page_fault so its performance could be
> measured.
> 
> Config     | Metric                        | Before | After
> ---------- | ----------------------------- | ------ | ------
> tdp_mmu=Y  | Populate memory time          | 6.76s  | 5.47s
> tdp_mmu=Y  | Iteration 2 dirty memory time | 2.83s  | 0.31s
> tdp_mmu=N  | Populate memory time          | 20.4s  | 18.7s
> tdp_mmu=N  | Iteration 2 dirty memory time | 2.65s  | 0.30s
> 
> The "Iteration 2 dirty memory time" results are especially compelling
> because they are equivalent to running the same test with a single
> memslot. In other words, fast_page_fault performance no longer scales
> with the number of memslots.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>

It's the *most* recently used slot index, of course. :)  That's true of 
lru_slot as well.

> +static inline struct kvm_memory_slot *get_slot(struct kvm_memslots *slots, int slot_index)
> +{
> +	if (slot_index < 0 || slot_index >= slots->used_slots)
> +		return NULL;
> +
> +	return &slots->memslots[slot_index];
> +}
> +

Since there are plenty of arrays inside struct kvm_memory_slot*, do we 
want to protect this against speculative out-of-bounds accesses with 
array_index_nospec?

> +static inline struct kvm_memory_slot *
> +search_memslots(struct kvm_memslots *slots, gfn_t gfn)
> +{
> +	int slot_index = __search_memslots(slots, gfn);
> +
> +	return get_slot(slots, slot_index);
>  }

Let's use this occasion to remove the duplication between 
__gfn_to_memslot and search_memslots; you can make search_memslots do 
the search and palce the LRU (ehm, MRU) code to __gfn_to_memslot only.  So:

- the new patch 1 (something like "make search_memslots search without 
LRU caching") is basically this series's patch 2, plus a tree-wide 
replacement of search_memslots with __gfn_to_memslot.

- the new patch 2 is this series's patch 1, except 
kvm_vcpu_gfn_to_memslot uses search_memslots instead of 
__search_memslots.  The comments in patch 2's commit message about the 
double misses move to this commit message.

> +	if (slot)
> +		vcpu->lru_slot_index = slot_index;

Let's call it lru_slot for consistency with the field of struct 
kvm_memory_slots.

Paolo

Re: [PATCH 3/6] KVM: x86/mmu: Speed up dirty logging in tdp_mmu_map_handle_target_level

[Paolo Bonzini]

On 31/07/21 00:37, David Matlack wrote:
> -	if (new_spte == iter->old_spte)
> +	if (new_spte == iter->old_spte) {
>   		ret = RET_PF_SPURIOUS;
> -	else if (!tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte))
> -		return RET_PF_RETRY;
> +	} else {
> +		if (!tdp_mmu_set_spte_atomic_no_dirty_log(vcpu->kvm, iter, new_spte))
> +			return RET_PF_RETRY;
> +
> +		/*
> +		 * Mark the gfn dirty here rather that through the vcpu-agnostic
> +		 * handle_changed_spte_dirty_log to leverage vcpu->lru_slot_index.
> +		 */
> +		if (is_writable_pte(new_spte))
> +			kvm_vcpu_mark_page_dirty(vcpu, iter->gfn);
> +	}

Looking at the remaining callers of tdp_mmu_set_spte_atomic we have:

* tdp_mmu_zap_spte_atomic calls it with REMOVED_SPTE as the new_spte, 
which is never writable

* kvm_tdp_mmu_map calls it for nonleaf SPTEs, which are always writable 
but should not be dirty.


So I think you should:

* change those two to tdp_mmu_set_spte_atomic_no_dirty_log

* add a WARN_ON_ONCE(iter->level > PG_LEVEL_4K) to tdp_mmu_set_spte_atomic

* put the kvm_vcpu_mark_page_dirty code directly in 
tdp_mmu_set_spte_atomic, instead of the call to 
handle_changed_spte_dirty_log

(I can't exclude I'm missing something though).

Paolo

Re: [PATCH 4/6] KVM: x86/mmu: Leverage vcpu->lru_slot_index for rmap_add and rmap_recycle

[Paolo Bonzini]

On 31/07/21 00:37, David Matlack wrote:
> rmap_add() and rmap_recycle() both run in the context of the vCPU and
> thus we can use kvm_vcpu_gfn_to_memslot() to look up the memslot. This
> enables rmap_add() and rmap_recycle() to take advantage of
> vcpu->lru_slot_index and avoid expensive memslot searching.
> 
> This change improves the performance of "Populate memory time" in
> dirty_log_perf_test with tdp_mmu=N. In addition to improving the
> performance, "Populate memory time" no longer scales with the number
> of memslots in the VM.
> 
> Command                         | Before           | After
> ------------------------------- | ---------------- | -------------
> ./dirty_log_perf_test -v64 -x1  | 15.18001570s     | 14.99469366s
> ./dirty_log_perf_test -v64 -x64 | 18.71336392s     | 14.98675076s
> 
> Signed-off-by: David Matlack <dmatlack@google.com>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> ---
>   arch/x86/kvm/mmu/mmu.c | 35 ++++++++++++++++++++---------------
>   1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index a8cdfd8d45c4..370a6ebc2ede 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1043,17 +1043,6 @@ static struct kvm_rmap_head *__gfn_to_rmap(gfn_t gfn, int level,
>   	return &slot->arch.rmap[level - PG_LEVEL_4K][idx];
>   }
>   
> -static struct kvm_rmap_head *gfn_to_rmap(struct kvm *kvm, gfn_t gfn,
> -					 struct kvm_mmu_page *sp)
> -{
> -	struct kvm_memslots *slots;
> -	struct kvm_memory_slot *slot;
> -
> -	slots = kvm_memslots_for_spte_role(kvm, sp->role);
> -	slot = __gfn_to_memslot(slots, gfn);
> -	return __gfn_to_rmap(gfn, sp->role.level, slot);
> -}
> -
>   static bool rmap_can_add(struct kvm_vcpu *vcpu)
>   {
>   	struct kvm_mmu_memory_cache *mc;
> @@ -1064,24 +1053,39 @@ static bool rmap_can_add(struct kvm_vcpu *vcpu)
>   
>   static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
>   {
> +	struct kvm_memory_slot *slot;
>   	struct kvm_mmu_page *sp;
>   	struct kvm_rmap_head *rmap_head;
>   
>   	sp = sptep_to_sp(spte);
>   	kvm_mmu_page_set_gfn(sp, spte - sp->spt, gfn);
> -	rmap_head = gfn_to_rmap(vcpu->kvm, gfn, sp);
> +	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
> +	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
>   	return pte_list_add(vcpu, spte, rmap_head);
>   }
>   
> +
>   static void rmap_remove(struct kvm *kvm, u64 *spte)
>   {
> +	struct kvm_memslots *slots;
> +	struct kvm_memory_slot *slot;
>   	struct kvm_mmu_page *sp;
>   	gfn_t gfn;
>   	struct kvm_rmap_head *rmap_head;
>   
>   	sp = sptep_to_sp(spte);
>   	gfn = kvm_mmu_page_get_gfn(sp, spte - sp->spt);
> -	rmap_head = gfn_to_rmap(kvm, gfn, sp);
> +
> +	/*
> +	 * Unlike rmap_add and rmap_recycle, rmap_remove does not run in the
> +	 * context of a vCPU so have to determine which memslots to use based
> +	 * on context information in sp->role.
> +	 */
> +	slots = kvm_memslots_for_spte_role(kvm, sp->role);
> +
> +	slot = __gfn_to_memslot(slots, gfn);
> +	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
> +
>   	__pte_list_remove(spte, rmap_head);
>   }
>   
> @@ -1628,12 +1632,13 @@ static bool kvm_test_age_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
>   
>   static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
>   {
> +	struct kvm_memory_slot *slot;
>   	struct kvm_rmap_head *rmap_head;
>   	struct kvm_mmu_page *sp;
>   
>   	sp = sptep_to_sp(spte);
> -
> -	rmap_head = gfn_to_rmap(vcpu->kvm, gfn, sp);
> +	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
> +	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
>   
>   	kvm_unmap_rmapp(vcpu->kvm, rmap_head, NULL, gfn, sp->role.level, __pte(0));
>   	kvm_flush_remote_tlbs_with_address(vcpu->kvm, sp->gfn,
> 

Re: [PATCH 5/6] KVM: x86/mmu: Rename __gfn_to_rmap to gfn_to_rmap

[Paolo Bonzini]

On 31/07/21 00:37, David Matlack wrote:
> gfn_to_rmap was removed in the previous patch so there is no need to
> retain the double underscore on __gfn_to_rmap.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>   arch/x86/kvm/mmu/mmu.c | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

apart from the extra mmu_audit.c changes.

Paolo

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 370a6ebc2ede..df493729d86c 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1034,8 +1034,8 @@ static bool pte_list_destroy(struct kvm_rmap_head *rmap_head,
>   	return true;
>   }
>   
> -static struct kvm_rmap_head *__gfn_to_rmap(gfn_t gfn, int level,
> -					   const struct kvm_memory_slot *slot)
> +static struct kvm_rmap_head *gfn_to_rmap(gfn_t gfn, int level,
> +					 const struct kvm_memory_slot *slot)
>   {
>   	unsigned long idx;
>   
> @@ -1060,7 +1060,7 @@ static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
>   	sp = sptep_to_sp(spte);
>   	kvm_mmu_page_set_gfn(sp, spte - sp->spt, gfn);
>   	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
> -	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
> +	rmap_head = gfn_to_rmap(gfn, sp->role.level, slot);
>   	return pte_list_add(vcpu, spte, rmap_head);
>   }
>   
> @@ -1084,7 +1084,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
>   	slots = kvm_memslots_for_spte_role(kvm, sp->role);
>   
>   	slot = __gfn_to_memslot(slots, gfn);
> -	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
> +	rmap_head = gfn_to_rmap(gfn, sp->role.level, slot);
>   
>   	__pte_list_remove(spte, rmap_head);
>   }
> @@ -1306,8 +1306,8 @@ static void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>   		return;
>   
>   	while (mask) {
> -		rmap_head = __gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
> -					  PG_LEVEL_4K, slot);
> +		rmap_head = gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
> +					PG_LEVEL_4K, slot);
>   		__rmap_write_protect(kvm, rmap_head, false);
>   
>   		/* clear the first set bit */
> @@ -1339,8 +1339,8 @@ static void kvm_mmu_clear_dirty_pt_masked(struct kvm *kvm,
>   		return;
>   
>   	while (mask) {
> -		rmap_head = __gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
> -					  PG_LEVEL_4K, slot);
> +		rmap_head = gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
> +					PG_LEVEL_4K, slot);
>   		__rmap_clear_dirty(kvm, rmap_head, slot);
>   
>   		/* clear the first set bit */
> @@ -1406,7 +1406,7 @@ bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
>   
>   	if (kvm_memslots_have_rmaps(kvm)) {
>   		for (i = min_level; i <= KVM_MAX_HUGEPAGE_LEVEL; ++i) {
> -			rmap_head = __gfn_to_rmap(gfn, i, slot);
> +			rmap_head = gfn_to_rmap(gfn, i, slot);
>   			write_protected |= __rmap_write_protect(kvm, rmap_head, true);
>   		}
>   	}
> @@ -1506,9 +1506,8 @@ rmap_walk_init_level(struct slot_rmap_walk_iterator *iterator, int level)
>   {
>   	iterator->level = level;
>   	iterator->gfn = iterator->start_gfn;
> -	iterator->rmap = __gfn_to_rmap(iterator->gfn, level, iterator->slot);
> -	iterator->end_rmap = __gfn_to_rmap(iterator->end_gfn, level,
> -					   iterator->slot);
> +	iterator->rmap = gfn_to_rmap(iterator->gfn, level, iterator->slot);
> +	iterator->end_rmap = gfn_to_rmap(iterator->end_gfn, level, iterator->slot);
>   }
>   
>   static void
> @@ -1638,7 +1637,7 @@ static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
>   
>   	sp = sptep_to_sp(spte);
>   	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
> -	rmap_head = __gfn_to_rmap(gfn, sp->role.level, slot);
> +	rmap_head = gfn_to_rmap(gfn, sp->role.level, slot);
>   
>   	kvm_unmap_rmapp(vcpu->kvm, rmap_head, NULL, gfn, sp->role.level, __pte(0));
>   	kvm_flush_remote_tlbs_with_address(vcpu->kvm, sp->gfn,
> 

Re: [PATCH 1/6] KVM: Cache the least recently used slot index per vCPU

[David Matlack]

On Mon, Aug 02, 2021 at 04:36:24PM +0200, Paolo Bonzini wrote:
> On 31/07/21 00:37, David Matlack wrote:
> > The memslot for a given gfn is looked up multiple times during page
> > fault handling. Avoid binary searching for it multiple times by caching
> > the least recently used slot. There is an existing VM-wide LRU slot but
> > that does not work well for cases where vCPUs are accessing memory in
> > different slots (see performance data below).
> > 
> > Another benefit of caching the least recently use slot (versus looking
> > up the slot once and passing around a pointer) is speeding up memslot
> > lookups *across* faults and during spte prefetching.
> > 
> > To measure the performance of this change I ran dirty_log_perf_test with
> > 64 vCPUs and 64 memslots and measured "Populate memory time" and
> > "Iteration 2 dirty memory time".  Tests were ran with eptad=N to force
> > dirty logging to use fast_page_fault so its performance could be
> > measured.
> > 
> > Config     | Metric                        | Before | After
> > ---------- | ----------------------------- | ------ | ------
> > tdp_mmu=Y  | Populate memory time          | 6.76s  | 5.47s
> > tdp_mmu=Y  | Iteration 2 dirty memory time | 2.83s  | 0.31s
> > tdp_mmu=N  | Populate memory time          | 20.4s  | 18.7s
> > tdp_mmu=N  | Iteration 2 dirty memory time | 2.65s  | 0.30s
> > 
> > The "Iteration 2 dirty memory time" results are especially compelling
> > because they are equivalent to running the same test with a single
> > memslot. In other words, fast_page_fault performance no longer scales
> > with the number of memslots.
> > 
> > Signed-off-by: David Matlack <dmatlack@google.com>
> 
> It's the *most* recently used slot index, of course. :)  That's true of
> lru_slot as well.

*facepalm*

I'll include a patch in v2 to fix the name of the existing lru_slot to
something like mru_slot or last_used_slot.

> 
> > +static inline struct kvm_memory_slot *get_slot(struct kvm_memslots *slots, int slot_index)
> > +{
> > +	if (slot_index < 0 || slot_index >= slots->used_slots)
> > +		return NULL;
> > +
> > +	return &slots->memslots[slot_index];
> > +}
> > +
> 
> Since there are plenty of arrays inside struct kvm_memory_slot*, do we want
> to protect this against speculative out-of-bounds accesses with
> array_index_nospec?

I'm not sure when it makes sense to use array_index_nospec. Perhaps this
is a good candidate since vcpu->lru_slot_index and the length of
memslots[] can both be controlled by userspace.

I'll play around with adding it to see if there are any performance
trade-offs.

> 
> > +static inline struct kvm_memory_slot *
> > +search_memslots(struct kvm_memslots *slots, gfn_t gfn)
> > +{
> > +	int slot_index = __search_memslots(slots, gfn);
> > +
> > +	return get_slot(slots, slot_index);
> >  }
> 
> Let's use this occasion to remove the duplication between __gfn_to_memslot
> and search_memslots; you can make search_memslots do the search and palce
> the LRU (ehm, MRU) code to __gfn_to_memslot only.  So:
> 
> - the new patch 1 (something like "make search_memslots search without LRU
> caching") is basically this series's patch 2, plus a tree-wide replacement
> of search_memslots with __gfn_to_memslot.
> 
> - the new patch 2 is this series's patch 1, except kvm_vcpu_gfn_to_memslot
> uses search_memslots instead of __search_memslots.  The comments in patch
> 2's commit message about the double misses move to this commit message.

Will do.

> 
> > +	if (slot)
> > +		vcpu->lru_slot_index = slot_index;
> 
> Let's call it lru_slot for consistency with the field of struct
> kvm_memory_slots.

I'll make sure the two have consistent names when I rename them to get
rid of "lru".

> 
> Paolo
> 

Re: [PATCH 3/6] KVM: x86/mmu: Speed up dirty logging in tdp_mmu_map_handle_target_level

[David Matlack]

On Mon, Aug 02, 2021 at 04:58:17PM +0200, Paolo Bonzini wrote:
> On 31/07/21 00:37, David Matlack wrote:
> > -	if (new_spte == iter->old_spte)
> > +	if (new_spte == iter->old_spte) {
> >   		ret = RET_PF_SPURIOUS;
> > -	else if (!tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte))
> > -		return RET_PF_RETRY;
> > +	} else {
> > +		if (!tdp_mmu_set_spte_atomic_no_dirty_log(vcpu->kvm, iter, new_spte))
> > +			return RET_PF_RETRY;
> > +
> > +		/*
> > +		 * Mark the gfn dirty here rather that through the vcpu-agnostic
> > +		 * handle_changed_spte_dirty_log to leverage vcpu->lru_slot_index.
> > +		 */
> > +		if (is_writable_pte(new_spte))
> > +			kvm_vcpu_mark_page_dirty(vcpu, iter->gfn);
> > +	}
> 
> Looking at the remaining callers of tdp_mmu_set_spte_atomic we have:
> 
> * tdp_mmu_zap_spte_atomic calls it with REMOVED_SPTE as the new_spte, which
> is never writable
> 
> * kvm_tdp_mmu_map calls it for nonleaf SPTEs, which are always writable but
> should not be dirty.
> 
> 
> So I think you should:
> 
> * change those two to tdp_mmu_set_spte_atomic_no_dirty_log
> 
> * add a WARN_ON_ONCE(iter->level > PG_LEVEL_4K) to tdp_mmu_set_spte_atomic
> 
> * put the kvm_vcpu_mark_page_dirty code directly in tdp_mmu_set_spte_atomic,
> instead of the call to handle_changed_spte_dirty_log
> 
> (I can't exclude I'm missing something though).

Makes sense. I'll take a look to confirm and make those changes in v2.

> 
> Paolo
> 

Re: [PATCH 1/6] KVM: Cache the least recently used slot index per vCPU

[Sean Christopherson]

On Mon, Aug 02, 2021, David Matlack wrote:
> I'll include a patch in v2 to fix the name of the existing lru_slot to
> something like mru_slot or last_used_slot.

I vote for last_used_slot, "mru" isn't exactly a common acronym, and the "mr" part
could easily be misinterpreted as Memory Region.