[kvm-unit-tests PATCH v2 1/5] s390x: Add specification exception test

[kvm-unit-tests PATCH v2 1/5] s390x: Add specification exception test

[Janis Schoetterl-Glausch]

Generate specification exceptions and check that they occur.
With the iterations argument one can check if specification
exception interpretation occurs, e.g. by using a high value and
checking that the debugfs counters are substantially lower.
The argument is also useful for estimating the performance benefit
of interpretation.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 s390x/Makefile      |   1 +
 s390x/spec_ex.c     | 182 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   3 +
 3 files changed, 186 insertions(+)
 create mode 100644 s390x/spec_ex.c

diff --git a/s390x/Makefile b/s390x/Makefile
index ef8041a..57d7c9e 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
 tests += $(TEST_DIR)/uv-host.elf
 tests += $(TEST_DIR)/edat.elf
 tests += $(TEST_DIR)/mvpg-sie.elf
+tests += $(TEST_DIR)/spec_ex.elf
 
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 ifneq ($(HOST_KEY_DOCUMENT),)
diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
new file mode 100644
index 0000000..dd0ee53
--- /dev/null
+++ b/s390x/spec_ex.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * © Copyright IBM Corp. 2021
+ *
+ * Specification exception test.
+ * Tests that specification exceptions occur when expected.
+ */
+#include <stdlib.h>
+#include <libcflat.h>
+#include <asm/interrupt.h>
+#include <asm/facility.h>
+
+static struct lowcore *lc = (struct lowcore *) 0;
+
+static bool expect_invalid_psw;
+static struct psw expected_psw;
+static struct psw fixup_psw;
+
+/* The standard program exception handler cannot deal with invalid old PSWs,
+ * especially not invalid instruction addresses, as in that case one cannot
+ * find the instruction following the faulting one from the old PSW.
+ * The PSW to return to is set by load_psw.
+ */
+static void fixup_invalid_psw(void)
+{
+	if (expect_invalid_psw) {
+		report(expected_psw.mask == lc->pgm_old_psw.mask
+		       && expected_psw.addr == lc->pgm_old_psw.addr,
+		       "Invalid program new PSW as expected");
+		expect_invalid_psw = false;
+	}
+	lc->pgm_old_psw = fixup_psw;
+}
+
+static void load_psw(struct psw psw)
+{
+	uint64_t r0 = 0, r1 = 0;
+
+	asm volatile (
+		"	epsw	%0,%1\n"
+		"	st	%0,%[mask]\n"
+		"	st	%1,4+%[mask]\n"
+		"	larl	%0,nop%=\n"
+		"	stg	%0,%[addr]\n"
+		"	lpswe	%[psw]\n"
+		"nop%=:	nop\n"
+		: "+&r"(r0), "+&a"(r1), [mask] "=&R"(fixup_psw.mask),
+		  [addr] "=&R"(fixup_psw.addr)
+		: [psw] "Q"(psw)
+		: "cc", "memory"
+	);
+}
+
+static void psw_bit_12_is_1(void)
+{
+	expected_psw.mask = 0x0008000000000000;
+	expected_psw.addr = 0x00000000deadbeee;
+	expect_invalid_psw = true;
+	load_psw(expected_psw);
+}
+
+static void bad_alignment(void)
+{
+	uint32_t words[5] = {0, 0, 0};
+	uint32_t (*bad_aligned)[4];
+
+	register uint64_t r1 asm("6");
+	register uint64_t r2 asm("7");
+	if (((uintptr_t)&words[0]) & 0xf)
+		bad_aligned = (uint32_t (*)[4])&words[0];
+	else
+		bad_aligned = (uint32_t (*)[4])&words[1];
+	asm volatile ("lpq %0,%2"
+		      : "=r"(r1), "=r"(r2)
+		      : "T"(*bad_aligned)
+	);
+}
+
+static void not_even(void)
+{
+	uint64_t quad[2];
+
+	register uint64_t r1 asm("7");
+	register uint64_t r2 asm("8");
+	asm volatile (".insn	rxy,0xe3000000008f,%0,%2" //lpq %0,%2
+		      : "=r"(r1), "=r"(r2)
+		      : "T"(quad)
+	);
+}
+
+struct spec_ex_trigger {
+	const char *name;
+	void (*func)(void);
+	void (*fixup)(void);
+};
+
+static const struct spec_ex_trigger spec_ex_triggers[] = {
+	{ "psw_bit_12_is_1", &psw_bit_12_is_1, &fixup_invalid_psw},
+	{ "bad_alignment", &bad_alignment, NULL},
+	{ "not_even", &not_even, NULL},
+	{ NULL, NULL, NULL},
+};
+
+struct args {
+	uint64_t iterations;
+};
+
+static void test_spec_ex(struct args *args,
+			 const struct spec_ex_trigger *trigger)
+{
+	uint16_t expected_pgm = PGM_INT_CODE_SPECIFICATION;
+	uint16_t pgm;
+	unsigned int i;
+
+	for (i = 0; i < args->iterations; i++) {
+		expect_pgm_int();
+		register_pgm_cleanup_func(trigger->fixup);
+		trigger->func();
+		register_pgm_cleanup_func(NULL);
+		pgm = clear_pgm_int();
+		if (pgm != expected_pgm) {
+			report(0,
+			       "Program interrupt: expected(%d) == received(%d)",
+			       expected_pgm,
+			       pgm);
+			return;
+		}
+	}
+	report(1,
+	       "Program interrupt: always expected(%d) == received(%d)",
+	       expected_pgm,
+	       expected_pgm);
+}
+
+static struct args parse_args(int argc, char **argv)
+{
+	struct args args = {
+		.iterations = 1,
+	};
+	unsigned int i;
+	long arg;
+	bool no_arg;
+	char *end;
+
+	for (i = 1; i < argc; i++) {
+		no_arg = true;
+		if (i < argc - 1) {
+			no_arg = *argv[i+1] == '\0';
+			arg = strtol(argv[i+1], &end, 10);
+			no_arg |= *end != '\0';
+			no_arg |= arg < 0;
+		}
+
+		if (!strcmp("--iterations", argv[i])) {
+			if (no_arg)
+				report_abort("--iterations needs a positive parameter");
+			args.iterations = arg;
+			++i;
+		} else {
+			report_abort("Unsupported parameter '%s'",
+				     argv[i]);
+		}
+	}
+	return args;
+}
+
+int main(int argc, char **argv)
+{
+	unsigned int i;
+
+	struct args args = parse_args(argc, argv);
+
+	report_prefix_push("specification exception");
+	for (i = 0; spec_ex_triggers[i].name; i++) {
+		report_prefix_push(spec_ex_triggers[i].name);
+		test_spec_ex(&args, &spec_ex_triggers[i]);
+		report_prefix_pop();
+	}
+	report_prefix_pop();
+
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 9e1802f..5f43d52 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -109,3 +109,6 @@ file = edat.elf
 
 [mvpg-sie]
 file = mvpg-sie.elf
+
+[spec_ex]
+file = spec_ex.elf
-- 
2.31.1

Re: [kvm-unit-tests PATCH v2 1/5] [kvm-unit-tests PATCH v2 0/5] Add specification exception tests

[Janis Schoetterl-Glausch]

Oops, forgot to Cc the lists on the cover letter, see below.


Test that specification exceptions cause the correct interruption code
during both normal and transactional execution.

The last three patches are cosmetic only and could be dropped.

Unrelated: There should not be * in the file patterns in MAINTAINERS,
should there?

v1 -> v2
	Add license and test description
	Split test patch into normal test and transactional execution test
	Add comments to
		invalid PSW fixup function
		with_transaction
	Rename some variables/functions
	Pass mask as single parameter to asm
	Get rid of report_info_if macro
	Introduce report_pass/fail and use them

Janis Schoetterl-Glausch (5):
  s390x: Add specification exception test
  s390x: Test specification exceptions during transaction
  lib: Introduce report_pass and report_fail
  Use report_fail(...) instead of report(0/false, ...)
  Use report_pass(...) instead of report(1/true, ...)

 s390x/Makefile           |   1 +
 lib/s390x/asm/arch_def.h |   1 +
 lib/libcflat.h           |   6 +-
 lib/report.c             |  20 ++-
 lib/s390x/css_lib.c      |  30 ++--
 x86/vmx.h                |  31 ++--
 arm/psci.c               |   2 +-
 arm/timer.c              |   2 +-
 s390x/css.c              |  22 +--
 s390x/diag288.c          |   2 +-
 s390x/selftest.c         |   2 +-
 s390x/smp.c              |  16 +-
 s390x/spec_ex.c          | 345 +++++++++++++++++++++++++++++++++++++++
 x86/asyncpf.c            |  11 +-
 x86/emulator.c           |   2 +-
 x86/hyperv_stimer.c      |  24 ++-
 x86/hyperv_synic.c       |   2 +-
 x86/svm_tests.c          | 180 ++++++++++----------
 x86/syscall.c            |   2 +-
 x86/taskswitch2.c        |   2 +-
 x86/tsc_adjust.c         |   2 +-
 x86/vmx.c                |  23 ++-
 x86/vmx_tests.c          | 172 ++++++++++---------
 s390x/unittests.cfg      |   3 +
 24 files changed, 630 insertions(+), 273 deletions(-)
 create mode 100644 s390x/spec_ex.c

base-commit: fe26131eec769cef7ad7e2e1e4e192aa0bdb2bba
-- 
2.31.1

Re: [kvm-unit-tests PATCH v2 1/5] s390x: Add specification exception test

[Janis Schoetterl-Glausch]

On 10/5/21 1:56 PM, Janosch Frank wrote:
> On 10/5/21 11:09, Janis Schoetterl-Glausch wrote:
>> Generate specification exceptions and check that they occur.
>> With the iterations argument one can check if specification
>> exception interpretation occurs, e.g. by using a high value and
>> checking that the debugfs counters are substantially lower.
>> The argument is also useful for estimating the performance benefit
>> of interpretation.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> ---
>>   s390x/Makefile      |   1 +
>>   s390x/spec_ex.c     | 182 ++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg |   3 +
>>   3 files changed, 186 insertions(+)
>>   create mode 100644 s390x/spec_ex.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ef8041a..57d7c9e 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>>   tests += $(TEST_DIR)/uv-host.elf
>>   tests += $(TEST_DIR)/edat.elf
>>   tests += $(TEST_DIR)/mvpg-sie.elf
>> +tests += $(TEST_DIR)/spec_ex.elf
>>     tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>   ifneq ($(HOST_KEY_DOCUMENT),)
>> diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
>> new file mode 100644
>> index 0000000..dd0ee53
>> --- /dev/null
>> +++ b/s390x/spec_ex.c
>> @@ -0,0 +1,182 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * © Copyright IBM Corp. 2021
>> + *
>> + * Specification exception test.
>> + * Tests that specification exceptions occur when expected.
>> + */
>> +#include <stdlib.h>
>> +#include <libcflat.h>
>> +#include <asm/interrupt.h>
>> +#include <asm/facility.h>
>> +
>> +static struct lowcore *lc = (struct lowcore *) 0;
>> +
>> +static bool expect_invalid_psw;
>> +static struct psw expected_psw;
>> +static struct psw fixup_psw;
>> +
>> +/* The standard program exception handler cannot deal with invalid old PSWs,
>> + * especially not invalid instruction addresses, as in that case one cannot
>> + * find the instruction following the faulting one from the old PSW.
>> + * The PSW to return to is set by load_psw.
>> + */
>> +static void fixup_invalid_psw(void)
>> +{
>> +    if (expect_invalid_psw) {
>> +        report(expected_psw.mask == lc->pgm_old_psw.mask
>> +               && expected_psw.addr == lc->pgm_old_psw.addr,
>> +               "Invalid program new PSW as expected");
>> +        expect_invalid_psw = false;
>> +    }
>> +    lc->pgm_old_psw = fixup_psw;
>> +}
>> +
>> +static void load_psw(struct psw psw)
>> +{
>> +    uint64_t r0 = 0, r1 = 0;
>> +
>> +    asm volatile (
>> +        "    epsw    %0,%1\n"
>> +        "    st    %0,%[mask]\n"
>> +        "    st    %1,4+%[mask]\n"
> 
> You're grabbing the mask for the fixup psw, right?

Yes

> Why don't you use the extract_psw_mask() function for that?

No reason, sounds like a good idea to use the function.
> 
> Also I'd recommend not mixing named operands and numeric operands, especially when the variables are then called r0 and r1.

I suppose I didn't name them because they're just scratch registers.
But using extract_psw_mask() will get rid of them anyway
> 
>> +        "    larl    %0,nop%=\n"
>> +        "    stg    %0,%[addr]\n"
> 
> This stores the address of the nop to the fixup psw addr.
> So far so good, but why is it only called "addr"?
> 
>> +        "    lpswe    %[psw]\n"
>> +        "nop%=:    nop\n"
>> +        : "+&r"(r0), "+&a"(r1), [mask] "=&R"(fixup_psw.mask),
>> +          [addr] "=&R"(fixup_psw.addr)
>> +        : [psw] "Q"(psw)
>> +        : "cc", "memory"
>> +    );
> 
> You made this a bit complicated and didn't document it.
> /*
>  * Setup fixup_psw before loading an invalid PSW so that *fixup_invalid_psw() can bring us back onto the right track.
>  */
> >> +}
>> +
>> +static void psw_bit_12_is_1(void)
>> +{
>> +    expected_psw.mask = 0x0008000000000000;
>> +    expected_psw.addr = 0x00000000deadbeee;
>> +    expect_invalid_psw = true;
>> +    load_psw(expected_psw);
>> +}
>> +

[...]

Re: [kvm-unit-tests PATCH v2 1/5] s390x: Add specification exception test

[Thomas Huth]

On 05/10/2021 11.09, Janis Schoetterl-Glausch wrote:
> Generate specification exceptions and check that they occur.
> With the iterations argument one can check if specification
> exception interpretation occurs, e.g. by using a high value and
> checking that the debugfs counters are substantially lower.
> The argument is also useful for estimating the performance benefit
> of interpretation.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>   s390x/Makefile      |   1 +
>   s390x/spec_ex.c     | 182 ++++++++++++++++++++++++++++++++++++++++++++
>   s390x/unittests.cfg |   3 +
>   3 files changed, 186 insertions(+)
>   create mode 100644 s390x/spec_ex.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ef8041a..57d7c9e 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>   tests += $(TEST_DIR)/uv-host.elf
>   tests += $(TEST_DIR)/edat.elf
>   tests += $(TEST_DIR)/mvpg-sie.elf
> +tests += $(TEST_DIR)/spec_ex.elf
>   
>   tests_binary = $(patsubst %.elf,%.bin,$(tests))
>   ifneq ($(HOST_KEY_DOCUMENT),)
> diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
> new file mode 100644
> index 0000000..dd0ee53
> --- /dev/null
> +++ b/s390x/spec_ex.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * © Copyright IBM Corp. 2021

Could we please avoid non-ASCII characters in source code if possible? ... 
it's maybe best if you do the Copyright line similar to the other *.c files 
from IBM that are already in the repository.

> + * Specification exception test.
> + * Tests that specification exceptions occur when expected.
> + */
> +#include <stdlib.h>
> +#include <libcflat.h>
> +#include <asm/interrupt.h>
> +#include <asm/facility.h>
> +
> +static struct lowcore *lc = (struct lowcore *) 0;
> +
> +static bool expect_invalid_psw;
> +static struct psw expected_psw;
> +static struct psw fixup_psw;
> +
> +/* The standard program exception handler cannot deal with invalid old PSWs,
> + * especially not invalid instruction addresses, as in that case one cannot
> + * find the instruction following the faulting one from the old PSW.
> + * The PSW to return to is set by load_psw.
> + */
> +static void fixup_invalid_psw(void)
> +{
> +	if (expect_invalid_psw) {
> +		report(expected_psw.mask == lc->pgm_old_psw.mask
> +		       && expected_psw.addr == lc->pgm_old_psw.addr,
> +		       "Invalid program new PSW as expected");
> +		expect_invalid_psw = false;
> +	}
> +	lc->pgm_old_psw = fixup_psw;
> +}
> +
> +static void load_psw(struct psw psw)
> +{
> +	uint64_t r0 = 0, r1 = 0;
> +
> +	asm volatile (
> +		"	epsw	%0,%1\n"
> +		"	st	%0,%[mask]\n"
> +		"	st	%1,4+%[mask]\n"
> +		"	larl	%0,nop%=\n"
> +		"	stg	%0,%[addr]\n"
> +		"	lpswe	%[psw]\n"
> +		"nop%=:	nop\n"
> +		: "+&r"(r0), "+&a"(r1), [mask] "=&R"(fixup_psw.mask),
> +		  [addr] "=&R"(fixup_psw.addr)

stg uses long displacement, so maybe the constraint should rather be "T" 
instead?

> +		: [psw] "Q"(psw)
> +		: "cc", "memory"
> +	);
> +}
> +
> +static void psw_bit_12_is_1(void)
> +{
> +	expected_psw.mask = 0x0008000000000000;
> +	expected_psw.addr = 0x00000000deadbeee;
> +	expect_invalid_psw = true;
> +	load_psw(expected_psw);
> +}
> +
> +static void bad_alignment(void)
> +{
> +	uint32_t words[5] = {0, 0, 0};
> +	uint32_t (*bad_aligned)[4];
> +
> +	register uint64_t r1 asm("6");
> +	register uint64_t r2 asm("7");
> +	if (((uintptr_t)&words[0]) & 0xf)
> +		bad_aligned = (uint32_t (*)[4])&words[0];
> +	else
> +		bad_aligned = (uint32_t (*)[4])&words[1];
> +	asm volatile ("lpq %0,%2"
> +		      : "=r"(r1), "=r"(r2)
> +		      : "T"(*bad_aligned)
> +	);
> +}
> +
> +static void not_even(void)
> +{
> +	uint64_t quad[2];
> +
> +	register uint64_t r1 asm("7");
> +	register uint64_t r2 asm("8");
> +	asm volatile (".insn	rxy,0xe3000000008f,%0,%2" //lpq %0,%2
> +		      : "=r"(r1), "=r"(r2)
> +		      : "T"(quad)
> +	);
> +}
> +
> +struct spec_ex_trigger {
> +	const char *name;
> +	void (*func)(void);
> +	void (*fixup)(void);
> +};
> +
> +static const struct spec_ex_trigger spec_ex_triggers[] = {
> +	{ "psw_bit_12_is_1", &psw_bit_12_is_1, &fixup_invalid_psw},
> +	{ "bad_alignment", &bad_alignment, NULL},
> +	{ "not_even", &not_even, NULL},
> +	{ NULL, NULL, NULL},
> +};
> +
> +struct args {
> +	uint64_t iterations;
> +};
> +
> +static void test_spec_ex(struct args *args,
> +			 const struct spec_ex_trigger *trigger)
> +{
> +	uint16_t expected_pgm = PGM_INT_CODE_SPECIFICATION;
> +	uint16_t pgm;
> +	unsigned int i;
> +
> +	for (i = 0; i < args->iterations; i++) {
> +		expect_pgm_int();
> +		register_pgm_cleanup_func(trigger->fixup);
> +		trigger->func();
> +		register_pgm_cleanup_func(NULL);
> +		pgm = clear_pgm_int();
> +		if (pgm != expected_pgm) {
> +			report(0,
> +			       "Program interrupt: expected(%d) == received(%d)",
> +			       expected_pgm,
> +			       pgm);
> +			return;
> +		}
> +	}
> +	report(1,
> +	       "Program interrupt: always expected(%d) == received(%d)",
> +	       expected_pgm,
> +	       expected_pgm);
> +}
> +
> +static struct args parse_args(int argc, char **argv)
> +{
> +	struct args args = {
> +		.iterations = 1,
> +	};
> +	unsigned int i;
> +	long arg;
> +	bool no_arg;
> +	char *end;
> +
> +	for (i = 1; i < argc; i++) {
> +		no_arg = true;
> +		if (i < argc - 1) {
> +			no_arg = *argv[i+1] == '\0';
> +			arg = strtol(argv[i+1], &end, 10);

Nit: It's more common to use spaces around the "+" (i.e. "i + 1")

> +			no_arg |= *end != '\0';
> +			no_arg |= arg < 0;
> +		}
> +
> +		if (!strcmp("--iterations", argv[i])) {
> +			if (no_arg)
> +				report_abort("--iterations needs a positive parameter");
> +			args.iterations = arg;
> +			++i;
> +		} else {
> +			report_abort("Unsupported parameter '%s'",
> +				     argv[i]);
> +		}
> +	}
> +	return args;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	unsigned int i;
> +
> +	struct args args = parse_args(argc, argv);
> +
> +	report_prefix_push("specification exception");
> +	for (i = 0; spec_ex_triggers[i].name; i++) {
> +		report_prefix_push(spec_ex_triggers[i].name);
> +		test_spec_ex(&args, &spec_ex_triggers[i]);
> +		report_prefix_pop();
> +	}
> +	report_prefix_pop();
> +
> +	return report_summary();
> +}

Apart from the nits, this looks fine to me.

  Thomas

Re: [kvm-unit-tests PATCH v2 1/5] s390x: Add specification exception test

[Janis Schoetterl-Glausch]

On 10/5/21 4:51 PM, Thomas Huth wrote:
> On 05/10/2021 11.09, Janis Schoetterl-Glausch wrote:
>> Generate specification exceptions and check that they occur.
>> With the iterations argument one can check if specification
>> exception interpretation occurs, e.g. by using a high value and
>> checking that the debugfs counters are substantially lower.
>> The argument is also useful for estimating the performance benefit
>> of interpretation.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> ---
>>   s390x/Makefile      |   1 +
>>   s390x/spec_ex.c     | 182 ++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg |   3 +
>>   3 files changed, 186 insertions(+)
>>   create mode 100644 s390x/spec_ex.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ef8041a..57d7c9e 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>>   tests += $(TEST_DIR)/uv-host.elf
>>   tests += $(TEST_DIR)/edat.elf
>>   tests += $(TEST_DIR)/mvpg-sie.elf
>> +tests += $(TEST_DIR)/spec_ex.elf
>>     tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>   ifneq ($(HOST_KEY_DOCUMENT),)
>> diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
>> new file mode 100644
>> index 0000000..dd0ee53
>> --- /dev/null
>> +++ b/s390x/spec_ex.c
>> @@ -0,0 +1,182 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * © Copyright IBM Corp. 2021
> 
> Could we please avoid non-ASCII characters in source code if possible? ... it's maybe best if you do the Copyright line similar to the other *.c files from IBM that are already in the repository.

Yes, I'll remove it. I thought it would be fine since it's in a comment,
didn't consider that it might cause trouble with some mail clients.
So that's grounds for removal by itself.
> 
>> + * Specification exception test.
>> + * Tests that specification exceptions occur when expected.
>> + */
>> +#include <stdlib.h>
>> +#include <libcflat.h>
>> +#include <asm/interrupt.h>
>> +#include <asm/facility.h>
>> +
>> +static struct lowcore *lc = (struct lowcore *) 0;
>> +
>> +static bool expect_invalid_psw;
>> +static struct psw expected_psw;
>> +static struct psw fixup_psw;
>> +
>> +/* The standard program exception handler cannot deal with invalid old PSWs,
>> + * especially not invalid instruction addresses, as in that case one cannot
>> + * find the instruction following the faulting one from the old PSW.
>> + * The PSW to return to is set by load_psw.
>> + */
>> +static void fixup_invalid_psw(void)
>> +{
>> +    if (expect_invalid_psw) {
>> +        report(expected_psw.mask == lc->pgm_old_psw.mask
>> +               && expected_psw.addr == lc->pgm_old_psw.addr,
>> +               "Invalid program new PSW as expected");
>> +        expect_invalid_psw = false;
>> +    }
>> +    lc->pgm_old_psw = fixup_psw;
>> +}
>> +
>> +static void load_psw(struct psw psw)
>> +{
>> +    uint64_t r0 = 0, r1 = 0;
>> +
>> +    asm volatile (
>> +        "    epsw    %0,%1\n"
>> +        "    st    %0,%[mask]\n"
>> +        "    st    %1,4+%[mask]\n"
>> +        "    larl    %0,nop%=\n"
>> +        "    stg    %0,%[addr]\n"
>> +        "    lpswe    %[psw]\n"
>> +        "nop%=:    nop\n"
>> +        : "+&r"(r0), "+&a"(r1), [mask] "=&R"(fixup_psw.mask),
>> +          [addr] "=&R"(fixup_psw.addr)
> 
> stg uses long displacement, so maybe the constraint should rather be "T" instead?

Good catch.
> 
>> +        : [psw] "Q"(psw)
>> +        : "cc", "memory"
>> +    );
>> +}
>> +

[...]

>> +static struct args parse_args(int argc, char **argv)
>> +{
>> +    struct args args = {
>> +        .iterations = 1,
>> +    };
>> +    unsigned int i;
>> +    long arg;
>> +    bool no_arg;
>> +    char *end;
>> +
>> +    for (i = 1; i < argc; i++) {
>> +        no_arg = true;
>> +        if (i < argc - 1) {
>> +            no_arg = *argv[i+1] == '\0';
>> +            arg = strtol(argv[i+1], &end, 10);
> 
> Nit: It's more common to use spaces around the "+" (i.e. "i + 1")

Ok
> 
>> +            no_arg |= *end != '\0';
>> +            no_arg |= arg < 0;
>> +        }
>> +
>> +        if (!strcmp("--iterations", argv[i])) {
>> +            if (no_arg)
>> +                report_abort("--iterations needs a positive parameter");
>> +            args.iterations = arg;
>> +            ++i;
>> +        } else {
>> +            report_abort("Unsupported parameter '%s'",
>> +                     argv[i]);
>> +        }
>> +    }
>> +    return args;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +    unsigned int i;
>> +
>> +    struct args args = parse_args(argc, argv);
>> +
>> +    report_prefix_push("specification exception");
>> +    for (i = 0; spec_ex_triggers[i].name; i++) {
>> +        report_prefix_push(spec_ex_triggers[i].name);
>> +        test_spec_ex(&args, &spec_ex_triggers[i]);
>> +        report_prefix_pop();
>> +    }
>> +    report_prefix_pop();
>> +
>> +    return report_summary();
>> +}
> 
> Apart from the nits, this looks fine to me.

Thanks for the review.
> 
>  Thomas
> 

[kvm-unit-tests PATCH v2 2/5] s390x: Test specification exceptions during transaction

[Janis Schoetterl-Glausch]

Program interruptions during transactional execution cause other
interruption codes.
Check that we see the expected code for (some) specification exceptions.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h |   1 +
 s390x/spec_ex.c          | 175 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 171 insertions(+), 5 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 302ef1f..665a4b2 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -235,6 +235,7 @@ static inline uint64_t stctg(int cr)
 	return value;
 }
 
