Re: [kvm-unit-tests PATCH 3/3] s390x: pv: Add IPL reset tests

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

On 2/17/23 17:42, Claudio Imbrenda wrote:
> On Wed,  1 Feb 2023 08:48:33 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> The diag308 requires extensive cooperation between the hypervisor and
>> the Ultravisor so the Ultravisor can make sure all necessary reset
>> steps have been done.
>>
>> Let's check if we get the correct validity errors.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
> 
> [...]
> 
>> +
>> +	/*
>> +	 * We need to perform several UV calls to emulate the subcode
>> +	 * 1. Failing to do that should result in a validity
>> +	 *
>> +	 * - Mark all cpus as stopped
>> +	 * - Reset the cpus, calling one gets an initial reset
>> +	 * - Load the reset PSW
>> +	 * - Unshare all
>> +	 * - Load the reset PSW
> 
> you forgot to mention prepare the reset, and the list does not reflect
> the order things are done in the code

Ok


> [...]

>> +/* Execute the diag500 which will set the subcode we execute in gr2 */
>> +diag	0, 0, 0x500
>> +
>> +/*
>> + * A valid PGM new PSW can be a real problem since we never fall out
>> + * of SIE and therefore effectively loop forever. 0 is a valid PSW
>> + * therefore we re-use the reset_psw as this has the short PSW
>> + * bit set which is invalid for a long PSW like the exception new
>> + * PSWs.
>> + *
>> + * For subcode 0/1 there are no PGMs to consider.
>> + */
>> +lgrl   %r5, reset_psw
>> +stg    %r5, GEN_LC_PGM_NEW_PSW
>> +
>> +/* Clean registers that are used */
>> +xgr	%r0, %r0
>> +xgr	%r1, %r1
>> +xgr	%r3, %r3
>> +xgr	%r4, %r4
>> +xgr	%r5, %r5
>> +xgr	%r6, %r6
>> +
>> +/* Subcode 0 - Modified Clear */
> 
> what about subcode 1?

My guess is that this hasn't been removed after a re-work of the code.
I suggest to remove the comment.

> 
>> +SET_RESET_PSW_ADDR done
>> +diag	%r0, %r2, 0x308
>> +
>> +/* Should never be executed because of the reset PSW */
>> +diag	0, 0, 0x44
>> +
>> +done:
>> +lghi	%r1, 42
>> +diag	%r1, 0, 0x9c
>> +
>> +
>> +	.align	8
>> +reset_psw:
>> +	.quad	0x0008000180000000
>