[PATCH v2 0/4] alpha: stack fixes

[PATCH v2 0/4] alpha: stack fixes

[Ivan Kokshaysky]

This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1]
Thanks to Magnus Lindholm for identifying that remarkably longstanding
bug.

The problem is that GCC expects 16-byte alignment of the incoming stack
since early 2004, as Maciej found out [2]:
  Having actually dug speculatively I can see that the psABI was changed in
 GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double
 improperly aligned)") back in Mar 2004, when the stack pointer alignment
 was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has
 various suspicious stack pointer adjustments, starting with SP_OFF which
 is not a whole multiple of 16.

Also, as Magnus noted, "ALPHA Calling Standard" [3] required the same:
 D.3.1 Stack Alignment
  This standard requires that stacks be octaword aligned at the time a
  new procedure is invoked.

However:
- the "normal" kernel stack is always misaligned by 8 bytes, thanks to
  the odd number of 64-bit words in 'struct pt_regs', which is the very
  first thing pushed onto the kernel thread stack;
- syscall, fault, interrupt etc. handlers may, or may not, receive aligned
  stack depending on numerous factors.

Somehow we got away with it until recently, when we ended up with
a stack corruption in kernel/smp.c:smp_call_function_single() due to
its use of 32-byte aligned local data and the compiler doing clever
things allocating it on the stack.

Patches 1-2 are preparatory; 3 - the main fix; 4 - fixes remaining
special cases.

Ivan.

[1] https://lore.kernel.org/rcu/CA+=Fv5R9NG+1SHU9QV9hjmavycHKpnNyerQ=Ei90G98ukRcRJA@mail.gmail.com/#r
[2] https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.me.uk/
[3] https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427.pdf
---
Changes in v2:
- patch #1: provide empty 'struct pt_regs' to fix compile failure in libbpf,
  reported by John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>;
  update comment and commit message accordingly;
- cc'ed <stable@vger.kernel.org> as older kernels ought to be fixed as well.
---
Ivan Kokshaysky (4):
  alpha/uapi: do not expose kernel-only stack frame structures
  alpha: replace hardcoded stack offsets with autogenerated ones
  alpha: make stack 16-byte aligned (most cases)
  alpha: align stack for page fault and user unaligned trap handlers

 arch/alpha/include/asm/ptrace.h      | 64 ++++++++++++++++++++++++++-
 arch/alpha/include/uapi/asm/ptrace.h | 65 ++--------------------------
 arch/alpha/kernel/asm-offsets.c      |  4 ++
 arch/alpha/kernel/entry.S            | 24 +++++-----
 arch/alpha/kernel/traps.c            |  2 +-
 arch/alpha/mm/fault.c                |  4 +-
 6 files changed, 83 insertions(+), 80 deletions(-)

-- 
2.39.5

[PATCH v2 1/4] alpha/uapi: do not expose kernel-only stack frame structures

[Ivan Kokshaysky]

Parts of asm/ptrace.h went into UAPI with commit 96433f6ee490
("UAPI: (Scripted) Disintegrate arch/alpha/include/asm") back in 2012.
At first glance it looked correct, as many other architectures expose
'struct pt_regs' for ptrace(2) PTRACE_GETREGS/PTRACE_SETREGS requests
and bpf(2) BPF_PROG_TYPE_KPROBE/BPF_PROG_TYPE_PERF_EVENT program
types.

On Alpha, however, these requests have never been implemented;
'struct pt_regs' describes internal kernel stack frame which has
nothing to do with userspace. Same applies to 'struct switch_stack',
as PTRACE_GETFPREG/PTRACE_SETFPREG are not implemented either.

Move this stuff back into internal asm, where we can ajust it
without causing a lot of confusion about possible UAPI breakage.

Cc: stable@vger.kernel.org
Fixes: 96433f6ee490 ("UAPI: (Scripted) Disintegrate arch/alpha/include/asm")
Signed-off-by: Ivan Kokshaysky <ink@unseen.parts>
---
 arch/alpha/include/asm/ptrace.h      | 62 +++++++++++++++++++++++++-
 arch/alpha/include/uapi/asm/ptrace.h | 65 ++--------------------------
 2 files changed, 64 insertions(+), 63 deletions(-)

diff --git a/arch/alpha/include/asm/ptrace.h b/arch/alpha/include/asm/ptrace.h
index 3557ce64ed21..693d4c5b4dc7 100644
--- a/arch/alpha/include/asm/ptrace.h
+++ b/arch/alpha/include/asm/ptrace.h
@@ -2,8 +2,68 @@
 #ifndef _ASMAXP_PTRACE_H
 #define _ASMAXP_PTRACE_H
 
-#include <uapi/asm/ptrace.h>
+/*
+ * This struct defines the way the registers are stored on the
+ * kernel stack during a system call or other kernel entry
+ *
+ * NOTE! I want to minimize the overhead of system calls, so this
+ * struct has as little information as possible. It does not have
+ *
+ *  - floating point regs: the kernel doesn't change those
+ *  - r9-15: saved by the C compiler
+ *
+ * This makes "fork()" and "exec()" a bit more complex, but should
+ * give us low system call latency.
+ */
 
