[PATCH 0/6] ARM: cleanup fault handling

[PATCH 0/6] ARM: cleanup fault handling

[Russell King (Oracle)]

Hi,

This series cleans up the 32-bit ARM fault handling:

1. ensure a consistent IRQ state in the user fault processing path.
2. split vmalloc fault processing in a similar way to x86
3. move is_..._fault() to the local fault.h header file
4. update the FSR deifnitions using BIT() and GENMASK()
5. move LPAE/non-LPAE specific FSR bits/masks along side users
6. get rid of #ifdefs within is_..._fault() by providing a LPAE
   and non-LPAE versions.

 arch/arm/mm/fault.c | 158 ++++++++++++++++++++++++----------------------------
 arch/arm/mm/fault.h |  42 ++++++++++++--
 2 files changed, 109 insertions(+), 91 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH 1/6] ARM: ensure interrupts are enabled in __do_user_fault()

[Russell King (Oracle)]

__do_user_fault() may be called from fault handling paths where the
interrupts are enabled or disabled. E.g. do_page_fault() calls this
with interrupts enabled, whereas do_sect_fault()->do_bad_area()
will call this with interrupts disabled. Since this is a userspace
fault, we know that interrupts were enabled in the parent context,
so call local_irq_enable() here to give a consistent interrupt state.

This is necessary for force_sig_info() when PREEMPT_RT is enabled.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index ed4330cc3f4e..6c27ebd49093 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -190,7 +190,8 @@ __do_kernel_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
 
 /*
  * Something tried to access memory that isn't in our memory map..
- * User mode accesses just cause a SIGSEGV
+ * User mode accesses just cause a SIGSEGV. Ensure interrupts are enabled
+ * for preempt RT.
  */
 static void
 __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
@@ -198,6 +199,8 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
 {
 	struct task_struct *tsk = current;
 
+	local_irq_enable();
+
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
@@ -268,6 +271,7 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
 		 * should not be faulting in kernel space, which includes the
 		 * vector/khelper page. Handle the branch predictor hardening
 		 * while interrupts are still disabled, then send a SIGSEGV.
+		 * Note that __do_user_fault() will enable interrupts.
 		 */
 		harden_branch_predictor();
 		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
-- 
2.47.3

[PATCH 2/6] ARM: move vmalloc() lazy-page table population

[Russell King (Oracle)]

Split the vmalloc() lazy-page table population from
do_translation_fault() into a new vmalloc_fault() function.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.c | 126 ++++++++++++++++++++++++--------------------
 1 file changed, 68 insertions(+), 58 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 6c27ebd49093..0f3b6cc516c1 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -261,6 +261,70 @@ static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
 }
 #endif
 
+/*
+ * Handle a vmalloc fault, copying the non-leaf page table entries from
+ * init_mm.pgd. Any kernel context can trigger this, so we must not sleep
+ * or enable interrupts. Having two CPUs execute this for the same page is
+ * no problem, we'll just copy the same data twice.
+ *
+ * Returns false on failure.
+ */
+static bool __kprobes __maybe_unused vmalloc_fault(unsigned long addr)
+{
+	unsigned int index;
+	pgd_t *pgd, *pgd_k;
+	p4d_t *p4d, *p4d_k;
+	pud_t *pud, *pud_k;
+	pmd_t *pmd, *pmd_k;
+
+	index = pgd_index(addr);
+
+	pgd = cpu_get_pgd() + index;
+	pgd_k = init_mm.pgd + index;
+
+	p4d = p4d_offset(pgd, addr);
+	p4d_k = p4d_offset(pgd_k, addr);
+
+	if (p4d_none(*p4d_k))
+		return false;
+	if (!p4d_present(*p4d))
+		set_p4d(p4d, *p4d_k);
+
+	pud = pud_offset(p4d, addr);
+	pud_k = pud_offset(p4d_k, addr);
+
+	if (pud_none(*pud_k))
+		return false;
+	if (!pud_present(*pud))
+		set_pud(pud, *pud_k);
+
+	pmd = pmd_offset(pud, addr);
+	pmd_k = pmd_offset(pud_k, addr);
+
+#ifdef CONFIG_ARM_LPAE
+	/*
+	 * Only one hardware entry per PMD with LPAE.
+	 */
+	index = 0;
+#else
+	/*
+	 * On ARM one Linux PGD entry contains two hardware entries (see page
+	 * tables layout in pgtable.h). We normally guarantee that we always
+	 * fill both L1 entries. But create_mapping() doesn't follow the rule.
+	 * It can create inidividual L1 entries, so here we have to call
+	 * pmd_none() check for the entry really corresponded to address, not
+	 * for the first of pair.
+	 */
+	index = (addr >> SECTION_SHIFT) & 1;
+#endif
+	if (pmd_none(pmd_k[index]))
+		return false;
+
+	copy_pmd(pmd, pmd_k);
+
+	return true;
+}
+
 static int __kprobes
 do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
 			     unsigned int fsr, struct pt_regs *regs)
