[PATCH Dovetail v3 0/6] Dovetail: arm: Finalize review results

[PATCH Dovetail v3 0/6] Dovetail: arm: Finalize review results

[Florian Bezdeka]

Hi Philippe,

This is the first try to fully process the feedback on the
interrupts_enabled() issue mentioned in the former patch series.

Input welcome.

I tried hard to keep everything easy, but changing the current
implementation triggered a lot of unexpected include / build problems, 
especially within the arch/arm/include/asm folder.

Testing looks good, I'm going to schedule a full CI run now.

To: xenomai@lists.linux.dev
Cc: Philippe Gerum <rpm@xenomai.org>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Tobias Schaffner <tobias.schaffner@siemens.com>

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
Changes in v3:
- Patch 1: Integrate review comments
- Patch 2: Integrate review comments, rename to dovetail_fault_entry 
- Link to v2: https://lore.kernel.org/r/20260610-wip-flo-v7-1-arm-pipelining-fixes-v2-0-62c9c477f8f2@siemens.com

Changes in v2:
- Resend. Forgot the list...

---
Florian Bezdeka (6):
      arm: irq_pipeline: Rename and move fault_entry_{enter,exit}
      arm: irq_pipeline: Fix dovetail_fault_{entry,exit} bypass in do_kernel_address_page_fault
      arm: irq_pipeline: Fix dovetail_fault_{entry,exit} bypass in hw_breakpoint_pending
      arm: irq_pipeline: Fix ordering problem in alignment trap handling
      dovetail: Decouple kernel/irq/pipeline.c from irqstate bit definitions
      arm: irq_pipeline: Fix IRQ state checking in trap entry paths

 arch/arm/include/asm/dovetail.h         |  1 +
 arch/arm/include/asm/irq_pipeline.h     | 41 ++++++++++----
 arch/arm/include/asm/trace/exceptions.h |  3 +-
 arch/arm/include/asm/trap_entry.h       | 83 +++++++++++++++++++++++++++++
 arch/arm/kernel/hw_breakpoint.c         |  8 ++-
 arch/arm/mm/alignment.c                 |  6 +--
 arch/arm/mm/fault.c                     | 94 ++++++---------------------------
 arch/arm64/include/asm/irq_pipeline.h   |  9 ++--
 kernel/irq/pipeline.c                   | 20 +++----
 9 files changed, 156 insertions(+), 109 deletions(-)
---
base-commit: 0ea6cb72940b40c147655ae13dd77ffcae553df9
change-id: 20260609-wip-flo-v7-1-arm-pipelining-fixes-9428542777f6

Best regards,
-- 
Florian Bezdeka <florian.bezdeka@siemens.com>

[PATCH Dovetail v3 1/6] arm: irq_pipeline: Rename and move fault_entry_{enter,exit}

[Florian Bezdeka]

Introducing a new header allows us to keep the implementation as
static inline function and make it re-usable for other compile units.
Moving the implementation into existing headers like asm/dovetail.h or
asm/irq_pipeline.h would require us to rewrite those helpers as macros.

Prefixing with dovetail_ to keep the Linux namespace clean.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm/include/asm/trap_entry.h | 83 ++++++++++++++++++++++++++++++++++++
 arch/arm/mm/fault.c               | 89 ++++++---------------------------------
 2 files changed, 96 insertions(+), 76 deletions(-)

