linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] s390: allow to build with llvm's integrated assembler
@ 2022-05-11 12:05 Heiko Carstens
  2022-05-11 12:05 ` [PATCH 1/8] s390/alternatives: provide identical sized orginal/alternative sequences Heiko Carstens
                   ` (9 more replies)
  0 siblings, 10 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

A couple of patches which in result make it finally possible to build the
kernel for s390 with llvm's integrated assembler. Several configs build
without errors or warnings, and the kernel also works as expected.

Note that patch 6 ("s390/boot: workaround llvm IAS bug") reveals a
miscompile. This looks like a bug in the instruction definitions of the mvc
and clc instructions(?). I'd like to ask people to look into this, since
this silently generated broken code.

This patch series is based on linux-next, which contains two additional
required s390 specific patches to make llvm's IAS work.

Thanks,
Heiko

Heiko Carstens (8):
  s390/alternatives: provide identical sized orginal/alternative sequences
  s390/alternatives: remove padding generation code
  s390/entry: shorten OUTSIDE macro
  s390/entry: workaround llvm's IAS limitations
  s390/purgatory: workaround llvm's IAS limitations
  s390/boot: workaround llvm IAS bug
  s390/boot: do not emit debug info for assembly with llvm's IAS
  scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390

 arch/s390/Makefile                      |  2 +
 arch/s390/boot/head.S                   | 34 +++++----
 arch/s390/include/asm/alternative-asm.h | 76 +++-----------------
 arch/s390/include/asm/alternative.h     | 93 ++++++-------------------
 arch/s390/include/asm/spinlock.h        |  2 +-
 arch/s390/kernel/alternative.c          | 61 +---------------
 arch/s390/kernel/entry.S                | 39 +++++++----
 arch/s390/lib/spinlock.c                |  4 +-
 arch/s390/purgatory/head.S              | 29 ++++++--
 scripts/min-tool-version.sh             |  3 +-
 10 files changed, 104 insertions(+), 239 deletions(-)

-- 
2.32.0


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

* [PATCH 1/8] s390/alternatives: provide identical sized orginal/alternative sequences
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-13  9:17   ` Vasily Gorbik
  2022-05-11 12:05 ` [PATCH 2/8] s390/alternatives: remove padding generation code Heiko Carstens
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

Explicitly provide identical sized original/alternative instruction
sequences. This way there is no need for the s390 specific alternatives
infrastructure to generate padding sequences.
The code which generates such sequences will be removed with a follow on
patch.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/spinlock.h |  2 +-
 arch/s390/kernel/entry.S         | 20 ++++++++++----------
 arch/s390/lib/spinlock.c         |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/s390/include/asm/spinlock.h b/arch/s390/include/asm/spinlock.h
index 10a460762e94..37127cd7749e 100644
--- a/arch/s390/include/asm/spinlock.h
+++ b/arch/s390/include/asm/spinlock.h
@@ -79,7 +79,7 @@ static inline void arch_spin_unlock(arch_spinlock_t *lp)
 	typecheck(int, lp->lock);
 	kcsan_release();
 	asm_inline volatile(
-		ALTERNATIVE("", ".insn rre,0xb2fa0000,7,0", 49)	/* NIAI 7 */
+		ALTERNATIVE("nop", ".insn rre,0xb2fa0000,7,0", 49) /* NIAI 7 */
 		"	sth	%1,%0\n"
 		: "=R" (((unsigned short *) &lp->lock)[1])
 		: "d" (0) : "cc", "memory");
diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index 685ccec02a27..a6b45eaa3450 100644
--- a/arch/s390/kernel/entry.S
+++ b/arch/s390/kernel/entry.S
@@ -53,19 +53,19 @@ STACK_INIT = STACK_SIZE - STACK_FRAME_OVERHEAD - __PT_SIZE
 _LPP_OFFSET	= __LC_LPP
 
 	.macro STBEAR address
-	ALTERNATIVE "", ".insn	s,0xb2010000,\address", 193
+	ALTERNATIVE "nop", ".insn s,0xb2010000,\address", 193
 	.endm
 
 	.macro LBEAR address
-	ALTERNATIVE "", ".insn	s,0xb2000000,\address", 193
+	ALTERNATIVE "nop", ".insn s,0xb2000000,\address", 193
 	.endm
 
 	.macro LPSWEY address,lpswe
-	ALTERNATIVE "b \lpswe", ".insn siy,0xeb0000000071,\address,0", 193
+	ALTERNATIVE "b \lpswe; nopr", ".insn siy,0xeb0000000071,\address,0", 193
 	.endm
 
 	.macro MBEAR reg
-	ALTERNATIVE "", __stringify(mvc __PT_LAST_BREAK(8,\reg),__LC_LAST_BREAK), 193
+	ALTERNATIVE "brcl 0,0", __stringify(mvc __PT_LAST_BREAK(8,\reg),__LC_LAST_BREAK), 193
 	.endm
 
 	.macro	CHECK_STACK savearea
@@ -121,16 +121,16 @@ _LPP_OFFSET	= __LC_LPP
 	.endm
 
 	.macro BPOFF
-	ALTERNATIVE "", ".insn rrf,0xb2e80000,0,0,12,0", 82
+	ALTERNATIVE "nop", ".insn rrf,0xb2e80000,0,0,12,0", 82
 	.endm
 
 	.macro BPON
-	ALTERNATIVE "", ".insn rrf,0xb2e80000,0,0,13,0", 82
+	ALTERNATIVE "nop", ".insn rrf,0xb2e80000,0,0,13,0", 82
 	.endm
 
 	.macro BPENTER tif_ptr,tif_mask
 	ALTERNATIVE "TSTMSK \tif_ptr,\tif_mask; jz .+8; .insn rrf,0xb2e80000,0,0,13,0", \
-		    "", 82
+		    "j .+12; nop; nop", 82
 	.endm
 
 	.macro BPEXIT tif_ptr,tif_mask
@@ -226,7 +226,7 @@ ENTRY(__switch_to)
 	aghi	%r3,__TASK_pid
 	mvc	__LC_CURRENT_PID(4,%r0),0(%r3)	# store pid of next
 	lmg	%r6,%r15,__SF_GPRS(%r15)	# load gprs of next task
-	ALTERNATIVE "", "lpp _LPP_OFFSET", 40
+	ALTERNATIVE "nop", "lpp _LPP_OFFSET", 40
 	BR_EX	%r14
 ENDPROC(__switch_to)
 
@@ -610,7 +610,7 @@ ENTRY(mcck_int_handler)
 	jno	0f
 	BPEXIT	__TI_flags(%r12),_TIF_ISOLATE_BP
 	stpt	__LC_EXIT_TIMER
-0:	ALTERNATIVE "", __stringify(lghi %r12,__LC_LAST_BREAK_SAVE_AREA),193
+0:	ALTERNATIVE "nop", __stringify(lghi %r12,__LC_LAST_BREAK_SAVE_AREA),193
 	LBEAR	0(%r12)
 	lmg	%r11,%r15,__PT_R11(%r11)
 	LPSWEY	__LC_RETURN_MCCK_PSW,__LC_RETURN_MCCK_LPSWE
@@ -646,7 +646,7 @@ ENTRY(mcck_int_handler)
 ENDPROC(mcck_int_handler)
 
 ENTRY(restart_int_handler)
-	ALTERNATIVE "", "lpp _LPP_OFFSET", 40
+	ALTERNATIVE "nop", "lpp _LPP_OFFSET", 40
 	stg	%r15,__LC_SAVE_AREA_RESTART
 	TSTMSK	__LC_RESTART_FLAGS,RESTART_FLAG_CTLREGS,4
 	jz	0f
diff --git a/arch/s390/lib/spinlock.c b/arch/s390/lib/spinlock.c
index 5e7ea8b111e8..04d4c6cf898e 100644
--- a/arch/s390/lib/spinlock.c
+++ b/arch/s390/lib/spinlock.c
@@ -75,7 +75,7 @@ static inline int arch_load_niai4(int *lock)
 	int owner;
 
 	asm_inline volatile(
-		ALTERNATIVE("", ".insn rre,0xb2fa0000,4,0", 49)	/* NIAI 4 */
+		ALTERNATIVE("nop", ".insn rre,0xb2fa0000,4,0", 49) /* NIAI 4 */
 		"	l	%0,%1\n"
 		: "=d" (owner) : "Q" (*lock) : "memory");
 	return owner;
