Re: [kvm-unit-tests PATCH v4 2/7] s390x: Add guest 2 AP test

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

 



On 2/20/24 17:38, Anthony Krowiak wrote:
I made a couple of function name change suggestions, but those are not
critical:

Acked-by: Anthony Krowiak <akrowiak@xxxxxxxxxxxxx>

On 2/2/24 9:59 AM, Janosch Frank wrote:
Add a test that checks the exceptions for the PQAP, NQAP and DQAP
adjunct processor (AP) crypto instructions.

Since triggering the exceptions doesn't require actual AP hardware,
this test can run without complicated setup.

Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
---
   s390x/Makefile      |   1 +
   s390x/ap.c          | 309 ++++++++++++++++++++++++++++++++++++++++++++
   s390x/unittests.cfg |   3 +
   3 files changed, 313 insertions(+)
   create mode 100644 s390x/ap.c

diff --git a/s390x/Makefile b/s390x/Makefile
index 4f6c627d..6d28a5bf 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -42,6 +42,7 @@ tests += $(TEST_DIR)/exittime.elf
   tests += $(TEST_DIR)/ex.elf
   tests += $(TEST_DIR)/topology.elf
   tests += $(TEST_DIR)/sie-dat.elf
+tests += $(TEST_DIR)/ap.elf
pv-tests += $(TEST_DIR)/pv-diags.elf
   pv-tests += $(TEST_DIR)/pv-icptcode.elf
diff --git a/s390x/ap.c b/s390x/ap.c
new file mode 100644
index 00000000..b3cee37a
--- /dev/null
+++ b/s390x/ap.c
@@ -0,0 +1,309 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AP instruction G2 tests
+ *
+ * Copyright (c) 2024 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <frankja@xxxxxxxxxxxxx>
+ */
+
+#include <libcflat.h>
+#include <interrupt.h>
+#include <bitops.h>
+#include <alloc_page.h>
+#include <asm/facility.h>
+#include <asm/time.h>
+#include <ap.h>
+
+/* For PQAP PGM checks where we need full control over the input */
+static void pqap(unsigned long grs[3])
+{
+	asm volatile(
+		"	lgr	0,%[r0]\n"
+		"	lgr	1,%[r1]\n"
+		"	lgr	2,%[r2]\n"
+		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
+		::  [r0] "d" (grs[0]), [r1] "d" (grs[1]), [r2] "d" (grs[2])
+		: "cc", "memory", "0", "1", "2");
+}
+
+static void test_pgms_pqap(void)


If I saw this function name without having read the patch description, I
wouldn't have any idea what is being tested.

Maybe test_pqap_pgm_chk?

If I'm not mistaken, then it should be consistent with the pgm checks for other instructions but I don't mind renaming it.



+{
+	unsigned long grs[3] = {};
+	struct pqap_r0 *r0 = (struct pqap_r0 *)grs;
+	uint8_t *data = alloc_page();
+	uint16_t pgm;
+	int fails = 0;
+	int i;
+
+	report_prefix_push("pqap");
+
+	/* Wrong FC code */
+	report_prefix_push("invalid fc");
+	r0->fc = 42;


Just out of curiosity, why 42? Why not some ridiculous number that will
never be used for a function code, like 4294967295?

No particular reason.

Increasing the number would increase the buffer zone of invalid values but there's currently only a hand full of values defined anyway.
I might add a couple of 0s to this for the next version and call it a day.





[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