[PATCH] s390/uapi: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

[PATCH] s390/uapi: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

[Thomas Huth]

__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.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 arch/s390/include/uapi/asm/ptrace.h | 5 +++--
 arch/s390/include/uapi/asm/schid.h  | 4 ++--
 arch/s390/include/uapi/asm/types.h  | 4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h
index bb0826024bb95..ea202072f1ad5 100644
--- a/arch/s390/include/uapi/asm/ptrace.h
+++ b/arch/s390/include/uapi/asm/ptrace.h
@@ -242,7 +242,8 @@
 #define PTRACE_OLDSETOPTIONS		21
 #define PTRACE_SYSEMU			31
 #define PTRACE_SYSEMU_SINGLESTEP	32
-#ifndef __ASSEMBLY__
+
+#ifndef __ASSEMBLER__
 #include <linux/stddef.h>
 #include <linux/types.h>
 
@@ -450,6 +451,6 @@ struct user_regs_struct {
 	unsigned long ieee_instruction_pointer;	/* obsolete, always 0 */
 };
 
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
 
 #endif /* _UAPI_S390_PTRACE_H */
diff --git a/arch/s390/include/uapi/asm/schid.h b/arch/s390/include/uapi/asm/schid.h
index a3e1cf1685534..d804d1a5b1b3f 100644
--- a/arch/s390/include/uapi/asm/schid.h
+++ b/arch/s390/include/uapi/asm/schid.h
@@ -4,7 +4,7 @@
 
 #include <linux/types.h>
 
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
 
 struct subchannel_id {
 	__u32 cssid : 8;
@@ -15,6 +15,6 @@ struct subchannel_id {
 	__u32 sch_no : 16;
 } __attribute__ ((packed, aligned(4)));
 
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
 
 #endif /* _UAPIASM_SCHID_H */
diff --git a/arch/s390/include/uapi/asm/types.h b/arch/s390/include/uapi/asm/types.h
index 84457dbb26b4a..4ab468c5032e3 100644
--- a/arch/s390/include/uapi/asm/types.h
+++ b/arch/s390/include/uapi/asm/types.h
@@ -10,7 +10,7 @@
 
 #include <asm-generic/int-ll64.h>
 
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
 
 typedef unsigned long addr_t;
 typedef __signed__ long saddr_t;
@@ -25,6 +25,6 @@ typedef struct {
 	};
 } __attribute__((packed, aligned(4))) __vector128;
 
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
 
 #endif /* _UAPI_S390_TYPES_H */
-- 
2.48.1

Re: [PATCH] s390/uapi: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

[Heiko Carstens]

On Mon, Mar 10, 2025 at 11:26:57AM +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.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  arch/s390/include/uapi/asm/ptrace.h | 5 +++--
>  arch/s390/include/uapi/asm/schid.h  | 4 ++--
>  arch/s390/include/uapi/asm/types.h  | 4 ++--
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h
> index bb0826024bb95..ea202072f1ad5 100644
> --- a/arch/s390/include/uapi/asm/ptrace.h
> +++ b/arch/s390/include/uapi/asm/ptrace.h
> @@ -242,7 +242,8 @@
>  #define PTRACE_OLDSETOPTIONS		21
>  #define PTRACE_SYSEMU			31
>  #define PTRACE_SYSEMU_SINGLESTEP	32
> -#ifndef __ASSEMBLY__
> +
> +#ifndef __ASSEMBLER__
>  #include <linux/stddef.h>
>  #include <linux/types.h>

...

Did this cause any sorts of problems? I can see this pattern all over
the place, so why is this now a problem?

Also, wouldn't it be better to fix this with an sed statement in
scripts/headers_install.sh instead? Otherwise this is going to be a
never ending story since those things will be re-introduced all the
time.

Re: [PATCH] s390/uapi: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

[Arnd Bergmann]

On Mon, Mar 10, 2025, at 11:49, Heiko Carstens wrote:
> On Mon, Mar 10, 2025 at 11:26:57AM +0100, Thomas Huth wrote:
>
> Did this cause any sorts of problems? I can see this pattern all over
> the place, so why is this now a problem?
>
> Also, wouldn't it be better to fix this with an sed statement in
> scripts/headers_install.sh instead? Otherwise this is going to be a
> never ending story since those things will be re-introduced all the
> time.

It should certainly be done in a consistent way across all
architectures and architecture-independent headers. I see that
all uapi headers use __ASSEMBLY__ consistently, while a few non-uapi
headers use __ASSEMBLER__.

glibc obviously defines __ASSEMBLY__ whenever it includes one
of the kernel headers that need this from a .S file. Unless
there is a known problem with the current code, leaving this
unchanged is probably the least risky way.

   Arnd

Re: [PATCH] s390/uapi: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

[Thomas Huth]

