Skip to contentSkip to conversation

s390x: Improve architectural compliance for diag308

PATCH In review Janosch Frank 3 participants 18 messages Nov 11, 2019 – Nov 14, 2019

Subsystems (2)
KVM s390
lore
Share & tools
lore

Janosch Frank

author
#0 lore raw

v2 cover s390x: Improve architectural compliance for diag308

When testing diag308 subcodes 0/1 on lpar with virtual mem set up, I experienced spec PGMs and addressing PGMs due to the tests not setting short psw bit 12 and leaving the DAT bit on. The problem was not found under KVM/QEMU, because Qemu just ignores all cpu mask bits.

v1 -> v2:
* Fixed comment in extra patch
* Now using pre-defined reset psw
* Fixed some comments

Janosch Frank (3):
s390x: Fix initial cr0 load comments
s390x: Add CR save area
s390x: Load reset psw on diag308 reset

 lib/s390x/asm-offsets.c  |  3 ++-
 lib/s390x/asm/arch_def.h |  5 +++--
 lib/s390x/interrupt.c    |  4 ++--
 lib/s390x/smp.c          |  2 +-
 s390x/cstart64.S         | 38 ++++++++++++++++++++++++--------------
 5 files changed, 32 insertions(+), 20 deletions(-)
Signature · 1 line
2.20.1
author
#1 lore raw

PATCH v2 1/3 s390x: Fix initial cr0 load comments

We need to load cr0 to have access to all fprs during save and restore of fprs. Saving conditionally on basis of the CR0 AFP bit would be a pain.

Signed-off-by Janosch Frank <[email protected]>
---
 s390x/cstart64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
s390x/cstart64.S +2 -2
index 8e2b21e..043e34a 100644
@@ -94,7 +94,7 @@ memsetxc:
 	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
 	/* save cr0 */
 	stctg	%c0, %c0, GEN_LC_SW_INT_CR0
-	/* load initial cr0 again */
+	/* load a cr0 that has the AFP control bit which enables all FPRs */
 	larl	%r1, initial_cr0
 	lctlg	%c0, %c0, 0(%r1)
 	/* save fprs 0-15 + fpc */

@@ -139,7 +139,7 @@ diag308_load_reset:
 	xgr	%r2, %r2
 	br	%r14
 	/* Success path */
-	/* We lost cr0 due to the reset */
+	/* load a cr0 that has the AFP control bit which enables all FPRs */
 0:	larl	%r1, initial_cr0
 	lctlg	%c0, %c0, 0(%r1)
 	RESTORE_REGS
Signature · 1 line
2.20.1
Reviewed-by on v2 1/3
#2 lore raw

Re: [kvm-unit-tests PATCH v2 1/3] s390x: Fix initial cr0 load comments

Quoted 24 lines · Janosch Frank

We need to load cr0 to have access to all fprs during save and restore of fprs. Saving conditionally on basis of the CR0 AFP bit would be a pain.

Signed-off-by Janosch Frank <[email protected]>
---
 s390x/cstart64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
s390x/cstart64.S +1 -1
index 8e2b21e..043e34a 100644
@@ -94,7 +94,7 @@ memsetxc:
 	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
 	/* save cr0 */
 	stctg	%c0, %c0, GEN_LC_SW_INT_CR0
-	/* load initial cr0 again */
+	/* load a cr0 that has the AFP control bit which enables all FPRs */
 	larl	%r1, initial_cr0
 	lctlg	%c0, %c0, 0(%r1)
 	/* save fprs 0-15 + fpc */

s390x/cstart64.S diag308_load_reset: · line 145

Quoted 8 lines
s390x/cstart64.S +1 -1
@@ -139,7 +139,7 @@ diag308_load_reset:
 	xgr	%r2, %r2
 	br	%r14
 	/* Success path */
-	/* We lost cr0 due to the reset */
+	/* load a cr0 that has the AFP control bit which enables all FPRs */
 0:	larl	%r1, initial_cr0
 	lctlg	%c0, %c0, 0(%r1)
 	RESTORE_REGS
Reviewed-by Thomas Huth <[email protected]>
author
#3 lore raw

PATCH v2 2/3 s390x: Add CR save area

In reply to Janosch Frank

If we run with DAT enabled and do a reset, we need to save the CRs to backup our ASCEs on a diag308 for example.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  2 +-
 lib/s390x/asm/arch_def.h |  4 ++--
 lib/s390x/interrupt.c    |  4 ++--
 lib/s390x/smp.c          |  2 +-
 s390x/cstart64.S         | 10 +++++-----
 5 files changed, 11 insertions(+), 11 deletions(-)
