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

Groups > linux.kernel > #1312910 > unrolled thread

Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

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 v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Michael Ellerman <mpe@ellerman.id.au> - 2016-01-20 07:10 +0100
    Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Torsten Duwe <duwe@lst.de> - 2016-01-20 10:10 +0100
    Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Petr Mladek <pmladek@suse.com> - 2016-01-20 10:50 +0100
      Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Petr Mladek <pmladek@suse.com> - 2016-01-21 12:40 +0100
    Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Jiri Kosina <jikos@kernel.org> - 2016-01-21 10:40 +0100
      Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Michael Ellerman <mpe@ellerman.id.au> - 2016-01-21 14:00 +0100
        Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Torsten Duwe <duwe@lst.de> - 2016-01-21 16:10 +0100
          Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Torsten Duwe <duwe@lst.de> - 2016-01-21 16:20 +0100
            Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Jiri Kosina <jikos@kernel.org> - 2016-01-21 22:30 +0100
              Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE  (ABI v2) Torsten Duwe <duwe@lst.de> - 2016-01-21 23:00 +0100

#1312910 — Re: [PATCH v5 0/9] ftrace with regs + live patching for ppc64 LE (ABI v2)

On Wed, 2016-01-06 at 15:17 +0100, Petr Mladek wrote:
> On Fri 2015-12-04 15:45:29, Torsten Duwe wrote:
> > Changes since v4:
> >   * change comment style in entry_64.S to C89
> >     (nobody is using assembler syntax comments there).
> >   * the bool function restore_r2 shouldn't return 2,
> >     that's a little confusing.
> >   * Test whether the compiler supports -mprofile-kernel
> >     and only then define CC_USING_MPROFILE_KERNEL
> >   * also make the return value of klp_check_compiler_support
> >     depend on that.
>
> Note that there is still needed the extra patch from
> http://thread.gmane.org/gmane.linux.kernel/2093867/focus=2099603
> to get the livepatching working.

Sorry which extra patch?

> Both ftrace with regs and live patching works for me with this patch
> set and the extra patch. So. for the whole patchset:
>
> Tested-by: Petr Mladek <pmladek@suse.com>

Can you give me some more info on how you're testing it? What config options,
toolchain etc.?

For me the series doesn't even boot, even with livepatching disabled.

cheers

[toc] | [next] | [standalone]

#1313007

On Wed, Jan 20, 2016 at 05:03:23PM +1100, Michael Ellerman wrote:
> On Wed, 2016-01-06 at 15:17 +0100, Petr Mladek wrote:
> > On Fri 2015-12-04 15:45:29, Torsten Duwe wrote:
> > > Changes since v4:
> > >   * change comment style in entry_64.S to C89
> > >     (nobody is using assembler syntax comments there).
> > >   * the bool function restore_r2 shouldn't return 2,
> > >     that's a little confusing.
> > >   * Test whether the compiler supports -mprofile-kernel
> > >     and only then define CC_USING_MPROFILE_KERNEL
> > >   * also make the return value of klp_check_compiler_support
> > >     depend on that.
> >
> > Note that there is still needed the extra patch from
> > http://thread.gmane.org/gmane.linux.kernel/2093867/focus=2099603
> > to get the livepatching working.
> 
> Sorry which extra patch?
Message-ID: <20151203160004.GE8047@pathway.suse.cz>
By Petr Mladek, "Re: [PATCH v4 0/9] ftrace with regs + live patching..."
2015-12-03. It is further up in the function call hierarchy and basically
tells the arch-independent KLP to call the normal entry point on ppc64le, and
that the _mcount call site is 16 bytes further.


> > Both ftrace with regs and live patching works for me with this patch
> > set and the extra patch. So. for the whole patchset:
> >
> > Tested-by: Petr Mladek <pmladek@suse.com>
> 
> Can you give me some more info on how you're testing it? What config options,
> toolchain etc.?
> 
> For me the series doesn't even boot, even with livepatching disabled.

May indeed be a toolchain issue. I had to fix gcc-4.8.5 to get "notrace" working
for -mprofile-kernel. That's a gcc bug.

What are you using?

The config in the v5 patch series should be waterproof, especially with KLP disabled
ftrace with regs must work (all self-tests succeeded). If you send me your config
(via PM I suggest, spare the lists) I can verify it with the toolchain here.
Petr made a suggestion to reshuffle the config options to have it cleaner;
I suggest to patch that separately.

	Torsten

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

#1313042

On Wed 2016-01-20 17:03:23, Michael Ellerman wrote:
> On Wed, 2016-01-06 at 15:17 +0100, Petr Mladek wrote:
> > On Fri 2015-12-04 15:45:29, Torsten Duwe wrote:
> > > Changes since v4:
> > >   * change comment style in entry_64.S to C89
> > >     (nobody is using assembler syntax comments there).
> > >   * the bool function restore_r2 shouldn't return 2,
> > >     that's a little confusing.
> > >   * Test whether the compiler supports -mprofile-kernel
> > >     and only then define CC_USING_MPROFILE_KERNEL
> > >   * also make the return value of klp_check_compiler_support
> > >     depend on that.
> >
> > Note that there is still needed the extra patch from
> > http://thread.gmane.org/gmane.linux.kernel/2093867/focus=2099603
> > to get the livepatching working.
> 
> Sorry which extra patch?

It was in an older reply and can be found at
http://thread.gmane.org/gmane.linux.kernel/2093867/focus=2099603


> > Both ftrace with regs and live patching works for me with this patch
> > set and the extra patch. So. for the whole patchset:
> >
> > Tested-by: Petr Mladek <pmladek@suse.com>
> 
> Can you give me some more info on how you're testing it? What config options,
> toolchain etc.?

