Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER

[Marcos Paulo de Souza <mpdesouza@suse.com>]

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