+struct pt_regs {
+	unsigned long r0;
+	unsigned long r1;
+	unsigned long r2;
+	unsigned long r3;
+	unsigned long r4;
+	unsigned long r5;
+	unsigned long r6;
+	unsigned long r7;
+	unsigned long r8;
+	unsigned long r19;
+	unsigned long r20;
+	unsigned long r21;
+	unsigned long r22;
+	unsigned long r23;
+	unsigned long r24;
+	unsigned long r25;
+	unsigned long r26;
+	unsigned long r27;
+	unsigned long r28;
+	unsigned long hae;
+/* JRP - These are the values provided to a0-a2 by PALcode */
+	unsigned long trap_a0;
+	unsigned long trap_a1;
+	unsigned long trap_a2;
+/* These are saved by PAL-code: */
+	unsigned long ps;
+	unsigned long pc;
+	unsigned long gp;
+	unsigned long r16;
+	unsigned long r17;
+	unsigned long r18;
+};
+
+/*
+ * This is the extended stack used by signal handlers and the context
+ * switcher: it's pushed after the normal "struct pt_regs".
+ */
+struct switch_stack {
+	unsigned long r9;
+	unsigned long r10;
+	unsigned long r11;
+	unsigned long r12;
+	unsigned long r13;
+	unsigned long r14;
+	unsigned long r15;
+	unsigned long r26;
+};
 
 #define arch_has_single_step()		(1)
 #define user_mode(regs) (((regs)->ps & 8) != 0)
diff --git a/arch/alpha/include/uapi/asm/ptrace.h b/arch/alpha/include/uapi/asm/ptrace.h
index 5ca45934fcbb..ad55dc283d8d 100644
--- a/arch/alpha/include/uapi/asm/ptrace.h
+++ b/arch/alpha/include/uapi/asm/ptrace.h
@@ -2,72 +2,13 @@
 #ifndef _UAPI_ASMAXP_PTRACE_H
 #define _UAPI_ASMAXP_PTRACE_H
 
-
 /*
- * This struct defines the way the registers are stored on the
- * kernel stack during a system call or other kernel entry
- *
- * NOTE! I want to minimize the overhead of system calls, so this
- * struct has as little information as possible. It does not have
- *
- *  - floating point regs: the kernel doesn't change those
- *  - r9-15: saved by the C compiler
+ * We don't HAVE_REGS_AND_STACK_ACCESS_API.
  *
- * This makes "fork()" and "exec()" a bit more complex, but should
- * give us low system call latency.
+ * Provide empty pt_regs structure for libbpf and the likes
+ * to avoid breaking the compilation.
  */
-
 struct pt_regs {
-	unsigned long r0;
-	unsigned long r1;
-	unsigned long r2;
-	unsigned long r3;
-	unsigned long r4;
-	unsigned long r5;
-	unsigned long r6;
-	unsigned long r7;
-	unsigned long r8;
-	unsigned long r19;
-	unsigned long r20;
-	unsigned long r21;
-	unsigned long r22;
-	unsigned long r23;
-	unsigned long r24;
-	unsigned long r25;
-	unsigned long r26;
-	unsigned long r27;
-	unsigned long r28;
-	unsigned long hae;
-/* JRP - These are the values provided to a0-a2 by PALcode */
-	unsigned long trap_a0;
-	unsigned long trap_a1;
-	unsigned long trap_a2;
-/* These are saved by PAL-code: */
-	unsigned long ps;
-	unsigned long pc;
-	unsigned long gp;
-	unsigned long r16;
-	unsigned long r17;
-	unsigned long r18;
 };
 
-/*
- * This is the extended stack used by signal handlers and the context
- * switcher: it's pushed after the normal "struct pt_regs".
- */
-struct switch_stack {
-	unsigned long r9;
-	unsigned long r10;
-	unsigned long r11;
-	unsigned long r12;
-	unsigned long r13;
-	unsigned long r14;
-	unsigned long r15;
-	unsigned long r26;
-#ifndef __KERNEL__
-	unsigned long fp[32];	/* fp[31] is fpcr */
-#endif
-};
-
-
 #endif /* _UAPI_ASMAXP_PTRACE_H */
-- 
2.39.5

[PATCH v2 2/4] alpha: replace hardcoded stack offsets with autogenerated ones

[Ivan Kokshaysky]

Cc: stable@vger.kernel.org
Signed-off-by: Ivan Kokshaysky <ink@unseen.parts>
---
 arch/alpha/kernel/asm-offsets.c | 4 ++++
 arch/alpha/kernel/entry.S       | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/kernel/asm-offsets.c b/arch/alpha/kernel/asm-offsets.c
index 4cfeae42c79a..e9dad60b147f 100644
--- a/arch/alpha/kernel/asm-offsets.c
+++ b/arch/alpha/kernel/asm-offsets.c
@@ -19,9 +19,13 @@ static void __used foo(void)
 	DEFINE(TI_STATUS, offsetof(struct thread_info, status));
 	BLANK();
 
+	DEFINE(SP_OFF, offsetof(struct pt_regs, ps));
 	DEFINE(SIZEOF_PT_REGS, sizeof(struct pt_regs));
 	BLANK();
 
