diff mbox series

[kvm-unit-tests,2/2] s390x: Remove DAT and add short indication psw bits on diag308 reset

Message ID 20191105162828.2490-3-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Improve architectural compliance for diag308 | expand

Commit Message

Janosch Frank Nov. 5, 2019, 4:28 p.m. UTC 
On a diag308 subcode 0 CRs will be reset, so we need to mask of PSW
DAT indication until we restore our CRs.

Also we need to set the short psw indication to be compliant with the
architecture.

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         | 20 ++++++++++++++------
 3 files changed, 17 insertions(+), 7 deletions(-)

Comments

David Hildenbrand Nov. 5, 2019, 7:53 p.m. UTC | #1 
On 05.11.19 17:28, Janosch Frank wrote:

In the subject "Disable" vs. "Remove" ?

> On a diag308 subcode 0 CRs will be reset, so we need to mask of PSW
> DAT indication until we restore our CRs.
> 
> Also we need to set the short psw indication to be compliant with the
> architecture.
> 
> 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         | 20 ++++++++++++++------
>   3 files changed, 17 insertions(+), 7 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 0455591..2e0dcf5 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -129,8 +129,15 @@ memsetxc:
>   .globl diag308_load_reset
>   diag308_load_reset:
>   	SAVE_REGS
> -	/* Save the first PSW word to the IPL PSW */
> +	/* Backup current PSW */

/*
  * Backup the current PSW MASK, as we have to restore it on
  * success.
  */

>   	epsw	%r0, %r1
> +	st	%r0, GEN_LC_SW_INT_PSW
> +	st	%r1, GEN_LC_SW_INT_PSW + 4

I was confused at first, but then I realized that you really only store 
the PSW mask here and not also the PSW address ...


> +	/* Disable DAT as the CRs will be reset too */
> +	nilh	%r0, 0xfbff
> +	/* Add psw bit 12 to indicate short psw */
> +	oilh	%r0, 0x0008

Why care about the old PSW mask here at all? Wouldn't it be easier to 
just construct a new PSW mask from scratch? (64bit, PSW bit 12 set ...)

Save it somewhere and just load it directly from memory.

> +	/* Save the first PSW word to the IPL PSW */
>   	st	%r0, 0
>   	/* Store the address and the bit for 31 bit addressing */
>   	larl    %r0, 0f
> @@ -142,12 +149,13 @@ diag308_load_reset:
>   	xgr	%r2, %r2
>   	br	%r14
>   	/* Success path */
> -	/* We lost cr0 due to the reset */
> -0:	larl	%r1, initial_cr0
> -	lctlg	%c0, %c0, 0(%r1)
> -	RESTORE_REGS
> +	/* Switch to z/Architecture mode and 64-bit */
> +0:	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:
>
Janosch Frank Nov. 6, 2019, 6:58 a.m. UTC | #2 
On 11/5/19 8:53 PM, David Hildenbrand wrote:
> On 05.11.19 17:28, Janosch Frank wrote:
> 
> In the subject "Disable" vs. "Remove" ?
> 
>> On a diag308 subcode 0 CRs will be reset, so we need to mask of PSW
>> DAT indication until we restore our CRs.
>>
>> Also we need to set the short psw indication to be compliant with the
>> architecture.
>>
>> 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         | 20 ++++++++++++++------
>>   3 files changed, 17 insertions(+), 7 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 0455591..2e0dcf5 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -129,8 +129,15 @@ memsetxc:
>>   .globl diag308_load_reset
>>   diag308_load_reset:
>>   	SAVE_REGS
>> -	/* Save the first PSW word to the IPL PSW */
>> +	/* Backup current PSW */
> 
> /*
>   * Backup the current PSW MASK, as we have to restore it on
>   * success.
>   */
> 
>>   	epsw	%r0, %r1
>> +	st	%r0, GEN_LC_SW_INT_PSW
>> +	st	%r1, GEN_LC_SW_INT_PSW + 4
> 
> I was confused at first, but then I realized that you really only store 
> the PSW mask here and not also the PSW address ...
> 
> 
>> +	/* Disable DAT as the CRs will be reset too */
>> +	nilh	%r0, 0xfbff
>> +	/* Add psw bit 12 to indicate short psw */
>> +	oilh	%r0, 0x0008
> 
> Why care about the old PSW mask here at all? Wouldn't it be easier to 
> just construct a new PSW mask from scratch? (64bit, PSW bit 12 set ...)
> 
> Save it somewhere and just load it directly from memory.

