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

[Janosch Frank <frankja@linux.ibm.com>]

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

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



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