Re: [PATCH v2 2/8] powerpc: Prepare to build with generic entry/exit framework

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Le 14/12/2025 à 14:02, Mukesh Kumar Chaurasiya a écrit :
> From: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
> 
> This patch introduces preparatory changes needed to support building
> PowerPC with the generic entry/exit (irqentry) framework.
> 
> The following infrastructure updates are added:
>   - Add a syscall_work field to struct thread_info to hold SYSCALL_WORK_* flags.
>   - Provide a stub implementation of arch_syscall_is_vdso_sigreturn(),
>     returning false for now.
>   - Introduce on_thread_stack() helper to detect if the current stack pointer
>     lies within the task’s kernel stack.
> 
> These additions enable later integration with the generic entry/exit
> infrastructure while keeping existing PowerPC behavior unchanged.
> 
> No functional change is intended in this patch.
> 
> Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
> ---
>   arch/powerpc/include/asm/entry-common.h | 11 +++++++++++
>   arch/powerpc/include/asm/stacktrace.h   |  6 ++++++
>   arch/powerpc/include/asm/syscall.h      |  5 +++++
>   arch/powerpc/include/asm/thread_info.h  |  1 +
>   4 files changed, 23 insertions(+)
>   create mode 100644 arch/powerpc/include/asm/entry-common.h
> 
> diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
> new file mode 100644
> index 000000000000..3af16d821d07
> --- /dev/null
> +++ b/arch/powerpc/include/asm/entry-common.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_PPC_ENTRY_COMMON_H
> +#define _ASM_PPC_ENTRY_COMMON_H
> +
> +#ifdef CONFIG_GENERIC_IRQ_ENTRY

Why do you need this #ifdef ? I see no reason, the build works well 
without this #ifdef.

At the time being, CONFIG_GENERIC_IRQ_ENTRY is never selected by 
powerpc, meaning you are introducing dead code. If really needed it 
would be more explicit to add a "#if 0"

> +
> +#include <asm/stacktrace.h>
> +
> +#endif /* CONFIG_GENERIC_IRQ_ENTRY */
> +#endif /* _ASM_PPC_ENTRY_COMMON_H */
> diff --git a/arch/powerpc/include/asm/stacktrace.h b/arch/powerpc/include/asm/stacktrace.h
> index 6149b53b3bc8..a81a9373d723 100644
> --- a/arch/powerpc/include/asm/stacktrace.h
> +++ b/arch/powerpc/include/asm/stacktrace.h
> @@ -10,4 +10,10 @@
>   
>   void show_user_instructions(struct pt_regs *regs);
>   
> +static inline bool on_thread_stack(void)

Shouldn't it be __always_inline ?

> +{
> +	return !(((unsigned long)(current->stack) ^ current_stack_pointer)
> +			& ~(THREAD_SIZE - 1));
> +}
> +
>   #endif /* _ASM_POWERPC_STACKTRACE_H */
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index 4b3c52ed6e9d..834fcc4f7b54 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -139,4 +139,9 @@ static inline int syscall_get_arch(struct task_struct *task)
>   	else
>   		return AUDIT_ARCH_PPC64;
>   }
> +
> +static inline bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs)
> +{
> +	return false;
> +}
>   #endif	/* _ASM_SYSCALL_H */
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index b0f200aba2b3..9c8270354f0b 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -57,6 +57,7 @@ struct thread_info {
>   #ifdef CONFIG_SMP
>   	unsigned int	cpu;
>   #endif
> +	unsigned long	syscall_work;		/* SYSCALL_WORK_ flags */

This is not used, why add it here ?

>   	unsigned long	local_flags;		/* private flags for thread */
>   #ifdef CONFIG_LIVEPATCH_64
>   	unsigned long *livepatch_sp;