Re: Regarding systemtap support for AArch64

[Sandeepa Prabhu <sandeepa.prabhu@linaro.org>]

On 2 October 2013 16:54, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
> (2013/10/02 13:17), Sandeepa Prabhu wrote:
>> Hi all,
>>
>> I have uploaded ARM64 kprobes work on Linaro public git:
>> git://git.linaro.org/people/sandeepa.prabhu/linux-aarch64.git  Branch:
>> kprobes_devel_v8.  Patches are published on LAKML too.  This is based
>> on v8 upstream kernel (3.12-rc1) right now, and works with linaro
>> boot-wrapper and fast model setup, though, not sure what it takes to
>> build for fedora.
>
> Thank you for the great work! :)
>
> At a glance, I have some comments (almost formatting issues) on it.
>  - kprobes def config patch should be moved after the
>    kprobe support.
Yes ideally, but it is not part of the patchset, I only added this new
config in my repo for development testing (for sample modules), actual
changes should go in in arm/arm64/defconfig or any other configs that
can enable it.

>  - Is it really need to use spinlock to protect break_hook?
Any cpu can remove breakpoint hooks right, and traversal happen in
debug exception context so mutex are not safe (can sleep/schedule out)
in debug exception.
>    Why can't we use rcu_lock as step_hook?
Need to check, but we need protection for list traversal only (if hook
removed from another cpu)

>  - patch_text should check the alignment in itself and return an error.
Yes, perhaps kprobe also should check the alignment before handling,
if debug addr (ELR_EL1) is not 32-bit aligned, it's an error and
should be a BUG() since this it cannot be handled, can fix this in
next version.

