[PATCH 08/41] arm64: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

Thomas Huth posted 41 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH 08/41] arm64: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers
Posted by Thomas Huth 9 months, 1 week ago
__ASSEMBLY__ is only defined by the Makefile of the kernel, so
this is not really useful for uapi headers (unless the userspace
Makefile defines it, too). Let's switch to __ASSEMBLER__ which
gets set automatically by the compiler when compiling assembly
code.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 arch/arm64/include/uapi/asm/kvm.h        | 2 +-
 arch/arm64/include/uapi/asm/ptrace.h     | 4 ++--
 arch/arm64/include/uapi/asm/sigcontext.h | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 568bf858f3198..f1ee246f7b14e 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -31,7 +31,7 @@
 #define KVM_SPSR_FIQ	4
 #define KVM_NR_SPSR	5
 
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
 #include <linux/psci.h>
 #include <linux/types.h>
 #include <asm/ptrace.h>
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index 0f39ba4f3efd4..6fed93fb2536f 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -80,7 +80,7 @@
 #define PTRACE_PEEKMTETAGS	  33
 #define PTRACE_POKEMTETAGS	  34
 
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
 
 /*
  * User structures for general purpose, floating point and debug registers.
@@ -332,6 +332,6 @@ struct user_gcs {
 	__u64 gcspr_el0;
 };
 
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
 
 #endif /* _UAPI__ASM_PTRACE_H */
diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index d42f7a92238b9..e29bf3e2d0ccd 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -17,7 +17,7 @@
 #ifndef _UAPI__ASM_SIGCONTEXT_H
 #define _UAPI__ASM_SIGCONTEXT_H
 
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
 
 #include <linux/types.h>
 
@@ -192,7 +192,7 @@ struct gcs_context {
 	__u64 reserved;
 };
 
-#endif /* !__ASSEMBLY__ */
+#endif /* !__ASSEMBLER__ */
 
 #include <asm/sve_context.h>
 
-- 
2.48.1
Re: [PATCH 08/41] arm64: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers
Posted by Will Deacon 9 months, 1 week ago
On Fri, Mar 14, 2025 at 08:09:39AM +0100, Thomas Huth wrote:
> __ASSEMBLY__ is only defined by the Makefile of the kernel, so
> this is not really useful for uapi headers (unless the userspace
> Makefile defines it, too). Let's switch to __ASSEMBLER__ which
> gets set automatically by the compiler when compiling assembly
> code.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  arch/arm64/include/uapi/asm/kvm.h        | 2 +-
>  arch/arm64/include/uapi/asm/ptrace.h     | 4 ++--
>  arch/arm64/include/uapi/asm/sigcontext.h | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)

Is there a risk of breaking userspace with this? I wonder if it would
be more conservative to do something like:

#if !defined(__ASSEMBLY__) && !defined(__ASSEMBLER__)

so that if somebody is doing '#define __ASSEMBLY__' then they get the
same behaviour as today.

Or maybe we don't care?

Will
Re: [PATCH 08/41] arm64: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers
Posted by Arnd Bergmann 9 months, 1 week ago
On Fri, Mar 14, 2025, at 12:55, Will Deacon wrote:
> On Fri, Mar 14, 2025 at 08:09:39AM +0100, Thomas Huth wrote:
>> __ASSEMBLY__ is only defined by the Makefile of the kernel, so
>> this is not really useful for uapi headers (unless the userspace
>> Makefile defines it, too). Let's switch to __ASSEMBLER__ which
>> gets set automatically by the compiler when compiling assembly
>> code.
>> 
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  arch/arm64/include/uapi/asm/kvm.h        | 2 +-
>>  arch/arm64/include/uapi/asm/ptrace.h     | 4 ++--
>>  arch/arm64/include/uapi/asm/sigcontext.h | 4 ++--
>>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> Is there a risk of breaking userspace with this? I wonder if it would
> be more conservative to do something like:
>
> #if !defined(__ASSEMBLY__) && !defined(__ASSEMBLER__)
>
> so that if somebody is doing '#define __ASSEMBLY__' then they get the
> same behaviour as today.
>
> Or maybe we don't care?