lib/s390x/asm-offsets.c +1 -1
index 6e2d259..4b213f8 100644
@@ -57,7 +57,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_GRS, lowcore, sw_int_grs);
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
-	OFFSET(GEN_LC_SW_INT_CR0, lowcore, sw_int_cr0);
+	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -2
index 96cca2e..07d4e5e 100644
@@ -78,8 +78,8 @@ struct lowcore {
 	uint64_t	sw_int_fprs[16];		/* 0x0280 */
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
-	uint64_t	sw_int_cr0;			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0310];	/* 0x0310 */
+	uint64_t	sw_int_crs[16];			/* 0x0308 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */
lib/s390x/interrupt.c +2 -2
index 5cade23..c9e2dc6 100644
@@ -124,13 +124,13 @@ void handle_ext_int(void)
 	}
 
 	if (lc->ext_int_code == EXT_IRQ_SERVICE_SIG) {
-		lc->sw_int_cr0 &= ~(1UL << 9);
+		lc->sw_int_crs[0] &= ~(1UL << 9);
 		sclp_handle_ext();
 	} else {
 		ext_int_expected = false;
 	}
 
-	if (!(lc->sw_int_cr0 & CR0_EXTM_MASK))
+	if (!(lc->sw_int_crs[0] & CR0_EXTM_MASK))
 		lc->ext_old_psw.mask &= ~PSW_MASK_EXT;
 }
lib/s390x/smp.c +1 -1
index 7602886..f57f420 100644
@@ -189,7 +189,7 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
 	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
 	lc->restart_new_psw.mask = 0x0000000180000000UL;
 	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
-	lc->sw_int_cr0 = 0x0000000000040000UL;
+	lc->sw_int_crs[0] = 0x0000000000040000UL;
 
 	/* Start processing */
 	rc = sigp_retry(cpu->addr, SIGP_RESTART, 0, NULL);
s390x/cstart64.S +5 -5
index 043e34a..4be20fc 100644
@@ -92,8 +92,8 @@ memsetxc:
 	.macro SAVE_REGS
 	/* save grs 0-15 */
 	stmg	%r0, %r15, GEN_LC_SW_INT_GRS
-	/* save cr0 */
-	stctg	%c0, %c0, GEN_LC_SW_INT_CR0
+	/* save crs 0-15 */
+	stctg	%c0, %c15, GEN_LC_SW_INT_CRS
 	/* load a cr0 that has the AFP control bit which enables all FPRs */
 	larl	%r1, initial_cr0
 	lctlg	%c0, %c0, 0(%r1)

@@ -112,8 +112,8 @@ memsetxc:
 	ld	\i, \i * 8(%r1)
 	.endr
 	lfpc	GEN_LC_SW_INT_FPC
-	/* restore cr0 */
-	lctlg	%c0, %c0, GEN_LC_SW_INT_CR0
+	/* restore crs 0-15 */
+	lctlg	%c0, %c15, GEN_LC_SW_INT_CRS
 	/* restore grs 0-15 */
 	lmg	%r0, %r15, GEN_LC_SW_INT_GRS
 	.endm

@@ -150,7 +150,7 @@ diag308_load_reset:
 smp_cpu_setup_state:
 	xgr	%r1, %r1
 	lmg     %r0, %r15, GEN_LC_SW_INT_GRS
-	lctlg   %c0, %c0, GEN_LC_SW_INT_CR0
+	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
 	br	%r14
 
 pgm_int:
Signature · 1 line
2.20.1
Reviewed-by on v2 2/3
#4 lore raw

Re: [kvm-unit-tests PATCH v2 2/3] s390x: Add CR save area

Quoted 7 lines

Am 11.11.2019 um 16:34 schrieb Janosch Frank <[email protected]>:

If we run with DAT enabled and do a reset, we need to save the CRs to backup our ASCEs on a diag308 for example.

Looks good

Reviewed-by David Hildenbrand <[email protected]>
Quoted 108 lines
Signed-off-by Janosch Frank <[email protected]>

---

lib/s390x/asm-offsets.c  |  2 +-
lib/s390x/asm/arch_def.h |  4 ++--
lib/s390x/interrupt.c    |  4 ++--
lib/s390x/smp.c          |  2 +-
s390x/cstart64.S         | 10 +++++-----
5 files changed, 11 insertions(+), 11 deletions(-)
lib/s390x/asm-offsets.c +1 -1
index 6e2d259..4b213f8 100644
@@ -57,7 +57,7 @@ int main(void)
   OFFSET(GEN_LC_SW_INT_GRS, lowcore, sw_int_grs);
   OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
   OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
