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

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

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

On 8/28/19 2:02 PM, Thomas Huth wrote:
> On 28/08/2019 13.36, 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>
>> ---
>>  s390x/Makefile |   1 +
>>  s390x/pfmf.c   |  10 ++++
>>  s390x/skey.c   |   5 ++
>>  s390x/skrf.c   | 128 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 144 insertions(+)
>>  create mode 100644 s390x/skrf.c
> [...]
>> +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("mvcos	%[dst],%[src],%[len]"
>> +		     : [dst] "+Q" (*(dst))
>> +		     : [src] "Q" (*(src)), [len] "d" (r3), "d" (oac)
> 
> Just a nit: I think you could write "*dst" instead of "*(dst)" and
> "*src" instead of "*(src)".
> 
>> +		     : "cc", "memory");
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	report_prefix_pop();
>> +}
>> +
>> +static void test_spka(void)
>> +{
>> +	report_prefix_push("spka");
>> +	expect_pgm_int();
>> +	asm volatile("spka	0xf0(0)\n");
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	report_prefix_pop();
>> +}
>> +
>> +static void test_tprot(void)
>> +{
>> +	report_prefix_push("tprot");
>> +	expect_pgm_int();
>> +	asm volatile("tprot	%[addr],0xf0(0)\n"
>> +		     : : [addr] "a" (pagebuf) : );
>> +	check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
>> +	report_prefix_pop();
>> +}
>> +
>> +int main(void)
>> +{
>> +	report_prefix_push("skrf");
>> +	if (!test_facility(169)) {
>> +		report_skip("storage key removal facility not available\n");
>> +		goto done;
>> +	}
>> +
>> +	test_facilities();
>> +	test_skey();
>> +	test_pfmf();
>> +	test_psw_key();
>> +	test_mvcos();
>> +	test_spka();
>> +	test_tprot();
>> +
>> +done:
>> +	report_prefix_pop();
>> +	return report_summary();
>> +}
> 
> I can't say much about the technical details here (since I don't have
> the doc for that "removal facility"), but apart from that, the patch
> looks fine to me now.
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> 
> (and I'll wait one or two more days for additional reviews before
> queuing the patches)
> 
Great, thank you!


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