Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1251755 > unrolled thread

Re: [PATCH] tools lib traceevent: update KVM plugin

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] tools lib traceevent: update KVM plugin Arnaldo Carvalho de Melo <acme@redhat.com> - 2015-10-20 16:40 +0200
    Re: [PATCH] tools lib traceevent: update KVM plugin Steven Rostedt <rostedt@goodmis.org> - 2015-10-20 16:50 +0200
      Re: [PATCH] tools lib traceevent: update KVM plugin Paolo Bonzini <pbonzini@redhat.com> - 2015-10-20 17:20 +0200
        Re: [PATCH] tools lib traceevent: update KVM plugin Steven Rostedt <rostedt@goodmis.org> - 2015-10-20 17:30 +0200
          Re: [PATCH] tools lib traceevent: update KVM plugin Paolo Bonzini <pbonzini@redhat.com> - 2015-10-20 17:40 +0200
            Re: [PATCH] tools lib traceevent: update KVM plugin Paolo Bonzini <pbonzini@redhat.com> - 2015-10-20 17:50 +0200
              Re: [PATCH] tools lib traceevent: update KVM plugin Arnaldo Carvalho de Melo <acme@redhat.com> - 2015-10-20 18:00 +0200
                Re: [PATCH] tools lib traceevent: update KVM plugin Steven Rostedt <rostedt@goodmis.org> - 2015-10-20 18:30 +0200
            Re: [PATCH] tools lib traceevent: update KVM plugin Steven Rostedt <rostedt@goodmis.org> - 2015-10-20 17:50 +0200

#1251755 — Re: [PATCH] tools lib traceevent: update KVM plugin

