From: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>

Add PowerPC-specific implementations of the generic syscall exit hooks
used by the generic entry/exit framework:

 - arch_exit_to_user_mode_work_prepare()
 - arch_exit_to_user_mode_work()

These helpers handle user state restoration when returning from the
kernel to userspace, including FPU/VMX/VSX state, transactional memory,
KUAP restore, and per-CPU accounting.

Additionally, move check_return_regs_valid() from interrupt.c to
interrupt.h so it can be shared by the new entry/exit logic, and add
arch_do_signal_or_restart() for use with the generic entry flow.

No functional change is intended with this patch.

Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
---
 arch/powerpc/include/asm/entry-common.h | 49 +++++++++++++++
 arch/powerpc/include/asm/interrupt.h    | 82 +++++++++++++++++++++++++
 arch/powerpc/kernel/interrupt.c         | 81 ------------------------
 arch/powerpc/kernel/signal.c            | 14 +++++
 4 files changed, 145 insertions(+), 81 deletions(-)

diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
index 093ece06ef79..e8ebd42a4e6d 100644
--- a/arch/powerpc/include/asm/entry-common.h
+++ b/arch/powerpc/include/asm/entry-common.h
@@ -8,6 +8,7 @@
 #include <asm/cputime.h>
 #include <asm/interrupt.h>
 #include <asm/stacktrace.h>
+#include <asm/switch_to.h>
 #include <asm/tm.h>
 
 static __always_inline void arch_enter_from_user_mode(struct pt_regs *regs)
@@ -104,5 +105,53 @@ static __always_inline void arch_enter_from_user_mode(struct pt_regs *regs)
 
 #define arch_enter_from_user_mode arch_enter_from_user_mode
 
+static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
+						  unsigned long ti_work)
+{
+	unsigned long mathflags;
+
+	if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && IS_ENABLED(CONFIG_PPC_FPU)) {
+		if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
+		    unlikely((ti_work & _TIF_RESTORE_TM))) {
+			restore_tm_state(regs);
+		} else {
+			mathflags = MSR_FP;
+
+			if (cpu_has_feature(CPU_FTR_VSX))
+				mathflags |= MSR_VEC | MSR_VSX;
+			else if (cpu_has_feature(CPU_FTR_ALTIVEC))
+				mathflags |= MSR_VEC;
+
+			/*
+			 * If userspace MSR has all available FP bits set,
+			 * then they are live and no need to restore. If not,
+			 * it means the regs were given up and restore_math
+			 * may decide to restore them (to avoid taking an FP
+			 * fault).
+			 */
+			if ((regs->msr & mathflags) != mathflags)
+				restore_math(regs);
+		}
+	}
+
+	check_return_regs_valid(regs);
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	local_paca->tm_scratch = regs->msr;
+#endif
+	/* Restore user access locks last */
+	kuap_user_restore(regs);
+}
+
+#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
+
+static __always_inline void arch_exit_to_user_mode(void)
+{
+	booke_load_dbcr0();
+
+	account_cpu_user_exit();
+}
+
+#define arch_exit_to_user_mode arch_exit_to_user_mode
+
 #endif /* CONFIG_GENERIC_IRQ_ENTRY */
 #endif /* _ASM_PPC_ENTRY_COMMON_H */
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index ca8a2cda9400..77ff8e33f8cd 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -68,6 +68,8 @@
 
 #include <linux/context_tracking.h>
 #include <linux/hardirq.h>
+#include <linux/sched/debug.h> /* for show_regs */
+
 #include <asm/cputime.h>
 #include <asm/firmware.h>
 #include <asm/ftrace.h>
@@ -172,6 +174,86 @@ static inline void booke_restore_dbcr0(void)
 #endif
 }
 