+	DEFINE(SWITCH_STACK_SIZE, sizeof(struct switch_stack));
+	BLANK();
+
 	DEFINE(HAE_CACHE, offsetof(struct alpha_machine_vector, hae_cache));
 	DEFINE(HAE_REG, offsetof(struct alpha_machine_vector, hae_register));
 }
diff --git a/arch/alpha/kernel/entry.S b/arch/alpha/kernel/entry.S
index dd26062d75b3..6fb38365539d 100644
--- a/arch/alpha/kernel/entry.S
+++ b/arch/alpha/kernel/entry.S
@@ -15,10 +15,6 @@
 	.set noat
 	.cfi_sections	.debug_frame
 
-/* Stack offsets.  */
-#define SP_OFF			184
-#define SWITCH_STACK_SIZE	64
-
 .macro	CFI_START_OSF_FRAME	func
 	.align	4
 	.globl	\func
-- 
2.39.5

[PATCH v2 3/4] alpha: make stack 16-byte aligned (most cases)

[Ivan Kokshaysky]

Add padding between the PAL-saved and kernel-saved registers
so that 'struct pt_regs' have an even number of 64-bit words.
This makes the stack properly aligned for most of the kernel
code, except two handlers which need special threatment.

Cc: stable@vger.kernel.org
Tested-by: Magnus Lindholm <linmag7@gmail.com>
Signed-off-by: Ivan Kokshaysky <ink@unseen.parts>
---
 arch/alpha/include/asm/ptrace.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/alpha/include/asm/ptrace.h b/arch/alpha/include/asm/ptrace.h
