[kvm-unit-tests PATCH v2 0/3] s390x: Improve architectural compliance for diag308

[kvm-unit-tests PATCH v2 0/3] s390x: Improve architectural compliance for diag308

[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

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

-- 
2.20.1

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

[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 <frankja@linux.ibm.com>
---
 s390x/cstart64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 8e2b21e..043e34a 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -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
-- 
2.20.1

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

[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 <frankja@linux.ibm.com>
---
 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(-)

diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
index 6e2d259..4b213f8 100644
--- a/lib/s390x/asm-offsets.c
+++ b/lib/s390x/asm-offsets.c
@@ -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);
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 96cca2e..07d4e5e 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -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 */
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 5cade23..c9e2dc6 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -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;
 }
 
diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index 7602886..f57f420 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -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);
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 043e34a..4be20fc 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -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:
-- 
2.20.1

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

[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 <frankja@linux.ibm.com>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
index 4b213f8..61d2658 100644
--- a/lib/s390x/asm-offsets.c
+++ b/lib/s390x/asm-offsets.c
@@ -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);
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 07d4e5e..7d25e4f 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -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 */
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 4be20fc..86dd4c4 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -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:
-- 
2.20.1

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

[David Hildenbrand]

> Am 11.11.2019 um 16:34 schrieb Janosch Frank <frankja@linux.ibm.com>:
> 
> 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 <david@redhat.com>

> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> 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(-)
> 
> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
> index 6e2d259..4b213f8 100644
> --- a/lib/s390x/asm-offsets.c
> +++ b/lib/s390x/asm-offsets.c
> @@ -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);
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 96cca2e..07d4e5e 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -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 */
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 5cade23..c9e2dc6 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -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;
> }
> 
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 7602886..f57f420 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -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);
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 043e34a..4be20fc 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -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:
> -- 
> 2.20.1
> 

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

[Thomas Huth]

On 11/11/2019 16.33, Janosch Frank wrote:
> 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 <frankja@linux.ibm.com>
> ---
>  s390x/cstart64.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 8e2b21e..043e34a 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -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
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Thomas Huth]

On 11/11/2019 16.33, Janosch Frank wrote:
> 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 <frankja@linux.ibm.com>
> ---
>  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 <thuth@redhat.com>

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

[Thomas Huth]

On 11/11/2019 16.33, Janosch Frank wrote:
> 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 <frankja@linux.ibm.com>
> ---
>  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 <thuth@redhat.com>

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

[David Hildenbrand]

On 11.11.19 16:33, Janosch Frank wrote:
> 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 <frankja@linux.ibm.com>
> ---
>  lib/s390x/asm-offsets.c  |  1 +
>  lib/s390x/asm/arch_def.h |  3 ++-
>  s390x/cstart64.S         | 24 +++++++++++++++++-------
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
> index 4b213f8..61d2658 100644
> --- a/lib/s390x/asm-offsets.c
> +++ b/lib/s390x/asm-offsets.c
> @@ -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);
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 07d4e5e..7d25e4f 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -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 */
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index 4be20fc..86dd4c4 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -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:
> 

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


-- 

Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v2 0/3] s390x: Improve architectural compliance for diag308

[David Hildenbrand]

On 11.11.19 16:33, Janosch Frank wrote:
> 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(-)
> 

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.

-- 

Thanks,

David / dhildenb

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

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 4722 bytes --]

On 11/12/19 1:09 PM, David Hildenbrand wrote:
> On 11.11.19 16:33, Janosch Frank wrote:
>> 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 <frankja@linux.ibm.com>
>> ---
>>  lib/s390x/asm-offsets.c  |  1 +
>>  lib/s390x/asm/arch_def.h |  3 ++-
>>  s390x/cstart64.S         | 24 +++++++++++++++++-------
>>  3 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
>> index 4b213f8..61d2658 100644
>> --- a/lib/s390x/asm-offsets.c
>> +++ b/lib/s390x/asm-offsets.c
>> @@ -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);
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 07d4e5e..7d25e4f 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -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 */
>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index 4be20fc..86dd4c4 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -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:
>>
> 
> 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...

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[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 <frankja@linux.ibm.com>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  3 ++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
index 4b213f8..61d2658 100644
--- a/lib/s390x/asm-offsets.c
+++ b/lib/s390x/asm-offsets.c
@@ -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);
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 07d4e5e..5f034a7 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -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 */
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 4be20fc..86dd4c4 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -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:
-- 
2.20.1

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

[Thomas Huth]