diff --git a/arch/arm/include/asm/trap_entry.h b/arch/arm/include/asm/trap_entry.h
new file mode 100644
index 0000000000000000000000000000000000000000..abe5f80cd301a8ca5ddb2ca350b80bf5708f0f79
--- /dev/null
+++ b/arch/arm/include/asm/trap_entry.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASMARM_TRAP_ENTRY_H
+#define _ASMARM_TRAP_ENTRY_H
+
+#include <asm/dovetail.h>
+#include <asm/trace/exceptions.h>
+
+#ifdef CONFIG_MMU
+#ifdef CONFIG_IRQ_PIPELINE
+/*
+ * We need to synchronize the virtual interrupt state with the hard
+ * interrupt state we received on entry, then turn hardirqs back on to
+ * allow code which does not require strict serialization to be
+ * preempted by an out-of-band activity.
+ */
+static inline unsigned long dovetail_fault_entry(int exception,
+						 struct pt_regs *regs)
+{
+	unsigned long flags;
+
+	trace_ARM_trap_entry(exception, regs);
+
+	flags = hard_local_save_flags();
+
+	/*
+	 * The companion core must demote the current context to
+	 * in-band stage if running oob on entry.
+	 */
+	mark_trap_entry(exception, regs);
+
+	if (raw_irqs_disabled_flags(flags)) {
+		stall_inband();
+		trace_hardirqs_off();
+	}
+
+	hard_local_irq_enable();
+
+	return flags;
+}
+
+static inline void dovetail_fault_exit(int exception, struct pt_regs *regs,
+				       unsigned long flags)
+{
+	WARN_ON_ONCE(irq_pipeline_debug() && hard_irqs_disabled());
+
+	/*
+	 * We expect kentry_exit_pipelined() to clear the stall bit if
+	 * kentry_enter_pipelined() observed it that way.
+	 */
+	mark_trap_exit(exception, regs);
+	trace_ARM_trap_exit(exception, regs);
+	hard_local_irq_restore(flags);
+}
+
+#else /* !CONFIG_IRQ_PIPELINE */
+
+#define dovetail_fault_entry(__exception, __regs)		\
+	do {							\
+		(void)(__exception);				\
+		(void)(__regs);					\
+	} while (0)
+
+#define dovetail_fault_exit(__exception, __regs, __flags)	\
+	do {							\
+		(void)(__exception);				\
+		(void)(__regs);					\
+		(void)(__flags);				\
+	} while (0)
+
+#endif /* !CONFIG_IRQ_PIPELINE */
+
+#else /* CONFIG_MMU */
+unsigned long dovetail_fault_entry(int exception, struct pt_regs *regs)
+{
+	return 0;
+}
+
+static inline void dovetail_fault_exit(int exception, struct pt_regs *regs,
+				       unsigned long combo)
+{ }
+#endif /* !CONFIG_MMU */
+
+#endif
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 49f5d628c4c9a91a3e879af0d541fd76665d78fe..b6b766652b753ef860d16b153685b0d3b582bffa 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -24,6 +24,7 @@
 #include <asm/system_info.h>
 #include <asm/tlbflush.h>
 #include <asm/dovetail.h>
+#include <asm/trap_entry.h>
 #define CREATE_TRACE_POINTS
 #include <asm/trace/exceptions.h>
 
@@ -38,61 +39,6 @@ bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
 	return addr >= TASK_SIZE && ULONG_MAX - addr >= size;
 }
 
-#ifdef CONFIG_IRQ_PIPELINE
-/*
- * We need to synchronize the virtual interrupt state with the hard
- * interrupt state we received on entry, then turn hardirqs back on to
- * allow code which does not require strict serialization to be
- * preempted by an out-of-band activity.
- */
-static inline
-unsigned long fault_entry(int exception, struct pt_regs *regs)
-{
-	unsigned long flags;
-
-	trace_ARM_trap_entry(exception, regs);
-
-	flags = hard_local_save_flags();
-
-	/*
-	 * The companion core must demote the current context to
-	 * in-band stage if running oob on entry.
-	 */
-	mark_trap_entry(exception, regs);
-
-	if (raw_irqs_disabled_flags(flags)) {
-		stall_inband();
-		trace_hardirqs_off();
-	}
-
-	hard_local_irq_enable();
-
-	return flags;
-}
-
-static inline
-void fault_exit(int exception, struct pt_regs *regs,
-		unsigned long flags)
-{
-	WARN_ON_ONCE(irq_pipeline_debug() && hard_irqs_disabled());
-
-	/*
-	 * We expect kentry_exit_pipelined() to clear the stall bit if
-	 * kentry_enter_pipelined() observed it that way.
-	 */
-	mark_trap_exit(exception, regs);
-	trace_ARM_trap_exit(exception, regs);
-	hard_local_irq_restore(flags);
-}
-
-#else	/* !CONFIG_IRQ_PIPELINE */
-
-#define fault_entry(__exception, __regs)  ({ 0; })
-#define fault_exit(__exception, __regs, __flags)  \
-	do { (void)(__flags); } while (0)
-
-#endif	/* !CONFIG_IRQ_PIPELINE */
-
 /*
  * This is useful to dump out the page tables associated with
  * 'addr' in mm 'mm'.
@@ -165,15 +111,6 @@ void show_pte(const char *lvl, struct mm_struct *mm, unsigned long addr)
 	pr_cont("\n");
 }
 #else					/* CONFIG_MMU */
-unsigned long fault_entry(int exception, struct pt_regs *regs)
-{
-	return 0;
-}
-
-static inline void fault_exit(int exception, struct pt_regs *regs,
-			unsigned long combo)
-{ }
-
 void show_pte(const char *lvl, struct mm_struct *mm, unsigned long addr)
 { }
 #endif					/* CONFIG_MMU */