index 693d4c5b4dc7..694b82ca62f3 100644
--- a/arch/alpha/include/asm/ptrace.h
+++ b/arch/alpha/include/asm/ptrace.h
@@ -41,6 +41,8 @@ struct pt_regs {
 	unsigned long trap_a0;
 	unsigned long trap_a1;
 	unsigned long trap_a2;
+/* This makes the stack 16-byte aligned as GCC expects */
+	unsigned long __pad0;
 /* These are saved by PAL-code: */
 	unsigned long ps;
 	unsigned long pc;
-- 
2.39.5

[PATCH v2 4/4] alpha: align stack for page fault and user unaligned trap handlers

[Ivan Kokshaysky]

do_page_fault() and do_entUna() are special because they use
non-standard stack frame layout. Fix them manually.

Cc: stable@vger.kernel.org
Tested-by: Magnus Lindholm <linmag7@gmail.com>
Suggested-by: "Maciej W. Rozycki" <macro@orcam.me.uk>
Signed-off-by: Ivan Kokshaysky <ink@unseen.parts>
---
 arch/alpha/kernel/entry.S | 20 ++++++++++----------
 arch/alpha/kernel/traps.c |  2 +-
 arch/alpha/mm/fault.c     |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/alpha/kernel/entry.S b/arch/alpha/kernel/entry.S
index 6fb38365539d..f4d41b4538c2 100644
--- a/arch/alpha/kernel/entry.S
+++ b/arch/alpha/kernel/entry.S
@@ -194,8 +194,8 @@ CFI_END_OSF_FRAME entArith
 CFI_START_OSF_FRAME entMM
 	SAVE_ALL
 /* save $9 - $15 so the inline exception code can manipulate them.  */
-	subq	$sp, 56, $sp
-	.cfi_adjust_cfa_offset	56
+	subq	$sp, 64, $sp
+	.cfi_adjust_cfa_offset	64
 	stq	$9, 0($sp)
 	stq	$10, 8($sp)
 	stq	$11, 16($sp)
@@ -210,7 +210,7 @@ CFI_START_OSF_FRAME entMM
 	.cfi_rel_offset	$13, 32
 	.cfi_rel_offset	$14, 40
 	.cfi_rel_offset	$15, 48
-	addq	$sp, 56, $19
+	addq	$sp, 64, $19
 /* handle the fault */
 	lda	$8, 0x3fff
 	bic	$sp, $8, $8
@@ -223,7 +223,7 @@ CFI_START_OSF_FRAME entMM
 	ldq	$13, 32($sp)
 	ldq	$14, 40($sp)
 	ldq	$15, 48($sp)
-	addq	$sp, 56, $sp
+	addq	$sp, 64, $sp
 	.cfi_restore	$9
 	.cfi_restore	$10
 	.cfi_restore	$11
@@ -231,7 +231,7 @@ CFI_START_OSF_FRAME entMM
 	.cfi_restore	$13
 	.cfi_restore	$14
 	.cfi_restore	$15
-	.cfi_adjust_cfa_offset	-56
+	.cfi_adjust_cfa_offset	-64
 /* finish up the syscall as normal.  */
 	br	ret_from_sys_call
 CFI_END_OSF_FRAME entMM
@@ -378,8 +378,8 @@ entUnaUser:
 	.cfi_restore	$0
 	.cfi_adjust_cfa_offset	-256
 	SAVE_ALL		/* setup normal kernel stack */
-	lda	$sp, -56($sp)
-	.cfi_adjust_cfa_offset	56
+	lda	$sp, -64($sp)
+	.cfi_adjust_cfa_offset	64
 	stq	$9, 0($sp)
 	stq	$10, 8($sp)
 	stq	$11, 16($sp)
@@ -395,7 +395,7 @@ entUnaUser:
 	.cfi_rel_offset	$14, 40
 	.cfi_rel_offset	$15, 48
 	lda	$8, 0x3fff
-	addq	$sp, 56, $19
+	addq	$sp, 64, $19
 	bic	$sp, $8, $8
 	jsr	$26, do_entUnaUser
 	ldq	$9, 0($sp)
@@ -405,7 +405,7 @@ entUnaUser:
 	ldq	$13, 32($sp)
 	ldq	$14, 40($sp)
 	ldq	$15, 48($sp)
-	lda	$sp, 56($sp)
+	lda	$sp, 64($sp)
 	.cfi_restore	$9
 	.cfi_restore	$10
 	.cfi_restore	$11
@@ -413,7 +413,7 @@ entUnaUser:
 	.cfi_restore	$13
 	.cfi_restore	$14
 	.cfi_restore	$15
-	.cfi_adjust_cfa_offset	-56
+	.cfi_adjust_cfa_offset	-64
 	br	ret_from_sys_call
 CFI_END_OSF_FRAME entUna
 
diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
index a9a38c80c4a7..7004397937cf 100644
--- a/arch/alpha/kernel/traps.c
+++ b/arch/alpha/kernel/traps.c
@@ -649,7 +649,7 @@ s_reg_to_mem (unsigned long s_reg)
 static int unauser_reg_offsets[32] = {
 	R(r0), R(r1), R(r2), R(r3), R(r4), R(r5), R(r6), R(r7), R(r8),
 	/* r9 ... r15 are stored in front of regs.  */
-	-56, -48, -40, -32, -24, -16, -8,
+	-64, -56, -48, -40, -32, -24, -16,	/* padding at -8 */
 	R(r16), R(r17), R(r18),
 	R(r19), R(r20), R(r21), R(r22), R(r23), R(r24), R(r25), R(r26),
 	R(r27), R(r28), R(gp),
diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c
index 8c9850437e67..a9816bbc9f34 100644
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -78,8 +78,8 @@ __load_new_mm_context(struct mm_struct *next_mm)
 
 /* Macro for exception fixup code to access integer registers.  */
 #define dpf_reg(r)							\
-	(((unsigned long *)regs)[(r) <= 8 ? (r) : (r) <= 15 ? (r)-16 :	\
-				 (r) <= 18 ? (r)+10 : (r)-10])
+	(((unsigned long *)regs)[(r) <= 8 ? (r) : (r) <= 15 ? (r)-17 :	\
+				 (r) <= 18 ? (r)+11 : (r)-10])
 
 asmlinkage void
 do_page_fault(unsigned long address, unsigned long mmcsr,
-- 
2.39.5

Re: [PATCH v2 0/4] alpha: stack fixes

[John Paul Adrian Glaubitz]

Hi Ivan,

On Fri, 2025-01-31 at 11:41 +0100, Ivan Kokshaysky wrote:
> This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1]
> Thanks to Magnus Lindholm for identifying that remarkably longstanding
> bug.
> 
> The problem is that GCC expects 16-byte alignment of the incoming stack
> since early 2004, as Maciej found out [2]:
>   Having actually dug speculatively I can see that the psABI was changed in
>  GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double
>  improperly aligned)") back in Mar 2004, when the stack pointer alignment
>  was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has
>  various suspicious stack pointer adjustments, starting with SP_OFF which
>  is not a whole multiple of 16.
> 
> Also, as Magnus noted, "ALPHA Calling Standard" [3] required the same:
>  D.3.1 Stack Alignment
>   This standard requires that stacks be octaword aligned at the time a
>   new procedure is invoked.
> 
> However:
> - the "normal" kernel stack is always misaligned by 8 bytes, thanks to
>   the odd number of 64-bit words in 'struct pt_regs', which is the very
>   first thing pushed onto the kernel thread stack;
> - syscall, fault, interrupt etc. handlers may, or may not, receive aligned
>   stack depending on numerous factors.
> 
> Somehow we got away with it until recently, when we ended up with
> a stack corruption in kernel/smp.c:smp_call_function_single() due to
> its use of 32-byte aligned local data and the compiler doing clever
> things allocating it on the stack.
> 
> Patches 1-2 are preparatory; 3 - the main fix; 4 - fixes remaining
> special cases.
> 
> Ivan.
> 
> [1] https://lore.kernel.org/rcu/CA+=Fv5R9NG+1SHU9QV9hjmavycHKpnNyerQ=Ei90G98ukRcRJA@mail.gmail.com/#r
> [2] https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.me.uk/
> [3] https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427.pdf
> ---
> Changes in v2:
> - patch #1: provide empty 'struct pt_regs' to fix compile failure in libbpf,
>   reported by John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>;
>   update comment and commit message accordingly;
> - cc'ed <stable@vger.kernel.org> as older kernels ought to be fixed as well.
> ---
> Ivan Kokshaysky (4):
>   alpha/uapi: do not expose kernel-only stack frame structures
>   alpha: replace hardcoded stack offsets with autogenerated ones
>   alpha: make stack 16-byte aligned (most cases)
>   alpha: align stack for page fault and user unaligned trap handlers
> 
>  arch/alpha/include/asm/ptrace.h      | 64 ++++++++++++++++++++++++++-
>  arch/alpha/include/uapi/asm/ptrace.h | 65 ++--------------------------
>  arch/alpha/kernel/asm-offsets.c      |  4 ++
>  arch/alpha/kernel/entry.S            | 24 +++++-----
>  arch/alpha/kernel/traps.c            |  2 +-
>  arch/alpha/mm/fault.c                |  4 +-
>  6 files changed, 83 insertions(+), 80 deletions(-)

Thanks, I'm testing the v2 series of the patches now.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH v2 0/4] alpha: stack fixes

[Michael Cree]

On Sat, Feb 01, 2025 at 10:46:43AM +0100, John Paul Adrian Glaubitz wrote:
> Hi Ivan,
> 
> On Fri, 2025-01-31 at 11:41 +0100, Ivan Kokshaysky wrote:
> > This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1]
> > Thanks to Magnus Lindholm for identifying that remarkably longstanding
> > bug.
> > 
> 
> Thanks, I'm testing the v2 series of the patches now.
> 
> Adrian