-    OFFSET(GEN_LC_SW_INT_CR0, lowcore, sw_int_cr0);
+    OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
   OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
   OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
   OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -2
index 96cca2e..07d4e5e 100644
@@ -78,8 +78,8 @@ struct lowcore {
   uint64_t    sw_int_fprs[16];        /* 0x0280 */
   uint32_t    sw_int_fpc;            /* 0x0300 */
   uint8_t        pad_0x0304[0x0308 - 0x0304];    /* 0x0304 */
-    uint64_t    sw_int_cr0;            /* 0x0308 */
-    uint8_t        pad_0x0310[0x11b0 - 0x0310];    /* 0x0310 */
+    uint64_t    sw_int_crs[16];            /* 0x0308 */
+    uint8_t        pad_0x0310[0x11b0 - 0x0388];    /* 0x0388 */
   uint64_t    mcck_ext_sa_addr;        /* 0x11b0 */
   uint8_t        pad_0x11b8[0x1200 - 0x11b8];    /* 0x11b8 */
   uint64_t    fprs_sa[16];            /* 0x1200 */
lib/s390x/interrupt.c +2 -2
index 5cade23..c9e2dc6 100644
@@ -124,13 +124,13 @@ void handle_ext_int(void)
   }
 
   if (lc->ext_int_code == EXT_IRQ_SERVICE_SIG) {
-        lc->sw_int_cr0 &= ~(1UL << 9);
+        lc->sw_int_crs[0] &= ~(1UL << 9);
       sclp_handle_ext();
   } else {
       ext_int_expected = false;
   }
 
-    if (!(lc->sw_int_cr0 & CR0_EXTM_MASK))
+    if (!(lc->sw_int_crs[0] & CR0_EXTM_MASK))
       lc->ext_old_psw.mask &= ~PSW_MASK_EXT;
 }
lib/s390x/smp.c +1 -1
index 7602886..f57f420 100644
@@ -189,7 +189,7 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
   cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
   lc->restart_new_psw.mask = 0x0000000180000000UL;
   lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
-    lc->sw_int_cr0 = 0x0000000000040000UL;
+    lc->sw_int_crs[0] = 0x0000000000040000UL;
 
   /* Start processing */
   rc = sigp_retry(cpu->addr, SIGP_RESTART, 0, NULL);
s390x/cstart64.S +4 -4
index 043e34a..4be20fc 100644
@@ -92,8 +92,8 @@ memsetxc:
   .macro SAVE_REGS
   /* save grs 0-15 */
   stmg    %r0, %r15, GEN_LC_SW_INT_GRS
-    /* save cr0 */
-    stctg    %c0, %c0, GEN_LC_SW_INT_CR0
+    /* save crs 0-15 */
+    stctg    %c0, %c15, GEN_LC_SW_INT_CRS
   /* load a cr0 that has the AFP control bit which enables all FPRs */
   larl    %r1, initial_cr0
   lctlg    %c0, %c0, 0(%r1)

@@ -112,8 +112,8 @@ memsetxc:
   ld    \i, \i * 8(%r1)
   .endr
   lfpc    GEN_LC_SW_INT_FPC
-    /* restore cr0 */
-    lctlg    %c0, %c0, GEN_LC_SW_INT_CR0
+    /* restore crs 0-15 */
+    lctlg    %c0, %c15, GEN_LC_SW_INT_CRS
   /* restore grs 0-15 */
   lmg    %r0, %r15, GEN_LC_SW_INT_GRS
   .endm

@@ -150,7 +150,7 @@ diag308_load_reset:

smp_cpu_setup_state:

   xgr    %r1, %r1
   lmg     %r0, %r15, GEN_LC_SW_INT_GRS
s390x/cstart64.S +1 -1
-    lctlg   %c0, %c0, GEN_LC_SW_INT_CR0
+    lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
   br    %r14

pgm_int:

Signature · 1 line
2.20.1
Reviewed-by on v2 2/3
#5 lore raw

Re: [kvm-unit-tests PATCH v2 2/3] s390x: Add CR save area

In reply to Janosch Frank
Quoted 11 lines · Janosch Frank

