* [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
@ 2021-06-21 20:43 Krish Sadhukhan
2021-06-21 20:43 ` Krish Sadhukhan
0 siblings, 1 reply; 9+ messages in thread
From: Krish Sadhukhan @ 2021-06-21 20:43 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, jmattson, seanjc, vkuznets, wanpengli, joro
[PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
arch/x86/kvm/trace.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
Krish Sadhukhan (1):
KVM: x86: Show guest mode in kvm_{entry,exit} tracepoints
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-06-21 20:43 [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints Krish Sadhukhan
@ 2021-06-21 20:43 ` Krish Sadhukhan
2021-07-30 19:37 ` Sean Christopherson
2021-08-02 14:18 ` Paolo Bonzini
0 siblings, 2 replies; 9+ messages in thread
From: Krish Sadhukhan @ 2021-06-21 20:43 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, jmattson, seanjc, vkuznets, wanpengli, joro
From debugging point of view, KVM entry and exit tracepoints are important
in that they indicate when a guest starts and exits. When L1 runs L2, there
is no way we can determine from KVM tracing when L1 starts/exits and when
L2 starts/exits as there is no marker in place today in those tracepoints.
Debugging becomes even more difficult when more than one L2 run in an L1 and
there is no way of determining which L2 from which L1 made the entries/exits.
Therefore, showing guest mode in the entry and exit tracepoints
will make debugging much easier. If an L1 runs multiple L2s, though we can
not identify the specific L2 from the entry and exit tracepoints, we still
will be able to determine whether it was L1 or an L2 that made the
entries and the exits. With this patch KVM entry and exit tracepoints will
show "guest_mode = 0" if it is a guest and "guest_mode = 1" if it is a
nested guest.
Signed-off-by: Krish Sdhukhan <krish.sadhukhan@oracle.com>
---
arch/x86/kvm/trace.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index b484141ea15b..44dba26c6be2 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -21,14 +21,17 @@ TRACE_EVENT(kvm_entry,
TP_STRUCT__entry(
__field( unsigned int, vcpu_id )
__field( unsigned long, rip )
+ __field( bool, guest_mode )
),
TP_fast_assign(
__entry->vcpu_id = vcpu->vcpu_id;
__entry->rip = kvm_rip_read(vcpu);
+ __entry->guest_mode = is_guest_mode(vcpu);
),
- TP_printk("vcpu %u, rip 0x%lx", __entry->vcpu_id, __entry->rip)
+ TP_printk("vcpu %u, rip 0x%lx, guest_mode %d", __entry->vcpu_id,
+ __entry->rip, __entry->guest_mode)
);
/*
@@ -285,6 +288,7 @@ TRACE_EVENT(name, \
__field( u32, intr_info ) \
__field( u32, error_code ) \
__field( unsigned int, vcpu_id ) \
+ __field( bool, guest_mode ) \
), \
\
TP_fast_assign( \
@@ -295,15 +299,17 @@ TRACE_EVENT(name, \
static_call(kvm_x86_get_exit_info)(vcpu, &__entry->info1, \
&__entry->info2, \
&__entry->intr_info, \
- &__entry->error_code); \
+ &__entry->error_code); \
+ __entry->guest_mode = is_guest_mode(vcpu); \
), \
\
TP_printk("vcpu %u reason %s%s%s rip 0x%lx info1 0x%016llx " \
- "info2 0x%016llx intr_info 0x%08x error_code 0x%08x", \
- __entry->vcpu_id, \
+ "info2 0x%016llx intr_info 0x%08x error_code 0x%08x " \
+ "guest_mode = %d", __entry->vcpu_id, \
kvm_print_exit_reason(__entry->exit_reason, __entry->isa), \
__entry->guest_rip, __entry->info1, __entry->info2, \
- __entry->intr_info, __entry->error_code) \
+ __entry->intr_info, __entry->error_code, \
+ __entry->guest_mode) \
)
/*
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-06-21 20:43 ` Krish Sadhukhan
@ 2021-07-30 19:37 ` Sean Christopherson
2021-08-02 14:18 ` Paolo Bonzini
1 sibling, 0 replies; 9+ messages in thread
From: Sean Christopherson @ 2021-07-30 19:37 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: kvm, pbonzini, jmattson, vkuznets, wanpengli, joro
On Mon, Jun 21, 2021, Krish Sadhukhan wrote:
> From debugging point of view, KVM entry and exit tracepoints are important
> in that they indicate when a guest starts and exits. When L1 runs L2, there
> is no way we can determine from KVM tracing when L1 starts/exits and when
> L2 starts/exits as there is no marker in place today in those tracepoints.
> Debugging becomes even more difficult when more than one L2 run in an L1 and
> there is no way of determining which L2 from which L1 made the entries/exits.
> Therefore, showing guest mode in the entry and exit tracepoints
> will make debugging much easier.
> If an L1 runs multiple L2s, though we can not identify the specific L2 from
> the entry and exit tracepoints, we still will be able to determine whether it
> was L1 or an L2 that made the entries and the exits.
Hmm, this is a solvable problem, and might even be worth solving immediately,
e.g. kvm_x86_ops.get_vmcx12 to retrieve the L1 GPA of the vmcs12/vmcb12.
More below.
> With this patch KVM entry and exit tracepoints will show "guest_mode = 0" if
> it is a guest and "guest_mode = 1" if it is a nested guest.
Uber pedantry, but technically not true, as trace_kvm_entry() doesn't have the
'=', and in the exit tracepoint, the '=' should be dropped to be consistent with
the existing format. Might be a moot point though.
> Signed-off-by: Krish Sdhukhan <krish.sadhukhan@oracle.com>
> ---
> arch/x86/kvm/trace.h | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index b484141ea15b..44dba26c6be2 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -21,14 +21,17 @@ TRACE_EVENT(kvm_entry,
> TP_STRUCT__entry(
> __field( unsigned int, vcpu_id )
> __field( unsigned long, rip )
> + __field( bool, guest_mode )
> ),
>
> TP_fast_assign(
> __entry->vcpu_id = vcpu->vcpu_id;
> __entry->rip = kvm_rip_read(vcpu);
> + __entry->guest_mode = is_guest_mode(vcpu);
> ),
>
> - TP_printk("vcpu %u, rip 0x%lx", __entry->vcpu_id, __entry->rip)
> + TP_printk("vcpu %u, rip 0x%lx, guest_mode %d", __entry->vcpu_id,
> + __entry->rip, __entry->guest_mode)
> );
>
> /*
> @@ -285,6 +288,7 @@ TRACE_EVENT(name, \
> __field( u32, intr_info ) \
> __field( u32, error_code ) \
> __field( unsigned int, vcpu_id ) \
> + __field( bool, guest_mode ) \
> ), \
> \
> TP_fast_assign( \
> @@ -295,15 +299,17 @@ TRACE_EVENT(name, \
> static_call(kvm_x86_get_exit_info)(vcpu, &__entry->info1, \
> &__entry->info2, \
> &__entry->intr_info, \
> - &__entry->error_code); \
> + &__entry->error_code); \
> + __entry->guest_mode = is_guest_mode(vcpu); \
> ), \
> \
> TP_printk("vcpu %u reason %s%s%s rip 0x%lx info1 0x%016llx " \
> - "info2 0x%016llx intr_info 0x%08x error_code 0x%08x", \
> - __entry->vcpu_id, \
> + "info2 0x%016llx intr_info 0x%08x error_code 0x%08x " \
> + "guest_mode = %d", __entry->vcpu_id, \
I'm biased because I've never really liked is_guest_mode(), but I think guest_mode
will be confusing to users. KVM developers are familiar with guest_mode() == L2,
but random debuggers/users are unlikely to make that connection.
A clever/heinous way to "solve" this issue and display vmcx12 iff L2 is active
would be to bastardize the L1/L2 strings and vmcx12 value formatting to yield:
vcpu 0 reason <reason> guest L1 rip 0xfffff ...
and
vcpu 0 reason <reason> guest L2 vmcx12 0xaaaaa rip 0xfffff ...
#define TRACE_EVENT_KVM_EXIT(name) \
TRACE_EVENT(name, \
TP_PROTO(unsigned int exit_reason, struct kvm_vcpu *vcpu, u32 isa), \
TP_ARGS(exit_reason, vcpu, isa), \
\
TP_STRUCT__entry( \
__field( unsigned int, exit_reason ) \
__field( unsigned long, guest_rip ) \
__field( u32, isa ) \
__field( u64, info1 ) \
__field( u64, info2 ) \
__field( u32, intr_info ) \
__field( u32, error_code ) \
__field( unsigned int, vcpu_id ) \
__field( u64, vmcx12 ) \
), \
\
TP_fast_assign( \
__entry->exit_reason = exit_reason; \
__entry->guest_rip = kvm_rip_read(vcpu); \
__entry->isa = isa; \
__entry->vcpu_id = vcpu->vcpu_id; \
static_call(kvm_x86_get_exit_info)(vcpu, &__entry->info1, \
&__entry->info2, \
&__entry->intr_info, \
&__entry->error_code); \
__entry->vmcx12 = is_guest_mode(vcpu) ? \
static_call(kvm_x86_get_vmcx12) : \
INVALID_PAGE; \
), \
\
TP_printk("vcpu %u reason %s%s%s guest %s%llx rip 0x%lx " \
"info1 0x%016llx info2 0x%016llx " \
"intr_info 0x%08x error_code 0x%08x", \
__entry->vcpu_id, \
kvm_print_exit_reason(__entry->exit_reason, __entry->isa), \
__entry->vmcx12 == INVALID_PAGE ? "L" : "L2 vmcx12 0x", \
__entry->vmcx12 == INVALID_PAGE ? 1 : __entry->vmcx12, \
__entry->guest_rip, __entry->info1, __entry->info2, \
__entry->intr_info, __entry->error_code) \
)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-06-21 20:43 ` Krish Sadhukhan
2021-07-30 19:37 ` Sean Christopherson
@ 2021-08-02 14:18 ` Paolo Bonzini
2021-08-02 16:34 ` Sean Christopherson
1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2021-08-02 14:18 UTC (permalink / raw)
To: Krish Sadhukhan, kvm; +Cc: jmattson, seanjc, vkuznets, wanpengli, joro
On 21/06/21 22:43, Krish Sadhukhan wrote:
> With this patch KVM entry and exit tracepoints will
> show "guest_mode = 0" if it is a guest and "guest_mode = 1" if it is a
> nested guest.
What about adding a "(nested)" suffix for L2, and nothing for L1?
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-08-02 14:18 ` Paolo Bonzini
@ 2021-08-02 16:34 ` Sean Christopherson
2021-08-02 16:39 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2021-08-02 16:34 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Krish Sadhukhan, kvm, jmattson, vkuznets, wanpengli, joro
On Mon, Aug 02, 2021, Paolo Bonzini wrote:
> On 21/06/21 22:43, Krish Sadhukhan wrote:
> > With this patch KVM entry and exit tracepoints will
> > show "guest_mode = 0" if it is a guest and "guest_mode = 1" if it is a
> > nested guest.
>
> What about adding a "(nested)" suffix for L2, and nothing for L1?
That'd work too, though it would be nice to get vmcx12 printed as well so that
it would be possible to determine which L2 is running without having to cross-
reference other tracepoints.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-08-02 16:34 ` Sean Christopherson
@ 2021-08-02 16:39 ` Paolo Bonzini
2021-08-02 22:21 ` Krish Sadhukhan
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:39 UTC (permalink / raw)
To: Sean Christopherson
Cc: Krish Sadhukhan, kvm, jmattson, vkuznets, wanpengli, joro
On 02/08/21 18:34, Sean Christopherson wrote:
> On Mon, Aug 02, 2021, Paolo Bonzini wrote:
>> On 21/06/21 22:43, Krish Sadhukhan wrote:
>>> With this patch KVM entry and exit tracepoints will
>>> show "guest_mode = 0" if it is a guest and "guest_mode = 1" if it is a
>>> nested guest.
>>
>> What about adding a "(nested)" suffix for L2, and nothing for L1?
>
> That'd work too, though it would be nice to get vmcx12 printed as well so that
> it would be possible to determine which L2 is running without having to cross-
> reference other tracepoints.
Yes, it would be nice but it would also clutter the output a bit.
It's like what we have already in kvm_inj_exception:
TP_printk("%s (0x%x)",
__print_symbolic(__entry->exception, kvm_trace_sym_exc),
/* FIXME: don't print error_code if not present */
__entry->has_error ? __entry->error_code : 0)
It could be done with a trace-cmd plugin, but that creates other issues since
it essentially forces the tracepoints to have a stable API.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-08-02 16:39 ` Paolo Bonzini
@ 2021-08-02 22:21 ` Krish Sadhukhan
2021-08-02 23:20 ` Jim Mattson
0 siblings, 1 reply; 9+ messages in thread
From: Krish Sadhukhan @ 2021-08-02 22:21 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: kvm, jmattson, vkuznets, wanpengli, joro
On 8/2/21 9:39 AM, Paolo Bonzini wrote:
> On 02/08/21 18:34, Sean Christopherson wrote:
>> On Mon, Aug 02, 2021, Paolo Bonzini wrote:
>>> On 21/06/21 22:43, Krish Sadhukhan wrote:
>>>> With this patch KVM entry and exit tracepoints will
>>>> show "guest_mode = 0" if it is a guest and "guest_mode = 1" if it is a
>>>> nested guest.
>>>
>>> What about adding a "(nested)" suffix for L2, and nothing for L1?
>>
>> That'd work too, though it would be nice to get vmcx12 printed as
>> well so that
>> it would be possible to determine which L2 is running without having
>> to cross-
>> reference other tracepoints.
>
> Yes, it would be nice but it would also clutter the output a bit.
> It's like what we have already in kvm_inj_exception:
>
> TP_printk("%s (0x%x)",
> __print_symbolic(__entry->exception,
> kvm_trace_sym_exc),
> /* FIXME: don't print error_code if not present */
> __entry->has_error ? __entry->error_code : 0)
>
> It could be done with a trace-cmd plugin, but that creates other
> issues since
> it essentially forces the tracepoints to have a stable API.
>
> Paolo
Also, the vmcs/vmcb address is vCPU-specific, so if L2 runs on 10 vCPUs,
traces will show 10 different addresses for the same L2 and it's not
convenient on a cloud host where hundreds of L1s and L2s run. IMHO, the
cleanest solution is if the hardware vendors can provide a UUID space in
the VMCS/VMCB itself. Today, we can print the UUID of L1 (by getting it
from QEMU), but there's no way we can find out the one of L2.
OK, for now, I will add "nested" for L2 and nothing for L1.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-08-02 22:21 ` Krish Sadhukhan
@ 2021-08-02 23:20 ` Jim Mattson
2021-08-03 0:03 ` Sean Christopherson
0 siblings, 1 reply; 9+ messages in thread
From: Jim Mattson @ 2021-08-02 23:20 UTC (permalink / raw)
To: Krish Sadhukhan
Cc: Paolo Bonzini, Sean Christopherson, kvm, vkuznets, wanpengli,
joro
On Mon, Aug 2, 2021 at 3:21 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
>
> On 8/2/21 9:39 AM, Paolo Bonzini wrote:
> > On 02/08/21 18:34, Sean Christopherson wrote:
> >> On Mon, Aug 02, 2021, Paolo Bonzini wrote:
> >>> On 21/06/21 22:43, Krish Sadhukhan wrote:
> >>>> With this patch KVM entry and exit tracepoints will
> >>>> show "guest_mode = 0" if it is a guest and "guest_mode = 1" if it is a
> >>>> nested guest.
> >>>
> >>> What about adding a "(nested)" suffix for L2, and nothing for L1?
> >>
> >> That'd work too, though it would be nice to get vmcx12 printed as
> >> well so that
> >> it would be possible to determine which L2 is running without having
> >> to cross-
> >> reference other tracepoints.
> >
> > Yes, it would be nice but it would also clutter the output a bit.
> > It's like what we have already in kvm_inj_exception:
> >
> > TP_printk("%s (0x%x)",
> > __print_symbolic(__entry->exception,
> > kvm_trace_sym_exc),
> > /* FIXME: don't print error_code if not present */
> > __entry->has_error ? __entry->error_code : 0)
> >
> > It could be done with a trace-cmd plugin, but that creates other
> > issues since
> > it essentially forces the tracepoints to have a stable API.
> >
> > Paolo
>
>
> Also, the vmcs/vmcb address is vCPU-specific, so if L2 runs on 10 vCPUs,
> traces will show 10 different addresses for the same L2 and it's not
> convenient on a cloud host where hundreds of L1s and L2s run.
The vmcx02 address is vCPU-specific. However, Sean asked for the
vmcx12 address, which is a GPA that is common across all vCPUs.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints
2021-08-02 23:20 ` Jim Mattson
@ 2021-08-03 0:03 ` Sean Christopherson
0 siblings, 0 replies; 9+ messages in thread
From: Sean Christopherson @ 2021-08-03 0:03 UTC (permalink / raw)
To: Jim Mattson
Cc: Krish Sadhukhan, Paolo Bonzini, kvm, vkuznets, wanpengli, joro
On Mon, Aug 02, 2021, Jim Mattson wrote:
> On Mon, Aug 2, 2021 at 3:21 PM Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote:
> >
> >
> > On 8/2/21 9:39 AM, Paolo Bonzini wrote:
> > > On 02/08/21 18:34, Sean Christopherson wrote:
> > >> On Mon, Aug 02, 2021, Paolo Bonzini wrote:
> > >>> On 21/06/21 22:43, Krish Sadhukhan wrote:
> > >>>> With this patch KVM entry and exit tracepoints will
> > >>>> show "guest_mode = 0" if it is a guest and "guest_mode = 1" if it is a
> > >>>> nested guest.
> > >>>
> > >>> What about adding a "(nested)" suffix for L2, and nothing for L1?
> > >>
> > >> That'd work too, though it would be nice to get vmcx12 printed as well
> > >> so that it would be possible to determine which L2 is running without
> > >> having to cross-reference other tracepoints.
> > >
> > > Yes, it would be nice but it would also clutter the output a bit.
But with my gross hack, it'd only clutter nested entries/exits.
> > > It's like what we have already in kvm_inj_exception:
> > >
> > > TP_printk("%s (0x%x)",
> > > __print_symbolic(__entry->exception, kvm_trace_sym_exc),
> > > /* FIXME: don't print error_code if not present */
> > > __entry->has_error ? __entry->error_code : 0)
> > >
> > > It could be done with a trace-cmd plugin, but that creates other issues
> > > since it essentially forces the tracepoints to have a stable API.
> >
> > Also, the vmcs/vmcb address is vCPU-specific, so if L2 runs on 10 vCPUs,
> > traces will show 10 different addresses for the same L2 and it's not
> > convenient on a cloud host where hundreds of L1s and L2s run.
>
> The vmcx02 address is vCPU-specific. However, Sean asked for the
> vmcx12 address, which is a GPA that is common across all vCPUs.
Ya. Obviously it doesn't help identifying L2 vCPU relationships, e.g. if an L2 VM
runs 10 vCPUs of its own, but in most cases the sequence of what was run for a
given L1 vCPU is what's interesting and relevant, whereas knowing which L2 vCPUs
belong to which L2 VM isn't often critical information for debug/triage.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-08-03 0:03 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 20:43 [PATCH] KVM: nVMX: nSVM: Show guest mode in kvm_{entry,exit} tracepoints Krish Sadhukhan
2021-06-21 20:43 ` Krish Sadhukhan
2021-07-30 19:37 ` Sean Christopherson
2021-08-02 14:18 ` Paolo Bonzini
2021-08-02 16:34 ` Sean Christopherson
2021-08-02 16:39 ` Paolo Bonzini
2021-08-02 22:21 ` Krish Sadhukhan
2021-08-02 23:20 ` Jim Mattson
2021-08-03 0:03 ` Sean Christopherson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.