@@ -86,7 +86,7 @@ static inline int arch_cmpxchg_niai8(int *lock, int old, int new)
 	int expected = old;
 
 	asm_inline volatile(
-		ALTERNATIVE("", ".insn rre,0xb2fa0000,8,0", 49)	/* NIAI 8 */
+		ALTERNATIVE("nop", ".insn rre,0xb2fa0000,8,0", 49) /* NIAI 8 */
 		"	cs	%0,%3,%1\n"
 		: "=d" (old), "=Q" (*lock)
 		: "0" (old), "d" (new), "Q" (*lock)
-- 
2.32.0


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

* [PATCH 2/8] s390/alternatives: remove padding generation code
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
  2022-05-11 12:05 ` [PATCH 1/8] s390/alternatives: provide identical sized orginal/alternative sequences Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-11 17:02   ` Heiko Carstens
  2022-05-13  9:18   ` Vasily Gorbik
  2022-05-11 12:05 ` [PATCH 3/8] s390/entry: shorten OUTSIDE macro Heiko Carstens
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

clang fails to handle ".if" statements in inline assembly which are heavily
used in the alternatives code.

To work around this remove this code, and enforce that users of
alternatives must specify original and alternative instruction sequences
which have identical sizes. Add a compile time check with two ".org"
statements similar to arm64.

In result not only clang can handle this, but also quite a lot of code can
be removed.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/alternative-asm.h | 76 +++-----------------
 arch/s390/include/asm/alternative.h     | 93 ++++++-------------------
 arch/s390/kernel/alternative.c          | 61 +---------------
 3 files changed, 31 insertions(+), 199 deletions(-)

diff --git a/arch/s390/include/asm/alternative-asm.h b/arch/s390/include/asm/alternative-asm.h
index bb3837d7387c..7db046596b93 100644
--- a/arch/s390/include/asm/alternative-asm.h
+++ b/arch/s390/include/asm/alternative-asm.h
@@ -4,19 +4,6 @@
 
 #ifdef __ASSEMBLY__
 
-/*
- * Check the length of an instruction sequence. The length may not be larger
- * than 254 bytes and it has to be divisible by 2.
- */
-.macro alt_len_check start,end
-	.if ( \end - \start ) > 254
-	.error "cpu alternatives does not support instructions blocks > 254 bytes\n"
-	.endif
-	.if ( \end - \start ) % 2
-	.error "cpu alternatives instructions length is odd\n"
-	.endif
-.endm
-
 /*
  * Issue one struct alt_instr descriptor entry (need to put it into
  * the section .altinstructions, see below). This entry contains
@@ -28,66 +15,29 @@
 	.long	\alt_start - .
 	.word	\feature
 	.byte	\orig_end - \orig_start
-	.byte	\alt_end - \alt_start
-.endm
-
-/*
- * Fill up @bytes with nops. The macro emits 6-byte nop instructions
- * for the bulk of the area, possibly followed by a 4-byte and/or
- * a 2-byte nop if the size of the area is not divisible by 6.
- */
-.macro alt_pad_fill bytes
-	.rept	( \bytes ) / 6
-	brcl	0,0
-	.endr
-	.rept	( \bytes ) % 6 / 4
-	nop
-	.endr
-	.rept	( \bytes ) % 6 % 4 / 2
-	nopr
-	.endr
-.endm
-
-/*
- * Fill up @bytes with nops. If the number of bytes is larger
- * than 6, emit a jg instruction to branch over all nops, then
- * fill an area of size (@bytes - 6) with nop instructions.
- */
-.macro alt_pad bytes
-	.if ( \bytes > 0 )
-	.if ( \bytes > 6 )
-	jg	. + \bytes
-	alt_pad_fill \bytes - 6
-	.else
-	alt_pad_fill \bytes
-	.endif
-	.endif
+	.org	. - ( \orig_end - \orig_start ) + ( \alt_end - \alt_start )
+	.org	. - ( \alt_end - \alt_start ) + ( \orig_end - \orig_start )
 .endm
 
 /*
  * Define an alternative between two instructions. If @feature is
  * present, early code in apply_alternatives() replaces @oldinstr with
- * @newinstr. ".skip" directive takes care of proper instruction padding
- * in case @newinstr is longer than @oldinstr.
+ * @newinstr.
  */
 .macro ALTERNATIVE oldinstr, newinstr, feature
 	.pushsection .altinstr_replacement,"ax"
 770:	\newinstr
 771:	.popsection
 772:	\oldinstr
-773:	alt_len_check 770b, 771b
-	alt_len_check 772b, 773b
-	alt_pad ( ( 771b - 770b ) - ( 773b - 772b ) )
-774:	.pushsection .altinstructions,"a"
-	alt_entry 772b, 774b, 770b, 771b, \feature
+773:	.pushsection .altinstructions,"a"
+	alt_entry 772b, 773b, 770b, 771b, \feature
 	.popsection
 .endm
 
 /*
  * Define an alternative between two instructions. If @feature is
  * present, early code in apply_alternatives() replaces @oldinstr with
- * @newinstr. ".skip" directive takes care of proper instruction padding
- * in case @newinstr is longer than @oldinstr.
+ * @newinstr.
  */
 .macro ALTERNATIVE_2 oldinstr, newinstr1, feature1, newinstr2, feature2
 	.pushsection .altinstr_replacement,"ax"
@@ -95,17 +45,9 @@
 771:	\newinstr2
 772:	.popsection
 773:	\oldinstr
-774:	alt_len_check 770b, 771b
-	alt_len_check 771b, 772b
-	alt_len_check 773b, 774b
-	.if ( 771b - 770b > 772b - 771b )
-	alt_pad ( ( 771b - 770b ) - ( 774b - 773b ) )
-	.else
-	alt_pad ( ( 772b - 771b ) - ( 774b - 773b ) )
-	.endif
-775:	.pushsection .altinstructions,"a"
-	alt_entry 773b, 775b, 770b, 771b,\feature1
-	alt_entry 773b, 775b, 771b, 772b,\feature2
+774:	.pushsection .altinstructions,"a"
+	alt_entry 773b, 774b, 770b, 771b,\feature1
+	alt_entry 773b, 774b, 771b, 772b,\feature2
 	.popsection
 .endm
 
diff --git a/arch/s390/include/asm/alternative.h b/arch/s390/include/asm/alternative.h
index 3f2856ed6808..904dd049f954 100644
--- a/arch/s390/include/asm/alternative.h
+++ b/arch/s390/include/asm/alternative.h
@@ -13,32 +13,25 @@ struct alt_instr {
 	s32 repl_offset;	/* offset to replacement instruction */
 	u16 facility;		/* facility bit set for replacement */
 	u8  instrlen;		/* length of original instruction */
-	u8  replacementlen;	/* length of new instruction */
 } __packed;
 
 void apply_alternative_instructions(void);
 void apply_alternatives(struct alt_instr *start, struct alt_instr *end);
 
 /*
- * |661:       |662:	  |6620      |663:
- * +-----------+---------------------+
- * | oldinstr  | oldinstr_padding    |
- * |	       +----------+----------+
- * |	       |	  |	     |
- * |	       | >6 bytes |6/4/2 nops|
- * |	       |6 bytes jg----------->
- * +-----------+---------------------+
- *		 ^^ static padding ^^
+ * +---------------------------------+
+ * |661:			     |662:
+ * | oldinstr			     |
+ * +---------------------------------+
  *
  * .altinstr_replacement section
- * +---------------------+-----------+
+ * +---------------------------------+
  * |6641:			     |6651:
  * | alternative instr 1	     |
- * +-----------+---------+- - - - - -+
- * |6642:		 |6652:      |
- * | alternative instr 2 | padding
- * +---------------------+- - - - - -+
- *			  ^ runtime ^
+ * +---------------------------------+
+ * |6642:			     |6652:
+ * | alternative instr 2	     |
+ * +---------------------------------+
  *
  * .altinstructions section
  * +---------------------------------+
@@ -47,77 +40,31 @@ void apply_alternatives(struct alt_instr *start, struct alt_instr *end);
  * +---------------------------------+
  */
 
-#define b_altinstr(num)	"664"#num
-#define e_altinstr(num)	"665"#num
-
-#define e_oldinstr_pad_end	"663"
+#define b_altinstr(num)		"664"#num
+#define e_altinstr(num)		"665"#num
 #define oldinstr_len		"662b-661b"
-#define oldinstr_total_len	e_oldinstr_pad_end"b-661b"
 #define altinstr_len(num)	e_altinstr(num)"b-"b_altinstr(num)"b"
-#define oldinstr_pad_len(num) \
-	"-(((" altinstr_len(num) ")-(" oldinstr_len ")) > 0) * " \
-	"((" altinstr_len(num) ")-(" oldinstr_len "))"
-
-#define INSTR_LEN_SANITY_CHECK(len)					\
-	".if " len " > 254\n"						\
-	"\t.error \"cpu alternatives does not support instructions "	\
-		"blocks > 254 bytes\"\n"				\
-	".endif\n"							\
-	".if (" len ") %% 2\n"						\
-	"\t.error \"cpu alternatives instructions length is odd\"\n"	\
-	".endif\n"
-
-#define OLDINSTR_PADDING(oldinstr, num)					\
-	".if " oldinstr_pad_len(num) " > 6\n"				\
-	"\tjg " e_oldinstr_pad_end "f\n"				\
-	"6620:\n"							\
-	"\t.rept (" oldinstr_pad_len(num) " - (6620b-662b)) / 2\n"	\
-	"\tnopr\n"							\
-	".else\n"							\
-	"\t.rept " oldinstr_pad_len(num) " / 6\n"			\
-	"\t.brcl 0,0\n"							\
-	"\t.endr\n"							\
-	"\t.rept " oldinstr_pad_len(num) " %% 6 / 4\n"			\
-	"\tnop\n"							\
-	"\t.endr\n"							\
-	"\t.rept " oldinstr_pad_len(num) " %% 6 %% 4 / 2\n"		\
-	"\tnopr\n"							\
-	".endr\n"							\
-	".endif\n"
-
-#define OLDINSTR(oldinstr, num)						\
-	"661:\n\t" oldinstr "\n662:\n"					\
-	OLDINSTR_PADDING(oldinstr, num)					\
-	e_oldinstr_pad_end ":\n"					\
-	INSTR_LEN_SANITY_CHECK(oldinstr_len)
-
-#define OLDINSTR_2(oldinstr, num1, num2)				\
-	"661:\n\t" oldinstr "\n662:\n"					\
-	".if " altinstr_len(num1) " < " altinstr_len(num2) "\n"		\
-	OLDINSTR_PADDING(oldinstr, num2)				\
-	".else\n"							\
-	OLDINSTR_PADDING(oldinstr, num1)				\
-	".endif\n"							\
-	e_oldinstr_pad_end ":\n"					\
-	INSTR_LEN_SANITY_CHECK(oldinstr_len)
+
+#define OLDINSTR(oldinstr) \
+	"661:\n\t" oldinstr "\n662:\n"
 
 #define ALTINSTR_ENTRY(facility, num)					\
 	"\t.long 661b - .\n"			/* old instruction */	\
 	"\t.long " b_altinstr(num)"b - .\n"	/* alt instruction */	\
 	"\t.word " __stringify(facility) "\n"	/* facility bit    */	\
-	"\t.byte " oldinstr_total_len "\n"	/* source len	   */	\
-	"\t.byte " altinstr_len(num) "\n"	/* alt instruction len */
+	"\t.byte " oldinstr_len "\n"		/* instruction len */	\
+	"\t.org . - (" oldinstr_len ") + (" altinstr_len(num) ")\n"	\
+	"\t.org . - (" altinstr_len(num) ") + (" oldinstr_len ")\n"
 
 #define ALTINSTR_REPLACEMENT(altinstr, num)	/* replacement */	\
-	b_altinstr(num)":\n\t" altinstr "\n" e_altinstr(num) ":\n"	\
-	INSTR_LEN_SANITY_CHECK(altinstr_len(num))
+	b_altinstr(num)":\n\t" altinstr "\n" e_altinstr(num) ":\n"
 
 /* alternative assembly primitive: */
 #define ALTERNATIVE(oldinstr, altinstr, facility) \
 	".pushsection .altinstr_replacement, \"ax\"\n"			\
 	ALTINSTR_REPLACEMENT(altinstr, 1)				\
 	".popsection\n"							\
-	OLDINSTR(oldinstr, 1)						\
+	OLDINSTR(oldinstr)						\
 	".pushsection .altinstructions,\"a\"\n"				\
 	ALTINSTR_ENTRY(facility, 1)					\
 	".popsection\n"
@@ -127,7 +74,7 @@ void apply_alternatives(struct alt_instr *start, struct alt_instr *end);
 	ALTINSTR_REPLACEMENT(altinstr1, 1)				\
 	ALTINSTR_REPLACEMENT(altinstr2, 2)				\
 	".popsection\n"							\
-	OLDINSTR_2(oldinstr, 1, 2)					\
+	OLDINSTR(oldinstr)						\
 	".pushsection .altinstructions,\"a\"\n"				\
 	ALTINSTR_ENTRY(facility1, 1)					\
 	ALTINSTR_ENTRY(facility2, 2)					\
diff --git a/arch/s390/kernel/alternative.c b/arch/s390/kernel/alternative.c
index cce0ddee2d02..e7bca29f9c34 100644
--- a/arch/s390/kernel/alternative.c
+++ b/arch/s390/kernel/alternative.c
@@ -7,8 +7,6 @@
 #include <asm/facility.h>
 #include <asm/nospec-branch.h>
 
-#define MAX_PATCH_LEN (255 - 1)
-
 static int __initdata_or_module alt_instr_disabled;
 
 static int __init disable_alternative_instructions(char *str)
@@ -19,85 +17,30 @@ static int __init disable_alternative_instructions(char *str)
 
 early_param("noaltinstr", disable_alternative_instructions);
 
-struct brcl_insn {
-	u16 opc;
-	s32 disp;
-} __packed;
-
-static u16 __initdata_or_module nop16 = 0x0700;
-static u32 __initdata_or_module nop32 = 0x47000000;
-static struct brcl_insn __initdata_or_module nop48 = {
-	0xc004, 0
-};
-
-static const void *nops[] __initdata_or_module = {
-	&nop16,
-	&nop32,
-	&nop48
-};
-
-static void __init_or_module add_jump_padding(void *insns, unsigned int len)
-{
-	struct brcl_insn brcl = {
-		0xc0f4,
-		len / 2
-	};
-
-	memcpy(insns, &brcl, sizeof(brcl));
-	insns += sizeof(brcl);
-	len -= sizeof(brcl);
-
-	while (len > 0) {
-		memcpy(insns, &nop16, 2);
-		insns += 2;
-		len -= 2;
-	}
-}
-
-static void __init_or_module add_padding(void *insns, unsigned int len)
-{
-	if (len > 6)
-		add_jump_padding(insns, len);
-	else if (len >= 2)
-		memcpy(insns, nops[len / 2 - 1], len);
-}
-
 static void __init_or_module __apply_alternatives(struct alt_instr *start,
 						  struct alt_instr *end)
 {
 	struct alt_instr *a;
 	u8 *instr, *replacement;
-	u8 insnbuf[MAX_PATCH_LEN];
 
 	/*
 	 * The scan order should be from start to end. A later scanned
 	 * alternative code can overwrite previously scanned alternative code.
 	 */
 	for (a = start; a < end; a++) {
-		int insnbuf_sz = 0;
-
 		instr = (u8 *)&a->instr_offset + a->instr_offset;
 		replacement = (u8 *)&a->repl_offset + a->repl_offset;
 
 		if (!__test_facility(a->facility, alt_stfle_fac_list))
 			continue;
 
-		if (unlikely(a->instrlen % 2 || a->replacementlen % 2)) {
+		if (unlikely(a->instrlen % 2)) {
 			WARN_ONCE(1, "cpu alternatives instructions length is "
 				     "odd, skipping patching\n");
 			continue;
 		}
 
-		memcpy(insnbuf, replacement, a->replacementlen);
-		insnbuf_sz = a->replacementlen;
-
-		if (a->instrlen > a->replacementlen) {
-			add_padding(insnbuf + a->replacementlen,
-				    a->instrlen - a->replacementlen);
-			insnbuf_sz += a->instrlen - a->replacementlen;
-		}
-
-		s390_kernel_write(instr, insnbuf, insnbuf_sz);
+		s390_kernel_write(instr, replacement, a->instrlen);
 	}
 }
 
-- 
2.32.0


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

* [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
  2022-05-11 12:05 ` [PATCH 1/8] s390/alternatives: provide identical sized orginal/alternative sequences Heiko Carstens
  2022-05-11 12:05 ` [PATCH 2/8] s390/alternatives: remove padding generation code Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-12 15:59   ` kernel test robot
                     ` (2 more replies)
  2022-05-11 12:05 ` [PATCH 4/8] s390/entry: workaround llvm's IAS limitations Heiko Carstens
                   ` (6 subsequent siblings)
  9 siblings, 3 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

Since the minimum architecture level has been raised to z10 a shorter
instruction sequence can be used to implement the OUTSIDE macro. This
also reduces the number of used registers within that macro to one.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/entry.S | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index a6b45eaa3450..e1664b45090f 100644
--- a/arch/s390/kernel/entry.S
+++ b/arch/s390/kernel/entry.S
@@ -169,11 +169,9 @@ _LPP_OFFSET	= __LC_LPP
 	 * @outside_label: jump here if @reg is outside of [@start..@end)
 	 */
 	.macro OUTSIDE reg,start,end,outside_label
-	lgr	%r14,\reg
-	larl	%r13,\start
-	slgr	%r14,%r13
-	lghi	%r13,\end - \start
-	clgr	%r14,%r13
+	larl	%r14,\start
+	slgrk	%r14,\reg,%r14
+	clgfi	%r14,\end - \start
 	jhe	\outside_label
 	.endm
 
-- 
2.32.0


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

* [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
                   ` (2 preceding siblings ...)
  2022-05-11 12:05 ` [PATCH 3/8] s390/entry: shorten OUTSIDE macro Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-11 17:30   ` Nathan Chancellor
  2022-05-11 12:05 ` [PATCH 5/8] s390/purgatory: " Heiko Carstens
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

llvm's integrated assembler cannot handle immediate values which are
calculated with two local labels:

<instantiation>:3:13: error: invalid operand for instruction
 clgfi %r14,.Lsie_done - .Lsie_gmap

Workaround this by adding clang specific code which reads the specific
value from memory. Since this code is within the hot paths of the kernel
and adds an additional memory reference, keep the original code, and add
ifdef'ed code.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/entry.S | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index e1664b45090f..ff7a75078e93 100644
--- a/arch/s390/kernel/entry.S
+++ b/arch/s390/kernel/entry.S
@@ -171,8 +171,19 @@ _LPP_OFFSET	= __LC_LPP
 	.macro OUTSIDE reg,start,end,outside_label
 	larl	%r14,\start
 	slgrk	%r14,\reg,%r14
+#ifdef CONFIG_CC_IS_CLANG
+	clgfrl	%r14,.Lrange_size\@
+#else
 	clgfi	%r14,\end - \start
+#endif
 	jhe	\outside_label
+#ifdef CONFIG_CC_IS_CLANG
+	.section .rodata, "a"
+	.align 4
+.Lrange_size\@:
+	.long	\end - \start
+	.previous
+#endif
 	.endm
 
 	.macro SIEEXIT
-- 
2.32.0


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

* [PATCH 5/8] s390/purgatory: workaround llvm's IAS limitations
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
                   ` (3 preceding siblings ...)
  2022-05-11 12:05 ` [PATCH 4/8] s390/entry: workaround llvm's IAS limitations Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-11 19:54   ` Nick Desaulniers
  2022-05-12 17:25   ` Heiko Carstens
  2022-05-11 12:05 ` [PATCH 6/8] s390/boot: workaround llvm IAS bug Heiko Carstens
                   ` (4 subsequent siblings)
  9 siblings, 2 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

llvm's integrated assembler cannot handle immediate values which are
calculated with two local labels:

arch/s390/purgatory/head.S:139:11: error: invalid operand for instruction
 aghi %r8,-(.base_crash-purgatory_start)

Workaround this by partially rewriting the code.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/purgatory/head.S | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/s390/purgatory/head.S b/arch/s390/purgatory/head.S
index 3d1c31e0cf3d..ac1a27a20b66 100644
--- a/arch/s390/purgatory/head.S
+++ b/arch/s390/purgatory/head.S
@@ -44,11 +44,14 @@
 .endm
 
 .macro MEMSWAP dst,src,buf,len
-10:	cghi	\len,bufsz
+10:	larl	%r0,purgatory_end
+	larl	%r1,stack
+	slgr	%r0,%r1
+	cgr	\len,%r0
 	jh	11f
 	lgr	%r4,\len
 	j	12f
-11:	lghi	%r4,bufsz
+11:	lgr	%r4,%r0
 
 12:	MEMCPY	\buf,\dst,%r4
 	MEMCPY	\dst,\src,%r4
@@ -135,12 +138,18 @@ ENTRY(purgatory_start)
 
 .start_crash_kernel:
 	/* Location of purgatory_start in crash memory */
+	larl	%r0,.base_crash
+	larl	%r1,purgatory_start
+	slgr	%r0,%r1
 	lgr	%r8,%r13
-	aghi	%r8,-(.base_crash-purgatory_start)
+	sgr	%r8,%r0
 
 	/* Destination for this code i.e. end of memory to be swapped. */
+	larl	%r0,purgatory_end
+	larl	%r1,purgatory_start
+	slgr	%r0,%r1
 	lg	%r9,crash_size-.base_crash(%r13)
-	aghi	%r9,-(purgatory_end-purgatory_start)
+	sgr	%r9,%r0
 
 	/* Destination in crash memory, i.e. same as r9 but in crash memory. */
 	lg	%r10,crash_start-.base_crash(%r13)
@@ -149,15 +158,19 @@ ENTRY(purgatory_start)
 	/* Buffer location (in crash memory) and size. As the purgatory is
 	 * behind the point of no return it can re-use the stack as buffer.
 	 */
-	lghi	%r11,bufsz
+	larl	%r11,purgatory_end
 	larl	%r12,stack
+	slgr	%r11,%r12
 
 	MEMCPY	%r12,%r9,%r11	/* dst	-> (crash) buf */
 	MEMCPY	%r9,%r8,%r11	/* self -> dst */
 
 	/* Jump to new location. */
 	lgr	%r7,%r9
-	aghi	%r7,.jump_to_dst-purgatory_start
+	larl	%r0,.jump_to_dst
+	larl	%r1,purgatory_start
+	slgr	%r0,%r1
+	agr	%r7,%r0
 	br	%r7
 
 .jump_to_dst:
@@ -169,7 +182,9 @@ ENTRY(purgatory_start)
 
 	/* Load new buffer location after jump */
 	larl	%r7,stack
-	aghi	%r10,stack-purgatory_start
+	larl	%r0,purgatory_start
+	slgrk	%r0,%r7,%r0
+	agr	%r10,%r0
 	MEMCPY	%r10,%r7,%r11	/* (new) buf -> (crash) buf */
 
 	/* Now the code is set up to run from its designated location. Start
-- 
2.32.0


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

* [PATCH 6/8] s390/boot: workaround llvm IAS bug
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
                   ` (4 preceding siblings ...)
  2022-05-11 12:05 ` [PATCH 5/8] s390/purgatory: " Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-11 19:50   ` Nick Desaulniers
  2022-05-11 12:05 ` [PATCH 7/8] s390/boot: do not emit debug info for assembly with llvm's IAS Heiko Carstens
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

For at least the mvc and clc instructions llvm's integrated assembler can
generate incorrect code. In particular this happens with decompressor boot
code. The reason seems to be that relocations for the second displacement
of each instruction are at incorrect locations (-/+: gas vs llvm IAS):

mvc     __LC_IO_NEW_PSW(16),.Lnewpsw

results in

        4:      d2 0f 01 f0 00 00       mvc     496(16,%r0),0
-                       8: R_390_12     .head.text+0x10
+                       6: R_390_12     .head.text+0x10

and
clc     0(3,%r4),.L_hdr
results in

      258:      d5 02 40 00 00 00       clc     0(3,%r4),0
-                       25c: R_390_12   .head.text+0x324
+                       25a: R_390_12   .head.text+0x324

Workaround this by writing the code in a different way.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/boot/head.S | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/s390/boot/head.S b/arch/s390/boot/head.S
index 2ced90172680..8402e1cd133b 100644
--- a/arch/s390/boot/head.S
+++ b/arch/s390/boot/head.S
@@ -42,7 +42,8 @@ ipl_start:
 # subroutine to wait for end I/O
 #
 .Lirqwait:
-	mvc	__LC_IO_NEW_PSW(16),.Lnewpsw	# set up IO interrupt psw
+	larl	%r13,.Lnewpsw		# set up IO interrupt psw
+	mvc	__LC_IO_NEW_PSW(16),0(%r13)
 	lpsw	.Lwaitpsw
 .Lioint:
 	br	%r14
@@ -155,9 +156,11 @@ ipl_start:
 	lr	%r2,%r3
 .Lnotrunc:
 	l	%r4,.Linitrd
-	clc	0(3,%r4),.L_hdr		# if it is HDRx
+	larl	%r13,.L_hdr
+	clc	0(3,%r4),0(%r13)	# if it is HDRx
 	bz	.Lagain1		# skip dataset header
-	clc	0(3,%r4),.L_eof		# if it is EOFx
+	larl	%r13,.L_eof
+	clc	0(3,%r4),0(%r13)	# if it is EOFx
 	bz	.Lagain1		# skip dateset trailer
 
 	lr	%r5,%r2
@@ -181,9 +184,11 @@ ipl_start:
 .Lrdcont:
 	l	%r2,.Linitrd
 
-	clc	0(3,%r2),.L_hdr		# skip HDRx and EOFx
+	larl	%r13,.L_hdr		# skip HDRx and EOFx
+	clc	0(3,%r2),0(%r13)
 	bz	.Lagain2
-	clc	0(3,%r2),.L_eof
+	larl	%r13,.L_eof
+	clc	0(3,%r2),0(%r13)
 	bz	.Lagain2
 
 #
@@ -260,20 +265,23 @@ SYM_CODE_START_LOCAL(startup_normal)
 	.fill	16,4,0x0
 0:	lmh	%r0,%r15,0(%r13)	# clear high-order half of gprs
 	sam64				# switch to 64 bit addressing mode
-	basr	%r13,0			# get base
-.LPG0:
-	mvc	__LC_EXT_NEW_PSW(16),.Lext_new_psw-.LPG0(%r13)
-	mvc	__LC_PGM_NEW_PSW(16),.Lpgm_new_psw-.LPG0(%r13)
-	mvc	__LC_IO_NEW_PSW(16),.Lio_new_psw-.LPG0(%r13)
+	larl	%r13,.Lext_new_psw
+	mvc	__LC_EXT_NEW_PSW(16),0(%r13)
+	larl	%r13,.Lpgm_new_psw
+	mvc	__LC_PGM_NEW_PSW(16),0(%r13)
+	larl	%r13,.Lio_new_psw
+	mvc	__LC_IO_NEW_PSW(16),0(%r13)
 	xc	0x200(256),0x200	# partially clear lowcore
 	xc	0x300(256),0x300
 	xc	0xe00(256),0xe00
 	xc	0xf00(256),0xf00
-	lctlg	%c0,%c15,.Lctl-.LPG0(%r13)	# load control registers
+	larl	%r13,.Lctl
+	lctlg	%c0,%c15,0(%r13)	# load control registers
 	stcke	__LC_BOOT_CLOCK
 	mvc	__LC_LAST_UPDATE_CLOCK(8),__LC_BOOT_CLOCK+1
-	spt	6f-.LPG0(%r13)
-	mvc	__LC_LAST_UPDATE_TIMER(8),6f-.LPG0(%r13)
+	larl	%r13,6f
+	spt	0(%r13)
+	mvc	__LC_LAST_UPDATE_TIMER(8),0(%r13)
 	larl	%r15,_stack_end-STACK_FRAME_OVERHEAD
 	brasl	%r14,sclp_early_setup_buffer
 	brasl	%r14,verify_facilities
-- 
2.32.0


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

* [PATCH 7/8] s390/boot: do not emit debug info for assembly with llvm's IAS
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
                   ` (5 preceding siblings ...)
  2022-05-11 12:05 ` [PATCH 6/8] s390/boot: workaround llvm IAS bug Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-11 19:40   ` Nick Desaulniers
  2022-05-11 12:05 ` [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390 Heiko Carstens
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

Commit ee6d777d3e93 ("s390/decompressor: support extra debug flags")
added extra debug flags, in particular debug info is created,
depending on config options.

With llvm's IAS this causes this compile warning:

arch/s390/boot/head.S:38:1: warning: DWARF2 only supports one section per compilation unit
.section ".head.text","ax"
^

This is a known problem and was addressed with a commit b8a9092330da
("Kbuild: do not emit debug info for assembly with LLVM_IAS=1").
Just do the same for s390 to get rid of this warning.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/s390/Makefile b/arch/s390/Makefile
index c59efc83f020..d73611b35164 100644
--- a/arch/s390/Makefile
+++ b/arch/s390/Makefile
@@ -20,7 +20,9 @@ LDFLAGS_vmlinux	:= -pie
 endif
 aflags_dwarf	:= -Wa,-gdwarf-2
 KBUILD_AFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -D__ASSEMBLY__
+ifndef CONFIG_AS_IS_LLVM
 KBUILD_AFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),$(aflags_dwarf))
+endif
 KBUILD_CFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -O2 -mpacked-stack
 KBUILD_CFLAGS_DECOMPRESSOR += -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY
 KBUILD_CFLAGS_DECOMPRESSOR += -fno-delete-null-pointer-checks -msoft-float -mbackchain
-- 
2.32.0


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

* [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
                   ` (6 preceding siblings ...)
  2022-05-11 12:05 ` [PATCH 7/8] s390/boot: do not emit debug info for assembly with llvm's IAS Heiko Carstens
@ 2022-05-11 12:05 ` Heiko Carstens
  2022-05-11 19:27   ` Nick Desaulniers
  2022-05-11 19:48 ` [PATCH 0/8] s390: allow to build with llvm's integrated assembler Nick Desaulniers
  2022-05-11 20:52 ` Nathan Chancellor
  9 siblings, 1 reply; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 12:05 UTC (permalink / raw)
  To: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov
  Cc: Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

Before version 14.0.0 llvm's integrated assembler fails to handle some
displacement variants:

arch/s390/purgatory/head.S:108:10: error: invalid operand for instruction
 lg %r11,kernel_type-.base_crash(%r13)

Instead of working around this and given that this is already fixed
raise the minimum clang version from 13.0.0 to 14.0.0.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 scripts/min-tool-version.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
index 53fe64856015..f1e8358ec19a 100755
--- a/scripts/min-tool-version.sh
+++ b/scripts/min-tool-version.sh
@@ -24,9 +24,8 @@ icc)
 	echo 16.0.3
 	;;
 llvm)
-	# https://lore.kernel.org/r/YMtib5hKVyNknZt3@osiris/
 	if [ "$SRCARCH" = s390 ]; then
-		echo 13.0.0
+		echo 14.0.0
 	else
 		echo 11.0.0
 	fi
-- 
2.32.0


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

* Re: [PATCH 2/8] s390/alternatives: remove padding generation code
  2022-05-11 12:05 ` [PATCH 2/8] s390/alternatives: remove padding generation code Heiko Carstens
@ 2022-05-11 17:02   ` Heiko Carstens
  2022-05-13  9:18   ` Vasily Gorbik
  1 sibling, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-11 17:02 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Wed, May 11, 2022 at 02:05:26PM +0200, Heiko Carstens wrote:
> clang fails to handle ".if" statements in inline assembly which are heavily
> used in the alternatives code.

FWIW, I missed to add error message(s) to the changelog:

In file included from ./include/linux/spinlock.h:93:
./arch/s390/include/asm/spinlock.h:81:3: error: expected absolute expression
                ALTERNATIVE("", ".insn rre,0xb2fa0000,7,0", 49) /* NIAI 7 */
                ^
./arch/s390/include/asm/alternative.h:118:2: note: expanded from macro 'ALTERNATIVE'
        ALTINSTR_REPLACEMENT(altinstr, 1)                               \
        ^
./arch/s390/include/asm/alternative.h:113:2: note: expanded from macro 'ALTINSTR_REPLACEMENT'
        INSTR_LEN_SANITY_CHECK(altinstr_len(num))
        ^
./arch/s390/include/asm/alternative.h:62:3: note: expanded from macro 'INSTR_LEN_SANITY_CHECK'
        ".if " len " > 254\n"                                           \
         ^
<inline asm>:5:5: note: instantiated into assembly here
.if 6651b-6641b > 254
    ^

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-11 12:05 ` [PATCH 4/8] s390/entry: workaround llvm's IAS limitations Heiko Carstens
@ 2022-05-11 17:30   ` Nathan Chancellor
  2022-05-12 17:24     ` Heiko Carstens
  0 siblings, 1 reply; 41+ messages in thread
From: Nathan Chancellor @ 2022-05-11 17:30 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

Hi Heiko,

On Wed, May 11, 2022 at 02:05:28PM +0200, Heiko Carstens wrote:
> llvm's integrated assembler cannot handle immediate values which are
> calculated with two local labels:
> 
> <instantiation>:3:13: error: invalid operand for instruction
>  clgfi %r14,.Lsie_done - .Lsie_gmap
> 
> Workaround this by adding clang specific code which reads the specific
> value from memory. Since this code is within the hot paths of the kernel
> and adds an additional memory reference, keep the original code, and add
> ifdef'ed code.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/kernel/entry.S | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> index e1664b45090f..ff7a75078e93 100644
> --- a/arch/s390/kernel/entry.S
> +++ b/arch/s390/kernel/entry.S
> @@ -171,8 +171,19 @@ _LPP_OFFSET	= __LC_LPP
>  	.macro OUTSIDE reg,start,end,outside_label
>  	larl	%r14,\start
>  	slgrk	%r14,\reg,%r14
> +#ifdef CONFIG_CC_IS_CLANG

I intend to put this series through my build and boot test matrix later
today but one fly by comment in the meantime. Should this be
CONFIG_AS_IS_LLVM if this is an integrated assembler limitation, rather
than a clang one?

> +	clgfrl	%r14,.Lrange_size\@
> +#else
>  	clgfi	%r14,\end - \start
> +#endif
>  	jhe	\outside_label
> +#ifdef CONFIG_CC_IS_CLANG
> +	.section .rodata, "a"
> +	.align 4
> +.Lrange_size\@:
> +	.long	\end - \start
> +	.previous
> +#endif
>  	.endm
>  
>  	.macro SIEEXIT
> -- 
> 2.32.0
> 
> 

Cheers,
Nathan

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

* Re: [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390
  2022-05-11 12:05 ` [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390 Heiko Carstens
@ 2022-05-11 19:27   ` Nick Desaulniers
  2022-05-11 19:56     ` Nick Desaulniers
  0 siblings, 1 reply; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-11 19:27 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> Before version 14.0.0 llvm's integrated assembler fails to handle some
> displacement variants:
>
> arch/s390/purgatory/head.S:108:10: error: invalid operand for instruction
>  lg %r11,kernel_type-.base_crash(%r13)
>
> Instead of working around this and given that this is already fixed
> raise the minimum clang version from 13.0.0 to 14.0.0.

Do you have the commit in LLVM that fixed this? Might be nice to link
to the particular commit in the commit message. Either way:
Acked-by: Nick Desaulniers <ndesaulniers@google.com>

(Thanks for the series, will pull down and test!)

If you have a github account, let me know it if you'd like to be cc'ed
when we wire this up in our CI.

>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  scripts/min-tool-version.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
> index 53fe64856015..f1e8358ec19a 100755
> --- a/scripts/min-tool-version.sh
> +++ b/scripts/min-tool-version.sh
> @@ -24,9 +24,8 @@ icc)
>         echo 16.0.3
>         ;;
>  llvm)
> -       # https://lore.kernel.org/r/YMtib5hKVyNknZt3@osiris/
>         if [ "$SRCARCH" = s390 ]; then
> -               echo 13.0.0
> +               echo 14.0.0
>         else
>                 echo 11.0.0
>         fi
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 7/8] s390/boot: do not emit debug info for assembly with llvm's IAS
  2022-05-11 12:05 ` [PATCH 7/8] s390/boot: do not emit debug info for assembly with llvm's IAS Heiko Carstens
@ 2022-05-11 19:40   ` Nick Desaulniers
  2022-05-12 17:30     ` Heiko Carstens
  0 siblings, 1 reply; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-11 19:40 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> Commit ee6d777d3e93 ("s390/decompressor: support extra debug flags")
> added extra debug flags, in particular debug info is created,
> depending on config options.
>
> With llvm's IAS this causes this compile warning:
>
> arch/s390/boot/head.S:38:1: warning: DWARF2 only supports one section per compilation unit
> .section ".head.text","ax"
> ^
>
> This is a known problem and was addressed with a commit b8a9092330da
> ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1").
> Just do the same for s390 to get rid of this warning.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> index c59efc83f020..d73611b35164 100644
> --- a/arch/s390/Makefile
> +++ b/arch/s390/Makefile
> @@ -20,7 +20,9 @@ LDFLAGS_vmlinux       := -pie
>  endif
>  aflags_dwarf   := -Wa,-gdwarf-2

^ or can we use a more modern variant of dwarf, like at least dwarf-4?

>  KBUILD_AFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -D__ASSEMBLY__
> +ifndef CONFIG_AS_IS_LLVM
>  KBUILD_AFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),$(aflags_dwarf))
> +endif
>  KBUILD_CFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -O2 -mpacked-stack
>  KBUILD_CFLAGS_DECOMPRESSOR += -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY
>  KBUILD_CFLAGS_DECOMPRESSOR += -fno-delete-null-pointer-checks -msoft-float -mbackchain
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 0/8] s390: allow to build with llvm's integrated assembler
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
                   ` (7 preceding siblings ...)
  2022-05-11 12:05 ` [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390 Heiko Carstens
@ 2022-05-11 19:48 ` Nick Desaulniers
  2022-05-12 19:04   ` Heiko Carstens
  2022-05-11 20:52 ` Nathan Chancellor
  9 siblings, 1 reply; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-11 19:48 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> A couple of patches which in result make it finally possible to build the
> kernel for s390 with llvm's integrated assembler. Several configs build
> without errors or warnings, and the kernel also works as expected.
>
> Note that patch 6 ("s390/boot: workaround llvm IAS bug") reveals a
> miscompile. This looks like a bug in the instruction definitions of the mvc
> and clc instructions(?). I'd like to ask people to look into this, since
> this silently generated broken code.
>
> This patch series is based on linux-next, which contains two additional
> required s390 specific patches to make llvm's IAS work.

I did a quick test of just a defconfig via:
$ ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- make CC=clang -j72 defconfig all
and this assembled then booted in qemu for me. Thanks for the work
that went into this!

Tested-by: Nick Desaulniers <ndesaulniers@google.com>

Sounds like Nathan is doing additional testing as well.

>
> Thanks,
> Heiko
>
> Heiko Carstens (8):
>   s390/alternatives: provide identical sized orginal/alternative sequences
>   s390/alternatives: remove padding generation code
>   s390/entry: shorten OUTSIDE macro
>   s390/entry: workaround llvm's IAS limitations
>   s390/purgatory: workaround llvm's IAS limitations
>   s390/boot: workaround llvm IAS bug
>   s390/boot: do not emit debug info for assembly with llvm's IAS
>   scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390
>
>  arch/s390/Makefile                      |  2 +
>  arch/s390/boot/head.S                   | 34 +++++----
>  arch/s390/include/asm/alternative-asm.h | 76 +++-----------------
>  arch/s390/include/asm/alternative.h     | 93 ++++++-------------------
>  arch/s390/include/asm/spinlock.h        |  2 +-
>  arch/s390/kernel/alternative.c          | 61 +---------------
>  arch/s390/kernel/entry.S                | 39 +++++++----
>  arch/s390/lib/spinlock.c                |  4 +-
>  arch/s390/purgatory/head.S              | 29 ++++++--
>  scripts/min-tool-version.sh             |  3 +-
>  10 files changed, 104 insertions(+), 239 deletions(-)
>
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 6/8] s390/boot: workaround llvm IAS bug
  2022-05-11 12:05 ` [PATCH 6/8] s390/boot: workaround llvm IAS bug Heiko Carstens
@ 2022-05-11 19:50   ` Nick Desaulniers
  0 siblings, 0 replies; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-11 19:50 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> For at least the mvc and clc instructions llvm's integrated assembler can
> generate incorrect code. In particular this happens with decompressor boot
> code. The reason seems to be that relocations for the second displacement
> of each instruction are at incorrect locations (-/+: gas vs llvm IAS):
>
> mvc     __LC_IO_NEW_PSW(16),.Lnewpsw
>
> results in
>
>         4:      d2 0f 01 f0 00 00       mvc     496(16,%r0),0
> -                       8: R_390_12     .head.text+0x10
> +                       6: R_390_12     .head.text+0x10
>
> and
> clc     0(3,%r4),.L_hdr
> results in
>
>       258:      d5 02 40 00 00 00       clc     0(3,%r4),0
> -                       25c: R_390_12   .head.text+0x324
> +                       25a: R_390_12   .head.text+0x324
>
> Workaround this by writing the code in a different way.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Please link to an LLVM bugreport for this.

> ---
>  arch/s390/boot/head.S | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/arch/s390/boot/head.S b/arch/s390/boot/head.S
> index 2ced90172680..8402e1cd133b 100644
> --- a/arch/s390/boot/head.S
> +++ b/arch/s390/boot/head.S
> @@ -42,7 +42,8 @@ ipl_start:
>  # subroutine to wait for end I/O
>  #
>  .Lirqwait:
> -       mvc     __LC_IO_NEW_PSW(16),.Lnewpsw    # set up IO interrupt psw
> +       larl    %r13,.Lnewpsw           # set up IO interrupt psw
> +       mvc     __LC_IO_NEW_PSW(16),0(%r13)
>         lpsw    .Lwaitpsw
>  .Lioint:
>         br      %r14
> @@ -155,9 +156,11 @@ ipl_start:
>         lr      %r2,%r3
>  .Lnotrunc:
>         l       %r4,.Linitrd
> -       clc     0(3,%r4),.L_hdr         # if it is HDRx
> +       larl    %r13,.L_hdr
> +       clc     0(3,%r4),0(%r13)        # if it is HDRx
>         bz      .Lagain1                # skip dataset header
> -       clc     0(3,%r4),.L_eof         # if it is EOFx
> +       larl    %r13,.L_eof
> +       clc     0(3,%r4),0(%r13)        # if it is EOFx
>         bz      .Lagain1                # skip dateset trailer
>
>         lr      %r5,%r2
> @@ -181,9 +184,11 @@ ipl_start:
>  .Lrdcont:
>         l       %r2,.Linitrd
>
> -       clc     0(3,%r2),.L_hdr         # skip HDRx and EOFx
> +       larl    %r13,.L_hdr             # skip HDRx and EOFx
> +       clc     0(3,%r2),0(%r13)
>         bz      .Lagain2
> -       clc     0(3,%r2),.L_eof
> +       larl    %r13,.L_eof
> +       clc     0(3,%r2),0(%r13)
>         bz      .Lagain2
>
>  #
> @@ -260,20 +265,23 @@ SYM_CODE_START_LOCAL(startup_normal)
>         .fill   16,4,0x0
>  0:     lmh     %r0,%r15,0(%r13)        # clear high-order half of gprs
>         sam64                           # switch to 64 bit addressing mode
> -       basr    %r13,0                  # get base
> -.LPG0:
> -       mvc     __LC_EXT_NEW_PSW(16),.Lext_new_psw-.LPG0(%r13)
> -       mvc     __LC_PGM_NEW_PSW(16),.Lpgm_new_psw-.LPG0(%r13)
> -       mvc     __LC_IO_NEW_PSW(16),.Lio_new_psw-.LPG0(%r13)
> +       larl    %r13,.Lext_new_psw
> +       mvc     __LC_EXT_NEW_PSW(16),0(%r13)
> +       larl    %r13,.Lpgm_new_psw
> +       mvc     __LC_PGM_NEW_PSW(16),0(%r13)
> +       larl    %r13,.Lio_new_psw
> +       mvc     __LC_IO_NEW_PSW(16),0(%r13)
>         xc      0x200(256),0x200        # partially clear lowcore
>         xc      0x300(256),0x300
>         xc      0xe00(256),0xe00
>         xc      0xf00(256),0xf00
> -       lctlg   %c0,%c15,.Lctl-.LPG0(%r13)      # load control registers
> +       larl    %r13,.Lctl
> +       lctlg   %c0,%c15,0(%r13)        # load control registers
>         stcke   __LC_BOOT_CLOCK
>         mvc     __LC_LAST_UPDATE_CLOCK(8),__LC_BOOT_CLOCK+1
> -       spt     6f-.LPG0(%r13)
> -       mvc     __LC_LAST_UPDATE_TIMER(8),6f-.LPG0(%r13)
> +       larl    %r13,6f
> +       spt     0(%r13)
> +       mvc     __LC_LAST_UPDATE_TIMER(8),0(%r13)
>         larl    %r15,_stack_end-STACK_FRAME_OVERHEAD
>         brasl   %r14,sclp_early_setup_buffer
>         brasl   %r14,verify_facilities
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 5/8] s390/purgatory: workaround llvm's IAS limitations
  2022-05-11 12:05 ` [PATCH 5/8] s390/purgatory: " Heiko Carstens
@ 2022-05-11 19:54   ` Nick Desaulniers
  2022-05-12 17:26     ` Heiko Carstens
  2022-05-12 17:25   ` Heiko Carstens
  1 sibling, 1 reply; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-11 19:54 UTC (permalink / raw)
  To: Heiko Carstens, Jonas Paulsson
  Cc: Vasily Gorbik, Alexander Gordeev, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> llvm's integrated assembler cannot handle immediate values which are
