* Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR
From: Thomas Gleixner @ 2026-07-03 9:27 UTC (permalink / raw)
To: Sven Schnelle
Cc: H. Peter Anvin, Michal Suchánek, Peter Zijlstra,
Jonathan Corbet, Shuah Khan, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Andy Lutomirski,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Andrew Donnellan,
Mark Rutland, Arnd Bergmann, Jiaxun Yang, Ryan Roberts,
Greg Kroah-Hartman, Mukesh Kumar Chaurasiya, Shrikanth Hegde,
Zong Li, Nam Cao, Deepak Gupta, Lukas Gerlach, Rui Qi, Kees Cook,
linux-doc, linux-kernel, loongarch, linuxppc-dev, linux-riscv,
linux-s390
In-Reply-To: <yt9dwlvca8rn.fsf@linux.ibm.com>
On Fri, Jul 03 2026 at 08:26, Sven Schnelle wrote:
> Thomas Gleixner <tglx@kernel.org> writes:
>> It's less than obvious and I have no objections to clean that up and
>> make it more intuitive, but I still fail to see what Michal is actually
>> trying to solve and what the magic flag is for. If s390 requires it,
>> then that's an s390 problem, but definitely x86 does not.
>
> The difference between x86 and s390 is that on s390, regs->gprs[2] is
> used for both the syscall number and the syscall return value.
> That was a design mistake early in the begin about 25 years ago, but
> it's ABI now, so it cannot be changed.
Cute.
> When seccomp decides to skip a syscall, it write a return value into
> regs->gprs[2]. When syscall_enter_from_user_mode_work() returns, it
> returns this number. If it's negative all is good - the 'if (likely(nr <
> NR_syscalls))' conditiion would just catch it and skip the syscall.
>
> But if it's a positive number, the code cannot distinguish whether
> that's a return value or a syscall number.
>
> So I introduced PIF_SYSCALL_RET_SET when converting s390 to generic
> entry. This flag tells the syscall code that a return value was set in
> ptregs and the syscall should be skipped.
You also could have added a 'syscall_ret' member to pt_regs, operate
on that for the return values (seccomp, syscall...) and swap it into
gprs[2] right before returning to user space.
> I'd like to see something like the change from Michal going in - cleaned
> up of course. It would allow us to get rid of PIF_SYSCALL_RET_SET.
I have no objections against cleaning it up and making it less
convoluted.
^ permalink raw reply
* Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR
From: Peter Zijlstra @ 2026-07-03 9:25 UTC (permalink / raw)
To: Sven Schnelle
Cc: Thomas Gleixner, H. Peter Anvin, Michal Suchánek,
Jonathan Corbet, Shuah Khan, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Andy Lutomirski,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Andrew Donnellan,
Mark Rutland, Arnd Bergmann, Jiaxun Yang, Ryan Roberts,
Greg Kroah-Hartman, Mukesh Kumar Chaurasiya, Shrikanth Hegde,
Zong Li, Nam Cao, Deepak Gupta, Lukas Gerlach, Rui Qi, Kees Cook,
linux-doc, linux-kernel, loongarch, linuxppc-dev, linux-riscv,
linux-s390
In-Reply-To: <yt9dwlvca8rn.fsf@linux.ibm.com>
On Fri, Jul 03, 2026 at 08:26:36AM +0200, Sven Schnelle wrote:
> The difference between x86 and s390 is that on s390, regs->gprs[2] is
> used for both the syscall number and the syscall return value.
> That was a design mistake early in the begin about 25 years ago, but
> it's ABI now, so it cannot be changed.
>
> When seccomp decides to skip a syscall, it write a return value into
> regs->gprs[2]. When syscall_enter_from_user_mode_work() returns, it
> returns this number. If it's negative all is good - the 'if (likely(nr <
> NR_syscalls))' conditiion would just catch it and skip the syscall.
You do have regs->orig_gpr2; I didn't fully track its usage, but can't
you treat that as the syscall nr and always consider regs->gprs[2] as the
return value?
^ permalink raw reply
* Re: [PATCH] powerpc/ptrace: Add offsetof check for exit_flags in pt_regs_check()
From: Amit Machhiwal @ 2026-07-03 8:55 UTC (permalink / raw)
To: Mukesh Kumar Chaurasiya (IBM)
Cc: maddy, mpe, npiggin, chleroy, mkchauras, sshegde, linuxppc-dev,
linux-kernel
In-Reply-To: <20260703074948.1668744-1-mkchauras@gmail.com>
On 2026/07/03 01:19 PM, Mukesh Kumar Chaurasiya (IBM) wrote:
> pt_regs_check() verifies that fields shared between struct pt_regs and
> struct user_pt_regs sit at the same offset, to catch any accidental
> layout divergence between the kernel and uapi structures.
>
> Add the missing check for exit_flags, following the same pattern as the
> existing checks for result, dsisr, dar and the other shared fields.
>
> Fixes: d7a6797e0bc1 ("powerpc: add exit_flags field in pt_regs")
You may want to add below in order to get this patch backported:
Cc: stable@vger.kernel.org
> Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
> ---
> arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> index 316d4f5ead8e..fb3fec73f66a 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -235,6 +235,8 @@ void __init pt_regs_check(void)
> offsetof(struct user_pt_regs, dsisr));
> BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
> offsetof(struct user_pt_regs, result));
> + BUILD_BUG_ON(offsetof(struct pt_regs, exit_flags) !=
> + offsetof(struct user_pt_regs, exit_flags));
At first, it appeared to be redundant as we already check
CHECK_REG(PT_EXIT_FLAGS, exit_flags);
which expands to:
#define CHECK_REG(_pt, _reg) \
BUILD_BUG_ON(_pt != (offsetof(struct user_pt_regs, _reg) / \
sizeof(unsigned long)));
But looking closely, it turns out to be non-redundant. CHECK_REG only
touches struct user_pt_regs — it validates the PT_EXIT_FLAGS constant
against the uapi layout. It says nothing about struct pt_regs.
struct pt_regs (asm/ptrace.h) embeds struct user_pt_regs in a union
alongside an anonymous struct that re-declares all the same fields with
kernel-internal aliases (dear/dar, esr/dsisr). exit_flags lives in that
anonymous struct. A field accidentally inserted there between result and
exit_flags would silently break the union aliasing without CHECK_REG
catching it. The new BUILD_BUG_ON closes that gap, consistent with how
result, dar and dsisr are already protected in the same block.
Looks good to me.
Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>
Thanks,
Amit
>
> BUILD_BUG_ON(sizeof(struct user_pt_regs) > sizeof(struct pt_regs));
>
> --
> 2.55.0
>
>
^ permalink raw reply
* Re: [PATCH V3] powerpc/syscall: Fix syscall skip handling for seccomp and ptrace
From: Mukesh Kumar Chaurasiya @ 2026-07-03 8:36 UTC (permalink / raw)
To: Michal Suchánek
Cc: oleg, maddy, mpe, npiggin, chleroy, sshegde, mchauras, thuth,
segher, ryan.roberts, ruanjinjie, linuxppc-dev, linux-kernel
In-Reply-To: <akdz6O-p9tdr1irz@kunlun.suse.cz>
On Fri, Jul 03, 2026 at 10:33:44AM +0200, Michal Suchánek wrote:
> On Fri, Jul 03, 2026 at 01:41:00PM +0530, Mukesh Kumar Chaurasiya (IBM) wrote:
> > After enabling GENERIC_ENTRY on PowerPC, syscall_enter_from_user_mode()
> > returns -1 as a sentinel to signal that seccomp or ptrace has intercepted
> > the syscall and already set a return value via syscall_set_return_value().
> > system_call_exception() was not handling this sentinel, and since -1UL
> > is >= NR_syscalls, the code fell into the out-of-range path and returned
> > -ENOSYS, overwriting the errno already placed in regs->gpr[3].
> >
> > The naive fix of checking r0 == -1L before the NR_syscalls bounds check
> > is ambiguous: a user legitimately calling syscall(-1) also produces r0 ==
> > -1L, and a tracer intercepting such a call would have its injected return
> > value silently discarded.
> >
> > Fix this properly by introducing regs->entry_flags, a kernel-internal
> > field in struct pt_regs (consuming one slot of the existing __pt_regs_pad
> > so the ABI is preserved), with SYSCALL_ENTRY_RET_SET as an out-of-band
> > flag. syscall_set_return_value() sets this flag whenever seccomp or ptrace
> > injects a return value. system_call_exception() zeros entry_flags before
> > calling syscall_enter_from_user_mode(), then checks and clears the flag
> > afterwards: if set, it returns regs->gpr[3] directly regardless of what
> > syscall number the user originally requested.
> >
> > This handles all seccomp actions correctly:
> >
> > - SECCOMP_RET_ERRNO, SECCOMP_RET_TRACE (no tracer), SECCOMP_RET_USER_NOTIF:
> > all call syscall_set_return_value(), flag is set, injected value returned.
> > - SECCOMP_RET_TRAP, SECCOMP_RET_KILL: call syscall_rollback() and deliver
> > a signal; flag is not set, but the process is dying so the return value
> > is irrelevant.
> >
> > The fix covers both ppc32 and ppc64 with no #ifdefs.
> >
> > Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> > Reported-by: Michal Suchánek <msuchanek@suse.de>
> > Closes: https://lore.kernel.org/all/ajpp-_XnbF3UTM_E@kunlun.suse.cz/
> > Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
> > ---
> > v2 -> v3:
> > - Last fix is not working for -1 syscall. Fixed that with this.
> > v2: https://lore.kernel.org/all/20260629182946.419552-1-mkchauras@gmail.com
> >
> > v1 -> v2:
> > - Fix issues in the previous fix (Michal)
> > v1: https://lore.kernel.org/all/20260624171520.772408-1-mkchauras@gmail.com
> >
> > arch/powerpc/include/asm/ptrace.h | 22 +++++++++++++++++++++-
> > arch/powerpc/include/asm/syscall.h | 6 ++++++
> > arch/powerpc/include/uapi/asm/ptrace.h | 6 ++++--
> > arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
> > arch/powerpc/kernel/syscall.c | 18 ++++++++++++++++++
> > 5 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> > index fdeb97421785..1a53d5cfa8db 100644
> > --- a/arch/powerpc/include/asm/ptrace.h
> > +++ b/arch/powerpc/include/asm/ptrace.h
> > @@ -54,8 +54,9 @@ struct pt_regs
> > };
> > unsigned long result;
> > unsigned long exit_flags;
> > + unsigned long entry_flags;
> > /* Maintain 16 byte interrupt stack alignment */
> > - unsigned long __pt_regs_pad[3];
> > + unsigned long __pt_regs_pad[2];
> > };
> > };
> > #if defined(CONFIG_PPC64) || defined(CONFIG_PPC_KUAP)
> > @@ -233,6 +234,25 @@ static inline unsigned long frame_pointer(struct pt_regs *regs)
> > #define current_pt_regs() \
> > ((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
> >
> > +/*
> > + * SYSCALL_ENTRY_RET_SET: seccomp or ptrace called syscall_set_return_value()
> > + * and wants the syscall skipped; regs->gpr[3] already holds the return value.
> > + */
> > +#define SYSCALL_ENTRY_RET_SET BIT(0)
> > +
> > +static inline void set_syscall_entry_ret(struct pt_regs *regs)
> > +{
> > + regs->entry_flags |= SYSCALL_ENTRY_RET_SET;
> > +}
> > +
> > +static inline bool test_and_clear_syscall_entry_ret(struct pt_regs *regs)
> > +{
> > + bool set = !!(regs->entry_flags & SYSCALL_ENTRY_RET_SET);
> > +
> > + regs->entry_flags &= ~SYSCALL_ENTRY_RET_SET;
> > + return set;
> > +}
> > +
> > /*
> > * The 4 low bits (0xf) are available as flags to overload the trap word,
> > * because interrupt vectors have minimum alignment of 0x10. TRAP_FLAGS_MASK
> > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> > index 834fcc4f7b54..9ae79326abe3 100644
> > --- a/arch/powerpc/include/asm/syscall.h
> > +++ b/arch/powerpc/include/asm/syscall.h
> > @@ -98,6 +98,12 @@ static inline void syscall_set_return_value(struct task_struct *task,
> > regs->gpr[3] = val;
> > }
> > }
> > + /*
> > + * Mark that a return value has been explicitly set by seccomp or
> > + * ptrace so that system_call_exception() can skip the syscall
> > + * unconditionally, even when the user requested syscall(-1).
> > + */
> > + set_syscall_entry_ret(regs);
> > }
> >
> > static inline void syscall_get_arguments(struct task_struct *task,
> > diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
> > index a393b7f2760a..2f2a43414fe6 100644
> > --- a/arch/powerpc/include/uapi/asm/ptrace.h
> > +++ b/arch/powerpc/include/uapi/asm/ptrace.h
> > @@ -56,7 +56,8 @@ struct pt_regs
> > unsigned long dsisr; /* on 4xx/Book-E used for ESR */
> > unsigned long result; /* Result of a system call */
> > unsigned long exit_flags; /* System call exit flags */
> > - unsigned long __pt_regs_pad[3]; /* Maintain 16 byte interrupt stack alignment */
> > + unsigned long entry_flags; /* System call entry flags */
> > + unsigned long __pt_regs_pad[2]; /* Maintain 16 byte interrupt stack alignment */
> > };
> >
> > #endif /* __ASSEMBLER__ */
> > @@ -117,7 +118,8 @@ struct pt_regs
> > #define PT_DSISR 42
> > #define PT_RESULT 43
> > #define PT_EXIT_FLAGS 44
> > -#define PT_PAD 47 /* 3 times */
> > +#define PT_ENTRY_FLAGS 45
> > +#define PT_PAD 46 /* 2 times */
> > #define PT_DSCR 48
> > #define PT_REGS_COUNT 48
> >
> > diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> > index 316d4f5ead8e..440d00690cf2 100644
> > --- a/arch/powerpc/kernel/ptrace/ptrace.c
> > +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> > @@ -235,6 +235,8 @@ void __init pt_regs_check(void)
> > offsetof(struct user_pt_regs, dsisr));
> > BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
> > offsetof(struct user_pt_regs, result));
> > + BUILD_BUG_ON(offsetof(struct pt_regs, entry_flags) !=
> > + offsetof(struct user_pt_regs, entry_flags));
> >
> > BUILD_BUG_ON(sizeof(struct user_pt_regs) > sizeof(struct pt_regs));
> >
> > diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
> > index a9da2af6efa8..c7458aae199d 100644
> > --- a/arch/powerpc/kernel/syscall.c
> > +++ b/arch/powerpc/kernel/syscall.c
> > @@ -19,9 +19,27 @@ notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
> > long ret;
> > syscall_fn f;
> >
> > + /*
> > + * Zero entry_flags before syscall_enter_from_user_mode() so that
> > + * syscall_set_return_value() can set SYSCALL_ENTRY_RET_SET as an
> > + * unambiguous out-of-band signal. The field is not initialised by
> > + * the entry assembly.
> > + */
> > + regs->entry_flags = 0;
> > add_random_kstack_offset();
> > r0 = syscall_enter_from_user_mode(regs, r0);
> >
> > + /*
> > + * Seccomp or ptrace may have set a return value and requested that
> > + * the syscall be skipped. syscall_set_return_value() sets
> > + * SYSCALL_ENTRY_RET_SET in regs->entry_flags as an
> > + * unambiguous out-of-band signal. This avoids the ambiguity of
> > + * using r0 == -1 as the skip sentinel when the user themselves
> > + * called syscall(-1).
> > + */
> > + if (unlikely(test_and_clear_syscall_entry_ret(regs)))
> > + return regs->gpr[3];
>
> Hello,
>
> shouldn't this use the getter to correctly decode the value for both scv
> and non-scv case?
>
> Thanks
>
> Michal
>
Oh yeah,
Correct. Lemme fix that.
Regards,
Mukesh
> > +
> > if (unlikely(r0 >= NR_syscalls)) {
> > if (unlikely(trap_is_unsupported_scv(regs))) {
> > /* Unsupported scv vector */
> > --
> > 2.55.0
> >
^ permalink raw reply
* Re: [PATCH V3] powerpc/syscall: Fix syscall skip handling for seccomp and ptrace
From: Michal Suchánek @ 2026-07-03 8:33 UTC (permalink / raw)
To: Mukesh Kumar Chaurasiya (IBM)
Cc: oleg, maddy, mpe, npiggin, chleroy, sshegde, mchauras, thuth,
segher, ryan.roberts, ruanjinjie, linuxppc-dev, linux-kernel
In-Reply-To: <20260703081100.1681924-1-mkchauras@gmail.com>
On Fri, Jul 03, 2026 at 01:41:00PM +0530, Mukesh Kumar Chaurasiya (IBM) wrote:
> After enabling GENERIC_ENTRY on PowerPC, syscall_enter_from_user_mode()
> returns -1 as a sentinel to signal that seccomp or ptrace has intercepted
> the syscall and already set a return value via syscall_set_return_value().
> system_call_exception() was not handling this sentinel, and since -1UL
> is >= NR_syscalls, the code fell into the out-of-range path and returned
> -ENOSYS, overwriting the errno already placed in regs->gpr[3].
>
> The naive fix of checking r0 == -1L before the NR_syscalls bounds check
> is ambiguous: a user legitimately calling syscall(-1) also produces r0 ==
> -1L, and a tracer intercepting such a call would have its injected return
> value silently discarded.
>
> Fix this properly by introducing regs->entry_flags, a kernel-internal
> field in struct pt_regs (consuming one slot of the existing __pt_regs_pad
> so the ABI is preserved), with SYSCALL_ENTRY_RET_SET as an out-of-band
> flag. syscall_set_return_value() sets this flag whenever seccomp or ptrace
> injects a return value. system_call_exception() zeros entry_flags before
> calling syscall_enter_from_user_mode(), then checks and clears the flag
> afterwards: if set, it returns regs->gpr[3] directly regardless of what
> syscall number the user originally requested.
>
> This handles all seccomp actions correctly:
>
> - SECCOMP_RET_ERRNO, SECCOMP_RET_TRACE (no tracer), SECCOMP_RET_USER_NOTIF:
> all call syscall_set_return_value(), flag is set, injected value returned.
> - SECCOMP_RET_TRAP, SECCOMP_RET_KILL: call syscall_rollback() and deliver
> a signal; flag is not set, but the process is dying so the return value
> is irrelevant.
>
> The fix covers both ppc32 and ppc64 with no #ifdefs.
>
> Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> Reported-by: Michal Suchánek <msuchanek@suse.de>
> Closes: https://lore.kernel.org/all/ajpp-_XnbF3UTM_E@kunlun.suse.cz/
> Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
> ---
> v2 -> v3:
> - Last fix is not working for -1 syscall. Fixed that with this.
> v2: https://lore.kernel.org/all/20260629182946.419552-1-mkchauras@gmail.com
>
> v1 -> v2:
> - Fix issues in the previous fix (Michal)
> v1: https://lore.kernel.org/all/20260624171520.772408-1-mkchauras@gmail.com
>
> arch/powerpc/include/asm/ptrace.h | 22 +++++++++++++++++++++-
> arch/powerpc/include/asm/syscall.h | 6 ++++++
> arch/powerpc/include/uapi/asm/ptrace.h | 6 ++++--
> arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
> arch/powerpc/kernel/syscall.c | 18 ++++++++++++++++++
> 5 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index fdeb97421785..1a53d5cfa8db 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -54,8 +54,9 @@ struct pt_regs
> };
> unsigned long result;
> unsigned long exit_flags;
> + unsigned long entry_flags;
> /* Maintain 16 byte interrupt stack alignment */
> - unsigned long __pt_regs_pad[3];
> + unsigned long __pt_regs_pad[2];
> };
> };
> #if defined(CONFIG_PPC64) || defined(CONFIG_PPC_KUAP)
> @@ -233,6 +234,25 @@ static inline unsigned long frame_pointer(struct pt_regs *regs)
> #define current_pt_regs() \
> ((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
>
> +/*
> + * SYSCALL_ENTRY_RET_SET: seccomp or ptrace called syscall_set_return_value()
> + * and wants the syscall skipped; regs->gpr[3] already holds the return value.
> + */
> +#define SYSCALL_ENTRY_RET_SET BIT(0)
> +
> +static inline void set_syscall_entry_ret(struct pt_regs *regs)
> +{
> + regs->entry_flags |= SYSCALL_ENTRY_RET_SET;
> +}
> +
> +static inline bool test_and_clear_syscall_entry_ret(struct pt_regs *regs)
> +{
> + bool set = !!(regs->entry_flags & SYSCALL_ENTRY_RET_SET);
> +
> + regs->entry_flags &= ~SYSCALL_ENTRY_RET_SET;
> + return set;
> +}
> +
> /*
> * The 4 low bits (0xf) are available as flags to overload the trap word,
> * because interrupt vectors have minimum alignment of 0x10. TRAP_FLAGS_MASK
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index 834fcc4f7b54..9ae79326abe3 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -98,6 +98,12 @@ static inline void syscall_set_return_value(struct task_struct *task,
> regs->gpr[3] = val;
> }
> }
> + /*
> + * Mark that a return value has been explicitly set by seccomp or
> + * ptrace so that system_call_exception() can skip the syscall
> + * unconditionally, even when the user requested syscall(-1).
> + */
> + set_syscall_entry_ret(regs);
> }
>
> static inline void syscall_get_arguments(struct task_struct *task,
> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
> index a393b7f2760a..2f2a43414fe6 100644
> --- a/arch/powerpc/include/uapi/asm/ptrace.h
> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
> @@ -56,7 +56,8 @@ struct pt_regs
> unsigned long dsisr; /* on 4xx/Book-E used for ESR */
> unsigned long result; /* Result of a system call */
> unsigned long exit_flags; /* System call exit flags */
> - unsigned long __pt_regs_pad[3]; /* Maintain 16 byte interrupt stack alignment */
> + unsigned long entry_flags; /* System call entry flags */
> + unsigned long __pt_regs_pad[2]; /* Maintain 16 byte interrupt stack alignment */
> };
>
> #endif /* __ASSEMBLER__ */
> @@ -117,7 +118,8 @@ struct pt_regs
> #define PT_DSISR 42
> #define PT_RESULT 43
> #define PT_EXIT_FLAGS 44
> -#define PT_PAD 47 /* 3 times */
> +#define PT_ENTRY_FLAGS 45
> +#define PT_PAD 46 /* 2 times */
> #define PT_DSCR 48
> #define PT_REGS_COUNT 48
>
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> index 316d4f5ead8e..440d00690cf2 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -235,6 +235,8 @@ void __init pt_regs_check(void)
> offsetof(struct user_pt_regs, dsisr));
> BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
> offsetof(struct user_pt_regs, result));
> + BUILD_BUG_ON(offsetof(struct pt_regs, entry_flags) !=
> + offsetof(struct user_pt_regs, entry_flags));
>
> BUILD_BUG_ON(sizeof(struct user_pt_regs) > sizeof(struct pt_regs));
>
> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
> index a9da2af6efa8..c7458aae199d 100644
> --- a/arch/powerpc/kernel/syscall.c
> +++ b/arch/powerpc/kernel/syscall.c
> @@ -19,9 +19,27 @@ notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
> long ret;
> syscall_fn f;
>
> + /*
> + * Zero entry_flags before syscall_enter_from_user_mode() so that
> + * syscall_set_return_value() can set SYSCALL_ENTRY_RET_SET as an
> + * unambiguous out-of-band signal. The field is not initialised by
> + * the entry assembly.
> + */
> + regs->entry_flags = 0;
> add_random_kstack_offset();
> r0 = syscall_enter_from_user_mode(regs, r0);
>
> + /*
> + * Seccomp or ptrace may have set a return value and requested that
> + * the syscall be skipped. syscall_set_return_value() sets
> + * SYSCALL_ENTRY_RET_SET in regs->entry_flags as an
> + * unambiguous out-of-band signal. This avoids the ambiguity of
> + * using r0 == -1 as the skip sentinel when the user themselves
> + * called syscall(-1).
> + */
> + if (unlikely(test_and_clear_syscall_entry_ret(regs)))
> + return regs->gpr[3];
Hello,
shouldn't this use the getter to correctly decode the value for both scv
and non-scv case?
Thanks
Michal
> +
> if (unlikely(r0 >= NR_syscalls)) {
> if (unlikely(trap_is_unsupported_scv(regs))) {
> /* Unsupported scv vector */
> --
> 2.55.0
>
^ permalink raw reply
* Re: [RFC PATCH 0/6] selftests/vfio: Add sPAPR TCE v2 coverage
From: Harsh Prateek Bora @ 2026-07-03 8:28 UTC (permalink / raw)
To: Narayana Murty N, alex, dmatlack, shuah, linuxppc-dev
Cc: amastro, rananta, kvm, linux-kselftest, linux-kernel, vaibhav,
sbhat
In-Reply-To: <20260703032806.40946-1-nnmlinux@linux.ibm.com>
+ linuxppc-dev
On 03/07/26 8:58 am, Narayana Murty N wrote:
> This RFC adds PowerPC sPAPR TCE v2 support to VFIO selftests.
>
> The series adds a selftest IOMMU mode for the legacy VFIO container
> backend, adds helpers for default and dynamic DMA windows, and exercises
> the sPAPR DDW path through the DMA mapping tests.
>
> sPAPR TCE v2 remains a legacy VFIO container backend. This series does
> not add iommufd or VFIO cdev support for sPAPR.
>
> The main points are
> 1. add VFIO_SELFTESTS_IOMMU_MODE for selecting a backend
> 2. add vfio_spapr_tce_v2_iommu as the PowerPC VFIO backend
> 3. preserve the platform default DMA window
> 4. create and tear down only selftest-created DDWs
> 5. prepare the DMA window before IOVA allocation
> 6. exercise DDW creation for hugepage DMA mappings
> 7. accept sPAPR-specific errno for invalid DMA ranges
>
> Example:
>
> VFIO_SELFTESTS_IOMMU_MODE=vfio_spapr_tce_v2_iommu
> ./vfio_dma_mapping_test <BDF>
>
> Observed coverage includes anonymous mappings through the default window
> and 2MB hugetlb mappings through a DDW. 1GB hugetlb mappings may skip when
> the platform rejects the requested DDW characteristics.
>
> Feedback is requested from the VFIO and PowerPC communities on:
> 1. whether the sPAPR TCE v2 helpers should remain in the common VFIO
> selftest library or move into a sPAPR-specific test helper?
> 2. whether DDW setup through hugepage mappings is the right initial
> coverage?
> 3. whether the sPAPR-specific overflow errno expectation should be
> accepted, skipped, or hidden behind a backend helper?
> 4. whether PowerPC should generate only the sPAPR TCE v2 fixture variants,
> or keep the generic IOMMU-mode matrix and skip unsupported modes.
>
> TODO: vfio_spapr_tce_v1_iommu yet to be covered.
>
> Narayana Murty N (6):
> selftests/vfio: allow selecting IOMMU backend from environment
> selftests/vfio: add sPAPR TCE v2 IOMMU mode
> selftests/vfio: add sPAPR TCE v2 DMA window helpers
> selftests/vfio: Exercise sPAPR DDW path for hugepage DMA mappings
> selftests/vfio: Accept sPAPR errno for DMA range overflow
> selftests/vfio: Enable VFIO selftests on ppc64 and ppc64le
>
> .../selftests/vfio/lib/include/libvfio.h | 9 +
> .../vfio/lib/include/libvfio/iommu.h | 26 ++
> tools/testing/selftests/vfio/lib/iommu.c | 280 +++++++++++++++++-
> tools/testing/selftests/vfio/lib/libvfio.c | 37 +++
> .../selftests/vfio/vfio_dma_mapping_test.c | 30 ++
> .../selftests/vfio/vfio_pci_device_test.c | 4 +-
> tools/testing/selftests/vfio/Makefile | 2 +-
> 7 files changed, 382 insertions(+), 6 deletions(-)
>
^ permalink raw reply
* [PATCH V3] powerpc/syscall: Fix syscall skip handling for seccomp and ptrace
From: Mukesh Kumar Chaurasiya (IBM) @ 2026-07-03 8:11 UTC (permalink / raw)
To: oleg, maddy, mpe, npiggin, chleroy, sshegde, mkchauras, mchauras,
thuth, segher, ryan.roberts, ruanjinjie, linuxppc-dev,
linux-kernel
Cc: Michal Suchánek
After enabling GENERIC_ENTRY on PowerPC, syscall_enter_from_user_mode()
returns -1 as a sentinel to signal that seccomp or ptrace has intercepted
the syscall and already set a return value via syscall_set_return_value().
system_call_exception() was not handling this sentinel, and since -1UL
is >= NR_syscalls, the code fell into the out-of-range path and returned
-ENOSYS, overwriting the errno already placed in regs->gpr[3].
The naive fix of checking r0 == -1L before the NR_syscalls bounds check
is ambiguous: a user legitimately calling syscall(-1) also produces r0 ==
-1L, and a tracer intercepting such a call would have its injected return
value silently discarded.
Fix this properly by introducing regs->entry_flags, a kernel-internal
field in struct pt_regs (consuming one slot of the existing __pt_regs_pad
so the ABI is preserved), with SYSCALL_ENTRY_RET_SET as an out-of-band
flag. syscall_set_return_value() sets this flag whenever seccomp or ptrace
injects a return value. system_call_exception() zeros entry_flags before
calling syscall_enter_from_user_mode(), then checks and clears the flag
afterwards: if set, it returns regs->gpr[3] directly regardless of what
syscall number the user originally requested.
This handles all seccomp actions correctly:
- SECCOMP_RET_ERRNO, SECCOMP_RET_TRACE (no tracer), SECCOMP_RET_USER_NOTIF:
all call syscall_set_return_value(), flag is set, injected value returned.
- SECCOMP_RET_TRAP, SECCOMP_RET_KILL: call syscall_rollback() and deliver
a signal; flag is not set, but the process is dying so the return value
is irrelevant.
The fix covers both ppc32 and ppc64 with no #ifdefs.
Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
Reported-by: Michal Suchánek <msuchanek@suse.de>
Closes: https://lore.kernel.org/all/ajpp-_XnbF3UTM_E@kunlun.suse.cz/
Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
---
v2 -> v3:
- Last fix is not working for -1 syscall. Fixed that with this.
v2: https://lore.kernel.org/all/20260629182946.419552-1-mkchauras@gmail.com
v1 -> v2:
- Fix issues in the previous fix (Michal)
v1: https://lore.kernel.org/all/20260624171520.772408-1-mkchauras@gmail.com
arch/powerpc/include/asm/ptrace.h | 22 +++++++++++++++++++++-
arch/powerpc/include/asm/syscall.h | 6 ++++++
arch/powerpc/include/uapi/asm/ptrace.h | 6 ++++--
arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
arch/powerpc/kernel/syscall.c | 18 ++++++++++++++++++
5 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index fdeb97421785..1a53d5cfa8db 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -54,8 +54,9 @@ struct pt_regs
};
unsigned long result;
unsigned long exit_flags;
+ unsigned long entry_flags;
/* Maintain 16 byte interrupt stack alignment */
- unsigned long __pt_regs_pad[3];
+ unsigned long __pt_regs_pad[2];
};
};
#if defined(CONFIG_PPC64) || defined(CONFIG_PPC_KUAP)
@@ -233,6 +234,25 @@ static inline unsigned long frame_pointer(struct pt_regs *regs)
#define current_pt_regs() \
((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
+/*
+ * SYSCALL_ENTRY_RET_SET: seccomp or ptrace called syscall_set_return_value()
+ * and wants the syscall skipped; regs->gpr[3] already holds the return value.
+ */
+#define SYSCALL_ENTRY_RET_SET BIT(0)
+
+static inline void set_syscall_entry_ret(struct pt_regs *regs)
+{
+ regs->entry_flags |= SYSCALL_ENTRY_RET_SET;
+}
+
+static inline bool test_and_clear_syscall_entry_ret(struct pt_regs *regs)
+{
+ bool set = !!(regs->entry_flags & SYSCALL_ENTRY_RET_SET);
+
+ regs->entry_flags &= ~SYSCALL_ENTRY_RET_SET;
+ return set;
+}
+
/*
* The 4 low bits (0xf) are available as flags to overload the trap word,
* because interrupt vectors have minimum alignment of 0x10. TRAP_FLAGS_MASK
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 834fcc4f7b54..9ae79326abe3 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -98,6 +98,12 @@ static inline void syscall_set_return_value(struct task_struct *task,
regs->gpr[3] = val;
}
}
+ /*
+ * Mark that a return value has been explicitly set by seccomp or
+ * ptrace so that system_call_exception() can skip the syscall
+ * unconditionally, even when the user requested syscall(-1).
+ */
+ set_syscall_entry_ret(regs);
}
static inline void syscall_get_arguments(struct task_struct *task,
diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
index a393b7f2760a..2f2a43414fe6 100644
--- a/arch/powerpc/include/uapi/asm/ptrace.h
+++ b/arch/powerpc/include/uapi/asm/ptrace.h
@@ -56,7 +56,8 @@ struct pt_regs
unsigned long dsisr; /* on 4xx/Book-E used for ESR */
unsigned long result; /* Result of a system call */
unsigned long exit_flags; /* System call exit flags */
- unsigned long __pt_regs_pad[3]; /* Maintain 16 byte interrupt stack alignment */
+ unsigned long entry_flags; /* System call entry flags */
+ unsigned long __pt_regs_pad[2]; /* Maintain 16 byte interrupt stack alignment */
};
#endif /* __ASSEMBLER__ */
@@ -117,7 +118,8 @@ struct pt_regs
#define PT_DSISR 42
#define PT_RESULT 43
#define PT_EXIT_FLAGS 44
-#define PT_PAD 47 /* 3 times */
+#define PT_ENTRY_FLAGS 45
+#define PT_PAD 46 /* 2 times */
#define PT_DSCR 48
#define PT_REGS_COUNT 48
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index 316d4f5ead8e..440d00690cf2 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -235,6 +235,8 @@ void __init pt_regs_check(void)
offsetof(struct user_pt_regs, dsisr));
BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
offsetof(struct user_pt_regs, result));
+ BUILD_BUG_ON(offsetof(struct pt_regs, entry_flags) !=
+ offsetof(struct user_pt_regs, entry_flags));
BUILD_BUG_ON(sizeof(struct user_pt_regs) > sizeof(struct pt_regs));
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index a9da2af6efa8..c7458aae199d 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -19,9 +19,27 @@ notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
long ret;
syscall_fn f;
+ /*
+ * Zero entry_flags before syscall_enter_from_user_mode() so that
+ * syscall_set_return_value() can set SYSCALL_ENTRY_RET_SET as an
+ * unambiguous out-of-band signal. The field is not initialised by
+ * the entry assembly.
+ */
+ regs->entry_flags = 0;
add_random_kstack_offset();
r0 = syscall_enter_from_user_mode(regs, r0);
+ /*
+ * Seccomp or ptrace may have set a return value and requested that
+ * the syscall be skipped. syscall_set_return_value() sets
+ * SYSCALL_ENTRY_RET_SET in regs->entry_flags as an
+ * unambiguous out-of-band signal. This avoids the ambiguity of
+ * using r0 == -1 as the skip sentinel when the user themselves
+ * called syscall(-1).
+ */
+ if (unlikely(test_and_clear_syscall_entry_ret(regs)))
+ return regs->gpr[3];
+
if (unlikely(r0 >= NR_syscalls)) {
if (unlikely(trap_is_unsupported_scv(regs))) {
/* Unsupported scv vector */
--
2.55.0
^ permalink raw reply related
* [PATCH 1/1] powerpc/eeh: Prevent EEH false positives on PMCSR reads in D3cold
From: Narayana Murty N @ 2026-07-03 3:36 UTC (permalink / raw)
To: mahesh, mpe, maddy
Cc: oohall, npiggin, chleroy, nnmlinux, linuxppc-dev, linux-kernel,
vaibhav, sbhat, harshpb
On pseries systems, RTAS-based PCI config space reads can trigger
false EEH events when attempting to read the Power Management Control
Status Register (PMCSR) while a device is in D3cold state. This occurs
because the device is powered off and cannot respond to config space
accesses, causing the platform to report an error condition.
This patch addresses the issue by:
1. Caching PM capability information in struct eeh_dev:
- pm_cap: Offset of the PM capability structure
- pmcsr_offset: Absolute config space offset of PMCSR register
2. Synchronizing PM capability data between pci_dev and eeh_dev:
- During device probe (eeh_probe_device), copy pm_cap from pci_dev
- During early init (pseries_eeh_init_edev), discover pm_cap via
firmware before pci_dev exists
3. Intercepting PMCSR reads in D3cold state:
- In rtas_pci_dn_write_config, detect PMCSR read attempts
- Check if device is in D3cold via pdev->current_state
- Return synthetic success value instead of performing RTAS call
- Prevents hardware access that would trigger false EEH event
The fix handles both early boot (before pci_dev exists) and runtime
scenarios, ensuring PM capability information is always available when
needed. By blocking RTAS reads to PMCSR when devices are in D3cold,
we prevent spurious EEH events while maintaining proper error detection
for genuine hardware failures.
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 9 +++++++
arch/powerpc/kernel/eeh.c | 16 ++++++++++++
arch/powerpc/kernel/rtas_pci.c | 26 ++++++++++++++++++++
arch/powerpc/platforms/pseries/eeh_pseries.c | 16 ++++++++++++
4 files changed, 67 insertions(+)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index b7ebb4ac2c71..224a3adcd34e 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -139,6 +139,15 @@ struct eeh_dev {
int pcie_cap; /* Saved PCIe capability */
int aer_cap; /* Saved AER capability */
int af_cap; /* Saved AF capability */
+ /*
+ * Cached PCI PM capability information.
+ * pm_cap == 0 means the device does not have PCI PM capability
+ * or it has not been discovered yet.
+ * pmcsr_offset is the absolute config-space offset of PMCSR:
+ * pm_cap + PCI_PM_CTRL
+ */
+ u8 pm_cap;
+ u16 pmcsr_offset;
struct eeh_pe *pe; /* Associated PE */
struct list_head entry; /* Membership in eeh_pe.edevs */
struct list_head rmv_entry; /* Membership in rmv_list */
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index bb836f02101c..4402166df8c0 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -997,6 +997,21 @@ int eeh_init(struct eeh_ops *ops)
return eeh_event_init();
}
+#ifdef CONFIG_EEH
+static void eeh_sync_pm_cap(struct eeh_dev *edev, struct pci_dev *pdev)
+{
+ if (!edev || !pdev)
+ return;
+
+ /*
+ * Prefer PCI core cached PM capability once pci_dev exists.
+ * If pm_cap is zero, clear pmcsr_offset as well.
+ */
+ edev->pm_cap = pdev->pm_cap;
+ edev->pmcsr_offset = pdev->pm_cap ? pdev->pm_cap + PCI_PM_CTRL : 0;
+}
+#endif
+
/**
* eeh_probe_device() - Perform EEH initialization for the indicated pci device
* @dev: pci device for which to set up EEH
@@ -1050,6 +1065,7 @@ void eeh_probe_device(struct pci_dev *dev)
/* bind the pdev and the edev together */
edev->pdev = dev;
dev->dev.archdata.edev = edev;
+ eeh_sync_pm_cap(edev, dev);
eeh_addr_cache_insert_dev(dev);
eeh_sysfs_add_device(dev);
}
diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index fccf96e897f6..9668cd0411e1 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -95,6 +95,24 @@ static int rtas_pci_read_config(struct pci_bus *bus,
return ret;
}
+static bool eeh_handle_pmcsr_read(struct eeh_dev *edev, int size,
+ u32 *val, int *pcibios_ret)
+{
+ struct pci_dev *pdev;
+
+ if (!edev)
+ return false;
+
+ pdev = edev->pdev;
+ if (!pdev || pdev->current_state != PCI_D3cold)
+ return false;
+
+ *val = EEH_IO_ERROR_VALUE(size);
+ *pcibios_ret = PCIBIOS_SUCCESSFUL;
+
+ return true;
+}
+
int rtas_pci_dn_write_config(struct pci_dn *pdn, int where, int size, u32 val)
{
unsigned long buid, addr;
@@ -108,6 +126,14 @@ int rtas_pci_dn_write_config(struct pci_dn *pdn, int where, int size, u32 val)
if (pdn->edev && pdn->edev->pe &&
(pdn->edev->pe->state & EEH_PE_CFG_BLOCKED))
return PCIBIOS_SET_FAILED;
+
+ if (unlikely(pdn->edev && pdn->edev->pmcsr_offset &&
+ size == 2 && where == pdn->edev->pmcsr_offset)) {
+ int pcibios_ret;
+
+ if (eeh_handle_pmcsr_read(pdn->edev, size, &val, &pcibios_ret))
+ return pcibios_ret;
+ }
#endif
addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index b12ef382fec7..0e7efb4bf2d4 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -351,6 +351,21 @@ static struct eeh_pe *pseries_eeh_pe_get_parent(struct eeh_dev *edev)
return NULL;
}
+static void pseries_eeh_init_pm_cap(struct pci_dn *pdn, struct eeh_dev *edev)
+{
+ edev->pm_cap = 0;
+ edev->pmcsr_offset = 0;
+
+ if (!pdn || !edev)
+ return;
+
+ edev->pm_cap = pseries_eeh_find_cap(pdn, PCI_CAP_ID_PM);
+ if (!edev->pm_cap)
+ return;
+
+ edev->pmcsr_offset = edev->pm_cap + PCI_PM_CTRL;
+}
+
/**
* pseries_eeh_init_edev - initialise the eeh_dev and eeh_pe for a pci_dn
*
@@ -408,6 +423,7 @@ static void pseries_eeh_init_edev(struct pci_dn *pdn)
edev->pcix_cap = pseries_eeh_find_cap(pdn, PCI_CAP_ID_PCIX);
edev->pcie_cap = pseries_eeh_find_cap(pdn, PCI_CAP_ID_EXP);
edev->aer_cap = pseries_eeh_find_ecap(pdn, PCI_EXT_CAP_ID_ERR);
+ pseries_eeh_init_pm_cap(pdn, edev);
edev->mode &= 0xFFFFFF00;
if ((pdn->class_code >> 8) == PCI_CLASS_BRIDGE_PCI) {
edev->mode |= EEH_DEV_BRIDGE;
--
2.51.1
^ permalink raw reply related
* Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR
From: Michal Suchánek @ 2026-07-03 7:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, Jonathan Corbet, Shuah Khan, Huacai Chen,
WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Andy Lutomirski, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Andrew Donnellan, Mark Rutland,
Arnd Bergmann, Jiaxun Yang, Ryan Roberts, Greg Kroah-Hartman,
Mukesh Kumar Chaurasiya, Shrikanth Hegde, Zong Li, Nam Cao,
Deepak Gupta, Lukas Gerlach, Rui Qi, Kees Cook, linux-doc,
linux-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390
In-Reply-To: <87jyrdnmrh.ffs@fw13>
On Thu, Jul 02, 2026 at 10:45:54PM +0200, Thomas Gleixner wrote:
> On Thu, Jul 02 2026 at 13:45, Michal Suchánek wrote:
> > On Thu, Jul 02, 2026 at 01:24:57PM +0200, Thomas Gleixner wrote:
> >> On Wed, Jul 01 2026 at 19:42, Michal Suchánek wrote:
> >> > The return value of syscall_enter_from_user_mode is used both for the
> >> > adjusted syscall number and the indicator that a syscall should be
> >> > skipped.
> >> >
> >> > As seccomp can be invoked on any syscall, including invalid ones this
> >> > somewhat undermines seccomp.
> >> >
> >> > While the seccomp variants that terminate the process do not need to
> >> > care about this for the filter that sets the syscall return value this
> >> > disctinction is required.
> >>
> >> You completely fail to explain why and what actual problem you are
> >> trying to solve. At least I can't figure it out from the above word
> >> salad.
> >
> > syscall_enter_from_user_mode returns the new syscall number after doing
> > something arbitrarry with it, including running seccomp.
> >
> > Wehn the syscall is already handled, eg. by seccomp filtering it returns
> > -1 as the new syscall number. -1 is an invalid syscall number but it can
> > still be filtered by seccomp.
>
> Once syscall_enter_from_user_mode() returns -1 nothing can filter it
> anymore.
>
> > When the syscall number was -1 to start with it's not possible to
> > determine if the syscall was fileterd from the return value. s390
> > returns the filtered state in a flag it sets on the regs structure,
> > avoiding this problem.
>
> What needs to determine whether the syscall was filtered or not?
The code that executes syscall_enter_from_user_mode() needs to determine
that.
After syscall_enter_from_user_mode() returns the syscall needs to be
executed or skipped.
'Executing' an invalid syscall boils down to setting the return value to
-ENOSYS.
But if the syscall number returned is -1 was the syscall filtered and
the return value set by syscall_enter_from_user_mode() or should it be
set by the caller to -ENOSYS?
>
> > However, the API should be specified in a way that does not require
> > everyone implementing such flag.
>
> Which exact problem does the flag solve?
To be able to tell if the syscall was handled or no, the return value
from syscall_enter_from_user_mode() is inconclusive.
That's what the flag is for. To be able to tell if the syscall was
handled without relying on the ambiguous return value of
syscall_enter_from_user_mode().
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v16 01/18] seccomp: Convert __secure_computing() to return boolean
From: Michal Suchánek @ 2026-07-03 7:51 UTC (permalink / raw)
To: Jinjie Ruan
Cc: oleg, richard.henderson, mattst88, linmag7, linux,
catalin.marinas, will, kees, guoren, chenhuacai, kernel, geert,
tsbogend, James.Bottomley, deller, maddy, mpe, npiggin, chleroy,
pjw, palmer, aou, alex, hca, gor, agordeev, borntraeger, svens,
ysato, dalias, glaubitz, richard, anton.ivanov, johannes, luto,
tglx, mingo, bp, dave.hansen, hpa, chris, jcmvbkbc, peterz, wad,
thuth, mark.rutland, ada.coupriediaz, kevin.brodsky, linusw,
yeoreum.yun, song, james.morse, anshuman.khandual, broonie,
liqiang01, pengcan, ryan.roberts, yangtiezhu, sshegde, mchauras,
austin.kim, jchrist, arnd, thomas.weissschuh, sohil.mehta,
andrew.cooper3, jgross, kas, x86, linux-alpha, linux-kernel,
linux-arm-kernel, linux-mm, linux-csky, loongarch, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-riscv, linux-s390,
linux-sh, linux-um
In-Reply-To: <20260629130616.642022-2-ruanjinjie@huawei.com>
On Mon, Jun 29, 2026 at 09:05:59PM +0800, Jinjie Ruan wrote:
> The return value of __secure_computing() currently uses 0 to indicate
> that a system call should be allowed, and -1 to indicate that it should
> be blocked/killed. This 0/-1 pattern is non-intuitive for a security
> check function and makes the control flow at the call sites less readable.
>
> Furthermore, any potential future changes to these return values would
> require a high-risk, error-prone audit of all its users across different
> architectures.
>
> Sanitize this logic by converting the return type of __secure_computing()
> to a proper boolean, where 'true' explicitly means 'allow' and 'false'
> means 'fail/deny'.
>
> Update all the two dozen or so call sites across the tree to align with
> this new boolean semantic. No functional changes are intended, as the
> callers still return -1 to the lower-level assembly entry code upon
> seccomp denial.
>
> Suggested-by: Thomas Gleixner <tglx@kernel.org>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> arch/alpha/kernel/ptrace.c | 2 +-
> arch/arm/kernel/ptrace.c | 2 +-
> arch/arm64/kernel/ptrace.c | 2 +-
> arch/csky/kernel/ptrace.c | 2 +-
> arch/m68k/kernel/ptrace.c | 2 +-
> arch/mips/kernel/ptrace.c | 2 +-
> arch/parisc/kernel/ptrace.c | 2 +-
> arch/sh/kernel/ptrace_32.c | 2 +-
> arch/um/kernel/skas/syscall.c | 2 +-
> arch/x86/entry/vsyscall/vsyscall_64.c | 2 +-
> arch/xtensa/kernel/ptrace.c | 3 +--
> include/linux/entry-common.h | 7 +++---
> include/linux/seccomp.h | 10 ++++----
> kernel/seccomp.c | 34 +++++++++++++--------------
> 14 files changed, 36 insertions(+), 38 deletions(-)
>
> diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
> index 0687760ea466..27d9847b1082 100644
> --- a/arch/alpha/kernel/ptrace.c
> +++ b/arch/alpha/kernel/ptrace.c
> @@ -387,7 +387,7 @@ asmlinkage unsigned long syscall_trace_enter(void)
> * If this fails, seccomp may already have set up the return value
> * (e.g. SECCOMP_RET_ERRNO / TRACE).
> */
> - if (secure_computing() == -1) {
> + if (!secure_computing()) {
> if (regs->r19 == 0 && regs->r0 == (unsigned long)-1)
> syscall_set_return_value(current, regs, -ENOSYS, 0);
> syscall_set_nr(current, regs, -1);
> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> index 7951b2c06fec..5210745725ca 100644
> --- a/arch/arm/kernel/ptrace.c
> +++ b/arch/arm/kernel/ptrace.c
> @@ -855,7 +855,7 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
>
> /* Do seccomp after ptrace; syscall may have changed. */
> #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
> - if (secure_computing() == -1)
> + if (!secure_computing())
> return -1;
> #else
> /* XXX: remove this once OABI gets fixed */
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 4d08598e2891..2ca6fab39a37 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -2420,7 +2420,7 @@ int syscall_trace_enter(struct pt_regs *regs)
> }
>
> /* Do the secure computing after ptrace; failures should be fast. */
> - if (secure_computing() == -1)
> + if (!secure_computing())
> return NO_SYSCALL;
>
> if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> diff --git a/arch/csky/kernel/ptrace.c b/arch/csky/kernel/ptrace.c
> index 6bb685a2646b..11c5eff41e9d 100644
> --- a/arch/csky/kernel/ptrace.c
> +++ b/arch/csky/kernel/ptrace.c
> @@ -323,7 +323,7 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> if (ptrace_report_syscall_entry(regs))
> return -1;
>
> - if (secure_computing() == -1)
> + if (!secure_computing())
> return -1;
>
> if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
> index cfa2df24eced..d2411404b9df 100644
> --- a/arch/m68k/kernel/ptrace.c
> +++ b/arch/m68k/kernel/ptrace.c
> @@ -281,7 +281,7 @@ asmlinkage int syscall_trace_enter(void)
> if (test_thread_flag(TIF_SYSCALL_TRACE))
> ret = ptrace_report_syscall_entry(task_pt_regs(current));
>
> - if (secure_computing() == -1)
> + if (!secure_computing())
> return -1;
>
> return ret;
> diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
> index 3f4c94c88124..0d809cda7542 100644
> --- a/arch/mips/kernel/ptrace.c
> +++ b/arch/mips/kernel/ptrace.c
> @@ -1328,7 +1328,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs)
> return -1;
> }
>
> - if (secure_computing())
> + if (!secure_computing())
> return -1;
Hello,
I am not fond of this logic inversion. The boolean is meaningless in
itself.
Previously -1 was used to indicate that the syscall was filtered but you
chose to invert the logic choosing true to mean syscall was not filtered.
You could choose true to mean that syscall was fitered avoiding this
inversion.
Sashiko points out some places in existing code where it supposedly
explodes which might or might not be true but any in-flight patches that
use secure_computing would also be affected.
Also please document what the value actually means. I do not see that
added as part of these series. As either interpretation of the boolean
value is equally valid it needs to be documented which one was chosen.
While there is a code comment in one of the functions that
secure_computing() calls I could not find any documentation for the
return value of secure_computing.
This could add a kernel-doc comment while touching the code anyway
https://www.kernel.org/doc/html/v7.2-rc1/doc-guide/kernel-doc.html
Thanks
Michal
>
> if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c
> index 8a17ab7e6e0b..565b51a48c8a 100644
> --- a/arch/parisc/kernel/ptrace.c
> +++ b/arch/parisc/kernel/ptrace.c
> @@ -351,7 +351,7 @@ long do_syscall_trace_enter(struct pt_regs *regs)
> }
>
> /* Do the secure computing check after ptrace. */
> - if (secure_computing() == -1)
> + if (!secure_computing())
> return -1;
>
> #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
> diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c
> index 06f765d71a29..8687f17cbe5a 100644
> --- a/arch/sh/kernel/ptrace_32.c
> +++ b/arch/sh/kernel/ptrace_32.c
> @@ -460,7 +460,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
> return -1;
> }
>
> - if (secure_computing() == -1)
> + if (!secure_computing())
> return -1;
>
> if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
> index ba7494f9bfe4..916cd7acceaf 100644
> --- a/arch/um/kernel/skas/syscall.c
> +++ b/arch/um/kernel/skas/syscall.c
> @@ -27,7 +27,7 @@ void handle_syscall(struct uml_pt_regs *r)
> goto out;
>
> /* Do the seccomp check after ptrace; failures should be fast. */
> - if (secure_computing() == -1)
> + if (!secure_computing())
> goto out;
>
> syscall = UPT_SYSCALL_NR(r);
> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
> index ea36de9fa864..6aed3987b9f9 100644
> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> @@ -198,7 +198,7 @@ static bool __emulate_vsyscall(struct pt_regs *regs, unsigned long address)
> regs->orig_ax = syscall_nr;
> regs->ax = -ENOSYS;
> tmp = secure_computing();
> - if ((!tmp && regs->orig_ax != syscall_nr) || regs->ip != address) {
> + if ((tmp && regs->orig_ax != syscall_nr) || regs->ip != address) {
> warn_bad_vsyscall(KERN_DEBUG, regs,
> "seccomp tried to change syscall nr or ip");
> force_exit_sig(SIGSYS);
> diff --git a/arch/xtensa/kernel/ptrace.c b/arch/xtensa/kernel/ptrace.c
> index b80d54b2ea34..ef78fcd318ff 100644
> --- a/arch/xtensa/kernel/ptrace.c
> +++ b/arch/xtensa/kernel/ptrace.c
> @@ -553,8 +553,7 @@ int do_syscall_trace_enter(struct pt_regs *regs)
> return 0;
> }
>
> - if (regs->syscall == NO_SYSCALL ||
> - secure_computing() == -1) {
> + if (regs->syscall == NO_SYSCALL || !secure_computing()) {
> do_syscall_trace_leave(regs);
> return 0;
> }
> diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
> index 416a3352261f..3f66320e46d3 100644
> --- a/include/linux/entry-common.h
> +++ b/include/linux/entry-common.h
> @@ -100,9 +100,8 @@ static __always_inline long syscall_trace_enter(struct pt_regs *regs, unsigned l
>
> /* Do seccomp after ptrace, to catch any tracer changes. */
> if (work & SYSCALL_WORK_SECCOMP) {
> - ret = __secure_computing();
> - if (ret == -1L)
> - return ret;
> + if (!__secure_computing())
> + return -1L;
> }
>
> /* Either of the above might have changed the syscall number */
> @@ -113,7 +112,7 @@ static __always_inline long syscall_trace_enter(struct pt_regs *regs, unsigned l
>
> syscall_enter_audit(regs, syscall);
>
> - return ret ? : syscall;
> + return syscall;
> }
>
> /**
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index 9b959972bf4a..7af3173f40e9 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -22,14 +22,14 @@
> #include <linux/atomic.h>
> #include <asm/seccomp.h>
>
> -extern int __secure_computing(void);
> +extern bool __secure_computing(void);
>
> #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
> -static inline int secure_computing(void)
> +static inline bool secure_computing(void)
> {
> if (unlikely(test_syscall_work(SECCOMP)))
> return __secure_computing();
> - return 0;
> + return true;
> }
> #else
> extern void secure_computing_strict(int this_syscall);
> @@ -50,11 +50,11 @@ static inline int seccomp_mode(struct seccomp *s)
> struct seccomp_data;
>
> #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
> -static inline int secure_computing(void) { return 0; }
> +static inline bool secure_computing(void) { return true; }
> #else
> static inline void secure_computing_strict(int this_syscall) { return; }
> #endif
> -static inline int __secure_computing(void) { return 0; }
> +static inline bool __secure_computing(void) { return true; }
>
> static inline long prctl_get_seccomp(void)
> {
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 066909393c38..1fec6efedab6 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -1100,12 +1100,12 @@ void secure_computing_strict(int this_syscall)
> else
> BUG();
> }
> -int __secure_computing(void)
> +bool __secure_computing(void)
> {
> int this_syscall = syscall_get_nr(current, current_pt_regs());
>
> secure_computing_strict(this_syscall);
> - return 0;
> + return true;
> }
> #else
>
> @@ -1256,7 +1256,7 @@ static int seccomp_do_user_notification(int this_syscall,
> return -1;
> }
>
> -static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> +static bool __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> {
> u32 filter_ret, action;
> struct seccomp_data sd;
> @@ -1294,7 +1294,7 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> case SECCOMP_RET_TRACE:
> /* We've been put in this state by the ptracer already. */
> if (recheck_after_trace)
> - return 0;
> + return true;
>
> /* ENOSYS these calls if there is no tracer attached. */
> if (!ptrace_event_enabled(current, PTRACE_EVENT_SECCOMP)) {
> @@ -1330,19 +1330,19 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> * a skip would have already been reported.
> */
> if (__seccomp_filter(this_syscall, true))
> - return -1;
> + return false;
>
> - return 0;
> + return true;
>
> case SECCOMP_RET_USER_NOTIF:
> if (seccomp_do_user_notification(this_syscall, match, &sd))
> goto skip;
>
> - return 0;
> + return true;
>
> case SECCOMP_RET_LOG:
> seccomp_log(this_syscall, 0, action, true);
> - return 0;
> + return true;
>
> case SECCOMP_RET_ALLOW:
> /*
> @@ -1350,7 +1350,7 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> * this action since SECCOMP_RET_ALLOW is the starting
> * state in seccomp_run_filters().
> */
> - return 0;
> + return true;
>
> case SECCOMP_RET_KILL_THREAD:
> case SECCOMP_RET_KILL_PROCESS:
> @@ -1367,46 +1367,46 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> } else {
> do_exit(SIGSYS);
> }
> - return -1; /* skip the syscall go directly to signal handling */
> + return false; /* skip the syscall go directly to signal handling */
> }
>
> unreachable();
>
> skip:
> seccomp_log(this_syscall, 0, action, match ? match->log : false);
> - return -1;
> + return false;
> }
> #else
> -static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> +static bool __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> {
> BUG();
>
> - return -1;
> + return false;
> }
> #endif
>
> -int __secure_computing(void)
> +bool __secure_computing(void)
> {
> int mode = current->seccomp.mode;
> int this_syscall;
>
> if (IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) &&
> unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> - return 0;
> + return true;
>
> this_syscall = syscall_get_nr(current, current_pt_regs());
>
> switch (mode) {
> case SECCOMP_MODE_STRICT:
> __secure_computing_strict(this_syscall); /* may call do_exit */
> - return 0;
> + return true;
> case SECCOMP_MODE_FILTER:
> return __seccomp_filter(this_syscall, false);
> /* Surviving SECCOMP_RET_KILL_* must be proactively impossible. */
> case SECCOMP_MODE_DEAD:
> WARN_ON_ONCE(1);
> do_exit(SIGKILL);
> - return -1;
> + return false;
> default:
> BUG();
> }
> --
> 2.34.1
>
^ permalink raw reply
* [PATCH] powerpc/ptrace: Add offsetof check for exit_flags in pt_regs_check()
From: Mukesh Kumar Chaurasiya (IBM) @ 2026-07-03 7:49 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, mkchauras, sshegde, mkchauras,
linuxppc-dev, linux-kernel
pt_regs_check() verifies that fields shared between struct pt_regs and
struct user_pt_regs sit at the same offset, to catch any accidental
layout divergence between the kernel and uapi structures.
Add the missing check for exit_flags, following the same pattern as the
existing checks for result, dsisr, dar and the other shared fields.
Fixes: d7a6797e0bc1 ("powerpc: add exit_flags field in pt_regs")
Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
---
arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index 316d4f5ead8e..fb3fec73f66a 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -235,6 +235,8 @@ void __init pt_regs_check(void)
offsetof(struct user_pt_regs, dsisr));
BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
offsetof(struct user_pt_regs, result));
+ BUILD_BUG_ON(offsetof(struct pt_regs, exit_flags) !=
+ offsetof(struct user_pt_regs, exit_flags));
BUILD_BUG_ON(sizeof(struct user_pt_regs) > sizeof(struct pt_regs));
--
2.55.0
^ permalink raw reply related
* Re: [kvm-unit-tests RFC PATCH 2/6] configure: Make arch_libdir a first-class entity
From: Chinmay Rath @ 2026-07-03 6:46 UTC (permalink / raw)
To: Thomas Huth
Cc: npiggin, harshpb, lvivier, linuxppc-dev, kvm, andrew.jones, sbhat
In-Reply-To: <7d9e6365-1b2d-4b18-8fb5-d8048fddb7f0@redhat.com>
On 6/10/26 17:00, Thomas Huth wrote:
> On 02/06/2026 08.48, Chinmay Rath wrote:
>> From: Nicholas Piggin <npiggin@gmail.com>
>>
>> arch_libdir was brought in to improve the heuristic determination of
>> the lib/ directory based on arch and testdir names, but it did not
>> entirely clean that mess up.
>>
>> Remove the arch_libdir->arch->testdir heuristic and just require
>> everybody sets arch_libdir correctly. Fail if the lib/arch or
>> lib/arch/asm directories can not be found.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
>> ---
>> Makefile | 2 +-
>> configure | 20 ++++++++++++--------
>> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> FYI, this patch seems to trigger yet another issue with Clang (I used
> v21):
>
> clang -no-integrated-as -std=gnu99 -ffreestanding -O2 -msoft-float
> -mno-altivec -I /root/kvm-unit-tests/lib -I
> /root/kvm-unit-tests/lib/libfdt -I lib -Wa,-mregnames -fverbose-asm -S
> -o lib/powerpc/asm-offsets.s lib/powerpc/asm-offsets.c
> lib/powerpc/asm-offsets.c:10:10: error: 'asm/smp.h' file not found
> with <angled> include; use
> "quotes" instead
> 10 | #include <asm/smp.h>
> | ^~~~~~~~~~~
> | "asm/smp.h"
> In file included from lib/powerpc/asm-offsets.c:10:
> lib/powerpc/asm/smp.h:5:10: fatal error: 'asm/processor.h' file not found
> 5 | #include <asm/processor.h>
> | ^~~~~~~~~~~~~~~~~
> 2 errors generated.
>
> Could you please have a look?
Hi Thomas, back from traveling and looked into this.
I am using Clang v21 as well :
# clang --version
clang version 21.1.8 (Fedora 21.1.8-4.fc43)
Target: ppc64le-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Configuration file: /etc/clang/ppc64le-redhat-linux-gnu-clang.cfg
I see the above file, asm-offsets.s being generated properly without any
error during the make process (used the extra flags for make that I have
mentioned in my other reply).
Snippet from make :
cat powerpc/boot_rom.bin.tmp >> powerpc/boot_rom.bin
rm -f powerpc/boot_rom.bin.tmp
clang -no-integrated-as -Wa,-mpower10 -std=gnu99 -ffreestanding -O2
-msoft-float -mno-altivec -I /root/chins/git/kvm-unit-tests/lib -I
/root/chins/git/kvm-unit-tests/lib/libfdt -I lib -Wa,-mregnames
-fverbose-asm -S -o lib/powerpc/asm-offsets.s
/root/chins/git/kvm-unit-tests/lib/powerpc/asm-offsets.c
(set -e; echo "#ifndef __ASM_OFFSETS_H__"; echo "#define
__ASM_OFFSETS_H__"; echo "/*"; echo " * Generated file. DO NOT MODIFY.";
echo " *"; echo " */"; echo ""; sed -ne
's:^[[:space:]]*\.ascii[[:space:]]*"\(.*\)".*:\1:;
/^->/{s:->#\(.*\):/* \1 */:; s:^->\([^ ]*\) [\$#]*\([-0-9]*\)
\(.*\):#define \1 \2 /* \3 */:; s:^->\([^ ]*\) [\$#]*\([^ ]*\)
\(.*\):#define \1 \2 /* \3 */:; s:->::; p;}' lib/powerpc/asm-offsets.s;
echo ""; echo "#endif" ) > lib/powerpc/asm-offsets.h
cp -f lib/powerpc/asm-offsets.h lib/generated/
clang -no-integrated-as -Wa,-mpower10 -std=gnu99 -ffreestanding -O2
-msoft-float -mno-altivec -I /root/chins/git/kvm-unit-tests/lib -I
/root/chins/git/kvm-unit-tests/lib/libfdt -I lib -Wa,-mregnames -g -MMD
-MP -MF powerpc/.selftest.d -fno-strict-aliasing -fno-common -Wall
-Wwrite-strings -Wempty-body -Wuninitialized -Wignored-qualifiers
-Wno-missing-braces -Werror -fomit-frame-pointer -fno-stack-protector
-Wno-frame-address -fno-pic -Wunused-but-set-parameter
-Wno-override-init -Wmissing-prototypes -Wstrict-prototypes
-mlittle-endian -c -o powerpc/selftest.o
/root/chins/git/kvm-unit-tests/powerpc/selftest.c
Infact, if I just execute that single command independently to build
asm-offsets.s, it works as well.
However, I noticed that if that single command is executed in a
directory (build or main directory) where configure command hasn't been
executed first, that is when the error you have shared above pops out.
It is probably because configure adds some file to ./lib and those are
included in the clang command using "-I lib"
Thanks,
Chinmay
>
> Thanks,
> Thomas
>
^ permalink raw reply
* Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR
From: Sven Schnelle @ 2026-07-03 6:26 UTC (permalink / raw)
To: Thomas Gleixner
Cc: H. Peter Anvin, Michal Suchánek, Peter Zijlstra,
Jonathan Corbet, Shuah Khan, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Andy Lutomirski,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Andrew Donnellan,
Mark Rutland, Arnd Bergmann, Jiaxun Yang, Ryan Roberts,
Greg Kroah-Hartman, Mukesh Kumar Chaurasiya, Shrikanth Hegde,
Zong Li, Nam Cao, Deepak Gupta, Lukas Gerlach, Rui Qi, Kees Cook,
linux-doc, linux-kernel, loongarch, linuxppc-dev, linux-riscv,
linux-s390
In-Reply-To: <87h5mhnjsr.ffs@fw13>
Thomas Gleixner <tglx@kernel.org> writes:
> If seccomp overwrites regs->eax and aborts any syscall (including -1) by
> returning -1, then the value seccomp wrote into regs->eax is preserved
> and returned to user space.
>
> The same applies for syscall_user_dispatch() and ptrace...() if they
> decide to overwrite regs->eax _and_ abort the syscall by letting
> syscall_enter_from_user_mode() return -1.
>
> trace_syscall_enter() is not any different. If the magic BPF in there
> rewrites the syscall number to -1 then either the original -ENOSYS or
> the BPF induced overwrite is returned to user space.
>
> It's less than obvious and I have no objections to clean that up and
> make it more intuitive, but I still fail to see what Michal is actually
> trying to solve and what the magic flag is for. If s390 requires it,
> then that's an s390 problem, but definitely x86 does not.
The difference between x86 and s390 is that on s390, regs->gprs[2] is
used for both the syscall number and the syscall return value.
That was a design mistake early in the begin about 25 years ago, but
it's ABI now, so it cannot be changed.
When seccomp decides to skip a syscall, it write a return value into
regs->gprs[2]. When syscall_enter_from_user_mode_work() returns, it
returns this number. If it's negative all is good - the 'if (likely(nr <
NR_syscalls))' conditiion would just catch it and skip the syscall.
But if it's a positive number, the code cannot distinguish whether
that's a return value or a syscall number.
So I introduced PIF_SYSCALL_RET_SET when converting s390 to generic
entry. This flag tells the syscall code that a return value was set in
ptregs and the syscall should be skipped.
I'd like to see something like the change from Michal going in - cleaned
up of course. It would allow us to get rid of PIF_SYSCALL_RET_SET.
^ permalink raw reply
* Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR
From: Sven Schnelle @ 2026-07-03 6:16 UTC (permalink / raw)
To: Michal Suchánek
Cc: Peter Zijlstra, Jonathan Corbet, Shuah Khan, Huacai Chen,
WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Andrew Donnellan, Mark Rutland,
Arnd Bergmann, Jiaxun Yang, Ryan Roberts, Greg Kroah-Hartman,
Mukesh Kumar Chaurasiya, Shrikanth Hegde, Zong Li, Nam Cao,
Deepak Gupta, Lukas Gerlach, Rui Qi, Kees Cook, linux-doc,
linux-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390
In-Reply-To: <akZV7kjVh37z63Nz@kunlun.suse.cz>
Michal Suchánek <msuchanek@suse.de> writes:
> On Thu, Jul 02, 2026 at 02:01:02PM +0200, Sven Schnelle wrote:
>> Michal Suchánek <msuchanek@suse.de> writes:
>>
>> > On Thu, Jul 02, 2026 at 10:12:35AM +0200, Sven Schnelle wrote:
>> >> Michal Suchánek <msuchanek@suse.de> writes:
>> >>
>> >> > The return value of syscall_enter_from_user_mode is used both for the
>> >> > adjusted syscall number and the indicator that a syscall should be
>> >> > skipped.
>> >> >
>> >> > As seccomp can be invoked on any syscall, including invalid ones this
>> >> > somewhat undermines seccomp.
>> >> >
>> >> > While the seccomp variants that terminate the process do not need to
>> >> > care about this for the filter that sets the syscall return value this
>> >> > disctinction is required.
>> >> >
>> >> > Pass the syscall number as a pointer to the inline entry functions, and
>> >> > use the return value exclusively for the indication that the syscall is
>> >> > already handled.
>> >> >
>> >> > This should avoid the need for the s390 PIF_SYSCALL_RET_SET which is the
>> >> > workaround for exactly this deficiency.
>> >>
>> >> I'm not sure whether PIF_SYSCALL_RET_SET can be removed - the syscall
>> >> return might still get set by PTRACE_SET_SYSCALL_INFO when the tracee is
>> >> stopped. This might be a positive number which can't be distinguished
>> >> from a syscall number. But maybe i'm missing something? It's been quite
>> >> a while since I touched all that ptrace stuff.
>> >
>> > When the syscall return value is set (in the registers) the return value
>> > which is also the modified syscall number is set to -1 indicating the
>> > syscall was handled. At least that's how the API is described.
>> >
>> > So yes, if the syscall number range is restricted or the syscall number
>> > is returned through a path different from the function return value the
>> > flag should not be needed in the entry path because the case can be
>> > detected through the return value alone.
>>
>> I'm still failing to see how this would work without an additional
>> flag. Assume a program (the tracee) is stopped because of a syscall
>> entry. The tracer then decides to skip the syscall and changes
>> regs->gpr2 (which contains either the syscall number or return value)
>> to contain 42. When the tracer than restarts the syscall, how does
>> do_syscall() know that gpr2 is now a return value and not a syscall number?
>
> Because then the return value from the syscall_enter_from_user_mode
> machinery would be -1 indicating the syscall should be skipped. That is
> how the return value of syscall_enter_from_user_mode is documented, I
> did not verify that it actually works that way for the tracing case on
> s390.
I read the code and tested - I think I confused the way how syscall are
intercepted by seccomp vs. ptrace. The PIF_SYSCALL_RET_SET is indeed
only required to indicate syscalls skipped via seccomp and not ptrace.
^ permalink raw reply
* Re: [PATCH v3] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
From: Ricardo Robaina @ 2026-07-02 22:49 UTC (permalink / raw)
To: Paul Moore
Cc: Venkat Rao Bagalkote, Christophe Leroy (CS GROUP),
Madhavan Srinivasan, Harsh Prateek Bora, Michael Ellerman,
Nicholas Piggin, Eric Paris, linux-kernel, linuxppc-dev, audit,
Thomas Weissschuh, Cédric Le Goater, ritesh.list
In-Reply-To: <CAABTaaCQCBuR9Ny6zBHNK=gy3Cd0WGj-f9qgzwuyjWD_i=v_dQ@mail.gmail.com>
On Thu, Jul 2, 2026 at 3:19 PM Ricardo Robaina <rrobaina@redhat.com> wrote:
>
> On Thu, Jul 2, 2026 at 10:28 AM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Thu, Jul 2, 2026 at 8:50 AM Venkat Rao Bagalkote
> > <venkat88@linux.ibm.com> wrote:
> > >
> > > Current remaining issue:
> > >
> > > 1. amcast_joinpart
> > > ------------------
> > > Still fails due to a missing Perl dependency:
> > >
> > > Can't locate Socket/Netlink.pm in @INC
> > >
> > > I searched the configured repositories and could not find a package
> > > providing Socket::Netlink / Socket/Netlink.pm on this EL10 ppc64le system.
> > >
> > > Latest run summary:
> > >
> > > amcast_joinpart/test ................. Can't locate Socket/Netlink.pm in
> > > @INC (you may need to install the Socket::Netlink module) (@INC entries
> > > checked: /usr/local/lib64/perl5/5.40 /usr/local/share/perl5/5.40
> > > /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl
> > > /usr/lib64/perl5 /usr/share/perl5 .) at amcast_joinpart/test line 10.
> > > BEGIN failed--compilation aborted at amcast_joinpart/test line 10.
> > > amcast_joinpart/test ................. Dubious, test returned 2 (wstat
> > > 512, 0x200)
> > > Failed 7/7 subtests
> > > backlog_wait_time_actual_reset/test .. ok
> > > bpf/test ............................. ok
> > > exec_execve/test ..................... ok
> > > exec_name/test ....................... ok
> > > fanotify/test ........................ ok
> > > field_compare/test ................... ok
> > > file_create/test ..................... ok
> > > file_delete/test ..................... ok
> > > file_permission/test ................. ok
> > > file_rename/test ..................... ok
> > > filter_exclude/test .................. ok
> > > filter_exit/test ..................... ok
> > > filter_saddr_fam/test ................ ok
> > > filter_sessionid/test ................ ok
> > > io_uring/test ........................ ok
> > > login_tty/test ....................... ok
> > > lost_reset/test ...................... ok
> > > netfilter_pkt/test ................... ok
> > > signal/test .......................... ok
> > > syscalls_file/test ................... ok
> > > syscall_module/test .................. ok
> > > time_change/test ..................... ok
> > > user_msg/test ........................ ok
> > >
> > > All tests now pass except amcast_joinpart, which is blocked by the
> > > missing Socket::Netlink Perl module.
> > >
> > > Please let me know if there is a recommended package source for
> > > Socket::Netlink on EL10 ppc64le.
> >
> > Thanks for following up on this Venkat.
> >
> > I'm adding your colleague Ricardo from the RH side of the house.
> > Ricardo is very familiar with audit and should be able to help you
> > with missing RHEL packages.
>
> Thanks for looping me in, Paul!
>
> Venkat, unfortunately, perl-Socket-Netlink is not packaged for RHEL-10
> ppc64le. An alternative is to use CPAN[1] to install the
> Socket::Netlink Perl module.
>
> # dnf install cpan
> # cpan App::cpanminus
> # cpanm Socket::Netlink
>
> I'll reserve a ppc64le machine and run the tests locally as well. I'll
> share the results shortly.
>
> [1] https://www.cpan.org/modules/INSTALL.html
>
>
> >
> > --
> > paul-moore.com
> >
Hello everyone,
I built an RHEL-10 kernel with the patch applied and verified that
audit-testsuite passes successfully on a ppc64le machine. Please find
the logs below:
Tested-by: Ricardo Robaina <rrobaina@redhat.com>
---
# uname -m
ppc64le
# uname -r
6.12.0-245.test.el10.ppc64le
# make test
...
chmod +x */test
Running as user root
with context unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
on system RHEL10
amcast_joinpart/test ................. ok
backlog_wait_time_actual_reset/test .. ok
bpf/test ............................. ok
coredump/test ........................ ok
exec_execve/test ..................... ok
exec_name/test ....................... ok
fanotify/test ........................ ok
field_compare/test ................... ok
file_create/test ..................... ok
file_delete/test ..................... ok
file_permission/test ................. ok
file_rename/test ..................... ok
filter_device/test ................... ok
filter_exclude/test .................. ok
filter_exit/test ..................... ok
filter_inode/test .................... ok
filter_saddr_fam/test ................ ok
filter_sessionid/test ................ ok
io_uring/test ........................ ok
login_tty/test ....................... ok
lost_reset/test ...................... ok
netfilter_pkt/test ................... ok
signal/test .......................... ok
syscalls_file/test ................... ok
syscall_module/test .................. ok
time_change/test ..................... ok
user_msg/test ........................ ok
All tests successful.
Files=27, Tests=300, 252 wallclock secs ( 0.12 usr 0.01 sys + 54.34
cusr 1.38 csys = 55.85 CPU)
Result: PASS
make[1]: Leaving directory '/root/rrobaina/audit-testsuite/tests'
---
I hope it helps!
-Ricardo
^ permalink raw reply
* Re: [PATCH v3] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
From: Ricardo Robaina @ 2026-07-02 18:19 UTC (permalink / raw)
To: Paul Moore
Cc: Venkat Rao Bagalkote, Christophe Leroy (CS GROUP),
Madhavan Srinivasan, Harsh Prateek Bora, Michael Ellerman,
Nicholas Piggin, Eric Paris, linux-kernel, linuxppc-dev, audit,
Thomas Weissschuh, Cédric Le Goater, ritesh.list
In-Reply-To: <CAHC9VhQvoK4uYP0NBnd4FGbXCQkDka4nHqdz9kAn2bGnm4qVHQ@mail.gmail.com>
On Thu, Jul 2, 2026 at 10:28 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Jul 2, 2026 at 8:50 AM Venkat Rao Bagalkote
> <venkat88@linux.ibm.com> wrote:
> >
> > Current remaining issue:
> >
> > 1. amcast_joinpart
> > ------------------
> > Still fails due to a missing Perl dependency:
> >
> > Can't locate Socket/Netlink.pm in @INC
> >
> > I searched the configured repositories and could not find a package
> > providing Socket::Netlink / Socket/Netlink.pm on this EL10 ppc64le system.
> >
> > Latest run summary:
> >
> > amcast_joinpart/test ................. Can't locate Socket/Netlink.pm in
> > @INC (you may need to install the Socket::Netlink module) (@INC entries
> > checked: /usr/local/lib64/perl5/5.40 /usr/local/share/perl5/5.40
> > /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl
> > /usr/lib64/perl5 /usr/share/perl5 .) at amcast_joinpart/test line 10.
> > BEGIN failed--compilation aborted at amcast_joinpart/test line 10.
> > amcast_joinpart/test ................. Dubious, test returned 2 (wstat
> > 512, 0x200)
> > Failed 7/7 subtests
> > backlog_wait_time_actual_reset/test .. ok
> > bpf/test ............................. ok
> > exec_execve/test ..................... ok
> > exec_name/test ....................... ok
> > fanotify/test ........................ ok
> > field_compare/test ................... ok
> > file_create/test ..................... ok
> > file_delete/test ..................... ok
> > file_permission/test ................. ok
> > file_rename/test ..................... ok
> > filter_exclude/test .................. ok
> > filter_exit/test ..................... ok
> > filter_saddr_fam/test ................ ok
> > filter_sessionid/test ................ ok
> > io_uring/test ........................ ok
> > login_tty/test ....................... ok
> > lost_reset/test ...................... ok
> > netfilter_pkt/test ................... ok
> > signal/test .......................... ok
> > syscalls_file/test ................... ok
> > syscall_module/test .................. ok
> > time_change/test ..................... ok
> > user_msg/test ........................ ok
> >
> > All tests now pass except amcast_joinpart, which is blocked by the
> > missing Socket::Netlink Perl module.
> >
> > Please let me know if there is a recommended package source for
> > Socket::Netlink on EL10 ppc64le.
>
> Thanks for following up on this Venkat.
>
> I'm adding your colleague Ricardo from the RH side of the house.
> Ricardo is very familiar with audit and should be able to help you
> with missing RHEL packages.
Thanks for looping me in, Paul!
Venkat, unfortunately, perl-Socket-Netlink is not packaged for RHEL-10
ppc64le. An alternative is to use CPAN[1] to install the
Socket::Netlink Perl module.
# dnf install cpan
# cpan App::cpanminus
# cpanm Socket::Netlink
I'll reserve a ppc64le machine and run the tests locally as well. I'll
share the results shortly.
[1] https://www.cpan.org/modules/INSTALL.html
>
> --
> paul-moore.com
>
^ permalink raw reply
* Re: [PATCH 3/4] USB: gadget: fsl-udc: fix dev_printk() device
From: Uwe Kleine-König @ 2026-07-02 22:38 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linuxppc-dev, linux-kernel
In-Reply-To: <20260702141536.90887-4-johan@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 754 bytes --]
Hello Johan,
On Thu, Jul 02, 2026 at 04:15:35PM +0200, Johan Hovold wrote:
> A change replacing custom printk() macros with dev_printk() incorrectly
> used the gadget struct device instead of the controller struct device
> (including for messages printed before the gadget device name has been
> initialised).
>
> Switch to using the controller platform device with dev_printk() so that
> the controller device and driver names are included in log messages as
> expected.
I don't know about "expected", because before 6025f20f16c2 the affected
device wasn't included in the message at all. But I agree that using the
not yet initialized device isn't nice, so:
Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Thanks
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR
From: Thomas Gleixner @ 2026-07-02 21:49 UTC (permalink / raw)
To: H. Peter Anvin, Michal Suchánek, Peter Zijlstra
Cc: Jonathan Corbet, Shuah Khan, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Andy Lutomirski, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Andrew Donnellan, Mark Rutland, Arnd Bergmann, Jiaxun Yang,
Ryan Roberts, Greg Kroah-Hartman, Mukesh Kumar Chaurasiya,
Shrikanth Hegde, Zong Li, Nam Cao, Deepak Gupta, Lukas Gerlach,
Rui Qi, Kees Cook, linux-doc, linux-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390
In-Reply-To: <BA7CD91D-C0E5-47A1-B49C-BC6AF6604182@zytor.com>
On Wed, Jul 01 2026 at 11:29, H. Peter Anvin wrote:
Can you please trim your replies? Scrolling through hundred lines of
useless quoted text is just annoying.
> On July 1, 2026 10:42:08 AM PDT, "Michal Suchánek" <msuchanek@suse.de> wrote:
>>-static __always_inline long syscall_enter_from_user_mode(struct pt_regs *regs, long syscall)
>>+static __always_inline long syscall_enter_from_user_mode(struct pt_regs *regs, long *syscall)
>> {
>> long ret;
>>
> 1. The type for a system call is int.
That ship has sailed long ago. man syscall ...
> 2. A valid system call number is always going to be positive.
That's true today.
> 3. Bits [30:24] are available for architecture ABI use. The
> "architecture independent" part of the system call number is therefore
> 24 bits wide.
>
> 4. The exact ABI is platform-specific, obviously, but as a general
> guideline (especially for new platforms/ABIs) should follow the rules
> for a platform "int" if practical. Notably, when passing a value in a
> register larger than 32 bits, which side of the calling interface is
> responsible for sign-extending a value passed in a register. If caller
> side, the kernel should validate, if callee side the kernel should
> ignore the additional bits and do the extension.
The kernel sign expands today already, i.e. for compat syscalls.
> 5. A negative system call number is guaranteed to return -ENOSYS
> (unless intercepted by seccomp, ptrace, or another mechanism under
> user space control.)
That's true today.
ASM entry:
regs->eax = -ENOSYS;
C entry:
nr = syscall_enter_from_user_mode(regs, nr);
if ((unsigned)nr < SYSCALL_MAX)
regs->eax = handle_syscall();
else if (nr != -1)
regs->eax = -ENOSYS;
....
If seccomp overwrites regs->eax and aborts any syscall (including -1) by
returning -1, then the value seccomp wrote into regs->eax is preserved
and returned to user space.
The same applies for syscall_user_dispatch() and ptrace...() if they
decide to overwrite regs->eax _and_ abort the syscall by letting
syscall_enter_from_user_mode() return -1.
trace_syscall_enter() is not any different. If the magic BPF in there
rewrites the syscall number to -1 then either the original -ENOSYS or
the BPF induced overwrite is returned to user space.
It's less than obvious and I have no objections to clean that up and
make it more intuitive, but I still fail to see what Michal is actually
trying to solve and what the magic flag is for. If s390 requires it,
then that's an s390 problem, but definitely x86 does not.
> 6. If the platform needs to algorithmically modify the system call
> number due to platform-specific concerns (say, the platform uses a
> 16-bit special purpose register for the syscall number, or it has
> multiple kernel entry points with different behavior), it should if at
> all possible transcode the system call number as necessary to match
> this convention in APIs that are exposed to general kernel code.
>
> For example, in the future I could very much see the IA32 code in the
> x86 kernel using bit 29 internally to indicate an ia32 system call,
> simplifying the is_compat implementation on x86.
I don't see how that makes it simpler. Those are two different entry
code paths and magic bits wont make that go away.
> It should not mean that passing bit 29 to either the syscall
> instruction or int $0x80 will be accepted.
Your proposal looks even more like a solution in search of a problem
than the original one.
Thanks,
tglx
^ permalink raw reply
* [PATCH 3/3] powerpc/boot: Fix treeboot-akebono CPU node lookup check
From: Thorsten Blum @ 2026-07-02 21:15 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Josh Boyer, Grant Likely,
Tony Breeds, Alistair Popple, Benjamin Herrenschmidt
Cc: linuxppc-dev, linux-kernel, Thorsten Blum
In-Reply-To: <20260702211554.56923-4-thorsten.blum@linux.dev>
fdt_node_offset_by_prop_value() returns a negative error code on
failure - fix the check accordingly.
Fixes: 2a2c74b2efcb ("IBM Akebono: Add the Akebono platform")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
arch/powerpc/boot/treeboot-akebono.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/treeboot-akebono.c b/arch/powerpc/boot/treeboot-akebono.c
index e3cc2599869c..1b529037480f 100644
--- a/arch/powerpc/boot/treeboot-akebono.c
+++ b/arch/powerpc/boot/treeboot-akebono.c
@@ -146,7 +146,7 @@ void platform_init(char *userdata)
node = fdt_node_offset_by_prop_value(_dtb_start, -1, "device_type",
"cpu", sizeof("cpu"));
- if (!node)
+ if (node < 0)
fatal("Cannot find cpu node\n");
timebase = fdt_getprop(_dtb_start, node, "timebase-frequency", &size);
if (timebase && (size == 4))
^ permalink raw reply related
* [PATCH 1/3] powerpc/boot: Fix simpleboot CPU node lookup check
From: Thorsten Blum @ 2026-07-02 21:15 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Josh Boyer, Grant Likely,
Tony Breeds, Alistair Popple, Benjamin Herrenschmidt
Cc: linuxppc-dev, linux-kernel, Thorsten Blum
fdt_node_offset_by_prop_value() returns a negative error code on
failure - fix the check accordingly.
Fixes: d2477b5cc8ca ("[POWERPC] bootwrapper: Add a firmware-independent simpleboot target.")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
arch/powerpc/boot/simpleboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/simpleboot.c b/arch/powerpc/boot/simpleboot.c
index c80691d83880..27591df41e9e 100644
--- a/arch/powerpc/boot/simpleboot.c
+++ b/arch/powerpc/boot/simpleboot.c
@@ -68,7 +68,7 @@ void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
/* finally, setup the timebase */
node = fdt_node_offset_by_prop_value(_dtb_start, -1, "device_type",
"cpu", sizeof("cpu"));
- if (!node)
+ if (node < 0)
fatal("Cannot find cpu node\n");
timebase = fdt_getprop(_dtb_start, node, "timebase-frequency", &size);
if (timebase && (size == 4))
^ permalink raw reply related
* [PATCH 2/3] powerpc/boot: Fix treeboot-currituck CPU node lookup check
From: Thorsten Blum @ 2026-07-02 21:15 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Josh Boyer, Grant Likely,
Tony Breeds, Alistair Popple, Benjamin Herrenschmidt
Cc: linuxppc-dev, linux-kernel, Thorsten Blum
In-Reply-To: <20260702211554.56923-4-thorsten.blum@linux.dev>
fdt_node_offset_by_prop_value() returns a negative error code on
failure - fix the check accordingly.
Fixes: 228d55053397 ("powerpc/47x: Add support for the new IBM currituck platform")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
arch/powerpc/boot/treeboot-currituck.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/treeboot-currituck.c b/arch/powerpc/boot/treeboot-currituck.c
index d53e8a592f81..5b5363b74f9f 100644
--- a/arch/powerpc/boot/treeboot-currituck.c
+++ b/arch/powerpc/boot/treeboot-currituck.c
@@ -102,7 +102,7 @@ void platform_init(void)
node = fdt_node_offset_by_prop_value(_dtb_start, -1, "device_type",
"cpu", sizeof("cpu"));
- if (!node)
+ if (node < 0)
fatal("Cannot find cpu node\n");
timebase = fdt_getprop(_dtb_start, node, "timebase-frequency", &size);
if (timebase && (size == 4))
^ permalink raw reply related
* Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR
From: Thomas Gleixner @ 2026-07-02 20:45 UTC (permalink / raw)
To: Michal Suchánek
Cc: Peter Zijlstra, Jonathan Corbet, Shuah Khan, Huacai Chen,
WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Andy Lutomirski, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Andrew Donnellan, Mark Rutland,
Arnd Bergmann, Jiaxun Yang, Ryan Roberts, Greg Kroah-Hartman,
Mukesh Kumar Chaurasiya, Shrikanth Hegde, Zong Li, Nam Cao,
Deepak Gupta, Lukas Gerlach, Rui Qi, Kees Cook, linux-doc,
linux-kernel, loongarch, linuxppc-dev, linux-riscv, linux-s390
In-Reply-To: <akZPakNl6JT_jgGd@kunlun.suse.cz>
On Thu, Jul 02 2026 at 13:45, Michal Suchánek wrote:
> On Thu, Jul 02, 2026 at 01:24:57PM +0200, Thomas Gleixner wrote:
>> On Wed, Jul 01 2026 at 19:42, Michal Suchánek wrote:
>> > The return value of syscall_enter_from_user_mode is used both for the
>> > adjusted syscall number and the indicator that a syscall should be
>> > skipped.
>> >
>> > As seccomp can be invoked on any syscall, including invalid ones this
>> > somewhat undermines seccomp.
>> >
>> > While the seccomp variants that terminate the process do not need to
>> > care about this for the filter that sets the syscall return value this
>> > disctinction is required.
>>
>> You completely fail to explain why and what actual problem you are
>> trying to solve. At least I can't figure it out from the above word
>> salad.
>
> syscall_enter_from_user_mode returns the new syscall number after doing
> something arbitrarry with it, including running seccomp.
>
> Wehn the syscall is already handled, eg. by seccomp filtering it returns
> -1 as the new syscall number. -1 is an invalid syscall number but it can
> still be filtered by seccomp.
Once syscall_enter_from_user_mode() returns -1 nothing can filter it
anymore.
> When the syscall number was -1 to start with it's not possible to
> determine if the syscall was fileterd from the return value. s390
> returns the filtered state in a flag it sets on the regs structure,
> avoiding this problem.
What needs to determine whether the syscall was filtered or not?
> However, the API should be specified in a way that does not require
> everyone implementing such flag.
Which exact problem does the flag solve?
>> > - instrumentation_begin();
>> > - if (!invoke_syscall(regs, nr) && nr != -1)
>> > - result_reg(regs) = __sys_ni_syscall(regs);
>> > - instrumentation_end();
>> > + /* Skip syscall when -1 is returned */
>> > + if (!syscall_enter_from_user_mode(regs, &nr)) {
>>
>> Seriously?
>>
>> If we go and separate the syscall number from the return value, then the
>> return value 0 means success and anything else fail. Which in other
>> words is a boolean. So instead of tastelessly adding a completely
>> nonsensical comment about -1 here, syscall_enter_from_user_mode() wants
>> to have the return value type bool with a proper boolean logic: true =
>> success, false = abort.
>
> We have that very same API down to __secure_computing() which returns
> boolean represented as -1 and 0 values. That does not mean it's not
> tasteless.
Hahahahaha.
We have a lot of functions which have a boolean return value but a
int/long return type for historical reasons.
We've added bool because it's not ambiguous and allows the compiler to
optimize better. It also makes the code more clear. Modern code uses a
non-boolean return type only when there is an actual reason for it,
e.g. propagating an error code all the way back through the call chain.
The historical 0=success <0 = errorcode model really want's to be
restricted to such cases.
Just for the record:
https://lore.kernel.org/all/67c3ae5c-d88b-4172-9996-4e2046b7e0dc@huawei.com/
https://lore.kernel.org/all/20260629130616.642022-2-ruanjinjie@huawei.com/
So I stand with my comment that it is sloppy and tasteless to slap an
argument into a pile of functions, claim separation of return value and
syscall number and leave the return value in an ill-defined state.
>> > @@ -168,8 +168,7 @@ __visible noinstr void do_int80_emulation(struct pt_regs *regs)
>> > nr = syscall_32_enter(regs);
>> >
>> > local_irq_enable();
>> > - nr = syscall_enter_from_user_mode_work(regs, nr);
>> > - do_syscall_32_irqs_on(regs, nr);
>> > + syscall_enter_from_user_mode_work(regs, &nr);
>>
>> How exactly is this ever going to invoke a valid syscall?
>
> That's one of the problems with giant all-in-one patch, things like this
> easily slip in. However, it is in cluded mostly for illustration, I
> don't expect anyone to merge this as-is.
It's a problem with hastily cobbled together slop. Even RFC patches
should at least be functional.
>> > + if (!syscall_enter_from_user_mode_work(regs, &nr)) {
>> > + nr &= GENMASK(31, 0);
>> > + do_syscall_32_irqs_on(regs, nr);
>>
>> do_syscall_32_irqs_on(regs, (int)nr);
>>
>> would be too simple, right?
>
> Also way less explicit.
Now you care about explicit, but the return value mess can be left
ambiguous, right?
Aside of that, the cast is very much explicit for people who can read C.
It would be great if you could sit back and come up with a very explicit
and comprehensible explanation for the problem you are trying to solve.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v3 4/4] panic: use sys_info_with_filter() to avoid duplicate backtraces
From: Bradley Morgan @ 2026-07-02 18:22 UTC (permalink / raw)
To: Petr Mladek
Cc: Feng Tang, Andrew Morton, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Madhavan Srinivasan, Douglas Anderson,
linux-kernel, linuxppc-dev, stable
In-Reply-To: <EC1E5A79-524A-45C2-9FE8-964EB0E18D76@grrlz.net>
On July 2, 2026 7:13:26 PM GMT+01:00, Bradley Morgan <include@grrlz.net>
wrote:
>On July 2, 2026 10:09:41 AM GMT+01:00, Petr Mladek <pmladek@suse.com>
>wrote:
>>On Mon 2026-06-29 13:54:18, Bradley Morgan wrote:
>>> On 29 June 2026 12:40:52 BST, Feng Tang <feng.tang@linux.alibaba.com>
>>> wrote:
>>> >On Fri, Jun 26, 2026 at 02:14:14PM +0200, Petr Mladek wrote:
>>> >> On Fri 2026-06-26 12:23:50, Petr Mladek wrote:
>>> >> > On Thu 2026-06-25 15:25:58, Bradley Morgan wrote:
>>> >> > > panic_other_cpus_shutdown() handles SYS_INFO_ALL_BT before
>>stopping
>>> >the
>>> >> > > other CPUs. Do not ask sys_info() to handle that bit again later
>>in
>>> >the
>>> >> > > panic path.
>>> >> > >
>>> >> > > Use sys_info_with_filter() so panic_print=all_bt does not
>request
>>> >more
>>> >> > > output after the CPUs are stopped.
>>> >> > >
>>> >> > > Fixes: a9af76a78760 ("watchdog: add sys_info sysctls to dump sys
>>> >info on system lockup")
>>> >> > > Cc: stable@vger.kernel.org
>>> >> > > Signed-off-by: Bradley Morgan <include@grrlz.net>
>>> >> > > ---
>>> >> > > kernel/panic.c | 2 +-
>>> >> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >> > >
>>> >> > > diff --git a/kernel/panic.c b/kernel/panic.c
>>> >> > > index 213725b612aa..eb842823df61 100644
>>> >> > > --- a/kernel/panic.c
>>> >> > > +++ b/kernel/panic.c
>>> >> > > @@ -680,7 +680,7 @@ void vpanic(const char *fmt, va_list args)
>>> >> > > */
>>> >> > > atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
>>> >> > >
>>> >> > > - sys_info(panic_print);
>>> >> > > + sys_info_with_filter(panic_print, SYS_INFO_ALL_BT);
>>> >> >
>>> >> > Hmm, this prevents printing backtraces from all CPUs completely.
>>> >> > But what if they were not printed?
>>> >> >
>>> >> > They might be printed by:
>>> >> >
>>> >> > static void panic_other_cpus_shutdown(bool crash_kexec)
>>> >> > {
>>> >> > if (panic_print & SYS_INFO_ALL_BT)
>>> >> > panic_trigger_all_cpu_backtrace();
>>> >> >
>>> >> > [...]
>>> >> > }
>>> >> >
>>> >> > But it checks only "panic_print" variable. It won't do anything
>>> >> > when (panic_print == 0).
>>> >> >
>>> >> > In this case, we might still want to print the backraces when
>>> >> > SYS_INFO_ALL_BT is set in kernel_si_info.
>>> >> >
>>> >> > > kmsg_dump_desc(KMSG_DUMP_PANIC, buf);
>>> >> >
>>> >> > Of course, we might fix panic_other_cpus_shutdown() to check also
>>> >> > kernel_si_info.
>>> >> >
>>> >> > But it all becomes very hairy. We have several levels:
>>> >> >
>>> >> > + watchdog-all_bt-specific option, e.g.
>>> >sysctl_hardlockup_all_cpu_backtrace
>>> >> >
>>> >> > + watchdog-specific si_info preferences, e.g.
>hardlockup_si_mask
>>> >> >
>>> >> > + panic-specific si_info: panic_print
>>> >> >
>>> >> > + universal fallback for any layer: kernel_si_info
>>> >> >
>>> >> > Now, we try to check all these variables back and forth to
>>> >> > trigger all backtraces or to avoid triggering them.
>>> >> > And it clearly does not work well and the code is more and more
>>> >> > hairy.
>>> >> >
>>> >> > I think about another approach. The word "waterfall" comes to my
>>mind.
>>> >> > Instead of checking all the settings back and forth, let's process
>>> >> > each setting one by one and just remember what has been done and
>>> >> > skip this in the next level.
>>> >> >
>>> >> > All the si_info actions seems to dump a global system state.
>>> >> > So, it would make sense to remember the state in a global variable
>>> >> > even when it might be modified by more CPUs in parallel.
>>> >> >
>>> >> > I am going to think more about it.
>>> >>
>>> >> I have created a POC using Gemini. I haven't tested it.
>>> >> But it looks acceptable. And the logic seems to be more
>>> >> straightforward.
>>> >>
>>> >> One drawback is that it requires adding the _reset()
>>> >> call for all sys_info() callers. It is fine in principle
>>> >> but it might complicate back-porting because all changes
>>> >> have to be done in one patch.
>>> >>
>>> >> But honestly, this is a nice to have fix. Most people could
>>> >> live happily without it.
>>> >>
>>> >> From 3c66436d9978030845a96bfaedd6b914536e2ac4 Mon Sep 17 00:00:00
>>2001
>>> >> From: Petr Mladek <pmladek@suse.com>
>>> >> Date: Fri, 26 Jun 2026 13:55:41 +0200
>>> >> Subject: [POC] sys_info: Introduce state-tracking APIs to prevent
>>> >duplicate
>>> >> backtraces
>>> >>
>>> >> In watchdog, panic, and hung task detection scenarios, sys_info()
>can
>>> >> be called multiple times or alongside direct backtrace triggers like
>>> >> trigger_allbutcpu_cpu_backtrace(). This results in identical
>>backtraces
>>> >> being dumped repeatedly from all CPUs, cluttering the kernel log and
>>> >> delaying or obscuring critical debug details.
>>> >>
>>> >> Introduce a state tracking bitmask and associated helpers:
>>> >> - sys_info_done(mask): Marks specific sys_info bits as already
>>printed.
>>> >> - sys_info_reset(): Resets the tracking state.
>>> >> - sys_info_is_done(mask): Checks if all bits in the mask have been
>>> >printed.
>>> >>
>>> >> Update sys_info() to automatically filter out already printed bits
>>> >> using this state. Integrate these APIs with the generic hardlockup
>>> >> and softlockup watchdogs, the PowerPC watchdog, the hung task
>>detector,
>>> >> and the panic core. This ensures that each piece of system
>>information
>>> >> and backtrace output is printed at most once per lockup/panic event,
>>> >> and the state is reset cleanly when a lockup does not trigger a
>>panic.
>>> >>
>>> >> Races between sys_info() callers are ignored. It should be
>acceptable
>>> >> because the output from various watchdogs has never been
>>synchronized.
>>> >> And panic() never returns.
>>> >>
>>> >> Assisted-by: gemini-1.5-flash
>>> >> Signed-off-by: Petr Mladek <pmladek@suse.com>
>>> >
>>> >Yep. There are cases that people want panic on task-hung or sw/hw
>>lockup,
>>> >and this could remove much duplication of sys info dump, thanks!
>>> >
>>> >Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
>>>
>>> Thanks,
>>>
>>> im feeling a new file to do all the force panic jazz, but putting tape
>>> on sys_info.c isn't bd either.
>>
>>I wonder how to move forward with this.
>>
>>Honestly, I am not sure what exactly you mean by creating another
>>API for tracking the reports so I could not judge it. Feel free
>>to sent some POC.
>>
>>Otherwise, I would go with my proposal to remember the printed states
>>by the sys_info API. I am not sure whether I should send a proper
>>patch or you would like to somehow improve it.
>>
>>Best Regards,
>>Petr
>>
>
>
>sup petr, here's my poc
>
>
>This should make my entire thing make sense
Actually, looks like churn to me.
we shouldn't do that.
It is on that gist though.
As in, I'm thinking about doing a new API to perhaps, make sys_info better?
But it's very complicated, and may need discussion, feel free to ignore
that POC...
crappy old me, need a coffee heh.
Thanks!
^ permalink raw reply
* Re: [PATCH v3 4/4] panic: use sys_info_with_filter() to avoid duplicate backtraces
From: Bradley Morgan @ 2026-07-02 18:13 UTC (permalink / raw)
To: Petr Mladek
Cc: Feng Tang, Andrew Morton, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Madhavan Srinivasan, Douglas Anderson,
linux-kernel, linuxppc-dev, stable
In-Reply-To: <akYq1YaCpZ0b4SBS@pathway.suse.cz>
On July 2, 2026 10:09:41 AM GMT+01:00, Petr Mladek <pmladek@suse.com>
wrote:
>On Mon 2026-06-29 13:54:18, Bradley Morgan wrote:
>> On 29 June 2026 12:40:52 BST, Feng Tang <feng.tang@linux.alibaba.com>
>> wrote:
>> >On Fri, Jun 26, 2026 at 02:14:14PM +0200, Petr Mladek wrote:
>> >> On Fri 2026-06-26 12:23:50, Petr Mladek wrote:
>> >> > On Thu 2026-06-25 15:25:58, Bradley Morgan wrote:
>> >> > > panic_other_cpus_shutdown() handles SYS_INFO_ALL_BT before
>stopping
>> >the
>> >> > > other CPUs. Do not ask sys_info() to handle that bit again later
>in
>> >the
>> >> > > panic path.
>> >> > >
>> >> > > Use sys_info_with_filter() so panic_print=all_bt does not request
>> >more
>> >> > > output after the CPUs are stopped.
>> >> > >
>> >> > > Fixes: a9af76a78760 ("watchdog: add sys_info sysctls to dump sys
>> >info on system lockup")
>> >> > > Cc: stable@vger.kernel.org
>> >> > > Signed-off-by: Bradley Morgan <include@grrlz.net>
>> >> > > ---
>> >> > > kernel/panic.c | 2 +-
>> >> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >> > >
>> >> > > diff --git a/kernel/panic.c b/kernel/panic.c
>> >> > > index 213725b612aa..eb842823df61 100644
>> >> > > --- a/kernel/panic.c
>> >> > > +++ b/kernel/panic.c
>> >> > > @@ -680,7 +680,7 @@ void vpanic(const char *fmt, va_list args)
>> >> > > */
>> >> > > atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
>> >> > >
>> >> > > - sys_info(panic_print);
>> >> > > + sys_info_with_filter(panic_print, SYS_INFO_ALL_BT);
>> >> >
>> >> > Hmm, this prevents printing backtraces from all CPUs completely.
>> >> > But what if they were not printed?
>> >> >
>> >> > They might be printed by:
>> >> >
>> >> > static void panic_other_cpus_shutdown(bool crash_kexec)
>> >> > {
>> >> > if (panic_print & SYS_INFO_ALL_BT)
>> >> > panic_trigger_all_cpu_backtrace();
>> >> >
>> >> > [...]
>> >> > }
>> >> >
>> >> > But it checks only "panic_print" variable. It won't do anything
>> >> > when (panic_print == 0).
>> >> >
>> >> > In this case, we might still want to print the backraces when
>> >> > SYS_INFO_ALL_BT is set in kernel_si_info.
>> >> >
>> >> > > kmsg_dump_desc(KMSG_DUMP_PANIC, buf);
>> >> >
>> >> > Of course, we might fix panic_other_cpus_shutdown() to check also
>> >> > kernel_si_info.
>> >> >
>> >> > But it all becomes very hairy. We have several levels:
>> >> >
>> >> > + watchdog-all_bt-specific option, e.g.
>> >sysctl_hardlockup_all_cpu_backtrace
>> >> >
>> >> > + watchdog-specific si_info preferences, e.g. hardlockup_si_mask
>> >> >
>> >> > + panic-specific si_info: panic_print
>> >> >
>> >> > + universal fallback for any layer: kernel_si_info
>> >> >
>> >> > Now, we try to check all these variables back and forth to
>> >> > trigger all backtraces or to avoid triggering them.
>> >> > And it clearly does not work well and the code is more and more
>> >> > hairy.
>> >> >
>> >> > I think about another approach. The word "waterfall" comes to my
>mind.
>> >> > Instead of checking all the settings back and forth, let's process
>> >> > each setting one by one and just remember what has been done and
>> >> > skip this in the next level.
>> >> >
>> >> > All the si_info actions seems to dump a global system state.
>> >> > So, it would make sense to remember the state in a global variable
>> >> > even when it might be modified by more CPUs in parallel.
>> >> >
>> >> > I am going to think more about it.
>> >>
>> >> I have created a POC using Gemini. I haven't tested it.
>> >> But it looks acceptable. And the logic seems to be more
>> >> straightforward.
>> >>
>> >> One drawback is that it requires adding the _reset()
>> >> call for all sys_info() callers. It is fine in principle
>> >> but it might complicate back-porting because all changes
>> >> have to be done in one patch.
>> >>
>> >> But honestly, this is a nice to have fix. Most people could
>> >> live happily without it.
>> >>
>> >> From 3c66436d9978030845a96bfaedd6b914536e2ac4 Mon Sep 17 00:00:00
>2001
>> >> From: Petr Mladek <pmladek@suse.com>
>> >> Date: Fri, 26 Jun 2026 13:55:41 +0200
>> >> Subject: [POC] sys_info: Introduce state-tracking APIs to prevent
>> >duplicate
>> >> backtraces
>> >>
>> >> In watchdog, panic, and hung task detection scenarios, sys_info() can
>> >> be called multiple times or alongside direct backtrace triggers like
>> >> trigger_allbutcpu_cpu_backtrace(). This results in identical
>backtraces
>> >> being dumped repeatedly from all CPUs, cluttering the kernel log and
>> >> delaying or obscuring critical debug details.
>> >>
>> >> Introduce a state tracking bitmask and associated helpers:
>> >> - sys_info_done(mask): Marks specific sys_info bits as already
>printed.
>> >> - sys_info_reset(): Resets the tracking state.
>> >> - sys_info_is_done(mask): Checks if all bits in the mask have been
>> >printed.
>> >>
>> >> Update sys_info() to automatically filter out already printed bits
>> >> using this state. Integrate these APIs with the generic hardlockup
>> >> and softlockup watchdogs, the PowerPC watchdog, the hung task
>detector,
>> >> and the panic core. This ensures that each piece of system
>information
>> >> and backtrace output is printed at most once per lockup/panic event,
>> >> and the state is reset cleanly when a lockup does not trigger a
>panic.
>> >>
>> >> Races between sys_info() callers are ignored. It should be acceptable
>> >> because the output from various watchdogs has never been
>synchronized.
>> >> And panic() never returns.
>> >>
>> >> Assisted-by: gemini-1.5-flash
>> >> Signed-off-by: Petr Mladek <pmladek@suse.com>
>> >
>> >Yep. There are cases that people want panic on task-hung or sw/hw
>lockup,
>> >and this could remove much duplication of sys info dump, thanks!
>> >
>> >Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
>>
>> Thanks,
>>
>> im feeling a new file to do all the force panic jazz, but putting tape
>> on sys_info.c isn't bd either.
>
>I wonder how to move forward with this.
>
>Honestly, I am not sure what exactly you mean by creating another
>API for tracking the reports so I could not judge it. Feel free
>to sent some POC.
>
>Otherwise, I would go with my proposal to remember the printed states
>by the sys_info API. I am not sure whether I should send a proper
>patch or you would like to somehow improve it.
>
>Best Regards,
>Petr
>
sup petr, here's my poc
This should make my entire thing make sense
From eb587ed749ff5993c517f29799b369185c5ee7d8 Mon Sep 17 00:00:00 2001
From: Bradley Morgan <include@grrlz.net>
Date: Thu, 2 Jul 2026 18:09:23 +0000
Subject: [POC] sys_info: Introduce incident state-tracking to prevent
duplicate diagnostics
In watchdog, panic, and hung task detection scenarios, sys_info()
can be called multiple times or alongside direct debug output
functions (like trigger_allbutcpu_cpu_backtrace(), print_modules(),
print_irqtrace_events(), and dump_stack()). This leads to identical
diagnostics and stack traces being dumped repeatedly, cluttering the
kernel log and delaying critical panics.
Introduce a state tracking bitmask and helpers in a new file,
lib/sys_info_filter.c:
- sys_info_filter_and_set(mask): Atomically tests which bits in a mask
have not yet been printed during the current incident, marks them as
printed, and returns that subset.
- sys_info_reset(): Clears the printed mask state.
Add SYS_INFO_MODULES, SYS_INFO_IRQTRACE, and SYS_INFO_STACK flags to
include/linux/sys_info.h, and handle them inside sys_info's diagnostic
dispatch.
Update the watchdogs, hung task detector, and panic core to call
sys_info_filter_and_set() to deduplicate their diagnostic printouts, and
sys_info_reset() when a warning incident concludes (e.g., when a stuck
CPU recovers, or a new hung task check round begins).
This ensures each piece of system diagnostic is printed at most once per
lockup/panic event, preventing console log spam.
Assisted-by: Gemini:gemini-3.5-flash
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
arch/powerpc/kernel/watchdog.c | 21 +++++-
include/linux/sys_info.h | 6 ++
kernel/hung_task.c | 4 +-
kernel/panic.c | 9 ++-
kernel/watchdog.c | 24 ++++++-
lib/Makefile | 2 +-
lib/sys_info.c | 38 ++---------
lib/sys_info_filter.c | 120 +++++++++++++++++++++++++++++++++
8 files changed, 184 insertions(+), 40 deletions(-)
create mode 100644 lib/sys_info_filter.c
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index c40c69368476..31035e28676a 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -160,6 +160,10 @@ static void wd_lockup_ipi(struct pt_regs *regs)
else
dump_stack();
+ /* Mark what we already printed so panic() won't repeat it. */
+ sys_info_filter_and_set(SYS_INFO_MODULES | SYS_INFO_IRQTRACE |
+ SYS_INFO_STACK);
+
/*
* __wd_nmi_output must be set after we printk from NMI context.
*
@@ -238,7 +242,8 @@ static void watchdog_smp_panic(int cpu)
if (sysctl_hardlockup_all_cpu_backtrace ||
(hardlockup_si_mask & SYS_INFO_ALL_BT)) {
- trigger_allbutcpu_cpu_backtrace(cpu);
+ if (sys_info_filter_and_set(SYS_INFO_ALL_BT))
+ trigger_allbutcpu_cpu_backtrace(cpu);
cpumask_clear(&wd_smp_cpus_ipi);
} else {
/*
@@ -254,6 +259,8 @@ static void watchdog_smp_panic(int cpu)
sys_info(hardlockup_si_mask & ~SYS_INFO_ALL_BT);
if (hardlockup_panic)
nmi_panic(NULL, "Hard LOCKUP");
+ else
+ sys_info_reset();
wd_end_reporting();
@@ -416,15 +423,23 @@ DEFINE_INTERRUPT_HANDLER_NMI(soft_nmi_interrupt)
print_irqtrace_events(current);
show_regs(regs);
+ /* Mark what we already printed so panic() won't repeat it. */
+ sys_info_filter_and_set(SYS_INFO_MODULES | SYS_INFO_IRQTRACE |
+ SYS_INFO_STACK);
+
xchg(&__wd_nmi_output, 1); // see wd_lockup_ipi
if (sysctl_hardlockup_all_cpu_backtrace ||
- (hardlockup_si_mask & SYS_INFO_ALL_BT))
- trigger_allbutcpu_cpu_backtrace(cpu);
+ (hardlockup_si_mask & SYS_INFO_ALL_BT)) {
+ if (sys_info_filter_and_set(SYS_INFO_ALL_BT))
+ trigger_allbutcpu_cpu_backtrace(cpu);
+ }
sys_info(hardlockup_si_mask & ~SYS_INFO_ALL_BT);
if (hardlockup_panic)
nmi_panic(regs, "Hard LOCKUP");
+ else
+ sys_info_reset();
wd_end_reporting();
}
diff --git a/include/linux/sys_info.h b/include/linux/sys_info.h
index a5bc3ea3d44b..f5a1b699143b 100644
--- a/include/linux/sys_info.h
+++ b/include/linux/sys_info.h
@@ -16,8 +16,14 @@
#define SYS_INFO_PANIC_CONSOLE_REPLAY 0x00000020
#define SYS_INFO_ALL_BT 0x00000040
#define SYS_INFO_BLOCKED_TASKS 0x00000080
+#define SYS_INFO_MODULES 0x00000100
+#define SYS_INFO_IRQTRACE 0x00000200
+#define SYS_INFO_STACK 0x00000400
void sys_info(unsigned long si_mask);
+unsigned long sys_info_effective_mask(unsigned long mask);
+unsigned long sys_info_filter_and_set(unsigned long si_mask);
+void sys_info_reset(void);
unsigned long sys_info_parse_param(char *str);
#ifdef CONFIG_SYSCTL
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 6fcc94ce4ca9..7a3738279503 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -557,8 +557,10 @@ static int watchdog(void *dummy)
t = hung_timeout_jiffies(hung_last_checked, interval);
if (t <= 0) {
if (!atomic_xchg(&reset_hung_task, 0) &&
- !hung_detector_suspended)
+ !hung_detector_suspended) {
+ sys_info_reset();
check_hung_uninterruptible_tasks(timeout);
+ }
hung_last_checked = jiffies;
continue;
}
diff --git a/kernel/panic.c b/kernel/panic.c
index 213725b612aa..94ce7a94f118 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -550,8 +550,12 @@ static void panic_trigger_all_cpu_backtrace(void)
*/
static void panic_other_cpus_shutdown(bool crash_kexec)
{
- if (panic_print & SYS_INFO_ALL_BT)
- panic_trigger_all_cpu_backtrace();
+ unsigned long mask = sys_info_effective_mask(panic_print);
+
+ if (mask & SYS_INFO_ALL_BT) {
+ if (sys_info_filter_and_set(SYS_INFO_ALL_BT))
+ panic_trigger_all_cpu_backtrace();
+ }
/*
* Note that smp_send_stop() is the usual SMP shutdown function,
@@ -649,6 +653,7 @@ void vpanic(const char *fmt, va_list args)
panic_this_cpu_backtrace_printed = true;
} else if (IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE)) {
dump_stack();
+ sys_info_filter_and_set(SYS_INFO_STACK);
panic_this_cpu_backtrace_printed = true;
}
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 87dd5e0f6968..3bc6f5fd5380 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -280,8 +280,13 @@ void watchdog_hardlockup_check(unsigned int cpu, struct pt_regs *regs)
trigger_single_cpu_backtrace(cpu);
}
+ /* Mark what we already printed so panic() won't repeat it. */
+ sys_info_filter_and_set(SYS_INFO_MODULES | SYS_INFO_IRQTRACE |
+ SYS_INFO_STACK);
+
if (hardlockup_all_cpu_backtrace) {
- trigger_allbutcpu_cpu_backtrace(cpu);
+ if (sys_info_filter_and_set(SYS_INFO_ALL_BT))
+ trigger_allbutcpu_cpu_backtrace(cpu);
if (!hardlockup_panic)
clear_bit_unlock(0, &hard_lockup_nmi_warn);
}
@@ -289,6 +294,8 @@ void watchdog_hardlockup_check(unsigned int cpu, struct pt_regs *regs)
sys_info(hardlockup_si_mask & ~SYS_INFO_ALL_BT);
if (hardlockup_panic)
nmi_panic(regs, "Hard LOCKUP");
+ else
+ sys_info_reset();
per_cpu(watchdog_hardlockup_warned, cpu) = true;
}
@@ -792,6 +799,8 @@ static int softlockup_fn(void *data)
}
/* watchdog kicker functions */
+static DEFINE_PER_CPU(bool, watchdog_stuck_previously);
+
static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
{
unsigned long touch_ts, period_ts, now;
@@ -864,6 +873,7 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
touch_ts = __this_cpu_read(watchdog_touch_ts);
duration = is_softlockup(touch_ts, period_ts, now);
if (unlikely(duration)) {
+ __this_cpu_write(watchdog_stuck_previously, true);
#ifdef CONFIG_SYSFS
++softlockup_count;
#endif
@@ -893,8 +903,13 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
dump_stack();
printk_cpu_sync_put_irqrestore(flags);
+ /* Mark what we already printed so panic() won't repeat it. */
+ sys_info_filter_and_set(SYS_INFO_MODULES | SYS_INFO_IRQTRACE |
+ SYS_INFO_STACK);
+
if (softlockup_all_cpu_backtrace) {
- trigger_allbutcpu_cpu_backtrace(smp_processor_id());
+ if (sys_info_filter_and_set(SYS_INFO_ALL_BT))
+ trigger_allbutcpu_cpu_backtrace(smp_processor_id());
if (!softlockup_panic)
clear_bit_unlock(0, &soft_lockup_nmi_warn);
}
@@ -905,6 +920,11 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
if (softlockup_panic && thresh_count >= softlockup_panic)
panic("softlockup: hung tasks");
+ } else {
+ if (__this_cpu_read(watchdog_stuck_previously)) {
+ __this_cpu_write(watchdog_stuck_previously, false);
+ sys_info_reset();
+ }
}
return HRTIMER_RESTART;
diff --git a/lib/Makefile b/lib/Makefile
index 7f75cc6edf94..521644a140c8 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -40,7 +40,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
nmi_backtrace.o win_minmax.o memcat_p.o \
- buildid.o objpool.o iomem_copy.o sys_info.o
+ buildid.o objpool.o iomem_copy.o sys_info.o sys_info_filter.o
lib-$(CONFIG_UNION_FIND) += union_find.o
lib-$(CONFIG_PRINTK) += dump_stack.o
diff --git a/lib/sys_info.c b/lib/sys_info.c
index f32a06ec9ed4..e188c5d924cb 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -2,12 +2,9 @@
#include <linux/array_size.h>
#include <linux/bitops.h>
#include <linux/cleanup.h>
-#include <linux/console.h>
#include <linux/log2.h>
#include <linux/kernel.h>
-#include <linux/ftrace.h>
-#include <linux/nmi.h>
-#include <linux/sched/debug.h>
+#include <linux/slab.h>
#include <linux/string.h>
#include <linux/sysctl.h>
@@ -22,6 +19,9 @@ static const char * const si_names[] = {
[ilog2(SYS_INFO_PANIC_CONSOLE_REPLAY)] = "",
[ilog2(SYS_INFO_ALL_BT)] = "all_bt",
[ilog2(SYS_INFO_BLOCKED_TASKS)] = "blocked_tasks",
+ [ilog2(SYS_INFO_MODULES)] = "modules",
+ [ilog2(SYS_INFO_IRQTRACE)] = "irqtrace",
+ [ilog2(SYS_INFO_STACK)] = "stack",
};
/*
@@ -29,7 +29,7 @@ static const char * const si_names[] = {
* If a kernel module calls sys_info() with "parameter == 0", then
* this mask will be used.
*/
-static unsigned long kernel_si_mask;
+unsigned long kernel_si_mask;
/* Expecting string like "xxx_sys_info=tasks,mem,timers,locks,ftrace,..." */
unsigned long sys_info_parse_param(char *str)
@@ -136,31 +136,7 @@ static int __init sys_info_sysctl_init(void)
subsys_initcall(sys_info_sysctl_init);
#endif
-static void __sys_info(unsigned long si_mask)
+unsigned long sys_info_effective_mask(unsigned long mask)
{
- if (si_mask & SYS_INFO_TASKS)
- show_state();
-
- if (si_mask & SYS_INFO_MEM)
- show_mem();
-
- if (si_mask & SYS_INFO_TIMERS)
- sysrq_timer_list_show();
-
- if (si_mask & SYS_INFO_LOCKS)
- debug_show_all_locks();
-
- if (si_mask & SYS_INFO_FTRACE)
- ftrace_dump(DUMP_ALL);
-
- if (si_mask & SYS_INFO_ALL_BT)
- trigger_all_cpu_backtrace();
-
- if (si_mask & SYS_INFO_BLOCKED_TASKS)
- show_state_filter(TASK_UNINTERRUPTIBLE);
-}
-
-void sys_info(unsigned long si_mask)
-{
- __sys_info(si_mask ? : kernel_si_mask);
+ return mask ? : READ_ONCE(kernel_si_mask);
}
diff --git a/lib/sys_info_filter.c b/lib/sys_info_filter.c
new file mode 100644
index 000000000000..b07b5dc3ce3c
--- /dev/null
+++ b/lib/sys_info_filter.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * sys_info incident diagnostic engine.
+ *
+ * Centralises the dispatching and deduplication of system diagnostic
+ * output (task lists, memory info, backtraces, module lists, etc.)
+ * that is requested during lockups, hung tasks, and panics.
+ *
+ * A global bitmask tracks which categories have already been printed
+ * during the current incident so that duplicate output is suppressed
+ * when multiple subsystems request the same diagnostics (e.g. a
+ * watchdog fires, prints sys_info, then calls panic() which tries
+ * to print sys_info again).
+ */
+#include <linux/console.h>
+#include <linux/ftrace.h>
+#include <linux/kernel.h>
+#include <linux/lockdep.h>
+#include <linux/module.h>
+#include <linux/nmi.h>
+#include <linux/sched/debug.h>
+
+#include <linux/sys_info.h>
+
+/*
+ * Bitmask of sys_info categories already printed during the current
+ * incident. Accessed locklessly via cmpxchg — races between CPUs
+ * during a lockup are tolerable because the output from different
+ * watchdogs has never been synchronised, and panic() never returns.
+ */
+static unsigned long sys_info_printed;
+
+/**
+ * sys_info_filter_and_set - atomically claim unprinted sys_info bits
+ * @si_mask: requested diagnostics bitmask
+ *
+ * Returns the subset of @si_mask that has NOT been printed yet during
+ * the current incident and marks those bits as printed. Returns 0
+ * if everything requested was already printed.
+ */
+unsigned long sys_info_filter_and_set(unsigned long si_mask)
+{
+ unsigned long old, new;
+
+ if (!si_mask)
+ return 0;
+
+ do {
+ old = READ_ONCE(sys_info_printed);
+ if (!(si_mask & ~old))
+ return 0;
+ new = old | si_mask;
+ } while (cmpxchg(&sys_info_printed, old, new) != old);
+
+ return si_mask & ~old;
+}
+
+/**
+ * sys_info_reset - clear the printed-state bitmask
+ *
+ * Called when an incident is over (lockup recovered, hung-task check
+ * round starts fresh) so that subsequent incidents produce output.
+ */
+void sys_info_reset(void)
+{
+ WRITE_ONCE(sys_info_printed, 0);
+}
+
+/*
+ * Dispatch the actual diagnostic output for each bit in @si_mask.
+ */
+static void __sys_info(unsigned long si_mask)
+{
+ if (si_mask & SYS_INFO_TASKS)
+ show_state();
+
+ if (si_mask & SYS_INFO_MEM)
+ show_mem();
+
+ if (si_mask & SYS_INFO_TIMERS)
+ sysrq_timer_list_show();
+
+ if (si_mask & SYS_INFO_LOCKS)
+ debug_show_all_locks();
+
+ if (si_mask & SYS_INFO_FTRACE)
+ ftrace_dump(DUMP_ALL);
+
+ if (si_mask & SYS_INFO_ALL_BT)
+ trigger_all_cpu_backtrace();
+
+ if (si_mask & SYS_INFO_BLOCKED_TASKS)
+ show_state_filter(TASK_UNINTERRUPTIBLE);
+
+ if (si_mask & SYS_INFO_MODULES)
+ print_modules();
+
+ if (si_mask & SYS_INFO_IRQTRACE)
+ print_irqtrace_events(current);
+
+ if (si_mask & SYS_INFO_STACK)
+ dump_stack();
+}
+
+/**
+ * sys_info - print system diagnostics, suppressing duplicates
+ * @si_mask: requested diagnostics bitmask (0 = use kernel_si_info default)
+ *
+ * Resolves the effective mask (falling back to the kernel-wide default
+ * when @si_mask is 0), filters out anything already printed during this
+ * incident, and dispatches the remaining diagnostics.
+ */
+void sys_info(unsigned long si_mask)
+{
+ unsigned long mask = sys_info_effective_mask(si_mask);
+ unsigned long filtered = sys_info_filter_and_set(mask);
+
+ if (filtered)
+ __sys_info(filtered);
+}
--
2.53.0
NOTE!!: This is AI generated!! This **MAY** not be the finished product,
this is ONLY the model!
Thanks!
^ permalink raw reply related
* [PATCH v3 1/8] ibmvfc: add basic FPIN support
From: Dave Marquardt via B4 Relay @ 2026-07-02 17:59 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
In-Reply-To: <20260702-ibmvfc-fpin-support-v3-0-d95b9547cf88@linux.ibm.com>
From: Dave Marquardt <davemarq@linux.ibm.com>
Add support for basic FPIN messages to the ibmvfc driver. This includes
- adding FPIN handling support to the async event handler
- offloading processing of FPIN messages to a work queue
- converting the VIOS FPIN message to a struct fc_els_fpin as used by
the Linux kernel
- passing the converted struct fc_els_fpin to fc_host_fpin_rcv for
processing
The FPIN message conversion routines include a common routine that
will also be used in patches 6 and 8, which add full and extended FPIN
support.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/Kconfig | 10 ++
drivers/scsi/ibmvscsi/Makefile | 1 +
drivers/scsi/ibmvscsi/ibmvfc.c | 255 ++++++++++++++++++++++++++++++++++-
drivers/scsi/ibmvscsi/ibmvfc.h | 16 +++
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 131 ++++++++++++++++++
5 files changed, 406 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index c3042393af23..d5fc7eb2ebb1 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -758,6 +758,16 @@ config SCSI_IBMVFC
To compile this driver as a module, choose M here: the
module will be called ibmvfc.
+config SCSI_IBMVFC_KUNIT_TEST
+ tristate "KUnit tests for the IBM POWER Virtual FC Client" if !KUNIT_ALL_TESTS
+ depends on SCSI_IBMVFC && KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Compile IBM POWER Virtual FC client KUnit tests. These tests
+ specifically test FPIN functionality. To compile this driver
+ as a module, choose M here: the module will be called
+ ibmvfc_kunit.
+
config SCSI_IBMVFC_TRACE
bool "enable driver internal trace"
depends on SCSI_IBMVFC
diff --git a/drivers/scsi/ibmvscsi/Makefile b/drivers/scsi/ibmvscsi/Makefile
index 5eb1cb1a0028..75dc7aee15a0 100644
--- a/drivers/scsi/ibmvscsi/Makefile
+++ b/drivers/scsi/ibmvscsi/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_SCSI_IBMVSCSI) += ibmvscsi.o
obj-$(CONFIG_SCSI_IBMVFC) += ibmvfc.o
+obj-$(CONFIG_SCSI_IBMVFC_KUNIT_TEST) += ibmvfc_kunit.o
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 3dd2adda195e..b63f41a6cfff 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -30,6 +30,9 @@
#include <scsi/scsi_tcq.h>
#include <scsi/scsi_transport_fc.h>
#include <scsi/scsi_bsg_fc.h>
+#include <kunit/visibility.h>
+#include <scsi/fc/fc_els.h>
+#include <linux/overflow.h>
#include "ibmvfc.h"
static unsigned int init_timeout = IBMVFC_INIT_TIMEOUT;
@@ -3137,6 +3140,7 @@ static const struct ibmvfc_async_desc ae_desc [] = {
{ "Halt", IBMVFC_AE_HALT, IBMVFC_DEFAULT_LOG_LEVEL },
{ "Resume", IBMVFC_AE_RESUME, IBMVFC_DEFAULT_LOG_LEVEL },
{ "Adapter Failed", IBMVFC_AE_ADAPTER_FAILED, IBMVFC_DEFAULT_LOG_LEVEL },
+ { "FPIN", IBMVFC_AE_FPIN, IBMVFC_DEFAULT_LOG_LEVEL },
};
static const struct ibmvfc_async_desc unknown_ae = {
@@ -3185,16 +3189,228 @@ static const char *ibmvfc_get_link_state(enum ibmvfc_ae_link_state state)
return "";
}
+#define IBMVFC_FPIN_CONGN_DESC_SZ (sizeof(struct fc_els_fpin) + sizeof(struct fc_fn_congn_desc))
+#define IBMVFC_FPIN_LI_DESC_SZ (sizeof(struct fc_els_fpin) + \
+ struct_size_t(struct fc_fn_li_desc, pname_list, 1))
+#define IBMVFC_FPIN_PEER_CONGN_DESC_SZ (sizeof(struct fc_els_fpin) + \
+ struct_size_t(struct fc_fn_peer_congn_desc, pname_list, 1))
+
+/**
+ * ibmvfc_fpin_size_helper(): compute fpin structure size based on fpin status
+ * @fpin_status: status value
+ *
+ * Return:
+ * 0: invalid fpin_status
+ * other: valid size
+ */
+static size_t ibmvfc_fpin_size_helper(u8 fpin_status)
+{
+ size_t size = 0;
+
+ switch (fpin_status) {
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ size = IBMVFC_FPIN_CONGN_DESC_SZ;
+ break;
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ size = IBMVFC_FPIN_PEER_CONGN_DESC_SZ;
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ size = IBMVFC_FPIN_LI_DESC_SZ;
+ break;
+ default:
+ break;
+ }
+
+ return size;
+}
+
+/**
+ * ibmvfc_common_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
+ * containing a descriptor.
+ *
+ * Allocate a struct fc_els_fpin containing a descriptor and populate
+ * based on data from *ibmvfc_fpin.
+ *
+ * Return:
+ * NULL - unable to allocate structure
+ * non-NULL - pointer to populated struct fc_els_fpin
+ */
+static struct fc_els_fpin *
+ibmvfc_common_fpin_to_desc(u8 fpin_status, __be64 wwpn, __be16 type, __be16 modifier,
+ __be32 threshold, __be32 event_count)
+{
+ struct fc_fn_peer_congn_desc *pdesc;
+ struct fc_fn_congn_desc *cdesc;
+ struct fc_fn_li_desc *ldesc;
+ struct fc_els_fpin *fpin;
+ size_t size;
+
+ size = ibmvfc_fpin_size_helper(fpin_status);
+ if (!size)
+ return NULL;
+
+ fpin = kzalloc(size, GFP_KERNEL);
+ if (!fpin)
+ return NULL;
+
+ fpin->fpin_cmd = ELS_FPIN;
+
+ switch (fpin_status) {
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ fpin->desc_len = cpu_to_be32(sizeof(struct fc_fn_congn_desc));
+ cdesc = (struct fc_fn_congn_desc *)fpin->fpin_desc;
+ cdesc->desc_tag = cpu_to_be32(ELS_DTAG_CONGESTION);
+ cdesc->desc_len = cpu_to_be32(FC_TLV_DESC_LENGTH_FROM_SZ(*cdesc));
+ cdesc->event_type = type;
+ cdesc->event_modifier = modifier;
+ cdesc->event_period = cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_PERIOD);
+ cdesc->severity = FPIN_CONGN_SEVERITY_WARNING;
+ break;
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ fpin->desc_len =
+ cpu_to_be32(struct_size_t(struct fc_fn_peer_congn_desc, pname_list, 1));
+ pdesc = (struct fc_fn_peer_congn_desc *)fpin->fpin_desc;
+ pdesc->desc_tag = cpu_to_be32(ELS_DTAG_PEER_CONGEST);
+ pdesc->desc_len = cpu_to_be32(struct_size_t(struct fc_fn_peer_congn_desc,
+ pname_list, 1) - FC_TLV_DESC_HDR_SZ);
+ pdesc->event_type = type;
+ pdesc->event_modifier = modifier;
+ pdesc->event_period = cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_PERIOD);
+ pdesc->attached_wwpn = wwpn;
+ pdesc->pname_count = cpu_to_be32(1);
+ pdesc->pname_list[0] = wwpn;
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ fpin->desc_len = cpu_to_be32(struct_size_t(struct fc_fn_li_desc, pname_list, 1));
+ ldesc = (struct fc_fn_li_desc *)fpin->fpin_desc;
+ ldesc->desc_tag = cpu_to_be32(ELS_DTAG_LNK_INTEGRITY);
+ ldesc->desc_len = cpu_to_be32(struct_size_t(struct fc_fn_li_desc,
+ pname_list, 1) - FC_TLV_DESC_HDR_SZ);
+ ldesc->event_type = type;
+ ldesc->event_modifier = modifier;
+ ldesc->event_threshold = threshold;
+ ldesc->event_count = event_count;
+ ldesc->attached_wwpn = wwpn;
+ ldesc->pname_count = cpu_to_be32(1);
+ ldesc->pname_list[0] = wwpn;
+ break;
+ default:
+ /* This should be caught above. */
+ kfree(fpin);
+ fpin = NULL;
+ break;
+ }
+
+ return fpin;
+}
+
+/**
+ * ibmvfc_basic_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
+ * containing a descriptor.
+ * @ibmvfc_fpin: Pointer to async crq
+ *
+ * Allocate a struct fc_els_fpin containing a descriptor and populate
+ * based on data from *ibmvfc_fpin.
+ *
+ * Return:
+ * NULL - unable to allocate structure
+ * non-NULL - pointer to populated struct fc_els_fpin
+ */
+static struct fc_els_fpin *
+ibmvfc_basic_fpin_to_desc(struct ibmvfc_async_crq *crq, u64 wwpn)
+{
+ __be16 type;
+
+ switch (crq->fpin_status) {
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ type = cpu_to_be16(FPIN_CONGN_DEVICE_SPEC);
+ break;
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ type = cpu_to_be16(FPIN_CONGN_CLEAR);
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ type = cpu_to_be16(FPIN_LI_UNKNOWN);
+ break;
+ default:
+ return (NULL);
+ }
+
+ return ibmvfc_common_fpin_to_desc(crq->fpin_status, cpu_to_be64(wwpn),
+ type, cpu_to_be16(0),
+ cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD),
+ cpu_to_be32(1));
+}
+
+/**
+ * ibmvfc_process_async_work - Process IBMVFC_AE_FPIN async CRQ from work queue
+ * @work: pointer to work_struct
+ */
+static void ibmvfc_process_async_work(struct work_struct *work)
+{
+ struct ibmvfc_target *tgt, *next;
+ struct ibmvfc_async_work *aw;
+ struct ibmvfc_async_crq *crq;
+ struct ibmvfc_host *vhost;
+ struct fc_els_fpin *fpin;
+ unsigned long flags;
+
+ aw = container_of(work, struct ibmvfc_async_work, async_work_s);
+ crq = &aw->crq;
+ vhost = aw->vhost;
+
+ if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
+ goto end;
+
+ spin_lock_irqsave(vhost->host->host_lock, flags);
+ list_for_each_entry_safe(tgt, next, &vhost->targets, queue) {
+ if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
+ continue;
+ if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
+ continue;
+ if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
+ continue;
+ if (!tgt->rport)
+ continue;
+ break;
+ }
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+
+ if (!tgt || !tgt->rport) {
+ dev_err_ratelimited(vhost->dev, "Invalid target for FPIN\n");
+ goto end;
+ }
+
+ fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
+ if (fpin) {
+ fc_host_fpin_rcv(tgt->vhost->host,
+ sizeof(*fpin) + be32_to_cpu(fpin->desc_len),
+ (char *)fpin, 0);
+ kfree(fpin);
+ } else
+ dev_err_ratelimited(vhost->dev,
+ "FPIN event %u received, unable to process\n",
+ crq->fpin_status);
+ end:
+ kfree(aw);
+}
+
/**
* ibmvfc_handle_async - Handle an async event from the adapter
* @crq: crq to process
* @vhost: ibmvfc host struct
*
**/
-static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
- struct ibmvfc_host *vhost)
+VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
+ struct ibmvfc_host *vhost)
{
const struct ibmvfc_async_desc *desc = ibmvfc_get_ae_desc(be64_to_cpu(crq->event));
+ struct ibmvfc_async_work *aw;
struct ibmvfc_target *tgt;
ibmvfc_log(vhost, desc->log_level, "%s event received. scsi_id: %llx, wwpn: %llx,"
@@ -3269,11 +3485,23 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
case IBMVFC_AE_HALT:
ibmvfc_link_down(vhost, IBMVFC_HALTED);
break;
+ case IBMVFC_AE_FPIN:
+ aw = kzalloc(sizeof(struct ibmvfc_async_work), GFP_ATOMIC);
+ if (aw) {
+ INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
+ aw->vhost = vhost;
+ aw->crq = *crq;
+ queue_work(vhost->fpin_workq, &aw->async_work_s);
+ } else
+ dev_err_ratelimited(vhost->dev,
+ "can't offload async CRQ to work queue\n");
+ break;
default:
dev_err(vhost->dev, "Unknown async event received: %lld\n", crq->event);
break;
}
}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_handle_async);
/**
* ibmvfc_handle_crq - Handles and frees received events in the CRQ
@@ -3803,8 +4031,6 @@ static void ibmvfc_tasklet(void *data)
/* Pull all the valid messages off the async CRQ */
while ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
ibmvfc_handle_async(async, vhost);
- async->valid = 0;
- wmb();
}
/* Pull all the valid messages off the CRQ */
@@ -3818,8 +4044,6 @@ static void ibmvfc_tasklet(void *data)
if ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
vio_disable_interrupts(vdev);
ibmvfc_handle_async(async, vhost);
- async->valid = 0;
- wmb();
} else if ((crq = ibmvfc_next_crq(vhost)) != NULL) {
vio_disable_interrupts(vdev);
ibmvfc_handle_crq(crq, vhost, &evt_doneq);
@@ -6364,9 +6588,15 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
INIT_WORK(&vhost->rport_add_work_q, ibmvfc_rport_add_thread);
mutex_init(&vhost->passthru_mutex);
- if ((rc = ibmvfc_alloc_mem(vhost)))
+ vhost->fpin_workq = alloc_workqueue("%s-fpin-workq-%u", 0, 0,
+ IBMVFC_NAME, shost->host_no);
+ if (vhost->fpin_workq == NULL)
goto free_scsi_host;
+ rc = ibmvfc_alloc_mem(vhost);
+ if (rc)
+ goto free_workq;
+
vhost->work_thread = kthread_run(ibmvfc_work, vhost, "%s_%d", IBMVFC_NAME,
shost->host_no);
@@ -6412,6 +6642,9 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
kthread_stop(vhost->work_thread);
free_host_mem:
ibmvfc_free_mem(vhost);
+free_workq:
+ destroy_workqueue(vhost->fpin_workq);
+ vhost->fpin_workq = NULL;
free_scsi_host:
scsi_host_put(shost);
out:
@@ -6453,6 +6686,8 @@ static void ibmvfc_remove(struct vio_dev *vdev)
ibmvfc_release_crq_queue(vhost);
ibmvfc_free_mem(vhost);
+ destroy_workqueue(vhost->fpin_workq);
+ vhost->fpin_workq = NULL;
spin_lock(&ibmvfc_driver_lock);
list_del(&vhost->queue);
spin_unlock(&ibmvfc_driver_lock);
@@ -6603,5 +6838,11 @@ static void __exit ibmvfc_module_exit(void)
fc_release_transport(ibmvfc_transport_template);
}
+VISIBLE_IF_KUNIT struct list_head *ibmvfc_get_headp(void)
+{
+ return &ibmvfc_head;
+}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_get_headp);
+
module_init(ibmvfc_module_init);
module_exit(ibmvfc_module_exit);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index c73ed2314ad0..f69e0605a78d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -671,8 +671,12 @@ enum ibmvfc_ae_fpin_status {
IBMVFC_AE_FPIN_PORT_CONGESTED = 0x2,
IBMVFC_AE_FPIN_PORT_CLEARED = 0x3,
IBMVFC_AE_FPIN_PORT_DEGRADED = 0x4,
+ IBMVFC_AE_FPIN_CONGESTION_CLEARED = 0x5,
};
+#define IBMVFC_FPIN_DEFAULT_EVENT_PERIOD (5*60*MSEC_PER_SEC) /* 5 minutes */
+#define IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD (5*60*MSEC_PER_SEC/2) /* 2.5 minutes */
+
struct ibmvfc_async_crq {
volatile u8 valid;
u8 link_state;
@@ -686,6 +690,12 @@ struct ibmvfc_async_crq {
__be64 reserved;
} __packed __aligned(8);
+struct ibmvfc_async_work {
+ struct ibmvfc_host *vhost;
+ struct ibmvfc_async_crq crq;
+ struct work_struct async_work_s;
+};
+
union ibmvfc_iu {
struct ibmvfc_mad_common mad_common;
struct ibmvfc_npiv_login_mad npiv_login;
@@ -914,6 +924,7 @@ struct ibmvfc_host {
struct work_struct rport_add_work_q;
wait_queue_head_t init_wait_q;
wait_queue_head_t work_wait_q;
+ struct workqueue_struct *fpin_workq;
};
#define DBG_CMD(CMD) do { if (ibmvfc_debug) CMD; } while (0)
@@ -953,4 +964,9 @@ struct ibmvfc_host {
#define ibmvfc_remove_trace_file(kobj, attr) do { } while (0)
#endif
+#ifdef VISIBLE_IF_KUNIT
+VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq, struct ibmvfc_host *vhost);
+VISIBLE_IF_KUNIT struct list_head *ibmvfc_get_headp(void);
+#endif
+
#endif
diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
new file mode 100644
index 000000000000..e41e2a49e549
--- /dev/null
+++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <kunit/test.h>
+#include <kunit/visibility.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_transport_fc.h>
+#include <linux/list.h>
+#include <linux/delay.h>
+#include "ibmvfc.h"
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+
+/**
+ * ibmvfc_async_fpin_event_test - unit test for IBMVFC_AE_FPIN parts of
+ * ibmvfc_handle_async
+ * @test: pointer to kunit structure
+ *
+ * Tests
+ * - error returns from ibmvfc_handle_async
+ * - statistics updates
+ *
+ * Return: void
+ */
+static void ibmvfc_async_fpin_test(struct kunit *test)
+{
+ u64 post[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ u64 pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ enum ibmvfc_ae_fpin_status fs;
+ struct fc_host_attrs *fc_host;
+ struct ibmvfc_async_crq crq[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ struct ibmvfc_target *tgt;
+ struct ibmvfc_host *vhost;
+ struct list_head *queue;
+ struct list_head *headp;
+
+ headp = ibmvfc_get_headp();
+ KUNIT_ASSERT_FALSE_MSG(test, list_empty(headp), "No ibmvfc devices available\n");
+ queue = headp->next;
+ vhost = container_of(queue, struct ibmvfc_host, queue);
+
+ KUNIT_ASSERT_GE_MSG(test, vhost->num_targets, 1, "No targets");
+ tgt = list_first_entry(&vhost->targets, struct ibmvfc_target, queue);
+ KUNIT_EXPECT_NOT_NULL(test, tgt->rport);
+
+ fc_host = shost_to_fc_host(vhost->host);
+
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ for (fs = IBMVFC_AE_FPIN_LINK_CONGESTED; fs <= IBMVFC_AE_FPIN_CONGESTION_CLEARED; fs++) {
+ crq[fs].valid = 0x80;
+ crq[fs].link_state = IBMVFC_AE_LS_LINK_UP;
+ crq[fs].fpin_status = fs;
+ crq[fs].event = cpu_to_be64(IBMVFC_AE_FPIN);
+ crq[fs].scsi_id = cpu_to_be64(tgt->scsi_id);
+ crq[fs].wwpn = cpu_to_be64(tgt->wwpn);
+ crq[fs].node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_handle_async(&crq[fs], vhost);
+ }
+
+ msleep(500U);
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ post[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_LINK_CONGESTED],
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_PORT_CONGESTED],
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_PORT_CLEARED],
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_PORT_DEGRADED],
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_CONGESTION_CLEARED],
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED]+1);
+
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ /* bad path */
+ crq[0].valid = 0x80;
+ crq[0].link_state = IBMVFC_AE_LS_LINK_UP;
+ crq[0].fpin_status = 0; /* bad value */
+ crq[0].event = cpu_to_be64(IBMVFC_AE_FPIN);
+ crq[0].scsi_id = cpu_to_be64(tgt->scsi_id);
+ crq[0].wwpn = cpu_to_be64(tgt->wwpn);
+ crq[0].node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_handle_async(&crq[0], vhost);
+
+ msleep(500U);
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ post[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_LINK_CONGESTED],
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_PORT_CONGESTED],
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_PORT_CLEARED],
+ post[IBMVFC_AE_FPIN_PORT_CLEARED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_PORT_DEGRADED],
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED],
+ post[IBMVFC_AE_FPIN_CONGESTION_CLEARED]);
+}
+
+static struct kunit_case ibmvfc_fpin_test_cases[] = {
+ KUNIT_CASE_SLOW(ibmvfc_async_fpin_test),
+ {},
+};
+
+static struct kunit_suite ibmvfc_fpin_test_suite = {
+ .name = "ibmvfc-fpin-test",
+ .test_cases = ibmvfc_fpin_test_cases,
+};
+kunit_test_init_section_suite(ibmvfc_fpin_test_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dave Marquardt <davemarq@linux.ibm.com>");
+MODULE_DESCRIPTION("Test module for IBM Virtual Fibre Channel Driver");
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).