Em Fri, Oct 09, 2015 at 10:10:13PM +0200, Paolo Bonzini escreveu:
> On 01/10/2015 12:28, Paolo Bonzini wrote:
> > The format of the role word has changed through the years and the
> > plugin was never updated; some VMX exit reasons were missing too.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  tools/lib/traceevent/plugin_kvm.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/lib/traceevent/plugin_kvm.c b/tools/lib/traceevent/plugin_kvm.c
> > index 88fe83dff7cd..18536f756577 100644
> > --- a/tools/lib/traceevent/plugin_kvm.c
> > +++ b/tools/lib/traceevent/plugin_kvm.c
> > @@ -124,7 +124,10 @@ static const char *disassemble(unsigned char *insn, int len, uint64_t rip,
> >  	_ER(WBINVD,		 54)		\
> >  	_ER(XSETBV,		 55)		\
> >  	_ER(APIC_WRITE,		 56)		\
> > -	_ER(INVPCID,		 58)
> > +	_ER(INVPCID,		 58)		\
> > +	_ER(PML_FULL,		 62)		\
> > +	_ER(XSAVES,		 63)		\
> > +	_ER(XRSTORS,		 64)
> >  
> >  #define SVM_EXIT_REASONS \
> >  	_ER(EXIT_READ_CR0,	0x000)		\
> > @@ -352,15 +355,18 @@ static int kvm_nested_vmexit_handler(struct trace_seq *s, struct pevent_record *
> >  union kvm_mmu_page_role {
> >  	unsigned word;
> >  	struct {
> > -		unsigned glevels:4;
> >  		unsigned level:4;
> > +		unsigned cr4_pae:1;
> >  		unsigned quadrant:2;
> > -		unsigned pad_for_nice_hex_output:6;
> >  		unsigned direct:1;
> >  		unsigned access:3;
> >  		unsigned invalid:1;
> > -		unsigned cr4_pge:1;
> >  		unsigned nxe:1;
> > +		unsigned cr0_wp:1;
> > +		unsigned smep_and_not_wp:1;
> > +		unsigned smap_and_not_wp:1;
> > +		unsigned pad_for_nice_hex_output:8;
> > +		unsigned smm:8;
> >  	};
> >  };
> >  
> > @@ -385,15 +391,18 @@ static int kvm_mmu_print_role(struct trace_seq *s, struct pevent_record *record,
> >  	if (pevent_is_file_bigendian(event->pevent) ==
> >  	    pevent_is_host_bigendian(event->pevent)) {
> >  
> > -		trace_seq_printf(s, "%u/%u q%u%s %s%s %spge %snxe",
> > +		trace_seq_printf(s, "%u q%u%s %s%s %spae %snxe %swp%s%s%s",
> >  				 role.level,
> > -				 role.glevels,
> >  				 role.quadrant,
> >  				 role.direct ? " direct" : "",
> >  				 access_str[role.access],
> >  				 role.invalid ? " invalid" : "",
> > -				 role.cr4_pge ? "" : "!",
> > -				 role.nxe ? "" : "!");
> > +				 role.cr4_pae ? "" : "!",
> > +				 role.nxe ? "" : "!",
> > +				 role.cr0_wp ? "" : "!",
> > +				 role.smep_and_not_wp ? " smep" : "",
> > +				 role.smap_and_not_wp ? " smap" : "",
> > +				 role.smm ? " smm" : "");
> >  	} else
> >  		trace_seq_printf(s, "WORD: %08x", role.word);
> >  
> > 
> 
> Ping?  Arnaldo, ok to include this patch in my tree?

Applying, I saw it before, but lost track, perhaps was waiting for
Steven Rostedt to chime in, but now I noticed he wasn't CCed, he is now.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1251763

On Tue, 20 Oct 2015 12:32:44 -0200
Arnaldo Carvalho de Melo <acme@redhat.com> wrote:


> > Ping?  Arnaldo, ok to include this patch in my tree?
> 
> Applying, I saw it before, but lost track, perhaps was waiting for
> Steven Rostedt to chime in, but now I noticed he wasn't CCed, he is now.

What happens if you run new perf on an older kernel. Is this new plugin
going to be screwed up? Plugins should be backward compatible.

Is the plugin even still needed? I'm looking at some of the kvm events
and they seem to be mostly self sufficient. What ones need a plugin
today?

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251787

On 20/10/2015 16:44, Steven Rostedt wrote:
> What happens if you run new perf on an older kernel. Is this new plugin
> going to be screwed up? Plugins should be backward compatible.

If you run new perf on older kernel, the new plugin will print the 
"role" field (see kvm_mmu_print_role) slightly incorrectly.  That said, 
the existing plugin was _also_ printing the role in a wildly wrong 
format, like 2.6.35 vintage; the glevels field was removed by commit 
5b7e0102ae74, "KVM: MMU: Replace role.glevels with role.cr4_pae", in 
April 2010.

Going forward it's really unlikely that the role will change apart from 
adding new bits.  These can be added to the plugin while keeping it 
backwards-compatible.  Addition to the role happen when you implement 
new virtual MMU features such as SMEP, SMAP or SMM.  That's once per year
or less.

> Is the plugin even still needed? I'm looking at some of the kvm events
> and they seem to be mostly self sufficient. What ones need a plugin
> today?

Yes, most of them are.  It's only needed for kvm_mmu_get_page, 
kvm_mmu_prepare_zap_page and kvm_emulate_insn.  The latter is only 
interesting if you install the disassembler library, and I wouldn't 
really care if it went away.

kvm_mmu_get_page and kvm_mmu_prepare_zap_page, however, have output like

    kvm_mmu_get_page: [FAILED TO PARSE] mmu_valid_gen=0x2 gfn=786432 role=1923 root_count=0 unsync=0 created=1

without the plugin vs.

    kvm_mmu_get_page: new sp gfn c0000 3 q0 direct rwx !pae !nxe !wp root 0 sync

with the plugin.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251806

On Tue, 20 Oct 2015 17:19:12 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 
> 
> On 20/10/2015 16:44, Steven Rostedt wrote:
> > What happens if you run new perf on an older kernel. Is this new plugin
> > going to be screwed up? Plugins should be backward compatible.
> 
> If you run new perf on older kernel, the new plugin will print the 
> "role" field (see kvm_mmu_print_role) slightly incorrectly.  That said, 
> the existing plugin was _also_ printing the role in a wildly wrong 
> format, like 2.6.35 vintage; the glevels field was removed by commit 
> 5b7e0102ae74, "KVM: MMU: Replace role.glevels with role.cr4_pae", in 
> April 2010.

Can we add a check if glevels field exists, and do the old format if it
does? I'm very strict on making sure event-parse works with all
incarnations of kernels.

> 
> Going forward it's really unlikely that the role will change apart from 
> adding new bits.  These can be added to the plugin while keeping it 
> backwards-compatible.  Addition to the role happen when you implement 
> new virtual MMU features such as SMEP, SMAP or SMM.  That's once per year
> or less.
> 
> > Is the plugin even still needed? I'm looking at some of the kvm events
> > and they seem to be mostly self sufficient. What ones need a plugin
> > today?
> 
> Yes, most of them are.  It's only needed for kvm_mmu_get_page, 
> kvm_mmu_prepare_zap_page and kvm_emulate_insn.  The latter is only 
> interesting if you install the disassembler library, and I wouldn't 
> really care if it went away.
> 
> kvm_mmu_get_page and kvm_mmu_prepare_zap_page, however, have output like
> 
>     kvm_mmu_get_page: [FAILED TO PARSE] mmu_valid_gen=0x2 gfn=786432 role=1923 root_count=0 unsync=0 created=1
> 
> without the plugin vs.
> 
>     kvm_mmu_get_page: new sp gfn c0000 3 q0 direct rwx !pae !nxe !wp root 0 sync
> 
> with the plugin.

Thanks for the update.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251809

On 20/10/2015 17:26, Steven Rostedt wrote:
> > > > What happens if you run new perf on an older kernel. Is this new plugin
> > > > going to be screwed up? Plugins should be backward compatible.
> > 
> > If you run new perf on older kernel, the new plugin will print the 
> > "role" field (see kvm_mmu_print_role) slightly incorrectly.  That said, 
> > the existing plugin was _also_ printing the role in a wildly wrong 
> > format, like 2.6.35 vintage; the glevels field was removed by commit 
> > 5b7e0102ae74, "KVM: MMU: Replace role.glevels with role.cr4_pae", in 
> > April 2010.
> 
> Can we add a check if glevels field exists, and do the old format if it
> does? I'm very strict on making sure event-parse works with all
> incarnations of kernels.

Note that this is not a glevels _tracepoint_ field.  The tracepoint
field is "role" and it is a 64-bit integer.  The plugin interprets the
bitfields of that integer, and the position/meaning of the bitfields has
changed.

I guess I could use the mmu_valid_gen field as a proxy (it was
introduced after glevels was removed), and only provide the "very old"
(up to 2.6.34) and "new" (4.2+) formats.  The "old" format (2.6.35-4.1)
was never implemented in the plugin; it may well remain that way.

However, it frankly seems a bit academic.  The parsing _will_ work,
apart from printing a nonsensical role just like it has always done for
the past four years.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251827

On 20/10/2015 17:48, Steven Rostedt wrote:
> On Tue, 20 Oct 2015 17:37:43 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> However, it frankly seems a bit academic.  The parsing _will_ work,
>> apart from printing a nonsensical role just like it has always done for
>> the past four years.
> 
> I'm not going to be too picky about it. But things like this may seem
> academic, but it's also what we forget about, and people still use
> 2.6.32 kernels, and this may come back and bite us later.
> 
> But it's more likely to bite you than me, so if you don't think it's
> worth while, then I'm not going to push it.

Thanks.  The role is not going to matter except in really weird cases
and hopefully those are just not there in 2.6.32 kernels.  Thanks also
for educating me so I won't get it wrong in the future!

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251834

Em Tue, Oct 20, 2015 at 05:49:22PM +0200, Paolo Bonzini escreveu:
> 
> 
> On 20/10/2015 17:48, Steven Rostedt wrote:
> > On Tue, 20 Oct 2015 17:37:43 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 
> >> However, it frankly seems a bit academic.  The parsing _will_ work,
> >> apart from printing a nonsensical role just like it has always done for
> >> the past four years.
> > 
> > I'm not going to be too picky about it. But things like this may seem
> > academic, but it's also what we forget about, and people still use
> > 2.6.32 kernels, and this may come back and bite us later.
> > 
> > But it's more likely to bite you than me, so if you don't think it's
> > worth while, then I'm not going to push it.
> 
> Thanks.  The role is not going to matter except in really weird cases
> and hopefully those are just not there in 2.6.32 kernels.  Thanks also
> for educating me so I won't get it wrong in the future!

Cool, Steven, can I take that as an Acked-by for this patch?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251857

On Tue, 20 Oct 2015 12:50:26 -0300
Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> Cool, Steven, can I take that as an Acked-by for this patch?
> 

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1251829

On Tue, 20 Oct 2015 17:37:43 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> However, it frankly seems a bit academic.  The parsing _will_ work,
> apart from printing a nonsensical role just like it has always done for
> the past four years.

I'm not going to be too picky about it. But things like this may seem
academic, but it's also what we forget about, and people still use
2.6.32 kernels, and this may come back and bite us later.

But it's more likely to bite you than me, so if you don't think it's
worth while, then I'm not going to push it.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web