Re: [kvm-unit-tests PATCH 3/3] s390x: Add storage key removal facility

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

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

On 8/27/19 7:58 PM, Thomas Huth wrote:
> On 27/08/2019 15.49, Janosch Frank wrote:
>> The storage key removal facility (stfle bit 169) makes all key related
>> instructions result in a special operation exception if they handle a
>> key.
>>
>> Let's make sure that the skey and pfmf tests only run non key code
>> (pfmf) or not at all (skey).
>>
>> Also let's test this new facility. As lots of instructions are
>> affected by this, only some of them are tested for now.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---

>> +static void test_skey(void)
>> +{
>> +	report_prefix_push("(i|s)ske");
>> +	expect_pgm_int();
>> +	set_storage_key(pagebuf, 0x30, 0);
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	expect_pgm_int();
>> +	get_storage_key(pagebuf);
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	report_prefix_pop();
> 
> Wouldn't it be better to have distinct prefixes for the two tests?

Will do

> 
>> +}
>> +
>> +static void test_pfmf(void)
>> +{
>> +	union pfmf_r1 r1;
>> +
>> +	report_prefix_push("pfmf");
>> +	r1.val = 0;
>> +	r1.reg.sk = 1;
>> +	r1.reg.fsc = PFMF_FSC_4K;
>> +	r1.reg.key = 0x30;
>> +	expect_pgm_int();
>> +	pfmf(r1.val, pagebuf);
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	report_prefix_pop();
>> +}
>> +
>> +static void test_psw_key(void)
>> +{
>> +	uint64_t psw_mask = extract_psw_mask() | 0xF0000000000000UL;
>> +
>> +	report_prefix_push("psw key");
>> +	expect_pgm_int();
>> +	load_psw_mask(psw_mask);
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	report_prefix_pop();
>> +}
>> +
>> +static void test_mvcos(void)
>> +{
>> +	uint64_t r3 = 64;
>> +	uint8_t *src = pagebuf;
>> +	uint8_t *dst = pagebuf + PAGE_SIZE;
>> +	/* K bit set, as well as keys */
>> +	register unsigned long oac asm("0") = 0xf002f002;
>> +
>> +	report_prefix_push("mvcos");
>> +	expect_pgm_int();
>> +	asm volatile(".machine \"z10\"\n"
>> +		     ".machine \"push\"\n"
> 
> Shouldn't that be the other way round? first push the current one, then
> set the new one?

Yes, I interpreted the documentation in the wrong way and it was a PPC
documentation anyway :)

> 
> Anyway, I've now also checked this patch in the CI:
> 
> diff a/s390x/Makefile b/s390x/Makefile
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -25,7 +25,7 @@ CFLAGS += -std=gnu99
>  CFLAGS += -ffreestanding
>  CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/s390x -I lib
>  CFLAGS += -O2
> -CFLAGS += -march=z900
> +CFLAGS += -march=z10
>  CFLAGS += -fno-delete-null-pointer-checks
>  LDFLAGS += -nostdlib -Wl,--build-id=none
> 
> ... and it also seems to work fine with the TCG there:
> 
> https://gitlab.com/huth/kvm-unit-tests/-/jobs/281450598
> 
> So I think you can simply change it in the Makefile instead.

z10 or directly something higher?

> 
>  Thomas
> 
>> +		     "mvcos	%[dst],%[src],%[len]\n"
>> +		     ".machine \"pop\"\n"
>> +		     : [dst] "+Q" (*(dst))
>> +		     : [src] "Q" (*(src)), [len] "d" (r3), "d" (oac)
>> +		     : "cc", "memory");
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	report_prefix_pop();
>> +}



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