> calculated with two local labels:
>
> arch/s390/purgatory/head.S:139:11: error: invalid operand for instruction
>  aghi %r8,-(.base_crash-purgatory_start)

I thought this was fixed in
https://github.com/ClangBuiltLinux/linux/issues/1420
https://reviews.llvm.org/D113341
(clang-14)
?

>
> Workaround this by partially rewriting the code.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/purgatory/head.S | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/arch/s390/purgatory/head.S b/arch/s390/purgatory/head.S
> index 3d1c31e0cf3d..ac1a27a20b66 100644
> --- a/arch/s390/purgatory/head.S
> +++ b/arch/s390/purgatory/head.S
> @@ -44,11 +44,14 @@
>  .endm
>
>  .macro MEMSWAP dst,src,buf,len
> -10:    cghi    \len,bufsz
> +10:    larl    %r0,purgatory_end
> +       larl    %r1,stack
> +       slgr    %r0,%r1
> +       cgr     \len,%r0
>         jh      11f
>         lgr     %r4,\len
>         j       12f
> -11:    lghi    %r4,bufsz
> +11:    lgr     %r4,%r0
>
>  12:    MEMCPY  \buf,\dst,%r4
>         MEMCPY  \dst,\src,%r4
> @@ -135,12 +138,18 @@ ENTRY(purgatory_start)
>
>  .start_crash_kernel:
>         /* Location of purgatory_start in crash memory */
> +       larl    %r0,.base_crash
> +       larl    %r1,purgatory_start
> +       slgr    %r0,%r1
>         lgr     %r8,%r13
> -       aghi    %r8,-(.base_crash-purgatory_start)
> +       sgr     %r8,%r0
>
>         /* Destination for this code i.e. end of memory to be swapped. */
> +       larl    %r0,purgatory_end
> +       larl    %r1,purgatory_start
> +       slgr    %r0,%r1
>         lg      %r9,crash_size-.base_crash(%r13)
> -       aghi    %r9,-(purgatory_end-purgatory_start)
> +       sgr     %r9,%r0
>
>         /* Destination in crash memory, i.e. same as r9 but in crash memory. */
>         lg      %r10,crash_start-.base_crash(%r13)
> @@ -149,15 +158,19 @@ ENTRY(purgatory_start)
>         /* Buffer location (in crash memory) and size. As the purgatory is
>          * behind the point of no return it can re-use the stack as buffer.
>          */
> -       lghi    %r11,bufsz
> +       larl    %r11,purgatory_end
>         larl    %r12,stack
> +       slgr    %r11,%r12
>
>         MEMCPY  %r12,%r9,%r11   /* dst  -> (crash) buf */
>         MEMCPY  %r9,%r8,%r11    /* self -> dst */
>
>         /* Jump to new location. */
>         lgr     %r7,%r9
> -       aghi    %r7,.jump_to_dst-purgatory_start
> +       larl    %r0,.jump_to_dst
> +       larl    %r1,purgatory_start
> +       slgr    %r0,%r1
> +       agr     %r7,%r0
>         br      %r7
>
>  .jump_to_dst:
> @@ -169,7 +182,9 @@ ENTRY(purgatory_start)
>
>         /* Load new buffer location after jump */
>         larl    %r7,stack
> -       aghi    %r10,stack-purgatory_start
> +       larl    %r0,purgatory_start
> +       slgrk   %r0,%r7,%r0
> +       agr     %r10,%r0
>         MEMCPY  %r10,%r7,%r11   /* (new) buf -> (crash) buf */
>
>         /* Now the code is set up to run from its designated location. Start
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390
  2022-05-11 19:27   ` Nick Desaulniers
@ 2022-05-11 19:56     ` Nick Desaulniers
  2022-05-12 19:06       ` Heiko Carstens
  0 siblings, 1 reply; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-11 19:56 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 12:27 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> >
> > Before version 14.0.0 llvm's integrated assembler fails to handle some
> > displacement variants:
> >
> > arch/s390/purgatory/head.S:108:10: error: invalid operand for instruction
> >  lg %r11,kernel_type-.base_crash(%r13)
> >
> > Instead of working around this and given that this is already fixed
> > raise the minimum clang version from 13.0.0 to 14.0.0.
>
> Do you have the commit in LLVM that fixed this? Might be nice to link

Maybe it's
https://reviews.llvm.org/D113341?

Also, these are the open issues we had for the integrated assembler.
https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aopen+is%3Aissue+label%3A%22%5BARCH%5D+s390%22+label%3A%22%5BTOOL%5D+integrated-as%22

Any chance you could include relevant Link tags on your commit
messages for patches that address these? It makes it easier to track
when/where things land if we ever intend to backport anything to
stable.

Or can any of those be closed out?

> to the particular commit in the commit message. Either way:
> Acked-by: Nick Desaulniers <ndesaulniers@google.com>
>
> (Thanks for the series, will pull down and test!)
>
> If you have a github account, let me know it if you'd like to be cc'ed
> when we wire this up in our CI.
>
> >
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> >  scripts/min-tool-version.sh | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
> > index 53fe64856015..f1e8358ec19a 100755
> > --- a/scripts/min-tool-version.sh
> > +++ b/scripts/min-tool-version.sh
> > @@ -24,9 +24,8 @@ icc)
> >         echo 16.0.3
> >         ;;
> >  llvm)
> > -       # https://lore.kernel.org/r/YMtib5hKVyNknZt3@osiris/
> >         if [ "$SRCARCH" = s390 ]; then
> > -               echo 13.0.0
> > +               echo 14.0.0
> >         else
> >                 echo 11.0.0
> >         fi
> > --
> > 2.32.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 0/8] s390: allow to build with llvm's integrated assembler
  2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
                   ` (8 preceding siblings ...)
  2022-05-11 19:48 ` [PATCH 0/8] s390: allow to build with llvm's integrated assembler Nick Desaulniers
@ 2022-05-11 20:52 ` Nathan Chancellor
  2022-05-12 19:03   ` Heiko Carstens
  9 siblings, 1 reply; 41+ messages in thread
From: Nathan Chancellor @ 2022-05-11 20:52 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

Hi Heiko,

On Wed, May 11, 2022 at 02:05:24PM +0200, Heiko Carstens wrote:
> A couple of patches which in result make it finally possible to build the
> kernel for s390 with llvm's integrated assembler. Several configs build
> without errors or warnings, and the kernel also works as expected.
> 
> Note that patch 6 ("s390/boot: workaround llvm IAS bug") reveals a
> miscompile. This looks like a bug in the instruction definitions of the mvc
> and clc instructions(?). I'd like to ask people to look into this, since
> this silently generated broken code.

I think it should be pretty simple to file a bug report for this since
it occurs in a standalone assembly file? I agree with Nick that there
should be a bug report filed and linked to in patch 6 so that we don't
lose track of it.

> This patch series is based on linux-next, which contains two additional
> required s390 specific patches to make llvm's IAS work.
> 
> Thanks,
> Heiko
> 
> Heiko Carstens (8):
>   s390/alternatives: provide identical sized orginal/alternative sequences
>   s390/alternatives: remove padding generation code
>   s390/entry: shorten OUTSIDE macro
>   s390/entry: workaround llvm's IAS limitations
>   s390/purgatory: workaround llvm's IAS limitations
>   s390/boot: workaround llvm IAS bug
>   s390/boot: do not emit debug info for assembly with llvm's IAS
>   scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390
> 
>  arch/s390/Makefile                      |  2 +
>  arch/s390/boot/head.S                   | 34 +++++----
>  arch/s390/include/asm/alternative-asm.h | 76 +++-----------------
>  arch/s390/include/asm/alternative.h     | 93 ++++++-------------------
>  arch/s390/include/asm/spinlock.h        |  2 +-
>  arch/s390/kernel/alternative.c          | 61 +---------------
>  arch/s390/kernel/entry.S                | 39 +++++++----
>  arch/s390/lib/spinlock.c                |  4 +-
>  arch/s390/purgatory/head.S              | 29 ++++++--
>  scripts/min-tool-version.sh             |  3 +-
>  10 files changed, 104 insertions(+), 239 deletions(-)

I applied this series to the latest s390 for-next branch (c4fb15578802)
and built a few in-tree and distribution configurations with clang-14
and clang-15 then boot tested them in QEMU with a simple buildroot
userspace. I did not see any new warnings or errors. This is awesome, I
am excited to get this wired up in our CI!

In case it is worthwhile:

Tested-by: Nathan Chancellor <nathan@kernel.org>

Cheers,
Nathan

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

* Re: [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-11 12:05 ` [PATCH 3/8] s390/entry: shorten OUTSIDE macro Heiko Carstens
@ 2022-05-12 15:59   ` kernel test robot
  2022-05-12 16:20   ` kernel test robot
  2022-05-12 17:21   ` Heiko Carstens
  2 siblings, 0 replies; 41+ messages in thread
From: kernel test robot @ 2022-05-12 15:59 UTC (permalink / raw)
  To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Jonas Paulsson,
	Ulrich Weigand, Masahiro Yamada, Alexander Egorenkov
  Cc: kbuild-all, Sven Schnelle, Andreas Krebbel, Nathan Chancellor,
	Nick Desaulniers, linux-kernel, linux-s390

Hi Heiko,

I love your patch! Yet something to improve:

[auto build test ERROR on s390/features]
[also build test ERROR on tip/locking/core masahiroy-kbuild/for-next linus/master v5.18-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Heiko-Carstens/s390-allow-to-build-with-llvm-s-integrated-assembler/20220511-201054
base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
config: s390-randconfig-p001-20220512 (https://download.01.org/0day-ci/archive/20220512/202205122320.fTZIQ9dT-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ad8a0a6488ba252aaa84b813dee2c949ae59d88a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Heiko-Carstens/s390-allow-to-build-with-llvm-s-integrated-assembler/20220511-201054
        git checkout ad8a0a6488ba252aaa84b813dee2c949ae59d88a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash arch/s390/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/s390/kernel/entry.S: Assembler messages:
>> arch/s390/kernel/entry.S:378: Error: Unrecognized opcode: `slgrk'
   arch/s390/kernel/entry.S:488: Error: Unrecognized opcode: `slgrk'
   arch/s390/kernel/entry.S:489: Error: Unrecognized opcode: `slgrk'
   arch/s390/kernel/entry.S:554: Error: Unrecognized opcode: `slgrk'
   arch/s390/kernel/entry.S:555: Error: Unrecognized opcode: `slgrk'


vim +378 arch/s390/kernel/entry.S

26a374ae7af8d7 arch/s390/kernel/entry.S   Martin Schwidefsky 2019-01-17  359  
^1da177e4c3f41 arch/s390/kernel/entry64.S Linus Torvalds     2005-04-16  360  /*
^1da177e4c3f41 arch/s390/kernel/entry64.S Linus Torvalds     2005-04-16  361   * Program check handler routine
^1da177e4c3f41 arch/s390/kernel/entry64.S Linus Torvalds     2005-04-16  362   */
^1da177e4c3f41 arch/s390/kernel/entry64.S Linus Torvalds     2005-04-16  363  
144d634a21caff arch/s390/kernel/entry64.S Jan Glauber        2011-07-24  364  ENTRY(pgm_check_handler)
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  365  	stpt	__LC_SYS_ENTER_TIMER
d768bd892fc8f0 arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  366  	BPOFF
c5328901aa1db1 arch/s390/kernel/entry64.S Martin Schwidefsky 2011-12-27  367  	stmg	%r8,%r15,__LC_SAVE_AREA_SYNC
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  368  	lg	%r12,__LC_CURRENT
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  369  	lghi	%r10,0
c5328901aa1db1 arch/s390/kernel/entry64.S Martin Schwidefsky 2011-12-27  370  	lmg	%r8,%r9,__LC_PGM_OLD_PSW
87d5986345219a arch/s390/kernel/entry.S   Heiko Carstens     2020-11-16  371  	tmhh	%r8,0x0001		# coming from user space?
87d5986345219a arch/s390/kernel/entry.S   Heiko Carstens     2020-11-16  372  	jno	.Lpgm_skip_asce
87d5986345219a arch/s390/kernel/entry.S   Heiko Carstens     2020-11-16  373  	lctlg	%c1,%c1,__LC_KERNEL_ASCE
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  374  	j	3f			# -> fault in user space
87d5986345219a arch/s390/kernel/entry.S   Heiko Carstens     2020-11-16  375  .Lpgm_skip_asce:
d0fc41071a6884 arch/s390/kernel/entry.S   Martin Schwidefsky 2015-06-22  376  #if IS_ENABLED(CONFIG_KVM)
0a5e2ec2647737 arch/s390/kernel/entry.S   Martin Schwidefsky 2017-10-05  377  	# cleanup critical section for program checks in sie64a
b5415c8f975506 arch/s390/kernel/entry.S   Alexander Gordeev  2021-06-07 @378  	OUTSIDE	%r9,.Lsie_gmap,.Lsie_done,1f
fbbdfca5c5535f arch/s390/kernel/entry.S   Alexander Gordeev  2021-06-18  379  	SIEEXIT
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  380  	lghi	%r10,_PIF_GUEST_FAULT
d0fc41071a6884 arch/s390/kernel/entry.S   Martin Schwidefsky 2015-06-22  381  #endif
0b38b5e1d0e2f3 arch/s390/kernel/entry.S   Sven Schnelle      2020-01-22  382  1:	tmhh	%r8,0x4000		# PER bit set in old PSW ?
0b38b5e1d0e2f3 arch/s390/kernel/entry.S   Sven Schnelle      2020-01-22  383  	jnz	2f			# -> enabled, can't be a double fault
c5328901aa1db1 arch/s390/kernel/entry64.S Martin Schwidefsky 2011-12-27  384  	tm	__LC_PGM_ILC+3,0x80	# check for per exception
86ed42f401cb8f arch/s390/kernel/entry64.S Martin Schwidefsky 2014-12-03  385  	jnz	.Lpgm_svcper		# -> single stepped svc
0b38b5e1d0e2f3 arch/s390/kernel/entry.S   Sven Schnelle      2020-01-22  386  2:	CHECK_STACK __LC_SAVE_AREA_SYNC
dc7ee00d4771b3 arch/s390/kernel/entry64.S Martin Schwidefsky 2013-04-24  387  	aghi	%r15,-(STACK_FRAME_OVERHEAD + __PT_SIZE)
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  388  	# CHECK_VMAP_STACK branches to stack_overflow or 4f
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  389  	CHECK_VMAP_STACK __LC_SAVE_AREA_SYNC,4f
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  390  3:	BPENTER __TI_flags(%r12),_TIF_ISOLATE_BP
c5328901aa1db1 arch/s390/kernel/entry64.S Martin Schwidefsky 2011-12-27  391  	lg	%r15,__LC_KERNEL_STACK
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  392  4:	la	%r11,STACK_FRAME_OVERHEAD(%r15)
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  393  	stg	%r10,__PT_FLAGS(%r11)
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  394  	xc	__SF_BACKCHAIN(8,%r15),__SF_BACKCHAIN(%r15)
c5328901aa1db1 arch/s390/kernel/entry64.S Martin Schwidefsky 2011-12-27  395  	stmg	%r0,%r7,__PT_R0(%r11)
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  396  	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_SYNC
3b051e89da70d4 arch/s390/kernel/entry.S   Sven Schnelle      2021-04-07  397  	mvc	__PT_LAST_BREAK(8,%r11),__LC_PGM_LAST_BREAK
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  398  	stmg	%r8,%r9,__PT_PSW(%r11)
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  399  
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  400  	# clear user controlled registers to prevent speculative use
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  401  	xgr	%r0,%r0
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  402  	xgr	%r1,%r1
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  403  	xgr	%r3,%r3
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  404  	xgr	%r4,%r4
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  405  	xgr	%r5,%r5
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  406  	xgr	%r6,%r6
7041d28115e91f arch/s390/kernel/entry.S   Martin Schwidefsky 2018-01-16  407  	xgr	%r7,%r7
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  408  	lgr	%r2,%r11
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  409  	brasl	%r14,__do_pgm_check
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  410  	tmhh	%r8,0x0001		# returning to user space?
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  411  	jno	.Lpgm_exit_kernel
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  412  	lctlg	%c1,%c1,__LC_USER_ASCE
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  413  	BPEXIT __TI_flags(%r12),_TIF_ISOLATE_BP
0cd9b7230cc57b arch/s390/kernel/entry.S   Heiko Carstens     2020-11-11  414  	stpt	__LC_EXIT_TIMER
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  415  .Lpgm_exit_kernel:
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  416  	mvc	__LC_RETURN_PSW(16),STACK_FRAME_OVERHEAD+__PT_PSW(%r15)
3b051e89da70d4 arch/s390/kernel/entry.S   Sven Schnelle      2021-04-07  417  	LBEAR	STACK_FRAME_OVERHEAD+__PT_LAST_BREAK(%r15)
56e62a73702836 arch/s390/kernel/entry.S   Sven Schnelle      2020-11-21  418  	lmg	%r0,%r15,STACK_FRAME_OVERHEAD+__PT_R0(%r15)
3b051e89da70d4 arch/s390/kernel/entry.S   Sven Schnelle      2021-04-07  419  	LPSWEY	__LC_RETURN_PSW,__LC_RETURN_LPSWE
^1da177e4c3f41 arch/s390/kernel/entry64.S Linus Torvalds     2005-04-16  420  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-11 12:05 ` [PATCH 3/8] s390/entry: shorten OUTSIDE macro Heiko Carstens
  2022-05-12 15:59   ` kernel test robot
