[PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Masami Hiramatsu]

Hi,

Here are the patches of kprobe-based event tracer for x86, version 3.
Since this feature seems to attract some developers, I'd like to
push these basic patches into -tip tree so that they can easily
play it.

This version supports only x86(-32/-64) (If someone is interested in
porting this to other architectures, I'd happy to help :)), and
no respawn-able probe support (this would be better to push -mm tree.)

This can be applied on the linux-2.6-tip tree.

This patchset includes following changes:
- Add kprobe-tracer plugin [1/4]
- Fix kernel_trap_sp() on x86 according to systemtap runtime. [2/4]
- Support register and memory fetching [3/4]
- Support symbol-based memory fetching (for global variables) [4/4]

Future items:
- .init function tracing support.
- Add kernel_trap_sp() and fetch_*() on other archs.
- Support name-based register fetching (ax, bx, and so on)
- Support indirect memory fetch from registers etc.
- Support primitive types(long, ulong, int, uint, etc) for args.
- Check insertion point safety by using instruction decoder.

We may need to separate above arch-dependent variables fetching
infrastructure.

kprobe-based event tracer
---------------------------

This tracer is similar to the events tracer which is based on Tracepoint
infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe
and kretprobe). It probes anywhere where kprobes can probe(this means, all
functions body except for __kprobes functions).

Unlike the function tracer, this tracer can probe instructions inside of
kernel functions. It allows you to check which instruction has been executed.

Unlike the Tracepoint based events tracer, this tracer can add new probe points
on the fly.

Similar to the events tracer, this tracer doesn't need to be activated via
current_tracer, instead of that, just set probe points via
/debug/tracing/kprobe_probes.

Synopsis of kprobe_probes:
  p SYMBOL[+offs|-offs]|MEMADDR [FETCHARGS]     : set a probe
  r SYMBOL[+0] [FETCHARGS]                      : set a return probe

 FETCHARGS:
  rN    : Fetch Nth register (N >= 0)
  sN    : Fetch Nth entry of stack (N >= 0)
  @ADDR : Fetch memory at ADDR (ADDR should be in kernel)
  @SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol)
  aN    : Fetch function argument. (N >= 1)(*)
  rv    : Fetch return value.(**)
  rp    : Fetch return address.(**)

  (*) aN may not correct on asmlinkaged functions and at function body.
  (**) only for return probe.

E.g.
  echo p do_sys_open a1 a2 a3 a4 > /debug/tracing/kprobe_probes

 This sets a kprobe on the top of do_sys_open() function with recording
1st to 4th arguments.

  echo r do_sys_open rv rp >> /debug/tracing/kprobe_probes

 This sets a kretprobe on the return point of do_sys_open() function with
recording return value and return address.

  echo > /debug/tracing/kprobe_probes

 This clears all probe points. and you can see the traced information via
/debug/tracing/trace.

  cat /debug/tracing/trace
# tracer: nop
#
#           TASK-PID    CPU#    TIMESTAMP  FUNCTION
#              | |       |          |         |
           <...>-2376  [001]   262.389131: do_sys_open: @do_sys_open+0 0xffffff9c 0x98db83e 0x8880 0x0
           <...>-2376  [001]   262.391166: sys_open: <-do_sys_open+0 0x5 0xc06e8ebb
           <...>-2376  [001]   264.384876: do_sys_open: @do_sys_open+0 0xffffff9c 0x98db83e 0x8880 0x0
           <...>-2376  [001]   264.386880: sys_open: <-do_sys_open+0 0x5 0xc06e8ebb
           <...>-2084  [001]   265.380330: do_sys_open: @do_sys_open+0 0xffffff9c 0x804be3e 0x0 0x1b6
           <...>-2084  [001]   265.380399: sys_open: <-do_sys_open+0 0x3 0xc06e8ebb

 @SYMBOL means that kernel hits a probe, and <-SYMBOL means kernel returns
from SYMBOL(e.g. "sys_open: <-do_sys_open+0" means kernel returns from
do_sys_open to sys_open).

 Documentation/ftrace.txt      |   67 ++++
 arch/x86/include/asm/ptrace.h |    4 +-
 kernel/trace/Kconfig          |    9 +
 kernel/trace/Makefile         |    1 +
 kernel/trace/trace_kprobe.c   |  760 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 839 insertions(+), 2 deletions(-)

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Ingo Molnar]