I've been running the patches on the 6.12.11 kernel for over 24 hours
now.  Going very well and, in particular, I would like to note that:

The thread-test in the pixman package which has been failing for over
year 10 years on real Alpha hardware now passes!

I have now successfully built guile-3.0 with threading support!
Previously guile would lock up on Alpha if threading support was
enabled.

So there are some very long-standing bugs seen in user space that are
fixed by this patch series.

Cheers,
Michael.

Re: [PATCH v2 0/4] alpha: stack fixes

[Magnus Lindholm]

Hi,

I've applied the patches to git 6.13.0-09954-g590a41bebc8c and the
system has been running for more than 24 hours without any problems,
I've generated some system load with building kernels and unpacking
large tar.xz files. The patch series seems to have fixed the
rcu-related issues with network interface renaming as well as the
kernel module unload. I'm now also running tests with memory
compaction enabled (CONFIG_COMPACTION). This used to cause seemingly
random segmentation faults when enabled on alpha. So far, memory
compaction seems to work with the patched kernel. With a little luck
the issues seen with memory compaction on alpha were related to stack
alignment problems as well.

In any case, very impressive work with putting together these patches,
this bodes well for the future for linux on alpha!

Regards

Magnus Lindholm

On Sun, Feb 2, 2025 at 12:13 AM Michael Cree <mcree@orcon.net.nz> wrote:
>
> On Sat, Feb 01, 2025 at 10:46:43AM +0100, John Paul Adrian Glaubitz wrote:
> > Hi Ivan,
> >
> > On Fri, 2025-01-31 at 11:41 +0100, Ivan Kokshaysky wrote:
> > > This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1]
> > > Thanks to Magnus Lindholm for identifying that remarkably longstanding
> > > bug.
> > >
> >
> > Thanks, I'm testing the v2 series of the patches now.
> >
> > Adrian
>
> I've been running the patches on the 6.12.11 kernel for over 24 hours
> now.  Going very well and, in particular, I would like to note that:
>
> The thread-test in the pixman package which has been failing for over
> year 10 years on real Alpha hardware now passes!
>
> I have now successfully built guile-3.0 with threading support!
> Previously guile would lock up on Alpha if threading support was
> enabled.
>
> So there are some very long-standing bugs seen in user space that are
> fixed by this patch series.
>
> Cheers,
> Michael.

Re: [PATCH v2 0/4] alpha: stack fixes

[Maciej W. Rozycki]

On Fri, 31 Jan 2025, Ivan Kokshaysky wrote:

> This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1]
> Thanks to Magnus Lindholm for identifying that remarkably longstanding
> bug.
> 
> The problem is that GCC expects 16-byte alignment of the incoming stack
> since early 2004, as Maciej found out [2]:
>   Having actually dug speculatively I can see that the psABI was changed in
>  GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double
>  improperly aligned)") back in Mar 2004, when the stack pointer alignment
>  was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has
>  various suspicious stack pointer adjustments, starting with SP_OFF which
>  is not a whole multiple of 16.
> 
> Also, as Magnus noted, "ALPHA Calling Standard" [3] required the same:
>  D.3.1 Stack Alignment
>   This standard requires that stacks be octaword aligned at the time a
>   new procedure is invoked.
> 
> However:
> - the "normal" kernel stack is always misaligned by 8 bytes, thanks to
>   the odd number of 64-bit words in 'struct pt_regs', which is the very
>   first thing pushed onto the kernel thread stack;
> - syscall, fault, interrupt etc. handlers may, or may not, receive aligned
>   stack depending on numerous factors.

 Would you please put this analysis into the commit description of 3/4?  
It gives a good justification for the change, so it seems appropriate to 
me to get it recorded along with the commit for posterity.

 NB I've been feeling a little bit unwell over the last couple of days and 
consequently I only started my GCC/glibc verification yesterday.  Current 
ETC is this coming Tue.  Perheps it's worth noting that I run this against 
6.3.0-rc5 with a couple of backports on top to resolve conflicts, as the 
current master does not support EV45 hardware anymore.  I'll let you know 
of the outcome.

  Maciej