@ 2022-05-12 16:20   ` kernel test robot
  2022-05-12 17:21   ` Heiko Carstens
  2 siblings, 0 replies; 41+ messages in thread
From: kernel test robot @ 2022-05-12 16:20 UTC (permalink / raw)
  To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Jonas Paulsson,
	Ulrich Weigand, Masahiro Yamada, Alexander Egorenkov
  Cc: llvm, kbuild-all, Sven Schnelle, Andreas Krebbel,
	Nathan Chancellor, Nick Desaulniers, linux-kernel, linux-s390

Hi Heiko,

I love your patch! Yet something to improve:

[auto build test ERROR on s390/features]
[also build test ERROR on tip/locking/core masahiroy-kbuild/for-next linus/master v5.18-rc6 next-20220512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Heiko-Carstens/s390-allow-to-build-with-llvm-s-integrated-assembler/20220511-201054
base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
config: s390-randconfig-r003-20220512 (https://download.01.org/0day-ci/archive/20220513/202205130050.x6Is9kJO-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 18dd123c56754edf62c7042dcf23185c3727610f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/ad8a0a6488ba252aaa84b813dee2c949ae59d88a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Heiko-Carstens/s390-allow-to-build-with-llvm-s-integrated-assembler/20220511-201054
        git checkout ad8a0a6488ba252aaa84b813dee2c949ae59d88a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> <instantiation>:2:2: error: instruction requires: distinct-ops
    slgrk %r14,%r9,%r14
    ^
   arch/s390/kernel/entry.S:378:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,1f
    ^
   <instantiation>:3:13: error: invalid operand for instruction
    clgfi %r14,.Lsie_done - .Lsie_gmap
               ^
   arch/s390/kernel/entry.S:378:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,1f
    ^
>> <instantiation>:2:2: error: instruction requires: distinct-ops
    slgrk %r14,%r9,%r14
    ^
   <instantiation>:12:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,0f
    ^
   arch/s390/kernel/entry.S:488:1: note: while in macro instantiation
   INT_HANDLER ext_int_handler,304,do_ext_irq
   ^
   <instantiation>:3:13: error: invalid operand for instruction
    clgfi %r14,.Lsie_done - .Lsie_gmap
               ^
   <instantiation>:12:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,0f
    ^
   arch/s390/kernel/entry.S:488:1: note: while in macro instantiation
   INT_HANDLER ext_int_handler,304,do_ext_irq
   ^
>> <instantiation>:2:2: error: instruction requires: distinct-ops
    slgrk %r14,%r9,%r14
    ^
   <instantiation>:12:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,0f
    ^
   arch/s390/kernel/entry.S:489:1: note: while in macro instantiation
   INT_HANDLER io_int_handler,368,do_io_irq
   ^
   <instantiation>:3:13: error: invalid operand for instruction
    clgfi %r14,.Lsie_done - .Lsie_gmap
               ^
   <instantiation>:12:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,0f
    ^
   arch/s390/kernel/entry.S:489:1: note: while in macro instantiation
   INT_HANDLER io_int_handler,368,do_io_irq
   ^
>> <instantiation>:2:2: error: instruction requires: distinct-ops
    slgrk %r14,%r9,%r14
    ^
   arch/s390/kernel/entry.S:554:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,6f
    ^
   <instantiation>:3:13: error: invalid operand for instruction
    clgfi %r14,.Lsie_done - .Lsie_gmap
               ^
   arch/s390/kernel/entry.S:554:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_gmap,.Lsie_done,6f
    ^
>> <instantiation>:2:2: error: instruction requires: distinct-ops
    slgrk %r14,%r9,%r14
    ^
   arch/s390/kernel/entry.S:555:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_entry,.Lsie_skip,4f
    ^
   <instantiation>:3:13: error: invalid operand for instruction
    clgfi %r14,.Lsie_skip - .Lsie_entry
               ^
   arch/s390/kernel/entry.S:555:2: note: while in macro instantiation
    OUTSIDE %r9,.Lsie_entry,.Lsie_skip,4f
    ^

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-11 12:05 ` [PATCH 3/8] s390/entry: shorten OUTSIDE macro Heiko Carstens
  2022-05-12 15:59   ` kernel test robot
  2022-05-12 16:20   ` kernel test robot
@ 2022-05-12 17:21   ` Heiko Carstens
  2022-05-12 18:00     ` Nick Desaulniers
  2 siblings, 1 reply; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 17:21 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Wed, May 11, 2022 at 02:05:27PM +0200, Heiko Carstens wrote:
> Since the minimum architecture level has been raised to z10 a shorter
> instruction sequence can be used to implement the OUTSIDE macro. This
> also reduces the number of used registers within that macro to one.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/kernel/entry.S | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> index a6b45eaa3450..e1664b45090f 100644
> --- a/arch/s390/kernel/entry.S
> +++ b/arch/s390/kernel/entry.S
> @@ -169,11 +169,9 @@ _LPP_OFFSET	= __LC_LPP
>  	 * @outside_label: jump here if @reg is outside of [@start..@end)
>  	 */
>  	.macro OUTSIDE reg,start,end,outside_label
> -	lgr	%r14,\reg
> -	larl	%r13,\start
> -	slgr	%r14,%r13
> -	lghi	%r13,\end - \start
> -	clgr	%r14,%r13
> +	larl	%r14,\start
> +	slgrk	%r14,\reg,%r14
> +	clgfi	%r14,\end - \start

Clever me.. slgrk was added with z196, and not z10.
So dropping this patch.

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-11 17:30   ` Nathan Chancellor
@ 2022-05-12 17:24     ` Heiko Carstens
  2022-05-12 19:06       ` Nathan Chancellor
  2022-05-16  9:07       ` Alexander Gordeev
  0 siblings, 2 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 17:24 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Wed, May 11, 2022 at 10:30:05AM -0700, Nathan Chancellor wrote:
> Hi Heiko,
> 
> On Wed, May 11, 2022 at 02:05:28PM +0200, Heiko Carstens wrote:
> > llvm's integrated assembler cannot handle immediate values which are
> > calculated with two local labels:
> > 
> > <instantiation>:3:13: error: invalid operand for instruction
> >  clgfi %r14,.Lsie_done - .Lsie_gmap
> > 
> > Workaround this by adding clang specific code which reads the specific
> > value from memory. Since this code is within the hot paths of the kernel
> > and adds an additional memory reference, keep the original code, and add
> > ifdef'ed code.
> > 
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> >  arch/s390/kernel/entry.S | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> > index e1664b45090f..ff7a75078e93 100644
> > --- a/arch/s390/kernel/entry.S
> > +++ b/arch/s390/kernel/entry.S
> > @@ -171,8 +171,19 @@ _LPP_OFFSET	= __LC_LPP
> >  	.macro OUTSIDE reg,start,end,outside_label
> >  	larl	%r14,\start
> >  	slgrk	%r14,\reg,%r14
> > +#ifdef CONFIG_CC_IS_CLANG
> 
> I intend to put this series through my build and boot test matrix later
> today but one fly by comment in the meantime. Should this be
> CONFIG_AS_IS_LLVM if this is an integrated assembler limitation, rather
> than a clang one?

Yes, that makes a lot of sense. Considering that I will drop the
previous patch within this series, the new version looks like:

From fe4fb0b014378d84ae517deaea338577b2ea6ae0 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <hca@linux.ibm.com>
Date: Sat, 7 May 2022 15:00:40 +0200
Subject: [PATCH 3/7] s390/entry: workaround llvm's IAS limitations

llvm's integrated assembler cannot handle immediate values which are
calculated with two local labels:

<instantiation>:3:13: error: invalid operand for instruction
 clgfi %r14,.Lsie_done - .Lsie_gmap

Workaround this by adding clang specific code which reads the specific
value from memory. Since this code is within the hot paths of the kernel
and adds an additional memory reference, keep the original code, and add
ifdef'ed code.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/entry.S | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index a6b45eaa3450..f2f30bfba1e9 100644
--- a/arch/s390/kernel/entry.S
+++ b/arch/s390/kernel/entry.S
@@ -172,9 +172,19 @@ _LPP_OFFSET	= __LC_LPP
 	lgr	%r14,\reg
 	larl	%r13,\start
 	slgr	%r14,%r13
-	lghi	%r13,\end - \start
-	clgr	%r14,%r13
+#ifdef CONFIG_AS_IS_LLVM
+	clgfrl	%r14,.Lrange_size\@
+#else
+	clgfi	%r14,\end - \start
+#endif
 	jhe	\outside_label
+#ifdef CONFIG_CC_IS_CLANG
+	.section .rodata, "a"
+	.align 4
+.Lrange_size\@:
+	.long	\end - \start
+	.previous
+#endif
 	.endm
 
 	.macro SIEEXIT
-- 
2.32.0

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

* Re: [PATCH 5/8] s390/purgatory: workaround llvm's IAS limitations
  2022-05-11 12:05 ` [PATCH 5/8] s390/purgatory: " Heiko Carstens
  2022-05-11 19:54   ` Nick Desaulniers
@ 2022-05-12 17:25   ` Heiko Carstens
  1 sibling, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 17:25 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Wed, May 11, 2022 at 02:05:29PM +0200, Heiko Carstens wrote:
> llvm's integrated assembler cannot handle immediate values which are
> calculated with two local labels:
> 
> arch/s390/purgatory/head.S:139:11: error: invalid operand for instruction
>  aghi %r8,-(.base_crash-purgatory_start)
> 
> Workaround this by partially rewriting the code.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/purgatory/head.S | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)
...
> -	aghi	%r10,stack-purgatory_start
> +	larl	%r0,purgatory_start
> +	slgrk	%r0,%r7,%r0
> +	agr	%r10,%r0

Same problem here: slgrk does not work for MARCH=z10. So new version
looks like:

From 99e011fa48d9fe90f6d3d58cf89f738afddf394f Mon Sep 17 00:00:00 2001
From: Heiko Carstens <hca@linux.ibm.com>
Date: Sat, 7 May 2022 20:08:55 +0200
Subject: [PATCH 4/7] s390/purgatory: workaround llvm's IAS limitations

llvm's integrated assembler cannot handle immediate values which are
calculated with two local labels:

arch/s390/purgatory/head.S:139:11: error: invalid operand for instruction
 aghi %r8,-(.base_crash-purgatory_start)

Workaround this by partially rewriting the code.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/purgatory/head.S | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/arch/s390/purgatory/head.S b/arch/s390/purgatory/head.S
index 3d1c31e0cf3d..6f835124ee82 100644
--- a/arch/s390/purgatory/head.S
+++ b/arch/s390/purgatory/head.S
@@ -44,11 +44,14 @@
 .endm
 
 .macro MEMSWAP dst,src,buf,len
-10:	cghi	\len,bufsz
+10:	larl	%r0,purgatory_end
+	larl	%r1,stack
+	slgr	%r0,%r1
+	cgr	\len,%r0
 	jh	11f
 	lgr	%r4,\len
 	j	12f
-11:	lghi	%r4,bufsz
+11:	lgr	%r4,%r0
 
 12:	MEMCPY	\buf,\dst,%r4
 	MEMCPY	\dst,\src,%r4
@@ -135,12 +138,18 @@ ENTRY(purgatory_start)
 
 .start_crash_kernel:
 	/* Location of purgatory_start in crash memory */
+	larl	%r0,.base_crash
+	larl	%r1,purgatory_start
+	slgr	%r0,%r1
 	lgr	%r8,%r13
-	aghi	%r8,-(.base_crash-purgatory_start)
+	sgr	%r8,%r0
 
 	/* Destination for this code i.e. end of memory to be swapped. */
+	larl	%r0,purgatory_end
+	larl	%r1,purgatory_start
+	slgr	%r0,%r1
 	lg	%r9,crash_size-.base_crash(%r13)
-	aghi	%r9,-(purgatory_end-purgatory_start)
+	sgr	%r9,%r0
 
 	/* Destination in crash memory, i.e. same as r9 but in crash memory. */
 	lg	%r10,crash_start-.base_crash(%r13)
@@ -149,15 +158,19 @@ ENTRY(purgatory_start)
 	/* Buffer location (in crash memory) and size. As the purgatory is
 	 * behind the point of no return it can re-use the stack as buffer.
 	 */
-	lghi	%r11,bufsz
+	larl	%r11,purgatory_end
 	larl	%r12,stack
+	slgr	%r11,%r12
 
 	MEMCPY	%r12,%r9,%r11	/* dst	-> (crash) buf */
 	MEMCPY	%r9,%r8,%r11	/* self -> dst */
 
 	/* Jump to new location. */
 	lgr	%r7,%r9
-	aghi	%r7,.jump_to_dst-purgatory_start
+	larl	%r0,.jump_to_dst
+	larl	%r1,purgatory_start
+	slgr	%r0,%r1
+	agr	%r7,%r0
 	br	%r7
 
 .jump_to_dst:
@@ -169,7 +182,10 @@ ENTRY(purgatory_start)
 
 	/* Load new buffer location after jump */
 	larl	%r7,stack
-	aghi	%r10,stack-purgatory_start
+	lgr	%r0,%r7
+	larl	%r1,purgatory_start
+	slgr	%r0,%r1
+	agr	%r10,%r0
 	MEMCPY	%r10,%r7,%r11	/* (new) buf -> (crash) buf */
 
 	/* Now the code is set up to run from its designated location. Start
-- 
2.32.0


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

* Re: [PATCH 5/8] s390/purgatory: workaround llvm's IAS limitations
  2022-05-11 19:54   ` Nick Desaulniers
@ 2022-05-12 17:26     ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 17:26 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Jonas Paulsson, Vasily Gorbik, Alexander Gordeev, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 12:54:41PM -0700, Nick Desaulniers wrote:
> On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> >
> > llvm's integrated assembler cannot handle immediate values which are
> > calculated with two local labels:
> >
> > arch/s390/purgatory/head.S:139:11: error: invalid operand for instruction
> >  aghi %r8,-(.base_crash-purgatory_start)
> 
> I thought this was fixed in
> https://github.com/ClangBuiltLinux/linux/issues/1420
> https://reviews.llvm.org/D113341
> (clang-14)
> ?

Looks like the referenced fix only works for displacements, bit not
for immediates.

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

* Re: [PATCH 7/8] s390/boot: do not emit debug info for assembly with llvm's IAS
  2022-05-11 19:40   ` Nick Desaulniers
@ 2022-05-12 17:30     ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 17:30 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 12:40:07PM -0700, Nick Desaulniers wrote:
> On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> >
> > Commit ee6d777d3e93 ("s390/decompressor: support extra debug flags")
> > added extra debug flags, in particular debug info is created,
> > depending on config options.
> >
> > With llvm's IAS this causes this compile warning:
> >
> > arch/s390/boot/head.S:38:1: warning: DWARF2 only supports one section per compilation unit
> > .section ".head.text","ax"
> > ^
> >
> > This is a known problem and was addressed with a commit b8a9092330da
> > ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1").
> > Just do the same for s390 to get rid of this warning.
> >
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> >  arch/s390/Makefile | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> > index c59efc83f020..d73611b35164 100644
> > --- a/arch/s390/Makefile
> > +++ b/arch/s390/Makefile
> > @@ -20,7 +20,9 @@ LDFLAGS_vmlinux       := -pie
> >  endif
> >  aflags_dwarf   := -Wa,-gdwarf-2
> 
> ^ or can we use a more modern variant of dwarf, like at least dwarf-4?

For now I would like that this works the same like what you introduced
with commit b8a9092330da ("Kbuild: do not emit debug info for assembly
with LLVM_IAS=1").

I wouldn't like to have dwarf-4 enabled only for this single file, and
disabled for all other asm files (assuming that dwarf-4 works).

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

* Re: [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-12 17:21   ` Heiko Carstens
@ 2022-05-12 18:00     ` Nick Desaulniers
  2022-05-12 19:15       ` Heiko Carstens
  0 siblings, 1 reply; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-12 18:00 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Thu, May 12, 2022 at 10:22 AM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> On Wed, May 11, 2022 at 02:05:27PM +0200, Heiko Carstens wrote:
> > Since the minimum architecture level has been raised to z10 a shorter
> > instruction sequence can be used to implement the OUTSIDE macro. This
> > also reduces the number of used registers within that macro to one.
> >
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> >  arch/s390/kernel/entry.S | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> > index a6b45eaa3450..e1664b45090f 100644
> > --- a/arch/s390/kernel/entry.S
> > +++ b/arch/s390/kernel/entry.S
> > @@ -169,11 +169,9 @@ _LPP_OFFSET      = __LC_LPP
> >        * @outside_label: jump here if @reg is outside of [@start..@end)
> >        */
> >       .macro OUTSIDE reg,start,end,outside_label
> > -     lgr     %r14,\reg
> > -     larl    %r13,\start
> > -     slgr    %r14,%r13
> > -     lghi    %r13,\end - \start
> > -     clgr    %r14,%r13
> > +     larl    %r14,\start
> > +     slgrk   %r14,\reg,%r14
> > +     clgfi   %r14,\end - \start
>
> Clever me.. slgrk was added with z196, and not z10.
> So dropping this patch.

How do the version numbers work for SystemZ? Is there a list/reference
you could link me to?  If it's too deep a rabbit hole, then nevermind,
but I would like to learn a little more about the architecture.
-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 0/8] s390: allow to build with llvm's integrated assembler
  2022-05-11 20:52 ` Nathan Chancellor
@ 2022-05-12 19:03   ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 19:03 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Wed, May 11, 2022 at 01:52:13PM -0700, Nathan Chancellor wrote:
> Hi Heiko,
> 
> On Wed, May 11, 2022 at 02:05:24PM +0200, Heiko Carstens wrote:
> > A couple of patches which in result make it finally possible to build the
> > kernel for s390 with llvm's integrated assembler. Several configs build
> > without errors or warnings, and the kernel also works as expected.
> > 
> > Note that patch 6 ("s390/boot: workaround llvm IAS bug") reveals a
> > miscompile. This looks like a bug in the instruction definitions of the mvc
> > and clc instructions(?). I'd like to ask people to look into this, since
> > this silently generated broken code.
> 
> I think it should be pretty simple to file a bug report for this since
> it occurs in a standalone assembly file? I agree with Nick that there
> should be a bug report filed and linked to in patch 6 so that we don't
> lose track of it.

https://github.com/llvm/llvm-project/issues/55411

> I applied this series to the latest s390 for-next branch (c4fb15578802)
> and built a few in-tree and distribution configurations with clang-14
> and clang-15 then boot tested them in QEMU with a simple buildroot
> userspace. I did not see any new warnings or errors. This is awesome, I
> am excited to get this wired up in our CI!
> 
> In case it is worthwhile:
> 
> Tested-by: Nathan Chancellor <nathan@kernel.org>

Yes, it is. Thanks a lot!

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

* Re: [PATCH 0/8] s390: allow to build with llvm's integrated assembler
  2022-05-11 19:48 ` [PATCH 0/8] s390: allow to build with llvm's integrated assembler Nick Desaulniers