+#define CTL0_TRANSACT_EX_CTL	(63 -  8)
 #define CTL0_LOW_ADDR_PROT	(63 - 35)
 #define CTL0_EDAT		(63 - 40)
 #define CTL0_IEP		(63 - 43)
diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
index dd0ee53..eaf48f4 100644
--- a/s390x/spec_ex.c
+++ b/s390x/spec_ex.c
@@ -4,9 +4,14 @@
  *
  * Specification exception test.
  * Tests that specification exceptions occur when expected.
+ * This includes specification exceptions occurring during transactional execution
+ * as these result in another interruption code (the transactional-execution-aborted
+ * bit is set).
  */
 #include <stdlib.h>
+#include <htmintrin.h>
 #include <libcflat.h>
+#include <asm/barrier.h>
 #include <asm/interrupt.h>
 #include <asm/facility.h>
 
@@ -91,18 +96,23 @@ static void not_even(void)
 struct spec_ex_trigger {
 	const char *name;
 	void (*func)(void);
+	bool transactable;
 	void (*fixup)(void);
 };
 
 static const struct spec_ex_trigger spec_ex_triggers[] = {
-	{ "psw_bit_12_is_1", &psw_bit_12_is_1, &fixup_invalid_psw},
-	{ "bad_alignment", &bad_alignment, NULL},
-	{ "not_even", &not_even, NULL},
-	{ NULL, NULL, NULL},
+	{ "psw_bit_12_is_1", &psw_bit_12_is_1, false, &fixup_invalid_psw},
+	{ "bad_alignment", &bad_alignment, true, NULL},
+	{ "not_even", &not_even, true, NULL},
+	{ NULL, NULL, true, NULL},
 };
 
 struct args {
 	uint64_t iterations;
+	uint64_t max_retries;
+	uint64_t suppress_info;
+	uint64_t max_failures;
+	bool diagnose;
 };
 
 static void test_spec_ex(struct args *args,
@@ -132,14 +142,135 @@ static void test_spec_ex(struct args *args,
 	       expected_pgm);
 }
 