@@ -278,9 +215,9 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	 * have no context to handle this fault with.
 	 */
 	  if (user_mode(regs)) {
-		irqflags = fault_entry(ARM_TRAP_ACCESS, regs);
+		irqflags = dovetail_fault_entry(ARM_TRAP_ACCESS, regs);
 		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
-		fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
+		dovetail_fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
 	  } else
 		/*
 		 * irq_pipeline: kernel faults are either quickly
@@ -391,9 +328,9 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
 		 * Note that __do_user_fault() will enable interrupts.
 		 */
 		harden_branch_predictor();
-		irqflags = fault_entry(ARM_TRAP_ACCESS, regs);
+		irqflags = dovetail_fault_entry(ARM_TRAP_ACCESS, regs);
 		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
-		fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
+		dovetail_fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
 	} else {
 		/*
 		 * Fault from kernel mode. Enable interrupts if they were
@@ -433,7 +370,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (addr >= TASK_SIZE)
 		return do_kernel_address_page_fault(mm, addr, fsr, regs);
 
-	irqflags = fault_entry(ARM_TRAP_ACCESS, regs);
+	irqflags = dovetail_fault_entry(ARM_TRAP_ACCESS, regs);
 
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (interrupts_enabled(regs))
@@ -595,7 +532,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 no_context:
 	__do_kernel_fault(mm, addr, fsr, regs);
 out:
-	fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
+	dovetail_fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
 
 	return 0;
 }
@@ -671,9 +608,9 @@ do_sect_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (addr >= TASK_SIZE && user_mode(regs))
 		harden_branch_predictor();
 
-	irqflags = fault_entry(ARM_TRAP_SECTION, regs);
+	irqflags = dovetail_fault_entry(ARM_TRAP_SECTION, regs);
 	do_bad_area(addr, fsr, regs);
-	fault_exit(ARM_TRAP_SECTION, regs, irqflags);
+	dovetail_fault_exit(ARM_TRAP_SECTION, regs, irqflags);
 	return 0;
 }
 #endif /* CONFIG_ARM_LPAE */
@@ -726,7 +663,7 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))
 		return;
 
-	irqflags = fault_entry(ARM_TRAP_DABT, regs);
+	irqflags = dovetail_fault_entry(ARM_TRAP_DABT, regs);
 	pr_alert("8<--- cut here ---\n");
 	pr_alert("Unhandled fault: %s (0x%03x) at 0x%08lx\n",
 		inf->name, fsr, addr);
@@ -734,7 +671,7 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 
 	arm_notify_die("", regs, inf->sig, inf->code, (void __user *)addr,
 		       fsr, 0);
-	fault_exit(ARM_TRAP_DABT, regs, irqflags);
+	dovetail_fault_exit(ARM_TRAP_DABT, regs, irqflags);
 }
 
 void __init
@@ -759,14 +696,14 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs)
 	if (!inf->fn(addr, ifsr | FSR_LNX_PF, regs))
 		return;
 
-	irqflags = fault_entry(ARM_TRAP_PABT, regs);
+	irqflags = dovetail_fault_entry(ARM_TRAP_PABT, regs);
 	pr_alert("8<--- cut here ---\n");
 	pr_alert("Unhandled prefetch abort: %s (0x%03x) at 0x%08lx\n",
 		inf->name, ifsr, addr);
 
 	arm_notify_die("", regs, inf->sig, inf->code, (void __user *)addr,
 		       ifsr, 0);
