linux-s390.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes
@ 2023-01-12 15:45 Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 1/7] s390x: Cleanup flat.lds Janosch Frank
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

A small set of fixes mostly related to the snippet support and error
management.

v2:
	* Added basic linker script cleanups
	  * Changed alignment from 64k to 4k
	  * Added more comments
	  * Removed unneeded sections
	* This patch set is a also preparation for a larger loader
          script change by Marc

Janosch Frank (7):
  s390x: Cleanup flat.lds
  s390x: snippets: c: Cleanup flat.lds
  s390x: Add a linker script to assembly snippets
  s390x: snippets: Fix SET_PSW_NEW_ADDR macro
  lib: s390x: sie: Set guest memory pointer
  s390x: Clear first stack frame and end backtrace early
  lib: s390x: Handle debug prints for SIE exceptions correctly

 lib/s390x/interrupt.c              | 46 ++++++++++++++++++++++++++----
 lib/s390x/sie.c                    |  1 +
 lib/s390x/sie.h                    |  2 ++
 lib/s390x/snippet.h                |  3 +-
 lib/s390x/stack.c                  |  2 ++
 s390x/Makefile                     |  5 ++--
 s390x/cpu.S                        |  6 ++--
 s390x/cstart64.S                   |  2 ++
 s390x/flat.lds                     | 19 +++---------
 s390x/mvpg-sie.c                   |  2 +-
 s390x/pv-diags.c                   |  6 ++--
 s390x/snippets/{c => asm}/flat.lds | 39 +++++++++----------------
 s390x/snippets/asm/macros.S        |  2 +-
 s390x/snippets/c/flat.lds          | 28 ++++++------------
 14 files changed, 87 insertions(+), 76 deletions(-)
 copy s390x/snippets/{c => asm}/flat.lds (55%)

-- 
2.34.1


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

* [kvm-unit-tests PATCH v2 1/7] s390x: Cleanup flat.lds
  2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
@ 2023-01-12 15:45 ` Janosch Frank
  2023-01-12 17:05   ` Claudio Imbrenda
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 2/7] s390x: snippets: c: " Janosch Frank
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

It seems like the loader file was copied over from another
architecture which has a different page size (64K) than s390 (4K).

Let's use a 4k alignment instead of the 64k one and remove unneeded
entries.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/flat.lds | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/s390x/flat.lds b/s390x/flat.lds
index de9da1a8..952f6cd4 100644
--- a/s390x/flat.lds
+++ b/s390x/flat.lds
@@ -24,20 +24,8 @@ SECTIONS
 		*(.text)
 		*(.text.*)
 	}
-	. = ALIGN(64K);
+	. = ALIGN(4K);
 	etext = .;
-	.opd : { *(.opd) }
-	. = ALIGN(16);
-	.dynamic : {
-		dynamic_start = .;
-		*(.dynamic)
-	}
-	.dynsym : {
-		dynsym_start = .;
-		*(.dynsym)
-	}
-	.rela.dyn : { *(.rela*) }
-	. = ALIGN(16);
 	.data : {
 		*(.data)
 		*(.data.rel*)
@@ -48,10 +36,11 @@ SECTIONS
 	__bss_start = .;
 	.bss : { *(.bss) }
 	__bss_end = .;
-	. = ALIGN(64K);
+	. = ALIGN(4K);
 	edata = .;
+	/* Reserve 64K for the stack */
 	. += 64K;
-	. = ALIGN(64K);
+	. = ALIGN(4K);
 	/*
 	 * stackptr set with initial stack frame preallocated
 	 */
-- 
2.34.1


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

* [kvm-unit-tests PATCH v2 2/7] s390x: snippets: c: Cleanup flat.lds
  2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 1/7] s390x: Cleanup flat.lds Janosch Frank
@ 2023-01-12 15:45 ` Janosch Frank
  2023-01-12 17:05   ` Claudio Imbrenda
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 3/7] s390x: Add a linker script to assembly snippets Janosch Frank
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

There are a lot of things in there which we don't need for snippets
and the alignments can be switched from 64K to 4K since that's the
s390 page size.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/snippets/c/flat.lds | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/s390x/snippets/c/flat.lds b/s390x/snippets/c/flat.lds
index 260ab1c4..9e5eb66b 100644
--- a/s390x/snippets/c/flat.lds
+++ b/s390x/snippets/c/flat.lds
@@ -16,27 +16,22 @@ SECTIONS
 		 QUAD(0x0000000000004000)
 	}
 	. = 0x4000;
+	/*
+	 * The stack grows down from 0x4000 to 0x2000, we pre-allocoate
+	 * a frame via the -160.
+	 */
 	stackptr = . - 160;
 	stacktop = .;
+	/* Start text 0x4000 */
 	.text : {
 		*(.init)
 		*(.text)
 		*(.text.*)
 	}
-	. = ALIGN(64K);
+	. = ALIGN(4K);
 	etext = .;
-	.opd : { *(.opd) }
-	. = ALIGN(16);
-	.dynamic : {
-		dynamic_start = .;
-		*(.dynamic)
-	}
-	.dynsym : {
-		dynsym_start = .;
-		*(.dynsym)
-	}
-	.rela.dyn : { *(.rela*) }
-	. = ALIGN(16);
+	/* End text */
+	/* Start data */
 	.data : {
 		*(.data)
 		*(.data.rel*)
@@ -44,11 +39,6 @@ SECTIONS
 	. = ALIGN(16);
 	.rodata : { *(.rodata) *(.rodata.*) }
 	. = ALIGN(16);
-	__bss_start = .;
 	.bss : { *(.bss) }
-	__bss_end = .;
-	. = ALIGN(64K);
-	edata = .;
-	. += 64K;
-	. = ALIGN(64K);
+	/* End data */
 }
-- 
2.34.1


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

* [kvm-unit-tests PATCH v2 3/7] s390x: Add a linker script to assembly snippets
  2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 1/7] s390x: Cleanup flat.lds Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 2/7] s390x: snippets: c: " Janosch Frank