>  - For ease of bisect debugging, I'd recommend you to split decoder and
>    simulator, and to reorder it as below.
> 1. decoder patch
> 2. kprobes basic patch (this doesn't support insns which need simulator)
> 3. simulator & kprobes enhancement patch
Good point, I will refine in next set of patches, I thought it is
difficult to split that way since we have unified decode table. One
basic question with this way of splitting,  "is it expected that  1.
and 2. (without simulate patch-3) can build and execute?"

>  - probes-*.c is not good name for the simulator. those should have
>    better name.
May be decode-arm64.c? Originally I had decode-* but the logic is
limited to kprobes and uprobes only so renamed that way.  Other cases
like jump_labels, use different decoding, and may not share same code.

>  - kprobes-arm64.c is also not good name. I think we can have
>    arch/arm64/kernel/kprobes/ directory on arm64 as same as x86.
>    we may have core.c and insn_map.c.
Hmmm, with our design uprobes should share probes-* or decode-* files,
then uprobes code also reside in arch/arm64/kernel/kprobes/ folder
itself.  Please suggest if this is fine? folder name may be
misleading.

>  - probes-common.c looks only for kprobes. This should be go into kprobes.c.
No, this file is common for kprobes 64-bit kernel, uprobes 32-bit
userspace, and uprobes 64-bit userspace, so needs to be in separate
file.

>  - It seems that a part of kretprobe logic is in the kprobes' patch.
Hmm I can refine it in next patchset, may be jprobes support too go in
as separate patch?

>
> IMHO, the simulator/decoder will be useful for other cases (KVM, mmiotrace etc.)
> So I recommend you to keep it away from kprobes as far as possible.
>
> And please CC the series to me next time
> (since I don't subscribe LAKML...).
Sorry, I did not CC you last time, I will add you from v2.

Masami,
Couple of questions:
1. My changes reside in arm/arm64 only right now, does it make sense
to publish on LKML so that core kprobes developers can review?
2. Wanted to change sample/kprobes/kprobe_example.c to check for the
missing case of ARM & ARM64 and print the relevant info. Can this
change be as part of same series (if going on LKML)?

Thanks a lot for your review, I would also need help on validation for
my work, please let me know if our repo holds good for systemtap
validation. I am interested in using fedora on v8 model, please
provide me some instructions to get the packages.

Cheers,
~Sandeepa
>
> Thank you again!
>
>
>>
>> Will,
>>
>> Is aarch64 fc19 port  public? I am interested in using fc on v8 fast
>> model, are there instructions about how to get the packages and
>> build/run them?
>>
>> Thanks,
>> Sandeepa
>>
>>
>>
>> On 30 September 2013 17:40, William Cohen <wcohen@redhat.com> wrote:
>>> On 09/29/2013 10:36 PM, Masami Hiramatsu wrote:
>>>> (2013/09/26 12:13), Sandeepa Prabhu wrote:
>>>>> Hi Will, Masami,
>>>>>
>>>>> Nice to hear from you, I am using ARM fast model/Foundation model with
>>>>> ARM v8 upstream kernel and a Linaro minimal busybox based ramdisk,
>>>>> testing with loadable modules for now (so don't have dependency on
>>>>> elfutils or GCC autoconf etc)
>>>>>
>>>>> Would you be interested to use Linaro kprobe work as a base for
>>>>> development and validation of systemtap-aarch64?  We are happy to
>>>>> share a public git repo where 'upstream' kernel can be built with
>>>>> kprobes support, which systemtap team can use for verification.  I can
>>>>> do this soon as I have most things working locally, except for
>>>>> kretprobes and 'boosting' support(systemtap can be run without these I
>>>>> believe).
>>>>
>>>> Of course I'm interested in that. Actually I've tried to boot up Linaro's
>>>> Aarch64 kernel on a simulator which ARM is distributing.
>>>> As far as I could see (at that time), aarch64 branch already has singlestep
>>>> implementation for debugging, but no primitive kprobes. And we need to know
>>>> the aarch64 ISA for decoding the binary to find the instructions which will
>>>> be affected by the out-of-line execution, like relative jump, call etc.
>>>>
>>>>> Looking forward for a collaboration :-)
>>>>
>>>> Me too ;)
>>>>
>>>> Thanks!
>>>>
>>>
>>> Hi All,
>>>
>>> I am still trying to get a better kernel running on the aarch64 simulator.  Currently the simulator is running and Fedora 19 image and Linux 3.9+ kernel, but I don't have any of the needed kernel-devel stuff for that kernel. As a work around I am using the aarch64 kernel rpms and attempting to build systemtap instrumentation modules using the following rpms:
>>>
>>> kernel-3.12.0-0.rc0.git20.1.x2.fc19.aarch64
>>> kernel-devel-3.12.0-0.rc0.git20.1.x2.fc19.aarch64
>>>
>>> These kernel rpms are cross compiled so some script executables are for AMD64 rather than aarch64.  I had to go through and recompile the script executables.  However, I was able to get an instrumentation modules with the current git checkout for simple hello world:
>>>
>>> $ sudo ../install/bin/stap  -v -m hello -r 3.12.0-0.rc0.git20.1.x2.fc19.aarch64 -p4 -k -e 'probe begin {printf("hello world\n")}'
>>>
>>> Pass 1: parsed user script and 92 library script(s) using 38784virt/30080res/5504shr/23360data kb, in 4730usr/30sys/4763real ms.
>>> Pass 2: analyzed script: 1 probe(s), 0 function(s), 0 embed(s), 0 global(s) using 39296virt/32000res/5952shr/23872data kb, in 80usr/0sys/81real ms.
>>> Pass 3: translated to C into "/tmp/stapzSdKrS/hello_src.c" using 39296virt/32000res/5952shr/23872data kb, in 10usr/0sys/7real ms.
>>> hello.ko
>>> Pass 4: compiled C into "hello.ko" in 114710usr/3620sys/120572real ms.
>>> Keeping temporary directory "/tmp/stapzSdKrS"
>>>
>>> There are still other things that need to be fixed in systemtap for aarch64.  However, if you are feeling adventurous you might try the current systemtap git checkout on aarch64.
>>>
>>> -Will
>>
>>
>
>
> --
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
>