Re: [kvm-unit-tests PATCH 5/5] s390x: Add test for STFLE interpretive execution (format-0)

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Wed, 13 Dec 2023 13:49:42 +0100
Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:

> The STFLE instruction indicates installed facilities.
> SIE can interpretively execute STFLE.
> Use a snippet guest executing STFLE to get the result of
> interpretive execution and check the result.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

[...]

>  static inline void setup_facilities(void)
> diff --git a/s390x/snippets/c/stfle.c b/s390x/snippets/c/stfle.c
> new file mode 100644
> index 00000000..eb024a6a
> --- /dev/null
> +++ b/s390x/snippets/c/stfle.c
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright IBM Corp. 2023
> + *
> + * Snippet used by the STLFE interpretive execution facilities test.
> + */
> +#include <libcflat.h>
> +#include <snippet-guest.h>
> +
> +int main(void)
> +{
> +	const unsigned int max_fac_len = 8;

why 8?

> +	uint64_t res[max_fac_len + 1];
> +
> +	res[0] = max_fac_len - 1;
> +	asm volatile ( "lg	0,%[len]\n"
> +		"	stfle	%[fac]\n"
> +		"	stg	0,%[len]\n"
> +		: [fac] "=QS"(*(uint64_t(*)[max_fac_len])&res[1]),
> +		  [len] "+RT"(res[0])
> +		:
> +		: "%r0", "cc"
> +	);
> +	force_exit_value((uint64_t)&res);
> +	return 0;
> +}
> diff --git a/s390x/stfle-sie.c b/s390x/stfle-sie.c
> new file mode 100644
> index 00000000..574319ed
> --- /dev/null
> +++ b/s390x/stfle-sie.c
> @@ -0,0 +1,132 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright IBM Corp. 2023
> + *
> + * SIE with STLFE interpretive execution facilities test.
> + */
> +#include <libcflat.h>
> +#include <stdlib.h>
> +#include <asm/facility.h>
> +#include <asm/time.h>
> +#include <snippet-host.h>
> +#include <alloc_page.h>
> +#include <sclp.h>
> +
> +static struct vm vm;
> +static uint64_t (*fac)[PAGE_SIZE / sizeof(uint64_t)];
> +static rand_state rand_s;
> +
> +static void setup_guest(void)
> +{
> +	extern const char SNIPPET_NAME_START(c, stfle)[];
> +	extern const char SNIPPET_NAME_END(c, stfle)[];
> +
> +	setup_vm();
> +	fac = alloc_pages_flags(0, AREA_DMA31);
> +
> +	snippet_setup_guest(&vm, false);
> +	snippet_init(&vm, SNIPPET_NAME_START(c, stfle),
> +		     SNIPPET_LEN(c, stfle), SNIPPET_UNPACK_OFF);
> +}
> +
> +struct guest_stfle_res {
> +	uint16_t len;
> +	uint64_t reg;
> +	unsigned char *mem;
> +};
> +
> +static struct guest_stfle_res run_guest(void)
> +{
> +	struct guest_stfle_res res;
> +	uint64_t guest_stfle_addr;
> +
> +	sie(&vm);
> +	assert(snippet_get_force_exit_value(&vm, &guest_stfle_addr));
> +	res.mem = &vm.guest_mem[guest_stfle_addr];
> +	memcpy(&res.reg, res.mem, sizeof(res.reg));
> +	res.len = (res.reg & 0xff) + 1;
> +	res.mem += sizeof(res.reg);
> +	return res;
> +}
> +
> +static void test_stfle_format_0(void)
> +{
> +	struct guest_stfle_res res;
> +
> +	report_prefix_push("format-0");
> +	for (int j = 0; j < stfle_size(); j++)
> +		WRITE_ONCE((*fac)[j], rand64(&rand_s));

do you really need random numbers? can't you use a static pattern?
maybe something like 0x0001020304050607 etc...

> +	vm.sblk->fac = (uint32_t)(uint64_t)fac;
> +	res = run_guest();
> +	report(res.len == stfle_size(), "stfle len correct");
> +	report(!memcmp(*fac, res.mem, res.len * sizeof(uint64_t)),
> +	       "Guest facility list as specified");

it seems like you are comparing the full facility list (stfle_size
doublewords long) with the result of STFLE in the guest, but the guest
is limited to 8 double words?

> +	report_prefix_pop();
> +}
> +
> +struct args {
> +	uint64_t seed;
> +};
> +
> +static bool parse_uint64_t(const char *arg, uint64_t *out)
> +{
> +	char *end;
> +	uint64_t num;
> +
> +	if (arg[0] == '\0')
> +		return false;
> +	num = strtoul(arg, &end, 0);
> +	if (end[0] != '\0')
> +		return false;
> +	*out = num;
> +	return true;
> +}
> +
> +static struct args parse_args(int argc, char **argv)
> +{
> +	struct args args;
> +	const char *flag;
> +	unsigned int i;
> +	uint64_t arg;
> +	bool has_arg;
> +
> +	stck(&args.seed);
> +
> +	for (i = 1; i < argc; i++) {
> +		if (i + 1 < argc)
> +			has_arg = parse_uint64_t(argv[i + 1], &arg);
> +		else
> +			has_arg = false;
> +
> +		flag = "--seed";
> +		if (!strcmp(flag, argv[i])) {
> +			if (!has_arg)
> +				report_abort("%s needs an uint64_t parameter", flag);
> +			args.seed = arg;
> +			++i;
> +			continue;
> +		}
> +		report_abort("Unsupported parameter '%s'",
> +			     argv[i]);
> +	}
> +
> +	return args;
> +}

this is lots of repeated code in all tests, I should really resurrect
and polish my patchseries for argument parsing

> +
> +int main(int argc, char **argv)
> +{
> +	struct args args = parse_args(argc, argv);
> +
> +	if (!sclp_facilities.has_sief2) {
> +		report_skip("SIEF2 facility unavailable");
> +		goto out;
> +	}
> +
> +	report_info("pseudo rand seed: 0x%lx", args.seed);
> +	rand_s = RAND_STATE_INIT(args.seed);
> +	setup_guest();
> +	if (test_facility(7))
> +		test_stfle_format_0();
> +out:
> +	return report_summary();
> +}