If we run with DAT enabled and do a reset, we need to save the CRs to backup our ASCEs on a diag308 for example.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  2 +-
 lib/s390x/asm/arch_def.h |  4 ++--
 lib/s390x/interrupt.c    |  4 ++--
 lib/s390x/smp.c          |  2 +-
 s390x/cstart64.S         | 10 +++++-----
 5 files changed, 11 insertions(+), 11 deletions(-)
Reviewed-by Thomas Huth <[email protected]>
author
#6 lore raw

PATCH v2 3/3 s390x: Load reset psw on diag308 reset

In reply to Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -1
index 07d4e5e..7d25e4f 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0390];	/* 0x0390 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */
s390x/cstart64.S +17 -7
index 4be20fc..86dd4c4 100644
@@ -126,13 +126,18 @@ memsetxc:
 .globl diag308_load_reset
 diag308_load_reset:
 	SAVE_REGS
-	/* Save the first PSW word to the IPL PSW */
+	/* Backup current PSW mask, as we have to restore it on success */
 	epsw	%r0, %r1
-	st	%r0, 0
-	/* Store the address and the bit for 31 bit addressing */
-	larl    %r0, 0f
-	oilh    %r0, 0x8000
-	st      %r0, 0x4
+	st	%r0, GEN_LC_SW_INT_PSW
+	st	%r1, GEN_LC_SW_INT_PSW + 4
+	/* Load reset psw mask (short psw, 64 bit) */
+	lg	%r0, reset_psw
+	/* Load the success label address */
+	larl    %r1, 0f
+	/* Or it to the mask */
+	ogr	%r0, %r1
+	/* Store it at the reset PSW location (real 0x0) */
+	stg	%r0, 0
 	/* Do the reset */
 	diag    %r0,%r2,0x308
 	/* Failure path */

@@ -144,7 +149,10 @@ diag308_load_reset:
 	lctlg	%c0, %c0, 0(%r1)
 	RESTORE_REGS
 	lhi	%r2, 1
-	br	%r14
+	larl	%r0, 1f
+	stg	%r0, GEN_LC_SW_INT_PSW + 8
+	lpswe	GEN_LC_SW_INT_PSW
+1:	br	%r14
 
 .globl smp_cpu_setup_state
 smp_cpu_setup_state:

@@ -184,6 +192,8 @@ svc_int:
 	lpswe	GEN_LC_SVC_OLD_PSW
 
 	.align	8
+reset_psw:
+	.quad	0x0008000180000000
 initial_psw:
 	.quad	0x0000000180000000, clear_bss_start
 pgm_int_psw:
Signature · 1 line
2.20.1
Reviewed-by on v2 3/3
#7 lore raw

Re: [kvm-unit-tests PATCH v2 3/3] s390x: Load reset psw on diag308 reset

Quoted 7 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Quoted 6 lines
Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
Reviewed-by Thomas Huth <[email protected]>
#8 lore raw

Re: [kvm-unit-tests PATCH v2 3/3] s390x: Load reset psw on diag308 reset

In reply to Janosch Frank
Quoted 83 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -1
index 07d4e5e..7d25e4f 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0390];	/* 0x0390 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */
s390x/cstart64.S +15 -7
index 4be20fc..86dd4c4 100644
@@ -126,13 +126,18 @@ memsetxc:
 .globl diag308_load_reset
 diag308_load_reset:
 	SAVE_REGS
-	/* Save the first PSW word to the IPL PSW */
+	/* Backup current PSW mask, as we have to restore it on success */
 	epsw	%r0, %r1
-	st	%r0, 0
-	/* Store the address and the bit for 31 bit addressing */
-	larl    %r0, 0f
-	oilh    %r0, 0x8000
-	st      %r0, 0x4
+	st	%r0, GEN_LC_SW_INT_PSW
+	st	%r1, GEN_LC_SW_INT_PSW + 4
+	/* Load reset psw mask (short psw, 64 bit) */
+	lg	%r0, reset_psw
+	/* Load the success label address */
+	larl    %r1, 0f
+	/* Or it to the mask */
+	ogr	%r0, %r1
+	/* Store it at the reset PSW location (real 0x0) */
+	stg	%r0, 0
 	/* Do the reset */
 	diag    %r0,%r2,0x308
 	/* Failure path */

@@ -144,7 +149,10 @@ diag308_load_reset:
 	lctlg	%c0, %c0, 0(%r1)
 	RESTORE_REGS
 	lhi	%r2, 1