+#define TRANSACTION_COMPLETED 4
+#define TRANSACTION_MAX_RETRIES 5
+
+/* NULL must be passed to __builtin_tbegin via constant, forbid diagnose from
+ * being NULL to keep things simple
+ */
+static int __attribute__((nonnull))
+with_transaction(void (*trigger)(void), struct __htm_tdb *diagnose)
+{
+	int cc;
+
+	cc = __builtin_tbegin(diagnose);
+	if (cc == _HTM_TBEGIN_STARTED) {
+		trigger();
+		__builtin_tend();
+		return -TRANSACTION_COMPLETED;
+	} else {
+		return -cc;
+	}
+}
+
+static int retry_transaction(const struct spec_ex_trigger *trigger, unsigned int max_retries,
+			     struct __htm_tdb *tdb, uint16_t expected_pgm)
+{
+	int trans_result, i;
+	uint16_t pgm;
+
+	for (i = 0; i < max_retries; i++) {
+		expect_pgm_int();
+		trans_result = with_transaction(trigger->func, tdb);
+		if (trans_result == -_HTM_TBEGIN_TRANSIENT) {
+			mb();
+			pgm = lc->pgm_int_code;
+			if (pgm == 0)
+				continue;
+			else if (pgm == expected_pgm)
+				return 0;
+		}
+		return trans_result;
+	}
+	return -TRANSACTION_MAX_RETRIES;
+}
+
+static void test_spec_ex_trans(struct args *args, const struct spec_ex_trigger *trigger)
+{
+	const uint16_t expected_pgm = PGM_INT_CODE_SPECIFICATION
+			      | PGM_INT_CODE_TX_ABORTED_EVENT;
+	union {
+		struct __htm_tdb tdb;
+		uint64_t dwords[sizeof(struct __htm_tdb) / sizeof(uint64_t)];
+	} diag;
+	unsigned int i, failures = 0;
+	int trans_result;
+
+	if (!test_facility(73)) {
+		report_skip("transactional-execution facility not installed");
+		return;
+	}
+	ctl_set_bit(0, CTL0_TRANSACT_EX_CTL); /* enable transactional-exec */
+
+	for (i = 0; i < args->iterations && failures <= args->max_failures; i++) {
+		register_pgm_cleanup_func(trigger->fixup);
+		trans_result = retry_transaction(trigger, args->max_retries, &diag.tdb, expected_pgm);
+		register_pgm_cleanup_func(NULL);
+		switch (trans_result) {
+		case 0:
+			continue;
+		case -_HTM_TBEGIN_INDETERMINATE:
+		case -_HTM_TBEGIN_PERSISTENT:
+			if (failures < args->suppress_info)
+				report_info("transaction failed with cc %d",
+					    -trans_result);
+			break;
+		case -_HTM_TBEGIN_TRANSIENT:
+			report(0,
+			       "Program interrupt: expected(%d) == received(%d)",
+			       expected_pgm,
+			       clear_pgm_int());
+			goto out;
+		case -TRANSACTION_COMPLETED:
+			report(0,
+			       "Transaction completed without exception");
+			goto out;
+		case -TRANSACTION_MAX_RETRIES:
+			if (failures < args->suppress_info)
+				report_info("Retried transaction %lu times without exception",
+					    args->max_retries);
+			break;
+		default:
+			report(0, "Invalid return transaction result");
+			goto out;
+		}
+
+		if (failures < args->suppress_info)
+			report_info("transaction abort code: %llu", diag.tdb.abort_code);
+		if (args->diagnose && failures < args->suppress_info) {
+			for (i = 0; i < 32; i++)
+				report_info("diag+%03d: %016lx", i*8, diag.dwords[i]);
+		}
+		++failures;
+	}
+	if (failures <= args->max_failures) {
+		report(1,
+		       "Program interrupt: always expected(%d) == received(%d), transaction failures: %u",
+		       expected_pgm,
+		       expected_pgm,
+		       failures);
+	} else {
+		report(0,
+		       "Too many transaction failures: %u", failures);
+	}
+	if (failures > args->suppress_info)
+		report_info("Suppressed some transaction failure information messages");
+
+out:
+	ctl_clear_bit(0, CTL0_TRANSACT_EX_CTL);
+}
+
 static struct args parse_args(int argc, char **argv)
 {
 	struct args args = {
 		.iterations = 1,
+		.max_retries = 20,
+		.suppress_info = 20,
+		.diagnose = false
 	};
 	unsigned int i;
 	long arg;
-	bool no_arg;
+	bool no_arg, max_failures = false;
 	char *end;
 
 	for (i = 1; i < argc; i++) {
@@ -156,11 +287,35 @@ static struct args parse_args(int argc, char **argv)
 				report_abort("--iterations needs a positive parameter");
 			args.iterations = arg;
 			++i;
+		} else if (!strcmp("--max-retries", argv[i])) {
+			if (no_arg)
+				report_abort("--max-retries needs a positive parameter");
+			args.max_retries = arg;
+			++i;
+		} else if (!strcmp("--suppress-info", argv[i])) {
+			if (no_arg)
+				report_abort("--suppress-info needs a positive parameter");
+			args.suppress_info = arg;
+			++i;
+		} else if (!strcmp("--max-failures", argv[i])) {
+			if (no_arg)
+				report_abort("--max-failures needs a positive parameter");
+			args.max_failures = arg;
+			max_failures = true;
+			++i;
+		} else if (!strcmp("--diagnose", argv[i])) {
+			args.diagnose = true;
+		} else if (!strcmp("--no-diagnose", argv[i])) {
+			args.diagnose = false;
 		} else {
 			report_abort("Unsupported parameter '%s'",
 				     argv[i]);
 		}
 	}
+
+	if (!max_failures)
+		args.max_failures = args.iterations / 1000;
+
 	return args;
 }
 
@@ -178,5 +333,15 @@ int main(int argc, char **argv)
 	}
 	report_prefix_pop();
 
+	report_prefix_push("specification exception during transaction");
+	for (i = 0; spec_ex_triggers[i].name; i++) {
+		if (spec_ex_triggers[i].transactable) {
+			report_prefix_push(spec_ex_triggers[i].name);
+			test_spec_ex_trans(&args, &spec_ex_triggers[i]);
+			report_prefix_pop();
+		}
+	}
+	report_prefix_pop();
+
 	return report_summary();
 }
-- 
2.31.1

[kvm-unit-tests PATCH v2 3/5] lib: Introduce report_pass and report_fail

[Janis Schoetterl-Glausch]

These functions can be used instead of report(1/true/0/false, ...)
and read a bit nicer.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
There is probably a better name than report_passed() for the function
without a message...

 lib/libcflat.h |  6 +++++-
 lib/report.c   | 20 +++++++++++++++++++-
 x86/vmx.h      |  6 +++---
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 39f4552..9bb7e08 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -106,7 +106,11 @@ extern void report_skip(const char *msg_fmt, ...)
 					__attribute__((format(printf, 1, 2)));
 extern void report_info(const char *msg_fmt, ...)
 					__attribute__((format(printf, 1, 2)));
-extern void report_pass(void);
+extern void report_pass(const char *msg_fmt, ...)
+					__attribute__((format(printf, 1, 2)));
+extern void report_fail(const char *msg_fmt, ...)
+					__attribute__((format(printf, 1, 2)));
+extern void report_passed(void);
 extern int report_summary(void);
 
 bool simple_glob(const char *text, const char *pattern);
diff --git a/lib/report.c b/lib/report.c
index 2255dc3..8e9bff5 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -19,7 +19,7 @@ static struct spinlock lock;
 
 #define PREFIX_DELIMITER ": "
 
-void report_pass(void)
+void report_passed(void)
 {
 	spin_lock(&lock);
 	tests++;
@@ -112,6 +112,24 @@ void report(bool pass, const char *msg_fmt, ...)
 	va_end(va);
 }
 
+void report_pass(const char *msg_fmt, ...)
+{
+	va_list va;
+
+	va_start(va, msg_fmt);
+	va_report(msg_fmt, true, false, false, va);
+	va_end(va);
+}
+
+void report_fail(const char *msg_fmt, ...)
+{
+	va_list va;
+
+	va_start(va, msg_fmt);
+	va_report(msg_fmt, false, false, false, va);
+	va_end(va);
+}
+
 void report_xfail(bool xfail, bool pass, const char *msg_fmt, ...)
 {
 	va_list va;
diff --git a/x86/vmx.h b/x86/vmx.h
index ebd014c..fd0174a 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -928,7 +928,7 @@ do { \
 		dump_stack(); \
 		__abort_test(); \
 	} \
-	report_pass(); \
+	report_passed(); \
 } while (0)
 
 #define TEST_ASSERT_MSG(cond, fmt, args...) \
@@ -939,7 +939,7 @@ do { \
 		dump_stack(); \
 		__abort_test(); \
 	} \
-	report_pass(); \
+	report_passed(); \
 } while (0)
 
 #define __TEST_EQ(a, b, a_str, b_str, assertion, fmt, args...) \
@@ -964,7 +964,7 @@ do { \
 		if (assertion) \
 			__abort_test(); \
 	} \
-	report_pass(); \
+	report_passed(); \
 } while (0)
 
 #define TEST_ASSERT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 1, "")
-- 
2.31.1

[kvm-unit-tests PATCH v2 4/5] Use report_fail(...) instead of report(0/false, ...)

[Janis Schoetterl-Glausch]

Whitespace is kept consistent with the rest of the file.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 lib/s390x/css_lib.c |  30 ++++----
 x86/vmx.h           |  25 ++++---
 arm/psci.c          |   2 +-
 arm/timer.c         |   2 +-
 s390x/css.c         |  18 ++---
 s390x/spec_ex.c     |   7 +-
 x86/asyncpf.c       |   4 +-
 x86/hyperv_stimer.c |   6 +-
 x86/hyperv_synic.c  |   2 +-
 x86/svm_tests.c     | 163 ++++++++++++++++++++++----------------------
 x86/vmx.c           |  17 +++--
 x86/vmx_tests.c     | 136 ++++++++++++++++++------------------
 12 files changed, 200 insertions(+), 212 deletions(-)

diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index efc7057..80e9e07 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -360,9 +360,8 @@ void css_irq_io(void)
 	sid = lowcore_ptr->subsys_id_word;
 	/* Lowlevel set the SID as interrupt parameter. */
 	if (lowcore_ptr->io_int_param != sid) {
-		report(0,
-		       "io_int_param: %x differs from subsys_id_word: %x",
-		       lowcore_ptr->io_int_param, sid);
+		report_fail("io_int_param: %x differs from subsys_id_word: %x",
+			    lowcore_ptr->io_int_param, sid);
 		goto pop;
 	}
 	report_prefix_pop();
@@ -373,15 +372,14 @@ void css_irq_io(void)
 	case 1:
 		dump_irb(&irb);
 		flags = dump_scsw_flags(irb.scsw.ctrl);
-		report(0,
-		       "I/O interrupt, but tsch returns CC 1 for subchannel %08x. SCSW flags: %s",
-		       sid, flags);
+		report_fail("I/O interrupt, but tsch returns CC 1 for subchannel %08x.SCSW flags: %s",
+			    sid, flags);
 		break;
 	case 2:
-		report(0, "tsch returns unexpected CC 2");
+		report_fail("tsch returns unexpected CC 2");
 		break;
 	case 3:
-		report(0, "tsch reporting sch %08x as not operational", sid);
+		report_fail("tsch reporting sch %08x as not operational", sid);
 		break;
 	case 0:
 		/* Stay humble on success */
@@ -435,30 +433,30 @@ int wait_and_check_io_completion(int schid)
 	report_prefix_push("check I/O completion");
 
 	if (lowcore_ptr->io_int_param != schid) {
-		report(0, "interrupt parameter: expected %08x got %08x",
-		       schid, lowcore_ptr->io_int_param);
+		report_fail("interrupt parameter: expected %08x got %08x",
+			    schid, lowcore_ptr->io_int_param);
 		ret = -1;
 		goto end;
 	}
 
 	/* Verify that device status is valid */
 	if (!(irb.scsw.ctrl & SCSW_SC_PENDING)) {
-		report(0, "No status pending after interrupt. Subch Ctrl: %08x",
-		       irb.scsw.ctrl);
+		report_fail("No status pending after interrupt. Subch Ctrl: %08x",
+			    irb.scsw.ctrl);
 		ret = -1;
 		goto end;
 	}
 
 	if (!(irb.scsw.ctrl & (SCSW_SC_SECONDARY | SCSW_SC_PRIMARY))) {
-		report(0, "Primary or secondary status missing. Subch Ctrl: %08x",
-		       irb.scsw.ctrl);
+		report_fail("Primary or secondary status missing. Subch Ctrl: %08x",
+			    irb.scsw.ctrl);
 		ret = -1;
 		goto end;
 	}
 
 	if (!(irb.scsw.dev_stat & (SCSW_DEVS_DEV_END | SCSW_DEVS_SCH_END))) {
-		report(0, "No device end or sch end. Dev. status: %02x",
-		       irb.scsw.dev_stat);
+		report_fail("No device end or sch end. Dev. status: %02x",
+			    irb.scsw.dev_stat);
 		ret = -1;
 		goto end;
 	}