+static inline void check_return_regs_valid(struct pt_regs *regs)
+{
+#ifdef CONFIG_PPC_BOOK3S_64
+	unsigned long trap, srr0, srr1;
+	static bool warned;
+	u8 *validp;
+	char *h;
+
+	if (trap_is_scv(regs))
+		return;
+
+	trap = TRAP(regs);
+	// EE in HV mode sets HSRRs like 0xea0
+	if (cpu_has_feature(CPU_FTR_HVMODE) && trap == INTERRUPT_EXTERNAL)
+		trap = 0xea0;
+
+	switch (trap) {
+	case 0x980:
+	case INTERRUPT_H_DATA_STORAGE:
+	case 0xe20:
+	case 0xe40:
+	case INTERRUPT_HMI:
+	case 0xe80:
+	case 0xea0:
+	case INTERRUPT_H_FAC_UNAVAIL:
+	case 0x1200:
+	case 0x1500:
+	case 0x1600:
+	case 0x1800:
+		validp = &local_paca->hsrr_valid;
+		if (!READ_ONCE(*validp))
+			return;
+
+		srr0 = mfspr(SPRN_HSRR0);
+		srr1 = mfspr(SPRN_HSRR1);
+		h = "H";
+
+		break;
+	default:
+		validp = &local_paca->srr_valid;
+		if (!READ_ONCE(*validp))
+			return;
+
+		srr0 = mfspr(SPRN_SRR0);
+		srr1 = mfspr(SPRN_SRR1);
+		h = "";
+		break;
+	}
+
+	if (srr0 == regs->nip && srr1 == regs->msr)
+		return;
+
+	/*
+	 * A NMI / soft-NMI interrupt may have come in after we found
+	 * srr_valid and before the SRRs are loaded. The interrupt then
+	 * comes in and clobbers SRRs and clears srr_valid. Then we load
+	 * the SRRs here and test them above and find they don't match.
+	 *
+	 * Test validity again after that, to catch such false positives.
+	 *
+	 * This test in general will have some window for false negatives
+	 * and may not catch and fix all such cases if an NMI comes in
+	 * later and clobbers SRRs without clearing srr_valid, but hopefully
+	 * such things will get caught most of the time, statistically
+	 * enough to be able to get a warning out.
+	 */
+	if (!READ_ONCE(*validp))
+		return;
+
+	if (!data_race(warned)) {
+		data_race(warned = true);
+		pr_warn("%sSRR0 was: %lx should be: %lx\n", h, srr0, regs->nip);
+		pr_warn("%sSRR1 was: %lx should be: %lx\n", h, srr1, regs->msr);
+		show_regs(regs);
+	}
+
+	WRITE_ONCE(*validp, 0); /* fixup */
+#endif
+}
+
 static inline void interrupt_enter_prepare(struct pt_regs *regs)
 {
 #ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 2a09ac5dabd6..f53d432f6087 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -4,7 +4,6 @@
 #include <linux/err.h>
 #include <linux/compat.h>
 #include <linux/rseq.h>
-#include <linux/sched/debug.h> /* for show_regs */
 
 #include <asm/kup.h>
 #include <asm/cputime.h>
@@ -78,86 +77,6 @@ static notrace __always_inline bool prep_irq_for_enabled_exit(bool restartable)
 	return true;
 }
 