I think the main risk we would have is user applications relying
on the __ASSEMBLER__ checks in new kernel headers and not defining
__ASSEMBLY__. This would result in the application not building
against old kernel headers that only check against __ASSEMBLY__.

Checking for both in the kernel headers does not solve this
problem, and I think we can still decide that we don't care:
in the worst case, an application using the headers from assembly
will have to get fixed later when it needs to be built against
old headers.

I checked that old gcc versions pass __ASSEMBLY__ at least as
far back as gcc-2.95, and it should be completely safe to
assume that no older gcc versions would be used on kernel
headers, and they probably would choke on c99 features like
'long long'. I would also assume that any other compiler that
may be used to include kernel headers has to have enough
gcc compatibility to define all the common macros.

      Arnd
Re: [PATCH 08/41] arm64: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers
Posted by Will Deacon 9 months, 1 week ago
On Fri, Mar 14, 2025 at 01:05:15PM +0100, Arnd Bergmann wrote:
> On Fri, Mar 14, 2025, at 12:55, Will Deacon wrote:
> > On Fri, Mar 14, 2025 at 08:09:39AM +0100, Thomas Huth wrote:
> >> __ASSEMBLY__ is only defined by the Makefile of the kernel, so
> >> this is not really useful for uapi headers (unless the userspace
> >> Makefile defines it, too). Let's switch to __ASSEMBLER__ which
> >> gets set automatically by the compiler when compiling assembly
> >> code.
> >> 
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Will Deacon <will@kernel.org>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>  arch/arm64/include/uapi/asm/kvm.h        | 2 +-
> >>  arch/arm64/include/uapi/asm/ptrace.h     | 4 ++--
> >>  arch/arm64/include/uapi/asm/sigcontext.h | 4 ++--
> >>  3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > Is there a risk of breaking userspace with this? I wonder if it would
> > be more conservative to do something like:
> >
> > #if !defined(__ASSEMBLY__) && !defined(__ASSEMBLER__)
> >
> > so that if somebody is doing '#define __ASSEMBLY__' then they get the
> > same behaviour as today.
> >
> > Or maybe we don't care?
> 
> I think the main risk we would have is user applications relying
> on the __ASSEMBLER__ checks in new kernel headers and not defining
> __ASSEMBLY__. This would result in the application not building
> against old kernel headers that only check against __ASSEMBLY__.

Hmm. I hadn't thought about the case of old headers :/

A quick Debian codesearch shows that glibc might #define __ASSEMBLY__
for some arch-specific headers:

https://codesearch.debian.net/search?q=%23define+__ASSEMBLY__&literal=1

which is what I was more worried about.

> Checking for both in the kernel headers does not solve this
> problem, and I think we can still decide that we don't care:
> in the worst case, an application using the headers from assembly
> will have to get fixed later when it needs to be built against
> old headers.

Old headers might also just be missing definitions that the application
wants, so I suppose there's always the potential for some manual effort
in that case.

Will
Re: [PATCH 08/41] arm64: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers
Posted by Thomas Huth 2 months, 1 week ago
On 14/03/2025 14.42, Will Deacon wrote:
> On Fri, Mar 14, 2025 at 01:05:15PM +0100, Arnd Bergmann wrote:
>> On Fri, Mar 14, 2025, at 12:55, Will Deacon wrote:
>>> On Fri, Mar 14, 2025 at 08:09:39AM +0100, Thomas Huth wrote:
>>>> __ASSEMBLY__ is only defined by the Makefile of the kernel, so
>>>> this is not really useful for uapi headers (unless the userspace
>>>> Makefile defines it, too). Let's switch to __ASSEMBLER__ which
>>>> gets set automatically by the compiler when compiling assembly
>>>> code.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   arch/arm64/include/uapi/asm/kvm.h        | 2 +-
>>>>   arch/arm64/include/uapi/asm/ptrace.h     | 4 ++--
>>>>   arch/arm64/include/uapi/asm/sigcontext.h | 4 ++--
>>>>   3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> Is there a risk of breaking userspace with this? I wonder if it would
>>> be more conservative to do something like:
>>>
>>> #if !defined(__ASSEMBLY__) && !defined(__ASSEMBLER__)
>>>
>>> so that if somebody is doing '#define __ASSEMBLY__' then they get the
>>> same behaviour as today.
>>>
>>> Or maybe we don't care?
>>
>> I think the main risk we would have is user applications relying
>> on the __ASSEMBLER__ checks in new kernel headers and not defining
>> __ASSEMBLY__. This would result in the application not building
>> against old kernel headers that only check against __ASSEMBLY__.
> 
> Hmm. I hadn't thought about the case of old headers :/
> 
> A quick Debian codesearch shows that glibc might #define __ASSEMBLY__
> for some arch-specific headers:
> 
> https://codesearch.debian.net/search?q=%23define+__ASSEMBLY__&literal=1
> 
> which is what I was more worried about.

  Hi!