@@ -496,10 +560,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
  * directly to do_kernel_address_page_fault() to handle.
  *
  * Otherwise, we're probably faulting in the vmalloc() area, so try to fix
- * that up. Note that we must not take any locks or enable interrupts in
- * this case.
+ * that up via vmalloc_fault().
  *
- * If vmalloc() fixup fails, that means the non-leaf page tables did not
+ * If vmalloc_fault() fails, that means the non-leaf page tables did not
  * contain an entry for this address, so handle this via
  * do_kernel_address_page_fault().
  */
@@ -508,65 +571,12 @@ static int __kprobes
 do_translation_fault(unsigned long addr, unsigned int fsr,
 		     struct pt_regs *regs)
 {
-	unsigned int index;
-	pgd_t *pgd, *pgd_k;
-	p4d_t *p4d, *p4d_k;
-	pud_t *pud, *pud_k;
-	pmd_t *pmd, *pmd_k;
-
 	if (addr < TASK_SIZE)
 		return do_page_fault(addr, fsr, regs);
 
-	if (user_mode(regs))
-		goto bad_area;
-
-	index = pgd_index(addr);
-
-	pgd = cpu_get_pgd() + index;
-	pgd_k = init_mm.pgd + index;
-
-	p4d = p4d_offset(pgd, addr);
-	p4d_k = p4d_offset(pgd_k, addr);
-
-	if (p4d_none(*p4d_k))
-		goto bad_area;
-	if (!p4d_present(*p4d))
-		set_p4d(p4d, *p4d_k);
-
-	pud = pud_offset(p4d, addr);
-	pud_k = pud_offset(p4d_k, addr);
-
-	if (pud_none(*pud_k))
-		goto bad_area;
-	if (!pud_present(*pud))
-		set_pud(pud, *pud_k);
-
-	pmd = pmd_offset(pud, addr);
-	pmd_k = pmd_offset(pud_k, addr);
-
-#ifdef CONFIG_ARM_LPAE
-	/*
-	 * Only one hardware entry per PMD with LPAE.
-	 */
-	index = 0;
-#else
-	/*
-	 * On ARM one Linux PGD entry contains two hardware entries (see page
-	 * tables layout in pgtable.h). We normally guarantee that we always
-	 * fill both L1 entries. But create_mapping() doesn't follow the rule.
-	 * It can create inidividual L1 entries, so here we have to call
-	 * pmd_none() check for the entry really corresponded to address, not
-	 * for the first of pair.
-	 */
-	index = (addr >> SECTION_SHIFT) & 1;
-#endif
-	if (pmd_none(pmd_k[index]))
-		goto bad_area;
-
-	copy_pmd(pmd, pmd_k);
-	return 0;
+	if (!user_mode(regs) && vmalloc_fault(addr))
+		return 0;
 
-bad_area:
 	do_kernel_address_page_fault(current->mm, addr, fsr, regs);
 
 	return 0;
-- 
2.47.3

[PATCH 3/6] ARM: move is_permission_fault() and is_translation_fault() to fault.h

[Russell King (Oracle)]

is_permission_fault() and is_translation_fault() are both conditional
on the FSR encodings, which are dependent on LPAE. We define the
constants in fault.h. Move these inline functions to fault.h to be
near the FSR definitions.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.c | 26 --------------------------
 arch/arm/mm/fault.h | 26 ++++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 0f3b6cc516c1..e62cc4be5adf 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -115,32 +115,6 @@ static inline bool is_write_fault(unsigned int fsr)
 	return (fsr & FSR_WRITE) && !(fsr & FSR_CM);
 }
 