* Masami Hiramatsu <mhiramat@redhat.com> wrote:

> Hi,
> 
> Here are the patches of kprobe-based event tracer for x86, version 
> 3. Since this feature seems to attract some developers, I'd like 
> to push these basic patches into -tip tree so that they can easily 
> play it.
> 
> This version supports only x86(-32/-64) (If someone is interested 
> in porting this to other architectures, I'd happy to help :)), and 
> no respawn-able probe support (this would be better to push -mm 
> tree.)
> 
> This can be applied on the linux-2.6-tip tree.

This bit:

> Future items:
> - Check insertion point safety by using instruction decoder.

is i believe a must-fix-before-merge item.

The functionality is genuinely useful, and if used dynamically on 
the host it can be a lot more versatile and a lot more accessible 
than a KGDB session - but code patching safety is a must-have.

It does not have to be a full decoder, just a simplified decoding 
run that starts from a known function-symbol address, and works its 
way down in the function looking at instruction boundaries, and 
figuring out whether the code patching is safe. If it sees anything 
it cannot deal with it bails out.

I suspect you could get very good practical results by supporting 
just a small fraction of the x86 instruction set architecture. If 
failures to insert a probe safely are printed out in clear terms:

   Could not insert probe at address 0xc01231234 due to:
   Unknown instruction: 48 8d 15 db ff ff ff 00 00 00

People will fill in the missing ISA bits quickly :-)

And people doing:

  asm(" .byte 0x00, 0x01, 0x02, 0x03;"); /* hehe, I broke the decoder! */

... in kernel .text functions will be talked to in private :)

So please lets do this now, it needs to happen.

Not having this was the main design failure of original kprobes, 
this fragility is what isolated kprobes from the rest of the 
instrumentation world and made it essentially a SystemTap-only 
special. And this problem is fixable.

It does not have to be a full solution, but it has to be a pretty 
safe one. If it's safe and there are no showstopper objections from 
others we can apply it to -tip

Can you see any fundamental reason why this couldnt be done?

Thanks,

	Ingo

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Masami Hiramatsu]

Ingo Molnar wrote:
> * Masami Hiramatsu <mhiramat@redhat.com> wrote:
> 
>> Hi,
>>
>> Here are the patches of kprobe-based event tracer for x86, version 
>> 3. Since this feature seems to attract some developers, I'd like 
>> to push these basic patches into -tip tree so that they can easily 
>> play it.
>>
>> This version supports only x86(-32/-64) (If someone is interested 
>> in porting this to other architectures, I'd happy to help :)), and 
>> no respawn-able probe support (this would be better to push -mm 
>> tree.)
>>
>> This can be applied on the linux-2.6-tip tree.
> 
> This bit:
> 
>> Future items:
>> - Check insertion point safety by using instruction decoder.
> 
> is i believe a must-fix-before-merge item.

Hi Ingo,

I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
decoder :-) which has been made originally for uprobe andd kprobes
jump-optimizer.

https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html

> The functionality is genuinely useful, and if used dynamically on 
> the host it can be a lot more versatile and a lot more accessible 
> than a KGDB session - but code patching safety is a must-have.
> 
> It does not have to be a full decoder, just a simplified decoding 
> run that starts from a known function-symbol address, and works its 
> way down in the function looking at instruction boundaries, and 
> figuring out whether the code patching is safe. If it sees anything 
> it cannot deal with it bails out.

Yeah, that is what I'll do.

> I suspect you could get very good practical results by supporting 
> just a small fraction of the x86 instruction set architecture. If 
> failures to insert a probe safely are printed out in clear terms:
> 
>    Could not insert probe at address 0xc01231234 due to:
>    Unknown instruction: 48 8d 15 db ff ff ff 00 00 00
> 
> People will fill in the missing ISA bits quickly :-)
> 
> And people doing:
> 
>   asm(" .byte 0x00, 0x01, 0x02, 0x03;"); /* hehe, I broke the decoder! */
> 
> ... in kernel .text functions will be talked to in private :)

Aha, that function will get illegal instruction exception :-) even
without kprobe.

> 
> So please lets do this now, it needs to happen.

Sure.