-static notrace void check_return_regs_valid(struct pt_regs *regs)
-{
-#ifdef CONFIG_PPC_BOOK3S_64
-	unsigned long trap, srr0, srr1;
-	static bool warned;
-	u8 *validp;
-	char *h;
-
-	if (trap_is_scv(regs))
-		return;
-
-	trap = TRAP(regs);
-	// EE in HV mode sets HSRRs like 0xea0
-	if (cpu_has_feature(CPU_FTR_HVMODE) && trap == INTERRUPT_EXTERNAL)
-		trap = 0xea0;
-
-	switch (trap) {
-	case 0x980:
-	case INTERRUPT_H_DATA_STORAGE:
-	case 0xe20:
-	case 0xe40:
-	case INTERRUPT_HMI:
-	case 0xe80:
-	case 0xea0:
-	case INTERRUPT_H_FAC_UNAVAIL:
-	case 0x1200:
-	case 0x1500:
-	case 0x1600:
-	case 0x1800:
-		validp = &local_paca->hsrr_valid;
-		if (!READ_ONCE(*validp))
-			return;
-
-		srr0 = mfspr(SPRN_HSRR0);
-		srr1 = mfspr(SPRN_HSRR1);
-		h = "H";
-
-		break;
-	default:
-		validp = &local_paca->srr_valid;
-		if (!READ_ONCE(*validp))
-			return;
-
-		srr0 = mfspr(SPRN_SRR0);
-		srr1 = mfspr(SPRN_SRR1);
-		h = "";
-		break;
-	}
-
-	if (srr0 == regs->nip && srr1 == regs->msr)
-		return;
-
-	/*
-	 * A NMI / soft-NMI interrupt may have come in after we found
-	 * srr_valid and before the SRRs are loaded. The interrupt then
-	 * comes in and clobbers SRRs and clears srr_valid. Then we load
-	 * the SRRs here and test them above and find they don't match.
-	 *
-	 * Test validity again after that, to catch such false positives.
-	 *
-	 * This test in general will have some window for false negatives
-	 * and may not catch and fix all such cases if an NMI comes in
-	 * later and clobbers SRRs without clearing srr_valid, but hopefully
-	 * such things will get caught most of the time, statistically
-	 * enough to be able to get a warning out.
-	 */
-	if (!READ_ONCE(*validp))
-		return;
-
-	if (!data_race(warned)) {
-		data_race(warned = true);
-		printk("%sSRR0 was: %lx should be: %lx\n", h, srr0, regs->nip);
-		printk("%sSRR1 was: %lx should be: %lx\n", h, srr1, regs->msr);
-		show_regs(regs);
-	}
-
-	WRITE_ONCE(*validp, 0); /* fixup */
-#endif
-}
-
 static notrace unsigned long
 interrupt_exit_user_prepare_main(unsigned long ret, struct pt_regs *regs)
 {
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index aa17e62f3754..719930cf4ae1 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -22,6 +22,11 @@
 
 #include "signal.h"
 
+/* This will be removed */
+#ifdef CONFIG_GENERIC_ENTRY
+#include <linux/entry-common.h>
+#endif /* CONFIG_GENERIC_ENTRY */
+
 #ifdef CONFIG_VSX
 unsigned long copy_fpr_to_user(void __user *to,
 			       struct task_struct *task)
@@ -368,3 +373,12 @@ void signal_fault(struct task_struct *tsk, struct pt_regs *regs,
 		printk_ratelimited(regs->msr & MSR_64BIT ? fm64 : fm32, tsk->comm,
 				   task_pid_nr(tsk), where, ptr, regs->nip, regs->link);
 }
+
+#ifdef CONFIG_GENERIC_ENTRY
+void arch_do_signal_or_restart(struct pt_regs *regs)
+{
+	BUG_ON(regs != current->thread.regs);
+	local_paca->generic_fw_flags |= GFW_RESTORE_ALL;
+	do_signal(current);
+}
+#endif /* CONFIG_GENERIC_ENTRY */
-- 
2.52.0

Le 14/12/2025 à 14:02, Mukesh Kumar Chaurasiya a écrit :
> From: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
> 
> Add PowerPC-specific implementations of the generic syscall exit hooks
> used by the generic entry/exit framework:
> 
>   - arch_exit_to_user_mode_work_prepare()
>   - arch_exit_to_user_mode_work()
> 
> These helpers handle user state restoration when returning from the
> kernel to userspace, including FPU/VMX/VSX state, transactional memory,
> KUAP restore, and per-CPU accounting.
> 
> Additionally, move check_return_regs_valid() from interrupt.c to
> interrupt.h so it can be shared by the new entry/exit logic, and add
> arch_do_signal_or_restart() for use with the generic entry flow.
> 
> No functional change is intended with this patch.
> 
> Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
> ---
>   arch/powerpc/include/asm/entry-common.h | 49 +++++++++++++++
>   arch/powerpc/include/asm/interrupt.h    | 82 +++++++++++++++++++++++++
>   arch/powerpc/kernel/interrupt.c         | 81 ------------------------
>   arch/powerpc/kernel/signal.c            | 14 +++++
>   4 files changed, 145 insertions(+), 81 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
> index 093ece06ef79..e8ebd42a4e6d 100644
> --- a/arch/powerpc/include/asm/entry-common.h
> +++ b/arch/powerpc/include/asm/entry-common.h
> @@ -8,6 +8,7 @@
>   #include <asm/cputime.h>
>   #include <asm/interrupt.h>
>   #include <asm/stacktrace.h>
> +#include <asm/switch_to.h>
>   #include <asm/tm.h>
>   
>   static __always_inline void arch_enter_from_user_mode(struct pt_regs *regs)
> @@ -104,5 +105,53 @@ static __always_inline void arch_enter_from_user_mode(struct pt_regs *regs)
>   
>   #define arch_enter_from_user_mode arch_enter_from_user_mode
>   
> +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
> +						  unsigned long ti_work)
> +{
> +	unsigned long mathflags;
> +
> +	if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && IS_ENABLED(CONFIG_PPC_FPU)) {
> +		if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
> +		    unlikely((ti_work & _TIF_RESTORE_TM))) {
> +			restore_tm_state(regs);
> +		} else {
> +			mathflags = MSR_FP;
> +
> +			if (cpu_has_feature(CPU_FTR_VSX))
> +				mathflags |= MSR_VEC | MSR_VSX;
> +			else if (cpu_has_feature(CPU_FTR_ALTIVEC))
> +				mathflags |= MSR_VEC;
> +
> +			/*
> +			 * If userspace MSR has all available FP bits set,
> +			 * then they are live and no need to restore. If not,
> +			 * it means the regs were given up and restore_math
> +			 * may decide to restore them (to avoid taking an FP
> +			 * fault).
> +			 */
> +			if ((regs->msr & mathflags) != mathflags)
> +				restore_math(regs);
> +		}
> +	}
> +
> +	check_return_regs_valid(regs);
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	local_paca->tm_scratch = regs->msr;
> +#endif
> +	/* Restore user access locks last */
> +	kuap_user_restore(regs);
> +}
> +
> +#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
> +
> +static __always_inline void arch_exit_to_user_mode(void)
> +{
> +	booke_load_dbcr0();
> +
> +	account_cpu_user_exit();
> +}
> +
> +#define arch_exit_to_user_mode arch_exit_to_user_mode
> +
>   #endif /* CONFIG_GENERIC_IRQ_ENTRY */
>   #endif /* _ASM_PPC_ENTRY_COMMON_H */
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index ca8a2cda9400..77ff8e33f8cd 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -68,6 +68,8 @@
>   
>   #include <linux/context_tracking.h>
>   #include <linux/hardirq.h>
> +#include <linux/sched/debug.h> /* for show_regs */
> +
>   #include <asm/cputime.h>
>   #include <asm/firmware.h>
>   #include <asm/ftrace.h>
> @@ -172,6 +174,86 @@ static inline void booke_restore_dbcr0(void)
>   #endif
>   }
>   
> +static inline void check_return_regs_valid(struct pt_regs *regs)