-	fault_exit(ARM_TRAP_PABT, regs, irqflags);
+	dovetail_fault_exit(ARM_TRAP_PABT, regs, irqflags);
 }
 
 /*

-- 
2.54.0

[PATCH Dovetail v3 2/6] arm: irq_pipeline: Fix dovetail_fault_{entry,exit} bypass in do_kernel_address_page_fault

[Florian Bezdeka]

In case do_kernel_address_page_fault() was entered over supervisor mode
the dovetail_fault_{entry,exit} machinery was bypassed. As a result the
inband IRQ mask might have been touched from the oob stage.

There is another problem with interrupts_enabled() in the supervisor
mode code path. That one will be addressed in a separate patch.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm/mm/fault.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index b6b766652b753ef860d16b153685b0d3b582bffa..08ebc043971000fd8d6245bcbb6c170d801c73b3 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -332,6 +332,7 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
 		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
 		dovetail_fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
 	} else {
+		irqflags = dovetail_fault_entry(ARM_TRAP_ACCESS, regs);
 		/*
 		 * Fault from kernel mode. Enable interrupts if they were
 		 * enabled in the parent context. Section (upper page table)
@@ -344,6 +345,8 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
 			local_irq_enable();
 
 		__do_kernel_fault(mm, addr, fsr, regs);
+
+		dovetail_fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
 	}
 
 	return 0;

-- 
2.54.0

[PATCH Dovetail v3 3/6] arm: irq_pipeline: Fix dovetail_fault_{entry,exit} bypass in hw_breakpoint_pending

[Florian Bezdeka]

HW breakpoint / watchpoint handling was bypassing the
dovetail_fault_{entry,exit} machinery. As a result it could happen that
the inband IRQ mask was touched from the OOB stage.

There is one more problem in the HW bp/wp handling related to
interrupts_enabled(). This one will be fixed in a separate patch.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm/include/asm/dovetail.h         | 1 +
 arch/arm/include/asm/trace/exceptions.h | 3 ++-
 arch/arm/kernel/hw_breakpoint.c         | 6 ++++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/dovetail.h b/arch/arm/include/asm/dovetail.h
index 795a1fb903ec29f4c2fb7b25e70980796804acc0..eeb5d16ad032044a2587822f6b35074d664a9bbc 100644
--- a/arch/arm/include/asm/dovetail.h
+++ b/arch/arm/include/asm/dovetail.h
@@ -16,6 +16,7 @@
 #define ARM_TRAP_VFP		6	/* VFP floating point exception */
 #define ARM_TRAP_UNDEFINSTR	7	/* Undefined instruction */
 #define ARM_TRAP_ALIGNMENT	8	/* Unaligned access exception */
+#define ARM_TRAP_HW_BREAK	9	/* HW break or watchpoint exception */
 
 #if !defined(__ASSEMBLY__)
 
diff --git a/arch/arm/include/asm/trace/exceptions.h b/arch/arm/include/asm/trace/exceptions.h
index bdb666b3da4e364e0d7e33337be3e8ad8aeedb88..f0164e55efc31a4f363764d2c5c84761f3ea34d4 100644
--- a/arch/arm/include/asm/trace/exceptions.h
+++ b/arch/arm/include/asm/trace/exceptions.h
@@ -21,7 +21,8 @@
 			__trace_trap(ARM_TRAP_FPU),		\
 			__trace_trap(ARM_TRAP_VFP),		\
 			__trace_trap(ARM_TRAP_UNDEFINSTR),	\
-			__trace_trap(ARM_TRAP_ALIGNMENT))
+			__trace_trap(ARM_TRAP_ALIGNMENT),	\
+			__trace_trap(ARM_TRAP_HW_BREAK))
 
 DECLARE_EVENT_CLASS(ARM_trap_event,
 	TP_PROTO(int trapnr, struct pt_regs *regs),
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index cd4b34c96e35e9e63e9a1ade1aeb415c22d32b00..6266380737dd88dca7fe8fde14b786a52ec42635 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -26,6 +26,7 @@
 #include <asm/current.h>
 #include <asm/hw_breakpoint.h>
 #include <asm/traps.h>
+#include <asm/trap_entry.h>
 
 /* Breakpoint currently in use for each BRP. */
 static DEFINE_PER_CPU(struct perf_event *, bp_on_reg[ARM_MAX_BRP]);
@@ -942,9 +943,12 @@ static void hw_breakpoint_cfi_handler(struct pt_regs *regs)
 static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
 				 struct pt_regs *regs)
 {
+	unsigned long irqflags;
 	int ret = 0;
 	u32 dscr;
 
+	irqflags = dovetail_fault_entry(ARM_TRAP_HW_BREAK, regs);
+
 	preempt_disable();
 
 	if (interrupts_enabled(regs))
@@ -973,6 +977,8 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
 
 	preempt_enable();
 
+	dovetail_fault_exit(ARM_TRAP_HW_BREAK, regs, irqflags);
+
 	return ret;
 }
 

-- 
2.54.0

Re: [PATCH Dovetail v3 3/6] arm: irq_pipeline: Fix dovetail_fault_{entry,exit} bypass in hw_breakpoint_pending

[Florian Bezdeka]