-	br	%r14
+	larl	%r0, 1f
+	stg	%r0, GEN_LC_SW_INT_PSW + 8
+	lpswe	GEN_LC_SW_INT_PSW
+1:	br	%r14
 
 .globl smp_cpu_setup_state
 smp_cpu_setup_state:

s390x/cstart64.S svc_int: · line 199

Quoted 8 lines
s390x/cstart64.S +2 -0
@@ -184,6 +192,8 @@ svc_int:
 	lpswe	GEN_LC_SVC_OLD_PSW
 
 	.align	8
+reset_psw:
+	.quad	0x0008000180000000
 initial_psw:
 	.quad	0x0000000180000000, clear_bss_start
 pgm_int_psw:

This patch breaks the smp test under TCG (no clue and no time to look into the details :) ):

timeout -k 1s --foreground 90s /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x -nodefaults -nographic -machine s390-ccw-virtio,accel=tcg -chardev stdio,id=con0 -device sclpconsole,chardev=con0 -kernel s390x/smp.elf -smp 1 -smp 2 # -initrd /tmp/tmp.EDi4y0tv58
SMP: Initializing, found 2 cpus
PASS: smp: start
PASS: smp: stop
FAIL: smp: stop store status: prefix
PASS: smp: stop store status: stack
PASS: smp: store status at address: running: incorrect state
PASS: smp: store status at address: running: status not written
PASS: smp: store status at address: stopped: status written
PASS: smp: ecall: ecall
PASS: smp: emcall: ecall
PASS: smp: cpu reset: cpu stopped
PASS: smp: reset initial: clear: psw
PASS: smp: reset initial: clear: prefix
PASS: smp: reset initial: clear: fpc
PASS: smp: reset initial: clear: cpu timer
PASS: smp: reset initial: clear: todpr
PASS: smp: reset initial: initialized: cr0 == 0xE0
PASS: smp: reset initial: initialized: cr14 == 0xC2000000
PASS: smp: reset initial: cpu stopped
SUMMARY: 18 tests, 1 unexpected failures

Signature · 3 lines
Thanks,

David / dhildenb
author
#9 lore raw

Re: [kvm-unit-tests PATCH v2 3/3] s390x: Load reset psw on diag308 reset

Quoted 85 lines · David Hildenbrand
Quoted 84 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -1
index 07d4e5e..7d25e4f 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0390];	/* 0x0390 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */
s390x/cstart64.S +15 -7
index 4be20fc..86dd4c4 100644
@@ -126,13 +126,18 @@ memsetxc:
 .globl diag308_load_reset
 diag308_load_reset:
 	SAVE_REGS
-	/* Save the first PSW word to the IPL PSW */
+	/* Backup current PSW mask, as we have to restore it on success */
 	epsw	%r0, %r1
-	st	%r0, 0
-	/* Store the address and the bit for 31 bit addressing */
-	larl    %r0, 0f
-	oilh    %r0, 0x8000
-	st      %r0, 0x4
+	st	%r0, GEN_LC_SW_INT_PSW
+	st	%r1, GEN_LC_SW_INT_PSW + 4
+	/* Load reset psw mask (short psw, 64 bit) */
+	lg	%r0, reset_psw
+	/* Load the success label address */
+	larl    %r1, 0f
+	/* Or it to the mask */
+	ogr	%r0, %r1
+	/* Store it at the reset PSW location (real 0x0) */
+	stg	%r0, 0
 	/* Do the reset */
 	diag    %r0,%r2,0x308
 	/* Failure path */

@@ -144,7 +149,10 @@ diag308_load_reset:
 	lctlg	%c0, %c0, 0(%r1)
 	RESTORE_REGS
 	lhi	%r2, 1
-	br	%r14
+	larl	%r0, 1f
+	stg	%r0, GEN_LC_SW_INT_PSW + 8
+	lpswe	GEN_LC_SW_INT_PSW
+1:	br	%r14
 
 .globl smp_cpu_setup_state
 smp_cpu_setup_state:

@@ -184,6 +192,8 @@ svc_int:
 	lpswe	GEN_LC_SVC_OLD_PSW
Quoted 10 lines

s390x/cstart64.S svc_int: · line 199

Quoted 7 lines
s390x/cstart64.S +2 -0
 	.align	8
+reset_psw:
+	.quad	0x0008000180000000
 initial_psw:
 	.quad	0x0000000180000000, clear_bss_start
 pgm_int_psw:

This patch breaks the smp test under TCG (no clue and no time to look into the details :) ):