@ 2023-01-12 15:45 ` Janosch Frank
  2023-01-12 17:05   ` Claudio Imbrenda
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 4/7] s390x: snippets: Fix SET_PSW_NEW_ADDR macro Janosch Frank
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

A linker script has a few benefits:
- Random data doesn't end up in the binary breaking tests
- We can easily define a lowcore and load the snippet from 0x0 instead
of 0x4000 which makes asm snippets behave like c snippets
- We can easily define an invalid PGM new PSW to ensure an exit on a
guest PGM

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/snippet.h         |  3 +--
 s390x/Makefile              |  5 +++--
 s390x/mvpg-sie.c            |  2 +-
 s390x/pv-diags.c            |  6 +++---
 s390x/snippets/asm/flat.lds | 41 +++++++++++++++++++++++++++++++++++++
 5 files changed, 49 insertions(+), 8 deletions(-)
 create mode 100644 s390x/snippets/asm/flat.lds

diff --git a/lib/s390x/snippet.h b/lib/s390x/snippet.h
index b17b2a4c..57045994 100644
--- a/lib/s390x/snippet.h
+++ b/lib/s390x/snippet.h
@@ -32,8 +32,7 @@
 
 #define SNIPPET_PV_TWEAK0	0x42UL
 #define SNIPPET_PV_TWEAK1	0UL
-#define SNIPPET_OFF_C		0
-#define SNIPPET_OFF_ASM		0x4000
+#define SNIPPET_UNPACK_OFF	0
 
 
 /*
diff --git a/s390x/Makefile b/s390x/Makefile
index 52a9d821..97a61611 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -136,7 +136,8 @@ $(SNIPPET_DIR)/c/%.o: $(SNIPPET_DIR)/c/%.c $(asm-offsets)
 	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
 
 $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o
-	$(OBJCOPY) -O binary -j ".rodata" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $(patsubst %.gbin,%.o,$@) $@
+	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/snippets/asm/flat.lds $<
+	$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
 	truncate -s '%4096' $@
 
 $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS)
@@ -145,7 +146,7 @@ $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS)
 	truncate -s '%4096' $@
 
 $(SNIPPET_DIR)/asm/%.hdr: $(SNIPPET_DIR)/asm/%.gbin $(HOST_KEY_DOCUMENT)
-	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x4000,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
+	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x0,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
 
 $(SNIPPET_DIR)/c/%.hdr: $(SNIPPET_DIR)/c/%.gbin $(HOST_KEY_DOCUMENT)
 	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x0,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
index 46a2edb6..99f4859b 100644
--- a/s390x/mvpg-sie.c
+++ b/s390x/mvpg-sie.c
@@ -87,7 +87,7 @@ static void setup_guest(void)
 
 	snippet_setup_guest(&vm, false);
 	snippet_init(&vm, SNIPPET_NAME_START(c, mvpg_snippet),
-		     SNIPPET_LEN(c, mvpg_snippet), SNIPPET_OFF_C);
+		     SNIPPET_LEN(c, mvpg_snippet), SNIPPET_UNPACK_OFF);
 
 	/* Enable MVPG interpretation as we want to test KVM and not ourselves */
 	vm.sblk->eca = ECA_MVPGI;
diff --git a/s390x/pv-diags.c b/s390x/pv-diags.c
index 9ced68c7..5165937a 100644
--- a/s390x/pv-diags.c
+++ b/s390x/pv-diags.c
@@ -28,7 +28,7 @@ static void test_diag_500(void)
 
 	snippet_pv_init(&vm, SNIPPET_NAME_START(asm, snippet_pv_diag_500),
 			SNIPPET_HDR_START(asm, snippet_pv_diag_500),
-			size_gbin, size_hdr, SNIPPET_OFF_ASM);
+			size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
 
 	sie(&vm);
 	report(vm.sblk->icptcode == ICPT_PV_INSTR && vm.sblk->ipa == 0x8302 &&
@@ -83,7 +83,7 @@ static void test_diag_288(void)
 
 	snippet_pv_init(&vm, SNIPPET_NAME_START(asm, snippet_pv_diag_288),
 			SNIPPET_HDR_START(asm, snippet_pv_diag_288),
-			size_gbin, size_hdr, SNIPPET_OFF_ASM);
+			size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
 
 	sie(&vm);
 	report(vm.sblk->icptcode == ICPT_PV_INSTR && vm.sblk->ipa == 0x8302 &&
@@ -124,7 +124,7 @@ static void test_diag_yield(void)
 
 	snippet_pv_init(&vm, SNIPPET_NAME_START(asm, snippet_pv_diag_yield),
 			SNIPPET_HDR_START(asm, snippet_pv_diag_yield),
-			size_gbin, size_hdr, SNIPPET_OFF_ASM);
+			size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
 
 	/* 0x44 */
 	report_prefix_push("0x44");
diff --git a/s390x/snippets/asm/flat.lds b/s390x/snippets/asm/flat.lds
new file mode 100644
index 00000000..ab0031ac
--- /dev/null
+++ b/s390x/snippets/asm/flat.lds
@@ -0,0 +1,41 @@
+SECTIONS
+{
+	.lowcore : {
+		/*
+		 * Initial short psw for disk boot, with 31 bit addressing for
+		 * non z/Arch environment compatibility and the instruction
+		 * address 0x4000.
+		 */
+		. = 0;
+		 LONG(0x00080000)
+		 LONG(0x80004000)
+		 /* Restart new PSW for booting via PSW restart. */
+		 . = 0x1a0;
+		 QUAD(0x0000000180000000)
+		 QUAD(0x0000000000004000)
+		 /*
+		  * Invalid PGM new PSW so we hopefully get a code 8
+		  * intercept on a PGM
+		  */
+		 . = 0x1d0;
+		 QUAD(0x0008000000000000)
+		 QUAD(0x0000000000000001)
+	}
+	. = 0x4000;
+	/* Start text 0x4000 */
+	.text : {
+		*(.text)
+		*(.text.*)
+	}
+	. = ALIGN(16);
+	etext = .;
+	/* End text */
+	/* Start data */
+	.data : {
+		*(.data)
+		*(.data.rel*)
+	}
+	. = ALIGN(16);
+	.rodata : { *(.rodata) *(.rodata.*) }
+	/* End data */
+}
-- 
2.34.1


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

* [kvm-unit-tests PATCH v2 4/7] s390x: snippets: Fix SET_PSW_NEW_ADDR macro
  2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
                   ` (2 preceding siblings ...)
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 3/7] s390x: Add a linker script to assembly snippets Janosch Frank
@ 2023-01-12 15:45 ` Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 5/7] lib: s390x: sie: Set guest memory pointer Janosch Frank
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

Let's store the psw mask instead of the address of the location where we
should load the mask from.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 s390x/snippets/asm/macros.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/snippets/asm/macros.S b/s390x/snippets/asm/macros.S
index 667fb6dc..09d7f5be 100644
--- a/s390x/snippets/asm/macros.S
+++ b/s390x/snippets/asm/macros.S
@@ -18,7 +18,7 @@
  */
 .macro SET_PSW_NEW_ADDR reg, psw_new_addr, addr_psw
 larl	\reg, psw_mask_64
-stg	\reg, \addr_psw
+mvc	\addr_psw(8,%r0), 0(\reg)
 larl	\reg, \psw_new_addr
 stg	\reg, \addr_psw + 8
 .endm
-- 
2.34.1


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

* [kvm-unit-tests PATCH v2 5/7] lib: s390x: sie: Set guest memory pointer
  2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
                   ` (3 preceding siblings ...)
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 4/7] s390x: snippets: Fix SET_PSW_NEW_ADDR macro Janosch Frank
@ 2023-01-12 15:45 ` Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 6/7] s390x: Clear first stack frame and end backtrace early Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 7/7] lib: s390x: Handle debug prints for SIE exceptions correctly Janosch Frank
  6 siblings, 0 replies; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

Seems like it was introduced but never set. It's nicer to have a
pointer than to cast the MSO of a VM.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/sie.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
index a71985b6..9241b4b4 100644
--- a/lib/s390x/sie.c
+++ b/lib/s390x/sie.c
@@ -93,6 +93,7 @@ void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len)
 
 	/* Guest memory chunks are always 1MB */
 	assert(!(guest_mem_len & ~HPAGE_MASK));
+	vm->guest_mem = (uint8_t *)guest_mem;
 	/* For non-PV guests we re-use the host's ASCE for ease of use */
 	vm->save_area.guest.asce = stctg(1);
 	/* Currently MSO/MSL is the easiest option */
-- 
2.34.1


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

* [kvm-unit-tests PATCH v2 6/7] s390x: Clear first stack frame and end backtrace early
  2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
                   ` (4 preceding siblings ...)
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 5/7] lib: s390x: sie: Set guest memory pointer Janosch Frank
@ 2023-01-12 15:45 ` Janosch Frank
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 7/7] lib: s390x: Handle debug prints for SIE exceptions correctly Janosch Frank
  6 siblings, 0 replies; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

When setting the first stack frame to 0, we can check for a 0
backchain pointer when doing backtraces to know when to stop.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/stack.c | 2 ++
 s390x/cstart64.S  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/s390x/stack.c b/lib/s390x/stack.c
index e714e07c..9f234a12 100644
--- a/lib/s390x/stack.c
+++ b/lib/s390x/stack.c
@@ -22,6 +22,8 @@ int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
 	for (depth = 0; stack && depth < max_depth; depth++) {
 		return_addrs[depth] = (void *)stack->grs[8];
 		stack = stack->back_chain;
+		if (!stack)
+			break;
 	}
 
 	return depth;
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 666a9567..6f83da2a 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -37,6 +37,8 @@ start:
 	sam64				# Set addressing mode to 64 bit
 	/* setup stack */
 	larl	%r15, stackptr
+	/* Clear first stack frame */
+	xc      0(160,%r15), 0(%r15)
 	/* setup initial PSW mask + control registers*/
 	larl	%r1, initial_psw
 	lpswe	0(%r1)
-- 
2.34.1


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