Re: [PATCH v2 1/4] alpha/uapi: do not expose kernel-only stack frame structures

[Maciej W. Rozycki]

On Fri, 31 Jan 2025, Ivan Kokshaysky wrote:

> Parts of asm/ptrace.h went into UAPI with commit 96433f6ee490
> ("UAPI: (Scripted) Disintegrate arch/alpha/include/asm") back in 2012.
> At first glance it looked correct, as many other architectures expose
> 'struct pt_regs' for ptrace(2) PTRACE_GETREGS/PTRACE_SETREGS requests
> and bpf(2) BPF_PROG_TYPE_KPROBE/BPF_PROG_TYPE_PERF_EVENT program
> types.
> 
> On Alpha, however, these requests have never been implemented;
> 'struct pt_regs' describes internal kernel stack frame which has
> nothing to do with userspace. Same applies to 'struct switch_stack',
> as PTRACE_GETFPREG/PTRACE_SETFPREG are not implemented either.

 I note that we, unusually, neither save nor even have room for statics in 
`struct pt_regs', so this structure by itself is unsuitable to pass the 
register file around with tracing calls and the like.  So it seems to me 
there's no point in exporting `struct pt_regs' in any way to the userland.

 What do you think about providing arch/alpha/include/asm/bpf_perf_event.h 
instead with either a dummy definition of `bpf_user_pt_regs_t', or perhaps 
one typedef'd to `struct sigcontext' (as it seems to provide all that's 
needed), and then reverting to v1 of arch/alpha/include/uapi/asm/ptrace.h 
(and then just copying the contents of arch/alpha/include/asm/ftrace.h 
over rather than leaving all the useless CPP stuff in) so that we don't 
have useless `struct pt_regs' exported at all?

> Move this stuff back into internal asm, where we can ajust it

s/ajust/adjust/ (NB scripts/checkpatch.pl does complain about it).

  Maciej

Re: [PATCH v2 2/4] alpha: replace hardcoded stack offsets with autogenerated ones

[Maciej W. Rozycki]

On Fri, 31 Jan 2025, Ivan Kokshaysky wrote:

> Cc: stable@vger.kernel.org
> Signed-off-by: Ivan Kokshaysky <ink@unseen.parts>
> ---

 Can you please add a short commit description (which is our requirement), 
even if to mention that this is to avoid having to maintain the assembly 
source by hand?

 LGTM otherwise.

Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk>

  Maciej

Re: [PATCH v2 3/4] alpha: make stack 16-byte aligned (most cases)

[Maciej W. Rozycki]

On Fri, 31 Jan 2025, Ivan Kokshaysky wrote:

> Add padding between the PAL-saved and kernel-saved registers
> so that 'struct pt_regs' have an even number of 64-bit words.
> This makes the stack properly aligned for most of the kernel
> code, except two handlers which need special threatment.

 LGTM except for the request from 0/4 to improve the change description.

Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk>

  Maciej

Re: [PATCH v2 4/4] alpha: align stack for page fault and user unaligned trap handlers

[Maciej W. Rozycki]

On Fri, 31 Jan 2025, Ivan Kokshaysky wrote:

> do_page_fault() and do_entUna() are special because they use
> non-standard stack frame layout. Fix them manually.

 We could reuse `struct switch_stack' here and clean up this stuff a 
little, but I guess it can be done later, when we've run out of more 
serious issues.  LGTM then.

Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk>

  Maciej

Re: [PATCH v2 0/4] alpha: stack fixes

[Michael Cree]

On Sun, Feb 02, 2025 at 10:43:59AM +0100, Magnus Lindholm wrote:
> I've applied the patches to git 6.13.0-09954-g590a41bebc8c and the
> system has been running for more than 24 hours without any problems,
> I've generated some system load with building kernels and unpacking
> large tar.xz files. The patch series seems to have fixed the
> rcu-related issues with network interface renaming as well as the
> kernel module unload. I'm now also running tests with memory
> compaction enabled (CONFIG_COMPACTION). This used to cause seemingly
> random segmentation faults when enabled on alpha. So far, memory
> compaction seems to work with the patched kernel. With a little luck
> the issues seen with memory compaction on alpha were related to stack
> alignment problems as well.

After 24 hours of really good going with the patches and
CONFIG_COMPACTION turned off, I rebooted with CONFIG_COMPACTION on
and within a couple of hours saw the random segmentation faults
reappear.  I have now rebooted with the kernel with
CONFIG_COMPACTION off and its been plain sailing for the last 12
hours.

So I suspect CONFIG_COMPACTION problem is something else.

Cheers,
Michael.

Re: [PATCH v2 1/4] alpha/uapi: do not expose kernel-only stack frame structures

[John Paul Adrian Glaubitz]

Hi Ivan,