On Mon, 2026-06-22 at 10:05 +0200, Florian Bezdeka wrote:
> HW breakpoint / watchpoint handling was bypassing the
> dovetail_fault_{entry,exit} machinery. As a result it could happen that
> the inband IRQ mask was touched from the OOB stage.
> 
> There is one more problem in the HW bp/wp handling related to
> interrupts_enabled(). This one will be fixed in a separate patch.
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  arch/arm/include/asm/dovetail.h         | 1 +
>  arch/arm/include/asm/trace/exceptions.h | 3 ++-
>  arch/arm/kernel/hw_breakpoint.c         | 6 ++++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/dovetail.h b/arch/arm/include/asm/dovetail.h
> index 795a1fb903ec29f4c2fb7b25e70980796804acc0..eeb5d16ad032044a2587822f6b35074d664a9bbc 100644
> --- a/arch/arm/include/asm/dovetail.h
> +++ b/arch/arm/include/asm/dovetail.h
> @@ -16,6 +16,7 @@
>  #define ARM_TRAP_VFP		6	/* VFP floating point exception */
>  #define ARM_TRAP_UNDEFINSTR	7	/* Undefined instruction */
>  #define ARM_TRAP_ALIGNMENT	8	/* Unaligned access exception */
> +#define ARM_TRAP_HW_BREAK	9	/* HW break or watchpoint exception */
>  
>  #if !defined(__ASSEMBLY__)
>  
> diff --git a/arch/arm/include/asm/trace/exceptions.h b/arch/arm/include/asm/trace/exceptions.h
> index bdb666b3da4e364e0d7e33337be3e8ad8aeedb88..f0164e55efc31a4f363764d2c5c84761f3ea34d4 100644
> --- a/arch/arm/include/asm/trace/exceptions.h
> +++ b/arch/arm/include/asm/trace/exceptions.h
> @@ -21,7 +21,8 @@
>  			__trace_trap(ARM_TRAP_FPU),		\
>  			__trace_trap(ARM_TRAP_VFP),		\
>  			__trace_trap(ARM_TRAP_UNDEFINSTR),	\
> -			__trace_trap(ARM_TRAP_ALIGNMENT))
> +			__trace_trap(ARM_TRAP_ALIGNMENT),	\
> +			__trace_trap(ARM_TRAP_HW_BREAK))
>  
>  DECLARE_EVENT_CLASS(ARM_trap_event,
>  	TP_PROTO(int trapnr, struct pt_regs *regs),
> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
> index cd4b34c96e35e9e63e9a1ade1aeb415c22d32b00..6266380737dd88dca7fe8fde14b786a52ec42635 100644
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -26,6 +26,7 @@
>  #include <asm/current.h>
>  #include <asm/hw_breakpoint.h>
>  #include <asm/traps.h>
> +#include <asm/trap_entry.h>
>  
>  /* Breakpoint currently in use for each BRP. */
>  static DEFINE_PER_CPU(struct perf_event *, bp_on_reg[ARM_MAX_BRP]);
> @@ -942,9 +943,12 @@ static void hw_breakpoint_cfi_handler(struct pt_regs *regs)
>  static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
>  				 struct pt_regs *regs)
>  {
> +	unsigned long irqflags;
>  	int ret = 0;
>  	u32 dscr;
>  
> +	irqflags = dovetail_fault_entry(ARM_TRAP_HW_BREAK, regs);
> +
>  	preempt_disable();

Philippe, is that the right ordering or should I move the
dovetail_fault_entry() code below the preempt_disable() call?

Normally we want preemption enabled as long as possible and I couldn't
find a reason why the oob notify mechanism needs it already disabled,
but as always I might have missed something.

>  
>  	if (interrupts_enabled(regs))
> @@ -973,6 +977,8 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
>  
>  	preempt_enable();
>  
> +	dovetail_fault_exit(ARM_TRAP_HW_BREAK, regs, irqflags);
> +
>  	return ret;
>  }
>  
> 
> -- 
> 2.54.0

Re: [PATCH Dovetail v3 3/6] arm: irq_pipeline: Fix dovetail_fault_{entry,exit} bypass in hw_breakpoint_pending

[Philippe Gerum]

Florian Bezdeka <florian.bezdeka@siemens.com> writes:

