mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] s390: Add assembler-level include guard to insn-common-asm.h
@ 2026-11-02 16:51 Steffen Eiden
  2026-11-03 16:39 ` Alexander Gordeev
  0 siblings, 1 reply; 3+ messages in thread
From: Steffen Eiden @ 2026-11-02 16:51 UTC (permalink / raw)
  To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle
  Cc: linux-s390, linux-kernel, Steffen Eiden

The CPP include guard of insn-common-asm.h is ineffective when the file
is read via a gas-level .include from inline asm. The assembler ignores
all '#' lines, so every .include redefines GR_NUM/VX_NUM and fails with
"Macro was already defined" if a translation unit emits more than one
such include. This is not the case right now but may lead to future
build issues.

Add a .ifndef guard with an L-prefixed (local, non-emitted) symbol so
the file can be include multiple times per assembly unit.

Fixes: 495bc70aa3225 ("s390/fpu: Move GR_NUM / VX_NUM macros to separate header file")
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/asm/insn-common-asm.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/s390/include/asm/insn-common-asm.h b/arch/s390/include/asm/insn-common-asm.h
index fd9b3cacb7c5..060116719846 100644
--- a/arch/s390/include/asm/insn-common-asm.h
+++ b/arch/s390/include/asm/insn-common-asm.h
@@ -9,6 +9,10 @@
 
 #ifdef __ASSEMBLER__
 
+/* Prevent duplicated GAS macro definitions. CPP guards have no effect for GAS.*/
+.ifndef .Lasm_s390_insn_common_asm_h
+.Lasm_s390_insn_common_asm_h = 1
+
 /*
  * GR_NUM - Retrieve general-purpose register number
  *
@@ -49,5 +53,7 @@
 	.endif
 .endm
 
+.endif	/* .Lasm_s390_insn_common_asm_h */
+
 #endif	/* __ASSEMBLER__ */
 #endif	/* __ASM_S390_INSN_COMMON_ASM_H */




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] s390: Add assembler-level include guard to insn-common-asm.h
  2026-11-02 16:51 [PATCH] s390: Add assembler-level include guard to insn-common-asm.h Steffen Eiden
@ 2026-11-03 16:39 ` Alexander Gordeev
  2026-11-04  8:40   ` Steffen Eiden
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Gordeev @ 2026-11-03 16:39 UTC (permalink / raw)
  To: Steffen Eiden
  Cc: Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	Sven Schnelle, linux-s390, linux-kernel

On Mon, Nov 02, 2026 at 05:51:03PM +0100, Steffen Eiden wrote:

Hi Steffen,

> The CPP include guard of insn-common-asm.h is ineffective when the file
> is read via a gas-level .include from inline asm. The assembler ignores
> all '#' lines, so every .include redefines GR_NUM/VX_NUM and fails with
> "Macro was already defined" if a translation unit emits more than one
> such include. This is not the case right now but may lead to future
> build issues.
> 
> Add a .ifndef guard with an L-prefixed (local, non-emitted) symbol so
> the file can be include multiple times per assembly unit.
> 
> Fixes: 495bc70aa3225 ("s390/fpu: Move GR_NUM / VX_NUM macros to separate header file")

I have a problem with Fixes here:
- as you noted above nothing fails right now
- the same behaviour was there before 495bc70aa3225 commit

Is there a real problem this patch fixes?

Do you happen to know why some other archs use .purgem in similar cases
(aka though appears ugly, should not we use .purgem too)?

> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>  arch/s390/include/asm/insn-common-asm.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/s390/include/asm/insn-common-asm.h b/arch/s390/include/asm/insn-common-asm.h
> index fd9b3cacb7c5..060116719846 100644
> --- a/arch/s390/include/asm/insn-common-asm.h
> +++ b/arch/s390/include/asm/insn-common-asm.h
> @@ -9,6 +9,10 @@
>  
>  #ifdef __ASSEMBLER__
>  
> +/* Prevent duplicated GAS macro definitions. CPP guards have no effect for GAS.*/
> +.ifndef .Lasm_s390_insn_common_asm_h
> +.Lasm_s390_insn_common_asm_h = 1
> +
>  /*
>   * GR_NUM - Retrieve general-purpose register number
>   *
> @@ -49,5 +53,7 @@
>  	.endif
>  .endm
>  
> +.endif	/* .Lasm_s390_insn_common_asm_h */
> +
>  #endif	/* __ASSEMBLER__ */
>  #endif	/* __ASM_S390_INSN_COMMON_ASM_H */

Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] s390: Add assembler-level include guard to insn-common-asm.h
  2026-11-03 16:39 ` Alexander Gordeev