On Fri, 2025-01-31 at 11:41 +0100, Ivan Kokshaysky wrote:
> Parts of asm/ptrace.h went into UAPI with commit 96433f6ee490
> ("UAPI: (Scripted) Disintegrate arch/alpha/include/asm") back in 2012.
> At first glance it looked correct, as many other architectures expose
> 'struct pt_regs' for ptrace(2) PTRACE_GETREGS/PTRACE_SETREGS requests
> and bpf(2) BPF_PROG_TYPE_KPROBE/BPF_PROG_TYPE_PERF_EVENT program
> types.
> 
> On Alpha, however, these requests have never been implemented;
> 'struct pt_regs' describes internal kernel stack frame which has
> nothing to do with userspace. Same applies to 'struct switch_stack',
> as PTRACE_GETFPREG/PTRACE_SETFPREG are not implemented either.
> 
> Move this stuff back into internal asm, where we can ajust it

Typo: ajust => adjust


Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH v2 0/4] alpha: stack fixes

[John Paul Adrian Glaubitz]

Hi Ivan,

On Sat, 2025-02-01 at 10:46 +0100, John Paul Adrian Glaubitz wrote:
> On Fri, 2025-01-31 at 11:41 +0100, Ivan Kokshaysky wrote:
> > This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1]
> > Thanks to Magnus Lindholm for identifying that remarkably longstanding
> > bug.
> > 
> > The problem is that GCC expects 16-byte alignment of the incoming stack
> > since early 2004, as Maciej found out [2]:
> >   Having actually dug speculatively I can see that the psABI was changed in
> >  GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double
> >  improperly aligned)") back in Mar 2004, when the stack pointer alignment
> >  was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has
> >  various suspicious stack pointer adjustments, starting with SP_OFF which
> >  is not a whole multiple of 16.
> > 
> > Also, as Magnus noted, "ALPHA Calling Standard" [3] required the same:
> >  D.3.1 Stack Alignment
> >   This standard requires that stacks be octaword aligned at the time a
> >   new procedure is invoked.
> > 
> > However:
> > - the "normal" kernel stack is always misaligned by 8 bytes, thanks to
> >   the odd number of 64-bit words in 'struct pt_regs', which is the very
> >   first thing pushed onto the kernel thread stack;
> > - syscall, fault, interrupt etc. handlers may, or may not, receive aligned
> >   stack depending on numerous factors.
> > 
> > Somehow we got away with it until recently, when we ended up with
> > a stack corruption in kernel/smp.c:smp_call_function_single() due to
> > its use of 32-byte aligned local data and the compiler doing clever
> > things allocating it on the stack.
> > 
> > Patches 1-2 are preparatory; 3 - the main fix; 4 - fixes remaining
> > special cases.
> > 
> > Ivan.
> > 
> > [1] https://lore.kernel.org/rcu/CA+=Fv5R9NG+1SHU9QV9hjmavycHKpnNyerQ=Ei90G98ukRcRJA@mail.gmail.com/#r
> > [2] https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.me.uk/
> > [3] https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427.pdf
> > ---
> > Changes in v2:
> > - patch #1: provide empty 'struct pt_regs' to fix compile failure in libbpf,
> >   reported by John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>;
> >   update comment and commit message accordingly;
> > - cc'ed <stable@vger.kernel.org> as older kernels ought to be fixed as well.
> > ---
> > Ivan Kokshaysky (4):
> >   alpha/uapi: do not expose kernel-only stack frame structures
> >   alpha: replace hardcoded stack offsets with autogenerated ones
> >   alpha: make stack 16-byte aligned (most cases)
> >   alpha: align stack for page fault and user unaligned trap handlers
> > 
> >  arch/alpha/include/asm/ptrace.h      | 64 ++++++++++++++++++++++++++-
> >  arch/alpha/include/uapi/asm/ptrace.h | 65 ++--------------------------
> >  arch/alpha/kernel/asm-offsets.c      |  4 ++
> >  arch/alpha/kernel/entry.S            | 24 +++++-----
> >  arch/alpha/kernel/traps.c            |  2 +-
> >  arch/alpha/mm/fault.c                |  4 +-
> >  6 files changed, 83 insertions(+), 80 deletions(-)
> 
> Thanks, I'm testing the v2 series of the patches now.

I have applied the series, but I am seeing gcc crashes from time to time:

/build/reproducible-path/palapeli-24.12.1/obj-alpha-linux-gnu/mime/palathumbcreator_autogen/include/thumbnail-creator.moc: In function ‘QObject* qt_plugin_instance()’:
/build/reproducible-path/palapeli-24.12.1/obj-alpha-linux-gnu/mime/palathumbcreator_autogen/include/thumbnail-creator.moc:328:1: error: unrecognizable insn:
  328 | QT_MOC_EXPORT_PLUGIN_V2(palathumbcreator_factory, palathumbcreator_factory, qt_pluginMetaDataV2_palathumbcreator_factory)
      | ^~~~~~~~~~~~~~~~~~~~~~~
(jump_insn 331 295 332 3 (set (pc)
        (address:DI 1)) -1
     (nil)
 -> 40)
during RTL pass: sched1
/build/reproducible-path/palapeli-24.12.1/obj-alpha-linux-gnu/mime/palathumbcreator_autogen/include/thumbnail-creator.moc:328:1: internal compiler error: in extract_insn, at recog.cc:2812
0x12195fc8b internal_error(char const*, ...)
	???:0
0x1201f37b7 fancy_abort(char const*, int, char const*)
	???:0
0x1201f0a6f _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
	???:0
