* [PATCH v3 1/4] KVM: X86: Trace vcpu_id for vmexit
2019-09-05 2:36 [PATCH v3 0/4] KVM: X86: Some tracepoint enhancements Peter Xu
@ 2019-09-05 2:36 ` Peter Xu
2019-09-05 15:59 ` Sean Christopherson
2019-09-05 2:36 ` [PATCH v3 2/4] KVM: X86: Remove tailing newline for tracepoints Peter Xu
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Peter Xu @ 2019-09-05 2:36 UTC (permalink / raw)
To: kvm
Cc: Sean Christopherson, Paolo Bonzini, Wanpeng Li, Krish Sadhukhan, peterx
Tracing the ID helps to pair vmenters and vmexits for guests with
multiple vCPUs.
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
arch/x86/kvm/trace.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index b5c831e79094..20d6cac9f157 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -232,17 +232,20 @@ TRACE_EVENT(kvm_exit,
__field( u32, isa )
__field( u64, info1 )
__field( u64, info2 )
+ __field( unsigned int, vcpu_id )
),
TP_fast_assign(
__entry->exit_reason = exit_reason;
__entry->guest_rip = kvm_rip_read(vcpu);
__entry->isa = isa;
+ __entry->vcpu_id = vcpu->vcpu_id;
kvm_x86_ops->get_exit_info(vcpu, &__entry->info1,
&__entry->info2);
),
- TP_printk("reason %s rip 0x%lx info %llx %llx",
+ TP_printk("vcpu %u reason %s rip 0x%lx info %llx %llx",
+ __entry->vcpu_id,
(__entry->isa == KVM_ISA_VMX) ?
__print_symbolic(__entry->exit_reason, VMX_EXIT_REASONS) :
__print_symbolic(__entry->exit_reason, SVM_EXIT_REASONS),
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 2/4] KVM: X86: Remove tailing newline for tracepoints
2019-09-05 2:36 [PATCH v3 0/4] KVM: X86: Some tracepoint enhancements Peter Xu
2019-09-05 2:36 ` [PATCH v3 1/4] KVM: X86: Trace vcpu_id for vmexit Peter Xu
@ 2019-09-05 2:36 ` Peter Xu
2019-09-05 2:36 ` [PATCH v3 3/4] KVM: X86: Tune PLE Window tracepoint Peter Xu
2019-09-05 2:36 ` [PATCH v3 4/4] KVM: VMX: Change ple_window type to unsigned int Peter Xu
3 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2019-09-05 2:36 UTC (permalink / raw)
To: kvm
Cc: Sean Christopherson, Paolo Bonzini, Wanpeng Li, Krish Sadhukhan, peterx
It's done by TP_printk() already.
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
arch/x86/kvm/trace.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 20d6cac9f157..8a7570f8c943 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1323,7 +1323,7 @@ TRACE_EVENT(kvm_avic_incomplete_ipi,
__entry->index = index;
),
- TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u\n",
+ TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u",
__entry->vcpu, __entry->icrh, __entry->icrl,
__entry->id, __entry->index)
);
@@ -1348,7 +1348,7 @@ TRACE_EVENT(kvm_avic_unaccelerated_access,
__entry->vec = vec;
),
- TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x\n",
+ TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x",
__entry->vcpu,
__entry->offset,
__print_symbolic(__entry->offset, kvm_trace_symbol_apic),
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 3/4] KVM: X86: Tune PLE Window tracepoint
2019-09-05 2:36 [PATCH v3 0/4] KVM: X86: Some tracepoint enhancements Peter Xu
2019-09-05 2:36 ` [PATCH v3 1/4] KVM: X86: Trace vcpu_id for vmexit Peter Xu
2019-09-05 2:36 ` [PATCH v3 2/4] KVM: X86: Remove tailing newline for tracepoints Peter Xu
@ 2019-09-05 2:36 ` Peter Xu
2019-09-05 2:36 ` [PATCH v3 4/4] KVM: VMX: Change ple_window type to unsigned int Peter Xu
3 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2019-09-05 2:36 UTC (permalink / raw)
To: kvm
Cc: Sean Christopherson, Paolo Bonzini, Wanpeng Li, Krish Sadhukhan, peterx
The PLE window tracepoint triggers even if the window is not changed,
and the wording can be a bit confusing too. One example line:
kvm_ple_window: vcpu 0: ple_window 4096 (shrink 4096)
It easily let people think of "the window now is 4096 which is
shrinked", but the truth is the value actually didn't change (4096).
Let's only dump this message if the value really changed, and we make
the message even simpler like:
kvm_ple_window: vcpu 4 old 4096 new 8192 (growed)
Signed-off-by: Peter Xu <peterx@redhat.com>
---
arch/x86/kvm/svm.c | 16 ++++++++--------
arch/x86/kvm/trace.h | 21 ++++++---------------
arch/x86/kvm/vmx/vmx.c | 14 ++++++++------
arch/x86/kvm/x86.c | 2 +-
4 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d685491fce4d..d5cb6b5a9254 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1269,11 +1269,11 @@ static void grow_ple_window(struct kvm_vcpu *vcpu)
pause_filter_count_grow,
pause_filter_count_max);
- if (control->pause_filter_count != old)
+ if (control->pause_filter_count != old) {
mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
-
- trace_kvm_ple_window_grow(vcpu->vcpu_id,
- control->pause_filter_count, old);
+ trace_kvm_ple_window_update(vcpu->vcpu_id,
+ control->pause_filter_count, old);
+ }
}
static void shrink_ple_window(struct kvm_vcpu *vcpu)
@@ -1287,11 +1287,11 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu)
pause_filter_count,
pause_filter_count_shrink,
pause_filter_count);
- if (control->pause_filter_count != old)
+ if (control->pause_filter_count != old) {
mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
-
- trace_kvm_ple_window_shrink(vcpu->vcpu_id,
- control->pause_filter_count, old);
+ trace_kvm_ple_window_update(vcpu->vcpu_id,
+ control->pause_filter_count, old);
+ }
}
static __init int svm_hardware_setup(void)
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 8a7570f8c943..f1177e03768f 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -890,36 +890,27 @@ TRACE_EVENT(kvm_pml_full,
TP_printk("vcpu %d: PML full", __entry->vcpu_id)
);
-TRACE_EVENT(kvm_ple_window,
- TP_PROTO(bool grow, unsigned int vcpu_id, int new, int old),
- TP_ARGS(grow, vcpu_id, new, old),
+TRACE_EVENT(kvm_ple_window_update,
+ TP_PROTO(unsigned int vcpu_id, int new, int old),
+ TP_ARGS(vcpu_id, new, old),
TP_STRUCT__entry(
- __field( bool, grow )
__field( unsigned int, vcpu_id )
__field( int, new )
__field( int, old )
),
TP_fast_assign(
- __entry->grow = grow;
__entry->vcpu_id = vcpu_id;
__entry->new = new;
__entry->old = old;
),
- TP_printk("vcpu %u: ple_window %d (%s %d)",
- __entry->vcpu_id,
- __entry->new,
- __entry->grow ? "grow" : "shrink",
- __entry->old)
+ TP_printk("vcpu %u old %d new %d (%s)",
+ __entry->vcpu_id, __entry->old, __entry->new,
+ __entry->old < __entry->new ? "growed" : "shrinked")
);
-#define trace_kvm_ple_window_grow(vcpu_id, new, old) \
- trace_kvm_ple_window(true, vcpu_id, new, old)
-#define trace_kvm_ple_window_shrink(vcpu_id, new, old) \
- trace_kvm_ple_window(false, vcpu_id, new, old)
-
TRACE_EVENT(kvm_pvclock_update,
TP_PROTO(unsigned int vcpu_id, struct pvclock_vcpu_time_info *pvclock),
TP_ARGS(vcpu_id, pvclock),
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 42ed3faa6af8..469c4134a4a7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5233,10 +5233,11 @@ static void grow_ple_window(struct kvm_vcpu *vcpu)
ple_window_grow,
ple_window_max);
- if (vmx->ple_window != old)
+ if (vmx->ple_window != old) {
vmx->ple_window_dirty = true;
-
- trace_kvm_ple_window_grow(vcpu->vcpu_id, vmx->ple_window, old);
+ trace_kvm_ple_window_update(vcpu->vcpu_id,
+ vmx->ple_window, old);
+ }
}
static void shrink_ple_window(struct kvm_vcpu *vcpu)
@@ -5248,10 +5249,11 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu)
ple_window_shrink,
ple_window);
- if (vmx->ple_window != old)
+ if (vmx->ple_window != old) {
vmx->ple_window_dirty = true;
-
- trace_kvm_ple_window_shrink(vcpu->vcpu_id, vmx->ple_window, old);
+ trace_kvm_ple_window_update(vcpu->vcpu_id,
+ vmx->ple_window, old);
+ }
}
/*
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 93b0bd45ac73..69ad184edc90 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10082,7 +10082,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_invlpga);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_skinit);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_intercepts);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_write_tsc_offset);
-EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_ple_window);
+EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_ple_window_update);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_pml_full);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_pi_irte_update);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_avic_unaccelerated_access);
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 4/4] KVM: VMX: Change ple_window type to unsigned int
2019-09-05 2:36 [PATCH v3 0/4] KVM: X86: Some tracepoint enhancements Peter Xu
` (2 preceding siblings ...)
2019-09-05 2:36 ` [PATCH v3 3/4] KVM: X86: Tune PLE Window tracepoint Peter Xu
@ 2019-09-05 2:36 ` Peter Xu
2019-09-05 15:58 ` Sean Christopherson
3 siblings, 1 reply; 8+ messages in thread
From: Peter Xu @ 2019-09-05 2:36 UTC (permalink / raw)
To: kvm
Cc: Sean Christopherson, Paolo Bonzini, Wanpeng Li, Krish Sadhukhan, peterx
The VMX ple_window is 32 bits wide, so logically it can overflow with
an int. The module parameter is declared as unsigned int which is
good, however the dynamic variable is not. Switching all the
ple_window references to use unsigned int.
The tracepoint changes will also affect SVM, but SVM is using an even
smaller width (16 bits) so it's always fine.
Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
arch/x86/kvm/trace.h | 8 ++++----
arch/x86/kvm/vmx/vmx.c | 4 ++--
arch/x86/kvm/vmx/vmx.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index f1177e03768f..ae924566c401 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -891,13 +891,13 @@ TRACE_EVENT(kvm_pml_full,
);
TRACE_EVENT(kvm_ple_window_update,
- TP_PROTO(unsigned int vcpu_id, int new, int old),
+ TP_PROTO(unsigned int vcpu_id, unsigned int new, unsigned int old),
TP_ARGS(vcpu_id, new, old),
TP_STRUCT__entry(
__field( unsigned int, vcpu_id )
- __field( int, new )
- __field( int, old )
+ __field( unsigned int, new )
+ __field( unsigned int, old )
),
TP_fast_assign(
@@ -906,7 +906,7 @@ TRACE_EVENT(kvm_ple_window_update,
__entry->old = old;
),
- TP_printk("vcpu %u old %d new %d (%s)",
+ TP_printk("vcpu %u old %u new %u (%s)",
__entry->vcpu_id, __entry->old, __entry->new,
__entry->old < __entry->new ? "growed" : "shrinked")
);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 469c4134a4a7..1dbb63ffdd6d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5227,7 +5227,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
static void grow_ple_window(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
- int old = vmx->ple_window;
+ unsigned int old = vmx->ple_window;
vmx->ple_window = __grow_ple_window(old, ple_window,
ple_window_grow,
@@ -5243,7 +5243,7 @@ static void grow_ple_window(struct kvm_vcpu *vcpu)
static void shrink_ple_window(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
- int old = vmx->ple_window;
+ unsigned int old = vmx->ple_window;
vmx->ple_window = __shrink_ple_window(old, ple_window,
ple_window_shrink,
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 82d0bc3a4d52..64d5a4890aa9 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -253,7 +253,7 @@ struct vcpu_vmx {
struct nested_vmx nested;
/* Dynamic PLE window. */
- int ple_window;
+ unsigned int ple_window;
bool ple_window_dirty;
bool req_immediate_exit;
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 4/4] KVM: VMX: Change ple_window type to unsigned int
2019-09-05 2:36 ` [PATCH v3 4/4] KVM: VMX: Change ple_window type to unsigned int Peter Xu
@ 2019-09-05 15:58 ` Sean Christopherson
2019-09-06 2:12 ` Peter Xu
0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2019-09-05 15:58 UTC (permalink / raw)
To: Peter Xu; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Krish Sadhukhan
On Thu, Sep 05, 2019 at 10:36:16AM +0800, Peter Xu wrote:
> The VMX ple_window is 32 bits wide, so logically it can overflow with
> an int. The module parameter is declared as unsigned int which is
> good, however the dynamic variable is not. Switching all the
> ple_window references to use unsigned int.
>
> The tracepoint changes will also affect SVM, but SVM is using an even
> smaller width (16 bits) so it's always fine.
>
> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> arch/x86/kvm/trace.h | 8 ++++----
> arch/x86/kvm/vmx/vmx.c | 4 ++--
> arch/x86/kvm/vmx/vmx.h | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index f1177e03768f..ae924566c401 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -891,13 +891,13 @@ TRACE_EVENT(kvm_pml_full,
> );
>
> TRACE_EVENT(kvm_ple_window_update,
> - TP_PROTO(unsigned int vcpu_id, int new, int old),
> + TP_PROTO(unsigned int vcpu_id, unsigned int new, unsigned int old),
> TP_ARGS(vcpu_id, new, old),
>
> TP_STRUCT__entry(
> __field( unsigned int, vcpu_id )
> - __field( int, new )
> - __field( int, old )
> + __field( unsigned int, new )
> + __field( unsigned int, old )
Changing the trace event storage needs to be done in patch 3/4, otherwise
we're knowingly introducing a bug (for one commit). Alternatively, swap
the order of the patches.
> ),
>
> TP_fast_assign(
> @@ -906,7 +906,7 @@ TRACE_EVENT(kvm_ple_window_update,
> __entry->old = old;
> ),
>
> - TP_printk("vcpu %u old %d new %d (%s)",
> + TP_printk("vcpu %u old %u new %u (%s)",
> __entry->vcpu_id, __entry->old, __entry->new,
> __entry->old < __entry->new ? "growed" : "shrinked")
> );
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 469c4134a4a7..1dbb63ffdd6d 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5227,7 +5227,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
> static void grow_ple_window(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> - int old = vmx->ple_window;
> + unsigned int old = vmx->ple_window;
>
> vmx->ple_window = __grow_ple_window(old, ple_window,
> ple_window_grow,
> @@ -5243,7 +5243,7 @@ static void grow_ple_window(struct kvm_vcpu *vcpu)
> static void shrink_ple_window(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> - int old = vmx->ple_window;
> + unsigned int old = vmx->ple_window;
>
> vmx->ple_window = __shrink_ple_window(old, ple_window,
> ple_window_shrink,
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 82d0bc3a4d52..64d5a4890aa9 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -253,7 +253,7 @@ struct vcpu_vmx {
> struct nested_vmx nested;
>
> /* Dynamic PLE window. */
> - int ple_window;
> + unsigned int ple_window;
> bool ple_window_dirty;
>
> bool req_immediate_exit;
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3 4/4] KVM: VMX: Change ple_window type to unsigned int
2019-09-05 15:58 ` Sean Christopherson
@ 2019-09-06 2:12 ` Peter Xu
0 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2019-09-06 2:12 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm, Paolo Bonzini, Wanpeng Li, Krish Sadhukhan
On Thu, Sep 05, 2019 at 08:58:23AM -0700, Sean Christopherson wrote:
> On Thu, Sep 05, 2019 at 10:36:16AM +0800, Peter Xu wrote:
> > The VMX ple_window is 32 bits wide, so logically it can overflow with
> > an int. The module parameter is declared as unsigned int which is
> > good, however the dynamic variable is not. Switching all the
> > ple_window references to use unsigned int.
> >
> > The tracepoint changes will also affect SVM, but SVM is using an even
> > smaller width (16 bits) so it's always fine.
> >
> > Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > arch/x86/kvm/trace.h | 8 ++++----
> > arch/x86/kvm/vmx/vmx.c | 4 ++--
> > arch/x86/kvm/vmx/vmx.h | 2 +-
> > 3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> > index f1177e03768f..ae924566c401 100644
> > --- a/arch/x86/kvm/trace.h
> > +++ b/arch/x86/kvm/trace.h
> > @@ -891,13 +891,13 @@ TRACE_EVENT(kvm_pml_full,
> > );
> >
> > TRACE_EVENT(kvm_ple_window_update,
> > - TP_PROTO(unsigned int vcpu_id, int new, int old),
> > + TP_PROTO(unsigned int vcpu_id, unsigned int new, unsigned int old),
> > TP_ARGS(vcpu_id, new, old),
> >
> > TP_STRUCT__entry(
> > __field( unsigned int, vcpu_id )
> > - __field( int, new )
> > - __field( int, old )
> > + __field( unsigned int, new )
> > + __field( unsigned int, old )
>
> Changing the trace event storage needs to be done in patch 3/4, otherwise
> we're knowingly introducing a bug (for one commit). Alternatively, swap
> the order of the patches.
I'll swap. Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 8+ messages in thread