Re: [PATCH v2] m68k: Avoid CONFIG_COLDFIRE switch in uapi header

Greg Ungerer
2024-02-20T14:13:33Z

On 20/2/24 02:01, Thomas Huth wrote:
> We should not use any CONFIG switches in uapi headers since these
> only work during kernel compilation; they are not defined for
> userspace. Fix it by moving the struct pt_regs to the kernel-internal
> header instead - struct pt_regs does not seem to be required for
> the userspace headers on m68k at all.
>
> Suggested-by: Greg Ungerer <gerg@linux-m68k.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> v2: Move the struct instead of changing the #ifdef
>
> See previous discussion here:
> https://lore.kernel.org/lkml/6e3f2a2e-2430-4b4f-9ead-d9a4d5e42713@linux-m68k.org/

I am fine with this. FWIW the following architectures do
not define pt_regs in their uapi/ptrace.h header either:
arc, arm64, loongarch, nios2, openrisc, riscv, s390, xtensa
Though quite a few of them have a user_pt_regs instead.

So for me:

Acked-by: Greg Ungerer <gerg@linux-m68k.org>

Geert, Arnd, do you have any thoughts on this?

Regards
Greg

> arch/m68k/include/asm/ptrace.h | 29 +++++++++++++++++++++++++++++
> arch/m68k/include/uapi/asm/ptrace.h | 28 ----------------------------
> scripts/headers_install.sh | 1 -
> 3 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/arch/m68k/include/asm/ptrace.h b/arch/m68k/include/asm/ptrace.h
> index ea5a80ca1ab33..f200712946603 100644
> --- a/arch/m68k/include/asm/ptrace.h
> +++ b/arch/m68k/include/asm/ptrace.h
> @@ -6,6 +6,35 @@
>
> #ifndef __ASSEMBLY__
>
> +/*
> + * This struct defines the way the registers are stored on the
> + * stack during a system call.
> + */
> +struct pt_regs {
> + long d1;
> + long d2;
> + long d3;
> + long d4;
> + long d5;
> + long a0;
> + long a1;
> + long a2;
> + long d0;
> + long orig_d0;
> + long stkadj;
> +#ifdef CONFIG_COLDFIRE
> + unsigned format : 4; /* frame format specifier */
> + unsigned vector : 12; /* vector offset */
> + unsigned short sr;
> + unsigned long pc;
> +#else
> + unsigned short sr;
> + unsigned long pc;
> + unsigned format : 4; /* frame format specifier */
> + unsigned vector : 12; /* vector offset */
> +#endif
> +};
> +
> #ifndef PS_S
> #define PS_S (0x2000)
> #define PS_M (0x1000)
> diff --git a/arch/m68k/include/uapi/asm/ptrace.h b/arch/m68k/include/uapi/asm/ptrace.h
> index 5b50ea592e002..a83bfe36dd10a 100644
> --- a/arch/m68k/include/uapi/asm/ptrace.h
> +++ b/arch/m68k/include/uapi/asm/ptrace.h
> @@ -24,34 +24,6 @@
>
> #ifndef __ASSEMBLY__
>
> -/* this struct defines the way the registers are stored on the
> - stack during a system call. */
> -
> -struct pt_regs {
> - long d1;
> - long d2;
> - long d3;
> - long d4;
> - long d5;
> - long a0;
> - long a1;
> - long a2;
> - long d0;
> - long orig_d0;
> - long stkadj;
> -#ifdef CONFIG_COLDFIRE
> - unsigned format : 4; /* frame format specifier */
> - unsigned vector : 12; /* vector offset */
> - unsigned short sr;
> - unsigned long pc;
> -#else
> - unsigned short sr;
> - unsigned long pc;
> - unsigned format : 4; /* frame format specifier */
> - unsigned vector : 12; /* vector offset */
> -#endif
> -};
> -
> /*
> * This is the extended stack used by signal handlers and the context
> * switcher: it's pushed after the normal "struct pt_regs".
> diff --git a/scripts/headers_install.sh b/scripts/headers_install.sh
> index f7d9b114de8f7..6bbccb43f7e72 100755
> --- a/scripts/headers_install.sh
> +++ b/scripts/headers_install.sh
> @@ -74,7 +74,6 @@ arch/arc/include/uapi/asm/page.h:CONFIG_ARC_PAGE_SIZE_16K
> arch/arc/include/uapi/asm/page.h:CONFIG_ARC_PAGE_SIZE_4K
> arch/arc/include/uapi/asm/swab.h:CONFIG_ARC_HAS_SWAPE
> arch/arm/include/uapi/asm/ptrace.h:CONFIG_CPU_ENDIAN_BE8
> -arch/m68k/include/uapi/asm/ptrace.h:CONFIG_COLDFIRE
> arch/nios2/include/uapi/asm/swab.h:CONFIG_NIOS2_CI_SWAB_NO
> arch/nios2/include/uapi/asm/swab.h:CONFIG_NIOS2_CI_SWAB_SUPPORT
> arch/x86/include/uapi/asm/auxvec.h:CONFIG_IA32_EMULATION