I forgot to fixup the offset calculation at the top of the patch once again...

Quoted 28 lines

timeout -k 1s --foreground 90s /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x -nodefaults

(quoted diff) +0 -3
-nographic -machine s390-ccw-virtio,accel=tcg -chardev stdio,id=con0
-device sclpconsole,chardev=con0 -kernel s390x/smp.elf -smp 1 -smp 2 #
-initrd /tmp/tmp.EDi4y0tv58

SMP: Initializing, found 2 cpus
PASS: smp: start
PASS: smp: stop
FAIL: smp: stop store status: prefix
PASS: smp: stop store status: stack
PASS: smp: store status at address: running: incorrect state
PASS: smp: store status at address: running: status not written
PASS: smp: store status at address: stopped: status written
PASS: smp: ecall: ecall
PASS: smp: emcall: ecall
PASS: smp: cpu reset: cpu stopped
PASS: smp: reset initial: clear: psw
PASS: smp: reset initial: clear: prefix
PASS: smp: reset initial: clear: fpc
PASS: smp: reset initial: clear: cpu timer
PASS: smp: reset initial: clear: todpr
PASS: smp: reset initial: initialized: cr0 == 0xE0
PASS: smp: reset initial: initialized: cr14 == 0xC2000000
PASS: smp: reset initial: cpu stopped
SUMMARY: 18 tests, 1 unexpected failures

#10 lore raw

Re: [kvm-unit-tests PATCH v2 3/3] s390x: Load reset psw on diag308 reset

Quoted 42 lines · Janosch Frank
Quoted 41 lines · David Hildenbrand
Quoted 40 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -1
index 07d4e5e..7d25e4f 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0390];	/* 0x0390 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */

[...]

Quoted 5 lines
Quoted 2 lines

This patch breaks the smp test under TCG (no clue and no time to look into the details :) ):

I forgot to fixup the offset calculation at the top of the patch once again...

Maybe add a

_Static_assert(sizeof(struct lowcore) == xyz)

after the struct definitions, to avoid that this happens again?

Thomas

author
#11 lore raw

Re: [kvm-unit-tests PATCH v2 3/3] s390x: Load reset psw on diag308 reset

Quoted 50 lines · Thomas Huth
Quoted 42 lines · Janosch Frank
Quoted 41 lines · David Hildenbrand
Quoted 40 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -1
index 07d4e5e..7d25e4f 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0390];	/* 0x0390 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */

[...]

Quoted 5 lines
Quoted 2 lines

This patch breaks the smp test under TCG (no clue and no time to look into the details :) ):

I forgot to fixup the offset calculation at the top of the patch once again...

Quoted 8 lines

Maybe add a

_Static_assert(sizeof(struct lowcore) == xyz)

after the struct definitions, to avoid that this happens again?

Thomas

How about this?
Or do we want to extend the struct to 8K and test for that?

lib/s390x/asm/arch_def.h +1 -0
index 5f034a7..cf6e1ca 100644
@@ -99,6 +99,7 @@ struct lowcore {
        uint8_t         pad_0x1400[0x1800 - 0x1400];    /* 0x1400 */
        uint8_t         pgm_int_tdb[0x1900 - 0x1800];   /* 0x1800 */
 } __attribute__ ((__packed__));
+_Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");
 
 #define PGM_INT_CODE_OPERATION                 0x01
 #define PGM_INT_CODE_PRIVILEGED_OPERATION      0x02
#12 lore raw

Re: [kvm-unit-tests PATCH v2 3/3] s390x: Load reset psw on diag308 reset

Quoted 68 lines · Janosch Frank
Quoted 50 lines · Thomas Huth
Quoted 42 lines · Janosch Frank
Quoted 41 lines · David Hildenbrand
Quoted 40 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -1
index 07d4e5e..7d25e4f 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0390];	/* 0x0390 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */

[...]

Quoted 5 lines
Quoted 2 lines

This patch breaks the smp test under TCG (no clue and no time to look into the details :) ):

I forgot to fixup the offset calculation at the top of the patch once again...

Quoted 8 lines

Maybe add a

_Static_assert(sizeof(struct lowcore) == xyz)

after the struct definitions, to avoid that this happens again?

Thomas

How about this?
Or do we want to extend the struct to 8K and test for that?

lib/s390x/asm/arch_def.h +0 -0
index 5f034a7..cf6e1ca 100644