diff --git a/x86/vmx.h b/x86/vmx.h
index fd0174a..dd869c2 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -923,8 +923,8 @@ void __abort_test(void);
 #define TEST_ASSERT(cond) \
 do { \
 	if (!(cond)) { \
-		report(0, "%s:%d: Assertion failed: %s", \
-		       __FILE__, __LINE__, #cond); \
+		report_fail("%s:%d: Assertion failed: %s", \
+			    __FILE__, __LINE__, #cond); \
 		dump_stack(); \
 		__abort_test(); \
 	} \
@@ -934,8 +934,8 @@ do { \
 #define TEST_ASSERT_MSG(cond, fmt, args...) \
 do { \
 	if (!(cond)) { \
-		report(0, "%s:%d: Assertion failed: %s\n" fmt, \
-		       __FILE__, __LINE__, #cond, ##args); \
+		report_fail("%s:%d: Assertion failed: %s\n" fmt, \
+			    __FILE__, __LINE__, #cond, ##args); \
 		dump_stack(); \
 		__abort_test(); \
 	} \
@@ -951,15 +951,14 @@ do { \
 		char _bin_b[BINSTR_SZ]; \
 		binstr(_a, _bin_a); \
 		binstr(_b, _bin_b); \
-		report(0, \
-		       "%s:%d: %s failed: (%s) == (%s)\n" \
-		       "\tLHS: %#018lx - %s - %lu\n" \
-		       "\tRHS: %#018lx - %s - %lu%s" fmt, \
-		       __FILE__, __LINE__, \
-		       assertion ? "Assertion" : "Expectation", a_str, b_str, \
-		       (unsigned long) _a, _bin_a, (unsigned long) _a, \
-		       (unsigned long) _b, _bin_b, (unsigned long) _b, \
-		       fmt[0] == '\0' ? "" : "\n", ## args); \
+		report_fail("%s:%d: %s failed: (%s) == (%s)\n" \
+			    "\tLHS: %#018lx - %s - %lu\n" \
+			    "\tRHS: %#018lx - %s - %lu%s" fmt, \
+			    __FILE__, __LINE__, \
+			    assertion ? "Assertion" : "Expectation", a_str, b_str, \
+			    (unsigned long) _a, _bin_a, (unsigned long) _a, \
+			    (unsigned long) _b, _bin_b, (unsigned long) _b, \
+			    fmt[0] == '\0' ? "" : "\n", ## args); \
 		dump_stack(); \
 		if (assertion) \
 			__abort_test(); \
diff --git a/arm/psci.c b/arm/psci.c
index ffc09a2..efa0722 100644
--- a/arm/psci.c
+++ b/arm/psci.c
@@ -156,7 +156,7 @@ done:
 #if 0
 	report_summary();
 	psci_invoke(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
-	report(false, "system-off");
+	report_fail("system-off");
 	return 1; /* only reaches here if system-off fails */
 #else
 	return report_summary();
diff --git a/arm/timer.c b/arm/timer.c
index 09e3f8f..27e2e61 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -311,7 +311,7 @@ static void test_init(void)
 	if (ptimer_unsupported && !ERRATA(7b6b46311a85)) {
 		report_skip("Skipping ptimer tests. Set ERRATA_7b6b46311a85=y to enable.");
 	} else if (ptimer_unsupported) {
-		report(false, "ptimer: read CNTP_CTL_EL0");
+		report_fail("ptimer: read CNTP_CTL_EL0");
 		report_info("ptimer: skipping remaining tests");
 	}
 
diff --git a/s390x/css.c b/s390x/css.c
index c340c53..dcb4d70 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -34,7 +34,7 @@ static void test_enumerate(void)
 		report(1, "Schid of first I/O device: 0x%08x", test_device_sid);
 		return;
 	}
-	report(0, "No I/O device found");
+	report_fail("No I/O device found");
 }
 
 static void test_enable(void)
@@ -69,8 +69,8 @@ static void test_sense(void)
 
 	ret = css_enable(test_device_sid, IO_SCH_ISC);
 	if (ret) {
-		report(0, "Could not enable the subchannel: %08x",
-		       test_device_sid);
+		report_fail("Could not enable the subchannel: %08x",
+			    test_device_sid);
 		return;
 	}
 
@@ -78,19 +78,19 @@ static void test_sense(void)
 
 	senseid = alloc_io_mem(sizeof(*senseid), 0);
 	if (!senseid) {
-		report(0, "Allocation of senseid");
+		report_fail("Allocation of senseid");
 		return;
 	}
 
 	ccw = ccw_alloc(CCW_CMD_SENSE_ID, senseid, sizeof(*senseid), CCW_F_SLI);
 	if (!ccw) {
-		report(0, "Allocation of CCW");
+		report_fail("Allocation of CCW");
 		goto error_ccw;
 	}
 
 	ret = start_ccw1_chain(test_device_sid, ccw);
 	if (ret) {
-		report(0, "Starting CCW chain");
+		report_fail("Starting CCW chain");
 		goto error;
 	}
 
@@ -107,14 +107,14 @@ static void test_sense(void)
 	} else if (ret != 0) {
 		len = sizeof(*senseid) - ret;
 		if (ret && len < CSS_SENSEID_COMMON_LEN) {
-			report(0, "transferred a too short length: %d", ret);
+			report_fail("transferred a too short length: %d", ret);
 			goto error;
 		} else if (ret && len)
 			report_info("transferred a shorter length: %d", len);
 	}
 
 	if (senseid->reserved != 0xff) {
-		report(0, "transferred garbage: 0x%02x", senseid->reserved);
+		report_fail("transferred garbage: 0x%02x", senseid->reserved);
 		goto error;
 	}
 
@@ -265,7 +265,7 @@ static void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb)
 	/* Read the SCHIB for this subchannel */
 	cc = stsch(schid, &schib);
 	if (cc) {
-		report(0, "stsch: sch %08x failed with cc=%d", schid, cc);
+		report_fail("stsch: sch %08x failed with cc=%d", schid, cc);
 		return;
 	}
 
diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
index eaf48f4..dacdfdb 100644
--- a/s390x/spec_ex.c
+++ b/s390x/spec_ex.c
@@ -129,10 +129,9 @@ static void test_spec_ex(struct args *args,
 		register_pgm_cleanup_func(NULL);
 		pgm = clear_pgm_int();
 		if (pgm != expected_pgm) {
-			report(0,
-			       "Program interrupt: expected(%d) == received(%d)",
-			       expected_pgm,
-			       pgm);
+			report_fail("Program interrupt: expected(%d) == received(%d)",
+				    expected_pgm,
+				    pgm);
 			return;
 		}
 	}
diff --git a/x86/asyncpf.c b/x86/asyncpf.c
index 8239e16..180395e 100644
--- a/x86/asyncpf.c
+++ b/x86/asyncpf.c
@@ -55,7 +55,7 @@ static void pf_isr(struct ex_regs *r)
 
 	switch (reason) {
 		case 0:
-			report(false, "unexpected #PF at %#lx", read_cr2());
+			report_fail("unexpected #PF at %#lx", read_cr2());
 			break;
 		case KVM_PV_REASON_PAGE_NOT_PRESENT:
 			phys = virt_to_pte_phys(phys_to_virt(read_cr3()), virt);
@@ -78,7 +78,7 @@ static void pf_isr(struct ex_regs *r)
 			phys = 0;
 			break;
 		default:
-			report(false, "unexpected async pf reason %" PRId32, reason);
+			report_fail("unexpected async pf reason %" PRId32, reason);
 			break;
 	}
 }
diff --git a/x86/hyperv_stimer.c b/x86/hyperv_stimer.c
index 75a69a1..874a393 100644
--- a/x86/hyperv_stimer.c
+++ b/x86/hyperv_stimer.c
@@ -96,7 +96,7 @@ static void process_stimer_msg(struct svcpu *svcpu,
 
     if (msg->header.message_type != HVMSG_TIMER_EXPIRED &&
         msg->header.message_type != HVMSG_NONE) {
-        report(false, "invalid Hyper-V SynIC msg type");
+        report_fail("invalid Hyper-V SynIC msg type");
         report_summary();
         abort();
     }
@@ -106,7 +106,7 @@ static void process_stimer_msg(struct svcpu *svcpu,
     }
 
     if (msg->header.payload_size < sizeof(*payload)) {
-        report(false, "invalid Hyper-V SynIC msg payload size");
+        report_fail("invalid Hyper-V SynIC msg payload size");
         report_summary();
         abort();
     }
@@ -114,7 +114,7 @@ static void process_stimer_msg(struct svcpu *svcpu,
     /* Now process timer expiration message */
 
     if (payload->timer_index >= ARRAY_SIZE(svcpu->timer)) {
-        report(false, "invalid Hyper-V SynIC timer index");
+        report_fail("invalid Hyper-V SynIC timer index");
         report_summary();
         abort();
     }
diff --git a/x86/hyperv_synic.c b/x86/hyperv_synic.c
index 25121ca..5ca593c 100644
--- a/x86/hyperv_synic.c
+++ b/x86/hyperv_synic.c
@@ -90,7 +90,7 @@ static void synic_test_prepare(void *ctx)
     }
     r = rdmsr(HV_X64_MSR_EOM);
     if (r != 0) {
-        report(false, "Hyper-V SynIC test, EOM read %#" PRIx64, r);
+        report_fail("Hyper-V SynIC test, EOM read %#" PRIx64, r);
         return;
     }
 
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 547994d..02910db 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -90,8 +90,8 @@ static bool finished_rsm_intercept(struct svm_test *test)
     switch (get_test_stage(test)) {
     case 0:
         if (vmcb->control.exit_code != SVM_EXIT_RSM) {
-            report(false, "VMEXIT not due to rsm. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to rsm. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->control.intercept &= ~(1 << INTERCEPT_RSM);
@@ -100,8 +100,8 @@ static bool finished_rsm_intercept(struct svm_test *test)
 
     case 1:
         if (vmcb->control.exit_code != SVM_EXIT_EXCP_BASE + UD_VECTOR) {
-            report(false, "VMEXIT not due to #UD. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to #UD. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->save.rip += 2;
@@ -210,7 +210,7 @@ static void test_dr_intercept(struct svm_test *test)
         }
 
         if (test->scratch != i) {
-            report(false, "dr%u read intercept", i);
+            report_fail("dr%u read intercept", i);
             failcnt++;
         }
     }
@@ -246,7 +246,7 @@ static void test_dr_intercept(struct svm_test *test)
         }
 
         if (test->scratch != i) {
-            report(false, "dr%u write intercept", i);
+            report_fail("dr%u write intercept", i);
             failcnt++;
         }
     }
@@ -346,7 +346,7 @@ static void test_msr_intercept(struct svm_test *test)
 
         /* Check that a read intercept occurred for MSR at msr_index */
         if (test->scratch != msr_index)
-            report(false, "MSR 0x%lx read intercept", msr_index);
+            report_fail("MSR 0x%lx read intercept", msr_index);
 
         /*
          * Poor man approach to generate a value that
@@ -358,7 +358,7 @@ static void test_msr_intercept(struct svm_test *test)
 
         /* Check that a write intercept occurred for MSR with msr_value */
         if (test->scratch != msr_value)
-            report(false, "MSR 0x%lx write intercept", msr_index);
+            report_fail("MSR 0x%lx write intercept", msr_index);
     }
 
     test->scratch = -2;
@@ -615,7 +615,7 @@ static void test_ioio(struct svm_test *test)
     return;
 
 fail:
-    report(false, "stage %d", get_test_stage(test));
+    report_fail("stage %d", get_test_stage(test));
     test->scratch = -1;
 }
 
@@ -689,7 +689,7 @@ static void sel_cr0_bug_test(struct svm_test *test)
      * are not in guest-mode anymore so we can't trigger an intercept.
      * Trigger a tripple-fault for now.
      */
-    report(false, "sel_cr0 test. Can not recover from this - exiting");
+    report_fail("sel_cr0 test. Can not recover from this - exiting");
     exit(report_summary());
 }
 
@@ -1101,8 +1101,8 @@ static bool pending_event_finished(struct svm_test *test)
     switch (get_test_stage(test)) {
     case 0:
         if (vmcb->control.exit_code != SVM_EXIT_INTR) {
-            report(false, "VMEXIT not due to pending interrupt. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to pending interrupt. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
 
@@ -1110,7 +1110,7 @@ static bool pending_event_finished(struct svm_test *test)
         vmcb->control.int_ctl &= ~V_INTR_MASKING_MASK;
 
         if (pending_event_guest_run) {
-            report(false, "Guest ran before host received IPI\n");
+            report_fail("Guest ran before host received IPI\n");
             return true;
         }
 
@@ -1119,14 +1119,14 @@ static bool pending_event_finished(struct svm_test *test)
         irq_disable();
 
         if (!pending_event_ipi_fired) {
-            report(false, "Pending interrupt not dispatched after IRQ enabled\n");
+            report_fail("Pending interrupt not dispatched after IRQ enabled\n");
             return true;
         }
         break;
 
     case 1:
         if (!pending_event_guest_run) {
-            report(false, "Guest did not resume when no interrupt\n");
+            report_fail("Guest did not resume when no interrupt\n");
             return true;
         }
         break;
@@ -1165,7 +1165,7 @@ static void pending_event_cli_test(struct svm_test *test)
 {
     if (pending_event_ipi_fired == true) {
         set_test_stage(test, -1);
-        report(false, "Interrupt preceeded guest");
+        report_fail("Interrupt preceeded guest");
         vmmcall();
     }
 
@@ -1176,7 +1176,7 @@ static void pending_event_cli_test(struct svm_test *test)
 
     if (pending_event_ipi_fired != true) {
         set_test_stage(test, -1);
-        report(false, "Interrupt not triggered by guest");
+        report_fail("Interrupt not triggered by guest");
     }
 
     vmmcall();
@@ -1194,8 +1194,8 @@ static void pending_event_cli_test(struct svm_test *test)
 static bool pending_event_cli_finished(struct svm_test *test)
 {
     if ( vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-        report(false, "VM_EXIT return to host is not EXIT_VMMCALL exit reason 0x%x",
-               vmcb->control.exit_code);
+        report_fail("VM_EXIT return to host is not EXIT_VMMCALL exit reason 0x%x",
+                    vmcb->control.exit_code);
         return true;
     }
 
@@ -1215,7 +1215,7 @@ static bool pending_event_cli_finished(struct svm_test *test)
 
     case 1:
         if (pending_event_ipi_fired == true) {
-            report(false, "Interrupt triggered by guest");
+            report_fail("Interrupt triggered by guest");
             return true;
         }
 
@@ -1224,7 +1224,7 @@ static bool pending_event_cli_finished(struct svm_test *test)
         irq_disable();
 
         if (pending_event_ipi_fired != true) {
-            report(false, "Interrupt not triggered by host");
+            report_fail("Interrupt not triggered by host");
             return true;
         }
 
@@ -1339,8 +1339,8 @@ static bool interrupt_finished(struct svm_test *test)
     case 0:
     case 2:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->save.rip += 3;
@@ -1352,8 +1352,8 @@ static bool interrupt_finished(struct svm_test *test)
     case 1:
     case 3:
         if (vmcb->control.exit_code != SVM_EXIT_INTR) {
-            report(false, "VMEXIT not due to intr intercept. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to intr intercept. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
 
@@ -1425,8 +1425,8 @@ static bool nmi_finished(struct svm_test *test)
     switch (get_test_stage(test)) {
     case 0:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->save.rip += 3;
@@ -1436,8 +1436,8 @@ static bool nmi_finished(struct svm_test *test)
 
     case 1:
         if (vmcb->control.exit_code != SVM_EXIT_NMI) {
-            report(false, "VMEXIT not due to NMI intercept. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to NMI intercept. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
 
@@ -1527,8 +1527,8 @@ static bool nmi_hlt_finished(struct svm_test *test)
     switch (get_test_stage(test)) {
     case 1:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->save.rip += 3;
@@ -1538,8 +1538,8 @@ static bool nmi_hlt_finished(struct svm_test *test)
 
     case 2:
         if (vmcb->control.exit_code != SVM_EXIT_NMI) {
-            report(false, "VMEXIT not due to NMI intercept. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to NMI intercept. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
 
@@ -1586,8 +1586,8 @@ static bool exc_inject_finished(struct svm_test *test)
     switch (get_test_stage(test)) {
     case 0:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->save.rip += 3;
@@ -1596,8 +1596,8 @@ static bool exc_inject_finished(struct svm_test *test)
 
     case 1:
         if (vmcb->control.exit_code != SVM_EXIT_ERR) {
-            report(false, "VMEXIT not due to error. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to error. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         report(count_exc == 0, "exception with vector 2 not injected");
@@ -1606,8 +1606,8 @@ static bool exc_inject_finished(struct svm_test *test)
 
     case 2:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->save.rip += 3;
@@ -1650,7 +1650,7 @@ static void virq_inject_prepare(struct svm_test *test)
 static void virq_inject_test(struct svm_test *test)
 {
     if (virq_fired) {
-        report(false, "virtual interrupt fired before L2 sti");
+        report_fail("virtual interrupt fired before L2 sti");
         set_test_stage(test, -1);
         vmmcall();
     }
@@ -1660,14 +1660,14 @@ static void virq_inject_test(struct svm_test *test)
     irq_disable();
 
     if (!virq_fired) {
-        report(false, "virtual interrupt not fired after L2 sti");
+        report_fail("virtual interrupt not fired after L2 sti");
         set_test_stage(test, -1);
     }
 
     vmmcall();
 
     if (virq_fired) {
-        report(false, "virtual interrupt fired before L2 sti after VINTR intercept");
+        report_fail("virtual interrupt fired before L2 sti after VINTR intercept");
         set_test_stage(test, -1);
         vmmcall();
     }
@@ -1677,7 +1677,7 @@ static void virq_inject_test(struct svm_test *test)
     irq_disable();
 
     if (!virq_fired) {
-        report(false, "virtual interrupt not fired after return from VINTR intercept");
+        report_fail("virtual interrupt not fired after return from VINTR intercept");
         set_test_stage(test, -1);
     }
 
@@ -1688,7 +1688,7 @@ static void virq_inject_test(struct svm_test *test)
     irq_disable();
 
     if (virq_fired) {
-        report(false, "virtual interrupt fired when V_IRQ_PRIO less than V_TPR");
+        report_fail("virtual interrupt fired when V_IRQ_PRIO less than V_TPR");
         set_test_stage(test, -1);
     }
 
@@ -1703,12 +1703,12 @@ static bool virq_inject_finished(struct svm_test *test)
     switch (get_test_stage(test)) {
     case 0:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         if (vmcb->control.int_ctl & V_IRQ_MASK) {
-            report(false, "V_IRQ not cleared on VMEXIT after firing");
+            report_fail("V_IRQ not cleared on VMEXIT after firing");
             return true;
         }
         virq_fired = false;
@@ -1719,12 +1719,12 @@ static bool virq_inject_finished(struct svm_test *test)
 
     case 1:
         if (vmcb->control.exit_code != SVM_EXIT_VINTR) {
-            report(false, "VMEXIT not due to vintr. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vintr. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         if (virq_fired) {
-            report(false, "V_IRQ fired before SVM_EXIT_VINTR");
+            report_fail("V_IRQ fired before SVM_EXIT_VINTR");
             return true;
         }
         vmcb->control.intercept &= ~(1ULL << INTERCEPT_VINTR);
@@ -1732,8 +1732,8 @@ static bool virq_inject_finished(struct svm_test *test)
 
     case 2:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         virq_fired = false;
@@ -1746,8 +1746,8 @@ static bool virq_inject_finished(struct svm_test *test)
 
     case 3:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         vmcb->control.intercept |= (1ULL << INTERCEPT_VINTR);
@@ -1756,8 +1756,8 @@ static bool virq_inject_finished(struct svm_test *test)
     case 4:
         // INTERCEPT_VINTR should be ignored because V_INTR_PRIO < V_TPR
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall. Exit reason 0x%x",
-                   vmcb->control.exit_code);
+            report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x",
+                        vmcb->control.exit_code);
             return true;
         }
         break;
@@ -1860,9 +1860,8 @@ static bool reg_corruption_finished(struct svm_test *test)
         irq_disable();
 
         if (guest_rip == insb_instruction_label && io_port_var != 0xAA) {
-            report(false,
-                   "RIP corruption detected after %d timer interrupts",
-                   isr_cnt);
+            report_fail("RIP corruption detected after %d timer interrupts",
+                        isr_cnt);
             return true;
         }
 
@@ -1943,8 +1942,8 @@ static bool init_intercept_finished(struct svm_test *test)
     vmcb->save.rip += 3;
 
     if (vmcb->control.exit_code != SVM_EXIT_INIT) {
-        report(false, "VMEXIT not due to init intercept. Exit reason 0x%x",
-               vmcb->control.exit_code);
+        report_fail("VMEXIT not due to init intercept. Exit reason 0x%x",
+                    vmcb->control.exit_code);
 
         return true;
         }
@@ -2045,8 +2044,8 @@ static bool host_rflags_finished(struct svm_test *test)
 	switch (get_test_stage(test)) {
 	case 0:
 		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-			report(false, "Unexpected VMEXIT. Exit reason 0x%x",
-			    vmcb->control.exit_code);
+			report_fail("Unexpected VMEXIT. Exit reason 0x%x",
+				    vmcb->control.exit_code);
 			return true;
 		}
 		vmcb->save.rip += 3;
@@ -2059,9 +2058,9 @@ static bool host_rflags_finished(struct svm_test *test)
 	case 1:
 		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL ||
 		    host_rflags_guest_main_flag != 1) {
-			report(false, "Unexpected VMEXIT or #DB handler"
-			    " invoked before guest main. Exit reason 0x%x",
-			    vmcb->control.exit_code);
+			report_fail("Unexpected VMEXIT or #DB handler"
+				    " invoked before guest main. Exit reason 0x%x",
+				    vmcb->control.exit_code);
 			return true;
 		}
 		vmcb->save.rip += 3;
@@ -2075,10 +2074,10 @@ static bool host_rflags_finished(struct svm_test *test)
 	case 2:
 		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL ||
 		    rip_detected != (u64)&vmrun_rip + 3) {
-			report(false, "Unexpected VMEXIT or RIP mismatch."
-			    " Exit reason 0x%x, RIP actual: %lx, RIP expected: "
-			    "%lx", vmcb->control.exit_code,
-			    (u64)&vmrun_rip + 3, rip_detected);
+			report_fail("Unexpected VMEXIT or RIP mismatch."
+				    " Exit reason 0x%x, RIP actual: %lx, RIP expected: "
+				    "%lx", vmcb->control.exit_code,
+				    (u64)&vmrun_rip + 3, rip_detected);
 			return true;
 		}
 		host_rflags_set_rf = true;
@@ -2092,11 +2091,11 @@ static bool host_rflags_finished(struct svm_test *test)
 		    host_rflags_guest_main_flag != 1 ||
 		    host_rflags_db_handler_flag > 1 ||
 		    read_rflags() & X86_EFLAGS_RF) {
-			report(false, "Unexpected VMEXIT or RIP mismatch or "
-			    "EFLAGS.RF not cleared."
-			    " Exit reason 0x%x, RIP actual: %lx, RIP expected: "
-			    "%lx", vmcb->control.exit_code,
-			    (u64)&vmrun_rip, rip_detected);
+			report_fail("Unexpected VMEXIT or RIP mismatch or "
+				    "EFLAGS.RF not cleared."
+				    " Exit reason 0x%x, RIP actual: %lx, RIP expected: "
+				    "%lx", vmcb->control.exit_code,
+				    (u64)&vmrun_rip, rip_detected);
 			return true;
 		}
 		host_rflags_set_tf = false;
@@ -2828,8 +2827,8 @@ static void svm_vmrun_errata_test(void)
         );
 
         if (svm_errata_reproduced) {
-            report(false, "Got #GP exception - svm errata reproduced at 0x%lx",
-                   physical);
+            report_fail("Got #GP exception - svm errata reproduced at 0x%lx",
+                        physical);
             break;
         }
 
@@ -2924,7 +2923,7 @@ static bool vgif_finished(struct svm_test *test)
     {
     case 0:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall.");
+            report_fail("VMEXIT not due to vmmcall.");
             return true;
         }
         vmcb->control.int_ctl |= V_GIF_ENABLED_MASK;
@@ -2933,11 +2932,11 @@ static bool vgif_finished(struct svm_test *test)
         break;
     case 1:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall.");
+            report_fail("VMEXIT not due to vmmcall.");
             return true;
         }
         if (!(vmcb->control.int_ctl & V_GIF_MASK)) {
-            report(false, "Failed to set VGIF when executing STGI.");
+            report_fail("Failed to set VGIF when executing STGI.");
             vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK;
             return true;
         }
@@ -2947,11 +2946,11 @@ static bool vgif_finished(struct svm_test *test)
         break;
     case 2:
         if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-            report(false, "VMEXIT not due to vmmcall.");
+            report_fail("VMEXIT not due to vmmcall.");
             return true;
         }
         if (vmcb->control.int_ctl & V_GIF_MASK) {
-            report(false, "Failed to clear VGIF when executing CLGI.");
+            report_fail("Failed to clear VGIF when executing CLGI.");
             vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK;
             return true;
         }
diff --git a/x86/vmx.c b/x86/vmx.c
index 2a32aa2..247de23 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1112,11 +1112,10 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 		if (!bad_pt_ad) {
 			bad_pt_ad |= (ept_pte & (EPT_ACCESS_FLAG|EPT_DIRTY_FLAG)) != expected_pt_ad;
 			if (bad_pt_ad)
-				report(false,
-				       "EPT - guest level %d page table A=%d/D=%d",
-				       l,
-				       !!(expected_pt_ad & EPT_ACCESS_FLAG),
-				       !!(expected_pt_ad & EPT_DIRTY_FLAG));
+				report_fail("EPT - guest level %d page table A=%d/D=%d",
+					    l,
+					    !!(expected_pt_ad & EPT_ACCESS_FLAG),
+					    !!(expected_pt_ad & EPT_DIRTY_FLAG));
 		}
 
 		pte = pt[offset];
@@ -1137,7 +1136,7 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 	gpa = (pt[offset] & PT_ADDR_MASK) | (guest_addr & offset_in_page);
 
 	if (!get_ept_pte(pml4, gpa, 1, &ept_pte)) {
-		report(false, "EPT - guest physical address is not mapped");
+		report_fail("EPT - guest physical address is not mapped");
 		return;
 	}
 	report((ept_pte & (EPT_ACCESS_FLAG | EPT_DIRTY_FLAG)) == expected_gpa_ad,
@@ -1883,7 +1882,7 @@ static int test_run(struct vmx_test *test)
 		int ret = 0;
 		if (test->init || test->guest_main || test->exit_handler ||
 		    test->syscall_handler) {
-			report(0, "V2 test cannot specify V1 callbacks.");
+			report_fail("V2 test cannot specify V1 callbacks.");
 			ret = 1;
 		}
 		if (ret)
@@ -1928,7 +1927,7 @@ static int test_run(struct vmx_test *test)
 		run_teardown_step(&teardown_steps[--teardown_count]);
 
 	if (launched && !guest_finished)
-		report(0, "Guest didn't run to completion.");
+		report_fail("Guest didn't run to completion.");
 
 out:
 	if (vmx_off()) {
@@ -2123,7 +2122,7 @@ int main(int argc, const char *argv[])
 			goto exit;
 	} else {
 		if (vmx_on()) {
-			report(0, "vmxon");
+			report_fail("vmxon");
 			goto exit;
 		}
 	}
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 4f712eb..872586a 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -58,7 +58,7 @@ static void basic_guest_main(void)
 
 static int basic_exit_handler(union exit_reason exit_reason)
 {
-	report(0, "Basic VMX test");
+	report_fail("Basic VMX test");
 	print_vmexit_info(exit_reason);
 	return VMX_TEST_EXIT;
 }
@@ -88,14 +88,14 @@ static int vmenter_exit_handler(union exit_reason exit_reason)
 	switch (exit_reason.basic) {
 	case VMX_VMCALL:
 		if (regs.rax != 0xABCD) {
-			report(0, "test vmresume");
+			report_fail("test vmresume");
 			return VMX_TEST_VMEXIT;
 		}
 		regs.rax = 0xFFFF;
 		vmcs_write(GUEST_RIP, guest_rip + 3);
 		return VMX_TEST_RESUME;
 	default:
-		report(0, "test vmresume");
+		report_fail("test vmresume");
 		print_vmexit_info(exit_reason);
 	}
 	return VMX_TEST_VMEXIT;
@@ -184,7 +184,7 @@ static int preemption_timer_exit_handler(union exit_reason exit_reason)
 			       "preemption timer with 0 value");
 			break;
 		default:
-			report(false, "Invalid stage.");
+			report_fail("Invalid stage.");
 			print_vmexit_info(exit_reason);
 			break;
 		}
@@ -206,12 +206,12 @@ static int preemption_timer_exit_handler(union exit_reason exit_reason)
 			       "Save preemption value");
 			return VMX_TEST_RESUME;
 		case 2:
-			report(0, "busy-wait for preemption timer");
+			report_fail("busy-wait for preemption timer");
 			vmx_set_test_stage(3);
 			vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
 			return VMX_TEST_RESUME;
 		case 3:
-			report(0, "preemption timer during hlt");
+			report_fail("preemption timer during hlt");
 			vmx_set_test_stage(4);
 			/* fall through */
 		case 4:
@@ -221,19 +221,18 @@ static int preemption_timer_exit_handler(union exit_reason exit_reason)
 			saved_rip = guest_rip + insn_len;
 			return VMX_TEST_RESUME;
 		case 5:
-			report(0,
-			       "preemption timer with 0 value (vmcall stage 5)");
+			report_fail("preemption timer with 0 value (vmcall stage 5)");
 			break;
 		default:
 			// Should not reach here
-			report(false, "unexpected stage, %d",
-			       vmx_get_test_stage());
+			report_fail("unexpected stage, %d",
+				    vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
 		}
 		break;
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 	vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT);
@@ -317,7 +316,7 @@ static void test_ctrl_pat_main(void)
 		printf("\tENT_LOAD_PAT is not supported.\n");
 	else {
 		if (guest_ia32_pat != 0) {
-			report(0, "Entry load PAT");
+			report_fail("Entry load PAT");
 			return;
 		}
 	}
@@ -383,7 +382,7 @@ static void test_ctrl_efer_main(void)
 		printf("\tENT_LOAD_EFER is not supported.\n");
 	else {
 		if (guest_ia32_efer != (ia32_efer ^ EFER_NX)) {
-			report(0, "Entry load EFER");
+			report_fail("Entry load EFER");
 			return;
 		}
 	}
@@ -436,13 +435,13 @@ static void cr_shadowing_main(void)
 	vmx_set_test_stage(0);
 	guest_cr0 = read_cr0();
 	if (vmx_get_test_stage() == 1)
-		report(0, "Read through CR0");
+		report_fail("Read through CR0");
 	else
 		vmcall();
 	vmx_set_test_stage(1);
 	guest_cr4 = read_cr4();
 	if (vmx_get_test_stage() == 2)
-		report(0, "Read through CR4");
+		report_fail("Read through CR4");
 	else
 		vmcall();
 	// Test write through
@@ -451,13 +450,13 @@ static void cr_shadowing_main(void)
 	vmx_set_test_stage(2);
 	write_cr0(guest_cr0);
 	if (vmx_get_test_stage() == 3)
-		report(0, "Write throuth CR0");
+		report_fail("Write throuth CR0");
 	else
 		vmcall();
 	vmx_set_test_stage(3);
 	write_cr4(guest_cr4);
 	if (vmx_get_test_stage() == 4)
-		report(0, "Write through CR4");
+		report_fail("Write through CR4");
 	else
 		vmcall();
 	// Test read shadow
@@ -474,13 +473,13 @@ static void cr_shadowing_main(void)
 	vmx_set_test_stage(6);
 	write_cr0(guest_cr0);
 	if (vmx_get_test_stage() == 7)
-		report(0, "Write shadowing CR0 (same value with shadow)");
+		report_fail("Write shadowing CR0 (same value with shadow)");
 	else
 		vmcall();
 	vmx_set_test_stage(7);
 	write_cr4(guest_cr4);
 	if (vmx_get_test_stage() == 8)
-		report(0, "Write shadowing CR4 (same value with shadow)");
+		report_fail("Write shadowing CR4 (same value with shadow)");
 	else
 		vmcall();
 	// Test write shadow (different value)
@@ -564,8 +563,8 @@ static int cr_shadowing_exit_handler(union exit_reason exit_reason)
 			break;
 		default:
 			// Should not reach here
-			report(false, "unexpected stage, %d",
-			       vmx_get_test_stage());
+			report_fail("unexpected stage, %d",
+				    vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
 		}
@@ -574,19 +573,19 @@ static int cr_shadowing_exit_handler(union exit_reason exit_reason)
 	case VMX_CR:
 		switch (vmx_get_test_stage()) {
 		case 4:
-			report(0, "Read shadowing CR0");
+			report_fail("Read shadowing CR0");
 			vmx_inc_test_stage();
 			break;
 		case 5:
-			report(0, "Read shadowing CR4");
+			report_fail("Read shadowing CR4");
 			vmx_inc_test_stage();
 			break;
 		case 6:
-			report(0, "Write shadowing CR0 (same value)");
+			report_fail("Write shadowing CR0 (same value)");
 			vmx_inc_test_stage();
 			break;
 		case 7:
-			report(0, "Write shadowing CR4 (same value)");
+			report_fail("Write shadowing CR4 (same value)");
 			vmx_inc_test_stage();
 			break;
 		case 8:
@@ -603,15 +602,15 @@ static int cr_shadowing_exit_handler(union exit_reason exit_reason)
 			break;
 		default:
 			// Should not reach here
-			report(false, "unexpected stage, %d",
-			       vmx_get_test_stage());
+			report_fail("unexpected stage, %d",
+				    vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
 		}
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 	return VMX_TEST_VMEXIT;
@@ -740,8 +739,8 @@ static int iobmp_exit_handler(union exit_reason exit_reason)
 			break;
 		default:
 			// Should not reach here
-			report(false, "unexpected stage, %d",
-			       vmx_get_test_stage());
+			report_fail("unexpected stage, %d",
+				    vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
 		}
@@ -761,8 +760,8 @@ static int iobmp_exit_handler(union exit_reason exit_reason)
 			break;
 		default:
 			// Should not reach here
-			report(false, "unexpected stage, %d",
-			       vmx_get_test_stage());
+			report_fail("unexpected stage, %d",
+				    vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
 		}
@@ -1178,7 +1177,7 @@ static void ept_common(void)
 	vmx_set_test_stage(0);
 	if (*((u32 *)data_page2) != MAGIC_VAL_1 ||
 			*((u32 *)data_page1) != MAGIC_VAL_1)
-		report(0, "EPT basic framework - read");
+		report_fail("EPT basic framework - read");
 	else {
 		*((u32 *)data_page2) = MAGIC_VAL_3;
 		vmcall();
@@ -1195,7 +1194,7 @@ static void ept_common(void)
 	vmcall();
 	*((u32 *)data_page1) = MAGIC_VAL_1;
 	if (vmx_get_test_stage() != 2) {
-		report(0, "EPT misconfigurations");
+		report_fail("EPT misconfigurations");
 		goto t1;
 	}
 	vmx_set_test_stage(2);
@@ -1283,7 +1282,7 @@ static int pml_exit_handler(union exit_reason exit_reason)
 			clear_ept_ad(pml4, guest_cr3, (unsigned long)data_page2);
 			break;
 		default:
-			report(false, "unexpected stage, %d.",
+			report_fail("unexpected stage, %d.",
 			       vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
@@ -1295,7 +1294,7 @@ static int pml_exit_handler(union exit_reason exit_reason)
 		vmcs_write(GUEST_PML_INDEX, PML_INDEX - 1);
 		return VMX_TEST_RESUME;
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 	return VMX_TEST_VMEXIT;
@@ -1338,7 +1337,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 						(unsigned long)data_page2,
 						EPT_RA | EPT_WA | EPT_EA);
 			} else
-				report(0, "EPT basic framework - write");
+				report_fail("EPT basic framework - write");
 			break;
 		case 1:
 			install_ept(pml4, (unsigned long)data_page1,
@@ -1380,7 +1379,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			break;
 		// Should not reach here
 		default:
-			report(false, "ERROR - unexpected stage, %d.",
+			report_fail("ERROR - unexpected stage, %d.",
 			       vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
@@ -1399,7 +1398,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			break;
 		// Should not reach here
 		default:
-			report(false, "ERROR - unexpected stage, %d.",
+			report_fail("ERROR - unexpected stage, %d.",
 			       vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
@@ -1455,14 +1454,14 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			break;
 		default:
 			// Should not reach here
-			report(false, "ERROR : unexpected stage, %d",
+			report_fail("ERROR : unexpected stage, %d",
 			       vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
 		}
 		return VMX_TEST_RESUME;
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 	return VMX_TEST_VMEXIT;
@@ -1612,7 +1611,7 @@ static int vpid_exit_handler(union exit_reason exit_reason)
 				vmx_inc_test_stage();
 			break;
 		default:
-			report(false, "ERROR: unexpected stage, %d",
+			report_fail("ERROR: unexpected stage, %d",
 					vmx_get_test_stage());
 			print_vmexit_info(exit_reason);
 			return VMX_TEST_VMEXIT;
@@ -1620,7 +1619,7 @@ static int vpid_exit_handler(union exit_reason exit_reason)
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 	return VMX_TEST_VMEXIT;
@@ -1791,7 +1790,7 @@ static int interrupt_exit_handler(union exit_reason exit_reason)
 			vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
 		return VMX_TEST_RESUME;
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 
@@ -1878,8 +1877,8 @@ static int nmi_hlt_exit_handler(union exit_reason exit_reason)
     switch (vmx_get_test_stage()) {
     case 1:
         if (exit_reason.basic != VMX_VMCALL) {
-            report(false, "VMEXIT not due to vmcall. Exit reason 0x%x",
-                   exit_reason.full);
+            report_fail("VMEXIT not due to vmcall. Exit reason 0x%x",
+                        exit_reason.full);
             print_vmexit_info(exit_reason);
             return VMX_TEST_VMEXIT;
         }
@@ -1893,8 +1892,8 @@ static int nmi_hlt_exit_handler(union exit_reason exit_reason)
 
     case 2:
         if (exit_reason.basic != VMX_EXC_NMI) {
-            report(false, "VMEXIT not due to NMI intercept. Exit reason 0x%x",
-                   exit_reason.full);
+            report_fail("VMEXIT not due to NMI intercept. Exit reason 0x%x",
+                        exit_reason.full);
             print_vmexit_info(exit_reason);
             return VMX_TEST_VMEXIT;
         }
@@ -2022,7 +2021,7 @@ static int dbgctls_exit_handler(union exit_reason exit_reason)
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	default:
-		report(false, "Unknown exit reason, %d", exit_reason.full);
+		report_fail("Unknown exit reason, %d", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 	return VMX_TEST_VMEXIT;
@@ -2118,7 +2117,7 @@ static void vmmcall_main(void)
 		"vmmcall\n\t"
 		::: "rax");
 
-	report(0, "VMMCALL");
+	report_fail("VMMCALL");
 }
 
 static int vmmcall_exit_handler(union exit_reason exit_reason)
@@ -2126,14 +2125,14 @@ static int vmmcall_exit_handler(union exit_reason exit_reason)
 	switch (exit_reason.basic) {
 	case VMX_VMCALL:
 		printf("here\n");
-		report(0, "VMMCALL triggers #UD");
+		report_fail("VMMCALL triggers #UD");
 		break;
 	case VMX_EXC_NMI:
 		report((vmcs_read(EXI_INTR_INFO) & 0xff) == UD_VECTOR,
 		       "VMMCALL triggers #UD");
 		break;
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 
@@ -2191,7 +2190,7 @@ static int disable_rdtscp_exit_handler(union exit_reason exit_reason)
 	case VMX_VMCALL:
 		switch (vmx_get_test_stage()) {
 		case 0:
-			report(false, "RDTSCP triggers #UD");
+			report_fail("RDTSCP triggers #UD");
 			vmx_inc_test_stage();
 			/* fallthrough */
 		case 1:
@@ -2199,13 +2198,13 @@ static int disable_rdtscp_exit_handler(union exit_reason exit_reason)
 			vmcs_write(GUEST_RIP, vmcs_read(GUEST_RIP) + 3);
 			return VMX_TEST_RESUME;
 		case 2:
-			report(false, "RDPID triggers #UD");
+			report_fail("RDPID triggers #UD");
 			break;
 		}
 		break;
 
 	default:
-		report(false, "Unknown exit reason, 0x%x", exit_reason.full);
+		report_fail("Unknown exit reason, 0x%x", exit_reason.full);
 		print_vmexit_info(exit_reason);
 	}
 	return VMX_TEST_VMEXIT;
@@ -2295,7 +2294,7 @@ static void exit_monitor_from_l2_main(void)
 
 static int exit_monitor_from_l2_handler(union exit_reason exit_reason)
 {
-	report(false, "The guest should have killed the VMM");
+	report_fail("The guest should have killed the VMM");
 	return VMX_TEST_EXIT;
 }
 
@@ -6070,8 +6069,7 @@ static void test_xapic_rd(
 		/* Reenter guest so it can consume/check rcx and exit again. */
 		enter_guest();
 	} else if (exit_reason_want != VMX_VMCALL) {
-		report(false, "Oops, bad exit expectation: %u.",
-		       exit_reason_want);
+		report_fail("Oops, bad exit expectation: %u.", exit_reason_want);
 	}
 
 	skip_exit_vmcall();
@@ -6134,8 +6132,7 @@ static void test_xapic_wr(
 		/* Reenter guest so it can consume/check rcx and exit again. */
 		enter_guest();
 	} else if (exit_reason_want != VMX_VMCALL) {
-		report(false, "Oops, bad exit expectation: %u.",
-		       exit_reason_want);
+		report_fail("Oops, bad exit expectation: %u.", exit_reason_want);
 	}
 
 	assert_exit_reason(VMX_VMCALL);
@@ -6153,8 +6150,7 @@ static void test_xapic_wr(
 		       "non-virtualized write; val is 0x%x, want 0x%x", got,
 		       val);
 	} else if (!expectation->virtualize_apic_accesses && checked) {
-		report(false,
-		       "Non-virtualized write was prematurely checked!");
+		report_fail("Non-virtualized write was prematurely checked!");
 	}
 
 	skip_exit_vmcall();
@@ -6313,7 +6309,7 @@ static void apic_reg_virt_test(void)
 			ok = apic_reg_virt_exit_expectation(
 				reg, apic_reg_virt_config, &expectation);
 			if (!ok) {
-				report(false, "Malformed test.");
+				report_fail("Malformed test.");
 				break;
 			}
 
@@ -6862,8 +6858,7 @@ static void test_x2apic_rd(
 	enter_guest();
 
 	if (exit_reason_want != VMX_VMCALL) {
-		report(false, "Oops, bad exit expectation: %u.",
-		       exit_reason_want);
+		report_fail("Oops, bad exit expectation: %u.", exit_reason_want);
 	}
 
 	skip_exit_vmcall();
@@ -6933,8 +6928,7 @@ static void test_x2apic_wr(
 		/* Reenter guest so it can consume/check rcx and exit again. */
 		enter_guest();
 	} else if (exit_reason_want != VMX_VMCALL) {
-		report(false, "Oops, bad exit expectation: %u.",
-		       exit_reason_want);
+		report_fail("Oops, bad exit expectation: %u.", exit_reason_want);
 	}
 
 	assert_exit_reason(VMX_VMCALL);
@@ -9901,7 +9895,7 @@ static void vmx_init_signal_test(void)
 	 */
 	delay(INIT_SIGNAL_TEST_DELAY);
 	if (vmx_get_test_stage() != 5) {
-		report(false, "Pending INIT signal didn't result in VMX exit");
+		report_fail("Pending INIT signal didn't result in VMX exit");
 		return;
 	}
 	report(init_signal_test_exit_reason == VMX_INIT,
@@ -10035,7 +10029,7 @@ static void sipi_test_ap_thread(void *data)
 		vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
 		vmx_set_test_stage(2);
 	} else {
-		report(0, "AP: Unexpected VMExit, reason=%ld", vmcs_read(EXI_REASON));
+		report_fail("AP: Unexpected VMExit, reason=%ld", vmcs_read(EXI_REASON));
 		vmx_off();
 		return;
 	}
@@ -10522,12 +10516,12 @@ static int invalid_msr_init(struct vmcs *vmcs)
 
 static void invalid_msr_main(void)
 {
-	report(0, "Invalid MSR load");
+	report_fail("Invalid MSR load");
 }
 
 static int invalid_msr_exit_handler(union exit_reason exit_reason)
 {
-	report(0, "Invalid MSR load");
+	report_fail("Invalid MSR load");
 	print_vmexit_info(exit_reason);
 	return VMX_TEST_EXIT;
 }
-- 
2.31.1

Re: [kvm-unit-tests PATCH v2 4/5] Use report_fail(...) instead of report(0/false, ...)

[Andrew Jones]

On Tue, Oct 05, 2021 at 11:09:20AM +0200, Janis Schoetterl-Glausch wrote:
> Whitespace is kept consistent with the rest of the file.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>  lib/s390x/css_lib.c |  30 ++++----
>  x86/vmx.h           |  25 ++++---
>  arm/psci.c          |   2 +-
>  arm/timer.c         |   2 +-
>  s390x/css.c         |  18 ++---
>  s390x/spec_ex.c     |   7 +-
>  x86/asyncpf.c       |   4 +-
>  x86/hyperv_stimer.c |   6 +-
>  x86/hyperv_synic.c  |   2 +-
>  x86/svm_tests.c     | 163 ++++++++++++++++++++++----------------------
>  x86/vmx.c           |  17 +++--
>  x86/vmx_tests.c     | 136 ++++++++++++++++++------------------
>  12 files changed, 200 insertions(+), 212 deletions(-)
>

Hi Janis,

Thank you for this cleanup.

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [kvm-unit-tests PATCH v2 4/5] Use report_fail(...) instead of report(0/false, ...)

[Thomas Huth]

On 05/10/2021 11.09, Janis Schoetterl-Glausch wrote:
> Whitespace is kept consistent with the rest of the file.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>   lib/s390x/css_lib.c |  30 ++++----
>   x86/vmx.h           |  25 ++++---
>   arm/psci.c          |   2 +-
>   arm/timer.c         |   2 +-
>   s390x/css.c         |  18 ++---
>   s390x/spec_ex.c     |   7 +-
>   x86/asyncpf.c       |   4 +-
>   x86/hyperv_stimer.c |   6 +-
>   x86/hyperv_synic.c  |   2 +-
>   x86/svm_tests.c     | 163 ++++++++++++++++++++++----------------------
>   x86/vmx.c           |  17 +++--
>   x86/vmx_tests.c     | 136 ++++++++++++++++++------------------
>   12 files changed, 200 insertions(+), 212 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests PATCH v2 5/5] Use report_pass(...) instead of report(1/true, ...)

[Janis Schoetterl-Glausch]

Whitespace is kept consistent with the rest of the file.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 s390x/css.c         |  4 ++--
 s390x/diag288.c     |  2 +-
 s390x/selftest.c    |  2 +-
 s390x/smp.c         | 16 ++++++++--------
 s390x/spec_ex.c     |  7 +++----
 x86/asyncpf.c       |  7 +++----
 x86/emulator.c      |  2 +-
 x86/hyperv_stimer.c | 18 ++++++++----------
 x86/svm_tests.c     | 17 ++++++++---------
 x86/syscall.c       |  2 +-
 x86/taskswitch2.c   |  2 +-
 x86/tsc_adjust.c    |  2 +-
 x86/vmx.c           |  6 +++---
 x86/vmx_tests.c     | 36 ++++++++++++++++++------------------
 14 files changed, 59 insertions(+), 64 deletions(-)

diff --git a/s390x/css.c b/s390x/css.c
index dcb4d70..881206b 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -31,7 +31,7 @@ static void test_enumerate(void)
 {
 	test_device_sid = css_enumerate();
 	if (test_device_sid & SCHID_ONE) {
-		report(1, "Schid of first I/O device: 0x%08x", test_device_sid);
+		report_pass("Schid of first I/O device: 0x%08x", test_device_sid);
 		return;
 	}
 	report_fail("No I/O device found");
@@ -178,7 +178,7 @@ static void test_schm(void)
 	/* Normal operation */
 	report_prefix_push("Normal operation");
 	schm(NULL, SCHM_MBU);
-	report(1, "SCHM call without address");
+	report_pass("SCHM call without address");
 	report_prefix_pop();
 }
 
diff --git a/s390x/diag288.c b/s390x/diag288.c
index 82b6ec1..072c04a 100644
--- a/s390x/diag288.c
+++ b/s390x/diag288.c
@@ -99,7 +99,7 @@ static void test_bite(void)
 		     "0:	nop\n"
 		     "		j	0b\n"
 		     "1:");
-	report(true, "restart");
+	report_pass("restart");
 }
 
 int main(void)
diff --git a/s390x/selftest.c b/s390x/selftest.c
index 0f099ca..239bc5e 100644
--- a/s390x/selftest.c
+++ b/s390x/selftest.c
@@ -78,7 +78,7 @@ int main(int argc, char**argv)
 {
 	report_prefix_push("selftest");
 
-	report(true, "true");
+	report_pass("true");
 	report(argc == 3, "argc == 3");
 	report(!strcmp(argv[0], "s390x/selftest.elf"), "argv[0] == PROGNAME");
 	report(!strcmp(argv[1], "test"), "argv[1] == test");
diff --git a/s390x/smp.c b/s390x/smp.c
index f25ec76..329ca92 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -47,7 +47,7 @@ static void test_start(void)
 	set_flag(0);
 	smp_cpu_start(1, psw);
 	wait_for_flag();
-	report(1, "start");
+	report_pass("start");
 }
 
 /*
@@ -75,7 +75,7 @@ static void test_restart(void)
 	set_flag(0);
 	smp_cpu_restart(1);
 	wait_for_flag();
-	report(1, "restart while running");
+	report_pass("restart while running");
 }
 
 static void test_stop(void)
@@ -87,7 +87,7 @@ static void test_stop(void)
 	 * implementation
 	 */
 	while (!smp_cpu_stopped(1)) {}
-	report(1, "stop");
+	report_pass("stop");
 }
 
 static void test_stop_store_status(void)
@@ -140,7 +140,7 @@ static void test_store_status(void)
 	smp_cpu_stop(1);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);
 	while (!status->prefix) { mb(); }
-	report(1, "status written");
+	report_pass("status written");
 	free_pages(status);
 	report_prefix_pop();
 	smp_cpu_stop(1);
@@ -160,7 +160,7 @@ static void ecall(void)
 	load_psw_mask(mask);
 	set_flag(1);
 	while (lc->ext_int_code != 0x1202) { mb(); }
-	report(1, "received");
+	report_pass("received");
 	set_flag(1);
 }
 
@@ -194,7 +194,7 @@ static void emcall(void)
 	load_psw_mask(mask);
 	set_flag(1);
 	while (lc->ext_int_code != 0x1201) { mb(); }
-	report(1, "received");
+	report_pass("received");
 	set_flag(1);
 }
 
@@ -225,7 +225,7 @@ static void test_sense_running(void)
 	smp_cpu_stop(1);
 	/* Make sure to have at least one time with a not running indication */
 	while(smp_sense_running_status(1));
-	report(true, "CPU1 sense claims not running");
+	report_pass("CPU1 sense claims not running");
 	report_prefix_pop();
 }
 
@@ -310,7 +310,7 @@ static void test_reset(void)
 	psw.addr = (unsigned long)test_local_ints;
 	smp_cpu_start(1, psw);
 	wait_for_flag();
-	report(true, "local interrupts cleared");
+	report_pass("local interrupts cleared");
 	report_prefix_pop();
 }
 
diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
index dacdfdb..bc62e02 100644
--- a/s390x/spec_ex.c
+++ b/s390x/spec_ex.c
@@ -135,10 +135,9 @@ static void test_spec_ex(struct args *args,
 			return;
 		}
 	}
-	report(1,
-	       "Program interrupt: always expected(%d) == received(%d)",
-	       expected_pgm,
-	       expected_pgm);
+	report_pass("Program interrupt: always expected(%d) == received(%d)",
+		    expected_pgm,
+		    expected_pgm);
 }
 
 #define TRANSACTION_COMPLETED 4
diff --git a/x86/asyncpf.c b/x86/asyncpf.c
index 180395e..9366c29 100644
--- a/x86/asyncpf.c
+++ b/x86/asyncpf.c
@@ -61,16 +61,15 @@ static void pf_isr(struct ex_regs *r)
 			phys = virt_to_pte_phys(phys_to_virt(read_cr3()), virt);
 			install_pte(phys_to_virt(read_cr3()), 1, virt, phys, 0);
 			write_cr3(read_cr3());
-			report(true,
-			       "Got not present #PF token %lx virt addr %p phys addr %#" PRIx64,
-			       read_cr2(), virt, phys);
+			report_pass("Got not present #PF token %lx virt addr %p phys addr %#" PRIx64,
+				    read_cr2(), virt, phys);
 			while(phys) {
 				safe_halt(); /* enables irq */
 				irq_disable();
 			}
 			break;
 		case KVM_PV_REASON_PAGE_READY:
-			report(true, "Got present #PF token %lx", read_cr2());
+			report_pass("Got present #PF token %lx", read_cr2());
 			if ((uint32_t)read_cr2() == ~0)
 				break;
 			install_pte(phys_to_virt(read_cr3()), 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK, 0);
diff --git a/x86/emulator.c b/x86/emulator.c
index 9fda1a0..c5f584a 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -268,7 +268,7 @@ static void test_pop(void *mem)
 		     "1: mov %[tmp], %%rsp"
 		     : [tmp]"=&r"(tmp) : [stack_top]"r"(stack_top)
 		     : "memory");
-	report(1, "ret");
+	report_pass("ret");
 
 	stack_top[-1] = 0x778899;
 	asm volatile("mov %[stack_top], %%r8 \n\t"
diff --git a/x86/hyperv_stimer.c b/x86/hyperv_stimer.c
index 874a393..7b7c985 100644
--- a/x86/hyperv_stimer.c
+++ b/x86/hyperv_stimer.c
@@ -234,7 +234,7 @@ static void stimer_test_periodic(int vcpu, struct stimer *timer1,
            (atomic_read(&timer2->fire_count) < 1000)) {
         pause();
     }
-    report(true, "Hyper-V SynIC periodic timers test vcpu %d", vcpu);
+    report_pass("Hyper-V SynIC periodic timers test vcpu %d", vcpu);
     stimer_shutdown(timer1);
     stimer_shutdown(timer2);
 }
@@ -246,7 +246,7 @@ static void stimer_test_one_shot(int vcpu, struct stimer *timer)
     while (atomic_read(&timer->fire_count) < 1) {
         pause();
     }
-    report(true, "Hyper-V SynIC one-shot test vcpu %d", vcpu);
+    report_pass("Hyper-V SynIC one-shot test vcpu %d", vcpu);
     stimer_shutdown(timer);
 }
 
@@ -257,8 +257,7 @@ static void stimer_test_auto_enable_one_shot(int vcpu, struct stimer *timer)
     while (atomic_read(&timer->fire_count) < 1) {
         pause();
     }
-    report(true, "Hyper-V SynIC auto-enable one-shot timer test vcpu %d",
-           vcpu);
+    report_pass("Hyper-V SynIC auto-enable one-shot timer test vcpu %d", vcpu);
     stimer_shutdown(timer);
 }
 
@@ -269,8 +268,7 @@ static void stimer_test_auto_enable_periodic(int vcpu, struct stimer *timer)
     while (atomic_read(&timer->fire_count) < 1000) {
         pause();
     }
-    report(true, "Hyper-V SynIC auto-enable periodic timer test vcpu %d",
-           vcpu);
+    report_pass("Hyper-V SynIC auto-enable periodic timer test vcpu %d", vcpu);
     stimer_shutdown(timer);
 }
 
@@ -298,7 +296,7 @@ static void stimer_test_one_shot_busy(int vcpu, struct stimer *timer)
     while (atomic_read(&timer->fire_count) < 1) {
         pause();
     }
-    report(true, "timer resumed when msg slot released: vcpu %d", vcpu);
+    report_pass("timer resumed when msg slot released: vcpu %d", vcpu);
 
     stimer_shutdown(timer);
 }
@@ -355,17 +353,17 @@ int main(int ac, char **av)
 {
 
     if (!synic_supported()) {
-        report(true, "Hyper-V SynIC is not supported");
+        report_pass("Hyper-V SynIC is not supported");
         goto done;
     }
 
     if (!stimer_supported()) {
-        report(true, "Hyper-V SynIC timers are not supported");
+        report_pass("Hyper-V SynIC timers are not supported");
         goto done;
     }
 
     if (!hv_time_ref_counter_supported()) {
-        report(true, "Hyper-V time reference counter is not supported");
+        report_pass("Hyper-V time reference counter is not supported");
         goto done;
     }
 
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 02910db..3344e28 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -1441,7 +1441,7 @@ static bool nmi_finished(struct svm_test *test)
             return true;
         }
 
-        report(true, "NMI intercept while running guest");
+        report_pass("NMI intercept while running guest");
         break;
 
     case 2:
@@ -1543,7 +1543,7 @@ static bool nmi_hlt_finished(struct svm_test *test)
             return true;
         }
 
-        report(true, "NMI intercept while running guest");
+        report_pass("NMI intercept while running guest");
         break;
 
     case 3:
@@ -1844,9 +1844,8 @@ static void reg_corruption_test(struct svm_test *test)
 static bool reg_corruption_finished(struct svm_test *test)
 {
     if (isr_cnt == 10000) {
-        report(true,
-               "No RIP corruption detected after %d timer interrupts",
-               isr_cnt);
+        report_pass("No RIP corruption detected after %d timer interrupts",
+                    isr_cnt);
         set_test_stage(test, 1);
         return true;
     }
@@ -1950,7 +1949,7 @@ static bool init_intercept_finished(struct svm_test *test)
 
     init_intercept = true;
 
-    report(true, "INIT to vcpu intercepted");
+    report_pass("INIT to vcpu intercepted");
 
     return true;
 }
@@ -2811,7 +2810,7 @@ static void svm_vmrun_errata_test(void)
         unsigned long *page = alloc_pages(1);
 
         if (!page) {
-            report(true, "All guest memory tested, no bug found");;
+            report_pass("All guest memory tested, no bug found");
             break;
         }
 
@@ -2940,7 +2939,7 @@ static bool vgif_finished(struct svm_test *test)
             vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK;
             return true;
         }
-        report(true, "STGI set VGIF bit.");
+        report_pass("STGI set VGIF bit.");
         vmcb->save.rip += 3;
         inc_test_stage(test);
         break;
@@ -2954,7 +2953,7 @@ static bool vgif_finished(struct svm_test *test)
             vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK;
             return true;
         }
-        report(true, "CLGI cleared VGIF bit.");
+        report_pass("CLGI cleared VGIF bit.");
         vmcb->save.rip += 3;
         inc_test_stage(test);
         vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK;
diff --git a/x86/syscall.c b/x86/syscall.c
index a8045cb..49e9f13 100644
--- a/x86/syscall.c
+++ b/x86/syscall.c
@@ -18,7 +18,7 @@ static void test_syscall_lazy_load(void)
     asm volatile("pushf; syscall; syscall_target: popf" : "=c"(tmp) : : "r11");
     write_ss(ss);
     // will crash horribly if broken
-    report(true, "MSR_*STAR eager loading");
+    report_pass("MSR_*STAR eager loading");
 }
 
 /*
diff --git a/x86/taskswitch2.c b/x86/taskswitch2.c
index ed3f99a..3c9af4c 100644
--- a/x86/taskswitch2.c
+++ b/x86/taskswitch2.c
@@ -260,7 +260,7 @@ static void test_vm86_switch(void)
         "popf\n"
         "iret\n"
     );
-    report(1, "VM86");
+    report_pass("VM86");
 }
 
 #define IOPL_SHIFT 12
diff --git a/x86/tsc_adjust.c b/x86/tsc_adjust.c
index 1f26b7a..3636b5e 100644
--- a/x86/tsc_adjust.c
+++ b/x86/tsc_adjust.c
@@ -34,7 +34,7 @@ int main(void)
 		       "MSR_IA32_TSC_ADJUST msr adjustment on tsc write");
 	}
 	else {
-		report(true, "MSR_IA32_TSC_ADJUST feature not enabled");
+		report_pass("MSR_IA32_TSC_ADJUST feature not enabled");
 	}
 	return report_summary();
 }
diff --git a/x86/vmx.c b/x86/vmx.c
index 247de23..20dc677 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1127,9 +1127,9 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 	}
 
 	if (!bad_pt_ad)
-		report(true, "EPT - guest page table structures A=%d/D=%d",
-		       !!(expected_pt_ad & EPT_ACCESS_FLAG),
-		       !!(expected_pt_ad & EPT_DIRTY_FLAG));
+		report_pass("EPT - guest page table structures A=%d/D=%d",
+			    !!(expected_pt_ad & EPT_ACCESS_FLAG),
+			    !!(expected_pt_ad & EPT_DIRTY_FLAG));
 
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	offset_in_page = guest_addr & ((1 << EPT_LEVEL_SHIFT(l)) - 1);
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 872586a..3b97cfa 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -53,7 +53,7 @@ static inline void vmcall(void)
 
 static void basic_guest_main(void)
 {
-	report(1, "Basic VMX test");
+	report_pass("Basic VMX test");
 }
 
 static int basic_exit_handler(union exit_reason exit_reason)
@@ -68,7 +68,7 @@ static void vmenter_main(void)
 	u64 rax;
 	u64 rsp, resume_rsp;
 
-	report(1, "test vmlaunch");
+	report_pass("test vmlaunch");
 
 	asm volatile(
 		"mov %%rsp, %0\n\t"
@@ -1184,9 +1184,9 @@ static void ept_common(void)
 		if (vmx_get_test_stage() == 1) {
 			if (*((u32 *)data_page1) == MAGIC_VAL_3 &&
 					*((u32 *)data_page2) == MAGIC_VAL_2)
-				report(1, "EPT basic framework");
+				report_pass("EPT basic framework");
 			else
-				report(1, "EPT basic framework - remap");
+				report_pass("EPT basic framework - remap");
 		}
 	}
 	// Test EPT Misconfigurations
@@ -1897,7 +1897,7 @@ static int nmi_hlt_exit_handler(union exit_reason exit_reason)
             print_vmexit_info(exit_reason);
             return VMX_TEST_VMEXIT;
         }
-        report(true, "NMI intercept while running guest");
+        report_pass("NMI intercept while running guest");
         vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
         break;
 
@@ -2156,12 +2156,12 @@ static void disable_rdtscp_ud_handler(struct ex_regs *regs)
 {
 	switch (vmx_get_test_stage()) {
 	case 0:
-		report(true, "RDTSCP triggers #UD");
+		report_pass("RDTSCP triggers #UD");
 		vmx_inc_test_stage();
 		regs->rip += 3;
 		break;
 	case 2:
-		report(true, "RDPID triggers #UD");
+		report_pass("RDPID triggers #UD");
 		vmx_inc_test_stage();
 		regs->rip += 4;
 		break;
@@ -2328,7 +2328,7 @@ static void v2_null_test(void)
 {
 	test_set_guest(v2_null_test_guest);
 	enter_guest();
-	report(1, __func__);
+	report_pass(__func__);
 }
 
 static void v2_multiple_entries_test_guest(void)
@@ -2346,7 +2346,7 @@ static void v2_multiple_entries_test(void)
 	skip_exit_vmcall();
 	enter_guest();
 	TEST_ASSERT_EQ(vmx_get_test_stage(), 2);
-	report(1, __func__);
+	report_pass(__func__);
 }
 
 static int fixture_test_data = 1;
@@ -2377,7 +2377,7 @@ static void fixture_test_case1(void)
 	TEST_ASSERT_EQ(2, fixture_test_data);
 	enter_guest();
 	TEST_ASSERT_EQ(3, fixture_test_data);
-	report(1, __func__);
+	report_pass(__func__);
 }
 
 static void fixture_test_case2(void)
@@ -2386,7 +2386,7 @@ static void fixture_test_case2(void)
 	TEST_ASSERT_EQ(2, fixture_test_data);
 	enter_guest();
 	TEST_ASSERT_EQ(3, fixture_test_data);
-	report(1, __func__);
+	report_pass(__func__);
 }
 
 enum ept_access_op {
@@ -6971,7 +6971,7 @@ static void test_x2apic_wr(
 			       got, val);
 			apic_write(reg, restore_val);
 		} else {
-			report(true, "non-virtualized and write-only OK");
+			report_pass("non-virtualized and write-only OK");
 		}
 	}
 	skip_exit_insn();
@@ -9581,7 +9581,7 @@ static void vmx_eoi_bitmap_ioapic_scan_test(void)
 
 	/* Let L2 finish */
 	enter_guest();
-	report(1, __func__);
+	report_pass(__func__);
 }
 
 #define HLT_WITH_RVI_VECTOR		(0xf1)