* [kvm-unit-tests PATCH v2 7/7] lib: s390x: Handle debug prints for SIE exceptions correctly
  2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
                   ` (5 preceding siblings ...)
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 6/7] s390x: Clear first stack frame and end backtrace early Janosch Frank
@ 2023-01-12 15:45 ` Janosch Frank
  2023-01-12 17:13   ` Claudio Imbrenda
  6 siblings, 1 reply; 13+ messages in thread
From: Janosch Frank @ 2023-01-12 15:45 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb

When we leave SIE due to an exception, we'll still have guest values
in registers 0 - 13 and that's not clearly portraied in our debug
prints. So let's fix that.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/interrupt.c | 46 ++++++++++++++++++++++++++++++++++++++-----
 lib/s390x/sie.h       |  2 ++
 s390x/cpu.S           |  6 ++++--
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index dadb7415..ff47c2c2 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -9,6 +9,7 @@
  */
 #include <libcflat.h>
 #include <asm/barrier.h>
+#include <asm/asm-offsets.h>
 #include <sclp.h>
 #include <interrupt.h>
 #include <sie.h>
@@ -188,9 +189,12 @@ static void print_storage_exception_information(void)
 	}
 }
 
-static void print_int_regs(struct stack_frame_int *stack)
+static void print_int_regs(struct stack_frame_int *stack, bool sie)
 {
+	struct kvm_s390_sie_block *sblk;
+
 	printf("\n");
+	printf("%s\n", sie ? "Guest registers:" : "Host registers:");
 	printf("GPRS:\n");
 	printf("%016lx %016lx %016lx %016lx\n",
 	       stack->grs1[0], stack->grs1[1], stack->grs0[0], stack->grs0[1]);
@@ -198,24 +202,56 @@ static void print_int_regs(struct stack_frame_int *stack)
 	       stack->grs0[2], stack->grs0[3], stack->grs0[4], stack->grs0[5]);
 	printf("%016lx %016lx %016lx %016lx\n",
 	       stack->grs0[6], stack->grs0[7], stack->grs0[8], stack->grs0[9]);
-	printf("%016lx %016lx %016lx %016lx\n",
-	       stack->grs0[10], stack->grs0[11], stack->grs0[12], stack->grs0[13]);
+
+	if (sie) {
+		sblk = (struct kvm_s390_sie_block *)stack->grs0[12];
+		printf("%016lx %016lx %016lx %016lx\n",
+		       stack->grs0[10], stack->grs0[11], sblk->gg14, sblk->gg15);
+	} else {
+		printf("%016lx %016lx %016lx %016lx\n",
+		       stack->grs0[10], stack->grs0[11], stack->grs0[12], stack->grs0[13]);
+	}
+
 	printf("\n");
 }
 
 static void print_pgm_info(struct stack_frame_int *stack)
 
 {
-	bool in_sie;
+	bool in_sie, in_sie_gregs;
+	struct vm_save_area *vregs;
 
 	in_sie = (lowcore.pgm_old_psw.addr >= (uintptr_t)sie_entry &&
 		  lowcore.pgm_old_psw.addr <= (uintptr_t)sie_exit);
+	in_sie_gregs = (lowcore.pgm_old_psw.addr >= (uintptr_t)sie_entry_gregs &&
+			lowcore.pgm_old_psw.addr <= (uintptr_t)sie_exit_gregs);
 
 	printf("\n");
 	printf("Unexpected program interrupt %s: %#x on cpu %d at %#lx, ilen %d\n",
 	       in_sie ? "in SIE" : "",
 	       lowcore.pgm_int_code, stap(), lowcore.pgm_old_psw.addr, lowcore.pgm_int_id);
-	print_int_regs(stack);
+
+	/*
+	 * If we fall out of SIE before loading the host registers,
+	 * then we need to do it here so we print the host registers
+	 * and not the guest registers.
+	 *
+	 * Back tracing is actually not a problem since SIE restores gr15.
+	 */
+	if (in_sie_gregs) {
+		print_int_regs(stack, true);
+		vregs = *((struct vm_save_area **)(stack->grs0[13] + __SF_SIE_SAVEAREA));
+
+		/*
+		 * The grs are not linear on the interrupt stack frame.
+		 * We copy 0 and 1 here and 2 - 15 with the memcopy below.
+		 */
+		stack->grs1[0] = vregs->host.grs[0];
+		stack->grs1[1] = vregs->host.grs[1];
+		/*  2 - 15 */
+		memcpy(stack->grs0, &vregs->host.grs[2], sizeof(stack->grs0) - 8);
+	}
+	print_int_regs(stack, false);
 	dump_stack();
 
 	/* Dump stack doesn't end with a \n so we add it here instead */
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
index a27a8401..147cb0f2 100644
--- a/lib/s390x/sie.h
+++ b/lib/s390x/sie.h
@@ -273,6 +273,8 @@ struct vm {
 
 extern void sie_entry(void);
 extern void sie_exit(void);
+extern void sie_entry_gregs(void);
+extern void sie_exit_gregs(void);
 extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_area);
 void sie(struct vm *vm);
 void sie_expect_validity(struct vm *vm);
diff --git a/s390x/cpu.S b/s390x/cpu.S
index 45bd551a..9155b044 100644
--- a/s390x/cpu.S
+++ b/s390x/cpu.S
@@ -82,7 +82,8 @@ sie64a:
 	# Store scb and save_area pointer into stack frame
 	stg	%r2,__SF_SIE_CONTROL(%r15)	# save control block pointer
 	stg	%r3,__SF_SIE_SAVEAREA(%r15)	# save guest register save area
-
+.globl sie_entry_gregs
+sie_entry_gregs:
 	# Load guest's gprs, fprs and fpc
 	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
 	ld	\i, \i * 8 + SIE_SAVEAREA_GUEST_FPRS(%r3)
@@ -121,7 +122,8 @@ sie_exit:
 	.endr
 	lfpc	SIE_SAVEAREA_HOST_FPC(%r14)
 	lmg	%r0,%r14,SIE_SAVEAREA_HOST_GRS(%r14)	# restore kernel registers
-
+.globl sie_exit_gregs
+sie_exit_gregs:
 	br	%r14
 
 	.align	8
-- 
2.34.1


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

* Re: [kvm-unit-tests PATCH v2 1/7] s390x: Cleanup flat.lds
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 1/7] s390x: Cleanup flat.lds Janosch Frank
@ 2023-01-12 17:05   ` Claudio Imbrenda
  0 siblings, 0 replies; 13+ messages in thread
From: Claudio Imbrenda @ 2023-01-12 17:05 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, thuth, david, nsg, nrb

