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

[David Hildenbrand <david@redhat.com>]

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);
> +}
> +
> +/*
> + * Check that diag308 with subcode 1 loads the PSW at address 0, i.e.
> + * that we can put a pointer into address 4 which then gets executed.
> + */
> +static void test_subcode1(void)
> +{
> +	uint64_t saved_psw = *(uint64_t *)0;
> +	long subcode = 1;
> +	long ret, tmp;
> +
> +	asm volatile (
> +		"	epsw	%0,%1\n"
> +		"	st	%0,0\n"
> +		"	larl	%0,0f\n"
> +		"	oilh	%0,0x8000\n"
> +		"	st	%0,4\n"
> +		"	diag	0,%2,0x308\n"
> +		"	lghi	%0,0\n"
> +		"	j	1f\n"
> +		"0:	lghi	%0,1\n"
> +		"1:"
> +		: "=&a"(ret), "=&a"(tmp) : "a"(subcode) : "memory");
> +
> +	*(uint64_t *)0 = saved_psw;
> +
> +	report("load normal reset done", ret == 1);
> +}
> +
> +/* Expect a specification exception when using an uneven register */
> +static void test_uneven_reg(unsigned int subcode)
> +{
> +	register unsigned long sc asm("6") = subcode;
> +	register unsigned long r3 asm("9") = 0x2000;
> +
> +	report_prefix_push("uneven register");
> +	expect_pgm_int();
> +	asm volatile ("diag %0,%1,0x308" :: "a"(r3), "a"(sc));
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +}
> +
> +/* Expect a specification exception when using an unaligned address */
> +static void test_unaligned_address(unsigned int subcode)
> +{
> +	register unsigned long sc asm("6") = subcode;
> +	register unsigned long addr asm("8") = 54321;
> +
> +	report_prefix_push("unaligned address");
> +	expect_pgm_int();
> +	asm volatile ("diag %0,%1,0x308" :: "a"(addr), "a"(sc));
> +	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +	report_prefix_pop();
> +}
> +
> +static void test_subcode5(void)
> +{
> +	test_uneven_reg(5);
> +	test_unaligned_address(5);
> +}
> +
> +static void test_subcode6(void)
> +{
> +	test_uneven_reg(6);
> +	test_unaligned_address(6);
> +}
> +
> +/* Unsupported subcodes should generate a specification exception */
> +static void test_unsupported_subcode(void)
> +{
> +	int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 };
> +	int idx;
> +
> +	for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) {
> +		report_prefix_pushf("0x%04x", subcodes[idx]);
> +		expect_pgm_int();
> +		asm volatile ("diag %0,%1,0x308" :: "a"(0), "a"(subcodes[idx]));
> +		check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +		report_prefix_pop();
> +	}
> +}
> +
> +static struct {
> +	const char *name;
> +	void (*func)(void);
> +} tests[] = {
> +	{ "privileged", test_priv },
> +	{ "subcode 1", test_subcode1 },
> +	{ "subcode 5", test_subcode5 },
> +	{ "subcode 6", test_subcode6 },
> +	{ "unsupported", test_unsupported_subcode },
> +	{ NULL, NULL }
> +};
> +
> +int main(int argc, char**argv)
> +{
> +	int i;
> +
> +	report_prefix_push("diag308");
> +	for (i = 0; tests[i].name; i++) {
> +		report_prefix_push(tests[i].name);
> +		tests[i].func();
> +		report_prefix_pop();
> +	}
> +	report_prefix_pop();
> +
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index e53c365..546b1f2 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -44,6 +44,9 @@ file = skey.elf
>  [diag10]
>  file = diag10.elf
>  
> +[diag308]
> +file = diag308.elf
> +
>  [pfmf]
>  file = pfmf.elf
>  
> 


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.

-- 

Thanks,

David / dhildenb