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