Sounds like a good idea, will do

> 
>> +	/* Save the first PSW word to the IPL PSW */
>>   	st	%r0, 0
>>   	/* Store the address and the bit for 31 bit addressing */
>>   	larl    %r0, 0f
>> @@ -142,12 +149,13 @@ diag308_load_reset:
>>   	xgr	%r2, %r2
>>   	br	%r14
>>   	/* Success path */
>> -	/* We lost cr0 due to the reset */
>> -0:	larl	%r1, initial_cr0
>> -	lctlg	%c0, %c0, 0(%r1)
>> -	RESTORE_REGS
>> +	/* Switch to z/Architecture mode and 64-bit */
>> +0:	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:
>>
> 
>
Janosch Frank Nov. 6, 2019, 10:19 a.m. UTC | #3 
On 11/5/19 5:28 PM, Janosch Frank wrote:
> On a diag308 subcode 0 CRs will be reset, so we need to mask of PSW
> DAT indication until we restore our CRs.
> 
> Also we need to set the short psw indication to be compliant with the
> architecture.
> 
> 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         | 20 ++++++++++++++------
>  3 files changed, 17 insertions(+), 7 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 */

That's obviously not correct, the psw is two DWORDS long, not one...

>  	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 0455591..2e0dcf5 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -129,8 +129,15 @@ memsetxc:
>  .globl diag308_load_reset
>  diag308_load_reset:
>  	SAVE_REGS
> -	/* Save the first PSW word to the IPL PSW */
> +	/* Backup current PSW */
>  	epsw	%r0, %r1
> +	st	%r0, GEN_LC_SW_INT_PSW
> +	st	%r1, GEN_LC_SW_INT_PSW + 4
> +	/* Disable DAT as the CRs will be reset too */
> +	nilh	%r0, 0xfbff
> +	/* Add psw bit 12 to indicate short psw */
> +	oilh	%r0, 0x0008
> +	/* Save the first PSW word to the IPL PSW */
>  	st	%r0, 0
>  	/* Store the address and the bit for 31 bit addressing */
>  	larl    %r0, 0f
> @@ -142,12 +149,13 @@ diag308_load_reset:
>  	xgr	%r2, %r2
>  	br	%r14
>  	/* Success path */
> -	/* We lost cr0 due to the reset */
> -0:	larl	%r1, initial_cr0
> -	lctlg	%c0, %c0, 0(%r1)
> -	RESTORE_REGS
> +	/* Switch to z/Architecture mode and 64-bit */
> +0:	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:
>
diff mbox series

Patch

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 0455591..2e0dcf5 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -129,8 +129,15 @@  memsetxc:
 .globl diag308_load_reset
 diag308_load_reset:
 	SAVE_REGS
-	/* Save the first PSW word to the IPL PSW */
+	/* Backup current PSW */
 	epsw	%r0, %r1
+	st	%r0, GEN_LC_SW_INT_PSW
+	st	%r1, GEN_LC_SW_INT_PSW + 4
+	/* Disable DAT as the CRs will be reset too */
+	nilh	%r0, 0xfbff
+	/* Add psw bit 12 to indicate short psw */
+	oilh	%r0, 0x0008
+	/* Save the first PSW word to the IPL PSW */
 	st	%r0, 0
 	/* Store the address and the bit for 31 bit addressing */
 	larl    %r0, 0f
@@ -142,12 +149,13 @@  diag308_load_reset:
 	xgr	%r2, %r2
 	br	%r14
 	/* Success path */
-	/* We lost cr0 due to the reset */
-0:	larl	%r1, initial_cr0
-	lctlg	%c0, %c0, 0(%r1)
-	RESTORE_REGS
+	/* Switch to z/Architecture mode and 64-bit */
+0:	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: