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

[John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>]

Hi Ivan,

On Wed, 2025-01-29 at 10:43 +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.
> 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.
> 
> 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 | 66 +---------------------------
>  2 files changed, 62 insertions(+), 66 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..2c08d74deac5 100644
> --- a/arch/alpha/include/uapi/asm/ptrace.h
> +++ b/arch/alpha/include/uapi/asm/ptrace.h
> @@ -2,72 +2,8 @@
>  #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
> - *
> - * 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".
> + * We have nothing to say to userspace.
>   */
> -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 */

This seems to break the build for the bpf tool on alpha:

In file included from libbpf.c:36:
/build/reproducible-path/linux-6.13~rc7/tools/include/uapi/linux/bpf_perf_event.h:14:28: error: field ‘regs’ has incomplete type
   14 |         bpf_user_pt_regs_t regs;
      |                            ^~~~

Adrian

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