lib/s390x/asm/arch_def.h struct lowcore { · line 102

Quoted 4 lines
lib/s390x/asm/arch_def.h +1 -0
@@ -99,6 +99,7 @@ struct lowcore {
        uint8_t         pad_0x1400[0x1800 - 0x1400];    /* 0x1400 */
        uint8_t         pgm_int_tdb[0x1900 - 0x1800];   /* 0x1800 */
 } __attribute__ ((__packed__));
+_Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");

Fine for me either way (either checking for 0x1900 or extending the struct to 8192).
Hmm, maybe we should go with 0x1900 for now, and extend the struct to 8192 bytes later if there is a reason to do it.

Thomas

author
#13 lore raw

PATCH v4 1/1 s390x: Load reset psw on diag308 reset

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Thomas Huth <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  4 +++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 21 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +3 -1
index 07d4e5e..cf6e1ca 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0398];	/* 0x0398 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */

@@ -98,6 +99,7 @@ struct lowcore {
 	uint8_t		pad_0x1400[0x1800 - 0x1400];	/* 0x1400 */
 	uint8_t		pgm_int_tdb[0x1900 - 0x1800];	/* 0x1800 */
 } __attribute__ ((__packed__));
+_Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");
 
 #define PGM_INT_CODE_OPERATION			0x01
 #define PGM_INT_CODE_PRIVILEGED_OPERATION	0x02
s390x/cstart64.S +17 -7
index 4be20fc..86dd4c4 100644
@@ -126,13 +126,18 @@ memsetxc:
 .globl diag308_load_reset
 diag308_load_reset:
 	SAVE_REGS
-	/* Save the first PSW word to the IPL PSW */
+	/* Backup current PSW mask, as we have to restore it on success */
 	epsw	%r0, %r1
-	st	%r0, 0
-	/* Store the address and the bit for 31 bit addressing */
-	larl    %r0, 0f
-	oilh    %r0, 0x8000
-	st      %r0, 0x4
+	st	%r0, GEN_LC_SW_INT_PSW
+	st	%r1, GEN_LC_SW_INT_PSW + 4
+	/* Load reset psw mask (short psw, 64 bit) */
+	lg	%r0, reset_psw
+	/* Load the success label address */
+	larl    %r1, 0f
+	/* Or it to the mask */
+	ogr	%r0, %r1
+	/* Store it at the reset PSW location (real 0x0) */
+	stg	%r0, 0
 	/* Do the reset */
 	diag    %r0,%r2,0x308
 	/* Failure path */

@@ -144,7 +149,10 @@ diag308_load_reset:
 	lctlg	%c0, %c0, 0(%r1)
 	RESTORE_REGS
 	lhi	%r2, 1
-	br	%r14
+	larl	%r0, 1f
+	stg	%r0, GEN_LC_SW_INT_PSW + 8
+	lpswe	GEN_LC_SW_INT_PSW
+1:	br	%r14
 
 .globl smp_cpu_setup_state
 smp_cpu_setup_state:

@@ -184,6 +192,8 @@ svc_int:
 	lpswe	GEN_LC_SVC_OLD_PSW
 
 	.align	8
+reset_psw:
+	.quad	0x0008000180000000
 initial_psw:
 	.quad	0x0000000180000000, clear_bss_start
 pgm_int_psw:
Signature · 1 line
2.20.1
#14 lore raw

Re: [kvm-unit-tests PATCH v4] s390x: Load reset psw on diag308 reset

Quoted 9 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Thomas Huth <[email protected]>

Queued to

https://github.com/davidhildenbrand/kvm-unit-tests.git s390x-next

I'll most probably wait a bit for the SCLP stuff to settle to send a pull request!

Signature · 3 lines
Thanks,

David / dhildenb
author
#15 lore raw

Re: [kvm-unit-tests PATCH v4] s390x: Load reset psw on diag308 reset

Quoted 11 lines · David Hildenbrand
Quoted 9 lines · Janosch Frank

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
Reviewed-by Thomas Huth <[email protected]>
Quoted 7 lines

Queued to

https://github.com/davidhildenbrand/kvm-unit-tests.git s390x-next

I'll most probably wait a bit for the SCLP stuff to settle to send a pull request!

Thanks!
I got some more things in my queue that I need to flush but I'm also waiting a bit for sclp and IO to settle down :)

author
#16 lore raw

PATCH v3 1/1 s390x: Load reset psw on diag308 reset

In reply to David Hildenbrand

On a diag308 subcode 0 CRs will be reset, so we need a PSW mask without DAT. Also we need to set the short psw indication to be compliant with the architecture.