> On Mon, 2026-06-22 at 10:05 +0200, Florian Bezdeka wrote:
>> HW breakpoint / watchpoint handling was bypassing the
>> dovetail_fault_{entry,exit} machinery. As a result it could happen that
>> the inband IRQ mask was touched from the OOB stage.
>> 
>> There is one more problem in the HW bp/wp handling related to
>> interrupts_enabled(). This one will be fixed in a separate patch.
>> 
>> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
>> ---
>>  arch/arm/include/asm/dovetail.h         | 1 +
>>  arch/arm/include/asm/trace/exceptions.h | 3 ++-
>>  arch/arm/kernel/hw_breakpoint.c         | 6 ++++++
>>  3 files changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm/include/asm/dovetail.h b/arch/arm/include/asm/dovetail.h
>> index 795a1fb903ec29f4c2fb7b25e70980796804acc0..eeb5d16ad032044a2587822f6b35074d664a9bbc 100644
>> --- a/arch/arm/include/asm/dovetail.h
>> +++ b/arch/arm/include/asm/dovetail.h
>> @@ -16,6 +16,7 @@
>>  #define ARM_TRAP_VFP		6	/* VFP floating point exception */
>>  #define ARM_TRAP_UNDEFINSTR	7	/* Undefined instruction */
>>  #define ARM_TRAP_ALIGNMENT	8	/* Unaligned access exception */
>> +#define ARM_TRAP_HW_BREAK	9	/* HW break or watchpoint exception */
>>  
>>  #if !defined(__ASSEMBLY__)
>>  
>> diff --git a/arch/arm/include/asm/trace/exceptions.h b/arch/arm/include/asm/trace/exceptions.h
>> index bdb666b3da4e364e0d7e33337be3e8ad8aeedb88..f0164e55efc31a4f363764d2c5c84761f3ea34d4 100644
>> --- a/arch/arm/include/asm/trace/exceptions.h
>> +++ b/arch/arm/include/asm/trace/exceptions.h
>> @@ -21,7 +21,8 @@
>>  			__trace_trap(ARM_TRAP_FPU),		\
>>  			__trace_trap(ARM_TRAP_VFP),		\
>>  			__trace_trap(ARM_TRAP_UNDEFINSTR),	\
>> -			__trace_trap(ARM_TRAP_ALIGNMENT))
>> +			__trace_trap(ARM_TRAP_ALIGNMENT),	\
>> +			__trace_trap(ARM_TRAP_HW_BREAK))
>>  
>>  DECLARE_EVENT_CLASS(ARM_trap_event,
>>  	TP_PROTO(int trapnr, struct pt_regs *regs),
>> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
>> index cd4b34c96e35e9e63e9a1ade1aeb415c22d32b00..6266380737dd88dca7fe8fde14b786a52ec42635 100644
>> --- a/arch/arm/kernel/hw_breakpoint.c
>> +++ b/arch/arm/kernel/hw_breakpoint.c
>> @@ -26,6 +26,7 @@
>>  #include <asm/current.h>
>>  #include <asm/hw_breakpoint.h>
>>  #include <asm/traps.h>
>> +#include <asm/trap_entry.h>
>>  
>>  /* Breakpoint currently in use for each BRP. */
>>  static DEFINE_PER_CPU(struct perf_event *, bp_on_reg[ARM_MAX_BRP]);
>> @@ -942,9 +943,12 @@ static void hw_breakpoint_cfi_handler(struct pt_regs *regs)
>>  static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
>>  				 struct pt_regs *regs)
>>  {
>> +	unsigned long irqflags;
>>  	int ret = 0;
>>  	u32 dscr;
>>  
>> +	irqflags = dovetail_fault_entry(ARM_TRAP_HW_BREAK, regs);
>> +
>>  	preempt_disable();
>
> Philippe, is that the right ordering or should I move the
> dovetail_fault_entry() code below the preempt_disable() call?
>
> Normally we want preemption enabled as long as possible and I couldn't
> find a reason why the oob notify mechanism needs it already disabled,
> but as always I might have missed something.
>

I don't see any reason to disable in-band preemption before handling the
fault entry, especially with a potential stage switch. The ordering you
used is correct.

-- 
Philippe.

[PATCH Dovetail v3 4/6] arm: irq_pipeline: Fix ordering problem in alignment trap handling

[Florian Bezdeka]

mark_trap_entry() will migrate to the inband stage and enable hard
IRQs. With that the IRQ pipelining is active, allowing hard IRQs to
arrive but keeping the inband stage stalled.

Moving the mark_trap_entry() above the - still wrong -
interrupts_enabled() to follow the same pattern as in other trap
handlers. As we are inband after the call we can now enable inband IRQs
if the parent context allows that.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm/mm/alignment.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 76f2716635b73a0001efc3308777fc6a3bd712ca..37023b96a468531c026ea5f6c47d25f3d49fd6a7 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -815,11 +815,11 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (addr >= TASK_SIZE && user_mode(regs))
 		harden_branch_predictor();
 
-	if (interrupts_enabled(regs))
-		hard_local_irq_enable();
-
 	mark_trap_entry(ARM_TRAP_ALIGNMENT, regs);
 
+	if (interrupts_enabled(regs))
+		local_irq_enable();
+
 	instrptr = instruction_pointer(regs);
 
 	if (thumb_mode(regs)) {

-- 
2.54.0

[PATCH Dovetail v3 5/6] dovetail: Decouple kernel/irq/pipeline.c from irqstate bit definitions

[Florian Bezdeka]

This only affects architectures where CONFIG_GENERIC_ENTRY is not set.

Moving the stall bit definitions into the architecture specific
implementations allows re-use of those definition within the arch
specifics.

Moving the defines to include/linux/irq_pipeline.h turned out to
trigger a lot of trouble as re-use will live in asm/ where including
a "general" header is quite hard.

The platform independent implementation will now need the following
arch specific hooks, all operating on the platform specific
struct pt_regs:
  - arch_kentry_irqs_enabled()
  - arch_kentry_lockdep_set()
  - arch_kentry_clear_irq_state()
  - arch_kentry_set_irqs_stalled()
  - arch_kentry_set_lockdep()

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm/include/asm/irq_pipeline.h   | 41 +++++++++++++++++++++++++++--------
 arch/arm64/include/asm/irq_pipeline.h |  9 ++++----
 kernel/irq/pipeline.c                 | 20 +++++------------
 3 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/arch/arm/include/asm/irq_pipeline.h b/arch/arm/include/asm/irq_pipeline.h
index 5e3a20149246505e8401bc7157d9c9f13673f45e..d1ee5cefd407a117eca5a1907848580e0a5bb622 100644
--- a/arch/arm/include/asm/irq_pipeline.h
+++ b/arch/arm/include/asm/irq_pipeline.h
@@ -98,15 +98,33 @@ static inline void arch_handle_irq_pipelined(struct pt_regs *regs)
 	handle_arch_irq(regs);
 }
 
-#define arch_kentry_get_irqstate(__regs)		\
-	({						\
-		to_svc_pt_regs(__regs)->irqstate;	\
-	})
-
-#define arch_kentry_set_irqstate(__regs, __irqstate)		\
-	do {							\
-		to_svc_pt_regs(__regs)->irqstate = __irqstate;	\
-	} while (0)
+#define KENTRY_STALL_BIT      BIT(0) /* Tracks INBAND_STALL_BIT */
+#define KENTRY_LOCKDEP_BIT    BIT(1) /* Tracks hardirqs_enabled */
+
+#define arch_kentry_irqs_enabled(__regs)			\
+({								\
+	!(to_svc_pt_regs(__regs)->irqstate & KENTRY_STALL_BIT);	\
+})
+
+#define arch_kentry_lockdep_set(__regs)				\
+({								\
+	to_svc_pt_regs(__regs)->irqstate & KENTRY_LOCKDEP_BIT;	\
+})
+
+#define arch_kentry_clear_irq_state(__regs)			\
+({								\
+	to_svc_pt_regs(__regs)->irqstate = 0;			\
+})
+
+#define arch_kentry_set_irqs_stalled(__regs)			\
+({								\
+	to_svc_pt_regs(__regs)->irqstate |= KENTRY_STALL_BIT;	\
+})
+
+#define arch_kentry_set_lockdep(__regs)				\
+({								\
+	to_svc_pt_regs(__regs)->irqstate |= KENTRY_LOCKDEP_BIT;	\
+})
 
 int handle_arch_irq_pipelined(struct pt_regs *regs);
 
@@ -142,6 +160,11 @@ static inline int arch_irqs_disabled_flags(unsigned long flags)
 	return native_irqs_disabled_flags(flags);
 }
 