On 12/11/2019 14.42, Janosch Frank wrote:
> On 11/12/19 1:09 PM, David Hildenbrand wrote:
>> On 11.11.19 16:33, Janosch Frank wrote:
>>> 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 <frankja@linux.ibm.com>
>>> ---
>>>  lib/s390x/asm-offsets.c  |  1 +
>>>  lib/s390x/asm/arch_def.h |  3 ++-
>>>  s390x/cstart64.S         | 24 +++++++++++++++++-------
>>>  3 files changed, 20 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
>>> index 4b213f8..61d2658 100644
>>> --- a/lib/s390x/asm-offsets.c
>>> +++ b/lib/s390x/asm-offsets.c
>>> @@ -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);
>>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>>> index 07d4e5e..7d25e4f 100644
>>> --- a/lib/s390x/asm/arch_def.h
>>> +++ b/lib/s390x/asm/arch_def.h
>>> @@ -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 */
[...]
>> 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

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

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 3090 bytes --]

On 11/12/19 5:17 PM, Thomas Huth wrote:
> On 12/11/2019 14.42, Janosch Frank wrote:
>> On 11/12/19 1:09 PM, David Hildenbrand wrote:
>>> On 11.11.19 16:33, Janosch Frank wrote:
>>>> 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 <frankja@linux.ibm.com>
>>>> ---
>>>>  lib/s390x/asm-offsets.c  |  1 +
>>>>  lib/s390x/asm/arch_def.h |  3 ++-
>>>>  s390x/cstart64.S         | 24 +++++++++++++++++-------
>>>>  3 files changed, 20 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
>>>> index 4b213f8..61d2658 100644
>>>> --- a/lib/s390x/asm-offsets.c
>>>> +++ b/lib/s390x/asm-offsets.c
>>>> @@ -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);
>>>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>>>> index 07d4e5e..7d25e4f 100644
>>>> --- a/lib/s390x/asm/arch_def.h
>>>> +++ b/lib/s390x/asm/arch_def.h
>>>> @@ -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 */
> [...]
>>> 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
> 

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

diff --git i/lib/s390x/asm/arch_def.h w/lib/s390x/asm/arch_def.h
index 5f034a7..cf6e1ca 100644
--- i/lib/s390x/asm/arch_def.h
+++ w/lib/s390x/asm/arch_def.h
@@ -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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Thomas Huth]

[-- Attachment #1.1: Type: text/plain, Size: 3324 bytes --]

On 13/11/2019 11.04, Janosch Frank wrote:
> On 11/12/19 5:17 PM, Thomas Huth wrote:
>> On 12/11/2019 14.42, Janosch Frank wrote:
>>> On 11/12/19 1:09 PM, David Hildenbrand wrote:
>>>> On 11.11.19 16:33, Janosch Frank wrote:
>>>>> 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 <frankja@linux.ibm.com>
>>>>> ---
>>>>>  lib/s390x/asm-offsets.c  |  1 +
>>>>>  lib/s390x/asm/arch_def.h |  3 ++-
>>>>>  s390x/cstart64.S         | 24 +++++++++++++++++-------
>>>>>  3 files changed, 20 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
>>>>> index 4b213f8..61d2658 100644
>>>>> --- a/lib/s390x/asm-offsets.c
>>>>> +++ b/lib/s390x/asm-offsets.c
>>>>> @@ -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);
>>>>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>>>>> index 07d4e5e..7d25e4f 100644
>>>>> --- a/lib/s390x/asm/arch_def.h
>>>>> +++ b/lib/s390x/asm/arch_def.h
>>>>> @@ -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 */
>> [...]
>>>> 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
>>
> 
> How about this?
> Or do we want to extend the struct to 8K and test for that?
> 
> diff --git i/lib/s390x/asm/arch_def.h w/lib/s390x/asm/arch_def.h
> index 5f034a7..cf6e1ca 100644
> --- i/lib/s390x/asm/arch_def.h
> +++ w/lib/s390x/asm/arch_def.h
> @@ -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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[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 <frankja@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 lib/s390x/asm-offsets.c  |  1 +
 lib/s390x/asm/arch_def.h |  4 +++-
 s390x/cstart64.S         | 24 +++++++++++++++++-------
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
index 4b213f8..61d2658 100644
--- a/lib/s390x/asm-offsets.c
+++ b/lib/s390x/asm-offsets.c
@@ -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);
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 07d4e5e..cf6e1ca 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -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
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 4be20fc..86dd4c4 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -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:
-- 
2.20.1

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

[David Hildenbrand]

On 13.11.19 12:24, Janosch Frank wrote:
> 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 <frankja@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

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,

David / dhildenb

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

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 840 bytes --]

On 11/13/19 7:06 PM, David Hildenbrand wrote:
> On 13.11.19 12:24, Janosch Frank wrote:
>> 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 <frankja@linux.ibm.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> 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 :)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]