0x1201f0ab7 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
	???:0
0x120b5ff97 extract_insn(rtx_insn*)
	???:0
0x12179d003 deps_analyze_insn(deps_desc*, rtx_insn*)
	???:0
0x12179d98f sched_analyze(deps_desc*, rtx_insn*, rtx_insn*)
	???:0
0x120bb0517 sched_rgn_compute_dependencies(int)
	???:0
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <file:///usr/share/doc/gcc-14/README.Bugs> for instructions.
The bug is not reproducible, so it is likely a hardware or OS problem.

See: https://buildd.debian.org/status/fetch.php?pkg=palapeli&arch=alpha&ver=4%3A24.12.1-1&stamp=1738215920&raw=0

But this might be related to CONFIG_COMPACTION as Michael Cree already mentioned
as this option is enabled in Debian by default on all architectures except for
m68k.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH v2 1/4] alpha/uapi: do not expose kernel-only stack frame structures

[Ivan Kokshaysky]

On Sun, Feb 02, 2025 at 05:39:52PM +0000, Maciej W. Rozycki wrote:
>  What do you think about providing arch/alpha/include/asm/bpf_perf_event.h 
> instead with either a dummy definition of `bpf_user_pt_regs_t', or perhaps 
> one typedef'd to `struct sigcontext' (as it seems to provide all that's 
> needed), and then reverting to v1 of arch/alpha/include/uapi/asm/ptrace.h 
> (and then just copying the contents of arch/alpha/include/asm/ftrace.h 
> over rather than leaving all the useless CPP stuff in) so that we don't 
> have useless `struct pt_regs' exported at all?

Probably that's the right thing to do. However, it implies adding

#elif defined(__alpha__)
#include "../../arch/alpha/include/uapi/asm/bpf_perf_event.h"

in tools/include/uapi/asm/bpf_perf_event.h. I'm afraid that will
result in too many loosely related changes for this patch series.

I'm starting to think that the best way for the time being is to keep
uapi/asm/ptrace.h and apply the fix there (i.e. revert to v0 patch
posted on linux-alpha). And mention the pt_regs vs uapi issue in the
commit message, of course, to deal with it later. Your opinion?

Ivan.

Re: [PATCH v2 1/4] alpha/uapi: do not expose kernel-only stack frame structures

[Maciej W. Rozycki]

On Mon, 3 Feb 2025, Ivan Kokshaysky wrote:

> >  What do you think about providing arch/alpha/include/asm/bpf_perf_event.h 
> > instead with either a dummy definition of `bpf_user_pt_regs_t', or perhaps 
> > one typedef'd to `struct sigcontext' (as it seems to provide all that's 
> > needed), and then reverting to v1 of arch/alpha/include/uapi/asm/ptrace.h 
> > (and then just copying the contents of arch/alpha/include/asm/ftrace.h 
> > over rather than leaving all the useless CPP stuff in) so that we don't 
> > have useless `struct pt_regs' exported at all?
> 
> Probably that's the right thing to do. However, it implies adding
> 
> #elif defined(__alpha__)
> #include "../../arch/alpha/include/uapi/asm/bpf_perf_event.h"
> 
> in tools/include/uapi/asm/bpf_perf_event.h. I'm afraid that will
> result in too many loosely related changes for this patch series.

 This seems to be the way, but...

> I'm starting to think that the best way for the time being is to keep
> uapi/asm/ptrace.h and apply the fix there (i.e. revert to v0 patch
> posted on linux-alpha). And mention the pt_regs vs uapi issue in the
> commit message, of course, to deal with it later. Your opinion?

 ... I agree.  I find this a pragmatic path of least resistance approach, 
which will keep backports to the minimum and prevent Greg from getting 
rightfully grumpy about it.  We need to get things straight here and BPF 
is the least of a problem.  Let's go for this minimal variant then.

 This will also buy us time to think what the structure format will be 
right for `bpf_user_pt_regs_t' and whether `struct sigcontext' is indeed 
the best fit.  Perhaps we'll want to define an entirely new structure and 
use it also for regset support, which I suppose would be good having, as 
it simplifies handling in debug software.  I'm not sure offhand though, 
which I suppose is a good sign to defer this stuff to a later change.

 NB GCC verification has completed successfully with no regressions (but 
no progressions either; there've been a couple of changes both ways with 
intermittent failures, mostly in Fortran and Go, but none related to this 
patch series), and glibc verification is still running; by the look of the 
progress current ETC is sadly Fri-Sat only.

 The previous glibc run completed in ~25h, but this time the testsuite 
includes recently added 576 extra formatted output `printf' tests, which I 
admit to have committed myself, which owing to their duration I proposed 
to be a part of the extended set, but the community insisted that they be 
a part of the regular set, to widen coverage.  This subset has already 
been running for ~30h and has made it through ~25%.  So there you go.

 For reference the 576 extra test cases complete in ~30m on POWER9 (in a 
serial run; a parallel run is obviously much faster on this 64-way SMP 
system, but infeasible with the UP Alpha over the network, so I need to 
compare apples to apples), which just shows how irrelevant the performance 
of these legacy systems nowadays is and it's just the matter of keeping 
them alive with current software that has been my objective all the time.

  Maciej