+#define arch_kentry_irqs_enabled(regs)	\
+({					\
+	!interrupts_enabled(regs);	\
+})
+
 #endif /* !CONFIG_IRQ_PIPELINE */
 
 #endif /* _ASM_ARM_IRQ_PIPELINE_H */
diff --git a/arch/arm64/include/asm/irq_pipeline.h b/arch/arm64/include/asm/irq_pipeline.h
index 09cb33b826cb63701678f40b61b65df885923db2..c4f9a39ad323bf4cd5c1ff59e43c64b1a87b49cf 100644
--- a/arch/arm64/include/asm/irq_pipeline.h
+++ b/arch/arm64/include/asm/irq_pipeline.h
@@ -102,10 +102,11 @@ static inline void arch_handle_irq_pipelined(struct pt_regs *regs)
  * the latter for now, until we enventually switch to using whichever
  * of them is available first.
  */
-#define arch_kentry_get_irqstate(__regs)	0
-
-#define arch_kentry_set_irqstate(__regs, __irqstate)	\
-	do { (void)__irqstate; } while (0)
+#define arch_kentry_irqs_enabled(__regs)	0
+#define arch_kentry_lockdep_set(__regs)		0
+#define arch_kentry_clear_irq_state(__regs)
+#define arch_kentry_set_irqs_stalled(__regs)
+#define arch_kentry_set_lockdep(__regs)
 
 #else  /* !CONFIG_IRQ_PIPELINE */
 