@ 2022-05-12 19:04   ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 19:04 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 12:48:34PM -0700, Nick Desaulniers wrote:
> On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> >
> > A couple of patches which in result make it finally possible to build the
> > kernel for s390 with llvm's integrated assembler. Several configs build
> > without errors or warnings, and the kernel also works as expected.
> >
> > Note that patch 6 ("s390/boot: workaround llvm IAS bug") reveals a
> > miscompile. This looks like a bug in the instruction definitions of the mvc
> > and clc instructions(?). I'd like to ask people to look into this, since
> > this silently generated broken code.
> >
> > This patch series is based on linux-next, which contains two additional
> > required s390 specific patches to make llvm's IAS work.
> 
> I did a quick test of just a defconfig via:
> $ ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- make CC=clang -j72 defconfig all
> and this assembled then booted in qemu for me. Thanks for the work
> that went into this!
> 
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>

Will add this too. Thank you!

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

* Re: [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390
  2022-05-11 19:56     ` Nick Desaulniers
@ 2022-05-12 19:06       ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 19:06 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Wed, May 11, 2022 at 12:56:27PM -0700, Nick Desaulniers wrote:
> On Wed, May 11, 2022 at 12:27 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Wed, May 11, 2022 at 5:05 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> > >
> > > Before version 14.0.0 llvm's integrated assembler fails to handle some
> > > displacement variants:
> > >
> > > arch/s390/purgatory/head.S:108:10: error: invalid operand for instruction
> > >  lg %r11,kernel_type-.base_crash(%r13)
> > >
> > > Instead of working around this and given that this is already fixed
> > > raise the minimum clang version from 13.0.0 to 14.0.0.
> >
> > Do you have the commit in LLVM that fixed this? Might be nice to link
> 
> Maybe it's
> https://reviews.llvm.org/D113341?

Yes, looks like it.

> Also, these are the open issues we had for the integrated assembler.
> https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aopen+is%3Aissue+label%3A%22%5BARCH%5D+s390%22+label%3A%22%5BTOOL%5D+integrated-as%22
> 
> Any chance you could include relevant Link tags on your commit
> messages for patches that address these? It makes it easier to track
> when/where things land if we ever intend to backport anything to
> stable.

Sure, will do.

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-12 17:24     ` Heiko Carstens
@ 2022-05-12 19:06       ` Nathan Chancellor
  2022-05-12 19:16         ` Heiko Carstens
  2022-05-16  9:07       ` Alexander Gordeev
  1 sibling, 1 reply; 41+ messages in thread
From: Nathan Chancellor @ 2022-05-12 19:06 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Thu, May 12, 2022 at 07:24:25PM +0200, Heiko Carstens wrote:
> On Wed, May 11, 2022 at 10:30:05AM -0700, Nathan Chancellor wrote:
> > Hi Heiko,
> > 
> > On Wed, May 11, 2022 at 02:05:28PM +0200, Heiko Carstens wrote:
> > > llvm's integrated assembler cannot handle immediate values which are
> > > calculated with two local labels:
> > > 
> > > <instantiation>:3:13: error: invalid operand for instruction
> > >  clgfi %r14,.Lsie_done - .Lsie_gmap
> > > 
> > > Workaround this by adding clang specific code which reads the specific
> > > value from memory. Since this code is within the hot paths of the kernel
> > > and adds an additional memory reference, keep the original code, and add
> > > ifdef'ed code.
> > > 
> > > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > > ---
> > >  arch/s390/kernel/entry.S | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> > > index e1664b45090f..ff7a75078e93 100644
> > > --- a/arch/s390/kernel/entry.S
> > > +++ b/arch/s390/kernel/entry.S
> > > @@ -171,8 +171,19 @@ _LPP_OFFSET	= __LC_LPP
> > >  	.macro OUTSIDE reg,start,end,outside_label
> > >  	larl	%r14,\start
> > >  	slgrk	%r14,\reg,%r14
> > > +#ifdef CONFIG_CC_IS_CLANG
> > 
> > I intend to put this series through my build and boot test matrix later
> > today but one fly by comment in the meantime. Should this be
> > CONFIG_AS_IS_LLVM if this is an integrated assembler limitation, rather
> > than a clang one?
> 
> Yes, that makes a lot of sense. Considering that I will drop the
> previous patch within this series, the new version looks like:
> 
> From fe4fb0b014378d84ae517deaea338577b2ea6ae0 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <hca@linux.ibm.com>
> Date: Sat, 7 May 2022 15:00:40 +0200
> Subject: [PATCH 3/7] s390/entry: workaround llvm's IAS limitations
> 
> llvm's integrated assembler cannot handle immediate values which are
> calculated with two local labels:
> 
> <instantiation>:3:13: error: invalid operand for instruction
>  clgfi %r14,.Lsie_done - .Lsie_gmap
> 
> Workaround this by adding clang specific code which reads the specific
> value from memory. Since this code is within the hot paths of the kernel
> and adds an additional memory reference, keep the original code, and add
> ifdef'ed code.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/kernel/entry.S | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> index a6b45eaa3450..f2f30bfba1e9 100644
> --- a/arch/s390/kernel/entry.S
> +++ b/arch/s390/kernel/entry.S
> @@ -172,9 +172,19 @@ _LPP_OFFSET	= __LC_LPP
>  	lgr	%r14,\reg
>  	larl	%r13,\start
>  	slgr	%r14,%r13
> -	lghi	%r13,\end - \start
> -	clgr	%r14,%r13
> +#ifdef CONFIG_AS_IS_LLVM
> +	clgfrl	%r14,.Lrange_size\@
> +#else
> +	clgfi	%r14,\end - \start
> +#endif
>  	jhe	\outside_label
> +#ifdef CONFIG_CC_IS_CLANG

I think this one also wants to be CONFIG_AS_IS_LLVM, right?

Other than that, seems fine to me, although I have no knowledge of s390
assembly so that statement probably means next to nothing :)

Cheers,
Nathan

> +	.section .rodata, "a"
> +	.align 4
> +.Lrange_size\@:
> +	.long	\end - \start
> +	.previous
> +#endif
>  	.endm
>  
>  	.macro SIEEXIT
> -- 
> 2.32.0

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

* Re: [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-12 18:00     ` Nick Desaulniers
@ 2022-05-12 19:15       ` Heiko Carstens
  2022-05-12 19:25         ` Nick Desaulniers
  0 siblings, 1 reply; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 19:15 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, linux-kernel, linux-s390

On Thu, May 12, 2022 at 11:00:31AM -0700, Nick Desaulniers wrote:
> On Thu, May 12, 2022 at 10:22 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> >
> > On Wed, May 11, 2022 at 02:05:27PM +0200, Heiko Carstens wrote:
> > > Since the minimum architecture level has been raised to z10 a shorter
> > > instruction sequence can be used to implement the OUTSIDE macro. This
> > > also reduces the number of used registers within that macro to one.
> > >
> > > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > > ---
> > >  arch/s390/kernel/entry.S | 8 +++-----
> > >  1 file changed, 3 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> > > index a6b45eaa3450..e1664b45090f 100644
> > > --- a/arch/s390/kernel/entry.S
> > > +++ b/arch/s390/kernel/entry.S
> > > @@ -169,11 +169,9 @@ _LPP_OFFSET      = __LC_LPP
> > >        * @outside_label: jump here if @reg is outside of [@start..@end)
> > >        */
> > >       .macro OUTSIDE reg,start,end,outside_label
> > > -     lgr     %r14,\reg
> > > -     larl    %r13,\start
> > > -     slgr    %r14,%r13
> > > -     lghi    %r13,\end - \start
> > > -     clgr    %r14,%r13
> > > +     larl    %r14,\start
> > > +     slgrk   %r14,\reg,%r14
> > > +     clgfi   %r14,\end - \start
> >
> > Clever me.. slgrk was added with z196, and not z10.
> > So dropping this patch.
> 
> How do the version numbers work for SystemZ? Is there a list/reference
> you could link me to?  If it's too deep a rabbit hole, then nevermind,
> but I would like to learn a little more about the architecture.

If it is only for the machine generations the following links might help:

https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history
(see linked pdf for list of machine names)

https://en.wikipedia.org/wiki/IBM_Z

There might be better sources, but that's all I could find right now.

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-12 19:06       ` Nathan Chancellor
@ 2022-05-12 19:16         ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-12 19:16 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Thu, May 12, 2022 at 12:06:59PM -0700, Nathan Chancellor wrote:
> > +#ifdef CONFIG_AS_IS_LLVM
> > +	clgfrl	%r14,.Lrange_size\@
> > +#else
> > +	clgfi	%r14,\end - \start
> > +#endif
> >  	jhe	\outside_label
> > +#ifdef CONFIG_CC_IS_CLANG
> 
> I think this one also wants to be CONFIG_AS_IS_LLVM, right?

Yes, of course.

> Other than that, seems fine to me, although I have no knowledge of s390
> assembly so that statement probably means next to nothing :)

Thanks for having a look! :)

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

* Re: [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-12 19:15       ` Heiko Carstens
@ 2022-05-12 19:25         ` Nick Desaulniers
  2022-05-13 12:29           ` Heiko Carstens
  0 siblings, 1 reply; 41+ messages in thread
From: Nick Desaulniers @ 2022-05-12 19:25 UTC (permalink / raw)
  To: Heiko Carstens, Nathan Chancellor
  Cc: Vasily Gorbik, Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, linux-kernel, linux-s390

On Thu, May 12, 2022 at 12:15 PM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> On Thu, May 12, 2022 at 11:00:31AM -0700, Nick Desaulniers wrote:
> > On Thu, May 12, 2022 at 10:22 AM Heiko Carstens <hca@linux.ibm.com> wrote:
> > >
> > > On Wed, May 11, 2022 at 02:05:27PM +0200, Heiko Carstens wrote:
> > > > Since the minimum architecture level has been raised to z10 a shorter
> > > > instruction sequence can be used to implement the OUTSIDE macro. This
> > > > also reduces the number of used registers within that macro to one.
> > > >
> > > > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > > > ---
> > > >  arch/s390/kernel/entry.S | 8 +++-----
> > > >  1 file changed, 3 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> > > > index a6b45eaa3450..e1664b45090f 100644
> > > > --- a/arch/s390/kernel/entry.S
> > > > +++ b/arch/s390/kernel/entry.S
> > > > @@ -169,11 +169,9 @@ _LPP_OFFSET      = __LC_LPP
> > > >        * @outside_label: jump here if @reg is outside of [@start..@end)
> > > >        */
> > > >       .macro OUTSIDE reg,start,end,outside_label
> > > > -     lgr     %r14,\reg
> > > > -     larl    %r13,\start
> > > > -     slgr    %r14,%r13
> > > > -     lghi    %r13,\end - \start
> > > > -     clgr    %r14,%r13
> > > > +     larl    %r14,\start
> > > > +     slgrk   %r14,\reg,%r14
> > > > +     clgfi   %r14,\end - \start
> > >
> > > Clever me.. slgrk was added with z196, and not z10.
> > > So dropping this patch.
> >
> > How do the version numbers work for SystemZ? Is there a list/reference
> > you could link me to?  If it's too deep a rabbit hole, then nevermind,
> > but I would like to learn a little more about the architecture.
>
> If it is only for the machine generations the following links might help:
>
> https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history
> (see linked pdf for list of machine names)
>
> https://en.wikipedia.org/wiki/IBM_Z
>
> There might be better sources, but that's all I could find right now.

Interesting! Thanks for the links.
I'm guessing in our CI that we should probably pursue testing some of
the newer revisions. Wasn't defconfig updated from z10 to z12 not too
long ago?
So probably
CONFIG_MARCH_Z13
CONFIG_MARCH_Z14
CONFIG_MARCH_Z15
CONFIG_MARCH_Z16

All look like they're still "supported" (and I'm guessing
CONFIG_MARCH_Z10 and CONFIG_MARCH_Z196 are not too much burden to
continue to maintain kernel support for), with a higher emphasis
perhaps on z15+z16?
-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 1/8] s390/alternatives: provide identical sized orginal/alternative sequences
  2022-05-11 12:05 ` [PATCH 1/8] s390/alternatives: provide identical sized orginal/alternative sequences Heiko Carstens
@ 2022-05-13  9:17   ` Vasily Gorbik
  0 siblings, 0 replies; 41+ messages in thread
