Instead of checking if the architecture running the test was powerpc,
check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
index dd802783ea849..c01a586866304 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
@@ -12,15 +12,14 @@
#include <linux/slab.h>
#include <linux/livepatch.h>
-#if defined(__x86_64__)
+#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
+#define FN_PREFIX
+#elif defined(__x86_64__)
#define FN_PREFIX __x64_
#elif defined(__s390x__)
#define FN_PREFIX __s390x_
#elif defined(__aarch64__)
#define FN_PREFIX __arm64_
-#else
-/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
-#define FN_PREFIX
#endif
/* Protects klp_pids */
--
2.52.0On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza wrote:
> Instead of checking if the architecture running the test was powerpc,
> check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
>
> No functional changes.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> index dd802783ea849..c01a586866304 100644
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> @@ -12,15 +12,14 @@
> #include <linux/slab.h>
> #include <linux/livepatch.h>
>
> -#if defined(__x86_64__)
> +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> +#define FN_PREFIX
> +#elif defined(__x86_64__)
> #define FN_PREFIX __x64_
> #elif defined(__s390x__)
> #define FN_PREFIX __s390x_
> #elif defined(__aarch64__)
> #define FN_PREFIX __arm64_
> -#else
> -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> -#define FN_PREFIX
The patch does maintain the previous behavior, but I'm wondering if the
original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was correct:
$ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE && !COMPAT
depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
Perhaps I just forgot what that additional piece of information that
explains the comment (highly probable these days), and if so, might be
nice to add to this commit since I don't see it in 6a71770442b5
("selftests: livepatch: Test livepatching a heavily called syscall").
Thanks,
--
Joe
On Mon, 2026-03-16 at 16:12 -0400, Joe Lawrence wrote:
> On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> wrote:
> > Instead of checking if the architecture running the test was
> > powerpc,
> > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
> >
> > No functional changes.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > index dd802783ea849..c01a586866304 100644
> > ---
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > +++
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > @@ -12,15 +12,14 @@
> > #include <linux/slab.h>
> > #include <linux/livepatch.h>
> >
> > -#if defined(__x86_64__)
> > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > +#define FN_PREFIX
> > +#elif defined(__x86_64__)
> > #define FN_PREFIX __x64_
> > #elif defined(__s390x__)
> > #define FN_PREFIX __s390x_
> > #elif defined(__aarch64__)
> > #define FN_PREFIX __arm64_
> > -#else
> > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > -#define FN_PREFIX
>
> The patch does maintain the previous behavior, but I'm wondering if
> the
> original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> correct:
>
> $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> !COMPAT
> depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
>
> Perhaps I just forgot what that additional piece of information that
> explains the comment (highly probable these days), and if so, might
> be
> nice to add to this commit since I don't see it in 6a71770442b5
> ("selftests: livepatch: Test livepatching a heavily called syscall").
Looking again at the code and at the symbols for SLE for ppc64le, I can
say that, even with ARCH_HAS_SYSCALL_WRAPPER being set, the syscall
names are not changed for ppc64le. Looking at
arch/powerpc/kernel/systbl.c:
#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
#define __SYSCALL(nr, entry) [nr] = entry,
#else
/*
* Coerce syscall handlers with arbitrary parameters to common type
* requires cast to void* to avoid -Wcast-function-type.
*/
#define __SYSCALL(nr, entry) [nr] = (void *) entry,
#endif
Differently from x86 and other already covered archs, the syscall name
remains the same on ppc. I will add a comment about it in the next
version, adding a case for ppc and an #else clause with an error, so
future architectures that support livepatching can add a patch here to
fix it for them.
>
> Thanks,
> --
> Joe
On Tue, 31 Mar 2026, Marcos Paulo de Souza wrote:
> On Mon, 2026-03-16 at 16:12 -0400, Joe Lawrence wrote:
> > On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> > wrote:
> > > Instead of checking if the architecture running the test was
> > > powerpc,
> > > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
> > >
> > > No functional changes.
> > >
> > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > > ---
> > > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > index dd802783ea849..c01a586866304 100644
> > > ---
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > +++
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > @@ -12,15 +12,14 @@
> > > #include <linux/slab.h>
> > > #include <linux/livepatch.h>
> > >
> > > -#if defined(__x86_64__)
> > > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > > +#define FN_PREFIX
> > > +#elif defined(__x86_64__)
> > > #define FN_PREFIX __x64_
> > > #elif defined(__s390x__)
> > > #define FN_PREFIX __s390x_
> > > #elif defined(__aarch64__)
> > > #define FN_PREFIX __arm64_
> > > -#else
> > > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > > -#define FN_PREFIX
> >
> > The patch does maintain the previous behavior, but I'm wondering if
> > the
> > original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> > correct:
> >
> > $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> > select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> > !COMPAT
> > depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> >
> > Perhaps I just forgot what that additional piece of information that
> > explains the comment (highly probable these days), and if so, might
> > be
> > nice to add to this commit since I don't see it in 6a71770442b5
> > ("selftests: livepatch: Test livepatching a heavily called syscall").
>
> Looking again at the code and at the symbols for SLE for ppc64le, I can
> say that, even with ARCH_HAS_SYSCALL_WRAPPER being set, the syscall
> names are not changed for ppc64le. Looking at
> arch/powerpc/kernel/systbl.c:
>
> #ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> #define __SYSCALL(nr, entry) [nr] = entry,
> #else
> /*
> * Coerce syscall handlers with arbitrary parameters to common type
> * requires cast to void* to avoid -Wcast-function-type.
> */
> #define __SYSCALL(nr, entry) [nr] = (void *) entry,
> #endif
I think this is not the complete picture though. The definition of the
syscall table did not change but the actual wrappers (or syscall functions
naming before) did change a couple of times even on powerpc if I am
reading the git history right. ARCH_HAS_SYSCALL_WRAPPER also changed how
syscall parameters are consumed (from registers to stack frame (pt_regs)).
It is not particularly relevant for getpid() which is SYSCALL_DEFINE0()
but I wanted to point that out.
Miroslav
On Mon, 2026-03-16 at 16:12 -0400, Joe Lawrence wrote:
> On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> wrote:
> > Instead of checking if the architecture running the test was
> > powerpc,
> > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
> >
> > No functional changes.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > index dd802783ea849..c01a586866304 100644
> > ---
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > +++
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > @@ -12,15 +12,14 @@
> > #include <linux/slab.h>
> > #include <linux/livepatch.h>
> >
> > -#if defined(__x86_64__)
> > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > +#define FN_PREFIX
> > +#elif defined(__x86_64__)
> > #define FN_PREFIX __x64_
> > #elif defined(__s390x__)
> > #define FN_PREFIX __s390x_
> > #elif defined(__aarch64__)
> > #define FN_PREFIX __arm64_
> > -#else
> > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > -#define FN_PREFIX
>
> The patch does maintain the previous behavior, but I'm wondering if
> the
> original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> correct:
>
> $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> !COMPAT
> depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
>
> Perhaps I just forgot what that additional piece of information that
> explains the comment (highly probable these days), and if so, might
> be
> nice to add to this commit since I don't see it in 6a71770442b5
> ("selftests: livepatch: Test livepatching a heavily called syscall").
Sorry for the late reply...
Well, so far the test always run fine for us, so I never looked why ppc
didn't have ARCH_HAS_SYSCALL_WRAPPER TBH... I can add some information
about it, sure.
>
> Thanks,
> --
> Joe
On Mon, 16 Mar 2026, Joe Lawrence wrote:
> On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza wrote:
> > Instead of checking if the architecture running the test was powerpc,
> > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
There is a typo...
s/CONF_ARCH_HAS_SYSCALL_WRAPPER/CONFIG_ARCH_HAS_SYSCALL_WRAPPER/
> >
> > No functional changes.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > index dd802783ea849..c01a586866304 100644
> > --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > @@ -12,15 +12,14 @@
> > #include <linux/slab.h>
> > #include <linux/livepatch.h>
> >
> > -#if defined(__x86_64__)
> > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > +#define FN_PREFIX
> > +#elif defined(__x86_64__)
> > #define FN_PREFIX __x64_
> > #elif defined(__s390x__)
> > #define FN_PREFIX __s390x_
> > #elif defined(__aarch64__)
> > #define FN_PREFIX __arm64_
> > -#else
> > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > -#define FN_PREFIX
>
> The patch does maintain the previous behavior, but I'm wondering if the
> original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was correct:
>
> $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE && !COMPAT
> depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
>
> Perhaps I just forgot what that additional piece of information that
> explains the comment (highly probable these days), and if so, might be
> nice to add to this commit since I don't see it in 6a71770442b5
> ("selftests: livepatch: Test livepatching a heavily called syscall").
I would take a bit further. We would rely on
CONFIG_ARCH_HAS_SYSCALL_WRAPPER being set/unset per listed architectures
"correctly" for us. If it changes somehow (though I cannot imagine reasons
for that but let's say we add new architecture. LoongArch also supports
live patching.), the above might evaluate to something broken.
So I would perhaps prefer to stay with the logic that defines FN_PREFIX
per architecture and has also #else branch for the rest. And more comments
never hurt.
Btw, see also
https://sashiko.dev/#/patchset/20260313-lp-tests-old-fixes-v1-0-71ac6dfb3253%40suse.com
for the Sashiko AI review. It also commented on this patch. Marcos, I
guess that you will look there and I will just omit what Sashiko found in
my review if I spot the same thing.
Miroslav
On Thu, 2026-03-19 at 13:54 +0100, Miroslav Benes wrote:
> On Mon, 16 Mar 2026, Joe Lawrence wrote:
>
> > On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> > wrote:
> > > Instead of checking if the architecture running the test was
> > > powerpc,
> > > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
>
> There is a typo...
> s/CONF_ARCH_HAS_SYSCALL_WRAPPER/CONFIG_ARCH_HAS_SYSCALL_WRAPPER/
Thanks, I'll fix it in my next version.
>
> > >
> > > No functional changes.
> > >
> > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > > ---
> > > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.
> > > c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall
> > > .c
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall
> > > .c
> > > index dd802783ea849..c01a586866304 100644
> > > ---
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall
> > > .c
> > > +++
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall
> > > .c
> > > @@ -12,15 +12,14 @@
> > > #include <linux/slab.h>
> > > #include <linux/livepatch.h>
> > >
> > > -#if defined(__x86_64__)
> > > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > > +#define FN_PREFIX
> > > +#elif defined(__x86_64__)
> > > #define FN_PREFIX __x64_
> > > #elif defined(__s390x__)
> > > #define FN_PREFIX __s390x_
> > > #elif defined(__aarch64__)
> > > #define FN_PREFIX __arm64_
> > > -#else
> > > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > > -#define FN_PREFIX
> >
> > The patch does maintain the previous behavior, but I'm wondering if
> > the
> > original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> > correct:
> >
> > $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> > select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> > !COMPAT
> > depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> >
> > Perhaps I just forgot what that additional piece of information
> > that
> > explains the comment (highly probable these days), and if so, might
> > be
> > nice to add to this commit since I don't see it in 6a71770442b5
> > ("selftests: livepatch: Test livepatching a heavily called
> > syscall").
>
> I would take a bit further. We would rely on
> CONFIG_ARCH_HAS_SYSCALL_WRAPPER being set/unset per listed
> architectures
> "correctly" for us. If it changes somehow (though I cannot imagine
> reasons
> for that but let's say we add new architecture. LoongArch also
> supports
> live patching.), the above might evaluate to something broken.
>
I agree. Given that nobody even complained about it, I would say that
people testing on ppc64le has this defined correctly. Whenever new
archs start supporting livepatching, we can always revisit.
> So I would perhaps prefer to stay with the logic that defines
> FN_PREFIX
> per architecture and has also #else branch for the rest. And more
> comments
> never hurt.
Agreed.
>
> Btw, see also
> https://sashiko.dev/#/patchset/20260313-lp-tests-old-fixes-v1-0-71ac6dfb3253%40suse.com
>
> for the Sashiko AI review. It also commented on this patch. Marcos, I
> guess that you will look there and I will just omit what Sashiko
> found in
> my review if I spot the same thing.
I already checked there. Maybe adding more context to the patch and
code will avoid further confusion about it. Let me add it in the v2.
Thanks for the reviews Miroslav and Joe!
>
> Miroslav
> > So I would perhaps prefer to stay with the logic that defines > > FN_PREFIX > > per architecture and has also #else branch for the rest. And more > > comments > > never hurt. > > Agreed. Hm, so I thought about a bit more and I very likely misunderstood the motivation behind the patch. I will speculate and correct me if I am wrong, please. The idea behind the whole patch set is to make the selftests run on older kernels which I think is something we should support. The issue is that old kernels (like mentioned 4.12) do not have syscall wrappers at all. getpid() syscall is just plain old sys_getpid there and not the current __x64_sys_getpid on x86. The patch fixes it by checking CONFIG_ARCH_HAS_SYSCALL_WRAPPER and defining FN_PREFIX accordingly. So, if this is correct, I think it should be done differently. We should have something like syscall_wrapper.h which would define FN_PREFIX for the supported architectures and different kernel versions since the wrappers may have changed a couple of times during the history. In that case there could then be an #else branch which might just error out with the message to add proper syscall wrapper naming. The changelog then should explain it because it is not in fact tight to powerpc. What do you think? Am I off again? Miroslav
On Fri, 2026-03-20 at 11:45 +0100, Miroslav Benes wrote:
> > > So I would perhaps prefer to stay with the logic that defines
> > > FN_PREFIX
> > > per architecture and has also #else branch for the rest. And more
> > > comments
> > > never hurt.
> >
> > Agreed.
>
> Hm, so I thought about a bit more and I very likely misunderstood the
> motivation behind the patch. I will speculate and correct me if I am
> wrong, please. The idea behind the whole patch set is to make the
> selftests run on older kernels which I think is something we should
> support. The issue is that old kernels (like mentioned 4.12) do not
> have
> syscall wrappers at all. getpid() syscall is just plain old
> sys_getpid
> there and not the current __x64_sys_getpid on x86. The patch fixes it
> by checking CONFIG_ARCH_HAS_SYSCALL_WRAPPER and defining FN_PREFIX
> accordingly.
Exactly. The definition was added on
commit 1bd21c6c21e848996339508d3ffb106d505256a8
Author: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Thu Apr 5 11:53:01 2018 +0200
syscalls/core: Introduce CONFIG_ARCH_HAS_SYSCALL_WRAPPER=y
>
> So, if this is correct, I think it should be done differently. We
> should
> have something like syscall_wrapper.h which would define FN_PREFIX
> for
> the supported architectures and different kernel versions since the
> wrappers may have changed a couple of times during the history. In
> that
> case there could then be an #else branch which might just error out
> with
> the message to add proper syscall wrapper naming.
Well, it seems too much for a simple test to me, but I can do that, no
problem.
>
> The changelog then should explain it because it is not in fact tight
> to
> powerpc.
Makes sense, I'll change it.
>
> What do you think? Am I off again?
I agree with everything, but adding another header file seems a little
too much work for a simple test case, but it's doable. Let me work on
it.
>
> Miroslav
© 2016 - 2026 Red Hat, Inc.