@@ -9709,7 +9709,7 @@ static void vmx_apic_passthrough(bool set_irq_line_from_thread)
 
 	/* Let L2 finish */
 	enter_guest();
-	report(1, __func__);
+	report_pass(__func__);
 }
 
 static void vmx_apic_passthrough_test(void)
@@ -9761,7 +9761,7 @@ static void vmx_apic_passthrough_tpr_threshold_test(void)
 	asm volatile ("nop");
 	report(vmx_apic_passthrough_tpr_threshold_ipi_isr_fired, "self-IPI fired");
 
-	report(1, __func__);
+	report_pass(__func__);
 }
 
 static u64 init_signal_test_exit_reason;
@@ -9958,7 +9958,7 @@ static void vmx_sipi_test_guest(void)
 
 		/* First SIPI signal */
 		apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_STARTUP | APIC_INT_ASSERT, id_map[1]);
-		report(1, "BSP(L2): Send first SIPI to cpu[%d]", id_map[1]);
+		report_pass("BSP(L2): Send first SIPI to cpu[%d]", id_map[1]);
 
 		/* wait AP enter guest */
 		while (vmx_get_test_stage() != 2)
@@ -9967,7 +9967,7 @@ static void vmx_sipi_test_guest(void)
 
 		/* Second SIPI signal should be ignored since AP is not in WAIT_SIPI state */
 		apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_STARTUP | APIC_INT_ASSERT, id_map[1]);