From: Vasily Gorbik @ 2022-05-13  9:17 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Wed, May 11, 2022 at 02:05:25PM +0200, Heiko Carstens wrote:
> Explicitly provide identical sized original/alternative instruction
> sequences. This way there is no need for the s390 specific alternatives
> infrastructure to generate padding sequences.
> The code which generates such sequences will be removed with a follow on
> patch.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/include/asm/spinlock.h |  2 +-
>  arch/s390/kernel/entry.S         | 20 ++++++++++----------
>  arch/s390/lib/spinlock.c         |  4 ++--
>  3 files changed, 13 insertions(+), 13 deletions(-)

Acked-by: Vasily Gorbik <gor@linux.ibm.com>

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

* Re: [PATCH 2/8] s390/alternatives: remove padding generation code
  2022-05-11 12:05 ` [PATCH 2/8] s390/alternatives: remove padding generation code Heiko Carstens
  2022-05-11 17:02   ` Heiko Carstens
@ 2022-05-13  9:18   ` Vasily Gorbik
  1 sibling, 0 replies; 41+ messages in thread
From: Vasily Gorbik @ 2022-05-13  9:18 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Alexander Gordeev, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Wed, May 11, 2022 at 02:05:26PM +0200, Heiko Carstens wrote:
> clang fails to handle ".if" statements in inline assembly which are heavily
> used in the alternatives code.
> 
> To work around this remove this code, and enforce that users of
> alternatives must specify original and alternative instruction sequences
> which have identical sizes. Add a compile time check with two ".org"
> statements similar to arm64.
> 
> In result not only clang can handle this, but also quite a lot of code can
> be removed.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/include/asm/alternative-asm.h | 76 +++-----------------
>  arch/s390/include/asm/alternative.h     | 93 ++++++-------------------
>  arch/s390/kernel/alternative.c          | 61 +---------------
>  3 files changed, 31 insertions(+), 199 deletions(-)

Acked-by: Vasily Gorbik <gor@linux.ibm.com>

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

* Re: [PATCH 3/8] s390/entry: shorten OUTSIDE macro
  2022-05-12 19:25         ` Nick Desaulniers