-static inline bool is_translation_fault(unsigned int fsr)
-{
-	int fs = fsr_fs(fsr);
-#ifdef CONFIG_ARM_LPAE
-	if ((fs & FS_MMU_NOLL_MASK) == FS_TRANS_NOLL)
-		return true;
-#else
-	if (fs == FS_L1_TRANS || fs == FS_L2_TRANS)
-		return true;
-#endif
-	return false;
-}
-
-static inline bool is_permission_fault(unsigned int fsr)
-{
-	int fs = fsr_fs(fsr);
-#ifdef CONFIG_ARM_LPAE
-	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
-		return true;
-#else
-	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
-		return true;
-#endif
-	return false;
-}
-
 static void die_kernel_fault(const char *msg, struct mm_struct *mm,
 			     unsigned long addr, unsigned int fsr,
 			     struct pt_regs *regs)
diff --git a/arch/arm/mm/fault.h b/arch/arm/mm/fault.h
index e8f8c1902544..e95f44757dc9 100644
--- a/arch/arm/mm/fault.h
+++ b/arch/arm/mm/fault.h
@@ -35,6 +35,32 @@ static inline int fsr_fs(unsigned int fsr)
 }
 #endif
 
+static inline bool is_translation_fault(unsigned int fsr)
+{
+	int fs = fsr_fs(fsr);
+#ifdef CONFIG_ARM_LPAE
+	if ((fs & FS_MMU_NOLL_MASK) == FS_TRANS_NOLL)
+		return true;
+#else
+	if (fs == FS_L1_TRANS || fs == FS_L2_TRANS)
+		return true;
+#endif
+	return false;
+}
+
+static inline bool is_permission_fault(unsigned int fsr)
+{
+	int fs = fsr_fs(fsr);
+#ifdef CONFIG_ARM_LPAE
+	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
+		return true;
+#else
+	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
+		return true;
+#endif
+	return false;
+}
+
 void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs);
 void early_abt_enable(void);
 asmlinkage void do_DataAbort(unsigned long addr, unsigned int fsr,
-- 
2.47.3

[PATCH 4/6] ARM: use BIT() and GENMASK() for fault status register fields

[Russell King (Oracle)]

Modernise the fault status field definitions by using BIT() and
GENMASK().

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mm/fault.h b/arch/arm/mm/fault.h
index e95f44757dc9..d2bdedaefe14 100644
--- a/arch/arm/mm/fault.h
+++ b/arch/arm/mm/fault.h
@@ -5,12 +5,12 @@
 /*
  * Fault status register encodings.  We steal bit 31 for our own purposes.
  */
-#define FSR_LNX_PF		(1 << 31)
-#define FSR_CM			(1 << 13)
-#define FSR_WRITE		(1 << 11)
-#define FSR_FS4			(1 << 10)
-#define FSR_FS3_0		(15)
-#define FSR_FS5_0		(0x3f)
+#define FSR_LNX_PF		BIT(31)
+#define FSR_CM			BIT(13)
+#define FSR_WRITE		BIT(11)
+#define FSR_FS4			BIT(10)
+#define FSR_FS3_0		GENMASK(3, 0)
+#define FSR_FS5_0		GENMASK(5, 0)
 
 #ifdef CONFIG_ARM_LPAE
 #define FSR_FS_AEA		17
-- 
2.47.3

[PATCH 5/6] ARM: move FSR fault status definitions before fsr_fs()

[Russell King (Oracle)]

The FSR's fault status bits depend on whether LPAE is enabled. Rather
than always exposing both LPAE and non-LPAE to all code, move them
inside the ifdef blocks dependent on LPAE to restrict their visibility.
No code other than fsr_fs() makes use of these.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/fault.h b/arch/arm/mm/fault.h
index d2bdedaefe14..44c0fad29cce 100644
--- a/arch/arm/mm/fault.h
+++ b/arch/arm/mm/fault.h
@@ -8,9 +8,6 @@
 #define FSR_LNX_PF		BIT(31)
 #define FSR_CM			BIT(13)
 #define FSR_WRITE		BIT(11)
-#define FSR_FS4			BIT(10)
-#define FSR_FS3_0		GENMASK(3, 0)
-#define FSR_FS5_0		GENMASK(5, 0)
 
 #ifdef CONFIG_ARM_LPAE
 #define FSR_FS_AEA		17
@@ -18,6 +15,8 @@
 #define FS_PERM_NOLL		0xC
 #define FS_MMU_NOLL_MASK	0x3C
 
+#define FSR_FS5_0		GENMASK(5, 0)
+
 static inline int fsr_fs(unsigned int fsr)
 {
 	return fsr & FSR_FS5_0;
@@ -29,6 +28,9 @@ static inline int fsr_fs(unsigned int fsr)
 #define FS_L1_PERM		0xD
 #define FS_L2_PERM		0xF
 
+#define FSR_FS4			BIT(10)
+#define FSR_FS3_0		GENMASK(3, 0)
+
 static inline int fsr_fs(unsigned int fsr)
 {
 	return (fsr & FSR_FS3_0) | (fsr & FSR_FS4) >> 6;
-- 
2.47.3

[PATCH 6/6] ARM: provide individual is_translation_fault() and is_permission_fault()

[Russell King (Oracle)]

Provide individual LPAE and non-LPAE definitions for both these
functions, rather than having ifdefs inside the function body. This
places the functions closer to their associated definitions.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/fault.h | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mm/fault.h b/arch/arm/mm/fault.h
index 44c0fad29cce..207f1b06941d 100644
--- a/arch/arm/mm/fault.h
+++ b/arch/arm/mm/fault.h
@@ -21,6 +21,20 @@ static inline int fsr_fs(unsigned int fsr)
 {
 	return fsr & FSR_FS5_0;
 }
+
+static inline bool is_translation_fault(unsigned int fsr)
+{
+	int fs = fsr_fs(fsr);
+
+	return (fs & FS_MMU_NOLL_MASK) == FS_TRANS_NOLL;
+}
+
+static inline bool is_permission_fault(unsigned int fsr)
+{
+	int fs = fsr_fs(fsr);
+
+	return (fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL;
+}
 #else
 #define FSR_FS_AEA		22
 #define FS_L1_TRANS		0x5
@@ -35,33 +49,21 @@ static inline int fsr_fs(unsigned int fsr)
 {
 	return (fsr & FSR_FS3_0) | (fsr & FSR_FS4) >> 6;
 }
-#endif
 
 static inline bool is_translation_fault(unsigned int fsr)
 {
 	int fs = fsr_fs(fsr);
-#ifdef CONFIG_ARM_LPAE
-	if ((fs & FS_MMU_NOLL_MASK) == FS_TRANS_NOLL)
-		return true;
-#else
-	if (fs == FS_L1_TRANS || fs == FS_L2_TRANS)
-		return true;
-#endif
-	return false;
+
+	return fs == FS_L1_TRANS || fs == FS_L2_TRANS;
 }
 
 static inline bool is_permission_fault(unsigned int fsr)
 {
 	int fs = fsr_fs(fsr);
-#ifdef CONFIG_ARM_LPAE
-	if ((fs & FS_MMU_NOLL_MASK) == FS_PERM_NOLL)
-		return true;
-#else
-	if (fs == FS_L1_PERM || fs == FS_L2_PERM)
-		return true;
-#endif
-	return false;
+
+	return fs == FS_L1_PERM || fs == FS_L2_PERM;
 }
+#endif
 
 void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs);
 void early_abt_enable(void);
-- 
2.47.3

Re: [PATCH 1/6] ARM: ensure interrupts are enabled in __do_user_fault()

[Sebastian Andrzej Siewior]

On 2026-02-27 15:19:08 [+0000], Russell King (Oracle) wrote:
> __do_user_fault() may be called from fault handling paths where the
> interrupts are enabled or disabled. E.g. do_page_fault() calls this
> with interrupts enabled, whereas do_sect_fault()->do_bad_area()
> will call this with interrupts disabled. Since this is a userspace
> fault, we know that interrupts were enabled in the parent context,
> so call local_irq_enable() here to give a consistent interrupt state.
> 
> This is necessary for force_sig_info() when PREEMPT_RT is enabled.

Reported-by: Yadi.hu <yadi.hu@windriver.com
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  arch/arm/mm/fault.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index ed4330cc3f4e..6c27ebd49093 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -190,7 +190,8 @@ __do_kernel_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
>  
>  /*
>   * Something tried to access memory that isn't in our memory map..
> - * User mode accesses just cause a SIGSEGV
> + * User mode accesses just cause a SIGSEGV. Ensure interrupts are enabled
> + * for preempt RT.
          PREEMPT_RT.
>   */
>  static void
>  __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,

Sebastian

Re: [PATCH 2/6] ARM: move vmalloc() lazy-page table population

[Sebastian Andrzej Siewior]

On 2026-02-27 15:19:13 [+0000], Russell King (Oracle) wrote:
> Split the vmalloc() lazy-page table population from
> do_translation_fault() into a new vmalloc_fault() function.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -261,6 +261,70 @@ static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
>  }
>  #endif
>  
> +/*
> + * Handle a vmalloc fault, copying the non-leaf page table entries from
> + * init_mm.pgd. Any kernel context can trigger this, so we must not sleep
> + * or enable interrupts. Having two CPUs execute this for the same page is

"unconditionally enable interrupts."? It wouldn't be wrong to enable
them if the calling context had them enabled, right?

> + * no problem, we'll just copy the same data twice.
> + *
> + * Returns false on failure.
> + */
> +static bool __kprobes __maybe_unused vmalloc_fault(unsigned long addr)

Maybe handle_vmalloc_fault().

> +{

Sebastian

Re: [PATCH 3/6] ARM: move is_permission_fault() and is_translation_fault() to fault.h

[Sebastian Andrzej Siewior]

On 2026-02-27 15:19:18 [+0000], Russell King (Oracle) wrote:
> is_permission_fault() and is_translation_fault() are both conditional
> on the FSR encodings, which are dependent on LPAE. We define the
> constants in fault.h. Move these inline functions to fault.h to be
> near the FSR definitions.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian

Re: [PATCH 4/6] ARM: use BIT() and GENMASK() for fault status register fields

[Sebastian Andrzej Siewior]

On 2026-02-27 15:19:23 [+0000], Russell King (Oracle) wrote:
> Modernise the fault status field definitions by using BIT() and
> GENMASK().
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian

Re: [PATCH 5/6] ARM: move FSR fault status definitions before fsr_fs()

[Sebastian Andrzej Siewior]

On 2026-02-27 15:19:28 [+0000], Russell King (Oracle) wrote:
> The FSR's fault status bits depend on whether LPAE is enabled. Rather
> than always exposing both LPAE and non-LPAE to all code, move them
> inside the ifdef blocks dependent on LPAE to restrict their visibility.
> No code other than fsr_fs() makes use of these.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian

Re: [PATCH 6/6] ARM: provide individual is_translation_fault() and is_permission_fault()

[Sebastian Andrzej Siewior]

On 2026-02-27 15:19:34 [+0000], Russell King (Oracle) wrote:
> Provide individual LPAE and non-LPAE definitions for both these
> functions, rather than having ifdefs inside the function body. This
> places the functions closer to their associated definitions.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian

Re: [PATCH 2/6] ARM: move vmalloc() lazy-page table population

[Russell King (Oracle)]

On Mon, Mar 02, 2026 at 11:43:51AM +0100, Sebastian Andrzej Siewior wrote:
> On 2026-02-27 15:19:13 [+0000], Russell King (Oracle) wrote:
> > Split the vmalloc() lazy-page table population from
> > do_translation_fault() into a new vmalloc_fault() function.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> > --- a/arch/arm/mm/fault.c
> > +++ b/arch/arm/mm/fault.c
> > @@ -261,6 +261,70 @@ static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
> >  }
> >  #endif
> >  
> > +/*
> > + * Handle a vmalloc fault, copying the non-leaf page table entries from
> > + * init_mm.pgd. Any kernel context can trigger this, so we must not sleep
> > + * or enable interrupts. Having two CPUs execute this for the same page is
> 
> "unconditionally enable interrupts."? It wouldn't be wrong to enable
> them if the calling context had them enabled, right?

I don't know where you got "unconditionally enable interrupts" from the
comment - it says the exact opposite.

> 
> > + * no problem, we'll just copy the same data twice.
> > + *
> > + * Returns false on failure.
> > + */
> > +static bool __kprobes __maybe_unused vmalloc_fault(unsigned long addr)
> 
> Maybe handle_vmalloc_fault().

x86:

static noinline int vmalloc_fault(unsigned long address)
{
...

I prefer to keep the name the same.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 0/6] ARM: cleanup fault handling

[Sebastian Andrzej Siewior]

On 2026-02-27 15:18:26 [+0000], Russell King (Oracle) wrote:
> Hi,
Hi,

> This series cleans up the 32-bit ARM fault handling:
> 
> 1. ensure a consistent IRQ state in the user fault processing path.
> 2. split vmalloc fault processing in a similar way to x86
> 3. move is_..._fault() to the local fault.h header file
> 4. update the FSR deifnitions using BIT() and GENMASK()
> 5. move LPAE/non-LPAE specific FSR bits/masks along side users
> 6. get rid of #ifdefs within is_..._fault() by providing a LPAE
>    and non-LPAE versions.

Thank you Russell. With this series (notably #1) the PREEMPT_RT support
can be enabled. Would you mind adding "ARM: Allow to enable RT" from
	https://lore.kernel.org/all/20260226111742.3598421-3-bigeasy@linutronix.de/

to the series/ merge it with it? Or should I just send it to your patch
system?

Sebastian

Re: [PATCH 2/6] ARM: move vmalloc() lazy-page table population

[Sebastian Andrzej Siewior]

On 2026-03-02 10:57:03 [+0000], Russell King (Oracle) wrote:
> > > --- a/arch/arm/mm/fault.c
> > > +++ b/arch/arm/mm/fault.c
> > > @@ -261,6 +261,70 @@ static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
> > >  }
> > >  #endif
> > >  
> > > +/*
> > > + * Handle a vmalloc fault, copying the non-leaf page table entries from
> > > + * init_mm.pgd. Any kernel context can trigger this, so we must not sleep
> > > + * or enable interrupts. Having two CPUs execute this for the same page is
> > 
> > "unconditionally enable interrupts."? It wouldn't be wrong to enable
> > them if the calling context had them enabled, right?
> 
> I don't know where you got "unconditionally enable interrupts" from the
> comment - it says the exact opposite.

My point. You could enable the interrupts if the parent context had them
enabled. It is not that you must keep them disabled because a second
fault/ interrupt would overwrite your CPU state.

> > > + * no problem, we'll just copy the same data twice.
> > > + *
> > > + * Returns false on failure.
> > > + */
> > > +static bool __kprobes __maybe_unused vmalloc_fault(unsigned long addr)
> > 
> > Maybe handle_vmalloc_fault().
> 
> x86:
> 
> static noinline int vmalloc_fault(unsigned long address)
> {
> ...
> 
> I prefer to keep the name the same.
Okay.

Sebastian

Re: [PATCH 2/6] ARM: move vmalloc() lazy-page table population

[Russell King (Oracle)]

On Mon, Mar 02, 2026 at 12:00:55PM +0100, Sebastian Andrzej Siewior wrote:
> On 2026-03-02 10:57:03 [+0000], Russell King (Oracle) wrote:
> > > > --- a/arch/arm/mm/fault.c
> > > > +++ b/arch/arm/mm/fault.c
> > > > @@ -261,6 +261,70 @@ static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
> > > >  }
> > > >  #endif
> > > >  
> > > > +/*
> > > > + * Handle a vmalloc fault, copying the non-leaf page table entries from
> > > > + * init_mm.pgd. Any kernel context can trigger this, so we must not sleep
> > > > + * or enable interrupts. Having two CPUs execute this for the same page is
> > > 
> > > "unconditionally enable interrupts."? It wouldn't be wrong to enable
> > > them if the calling context had them enabled, right?
> > 
> > I don't know where you got "unconditionally enable interrupts" from the
> > comment - it says the exact opposite.
> 
> My point. You could enable the interrupts if the parent context had them
> enabled. It is not that you must keep them disabled because a second
> fault/ interrupt would overwrite your CPU state.

We've gone from the code always having interrupts disabled, to your
review response that always wants them to be enabled (which is what
*un*conditionally enable interrupts means) to now conditionally enable
interrupts.

We are reading the CPU's page table pointer (the one programmed in
hardware) here, and we don't want that changing while we're fixing
the fault. So, interrupts need to stay disabled. x86 doesn't enable
interrupts in this path either, and we shouldn't be having differing
behaviour without good reason.

There are two places where x86 enables interrupts in its fault
handling. One of them is while handling a user address fault in
do_user_addr_fault(). The other is __bad_area_nosemaphore() where we
also know that we've come from user mode. All other fault handling
happens with interrupts disabled.

In any case, we've always had interrupts disabled in this path. Any
change to that would need to be a separate patch.

So, the comment in the patch is 100% correct.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/6] ARM: move vmalloc() lazy-page table population

[Sebastian Andrzej Siewior]

On 2026-03-02 11:19:52 [+0000], Russell King (Oracle) wrote:
> We've gone from the code always having interrupts disabled, to your
> review response that always wants them to be enabled (which is what
> *un*conditionally enable interrupts means) to now conditionally enable
> interrupts.

I'm sorry but this got out of hand.
The wording was we must "not enable interrupts" and I was curious if
this is because we destroy some information (on the following fault/
interrupt) _or_ not. If we don't destroy anything then it would be okay
to enable them conditionally. At no point I am suggesting to do so.

Sorry for the confusion here.

Sebastian

Re: [PATCH 0/6] ARM: cleanup fault handling

[Sebastian Andrzej Siewior]

On 2026-03-02 11:57:37 [+0100], To Russell King (Oracle) wrote:
> On 2026-02-27 15:18:26 [+0000], Russell King (Oracle) wrote:
> > Hi,
> Hi,
Hi Russell,

> > This series cleans up the 32-bit ARM fault handling:
> > 
> > 1. ensure a consistent IRQ state in the user fault processing path.
> > 2. split vmalloc fault processing in a similar way to x86
> > 3. move is_..._fault() to the local fault.h header file
> > 4. update the FSR deifnitions using BIT() and GENMASK()
> > 5. move LPAE/non-LPAE specific FSR bits/masks along side users
> > 6. get rid of #ifdefs within is_..._fault() by providing a LPAE
> >    and non-LPAE versions.
> 
> Thank you Russell. With this series (notably #1) the PREEMPT_RT support
> can be enabled. Would you mind adding "ARM: Allow to enable RT" from
> 	https://lore.kernel.org/all/20260226111742.3598421-3-bigeasy@linutronix.de/
> 
> to the series/ merge it with it? Or should I just send it to your patch
> system?

Russell, any suggestion how to move on here?

Sebastian