On Thu, 12 Jan 2023 15:45:42 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> It seems like the loader file was copied over from another
> architecture which has a different page size (64K) than s390 (4K).
> 
> Let's use a 4k alignment instead of the 64k one and remove unneeded
> entries.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  s390x/flat.lds | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/s390x/flat.lds b/s390x/flat.lds
> index de9da1a8..952f6cd4 100644
> --- a/s390x/flat.lds
> +++ b/s390x/flat.lds
> @@ -24,20 +24,8 @@ SECTIONS
>  		*(.text)
>  		*(.text.*)
>  	}
> -	. = ALIGN(64K);
> +	. = ALIGN(4K);
>  	etext = .;
> -	.opd : { *(.opd) }
> -	. = ALIGN(16);
> -	.dynamic : {
> -		dynamic_start = .;
> -		*(.dynamic)
> -	}
> -	.dynsym : {
> -		dynsym_start = .;
> -		*(.dynsym)
> -	}
> -	.rela.dyn : { *(.rela*) }
> -	. = ALIGN(16);
>  	.data : {
>  		*(.data)
>  		*(.data.rel*)
> @@ -48,10 +36,11 @@ SECTIONS
>  	__bss_start = .;
>  	.bss : { *(.bss) }
>  	__bss_end = .;
> -	. = ALIGN(64K);
> +	. = ALIGN(4K);
>  	edata = .;
> +	/* Reserve 64K for the stack */
>  	. += 64K;
> -	. = ALIGN(64K);
> +	. = ALIGN(4K);
>  	/*
>  	 * stackptr set with initial stack frame preallocated
>  	 */


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

* Re: [kvm-unit-tests PATCH v2 2/7] s390x: snippets: c: Cleanup flat.lds
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 2/7] s390x: snippets: c: " Janosch Frank
@ 2023-01-12 17:05   ` Claudio Imbrenda
  0 siblings, 0 replies; 13+ messages in thread
From: Claudio Imbrenda @ 2023-01-12 17:05 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, thuth, david, nsg, nrb

On Thu, 12 Jan 2023 15:45:43 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> There are a lot of things in there which we don't need for snippets
> and the alignments can be switched from 64K to 4K since that's the
> s390 page size.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  s390x/snippets/c/flat.lds | 28 +++++++++-------------------
>  1 file changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/s390x/snippets/c/flat.lds b/s390x/snippets/c/flat.lds
> index 260ab1c4..9e5eb66b 100644
> --- a/s390x/snippets/c/flat.lds
> +++ b/s390x/snippets/c/flat.lds
> @@ -16,27 +16,22 @@ SECTIONS
>  		 QUAD(0x0000000000004000)
>  	}
>  	. = 0x4000;
> +	/*
> +	 * The stack grows down from 0x4000 to 0x2000, we pre-allocoate
> +	 * a frame via the -160.
> +	 */
>  	stackptr = . - 160;
>  	stacktop = .;
> +	/* Start text 0x4000 */
>  	.text : {
>  		*(.init)
>  		*(.text)
>  		*(.text.*)
>  	}
> -	. = ALIGN(64K);
> +	. = ALIGN(4K);
>  	etext = .;
> -	.opd : { *(.opd) }
> -	. = ALIGN(16);
> -	.dynamic : {
> -		dynamic_start = .;
> -		*(.dynamic)
> -	}
> -	.dynsym : {
> -		dynsym_start = .;
> -		*(.dynsym)
> -	}
> -	.rela.dyn : { *(.rela*) }
> -	. = ALIGN(16);
> +	/* End text */
> +	/* Start data */
>  	.data : {
>  		*(.data)
>  		*(.data.rel*)
> @@ -44,11 +39,6 @@ SECTIONS
>  	. = ALIGN(16);
>  	.rodata : { *(.rodata) *(.rodata.*) }
>  	. = ALIGN(16);
> -	__bss_start = .;
>  	.bss : { *(.bss) }
> -	__bss_end = .;
> -	. = ALIGN(64K);
> -	edata = .;
> -	. += 64K;
> -	. = ALIGN(64K);
> +	/* End data */
>  }


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

* Re: [kvm-unit-tests PATCH v2 3/7] s390x: Add a linker script to assembly snippets
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 3/7] s390x: Add a linker script to assembly snippets Janosch Frank
@ 2023-01-12 17:05   ` Claudio Imbrenda
  0 siblings, 0 replies; 13+ messages in thread
From: Claudio Imbrenda @ 2023-01-12 17:05 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, thuth, david, nsg, nrb