Let's therefore define a reset PSW mask with 64 bit addressing and short PSW indication that is compliant with architecture and use it.

Signed-off-by Janosch Frank <[email protected]>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)
lib/s390x/asm-offsets.c +1 -0
index 4b213f8..61d2658 100644
@@ -58,6 +58,7 @@ int main(void)
 	OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
 	OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
 	OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
+	OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
 	OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
 	OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
 	OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
lib/s390x/asm/arch_def.h +2 -1
index 07d4e5e..5f034a7 100644
@@ -79,7 +79,8 @@ struct lowcore {
 	uint32_t	sw_int_fpc;			/* 0x0300 */
 	uint8_t		pad_0x0304[0x0308 - 0x0304];	/* 0x0304 */
 	uint64_t	sw_int_crs[16];			/* 0x0308 */
-	uint8_t		pad_0x0310[0x11b0 - 0x0388];	/* 0x0388 */
+	struct psw	sw_int_psw;			/* 0x0388 */
+	uint8_t		pad_0x0310[0x11b0 - 0x0398];	/* 0x0398 */
 	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
 	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
 	uint64_t	fprs_sa[16];			/* 0x1200 */
s390x/cstart64.S +17 -7
index 4be20fc..86dd4c4 100644
@@ -126,13 +126,18 @@ memsetxc:
 .globl diag308_load_reset
 diag308_load_reset:
 	SAVE_REGS
-	/* Save the first PSW word to the IPL PSW */
+	/* Backup current PSW mask, as we have to restore it on success */
 	epsw	%r0, %r1
-	st	%r0, 0
-	/* Store the address and the bit for 31 bit addressing */
-	larl    %r0, 0f
-	oilh    %r0, 0x8000
-	st      %r0, 0x4
+	st	%r0, GEN_LC_SW_INT_PSW
+	st	%r1, GEN_LC_SW_INT_PSW + 4
+	/* Load reset psw mask (short psw, 64 bit) */
+	lg	%r0, reset_psw
+	/* Load the success label address */
+	larl    %r1, 0f
+	/* Or it to the mask */
+	ogr	%r0, %r1
+	/* Store it at the reset PSW location (real 0x0) */
+	stg	%r0, 0
 	/* Do the reset */
 	diag    %r0,%r2,0x308
 	/* Failure path */

@@ -144,7 +149,10 @@ diag308_load_reset:
 	lctlg	%c0, %c0, 0(%r1)
 	RESTORE_REGS
 	lhi	%r2, 1
-	br	%r14
+	larl	%r0, 1f
+	stg	%r0, GEN_LC_SW_INT_PSW + 8
+	lpswe	GEN_LC_SW_INT_PSW
+1:	br	%r14
 
 .globl smp_cpu_setup_state
 smp_cpu_setup_state:

@@ -184,6 +192,8 @@ svc_int:
 	lpswe	GEN_LC_SVC_OLD_PSW
 
 	.align	8
+reset_psw:
+	.quad	0x0008000180000000
 initial_psw:
 	.quad	0x0000000180000000, clear_bss_start
 pgm_int_psw:
Signature · 1 line
2.20.1
#17 lore raw
In reply to Janosch Frank
Quoted 15 lines · Janosch Frank

When testing diag308 subcodes 0/1 on lpar with virtual mem set up, I experienced spec PGMs and addressing PGMs due to the tests not setting short psw bit 12 and leaving the DAT bit on. The problem was not found under KVM/QEMU, because Qemu just ignores all cpu mask bits.

v1 -> v2:
* Fixed comment in extra patch
* Now using pre-defined reset psw
* Fixed some comments

Janosch Frank (3):
s390x: Fix initial cr0 load comments
s390x: Add CR save area
s390x: Load reset psw on diag308 reset

Quoted 7 lines
 lib/s390x/asm-offsets.c  |  3 ++-
 lib/s390x/asm/arch_def.h |  5 +++--
 lib/s390x/interrupt.c    |  4 ++--
 lib/s390x/smp.c          |  2 +-
 s390x/cstart64.S         | 38 ++++++++++++++++++++++++--------------
 5 files changed, 32 insertions(+), 20 deletions(-)

I'll queue the first two patches for now to

https://github.com/davidhildenbrand/kvm-unit-tests.git s390x-next

And wait with the first until we know if it's a TCG or a kvm-unit-tests bug that makes the SMP test fail.

Signature · 3 lines
Thanks,

David / dhildenb