Re: [kvm-unit-tests PATCH v1 2/2] s390x: add exittime tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 30/08/2022 13.56, Nico Boehr wrote:
Add a test to measure the execution time of several instructions. This
can be helpful in finding performance regressions in hypervisor code.

All tests are currently reported as PASS, since the baseline for their
execution time depends on the respective environment and since needs to
be determined on a case-by-case basis.

Signed-off-by: Nico Boehr <nrb@xxxxxxxxxxxxx>
---
  s390x/Makefile      |   1 +
  s390x/exittime.c    | 258 ++++++++++++++++++++++++++++++++++++++++++++
  s390x/unittests.cfg |   4 +
  3 files changed, 263 insertions(+)
  create mode 100644 s390x/exittime.c

diff --git a/s390x/Makefile b/s390x/Makefile
index efd5e0c13102..5dcac244767f 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -34,6 +34,7 @@ tests += $(TEST_DIR)/migration.elf
  tests += $(TEST_DIR)/pv-attest.elf
  tests += $(TEST_DIR)/migration-cmm.elf
  tests += $(TEST_DIR)/migration-skey.elf
+tests += $(TEST_DIR)/exittime.elf
pv-tests += $(TEST_DIR)/pv-diags.elf diff --git a/s390x/exittime.c b/s390x/exittime.c
new file mode 100644
index 000000000000..dc8329116b77
--- /dev/null
+++ b/s390x/exittime.c
@@ -0,0 +1,258 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Measure run time of various instructions. Can be used to find runtime
+ * regressions of instructions which cause exits.
+ *
+ * Copyright IBM Corp. 2022
+ *
+ * Authors:
+ *  Nico Boehr <nrb@xxxxxxxxxxxxx>
+ */
+#include <libcflat.h>
+#include <smp.h>
+#include <sclp.h>
+#include <asm/time.h>
+#include <asm/sigp.h>
+#include <asm/interrupt.h>
+#include <asm/page.h>
+
+char pagebuf[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
+
+static void test_sigp_sense_running(long destcpu)
+{
+	smp_sigp(destcpu, SIGP_SENSE_RUNNING, 0, NULL);
+}
+
+static void test_nop(long ignore)
+{
+	asm volatile("nop" : : : "memory");
+}

What's the purpose of testing "nop"? ... it does not trap to the hypervisor... ? Is it just for reference? Then a comment might be helpful here.

+static void test_diag9c(long destcpu)
+{
+	asm volatile("diag %[destcpu],0,0x9c"
+		:
+		: [destcpu] "d" (destcpu)
+		:
+	);
+}
+
+static long setup_get_this_cpuaddr(long ignore)
+{
+	return stap();
+}
+
+static void test_diag44(long ignore)
+{
+	asm volatile("diag 0,0,0x44" : : : );

Drop the " : : : " please.

+}
+
+static void test_stnsm(long ignore)
+{
+	int out;
+
+	asm volatile(
+		"stnsm %[out],0xff"
+		: [out] "=Q" (out)
+		:
+		: "memory"

I don't think you need the "memory" clobber here, do you?

+	);
+}
+
+static void test_stosm(long ignore)
+{
+	int out;
+
+	asm volatile(
+		"stosm %[out],0"
+		: [out] "=Q" (out)
+		:
+		: "memory"

dito

+	);
+}
+
+static long setup_ssm(long ignore)
+{
+	long system_mask = 0;
+
+	asm volatile(
+		"stosm %[system_mask],0"
+		: [system_mask] "=Q" (system_mask)
+		:
+		: "memory"
+	);
+
+	return system_mask;
+}
+
+static void test_ssm(long old_system_mask)
+{
+	asm volatile(
+		"ssm %[old_system_mask]"
+		:
+		: [old_system_mask] "Q" (old_system_mask)
+		:
+	);
+}
+
+static long setup_lctl4(long ignore)
+{
+	long ctl4_orig = 0;
+
+	asm volatile(
+		"stctg 4,4,%[ctl4_orig]"
+		: [ctl4_orig] "=S" (ctl4_orig)
+		:
+		: "memory"
+	);
+
+	return ctl4_orig;
+}
+
+static void test_lctl4(long ctl4_orig)
+{
+	asm volatile(
+		"lctlg 4,4,%[ctl4_orig]"
+		:
+		: [ctl4_orig] "S" (ctl4_orig)
+		:
+	);
+}
+
+static void test_stpx(long ignore)
+{
+	unsigned int prefix;
+
+	asm volatile(
+		"stpx %[prefix]"
+		: [prefix] "=S" (prefix)

STPX seems to have only a short displacement, so it should have "=Q" instead of "=S" ?

+		:
+		: "memory"
+	);
+}
+
+static void test_stfl(long ignore)
+{
+	asm volatile(
+		"stfl 0" : : : "memory"
+	);
+}
+
+static void test_epsw(long ignore)
+{
+	long r1, r2;
+
+	asm volatile(
+		"epsw %[r1], %[r2]"
+		: [r1] "=d" (r1), [r2] "=d" (r2)
+		:
+		:
+	);
+}
+
+static void test_illegal(long ignore)
+{
+	expect_pgm_int();
+	asm volatile(
+		".word 0"
+		:
+		:
+		:
+	);
+	clear_pgm_int();
+}
+
+static long setup_servc(long arg)
+{
+	memset(pagebuf, 0, PAGE_SIZE);
+	return arg;
+}
+
+static void test_servc(long ignore)
+{
+	SCCB *sccb = (SCCB*) pagebuf;
+	sccb->h.length = 8;
+	servc(0, (unsigned long) sccb);
+}
+
+static void test_stsi(long fc)
+{
+	stsi(pagebuf, fc, 2, 2);
+}
+
+struct test {
+	char name[100];

"const char *name" instead of using an array here, please. Otherwise this wastes a lot of space in the binary.

...
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index f7b1fc3dbca1..c11d1d987c82 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -185,3 +185,7 @@ groups = migration
  [migration-skey]
  file = migration-skey.elf
  groups = migration
+
+[exittime]
+file = exittime.elf
+smp = 2

I wonder whether we should execute this test by default, since nothing can fail here? I assume this is rather something that you want to run manually?

 Thomas




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux