kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU
@ 2020-10-27 17:59 Ben Gardon
  2020-10-27 17:59 ` [PATCH 2/2] kvm: x86/mmu: Add TDP MMU SPTE changed trace point Ben Gardon
  2020-11-16 19:15 ` [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU Ben Gardon
  0 siblings, 2 replies; 5+ messages in thread
From: Ben Gardon @ 2020-10-27 17:59 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Peter Xu, Sean Christopherson, Peter Shier,
	Junaid Shahid, Peter Feiner, Ben Gardon

The TDP MMU was initially implemented without some of the usual
tracepoints found in mmu.c. Correct this discrepancy by adding the
missing trace points to the TDP MMU.

Tested: ran the demand paging selftest on an Intel Skylake machine with
	all the trace points used by the TDP MMU enabled and observed
	them firing with expected values.

This patch can be viewed in Gerrit at:
https://linux-review.googlesource.com/c/virt/kvm/kvm/+/3812

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 27e381c9da6c2..047e2d966abef 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -7,6 +7,8 @@
 #include "tdp_mmu.h"
 #include "spte.h"
 
+#include <trace/events/kvm.h>
+
 #ifdef CONFIG_X86_64
 static bool __read_mostly tdp_mmu_enabled = false;
 module_param_named(tdp_mmu, tdp_mmu_enabled, bool, 0644);
@@ -101,6 +103,8 @@ static struct kvm_mmu_page *alloc_tdp_mmu_page(struct kvm_vcpu *vcpu, gfn_t gfn,
 	sp->gfn = gfn;
 	sp->tdp_mmu_page = true;
 
+	trace_kvm_mmu_get_page(sp, true);
+
 	return sp;
 }
 
@@ -271,6 +275,8 @@ static void __handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn,
 		pt = spte_to_child_pt(old_spte, level);
 		sp = sptep_to_sp(pt);
 
+		trace_kvm_mmu_prepare_zap_page(sp);
+
 		list_del(&sp->link);
 
 		if (sp->lpage_disallowed)
@@ -473,11 +479,13 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu, int write,
 	if (unlikely(is_noslot_pfn(pfn))) {
 		new_spte = make_mmio_spte(vcpu, iter->gfn, ACC_ALL);
 		trace_mark_mmio_spte(iter->sptep, iter->gfn, new_spte);
-	} else
+	} else {
 		make_spte_ret = make_spte(vcpu, ACC_ALL, iter->level, iter->gfn,
 					 pfn, iter->old_spte, prefault, true,
 					 map_writable, !shadow_accessed_mask,
 					 &new_spte);
+		trace_kvm_mmu_set_spte(iter->level, iter->gfn, iter->sptep);
+	}
 
 	if (new_spte == iter->old_spte)
 		ret = RET_PF_SPURIOUS;
@@ -691,6 +699,8 @@ static int age_gfn_range(struct kvm *kvm, struct kvm_memory_slot *slot,
 
 		tdp_mmu_set_spte_no_acc_track(kvm, &iter, new_spte);
 		young = 1;
+
+		trace_kvm_age_page(iter.gfn, iter.level, slot, young);
 	}
 
 	return young;
-- 
2.29.0.rc2.309.g374f81d7ae-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] kvm: x86/mmu: Add TDP MMU SPTE changed trace point
  2020-10-27 17:59 [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU Ben Gardon
@ 2020-10-27 17:59 ` Ben Gardon
  2020-11-16 19:15 ` [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU Ben Gardon
  1 sibling, 0 replies; 5+ messages in thread
From: Ben Gardon @ 2020-10-27 17:59 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Peter Xu, Sean Christopherson, Peter Shier,
	Junaid Shahid, Peter Feiner, Ben Gardon

Add an extremely verbose trace point to the TDP MMU to log all SPTE
changes, regardless of callstack / motivation. This is useful when a
complete picture of the paging structure is needed or a change cannot be
explained with the other, existing trace points.

Tested: ran the demand paging selftest on an Intel Skylake machine with
	all the trace points used by the TDP MMU enabled and observed
	them firing with expected values.

This patch can be viewed in Gerrit at:
https://linux-review.googlesource.com/c/virt/kvm/kvm/+/3813

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 arch/x86/kvm/mmu/mmutrace.h | 29 +++++++++++++++++++++++++++++
 arch/x86/kvm/mmu/tdp_mmu.c  |  2 ++
 2 files changed, 31 insertions(+)

diff --git a/arch/x86/kvm/mmu/mmutrace.h b/arch/x86/kvm/mmu/mmutrace.h
index 213699b27b448..e798489b56b55 100644
--- a/arch/x86/kvm/mmu/mmutrace.h
+++ b/arch/x86/kvm/mmu/mmutrace.h
@@ -381,6 +381,35 @@ TRACE_EVENT(
 	)
 );
 
+TRACE_EVENT(
+	kvm_tdp_mmu_spte_changed,
+	TP_PROTO(int as_id, gfn_t gfn, int level, u64 old_spte, u64 new_spte),
+	TP_ARGS(as_id, gfn, level, old_spte, new_spte),
+
+	TP_STRUCT__entry(
+		__field(u64, gfn)
+		__field(u64, old_spte)
+		__field(u64, new_spte)
+		/* Level cannot be larger than 5 on x86, so it fits in a u8. */
+		__field(u8, level)
+		/* as_id can only be 0 or 1 x86, so it fits in a u8. */
+		__field(u8, as_id)
+	),
+
+	TP_fast_assign(
+		__entry->gfn = gfn;
+		__entry->old_spte = old_spte;
+		__entry->new_spte = new_spte;
+		__entry->level = level;
+		__entry->as_id = as_id;
+	),
+
+	TP_printk("as id %d gfn %llx level %d old_spte %llx new_spte %llx",
+		  __entry->as_id, __entry->gfn, __entry->level,
+		  __entry->old_spte, __entry->new_spte
+	)
+);
+
 #endif /* _TRACE_KVMMMU_H */
 
 #undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 047e2d966abef..5820c36ccfdca 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -241,6 +241,8 @@ static void __handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn,
 	if (old_spte == new_spte)
 		return;
 
+	trace_kvm_tdp_mmu_spte_changed(as_id, gfn, level, old_spte, new_spte);
+
 	/*
 	 * The only times a SPTE should be changed from a non-present to
 	 * non-present state is when an MMIO entry is installed/modified/
-- 
2.29.0.rc2.309.g374f81d7ae-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU
  2020-10-27 17:59 [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU Ben Gardon
  2020-10-27 17:59 ` [PATCH 2/2] kvm: x86/mmu: Add TDP MMU SPTE changed trace point Ben Gardon
@ 2020-11-16 19:15 ` Ben Gardon
  2020-11-16 19:25   ` Paolo Bonzini
  1 sibling, 1 reply; 5+ messages in thread
From: Ben Gardon @ 2020-11-16 19:15 UTC (permalink / raw)
  To: LKML, kvm
  Cc: Paolo Bonzini, Peter Xu, Sean Christopherson, Peter Shier,
	Junaid Shahid, Peter Feiner

On Tue, Oct 27, 2020 at 10:59 AM Ben Gardon <bgardon@google.com> wrote:
>
> The TDP MMU was initially implemented without some of the usual
> tracepoints found in mmu.c. Correct this discrepancy by adding the
> missing trace points to the TDP MMU.
>
> Tested: ran the demand paging selftest on an Intel Skylake machine with
>         all the trace points used by the TDP MMU enabled and observed
>         them firing with expected values.
>
> This patch can be viewed in Gerrit at:
> https://linux-review.googlesource.com/c/virt/kvm/kvm/+/3812
>
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  arch/x86/kvm/mmu/tdp_mmu.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 27e381c9da6c2..047e2d966abef 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -7,6 +7,8 @@
>  #include "tdp_mmu.h"
>  #include "spte.h"
>
> +#include <trace/events/kvm.h>
> +
>  #ifdef CONFIG_X86_64
>  static bool __read_mostly tdp_mmu_enabled = false;
>  module_param_named(tdp_mmu, tdp_mmu_enabled, bool, 0644);
> @@ -101,6 +103,8 @@ static struct kvm_mmu_page *alloc_tdp_mmu_page(struct kvm_vcpu *vcpu, gfn_t gfn,
>         sp->gfn = gfn;
>         sp->tdp_mmu_page = true;
>
> +       trace_kvm_mmu_get_page(sp, true);
> +
>         return sp;
>  }
>
> @@ -271,6 +275,8 @@ static void __handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn,
>                 pt = spte_to_child_pt(old_spte, level);
>                 sp = sptep_to_sp(pt);
>
> +               trace_kvm_mmu_prepare_zap_page(sp);
> +
>                 list_del(&sp->link);
>
>                 if (sp->lpage_disallowed)
> @@ -473,11 +479,13 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu, int write,
>         if (unlikely(is_noslot_pfn(pfn))) {
>                 new_spte = make_mmio_spte(vcpu, iter->gfn, ACC_ALL);
>                 trace_mark_mmio_spte(iter->sptep, iter->gfn, new_spte);
> -       } else
> +       } else {
>                 make_spte_ret = make_spte(vcpu, ACC_ALL, iter->level, iter->gfn,
>                                          pfn, iter->old_spte, prefault, true,
>                                          map_writable, !shadow_accessed_mask,
>                                          &new_spte);
> +               trace_kvm_mmu_set_spte(iter->level, iter->gfn, iter->sptep);
> +       }
>
>         if (new_spte == iter->old_spte)
>                 ret = RET_PF_SPURIOUS;
> @@ -691,6 +699,8 @@ static int age_gfn_range(struct kvm *kvm, struct kvm_memory_slot *slot,
>
>                 tdp_mmu_set_spte_no_acc_track(kvm, &iter, new_spte);
>                 young = 1;
> +
> +               trace_kvm_age_page(iter.gfn, iter.level, slot, young);
>         }
>
>         return young;
> --
> 2.29.0.rc2.309.g374f81d7ae-goog
>

If anyone has time to review this short series, I'd appreciate it. I
don't want these to get lost in the shuffle.
Thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU
  2020-11-16 19:15 ` [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU Ben Gardon
@ 2020-11-16 19:25   ` Paolo Bonzini
  2020-11-17 11:04     ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2020-11-16 19:25 UTC (permalink / raw)
  To: Ben Gardon, LKML, kvm
  Cc: Peter Xu, Sean Christopherson, Peter Shier, Junaid Shahid, Peter Feiner

On 16/11/20 20:15, Ben Gardon wrote:
> If anyone has time to review this short series, I'd appreciate it. I
> don't want these to get lost in the shuffle.
> Thanks!

Yup, it's on the list. :)

Paolo


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU
  2020-11-16 19:25   ` Paolo Bonzini
@ 2020-11-17 11:04     ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2020-11-17 11:04 UTC (permalink / raw)
  To: Ben Gardon, LKML, kvm
  Cc: Peter Xu, Sean Christopherson, Peter Shier, Junaid Shahid, Peter Feiner

On 16/11/20 20:25, Paolo Bonzini wrote:
> On 16/11/20 20:15, Ben Gardon wrote:
>> If anyone has time to review this short series, I'd appreciate it. I
>> don't want these to get lost in the shuffle.
>> Thanks!
> 
> Yup, it's on the list. :)

Queued, thanks.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-17 11:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27 17:59 [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU Ben Gardon
2020-10-27 17:59 ` [PATCH 2/2] kvm: x86/mmu: Add TDP MMU SPTE changed trace point Ben Gardon
2020-11-16 19:15 ` [PATCH 1/2] kvm: x86/mmu: Add existing trace points to TDP MMU Ben Gardon
2020-11-16 19:25   ` Paolo Bonzini
2020-11-17 11:04     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).