> Not having this was the main design failure of original kprobes, 
> this fragility is what isolated kprobes from the rest of the 
> instrumentation world and made it essentially a SystemTap-only 
> special. And this problem is fixable.
> 
> It does not have to be a full solution, but it has to be a pretty 
> safe one. If it's safe and there are no showstopper objections from 
> others we can apply it to -tip
> 
> Can you see any fundamental reason why this couldnt be done?

Nope, because we've done :-)

Thanks!

> 
> Thanks,
> 
> 	Ingo

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Ingo Molnar]

* Masami Hiramatsu <mhiramat@redhat.com> wrote:

> Ingo Molnar wrote:
> > * Masami Hiramatsu <mhiramat@redhat.com> wrote:
> > 
> >> Hi,
> >>
> >> Here are the patches of kprobe-based event tracer for x86, version 
> >> 3. Since this feature seems to attract some developers, I'd like 
> >> to push these basic patches into -tip tree so that they can easily 
> >> play it.
> >>
> >> This version supports only x86(-32/-64) (If someone is interested 
> >> in porting this to other architectures, I'd happy to help :)), and 
> >> no respawn-able probe support (this would be better to push -mm 
> >> tree.)
> >>
> >> This can be applied on the linux-2.6-tip tree.
> > 
> > This bit:
> > 
> >> Future items:
> >> - Check insertion point safety by using instruction decoder.
> > 
> > is i believe a must-fix-before-merge item.
> 
> Hi Ingo,
> 
> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
> decoder :-) which has been made originally for uprobe andd kprobes
> jump-optimizer.
> 
> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html

looks cool. Needs to be put somewhere in arch/x86/lib/, provided as 
a generic facility, with a Kconfig variable that says that the 
architecture supports it and then the kprobes-tracer could make 
immediate use of it, right?

> 
> > The functionality is genuinely useful, and if used dynamically on 
> > the host it can be a lot more versatile and a lot more accessible 
> > than a KGDB session - but code patching safety is a must-have.
> > 
> > It does not have to be a full decoder, just a simplified decoding 
> > run that starts from a known function-symbol address, and works its 
> > way down in the function looking at instruction boundaries, and 
> > figuring out whether the code patching is safe. If it sees anything 
> > it cannot deal with it bails out.
> 
> Yeah, that is what I'll do.
> 
> > I suspect you could get very good practical results by supporting 
> > just a small fraction of the x86 instruction set architecture. If 
> > failures to insert a probe safely are printed out in clear terms:
> > 
> >    Could not insert probe at address 0xc01231234 due to:
> >    Unknown instruction: 48 8d 15 db ff ff ff 00 00 00
> > 
> > People will fill in the missing ISA bits quickly :-)
> > 
> > And people doing:
> > 
> >   asm(" .byte 0x00, 0x01, 0x02, 0x03;"); /* hehe, I broke the decoder! */
> > 
> > ... in kernel .text functions will be talked to in private :)
> 
> Aha, that function will get illegal instruction exception :-) even
> without kprobe.

Not if it's under a never-true (not provable to the compiler) branch 
condition but i digress :)

> > Can you see any fundamental reason why this couldnt be done?
> 
> Nope, because we've done :-)

Cool :)

	Ingo

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Masami Hiramatsu]

Ingo Molnar wrote:
> * Masami Hiramatsu <mhiramat@redhat.com> wrote:
> 
>> Ingo Molnar wrote:
>>> * Masami Hiramatsu <mhiramat@redhat.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> Here are the patches of kprobe-based event tracer for x86, version 
>>>> 3. Since this feature seems to attract some developers, I'd like 
>>>> to push these basic patches into -tip tree so that they can easily 
>>>> play it.
>>>>
>>>> This version supports only x86(-32/-64) (If someone is interested 
>>>> in porting this to other architectures, I'd happy to help :)), and 
>>>> no respawn-able probe support (this would be better to push -mm 
>>>> tree.)
>>>>
>>>> This can be applied on the linux-2.6-tip tree.
>>> This bit:
>>>
>>>> Future items:
>>>> - Check insertion point safety by using instruction decoder.
>>> is i believe a must-fix-before-merge item.
>> Hi Ingo,
>>
>> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
>> decoder :-) which has been made originally for uprobe andd kprobes
>> jump-optimizer.
>>
>> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html
> 
> looks cool. Needs to be put somewhere in arch/x86/lib/, provided as 
> a generic facility, with a Kconfig variable that says that the 
> architecture supports it and then the kprobes-tracer could make 
> immediate use of it, right?