@ 2022-05-13 12:29           ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-13 12:29 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Vasily Gorbik, Alexander Gordeev,
	Jonas Paulsson, Ulrich Weigand, Masahiro Yamada,
	Alexander Egorenkov, Sven Schnelle, Andreas Krebbel,
	linux-kernel, linux-s390

On Thu, May 12, 2022 at 12:25:01PM -0700, Nick Desaulniers wrote:
> Interesting! Thanks for the links.
> I'm guessing in our CI that we should probably pursue testing some of
> the newer revisions. Wasn't defconfig updated from z10 to z12 not too
> long ago?

Yes, that was 1.5 years ago (commit ac94a2911e84 ("s390: update defconfigs")).

> So probably
> CONFIG_MARCH_Z13
> CONFIG_MARCH_Z14
> CONFIG_MARCH_Z15
> CONFIG_MARCH_Z16
> 
> All look like they're still "supported" (and I'm guessing
> CONFIG_MARCH_Z10 and CONFIG_MARCH_Z196 are not too much burden to
> continue to maintain kernel support for), with a higher emphasis
> perhaps on z15+z16?

That makes sense. However for the kernel it doesn't make a difference
if compiled for z15 or z16 - there were no general instructions added
with z16 that would make any difference for the kernel.
All new instructions that came with z16 are either only relevant for
user space, or used via .insn notation in the kernel, and only used if
some hardware feature is present.

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-12 17:24     ` Heiko Carstens
  2022-05-12 19:06       ` Nathan Chancellor
