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

Claudio Imbrenda
2023-12-13T17:00:57Z

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();
> +}