Yeah, I'd rather add a safety checker in kprobes-x86 itself, because
sometimes it has to fixup instructions modified by previous kprobes.

Thanks,

>>> The functionality is genuinely useful, and if used dynamically on 
>>> the host it can be a lot more versatile and a lot more accessible 
>>> than a KGDB session - but code patching safety is a must-have.
>>>
>>> It does not have to be a full decoder, just a simplified decoding 
>>> run that starts from a known function-symbol address, and works its 
>>> way down in the function looking at instruction boundaries, and 
>>> figuring out whether the code patching is safe. If it sees anything 
>>> it cannot deal with it bails out.
>> Yeah, that is what I'll do.
>>
>>> I suspect you could get very good practical results by supporting 
>>> just a small fraction of the x86 instruction set architecture. If 
>>> failures to insert a probe safely are printed out in clear terms:
>>>
>>>    Could not insert probe at address 0xc01231234 due to:
>>>    Unknown instruction: 48 8d 15 db ff ff ff 00 00 00
>>>
>>> People will fill in the missing ISA bits quickly :-)
>>>
>>> And people doing:
>>>
>>>   asm(" .byte 0x00, 0x01, 0x02, 0x03;"); /* hehe, I broke the decoder! */
>>>
>>> ... in kernel .text functions will be talked to in private :)
>> Aha, that function will get illegal instruction exception :-) even
>> without kprobe.
> 
> Not if it's under a never-true (not provable to the compiler) branch 
> condition but i digress :)
> 
>>> Can you see any fundamental reason why this couldnt be done?
>> Nope, because we've done :-)
> 
> Cool :)
> 
> 	Ingo

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Ingo Molnar]

* Masami Hiramatsu <mhiramat@redhat.com> wrote:

> >> I agreed. Fortunately, Jim Keniston and I wrote an x86 
> >> instruction decoder :-) which has been made originally for 
> >> uprobe andd kprobes jump-optimizer.
> >>
> >> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html
> > 
> > looks cool. Needs to be put somewhere in arch/x86/lib/, provided 
> > as a generic facility, with a Kconfig variable that says that 
> > the architecture supports it and then the kprobes-tracer could 
> > make immediate use of it, right?
> 
> Yeah, I'd rather add a safety checker in kprobes-x86 itself, 
> because sometimes it has to fixup instructions modified by 
> previous kprobes.

Oh, certainly! I didnt know how much you wanted to check things on 
the kprobes side but by all means please add it there too, it will 
be for the better.

A clear and actionable debug message to the syslog is important for 
the case where the decoder rejects an action. This is especially 
important for the kprobes side - we dont want silent breakage of 
tools.

	Ingo

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Andi Kleen]

Masami Hiramatsu <mhiramat@redhat.com> writes:
>
> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
> decoder :-) which has been made originally for uprobe andd kprobes
> jump-optimizer.
>
> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html

An alternative would be to adapt the x86 interpreter in KVM.
I thought for some time that that one should be available in 
a more generic form in a library.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Masami Hiramatsu]

Andi Kleen wrote:
> Masami Hiramatsu <mhiramat@redhat.com> writes:
>> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
>> decoder :-) which has been made originally for uprobe andd kprobes
>> jump-optimizer.
>>
>> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html
> 
> An alternative would be to adapt the x86 interpreter in KVM.
> I thought for some time that that one should be available in 
> a more generic form in a library.

As far as I can see, KVM's instruction emulator is incomplete
(it doesn't cover all instructions...) and aims to emulate
instructions, not to analyze (so I couldn't relay on it).

Anyway, I think making x86 interpreter in a library is a good idea.

Thank you,

> 
> -Andi
> 

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Andi Kleen]

On Wed, Apr 01, 2009 at 04:51:00PM -0400, Masami Hiramatsu wrote:
> Andi Kleen wrote:
> > Masami Hiramatsu <mhiramat@redhat.com> writes:
> >> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
> >> decoder :-) which has been made originally for uprobe andd kprobes
> >> jump-optimizer.
> >>
> >> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html
> > 
> > An alternative would be to adapt the x86 interpreter in KVM.
> > I thought for some time that that one should be available in 
> > a more generic form in a library.
> 
> As far as I can see, KVM's instruction emulator is incomplete

