Re: [kvm-unit-tests v2 PATCH] s390x: Add a test for the diagnose 0x308 hypercall

[Thomas Huth <thuth@redhat.com>]

On 16/04/2019 13.57, David Hildenbrand wrote:
> On 15.04.19 14:12, Thomas Huth wrote:
>> The original idea for this test is to have an easy way to check for a
>> problem that has been fixed in QEMU recently: QEMU simply aborted if this
>> diagnose call was called with an unsupported subcode (e.g. 2). The problem
>> has been fixed in QEMU commit 37dbd1f4d4805edcd18d94eb202bb3461b3cd52d
>> ("Return specification exception for unimplemented diag 308 subcodes"),
>> and this test now should make sure that we do not regress here again.
>> While we're at it, also check whether LOAD NORMAL via subcode 1 works
>> correctly, whether the diagnose call is blocked correctly in problem state
>> and whether subcodes 5 and 6 are generating specification exceptions for
>> illegal parameters as expected.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  v2: Use "epsw" in test_subcode1() to get the right upper bits of the PSW
>>
>>  s390x/Makefile      |   1 +
>>  s390x/diag308.c     | 130 ++++++++++++++++++++++++++++++++++++++++++++
>>  s390x/unittests.cfg |   3 +
>>  3 files changed, 134 insertions(+)
>>  create mode 100644 s390x/diag308.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index af40fd4..1f21ddb 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -5,6 +5,7 @@ tests += $(TEST_DIR)/sieve.elf
>>  tests += $(TEST_DIR)/sthyi.elf
>>  tests += $(TEST_DIR)/skey.elf
>>  tests += $(TEST_DIR)/diag10.elf
>> +tests += $(TEST_DIR)/diag308.elf
>>  tests += $(TEST_DIR)/pfmf.elf
>>  tests += $(TEST_DIR)/cmm.elf
>>  tests += $(TEST_DIR)/vector.elf
>> diff --git a/s390x/diag308.c b/s390x/diag308.c
>> new file mode 100644
>> index 0000000..70a3b1e
>> --- /dev/null
>> +++ b/s390x/diag308.c
>> @@ -0,0 +1,130 @@
>> +/*
>> + * Diagnose 0x308 hypercall tests
>> + *
>> + * Copyright (c) 2019 Thomas Huth, Red Hat Inc.
>> + *
>> + * This code is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2, or (at
>> + * your option) any later version.
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm/interrupt.h>
>> +
>> +/* The diagnose calls should be blocked in problem state */
>> +static void test_priv(void)
>> +{
>> +	expect_pgm_int();
>> +	enter_pstate();
>> +	asm volatile ("diag %0,%1,0x308" :: "a"(0), "a"(3));
>> +	check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
>> +}
[...]
> 
> You might want to use "g" instead of "a", as we are not dealing with
> address registers (base/index). Apart from that, nothing major jumped at me.
> 

Copy-n-past "bug" from diag10.c ;-)

I'll switch it to "d" which is should be the right thing on s390x.

 Thanks,
  Thomas