You need to fulfill all dependencies for CONFIG_LIVEPATCH, see
kernel/livepatch/Kconfig. Please, find attached the config that
that I used.

I did the testing on PPC64LE with a kernel based on 4.4.0-rc8
using the attached config. I used the following stuff:

$> gcc --version
gcc (SUSE Linux) 4.8.5
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$> rpm -q binutils
binutils-2.25.0-13.1.ppc64le


I tested it the following way:

# booted the compiled kernel and printed the default cmdline
$> cat /proc/cmdline
BOOT_IMAGE=/boot/vmlinuz-4.4.0-rc3-11-default+ root=UUID=...

# tried function_graph tracer to check ftrace with regs
echo function_graph >/sys/kernel/debug/tracing/current_tracer ; \
echo 1 >/sys/kernel/debug/tracing/tracing_on ; \
sleep 1 ; \
/usr/bin/ls /proc ; \
echo 0 >/sys/kernel/debug/tracing/tracing_on ; \
less /sys/kernel/debug/tracing/trace

# loaded the patch and printed the patch cmdline
$> modprobe livepatch-sample
$> cat /proc/cmdline
this has been live patched

# tried to disable and enable the patch
$> echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled
$> cat /proc/cmdline
BOOT_IMAGE=/boot/vmlinuz-4.4.0-rc3-11-default+ root=UUID=...
$> echo 1 > /sys/kernel/livepatch/livepatch_sample/enabled
$> cat /proc/cmdline
this has been live patched

# also checked messages
$> dmesg | tail -n 4
[   33.673057] livepatch: tainting kernel with TAINT_LIVEPATCH
[   33.673068] livepatch: enabling patch 'livepatch_sample'
[ 1997.098257] livepatch: disabling patch 'livepatch_sample'
[ 2079.696277] livepatch: enabling patch 'livepatch_sample'


> For me the series doesn't even boot, even with livepatching disabled.

I wonder if you have enabled CONFIG_FTRACE_STARTUP_TEST and if
the ftrace with regs fails on your setup.

Best Regards,
Petr

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

#1314125

On Wed 2016-01-20 10:48:30, Petr Mladek wrote:
> I did the testing on PPC64LE with a kernel based on 4.4.0-rc8
> using the attached config. I used the following stuff:

Ah, I forgot to attach it. Also it is rahter big. Please,
find it at http://pastebin.com/tzJ3mdUd

Best Regards,
Petr

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

#1314033

On Wed, 20 Jan 2016, Michael Ellerman wrote:

> For me the series doesn't even boot, even with livepatching disabled.

Could you please post config and dmesg from that non-booting kernel?

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

#1314170

On Thu, 2016-01-21 at 10:33 +0100, Jiri Kosina wrote:
> On Wed, 20 Jan 2016, Michael Ellerman wrote:
> 
> > For me the series doesn't even boot, even with livepatching disabled.
> 
> Could you please post config and dmesg from that non-booting kernel?

Sorry been busy.

There is no dmesg :)

It gets stuck in early_setup() before the console is even found.

I'll try with Petr's config and see if that helps.

Also I'm using gcc 6.0 built from mainline just last week, and binutils
similarly. So possibly that is part of the problem.

cheers

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

#1314254

On Thu, Jan 21, 2016 at 11:54:51PM +1100, Michael Ellerman wrote:
> There is no dmesg :)
> 
> It gets stuck in early_setup() before the console is even found.

Confirmed.
| Device tree struct  0x00000000014b0000 -> 0x00000000014c0000
| Quiescing Open Firmware ...
| Booting Linux via __start() ...
and that's it.

gcc-6 --version
gcc-6 (SUSE Linux) 6.0.0 20160108 (experimental) [trunk revision 232162]

> Also I'm using gcc 6.0 built from mainline just last week, and binutils
> similarly. So possibly that is part of the problem.

Confirmed. It _is_ the problem.
mcount call sites look normal on first sight...

	Torsten

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

#1314263

On Thu, Jan 21, 2016 at 04:06:33PM +0100, Torsten Duwe wrote:
> mcount call sites looks normal on first sight...

Not quite.
LR is not saved on the stack before the call.
Argh!

Petr, this looks like 12 bytes offset for gcc-6.
I think I can work around the rest.

	Torsten

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

#1314545

On Thu, 21 Jan 2016, Torsten Duwe wrote:

> > mcount call sites looks normal on first sight...
> 
> Not quite.
> LR is not saved on the stack before the call.
> Argh!
> 
> Petr, this looks like 12 bytes offset for gcc-6.
> I think I can work around the rest.

Are we sure that gcc is doing the right thing here?

I am far from claiming understanding of ppc64 ABI, but from what Vojtech 
told me I understood that saving link register is necessary for (at least) 
graph tracer to work properly.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

#1314560

On Thu, Jan 21, 2016 at 10:29:13PM +0100, Jiri Kosina wrote:
> On Thu, 21 Jan 2016, Torsten Duwe wrote:
> 
> > > mcount call sites looks normal on first sight...
> > 
> > Not quite.
> > LR is not saved on the stack before the call.
> > Argh!
> > 
> > Petr, this looks like 12 bytes offset for gcc-6.
> > I think I can work around the rest.
> 
> Are we sure that gcc is doing the right thing here?
> 
> I am far from claiming understanding of ppc64 ABI, but from what Vojtech 
> told me I understood that saving link register is necessary for (at least) 
> graph tracer to work properly.

It is held in R0 only, and saved right after _mcount. Thus, _mcount just
must not clobber R0 or save it the same way as it's done afterwards or
like gcc4 does it.

I'll make a v6 that's compiler agnostic. It's a few lines to change
for the kernel proper, and I'll have to have a look at the trampolines
for modules.

	Torsten

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web