That's fine for you -- you only care about a subset of instructions
anyways, don't you?

> (it doesn't cover all instructions...) and aims to emulate
> instructions, not to analyze (so I couldn't relay on it).

You can use it to analyze, just plug in the right callbacks that
do nothing. I looked at it some time ago for doing instruction
length checking for some application, but that application
then disappeared. The main obstacle with making it a library 
is that some KVM specific dependencies have crept in that would
need to be abstracted again, but I don't think it would need a lot of 
effort,

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Masami Hiramatsu]

Andi Kleen wrote:
> On Wed, Apr 01, 2009 at 04:51:00PM -0400, Masami Hiramatsu wrote:
>> Andi Kleen wrote:
>>> Masami Hiramatsu <mhiramat@redhat.com> writes:
>>>> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
>>>> decoder :-) which has been made originally for uprobe andd kprobes
>>>> jump-optimizer.
>>>>
>>>> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html
>>> An alternative would be to adapt the x86 interpreter in KVM.
>>> I thought for some time that that one should be available in 
>>> a more generic form in a library.
>> As far as I can see, KVM's instruction emulator is incomplete
> 
> That's fine for you -- you only care about a subset of instructions
> anyways, don't you?

Actually, (in my case) I just need to decode non-FPU instructions,
because I'd like to check whether kprobe is on the instruction
boundary.

However, KVM's insn decoder can't decode some elemental
instructions, and instruction flags are incorrect.
I had written instruction decoder based on it, but the result
was so awful!
So soon, I had to rewrite it based on Intel's manual entirely :-(


>> (it doesn't cover all instructions...) and aims to emulate
>> instructions, not to analyze (so I couldn't relay on it).
> 
> You can use it to analyze, just plug in the right callbacks that
> do nothing. I looked at it some time ago for doing instruction
> length checking for some application, but that application
> then disappeared. The main obstacle with making it a library 
> is that some KVM specific dependencies have crept in that would
> need to be abstracted again, but I don't think it would need a lot of 
> effort,

Sorry, but I don't think so. Current KVM's decoder is much more
focusing on preparing instructions emulation. It requires
vcpu setup, fetching operators and so on. I think it needs to
diet their code (or well splitting from emulator).

Anyway, I don't stick with my decoder. If they can provide more
generic interfaces, I'd be happy to use it. :-)

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Andi Kleen]

On Wed, Apr 01, 2009 at 07:21:55PM -0400, Masami Hiramatsu wrote:
> Andi Kleen wrote:
> > On Wed, Apr 01, 2009 at 04:51:00PM -0400, Masami Hiramatsu wrote:
> >> Andi Kleen wrote:
> >>> Masami Hiramatsu <mhiramat@redhat.com> writes:
> >>>> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
> >>>> decoder :-) which has been made originally for uprobe andd kprobes
> >>>> jump-optimizer.
> >>>>
> >>>> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html
> >>> An alternative would be to adapt the x86 interpreter in KVM.
> >>> I thought for some time that that one should be available in 
> >>> a more generic form in a library.
> >> As far as I can see, KVM's instruction emulator is incomplete
> > 
> > That's fine for you -- you only care about a subset of instructions
> > anyways, don't you?
> 
> Actually, (in my case) I just need to decode non-FPU instructions,

What does it have to do with the FPU?  I don't think the KVM
one is aimed at those either.

> because I'd like to check whether kprobe is on the instruction
> boundary.
> 
> However, KVM's insn decoder can't decode some elemental
> instructions, and instruction flags are incorrect.

What flags?  EFLAGS? 

> I had written instruction decoder based on it, but the result
> was so awful!

What were the problems?

Did you report the problems to the KVM maintainers?

I still think it would be better to have a single good
decoder than a multitude of different ones tailored to specific
cases. 

> So soon, I had to rewrite it based on Intel's manual entirely :-(

Ok then perhaps KVM could benefit from your work too?

> > do nothing. I looked at it some time ago for doing instruction
> > length checking for some application, but that application
> > then disappeared. The main obstacle with making it a library 
> > is that some KVM specific dependencies have crept in that would
> > need to be abstracted again, but I don't think it would need a lot of 
> > effort,
> 
> Sorry, but I don't think so. Current KVM's decoder is much more
> focusing on preparing instructions emulation. It requires
> vcpu setup, fetching operators and so on. I think it needs to
> diet their code (or well splitting from emulator).

