Re: [PATCH v2 5/6] selftests: kvm: s390: Streamline uc_skey test to issue iske after sske

[Hariharan Mari <hari55@linux.ibm.com>]

On 2024-12-16 10:21, Christoph Schlameuss wrote:
> In some rare situations a non default storage key is already set on the
> memory used by the test. Within normal VMs the key is reset / zapped
> when the memory is added to the VM. This is not the case for ucontrol
> VMs. With the initial iske check removed this test case can work in all
> situations. The function of the iske instruction is still validated by
> the remaining code.
> 
> Fixes: 7d900f8ac191 ("selftests: kvm: s390: Add uc_skey VM test case")
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

Tested-by: Hariharan Mari <hari55@linux.ibm.com>
> ---
>  .../selftests/kvm/s390x/ucontrol_test.c       | 22 +++++--------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> index 8f306395696e..ef3e391811b3 100644
> --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> @@ -88,10 +88,6 @@ asm("test_skey_asm:\n"
>  	"	ahi	%r0,1\n"
>  	"	st	%r1,0(%r5,%r6)\n"
> 
> -	"	iske	%r1,%r6\n"
> -	"	ahi	%r0,1\n"
> -	"	diag	0,0,0x44\n"
> -
>  	"	sske	%r1,%r6\n"
>  	"	xgr	%r1,%r1\n"
>  	"	iske	%r1,%r6\n"
> @@ -593,7 +589,9 @@ TEST_F(uc_kvm, uc_skey)
>  	ASSERT_EQ(true, uc_handle_exit(self));
>  	ASSERT_EQ(1, sync_regs->gprs[0]);
> 
> -	/* ISKE */
> +	/* SSKE + ISKE */
> +	sync_regs->gprs[1] = skeyvalue;
> +	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
>  	ASSERT_EQ(0, uc_run_once(self));
> 
>  	/*
> @@ -607,19 +605,9 @@ TEST_F(uc_kvm, uc_skey)
>  	TEST_ASSERT_EQ(ICPT_INST, sie_block->icptcode);
>  	TEST_REQUIRE(sie_block->ipa != 0xb229);
> 
> -	/* ISKE contd. */
> +	/* SSKE + ISKE contd. */
>  	ASSERT_EQ(false, uc_handle_exit(self));
>  	ASSERT_EQ(2, sync_regs->gprs[0]);
> -	/* assert initial skey (ACC = 0, R & C = 1) */
> -	ASSERT_EQ(0x06, sync_regs->gprs[1]);
> -	uc_assert_diag44(self);
> -
> -	/* SSKE + ISKE */
> -	sync_regs->gprs[1] = skeyvalue;
> -	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
> -	ASSERT_EQ(0, uc_run_once(self));
> -	ASSERT_EQ(false, uc_handle_exit(self));
> -	ASSERT_EQ(3, sync_regs->gprs[0]);
>  	ASSERT_EQ(skeyvalue, sync_regs->gprs[1]);
>  	uc_assert_diag44(self);
> 
> @@ -628,7 +616,7 @@ TEST_F(uc_kvm, uc_skey)
>  	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
>  	ASSERT_EQ(0, uc_run_once(self));
>  	ASSERT_EQ(false, uc_handle_exit(self));
> -	ASSERT_EQ(4, sync_regs->gprs[0]);
> +	ASSERT_EQ(3, sync_regs->gprs[0]);
>  	/* assert R reset but rest of skey unchanged */
>  	ASSERT_EQ(skeyvalue & 0xfa, sync_regs->gprs[1]);
>  	ASSERT_EQ(0, sync_regs->gprs[1] & 0x04);