On 20.03.2018 12:19, Janosch Frank wrote:
> The Store Hypervisor Information (STHYI) instruction provides system
> information like available CPU resources on each system level. The
> instruction is fully emulated by the hypervisor.
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
> ---
> s390x/Makefile | 1 +
> s390x/sthyi.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> s390x/sthyi.h | 131 +++++++++++++++++++++++++++++
> s390x/unittests.cfg | 4 +
> 4 files changed, 368 insertions(+)
> create mode 100644 s390x/sthyi.c
> create mode 100644 s390x/sthyi.h
>
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 2d3336c..e062727 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -2,6 +2,7 @@ tests = $(TEST_DIR)/selftest.elf
> tests += $(TEST_DIR)/intercept.elf
> tests += $(TEST_DIR)/emulator.elf
> tests += $(TEST_DIR)/sieve.elf
> +tests += $(TEST_DIR)/sthyi.elf
>
> all: directories test_cases
>
> diff --git a/s390x/sthyi.c b/s390x/sthyi.c
> new file mode 100644
> index 0000000..a557b49
> --- /dev/null
> +++ b/s390x/sthyi.c
> @@ -0,0 +1,232 @@
> +/*
> + * Tests exceptions and data validity for the emulated sthyi
> + * instruction.
> + *
> + * Copyright 2018 IBM Corp.
> + *
> + * Authors:
> + * Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
> + *
> + * This code is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Library General Public License version 2.
> + */
> +#include <libcflat.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/interrupt.h>
> +#include <asm/page.h>
> +#include <asm/facility.h>
> +
> +#include "sthyi.h"
> +
> +static uint8_t pagebuf[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
> +static char null_buf[32] = {};
[...]
> +
> +static void test_exception_addr(void)
> +{
> + expect_pgm_int();
> + sthyi(42042, 0);
> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +}
> +
> +static void test_exception_reg_odd(void)
> +{
> + expect_pgm_int();
> + sthyi_ill_reg_odd1();
> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> + expect_pgm_int();
> + sthyi_ill_reg_odd2();
> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +}
> +
> +static void test_exception_reg_equal(void)
> +{
> + expect_pgm_int();
> + sthyi_ill_reg_eq();
> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +}
[...]
> +int main(void)
> +{
> + bool has_sthyi = test_facility(74);
> +
> + report_prefix_push("sthyi");
> +
> + /* Test for availability */
> + report_xfail("STHYI available", !has_sthyi, has_sthyi);
> + if (!has_sthyi)
> + goto done;
> +
> + /* Test register/argument checking. */
> + test_exception_addr();
> + report_prefix_push("Register check");
> + test_exception_reg_odd();
> + test_exception_reg_equal();
> + report_prefix_pop();
It's rather cosmetical, but in case you respin, I'd prefer if you could
move the report_prefix_push() and report_prefix_pop() statements into
the test_exception_reg_odd() and test_exception_reg_equal() functions
themselves, and also add them to test_exception_addr(), and use a
different prefix string in each of the functions. Then it will be much
easier to distinguish the tests in the output later.
Anyway, the rest of the patch looks fine to me, so with or without that
modification:
Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