On Thu, 12 Jan 2023 15:45:44 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> A linker script has a few benefits:
> - Random data doesn't end up in the binary breaking tests
> - We can easily define a lowcore and load the snippet from 0x0 instead
> of 0x4000 which makes asm snippets behave like c snippets
> - We can easily define an invalid PGM new PSW to ensure an exit on a
> guest PGM
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/s390x/snippet.h         |  3 +--
>  s390x/Makefile              |  5 +++--
>  s390x/mvpg-sie.c            |  2 +-
>  s390x/pv-diags.c            |  6 +++---
>  s390x/snippets/asm/flat.lds | 41 +++++++++++++++++++++++++++++++++++++
>  5 files changed, 49 insertions(+), 8 deletions(-)
>  create mode 100644 s390x/snippets/asm/flat.lds
> 
> diff --git a/lib/s390x/snippet.h b/lib/s390x/snippet.h
> index b17b2a4c..57045994 100644
> --- a/lib/s390x/snippet.h
> +++ b/lib/s390x/snippet.h
> @@ -32,8 +32,7 @@
>  
>  #define SNIPPET_PV_TWEAK0	0x42UL
>  #define SNIPPET_PV_TWEAK1	0UL
> -#define SNIPPET_OFF_C		0
> -#define SNIPPET_OFF_ASM		0x4000
> +#define SNIPPET_UNPACK_OFF	0
>  
>  
>  /*
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 52a9d821..97a61611 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -136,7 +136,8 @@ $(SNIPPET_DIR)/c/%.o: $(SNIPPET_DIR)/c/%.c $(asm-offsets)
>  	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
>  
>  $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o
> -	$(OBJCOPY) -O binary -j ".rodata" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $(patsubst %.gbin,%.o,$@) $@
> +	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/snippets/asm/flat.lds $<
> +	$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
>  	truncate -s '%4096' $@
>  
>  $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS)
> @@ -145,7 +146,7 @@ $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS)
>  	truncate -s '%4096' $@
>  
>  $(SNIPPET_DIR)/asm/%.hdr: $(SNIPPET_DIR)/asm/%.gbin $(HOST_KEY_DOCUMENT)
> -	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x4000,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
> +	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x0,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
>  
>  $(SNIPPET_DIR)/c/%.hdr: $(SNIPPET_DIR)/c/%.gbin $(HOST_KEY_DOCUMENT)
>  	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x0,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> index 46a2edb6..99f4859b 100644
> --- a/s390x/mvpg-sie.c
> +++ b/s390x/mvpg-sie.c
> @@ -87,7 +87,7 @@ static void setup_guest(void)
>  
>  	snippet_setup_guest(&vm, false);
>  	snippet_init(&vm, SNIPPET_NAME_START(c, mvpg_snippet),
> -		     SNIPPET_LEN(c, mvpg_snippet), SNIPPET_OFF_C);
> +		     SNIPPET_LEN(c, mvpg_snippet), SNIPPET_UNPACK_OFF);
>  
>  	/* Enable MVPG interpretation as we want to test KVM and not ourselves */
>  	vm.sblk->eca = ECA_MVPGI;
> diff --git a/s390x/pv-diags.c b/s390x/pv-diags.c
> index 9ced68c7..5165937a 100644
> --- a/s390x/pv-diags.c
> +++ b/s390x/pv-diags.c
> @@ -28,7 +28,7 @@ static void test_diag_500(void)
>  
>  	snippet_pv_init(&vm, SNIPPET_NAME_START(asm, snippet_pv_diag_500),
>  			SNIPPET_HDR_START(asm, snippet_pv_diag_500),
> -			size_gbin, size_hdr, SNIPPET_OFF_ASM);
> +			size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
>  
>  	sie(&vm);
>  	report(vm.sblk->icptcode == ICPT_PV_INSTR && vm.sblk->ipa == 0x8302 &&
> @@ -83,7 +83,7 @@ static void test_diag_288(void)
>  
>  	snippet_pv_init(&vm, SNIPPET_NAME_START(asm, snippet_pv_diag_288),
>  			SNIPPET_HDR_START(asm, snippet_pv_diag_288),
> -			size_gbin, size_hdr, SNIPPET_OFF_ASM);
> +			size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
>  
>  	sie(&vm);
>  	report(vm.sblk->icptcode == ICPT_PV_INSTR && vm.sblk->ipa == 0x8302 &&
> @@ -124,7 +124,7 @@ static void test_diag_yield(void)
>  
>  	snippet_pv_init(&vm, SNIPPET_NAME_START(asm, snippet_pv_diag_yield),
>  			SNIPPET_HDR_START(asm, snippet_pv_diag_yield),
> -			size_gbin, size_hdr, SNIPPET_OFF_ASM);
> +			size_gbin, size_hdr, SNIPPET_UNPACK_OFF);
>  
>  	/* 0x44 */
>  	report_prefix_push("0x44");
> diff --git a/s390x/snippets/asm/flat.lds b/s390x/snippets/asm/flat.lds
> new file mode 100644
> index 00000000..ab0031ac
> --- /dev/null
> +++ b/s390x/snippets/asm/flat.lds
> @@ -0,0 +1,41 @@
> +SECTIONS
> +{
> +	.lowcore : {
> +		/*
> +		 * Initial short psw for disk boot, with 31 bit addressing for
> +		 * non z/Arch environment compatibility and the instruction
> +		 * address 0x4000.
> +		 */
> +		. = 0;
> +		 LONG(0x00080000)
> +		 LONG(0x80004000)
> +		 /* Restart new PSW for booting via PSW restart. */
> +		 . = 0x1a0;
> +		 QUAD(0x0000000180000000)
> +		 QUAD(0x0000000000004000)
> +		 /*
> +		  * Invalid PGM new PSW so we hopefully get a code 8
> +		  * intercept on a PGM
> +		  */
> +		 . = 0x1d0;
> +		 QUAD(0x0008000000000000)
> +		 QUAD(0x0000000000000001)
> +	}
> +	. = 0x4000;
> +	/* Start text 0x4000 */
> +	.text : {
> +		*(.text)
> +		*(.text.*)
> +	}
> +	. = ALIGN(16);
> +	etext = .;
> +	/* End text */
> +	/* Start data */
> +	.data : {
> +		*(.data)
> +		*(.data.rel*)
> +	}
> +	. = ALIGN(16);
> +	.rodata : { *(.rodata) *(.rodata.*) }
> +	/* End data */
> +}


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

* Re: [kvm-unit-tests PATCH v2 7/7] lib: s390x: Handle debug prints for SIE exceptions correctly
  2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 7/7] lib: s390x: Handle debug prints for SIE exceptions correctly Janosch Frank
@ 2023-01-12 17:13   ` Claudio Imbrenda
  2023-01-13 15:50     ` Janosch Frank
  0 siblings, 1 reply; 13+ messages in thread
From: Claudio Imbrenda @ 2023-01-12 17:13 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, thuth, david, nsg, nrb