@ 2022-05-16  9:07       ` Alexander Gordeev
  2022-05-16 10:19         ` Heiko Carstens
  1 sibling, 1 reply; 41+ messages in thread
From: Alexander Gordeev @ 2022-05-16  9:07 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Nathan Chancellor, Vasily Gorbik, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Thu, May 12, 2022 at 07:24:25PM +0200, Heiko Carstens wrote:
> From fe4fb0b014378d84ae517deaea338577b2ea6ae0 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <hca@linux.ibm.com>
> Date: Sat, 7 May 2022 15:00:40 +0200
> Subject: [PATCH 3/7] s390/entry: workaround llvm's IAS limitations
> 
> llvm's integrated assembler cannot handle immediate values which are
> calculated with two local labels:
> 
> <instantiation>:3:13: error: invalid operand for instruction
>  clgfi %r14,.Lsie_done - .Lsie_gmap
> 
> Workaround this by adding clang specific code which reads the specific
> value from memory. Since this code is within the hot paths of the kernel
> and adds an additional memory reference, keep the original code, and add
> ifdef'ed code.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/kernel/entry.S | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> index a6b45eaa3450..f2f30bfba1e9 100644
> --- a/arch/s390/kernel/entry.S
> +++ b/arch/s390/kernel/entry.S
> @@ -172,9 +172,19 @@ _LPP_OFFSET	= __LC_LPP
>  	lgr	%r14,\reg
>  	larl	%r13,\start
>  	slgr	%r14,%r13
> -	lghi	%r13,\end - \start
> -	clgr	%r14,%r13
> +#ifdef CONFIG_AS_IS_LLVM
> +	clgfrl	%r14,.Lrange_size\@




> +#else
> +	clgfi	%r14,\end - \start
> +#endif
>  	jhe	\outside_label
> +#ifdef CONFIG_CC_IS_CLANG
> +	.section .rodata, "a"
> +	.align 4
> +.Lrange_size\@:
> +	.long	\end - \start

Isn't the machine check handler refers to this memory before checking
unrecoverable storage errors (with CHKSTG macro) as result of this change?

> +	.previous
> +#endif
>  	.endm
>  
>  	.macro SIEEXIT
> -- 
> 2.32.0

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-16  9:07       ` Alexander Gordeev
@ 2022-05-16 10:19         ` Heiko Carstens
  2022-05-16 10:42           ` Jonas Paulsson
  2022-05-16 11:11           ` Alexander Gordeev
  0 siblings, 2 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-16 10:19 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: Nathan Chancellor, Vasily Gorbik, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Mon, May 16, 2022 at 11:07:43AM +0200, Alexander Gordeev wrote:
> > diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> > index a6b45eaa3450..f2f30bfba1e9 100644
> > --- a/arch/s390/kernel/entry.S
> > +++ b/arch/s390/kernel/entry.S
> > @@ -172,9 +172,19 @@ _LPP_OFFSET	= __LC_LPP
> >  	lgr	%r14,\reg
> >  	larl	%r13,\start
> >  	slgr	%r14,%r13
> > -	lghi	%r13,\end - \start
> > -	clgr	%r14,%r13
> > +#ifdef CONFIG_AS_IS_LLVM
> > +	clgfrl	%r14,.Lrange_size\@
> > +#else
> > +	clgfi	%r14,\end - \start
> > +#endif
> >  	jhe	\outside_label
> > +#ifdef CONFIG_CC_IS_CLANG
> > +	.section .rodata, "a"
> > +	.align 4
> > +.Lrange_size\@:
> > +	.long	\end - \start
> 
> Isn't the machine check handler refers to this memory before checking
> unrecoverable storage errors (with CHKSTG macro) as result of this change?

Yes, indeed. However implementing this without another register will
be quite of a challenge. So what I would prefer in any case: just
assume that this minimal set of memory accesses work. Actually I'd
seriously like to go a bit further, and even move the checks for
storage errors back to C for two reasons:

- this would make the machine check handler entry code easier again
- it would also allow to enter the machine check handler with DAT on

After all we rely anyway on the fact that at least the local lowcore +
the page(s) which contain text are still accessible. Assuming that a
couple of page tables also work won't make this much worse, but the
code much easier.

So I'd suggest: leave this code as is, and at some later point move
"rework" the early machine check handler code.

What do you think?

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-16 10:19         ` Heiko Carstens
@ 2022-05-16 10:42           ` Jonas Paulsson
  2022-05-16 11:11           ` Alexander Gordeev
  1 sibling, 0 replies; 41+ messages in thread
From: Jonas Paulsson @ 2022-05-16 10:42 UTC (permalink / raw)
  To: Heiko Carstens, Alexander Gordeev
  Cc: Nathan Chancellor, Vasily Gorbik, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

I will try to get a patch for clang ready soon... /Jonas

On 2022-05-16 12:19 em, Heiko Carstens wrote:
> On Mon, May 16, 2022 at 11:07:43AM +0200, Alexander Gordeev wrote:
>>> diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
>>> index a6b45eaa3450..f2f30bfba1e9 100644
>>> --- a/arch/s390/kernel/entry.S
>>> +++ b/arch/s390/kernel/entry.S
>>> @@ -172,9 +172,19 @@ _LPP_OFFSET	= __LC_LPP
>>>   	lgr	%r14,\reg
>>>   	larl	%r13,\start
>>>   	slgr	%r14,%r13
>>> -	lghi	%r13,\end - \start
>>> -	clgr	%r14,%r13
>>> +#ifdef CONFIG_AS_IS_LLVM
>>> +	clgfrl	%r14,.Lrange_size\@
>>> +#else
>>> +	clgfi	%r14,\end - \start
>>> +#endif
>>>   	jhe	\outside_label
>>> +#ifdef CONFIG_CC_IS_CLANG
>>> +	.section .rodata, "a"
>>> +	.align 4
>>> +.Lrange_size\@:
>>> +	.long	\end - \start
>> Isn't the machine check handler refers to this memory before checking
>> unrecoverable storage errors (with CHKSTG macro) as result of this change?
> Yes, indeed. However implementing this without another register will
> be quite of a challenge. So what I would prefer in any case: just
> assume that this minimal set of memory accesses work. Actually I'd
> seriously like to go a bit further, and even move the checks for
> storage errors back to C for two reasons:
>
> - this would make the machine check handler entry code easier again
> - it would also allow to enter the machine check handler with DAT on
>
> After all we rely anyway on the fact that at least the local lowcore +
> the page(s) which contain text are still accessible. Assuming that a
> couple of page tables also work won't make this much worse, but the
> code much easier.
>
> So I'd suggest: leave this code as is, and at some later point move
> "rework" the early machine check handler code.
>
> What do you think?

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-16 10:19         ` Heiko Carstens
  2022-05-16 10:42           ` Jonas Paulsson
@ 2022-05-16 11:11           ` Alexander Gordeev
  2022-05-16 14:05             ` Heiko Carstens
  1 sibling, 1 reply; 41+ messages in thread
From: Alexander Gordeev @ 2022-05-16 11:11 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Nathan Chancellor, Vasily Gorbik, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Mon, May 16, 2022 at 12:19:45PM +0200, Heiko Carstens wrote:
> On Mon, May 16, 2022 at 11:07:43AM +0200, Alexander Gordeev wrote:
> > Isn't the machine check handler refers to this memory before checking
> > unrecoverable storage errors (with CHKSTG macro) as result of this change?
> 
> Yes, indeed. However implementing this without another register will
> be quite of a challenge. So what I would prefer in any case: just
> assume that this minimal set of memory accesses work. Actually I'd
> seriously like to go a bit further, and even move the checks for
> storage errors back to C for two reasons:
> 
> - this would make the machine check handler entry code easier again
> - it would also allow to enter the machine check handler with DAT on
> 
> After all we rely anyway on the fact that at least the local lowcore +
> the page(s) which contain text are still accessible. Assuming that a
> couple of page tables also work won't make this much worse, but the
> code much easier.
> 
> So I'd suggest: leave this code as is, and at some later point move
> "rework" the early machine check handler code.
> 
> What do you think?

Sounds very reasonable. Please, find my:

Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>


Also, how such a follow-up looks to you?

	lgr	%r14,\reg
#ifdef CONFIG_AS_IS_LLVM
	larl	%r13,\start
	slgr	%r14,%r13
	clgfrl	%r14,.Lrange_size\@
#else
	slgfi	%r14,\start
	clgfi	%r14,\end - \start
#endif

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

* Re: [PATCH 4/8] s390/entry: workaround llvm's IAS limitations
  2022-05-16 11:11           ` Alexander Gordeev
@ 2022-05-16 14:05             ` Heiko Carstens
  0 siblings, 0 replies; 41+ messages in thread
From: Heiko Carstens @ 2022-05-16 14:05 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: Nathan Chancellor, Vasily Gorbik, Jonas Paulsson, Ulrich Weigand,
	Masahiro Yamada, Alexander Egorenkov, Sven Schnelle,
	Andreas Krebbel, Nathan Chancellor, Nick Desaulniers,
	linux-kernel, linux-s390

On Mon, May 16, 2022 at 01:11:48PM +0200, Alexander Gordeev wrote:
> > So I'd suggest: leave this code as is, and at some later point move
> > "rework" the early machine check handler code.
> > 
> > What do you think?
> 
> Sounds very reasonable. Please, find my:
> 
> Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>

Thanks!

> Also, how such a follow-up looks to you?
...
> 	slgfi	%r14,\start
> 	clgfi	%r14,\end - \start

I think using an address as an immediate value is a step in the wrong
direction, since I'd like to have all code pc-relative. And as far as
I can tell this new construct would only work as long as \start has an
absolute address that is low enough so that it would work / fit with
slgfi.
Of course this will likely always be the case, but I still think this
is not the way we should go.

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

end of thread, other threads:[~2022-05-16 14:06 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-11 12:05 [PATCH 0/8] s390: allow to build with llvm's integrated assembler Heiko Carstens
2022-05-11 12:05 ` [PATCH 1/8] s390/alternatives: provide identical sized orginal/alternative sequences Heiko Carstens
2022-05-13  9:17   ` Vasily Gorbik
2022-05-11 12:05 ` [PATCH 2/8] s390/alternatives: remove padding generation code Heiko Carstens
2022-05-11 17:02   ` Heiko Carstens
2022-05-13  9:18   ` Vasily Gorbik
2022-05-11 12:05 ` [PATCH 3/8] s390/entry: shorten OUTSIDE macro Heiko Carstens
2022-05-12 15:59   ` kernel test robot
2022-05-12 16:20   ` kernel test robot
2022-05-12 17:21   ` Heiko Carstens
2022-05-12 18:00     ` Nick Desaulniers
2022-05-12 19:15       ` Heiko Carstens
2022-05-12 19:25         ` Nick Desaulniers
2022-05-13 12:29           ` Heiko Carstens
2022-05-11 12:05 ` [PATCH 4/8] s390/entry: workaround llvm's IAS limitations Heiko Carstens
2022-05-11 17:30   ` Nathan Chancellor
2022-05-12 17:24     ` Heiko Carstens
2022-05-12 19:06       ` Nathan Chancellor
2022-05-12 19:16         ` Heiko Carstens
2022-05-16  9:07       ` Alexander Gordeev
2022-05-16 10:19         ` Heiko Carstens
2022-05-16 10:42           ` Jonas Paulsson
2022-05-16 11:11           ` Alexander Gordeev
2022-05-16 14:05             ` Heiko Carstens
2022-05-11 12:05 ` [PATCH 5/8] s390/purgatory: " Heiko Carstens
2022-05-11 19:54   ` Nick Desaulniers
2022-05-12 17:26     ` Heiko Carstens
2022-05-12 17:25   ` Heiko Carstens
2022-05-11 12:05 ` [PATCH 6/8] s390/boot: workaround llvm IAS bug Heiko Carstens
2022-05-11 19:50   ` Nick Desaulniers
2022-05-11 12:05 ` [PATCH 7/8] s390/boot: do not emit debug info for assembly with llvm's IAS Heiko Carstens
2022-05-11 19:40   ` Nick Desaulniers
2022-05-12 17:30     ` Heiko Carstens
2022-05-11 12:05 ` [PATCH 8/8] scripts/min-tool-version.sh: raise minimum clang version to 14.0.0 for s390 Heiko Carstens
2022-05-11 19:27   ` Nick Desaulniers
2022-05-11 19:56     ` Nick Desaulniers
2022-05-12 19:06       ` Heiko Carstens
2022-05-11 19:48 ` [PATCH 0/8] s390: allow to build with llvm's integrated assembler Nick Desaulniers
2022-05-12 19:04   ` Heiko Carstens
2022-05-11 20:52 ` Nathan Chancellor
2022-05-12 19:03   ` Heiko Carstens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).