On 10/03/2025 12.07, Arnd Bergmann wrote:
> On Mon, Mar 10, 2025, at 11:49, Heiko Carstens wrote:
>> On Mon, Mar 10, 2025 at 11:26:57AM +0100, Thomas Huth wrote:
>>
>> Did this cause any sorts of problems? I can see this pattern all over
>> the place, so why is this now a problem?
>>
>> Also, wouldn't it be better to fix this with an sed statement in
>> scripts/headers_install.sh instead? Otherwise this is going to be a
>> never ending story since those things will be re-introduced all the
>> time.
> 
> It should certainly be done in a consistent way across all
> architectures and architecture-independent headers. I see that
> all uapi headers use __ASSEMBLY__ consistently, while a few non-uapi
> headers use __ASSEMBLER__.
> 
> glibc obviously defines __ASSEMBLY__ whenever it includes one
> of the kernel headers that need this from a .S file. Unless
> there is a known problem with the current code, leaving this
> unchanged is probably the least risky way.

Well, this seems to be constant source of confusion. It got my attention by 
Sean's recent patch for kvm-unit-tests here:

  https://lore.kernel.org/kvm/20250222014526.2302653-1-seanjc@google.com/

Quoting: "This is essentially a "rage" patch after spending
way, way too much time trying to understand why I couldn't include some
__ASSEMBLY__ protected headers in x86 assembly files."

But also if you search the net for this, there are lots of other spots where 
people get it wrong, e.g.:

  https://stackoverflow.com/questions/28924355/gcc-assembler-preprocessor-not-compatible-with-standard-headers
  https://forums.raspberrypi.com/viewtopic.php?p=1652944#p1653834
  https://github.com/riscv-software-src/opensbi/issues/199

So I thought it would be a good idea to standardize on the #define that is 
set by the compiler already. IMHO it would be great to get it replaced in 
the whole kernel, but that's a little bit bold for one patch. So the obvious 
first step towards that direction is to replace it in the uapi header files 
first, where it hopefully will help to reduce the confusion in userspace. 
So unless you really don't like this idea at all, I could continue with the 
uapi headers for the other architectures, too?

  Thomas

Re: [PATCH] s390/uapi: Replace __ASSEMBLY__ with __ASSEMBLER__ in uapi headers

[Arnd Bergmann]

On Mon, Mar 10, 2025, at 13:14, Thomas Huth wrote:
> On 10/03/2025 12.07, Arnd Bergmann wrote:
>> On Mon, Mar 10, 2025, at 11:49, Heiko Carstens wrote:
>>> On Mon, Mar 10, 2025 at 11:26:57AM +0100, Thomas Huth wrote:
>>>
>>> Did this cause any sorts of problems? I can see this pattern all over
>>> the place, so why is this now a problem?
>>>
>>> Also, wouldn't it be better to fix this with an sed statement in
>>> scripts/headers_install.sh instead? Otherwise this is going to be a
>>> never ending story since those things will be re-introduced all the
>>> time.
>> 
>> It should certainly be done in a consistent way across all
>> architectures and architecture-independent headers. I see that
>> all uapi headers use __ASSEMBLY__ consistently, while a few non-uapi
>> headers use __ASSEMBLER__.
>> 
>> glibc obviously defines __ASSEMBLY__ whenever it includes one
>> of the kernel headers that need this from a .S file. Unless
>> there is a known problem with the current code, leaving this
>> unchanged is probably the least risky way.
>
> Well, this seems to be constant source of confusion. It got my attention by 
> Sean's recent patch for kvm-unit-tests here:
>
>   https://lore.kernel.org/kvm/20250222014526.2302653-1-seanjc@google.com/
>
> Quoting: "This is essentially a "rage" patch after spending
> way, way too much time trying to understand why I couldn't include some
> __ASSEMBLY__ protected headers in x86 assembly files."
>
> But also if you search the net for this, there are lots of other spots where 
> people get it wrong, e.g.:
>
>   
> https://stackoverflow.com/questions/28924355/gcc-assembler-preprocessor-not-compatible-with-standard-headers
>   https://forums.raspberrypi.com/viewtopic.php?p=1652944#p1653834
>   https://github.com/riscv-software-src/opensbi/issues/199

Right

> So I thought it would be a good idea to standardize on the #define that is 
> set by the compiler already. IMHO it would be great to get it replaced in 
> the whole kernel, but that's a little bit bold for one patch. So the obvious 
> first step towards that direction is to replace it in the uapi header files 
> first, where it hopefully will help to reduce the confusion in userspace. 
> So unless you really don't like this idea at all, I could continue with the 
> uapi headers for the other architectures, too?

Standardizing on one of the two is good, and using the one that the
toolchain already provides makes sense. I would prefer a large patch
that replaces all of them (uapi and internal) and removes the
definition at the same time, the way the kvm patch does, but it's
possible that this causes conflicts with architecture specific
patches.

There is a risk of regression when changing the uapi headers, when
new users of the uapi headers don't define __ASSEMBLER__:
This would then work with new kernel headers but not when building
against older headers.

There is also a (smaller) risk when there is userland building
assembler files with a compiler other than gcc or clang, if they set
__ASSEMBLY__ manually but the compiler does not set __ASSEMBLER__.
I checked that gcc has defined __ASSEMBLER__ at least as far back
as egcs-1.0.0 and gcc-2.95, probably longer.

     Arnd