the vcpu stuff can be all dummies. If you look at the original
Xen version of it before it forked it was better isolated there.
The other stuff that crept in in the KVM version could be also
fixed.


> Anyway, I don't stick with my decoder. If they can provide more
> generic interfaces, I'd be happy to use it. :-)

I suspect "they" would need some help.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH -tip 0/4 V3] tracing: kprobe-based event tracer

[Masami Hiramatsu]

Andi Kleen wrote:
> On Wed, Apr 01, 2009 at 07:21:55PM -0400, Masami Hiramatsu wrote:
>> Andi Kleen wrote:
>>> On Wed, Apr 01, 2009 at 04:51:00PM -0400, Masami Hiramatsu wrote:
>>>> Andi Kleen wrote:
>>>>> Masami Hiramatsu <mhiramat@redhat.com> writes:
>>>>>> I agreed. Fortunately, Jim Keniston and I wrote an x86 instruction
>>>>>> decoder :-) which has been made originally for uprobe andd kprobes
>>>>>> jump-optimizer.
>>>>>>
>>>>>> https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html
>>>>> An alternative would be to adapt the x86 interpreter in KVM.
>>>>> I thought for some time that that one should be available in 
>>>>> a more generic form in a library.
>>>> As far as I can see, KVM's instruction emulator is incomplete
>>> That's fine for you -- you only care about a subset of instructions
>>> anyways, don't you?
>> Actually, (in my case) I just need to decode non-FPU instructions,
> 
> What does it have to do with the FPU?  I don't think the KVM
> one is aimed at those either.

Nothing, at least in kernel :). However, as I said before,
uprobe developers want to use this decoder for decoding
FPU instructions. Fortunately, this decoder can cover
those instructions too.

>> because I'd like to check whether kprobe is on the instruction
>> boundary.
>>
>> However, KVM's insn decoder can't decode some elemental
>> instructions, and instruction flags are incorrect.
> 
> What flags?  EFLAGS? 

No, KVM's decoder has instruction classification flags for
each instructions, and some of those flags are not correct.

>> I had written instruction decoder based on it, but the result
>> was so awful!
> 
> What were the problems?

It couldn't decode kernel binary correctly and found many bugs...

https://www.redhat.com/archives/utrace-devel/2009-March/msg00013.html

On the other hand, this decoder already verified that the result
is same as objdump's output.

https://www.redhat.com/archives/utrace-devel/2009-March/msg00031.html


> Did you report the problems to the KVM maintainers?

No, sorry, because I wrote a patch just referring KVM decoder.
I didn't use KVM decoder code itself.
I guess KVM uses their decoder only for emulating a
limited number of instructions. In that case, it will be OK for KVM.


> I still think it would be better to have a single good
> decoder than a multitude of different ones tailored to specific
> cases. 

Sure, why not? I agreed we'd better have a single decoder in the end.
However, I think KVM decoder is too big and complex (and tailored?)
to start with...
So, IMHO, we'd better have a "transition period" to clarify
demands from user components, to discuss how we can integrate it.

>> So soon, I had to rewrite it based on Intel's manual entirely :-(
> 
> Ok then perhaps KVM could benefit from your work too?

If their purpose is covering all instructions, Yes.

>>> do nothing. I looked at it some time ago for doing instruction
>>> length checking for some application, but that application
>>> then disappeared. The main obstacle with making it a library 
>>> is that some KVM specific dependencies have crept in that would
>>> need to be abstracted again, but I don't think it would need a lot of 
>>> effort,
>> Sorry, but I don't think so. Current KVM's decoder is much more
>> focusing on preparing instructions emulation. It requires
>> vcpu setup, fetching operators and so on. I think it needs to
>> diet their code (or well splitting from emulator).
> 
> the vcpu stuff can be all dummies. If you look at the original
> Xen version of it before it forked it was better isolated there.
> The other stuff that crept in in the KVM version could be also
> fixed.
> 
> 
>> Anyway, I don't stick with my decoder. If they can provide more
>> generic interfaces, I'd be happy to use it. :-)
> 
> I suspect "they" would need some help.

Sure, I agreed.

KVM developers, I'll cross-post our x86 instruction decoder to
KVM-ML. If you are interested in, please comment on it :)

Thank you,


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com