-		report(1, "BSP(L2): Send second SIPI to cpu[%d]", id_map[1]);
+		report_pass("BSP(L2): Send second SIPI to cpu[%d]", id_map[1]);
 
 		/* Delay a while to check whether second SIPI would cause VMExit */
 		delay(SIPI_SIGNAL_TEST_DELAY);
@@ -10025,7 +10025,7 @@ static void sipi_test_ap_thread(void *data)
 	enter_guest();
 
 	if (vmcs_read(EXI_REASON) == VMX_SIPI) {
-		report(1, "AP: Handle SIPI VMExit");
+		report_pass("AP: Handle SIPI VMExit");
 		vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
 		vmx_set_test_stage(2);
 	} else {
-- 
2.31.1

Re: [kvm-unit-tests PATCH v2 5/5] Use report_pass(...) instead of report(1/true, ...)

[Thomas Huth]

On 05/10/2021 11.09, Janis Schoetterl-Glausch wrote:
> Whitespace is kept consistent with the rest of the file.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>   s390x/css.c         |  4 ++--
>   s390x/diag288.c     |  2 +-
>   s390x/selftest.c    |  2 +-
>   s390x/smp.c         | 16 ++++++++--------
>   s390x/spec_ex.c     |  7 +++----
>   x86/asyncpf.c       |  7 +++----
>   x86/emulator.c      |  2 +-
>   x86/hyperv_stimer.c | 18 ++++++++----------
>   x86/svm_tests.c     | 17 ++++++++---------
>   x86/syscall.c       |  2 +-
>   x86/taskswitch2.c   |  2 +-
>   x86/tsc_adjust.c    |  2 +-
>   x86/vmx.c           |  6 +++---
>   x86/vmx_tests.c     | 36 ++++++++++++++++++------------------
>   14 files changed, 59 insertions(+), 64 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH v2 5/5] Use report_pass(...) instead of report(1/true, ...)

