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 <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
v2: Move the struct instead of changing the #ifdef
See previous discussion here:
https://lore.kernel.org/lkml/[email protected]/
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
--
2.43.2
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 <[email protected]>
> Signed-off-by: Thomas Huth <[email protected]>
> ---
> v2: Move the struct instead of changing the #ifdef
>
> See previous discussion here:
> https://lore.kernel.org/lkml/[email protected]/
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 <[email protected]>
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
On Tue, Feb 20, 2024, at 15:13, Greg Ungerer wrote:
> 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 <[email protected]>
>> Signed-off-by: Thomas Huth <[email protected]>
>> ---
>> v2: Move the struct instead of changing the #ifdef
>>
>> See previous discussion here:
>> https://lore.kernel.org/lkml/[email protected]/
>
> 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 <[email protected]>
>
> Geert, Arnd, do you have any thoughts on this?
It clearly doesn't change the ABI, so that part is fine.
If asm/ptrace.h is included by some userspace tool to
get the definition, it might cause a compile-time error
that needs a trivial source change.
This could be needed for ptrace (gdb, strace) or signal
handling and setjmp (libc), though it's more likely that these
already have their own copies.
Arnd
On 20/02/2024 16.09, Arnd Bergmann wrote:
> On Tue, Feb 20, 2024, at 15:13, Greg Ungerer wrote:
>> 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 <[email protected]>
>>> Signed-off-by: Thomas Huth <[email protected]>
>>> ---
>>> v2: Move the struct instead of changing the #ifdef
>>>
>>> See previous discussion here:
>>> https://lore.kernel.org/lkml/[email protected]/
>>
>> 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 <[email protected]>
>>
>> Geert, Arnd, do you have any thoughts on this?
>
> It clearly doesn't change the ABI, so that part is fine.
>
> If asm/ptrace.h is included by some userspace tool to
> get the definition, it might cause a compile-time error
> that needs a trivial source change.
>
> This could be needed for ptrace (gdb, strace) or signal
> handling and setjmp (libc), though it's more likely that these
> already have their own copies.
If we still feel unsure, we should maybe rather go with v1:
https://lore.kernel.org/lkml/[email protected]/
?
Thomas
Hi Thomas,
On 23/2/24 18:13, Thomas Huth wrote:
> On 20/02/2024 16.09, Arnd Bergmann wrote:
>> On Tue, Feb 20, 2024, at 15:13, Greg Ungerer wrote:
>>> 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 <[email protected]>
>>>> Signed-off-by: Thomas Huth <[email protected]>
>>>> ---
>>>> v2: Move the struct instead of changing the #ifdef
>>>>
>>>> See previous discussion here:
>>>> https://lore.kernel.org/lkml/[email protected]/
>>>
>>> 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 <[email protected]>
>>>
>>> Geert, Arnd, do you have any thoughts on this?
>>
>> It clearly doesn't change the ABI, so that part is fine.
>>
>> If asm/ptrace.h is included by some userspace tool to
>> get the definition, it might cause a compile-time error
>> that needs a trivial source change.
>>
>> This could be needed for ptrace (gdb, strace) or signal
>> handling and setjmp (libc), though it's more likely that these
>> already have their own copies.
>
> If we still feel unsure, we should maybe rather go with v1:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> ?
We have not had much movement on this.
So... I am confidant that v2 is good, but lets err on the side of caution first up.
I have applied v1 to the m68knommu git tree, for-next branch.
Thanks
Greg