@ 2026-11-04  8:40   ` Steffen Eiden
  0 siblings, 0 replies; 3+ messages in thread
From: Steffen Eiden @ 2026-11-04  8:40 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
	Sven Schnelle, linux-s390, linux-kernel

On Tue, Nov 03, 2026 at 05:39:19PM +0100, Alexander Gordeev wrote:
> On Mon, Nov 02, 2026 at 05:51:03PM +0100, Steffen Eiden wrote:
> 
> Hi Steffen,
> 
> > The CPP include guard of insn-common-asm.h is ineffective when the file
> > is read via a gas-level .include from inline asm. The assembler ignores
> > all '#' lines, so every .include redefines GR_NUM/VX_NUM and fails with
> > "Macro was already defined" if a translation unit emits more than one
> > such include. This is not the case right now but may lead to future
> > build issues.
> > 
> > Add a .ifndef guard with an L-prefixed (local, non-emitted) symbol so
> > the file can be include multiple times per assembly unit.
> > 
> > Fixes: 495bc70aa3225 ("s390/fpu: Move GR_NUM / VX_NUM macros to separate header file")
> 
> I have a problem with Fixes here:
> - as you noted above nothing fails right now
> - the same behaviour was there before 495bc70aa3225 commit
>

I have no problem with removing it. You are right this does not fix a
problem right now but prevents future issues. Let's remove it.

> Is there a real problem this patch fixes?
>

Yes. The second batch of arm-on-s390 patches use the GR_NUM macro as
well. I'll send an update for this series soon. And it happens that the
same compilation unit that uses the GR_NUM macro for creating a new
instruction format (not supported by binutils) also uses some FPU stuff.
So the asm marco will be defined twice. That's how I found this issue in
the first place.

I sent this as a separate patch in the hope that this fix gets into 7.4

> Do you happen to know why some other archs use .purgem in similar cases
> (aka though appears ugly, should not we use .purgem too)?
> 

.purgem can be used to undefine a macro for when you want to use it once.
That's how the other arch use it. Define a macro, use it once, purge it.
All in the same file.

We have a different use case here. We defined a reusable macro in a
separate header. So if we want to use .purgem instead the users of the
macro would need to purge it after they used it. This seems to britle
for me as purging it can be forgotten and the issue can manifest way
later in a complete separate patch.

	Steffen

> > Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> > ---
> >  arch/s390/include/asm/insn-common-asm.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/s390/include/asm/insn-common-asm.h b/arch/s390/include/asm/insn-common-asm.h
> > index fd9b3cacb7c5..060116719846 100644
> > --- a/arch/s390/include/asm/insn-common-asm.h
> > +++ b/arch/s390/include/asm/insn-common-asm.h
> > @@ -9,6 +9,10 @@
> >  
> >  #ifdef __ASSEMBLER__
> >  
> > +/* Prevent duplicated GAS macro definitions. CPP guards have no effect for GAS.*/
> > +.ifndef .Lasm_s390_insn_common_asm_h
> > +.Lasm_s390_insn_common_asm_h = 1
> > +
> >  /*
> >   * GR_NUM - Retrieve general-purpose register number
> >   *
> > @@ -49,5 +53,7 @@
> >  	.endif
> >  .endm
> >  
> > +.endif	/* .Lasm_s390_insn_common_asm_h */
> > +
> >  #endif	/* __ASSEMBLER__ */
> >  #endif	/* __ASM_S390_INSN_COMMON_ASM_H */
> 
> Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-04  8:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 16:51 [PATCH] s390: Add assembler-level include guard to insn-common-asm.h Steffen Eiden
2026-11-03 16:39 ` Alexander Gordeev
2026-11-04  8:40   ` Steffen Eiden

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®