[Thomas Huth]

On 05/10/2021 11.09, Janis Schoetterl-Glausch wrote:
> Whitespace is kept consistent with the rest of the file.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>   s390x/css.c         |  4 ++--
>   s390x/diag288.c     |  2 +-
>   s390x/selftest.c    |  2 +-
>   s390x/smp.c         | 16 ++++++++--------
>   s390x/spec_ex.c     |  7 +++----
>   x86/asyncpf.c       |  7 +++----
>   x86/emulator.c      |  2 +-
>   x86/hyperv_stimer.c | 18 ++++++++----------
>   x86/svm_tests.c     | 17 ++++++++---------
>   x86/syscall.c       |  2 +-
>   x86/taskswitch2.c   |  2 +-
>   x86/tsc_adjust.c    |  2 +-
>   x86/vmx.c           |  6 +++---
>   x86/vmx_tests.c     | 36 ++++++++++++++++++------------------
>   14 files changed, 59 insertions(+), 64 deletions(-)
> 
> diff --git a/s390x/css.c b/s390x/css.c
> index dcb4d70..881206b 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -31,7 +31,7 @@ static void test_enumerate(void)
>   {
>   	test_device_sid = css_enumerate();
>   	if (test_device_sid & SCHID_ONE) {
> -		report(1, "Schid of first I/O device: 0x%08x", test_device_sid);
> +		report_pass("Schid of first I/O device: 0x%08x", test_device_sid);
>   		return;
>   	}
...

Thanks, I've applied patches 3 - 5 now to the repository, since there were 
no objections and they are independent from your spec_ex work. (Dropped the 
hunk to spec_ex.c here of course, so please integrate that in the next 
version of that patch directly instead).

  Thomas

Re: [kvm-unit-tests PATCH v2 0/5] Add specification exception tests

[Thomas Huth]

On 05/10/2021 12.30, Janis Schoetterl-Glausch wrote:
> On Tue, Oct 05, 2021 at 11:13:22AM +0200, Thomas Huth wrote:
>> On 05/10/2021 11.09, Janis Schoetterl-Glausch wrote:
>>> Test that specification exceptions cause the correct interruption code
>>> during both normal and transactional execution.
>>>
>>> The last three patches are cosmetic only and could be dropped.
>>>
>>> Unrelated: There should not be * in the file patterns in MAINTAINERS,
>>> should there?
>>
>> I think those could be dropped, indeed. Care to send a patch?
> 
> You mean the * patterns, not the cosmetic patches, correct?
> 
> -- >8 --
> Subject: [kvm-unit-tests PATCH] MAINTAINERS: Include subdirectories in file
>   patterns
> 
> The * pattern does not cover subdirectories, so get_maintainer.pl does
> not know who maintains e.g. lib/<arch>/asm/*.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>   MAINTAINERS | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4fc01a5..15b5e1b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -68,9 +68,9 @@ M: Andrew Jones <drjones@redhat.com>
>   S: Supported
>   L: kvm@vger.kernel.org
>   L: kvmarm@lists.cs.columbia.edu
> -F: arm/*
> -F: lib/arm/*
> -F: lib/arm64/*
> +F: arm/
> +F: lib/arm/
> +F: lib/arm64/
>   T: https://gitlab.com/rhdrjones/kvm-unit-tests.git
>   
>   POWERPC
> @@ -79,9 +79,9 @@ M: Thomas Huth <thuth@redhat.com>
>   S: Maintained
>   L: kvm@vger.kernel.org
>   L: kvm-ppc@vger.kernel.org
> -F: powerpc/*
> -F: lib/powerpc/*
> -F: lib/ppc64/*
> +F: powerpc/
> +F: lib/powerpc/
> +F: lib/ppc64/
>   
>   S390X
>   M: Thomas Huth <thuth@redhat.com>
> @@ -92,13 +92,13 @@ R: Claudio Imbrenda <imbrenda@linux.ibm.com>
>   R: David Hildenbrand <david@redhat.com>
>   L: kvm@vger.kernel.org
>   L: linux-s390@vger.kernel.org
> -F: s390x/*
> -F: lib/s390x/*
> +F: s390x/
> +F: lib/s390x/
>   
>   X86
>   M: Paolo Bonzini <pbonzini@redhat.com>
>   S: Supported
>   L: kvm@vger.kernel.org
> -F: x86/*
> -F: lib/x86/*
> +F: x86/
> +F: lib/x86/
>   T: https://gitlab.com/bonzini/kvm-unit-tests.git
> 
> base-commit: fe26131eec769cef7ad7e2e1e4e192aa0bdb2bba
> 

Thanks, applied.

  Thomas