FWIW, the x86 patches have been merged since kernel v6.15, and as far as I 
know, there haven't been any complaints about the change there yet. Thus I 
assume the changes should be ok.

So I rebased the arm64 patch now, too, and resend them separately as a v2:

  https://lore.kernel.org/lkml/20251010130116.828465-1-thuth@redhat.com/

  Thomas
Re: [PATCH 08/41] arm64: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers
Posted by Thomas Huth 9 months, 1 week ago
On 14/03/2025 14.42, Will Deacon wrote:
> On Fri, Mar 14, 2025 at 01:05:15PM +0100, Arnd Bergmann wrote:
>> On Fri, Mar 14, 2025, at 12:55, Will Deacon wrote:
>>> On Fri, Mar 14, 2025 at 08:09:39AM +0100, Thomas Huth wrote:
>>>> __ASSEMBLY__ is only defined by the Makefile of the kernel, so
>>>> this is not really useful for uapi headers (unless the userspace
>>>> Makefile defines it, too). Let's switch to __ASSEMBLER__ which
>>>> gets set automatically by the compiler when compiling assembly
>>>> code.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   arch/arm64/include/uapi/asm/kvm.h        | 2 +-
>>>>   arch/arm64/include/uapi/asm/ptrace.h     | 4 ++--
>>>>   arch/arm64/include/uapi/asm/sigcontext.h | 4 ++--
>>>>   3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> Is there a risk of breaking userspace with this? I wonder if it would
>>> be more conservative to do something like:
>>>
>>> #if !defined(__ASSEMBLY__) && !defined(__ASSEMBLER__)
>>>
>>> so that if somebody is doing '#define __ASSEMBLY__' then they get the
>>> same behaviour as today.
>>>
>>> Or maybe we don't care?
>>
>> I think the main risk we would have is user applications relying
>> on the __ASSEMBLER__ checks in new kernel headers and not defining
>> __ASSEMBLY__. This would result in the application not building
>> against old kernel headers that only check against __ASSEMBLY__.
> 
> Hmm. I hadn't thought about the case of old headers :/
> 
> A quick Debian codesearch shows that glibc might #define __ASSEMBLY__
> for some arch-specific headers:
> 
> https://codesearch.debian.net/search?q=%23define+__ASSEMBLY__&literal=1
> 
> which is what I was more worried about.

Since both, GCC and Clang, define __ASSEMBLER__ since a long time (Arnd 
checked GCC 2.95, and I checked that at least Clang 7.0 still has it), I 
think the only problem might be other compiler toolchains that might not set 
__ASSEMBLER__ automatically. I just checked Tiny-C 0.9.27, and that also 
sets __ASSEMBLER__ already. And according to 
https://github.com/IanHarvey/pcc/blob/master/cc/cc/cc.1#L405 it is also set 
in PCC.

I haven't spotted it in LCC though (which seems to be an old C89 compiler if 
I got it right). So if we are worried about such exotic old compilers, it's 
maybe better to check both, __ASSEMBLY__ and __ASSEMBLER__ in the uapi 
files? Or would it be ok to force those few people to set __ASSEMBLER__ 
manually in their Makefiles (just like they had to do before with 
__ASSEMBLY__) in case they want to compile assembler code with such exotic 
compilers and new kernel headers?

  Thomas