On Thu, 12 Jan 2023 15:45:48 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> When we leave SIE due to an exception, we'll still have guest values
> in registers 0 - 13 and that's not clearly portraied in our debug
> prints. So let's fix that.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/interrupt.c | 46 ++++++++++++++++++++++++++++++++++++++-----
>  lib/s390x/sie.h       |  2 ++
>  s390x/cpu.S           |  6 ++++--
>  3 files changed, 47 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index dadb7415..ff47c2c2 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -9,6 +9,7 @@
>   */
>  #include <libcflat.h>
>  #include <asm/barrier.h>
> +#include <asm/asm-offsets.h>
>  #include <sclp.h>
>  #include <interrupt.h>
>  #include <sie.h>
> @@ -188,9 +189,12 @@ static void print_storage_exception_information(void)
>  	}
>  }
>  
> -static void print_int_regs(struct stack_frame_int *stack)
> +static void print_int_regs(struct stack_frame_int *stack, bool sie)
>  {
> +	struct kvm_s390_sie_block *sblk;
> +
>  	printf("\n");
> +	printf("%s\n", sie ? "Guest registers:" : "Host registers:");
>  	printf("GPRS:\n");
>  	printf("%016lx %016lx %016lx %016lx\n",
>  	       stack->grs1[0], stack->grs1[1], stack->grs0[0], stack->grs0[1]);
> @@ -198,24 +202,56 @@ static void print_int_regs(struct stack_frame_int *stack)
>  	       stack->grs0[2], stack->grs0[3], stack->grs0[4], stack->grs0[5]);
>  	printf("%016lx %016lx %016lx %016lx\n",
>  	       stack->grs0[6], stack->grs0[7], stack->grs0[8], stack->grs0[9]);
> -	printf("%016lx %016lx %016lx %016lx\n",
> -	       stack->grs0[10], stack->grs0[11], stack->grs0[12], stack->grs0[13]);
> +
> +	if (sie) {
> +		sblk = (struct kvm_s390_sie_block *)stack->grs0[12];
> +		printf("%016lx %016lx %016lx %016lx\n",
> +		       stack->grs0[10], stack->grs0[11], sblk->gg14, sblk->gg15);
> +	} else {
> +		printf("%016lx %016lx %016lx %016lx\n",
> +		       stack->grs0[10], stack->grs0[11], stack->grs0[12], stack->grs0[13]);
> +	}
> +
>  	printf("\n");
>  }
>  
>  static void print_pgm_info(struct stack_frame_int *stack)
>  
>  {
> -	bool in_sie;
> +	bool in_sie, in_sie_gregs;
> +	struct vm_save_area *vregs;
>  
>  	in_sie = (lowcore.pgm_old_psw.addr >= (uintptr_t)sie_entry &&
>  		  lowcore.pgm_old_psw.addr <= (uintptr_t)sie_exit);
> +	in_sie_gregs = (lowcore.pgm_old_psw.addr >= (uintptr_t)sie_entry_gregs &&
> +			lowcore.pgm_old_psw.addr <= (uintptr_t)sie_exit_gregs);

can you explain why <= instead of < ? (I think I know why, but a
comment would not hurt)

with that fixed:

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

>  
>  	printf("\n");
>  	printf("Unexpected program interrupt %s: %#x on cpu %d at %#lx, ilen %d\n",
>  	       in_sie ? "in SIE" : "",
>  	       lowcore.pgm_int_code, stap(), lowcore.pgm_old_psw.addr, lowcore.pgm_int_id);
> -	print_int_regs(stack);
> +
> +	/*
> +	 * If we fall out of SIE before loading the host registers,
> +	 * then we need to do it here so we print the host registers
> +	 * and not the guest registers.
> +	 *
> +	 * Back tracing is actually not a problem since SIE restores gr15.
> +	 */
> +	if (in_sie_gregs) {
> +		print_int_regs(stack, true);
> +		vregs = *((struct vm_save_area **)(stack->grs0[13] + __SF_SIE_SAVEAREA));
> +
> +		/*
> +		 * The grs are not linear on the interrupt stack frame.
> +		 * We copy 0 and 1 here and 2 - 15 with the memcopy below.
> +		 */
> +		stack->grs1[0] = vregs->host.grs[0];
> +		stack->grs1[1] = vregs->host.grs[1];
> +		/*  2 - 15 */
> +		memcpy(stack->grs0, &vregs->host.grs[2], sizeof(stack->grs0) - 8);
> +	}
> +	print_int_regs(stack, false);
>  	dump_stack();
>  
>  	/* Dump stack doesn't end with a \n so we add it here instead */
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index a27a8401..147cb0f2 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -273,6 +273,8 @@ struct vm {
>  
>  extern void sie_entry(void);
>  extern void sie_exit(void);
> +extern void sie_entry_gregs(void);
> +extern void sie_exit_gregs(void);
>  extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_area);
>  void sie(struct vm *vm);
>  void sie_expect_validity(struct vm *vm);
> diff --git a/s390x/cpu.S b/s390x/cpu.S
> index 45bd551a..9155b044 100644
> --- a/s390x/cpu.S
> +++ b/s390x/cpu.S
> @@ -82,7 +82,8 @@ sie64a:
>  	# Store scb and save_area pointer into stack frame
>  	stg	%r2,__SF_SIE_CONTROL(%r15)	# save control block pointer
>  	stg	%r3,__SF_SIE_SAVEAREA(%r15)	# save guest register save area
> -
> +.globl sie_entry_gregs
> +sie_entry_gregs:
>  	# Load guest's gprs, fprs and fpc
>  	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
>  	ld	\i, \i * 8 + SIE_SAVEAREA_GUEST_FPRS(%r3)
> @@ -121,7 +122,8 @@ sie_exit:
>  	.endr
>  	lfpc	SIE_SAVEAREA_HOST_FPC(%r14)
>  	lmg	%r0,%r14,SIE_SAVEAREA_HOST_GRS(%r14)	# restore kernel registers
> -
> +.globl sie_exit_gregs
> +sie_exit_gregs:
>  	br	%r14
>  
>  	.align	8


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

* Re: [kvm-unit-tests PATCH v2 7/7] lib: s390x: Handle debug prints for SIE exceptions correctly
  2023-01-12 17:13   ` Claudio Imbrenda
@ 2023-01-13 15:50     ` Janosch Frank
  0 siblings, 0 replies; 13+ messages in thread
From: Janosch Frank @ 2023-01-13 15:50 UTC (permalink / raw)
  To: Claudio Imbrenda; +Cc: kvm, linux-s390, thuth, david, nsg, nrb

On 1/12/23 18:13, Claudio Imbrenda wrote:
> On Thu, 12 Jan 2023 15:45:48 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> When we leave SIE due to an exception, we'll still have guest values
>> in registers 0 - 13 and that's not clearly portraied in our debug
>> prints. So let's fix that.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/interrupt.c | 46 ++++++++++++++++++++++++++++++++++++++-----
>>   lib/s390x/sie.h       |  2 ++
>>   s390x/cpu.S           |  6 ++++--
>>   3 files changed, 47 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index dadb7415..ff47c2c2 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -9,6 +9,7 @@
>>    */
>>   #include <libcflat.h>
>>   #include <asm/barrier.h>
>> +#include <asm/asm-offsets.h>
>>   #include <sclp.h>
>>   #include <interrupt.h>
>>   #include <sie.h>
>> @@ -188,9 +189,12 @@ static void print_storage_exception_information(void)
>>   	}
>>   }
>>   
>> -static void print_int_regs(struct stack_frame_int *stack)
>> +static void print_int_regs(struct stack_frame_int *stack, bool sie)
>>   {
>> +	struct kvm_s390_sie_block *sblk;
>> +
>>   	printf("\n");
>> +	printf("%s\n", sie ? "Guest registers:" : "Host registers:");
>>   	printf("GPRS:\n");
>>   	printf("%016lx %016lx %016lx %016lx\n",
>>   	       stack->grs1[0], stack->grs1[1], stack->grs0[0], stack->grs0[1]);
>> @@ -198,24 +202,56 @@ static void print_int_regs(struct stack_frame_int *stack)
>>   	       stack->grs0[2], stack->grs0[3], stack->grs0[4], stack->grs0[5]);
>>   	printf("%016lx %016lx %016lx %016lx\n",
>>   	       stack->grs0[6], stack->grs0[7], stack->grs0[8], stack->grs0[9]);
>> -	printf("%016lx %016lx %016lx %016lx\n",
>> -	       stack->grs0[10], stack->grs0[11], stack->grs0[12], stack->grs0[13]);
>> +
>> +	if (sie) {
>> +		sblk = (struct kvm_s390_sie_block *)stack->grs0[12];
>> +		printf("%016lx %016lx %016lx %016lx\n",
>> +		       stack->grs0[10], stack->grs0[11], sblk->gg14, sblk->gg15);
>> +	} else {
>> +		printf("%016lx %016lx %016lx %016lx\n",
>> +		       stack->grs0[10], stack->grs0[11], stack->grs0[12], stack->grs0[13]);
>> +	}
>> +
>>   	printf("\n");
>>   }
>>   
>>   static void print_pgm_info(struct stack_frame_int *stack)
>>   
>>   {
>> -	bool in_sie;
>> +	bool in_sie, in_sie_gregs;
>> +	struct vm_save_area *vregs;
>>   
>>   	in_sie = (lowcore.pgm_old_psw.addr >= (uintptr_t)sie_entry &&
>>   		  lowcore.pgm_old_psw.addr <= (uintptr_t)sie_exit);
>> +	in_sie_gregs = (lowcore.pgm_old_psw.addr >= (uintptr_t)sie_entry_gregs &&
>> +			lowcore.pgm_old_psw.addr <= (uintptr_t)sie_exit_gregs);
> 
> can you explain why <= instead of < ? (I think I know why, but a
> comment would not hurt)

This might be wrong now that the exit label points behind a lmg which 
can only cause the operation or the addressing exception.

The operation exception can only happen on a LMY without the long 
displacement HW support and the addressing exception should only happen 
in circumstances where any processed instruction will likely result in a 
new exception being thrown.

> 
> with that fixed:
> 
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> 
>>   
>>   	printf("\n");
>>   	printf("Unexpected program interrupt %s: %#x on cpu %d at %#lx, ilen %d\n",
>>   	       in_sie ? "in SIE" : "",
>>   	       lowcore.pgm_int_code, stap(), lowcore.pgm_old_psw.addr, lowcore.pgm_int_id);
>> -	print_int_regs(stack);
>> +
>> +	/*
>> +	 * If we fall out of SIE before loading the host registers,
>> +	 * then we need to do it here so we print the host registers
>> +	 * and not the guest registers.
>> +	 *
>> +	 * Back tracing is actually not a problem since SIE restores gr15.
>> +	 */
>> +	if (in_sie_gregs) {
>> +		print_int_regs(stack, true);
>> +		vregs = *((struct vm_save_area **)(stack->grs0[13] + __SF_SIE_SAVEAREA));
>> +
>> +		/*
>> +		 * The grs are not linear on the interrupt stack frame.
>> +		 * We copy 0 and 1 here and 2 - 15 with the memcopy below.
>> +		 */
>> +		stack->grs1[0] = vregs->host.grs[0];
>> +		stack->grs1[1] = vregs->host.grs[1];
>> +		/*  2 - 15 */
>> +		memcpy(stack->grs0, &vregs->host.grs[2], sizeof(stack->grs0) - 8);
>> +	}
>> +	print_int_regs(stack, false);
>>   	dump_stack();
>>   
>>   	/* Dump stack doesn't end with a \n so we add it here instead */
>> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
>> index a27a8401..147cb0f2 100644
>> --- a/lib/s390x/sie.h
>> +++ b/lib/s390x/sie.h
>> @@ -273,6 +273,8 @@ struct vm {
>>   
>>   extern void sie_entry(void);
>>   extern void sie_exit(void);
>> +extern void sie_entry_gregs(void);
>> +extern void sie_exit_gregs(void);
>>   extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_area);
>>   void sie(struct vm *vm);
>>   void sie_expect_validity(struct vm *vm);
>> diff --git a/s390x/cpu.S b/s390x/cpu.S
>> index 45bd551a..9155b044 100644
>> --- a/s390x/cpu.S
>> +++ b/s390x/cpu.S
>> @@ -82,7 +82,8 @@ sie64a:
>>   	# Store scb and save_area pointer into stack frame
>>   	stg	%r2,__SF_SIE_CONTROL(%r15)	# save control block pointer
>>   	stg	%r3,__SF_SIE_SAVEAREA(%r15)	# save guest register save area
>> -
>> +.globl sie_entry_gregs
>> +sie_entry_gregs:
>>   	# Load guest's gprs, fprs and fpc
>>   	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
>>   	ld	\i, \i * 8 + SIE_SAVEAREA_GUEST_FPRS(%r3)
>> @@ -121,7 +122,8 @@ sie_exit:
>>   	.endr
>>   	lfpc	SIE_SAVEAREA_HOST_FPC(%r14)
>>   	lmg	%r0,%r14,SIE_SAVEAREA_HOST_GRS(%r14)	# restore kernel registers
>> -
>> +.globl sie_exit_gregs
>> +sie_exit_gregs:
>>   	br	%r14
>>   
>>   	.align	8
> 


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

end of thread, other threads:[~2023-01-13 16:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-12 15:45 [kvm-unit-tests PATCH v2 0/7] s390x: Snippet fixes Janosch Frank
2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 1/7] s390x: Cleanup flat.lds Janosch Frank
2023-01-12 17:05   ` Claudio Imbrenda
2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 2/7] s390x: snippets: c: " Janosch Frank
2023-01-12 17:05   ` Claudio Imbrenda
2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 3/7] s390x: Add a linker script to assembly snippets Janosch Frank
2023-01-12 17:05   ` Claudio Imbrenda
2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 4/7] s390x: snippets: Fix SET_PSW_NEW_ADDR macro Janosch Frank
2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 5/7] lib: s390x: sie: Set guest memory pointer Janosch Frank
2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 6/7] s390x: Clear first stack frame and end backtrace early Janosch Frank
2023-01-12 15:45 ` [kvm-unit-tests PATCH v2 7/7] lib: s390x: Handle debug prints for SIE exceptions correctly Janosch Frank
2023-01-12 17:13   ` Claudio Imbrenda
2023-01-13 15:50     ` Janosch Frank

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