diff --git a/kernel/irq/pipeline.c b/kernel/irq/pipeline.c
index ced7003a8444125ff38d71bf867ce34cfa0235f1..34a62805def0e3a5a70893a8ea71d6c200bd8e70 100644
--- a/kernel/irq/pipeline.c
+++ b/kernel/irq/pipeline.c
@@ -1401,33 +1401,26 @@ void sync_current_irq_stage(void) /* hard irqs off */
  * in kernel context, indexed on the current register frame.
  */
 
-#define KENTRY_STALL_BIT      BIT(0) /* Tracks INBAND_STALL_BIT */
-#define KENTRY_LOCKDEP_BIT    BIT(1) /* Tracks hardirqs_enabled */
-
 asmlinkage __visible noinstr void kentry_enter_pipelined(struct pt_regs *regs)
 {
-	long irqstate = 0;
-
 	WARN_ON(irq_pipeline_debug() && !hard_irqs_disabled());
 
 	if (!running_inband())
 		return;
 
+	arch_kentry_clear_irq_state(regs);
+
 	if (lockdep_read_irqs_state())
-		irqstate |= KENTRY_LOCKDEP_BIT;
+		arch_kentry_set_lockdep(regs);
 
 	if (irqs_disabled())
-		irqstate |= KENTRY_STALL_BIT;
+		arch_kentry_set_irqs_stalled(regs);
 	else
 		trace_hardirqs_off();
-
-	arch_kentry_set_irqstate(regs, irqstate);
 }
 
 asmlinkage void __visible noinstr kentry_exit_pipelined(struct pt_regs *regs)
 {
-	long irqstate;
-
 	WARN_ON(irq_pipeline_debug() && !hard_irqs_disabled());
 
 	if (!running_inband())
@@ -1444,13 +1437,12 @@ asmlinkage void __visible noinstr kentry_exit_pipelined(struct pt_regs *regs)
 	 * do flip the stall bit, but are not tracked by lockdep).
 	 */
 
-	irqstate = arch_kentry_get_irqstate(regs);
-	if (!(irqstate & KENTRY_STALL_BIT)) {
+	if (arch_kentry_irqs_enabled(regs)) {
 		stall_inband_nocheck();
 		trace_hardirqs_on();
 		unstall_inband_nocheck();
 	} else {
-		lockdep_write_irqs_state(!!(irqstate & KENTRY_LOCKDEP_BIT));
+		lockdep_write_irqs_state(arch_kentry_lockdep_set(regs));
 	}
 }
 

-- 
2.54.0

[PATCH Dovetail v3 6/6] arm: irq_pipeline: Fix IRQ state checking in trap entry paths

[Florian Bezdeka]

A couple of trap handlers where checking the hardware IRQ state after
inband migration instead of the inband stage state.

As we now have arch_kentry_irqs_enabled() available we can migrate all
callers.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm/kernel/hw_breakpoint.c | 2 +-
 arch/arm/mm/alignment.c         | 2 +-
 arch/arm/mm/fault.c             | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index 6266380737dd88dca7fe8fde14b786a52ec42635..c94383d8d81da902d9bee1240ad14d91526f70a4 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -951,7 +951,7 @@ static int hw_breakpoint_pending(unsigned long addr, unsigned int fsr,
 
 	preempt_disable();
 
-	if (interrupts_enabled(regs))
+	if (arch_kentry_irqs_enabled(regs))
 		local_irq_enable();
 
 	/* We only handle watchpoints and hardware breakpoints. */
diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 37023b96a468531c026ea5f6c47d25f3d49fd6a7..1af7d306fcb1f09497b026e30ad49891ec5a3ff1 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -817,7 +817,7 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 
 	mark_trap_entry(ARM_TRAP_ALIGNMENT, regs);
 
-	if (interrupts_enabled(regs))
+	if (arch_kentry_irqs_enabled(regs))
 		local_irq_enable();
 
 	instrptr = instruction_pointer(regs);
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 08ebc043971000fd8d6245bcbb6c170d801c73b3..049c2733e695ee4e4a4d228f023c2e0bf755196c 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -341,7 +341,7 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
 		 * PTE or PTE permission fault. This may happen in exceptional
 		 * circumstances and need the fixup tables to be walked.
 		 */
-		if (interrupts_enabled(regs))
+		if (arch_kentry_irqs_enabled(regs))
 			local_irq_enable();
 
 		__do_kernel_fault(mm, addr, fsr, regs);

-- 
2.54.0