This was previously a notrace function. Should it be marked 
__always_inline instead of just inline ?

> +{
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	unsigned long trap, srr0, srr1;
> +	static bool warned;
> +	u8 *validp;
> +	char *h;
> +
> +	if (trap_is_scv(regs))
> +		return;
> +
> +	trap = TRAP(regs);
> +	// EE in HV mode sets HSRRs like 0xea0
> +	if (cpu_has_feature(CPU_FTR_HVMODE) && trap == INTERRUPT_EXTERNAL)
> +		trap = 0xea0;
> +
> +	switch (trap) {
> +	case 0x980:
> +	case INTERRUPT_H_DATA_STORAGE:
> +	case 0xe20:
> +	case 0xe40:
> +	case INTERRUPT_HMI:
> +	case 0xe80:
> +	case 0xea0:
> +	case INTERRUPT_H_FAC_UNAVAIL:
> +	case 0x1200:
> +	case 0x1500:
> +	case 0x1600:
> +	case 0x1800:
> +		validp = &local_paca->hsrr_valid;
> +		if (!READ_ONCE(*validp))
> +			return;
> +
> +		srr0 = mfspr(SPRN_HSRR0);
> +		srr1 = mfspr(SPRN_HSRR1);
> +		h = "H";
> +
> +		break;
> +	default:
> +		validp = &local_paca->srr_valid;
> +		if (!READ_ONCE(*validp))
> +			return;
> +
> +		srr0 = mfspr(SPRN_SRR0);
> +		srr1 = mfspr(SPRN_SRR1);
> +		h = "";
> +		break;
> +	}
> +
> +	if (srr0 == regs->nip && srr1 == regs->msr)
> +		return;
> +
> +	/*
> +	 * A NMI / soft-NMI interrupt may have come in after we found
> +	 * srr_valid and before the SRRs are loaded. The interrupt then
> +	 * comes in and clobbers SRRs and clears srr_valid. Then we load
> +	 * the SRRs here and test them above and find they don't match.
> +	 *
> +	 * Test validity again after that, to catch such false positives.
> +	 *
> +	 * This test in general will have some window for false negatives
> +	 * and may not catch and fix all such cases if an NMI comes in
> +	 * later and clobbers SRRs without clearing srr_valid, but hopefully
> +	 * such things will get caught most of the time, statistically
> +	 * enough to be able to get a warning out.
> +	 */
> +	if (!READ_ONCE(*validp))
> +		return;
> +
> +	if (!data_race(warned)) {
> +		data_race(warned = true);
> +		pr_warn("%sSRR0 was: %lx should be: %lx\n", h, srr0, regs->nip);
> +		pr_warn("%sSRR1 was: %lx should be: %lx\n", h, srr1, regs->msr);
> +		show_regs(regs);
> +	}
> +
> +	WRITE_ONCE(*validp, 0); /* fixup */
> +#endif
> +}
> +
>   static inline void interrupt_enter_prepare(struct pt_regs *regs)
>   {
>   #ifdef CONFIG_PPC64
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 2a09ac5dabd6..f53d432f6087 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -4,7 +4,6 @@
>   #include <linux/err.h>
>   #include <linux/compat.h>
>   #include <linux/rseq.h>
> -#include <linux/sched/debug.h> /* for show_regs */
>   
>   #include <asm/kup.h>
>   #include <asm/cputime.h>
> @@ -78,86 +77,6 @@ static notrace __always_inline bool prep_irq_for_enabled_exit(bool restartable)
>   	return true;
>   }
>   
> -static notrace void check_return_regs_valid(struct pt_regs *regs)
> -{
> -#ifdef CONFIG_PPC_BOOK3S_64
> -	unsigned long trap, srr0, srr1;
> -	static bool warned;
> -	u8 *validp;
> -	char *h;
> -
> -	if (trap_is_scv(regs))
> -		return;
> -
> -	trap = TRAP(regs);
> -	// EE in HV mode sets HSRRs like 0xea0
> -	if (cpu_has_feature(CPU_FTR_HVMODE) && trap == INTERRUPT_EXTERNAL)
> -		trap = 0xea0;
> -
> -	switch (trap) {
> -	case 0x980:
> -	case INTERRUPT_H_DATA_STORAGE:
> -	case 0xe20:
> -	case 0xe40:
> -	case INTERRUPT_HMI:
> -	case 0xe80:
> -	case 0xea0:
> -	case INTERRUPT_H_FAC_UNAVAIL:
> -	case 0x1200:
> -	case 0x1500:
> -	case 0x1600:
> -	case 0x1800:
> -		validp = &local_paca->hsrr_valid;
> -		if (!READ_ONCE(*validp))
> -			return;
> -
> -		srr0 = mfspr(SPRN_HSRR0);
> -		srr1 = mfspr(SPRN_HSRR1);
> -		h = "H";
> -
> -		break;
> -	default:
> -		validp = &local_paca->srr_valid;
> -		if (!READ_ONCE(*validp))
> -			return;
> -
> -		srr0 = mfspr(SPRN_SRR0);
> -		srr1 = mfspr(SPRN_SRR1);
> -		h = "";
> -		break;
> -	}
> -
> -	if (srr0 == regs->nip && srr1 == regs->msr)
> -		return;
> -
> -	/*
> -	 * A NMI / soft-NMI interrupt may have come in after we found
> -	 * srr_valid and before the SRRs are loaded. The interrupt then
> -	 * comes in and clobbers SRRs and clears srr_valid. Then we load
> -	 * the SRRs here and test them above and find they don't match.
> -	 *
> -	 * Test validity again after that, to catch such false positives.
> -	 *
> -	 * This test in general will have some window for false negatives
> -	 * and may not catch and fix all such cases if an NMI comes in
> -	 * later and clobbers SRRs without clearing srr_valid, but hopefully
> -	 * such things will get caught most of the time, statistically
> -	 * enough to be able to get a warning out.
> -	 */
> -	if (!READ_ONCE(*validp))
> -		return;
> -
> -	if (!data_race(warned)) {
> -		data_race(warned = true);
> -		printk("%sSRR0 was: %lx should be: %lx\n", h, srr0, regs->nip);
> -		printk("%sSRR1 was: %lx should be: %lx\n", h, srr1, regs->msr);
> -		show_regs(regs);
> -	}
> -
> -	WRITE_ONCE(*validp, 0); /* fixup */
> -#endif
> -}
> -
>   static notrace unsigned long
>   interrupt_exit_user_prepare_main(unsigned long ret, struct pt_regs *regs)
>   {
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index aa17e62f3754..719930cf4ae1 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -22,6 +22,11 @@
>   
>   #include "signal.h"
>   
> +/* This will be removed */
> +#ifdef CONFIG_GENERIC_ENTRY

Is this #ifdef really needed ?

> +#include <linux/entry-common.h>
> +#endif /* CONFIG_GENERIC_ENTRY */
> +
>   #ifdef CONFIG_VSX
>   unsigned long copy_fpr_to_user(void __user *to,
>   			       struct task_struct *task)
> @@ -368,3 +373,12 @@ void signal_fault(struct task_struct *tsk, struct pt_regs *regs,
>   		printk_ratelimited(regs->msr & MSR_64BIT ? fm64 : fm32, tsk->comm,
>   				   task_pid_nr(tsk), where, ptr, regs->nip, regs->link);
>   }
> +
> +#ifdef CONFIG_GENERIC_ENTRY

Why is this #ifdef needed ?

> +void arch_do_signal_or_restart(struct pt_regs *regs)
> +{
> +	BUG_ON(regs != current->thread.regs);

Is this BUG_ON() needed ? Can't we use something smoother ?

> +	local_paca->generic_fw_flags |= GFW_RESTORE_ALL;
> +	do_signal(current);
> +}
> +#endif /* CONFIG_GENERIC_ENTRY */

On Tue, Dec 16, 2025 at 10:46:28AM +0100, Christophe Leroy (CS GROUP) wrote:
> 
> 
> Le 14/12/2025 à 14:02, Mukesh Kumar Chaurasiya a écrit :
> > From: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
> > 
> > Add PowerPC-specific implementations of the generic syscall exit hooks
> > used by the generic entry/exit framework:
> > 
> >   - arch_exit_to_user_mode_work_prepare()
> >   - arch_exit_to_user_mode_work()
> > 
> > These helpers handle user state restoration when returning from the
> > kernel to userspace, including FPU/VMX/VSX state, transactional memory,
> > KUAP restore, and per-CPU accounting.
> > 
> > Additionally, move check_return_regs_valid() from interrupt.c to
> > interrupt.h so it can be shared by the new entry/exit logic, and add
> > arch_do_signal_or_restart() for use with the generic entry flow.
> > 
> > No functional change is intended with this patch.
> > 
> > Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
> > ---
> >   arch/powerpc/include/asm/entry-common.h | 49 +++++++++++++++
> >   arch/powerpc/include/asm/interrupt.h    | 82 +++++++++++++++++++++++++
> >   arch/powerpc/kernel/interrupt.c         | 81 ------------------------
> >   arch/powerpc/kernel/signal.c            | 14 +++++
> >   4 files changed, 145 insertions(+), 81 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
> > index 093ece06ef79..e8ebd42a4e6d 100644
> > --- a/arch/powerpc/include/asm/entry-common.h
> > +++ b/arch/powerpc/include/asm/entry-common.h
> > @@ -8,6 +8,7 @@
> >   #include <asm/cputime.h>
> >   #include <asm/interrupt.h>
> >   #include <asm/stacktrace.h>
> > +#include <asm/switch_to.h>
> >   #include <asm/tm.h>
> >   static __always_inline void arch_enter_from_user_mode(struct pt_regs *regs)
> > @@ -104,5 +105,53 @@ static __always_inline void arch_enter_from_user_mode(struct pt_regs *regs)
> >   #define arch_enter_from_user_mode arch_enter_from_user_mode
> > +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
> > +						  unsigned long ti_work)
> > +{
> > +	unsigned long mathflags;
> > +
> > +	if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && IS_ENABLED(CONFIG_PPC_FPU)) {
> > +		if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
> > +		    unlikely((ti_work & _TIF_RESTORE_TM))) {
> > +			restore_tm_state(regs);
> > +		} else {
> > +			mathflags = MSR_FP;
> > +
> > +			if (cpu_has_feature(CPU_FTR_VSX))
> > +				mathflags |= MSR_VEC | MSR_VSX;
> > +			else if (cpu_has_feature(CPU_FTR_ALTIVEC))
> > +				mathflags |= MSR_VEC;
> > +
> > +			/*
> > +			 * If userspace MSR has all available FP bits set,
> > +			 * then they are live and no need to restore. If not,
> > +			 * it means the regs were given up and restore_math
> > +			 * may decide to restore them (to avoid taking an FP
> > +			 * fault).
> > +			 */
> > +			if ((regs->msr & mathflags) != mathflags)
> > +				restore_math(regs);
> > +		}
> > +	}
> > +
> > +	check_return_regs_valid(regs);
> > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > +	local_paca->tm_scratch = regs->msr;
> > +#endif
> > +	/* Restore user access locks last */
> > +	kuap_user_restore(regs);
> > +}
> > +
> > +#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
> > +
> > +static __always_inline void arch_exit_to_user_mode(void)
> > +{
> > +	booke_load_dbcr0();
> > +
> > +	account_cpu_user_exit();
> > +}
> > +
> > +#define arch_exit_to_user_mode arch_exit_to_user_mode
> > +
> >   #endif /* CONFIG_GENERIC_IRQ_ENTRY */
> >   #endif /* _ASM_PPC_ENTRY_COMMON_H */
> > diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> > index ca8a2cda9400..77ff8e33f8cd 100644
> > --- a/arch/powerpc/include/asm/interrupt.h
> > +++ b/arch/powerpc/include/asm/interrupt.h
> > @@ -68,6 +68,8 @@
> >   #include <linux/context_tracking.h>
> >   #include <linux/hardirq.h>
> > +#include <linux/sched/debug.h> /* for show_regs */
> > +
> >   #include <asm/cputime.h>
> >   #include <asm/firmware.h>
> >   #include <asm/ftrace.h>
> > @@ -172,6 +174,86 @@ static inline void booke_restore_dbcr0(void)
> >   #endif
> >   }
> > +static inline void check_return_regs_valid(struct pt_regs *regs)
> 
> This was previously a notrace function. Should it be marked __always_inline
> instead of just inline ?
> 
Yes it should. Will fix this too.
> > +{
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > +	unsigned long trap, srr0, srr1;
> > +	static bool warned;
> > +	u8 *validp;
> > +	char *h;
> > +
> > +	if (trap_is_scv(regs))
> > +		return;
> > +
> > +	trap = TRAP(regs);
> > +	// EE in HV mode sets HSRRs like 0xea0
> > +	if (cpu_has_feature(CPU_FTR_HVMODE) && trap == INTERRUPT_EXTERNAL)
> > +		trap = 0xea0;
> > +
> > +	switch (trap) {
> > +	case 0x980:
> > +	case INTERRUPT_H_DATA_STORAGE:
> > +	case 0xe20:
> > +	case 0xe40:
> > +	case INTERRUPT_HMI:
> > +	case 0xe80:
> > +	case 0xea0:
> > +	case INTERRUPT_H_FAC_UNAVAIL:
> > +	case 0x1200:
> > +	case 0x1500:
> > +	case 0x1600:
> > +	case 0x1800:
> > +		validp = &local_paca->hsrr_valid;
> > +		if (!READ_ONCE(*validp))
> > +			return;
> > +
> > +		srr0 = mfspr(SPRN_HSRR0);
> > +		srr1 = mfspr(SPRN_HSRR1);
> > +		h = "H";
> > +
> > +		break;
> > +	default:
> > +		validp = &local_paca->srr_valid;
> > +		if (!READ_ONCE(*validp))
> > +			return;
> > +
> > +		srr0 = mfspr(SPRN_SRR0);
> > +		srr1 = mfspr(SPRN_SRR1);
> > +		h = "";
> > +		break;
> > +	}
> > +
> > +	if (srr0 == regs->nip && srr1 == regs->msr)
> > +		return;
> > +
> > +	/*
> > +	 * A NMI / soft-NMI interrupt may have come in after we found
> > +	 * srr_valid and before the SRRs are loaded. The interrupt then
> > +	 * comes in and clobbers SRRs and clears srr_valid. Then we load
> > +	 * the SRRs here and test them above and find they don't match.
> > +	 *
> > +	 * Test validity again after that, to catch such false positives.
> > +	 *
> > +	 * This test in general will have some window for false negatives
> > +	 * and may not catch and fix all such cases if an NMI comes in
> > +	 * later and clobbers SRRs without clearing srr_valid, but hopefully
> > +	 * such things will get caught most of the time, statistically
> > +	 * enough to be able to get a warning out.
> > +	 */
> > +	if (!READ_ONCE(*validp))
> > +		return;
> > +
> > +	if (!data_race(warned)) {
> > +		data_race(warned = true);
> > +		pr_warn("%sSRR0 was: %lx should be: %lx\n", h, srr0, regs->nip);
> > +		pr_warn("%sSRR1 was: %lx should be: %lx\n", h, srr1, regs->msr);
> > +		show_regs(regs);
> > +	}
> > +
> > +	WRITE_ONCE(*validp, 0); /* fixup */
> > +#endif
> > +}
> > +
> >   static inline void interrupt_enter_prepare(struct pt_regs *regs)
> >   {
> >   #ifdef CONFIG_PPC64
> > diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> > index 2a09ac5dabd6..f53d432f6087 100644
> > --- a/arch/powerpc/kernel/interrupt.c
> > +++ b/arch/powerpc/kernel/interrupt.c
> > @@ -4,7 +4,6 @@
> >   #include <linux/err.h>
> >   #include <linux/compat.h>
> >   #include <linux/rseq.h>
> > -#include <linux/sched/debug.h> /* for show_regs */
> >   #include <asm/kup.h>
> >   #include <asm/cputime.h>
> > @@ -78,86 +77,6 @@ static notrace __always_inline bool prep_irq_for_enabled_exit(bool restartable)
> >   	return true;
> >   }
> > -static notrace void check_return_regs_valid(struct pt_regs *regs)
> > -{
> > -#ifdef CONFIG_PPC_BOOK3S_64
> > -	unsigned long trap, srr0, srr1;
> > -	static bool warned;
> > -	u8 *validp;
> > -	char *h;
> > -
> > -	if (trap_is_scv(regs))
> > -		return;
> > -
> > -	trap = TRAP(regs);
> > -	// EE in HV mode sets HSRRs like 0xea0
> > -	if (cpu_has_feature(CPU_FTR_HVMODE) && trap == INTERRUPT_EXTERNAL)
> > -		trap = 0xea0;
> > -
> > -	switch (trap) {
> > -	case 0x980:
> > -	case INTERRUPT_H_DATA_STORAGE:
> > -	case 0xe20:
> > -	case 0xe40:
> > -	case INTERRUPT_HMI:
> > -	case 0xe80:
> > -	case 0xea0:
> > -	case INTERRUPT_H_FAC_UNAVAIL:
> > -	case 0x1200:
> > -	case 0x1500:
> > -	case 0x1600:
> > -	case 0x1800:
> > -		validp = &local_paca->hsrr_valid;
> > -		if (!READ_ONCE(*validp))
> > -			return;
> > -
> > -		srr0 = mfspr(SPRN_HSRR0);
> > -		srr1 = mfspr(SPRN_HSRR1);
> > -		h = "H";
> > -
> > -		break;
> > -	default:
> > -		validp = &local_paca->srr_valid;
> > -		if (!READ_ONCE(*validp))
> > -			return;
> > -
> > -		srr0 = mfspr(SPRN_SRR0);
> > -		srr1 = mfspr(SPRN_SRR1);
> > -		h = "";
> > -		break;
> > -	}
> > -
> > -	if (srr0 == regs->nip && srr1 == regs->msr)
> > -		return;
> > -
> > -	/*
> > -	 * A NMI / soft-NMI interrupt may have come in after we found
> > -	 * srr_valid and before the SRRs are loaded. The interrupt then
> > -	 * comes in and clobbers SRRs and clears srr_valid. Then we load
> > -	 * the SRRs here and test them above and find they don't match.
> > -	 *
> > -	 * Test validity again after that, to catch such false positives.
> > -	 *
> > -	 * This test in general will have some window for false negatives
> > -	 * and may not catch and fix all such cases if an NMI comes in
> > -	 * later and clobbers SRRs without clearing srr_valid, but hopefully
> > -	 * such things will get caught most of the time, statistically
> > -	 * enough to be able to get a warning out.
> > -	 */
> > -	if (!READ_ONCE(*validp))
> > -		return;
> > -
> > -	if (!data_race(warned)) {
> > -		data_race(warned = true);
> > -		printk("%sSRR0 was: %lx should be: %lx\n", h, srr0, regs->nip);
> > -		printk("%sSRR1 was: %lx should be: %lx\n", h, srr1, regs->msr);
> > -		show_regs(regs);
> > -	}
> > -
> > -	WRITE_ONCE(*validp, 0); /* fixup */
> > -#endif
> > -}
> > -
> >   static notrace unsigned long
> >   interrupt_exit_user_prepare_main(unsigned long ret, struct pt_regs *regs)
> >   {
> > diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> > index aa17e62f3754..719930cf4ae1 100644
> > --- a/arch/powerpc/kernel/signal.c
> > +++ b/arch/powerpc/kernel/signal.c
> > @@ -22,6 +22,11 @@
> >   #include "signal.h"
> > +/* This will be removed */
> > +#ifdef CONFIG_GENERIC_ENTRY
> 
> Is this #ifdef really needed ?
Will fix this.
> 
> > +#include <linux/entry-common.h>
> > +#endif /* CONFIG_GENERIC_ENTRY */
> > +
> >   #ifdef CONFIG_VSX
> >   unsigned long copy_fpr_to_user(void __user *to,
> >   			       struct task_struct *task)
> > @@ -368,3 +373,12 @@ void signal_fault(struct task_struct *tsk, struct pt_regs *regs,
> >   		printk_ratelimited(regs->msr & MSR_64BIT ? fm64 : fm32, tsk->comm,
> >   				   task_pid_nr(tsk), where, ptr, regs->nip, regs->link);
> >   }
> > +
> > +#ifdef CONFIG_GENERIC_ENTRY
> 
> Why is this #ifdef needed ?
> 
> > +void arch_do_signal_or_restart(struct pt_regs *regs)
> > +{
> > +	BUG_ON(regs != current->thread.regs);
> 
> Is this BUG_ON() needed ? Can't we use something smoother ?
> 
I am not sure about what to do here. Proceeding with this seemed
dangerous. So went with a BUG_ON. Can you suggest if something better
comes to your mind.

Regards,
Mukesh
> > +	local_paca->generic_fw_flags |= GFW_RESTORE_ALL;
> > +	do_signal(current);
> > +}
> > +#endif /* CONFIG_GENERIC_ENTRY */
>