Re: [PATCH qemu v4 2/2] tests/tcg/aarch64: Add testcases for IC IVAU and dual-mapped code

2023-06-27 Thread Peter Maydell
On Mon, 26 Jun 2023 at 15:15, ~jhogberg  wrote:
>
> From: John Högberg 
>
> https://gitlab.com/qemu-project/qemu/-/issues/1034
>
> Signed-off-by: John Högberg 


> +static int self_modification_test(uint32_t *rw_data, const uint32_t 
> *exec_data)
> +{
> +/*
> + * This test is self-modifying in an attempt to cover an edge case where
> + * the IC IVAU instruction invalidates itself.
> + *
> + * Note that the IC IVAU instruction is 16 bytes into the function, in 
> what
> + * will be the same cache line as the modifed instruction on machines 
> with

typo: "modified".

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM



[PATCH qemu v4 2/2] tests/tcg/aarch64: Add testcases for IC IVAU and dual-mapped code

2023-06-26 Thread ~jhogberg
From: John Högberg 

https://gitlab.com/qemu-project/qemu/-/issues/1034

Signed-off-by: John Högberg 
---
 tests/tcg/aarch64/Makefile.target |   3 +-
 tests/tcg/aarch64/icivau.c| 189 ++
 2 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/aarch64/icivau.c

diff --git a/tests/tcg/aarch64/Makefile.target 
b/tests/tcg/aarch64/Makefile.target
index 3430fd3cd8..de6566d0d4 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -9,9 +9,10 @@ AARCH64_SRC=$(SRC_PATH)/tests/tcg/aarch64
 VPATH  += $(AARCH64_SRC)
 
 # Base architecture tests
-AARCH64_TESTS=fcvt pcalign-a64
+AARCH64_TESTS=fcvt pcalign-a64 icivau
 
 fcvt: LDFLAGS+=-lm
+icivau: LDFLAGS+=-lrt
 
 run-fcvt: fcvt
$(call run-test,$<,$(QEMU) $<, "$< on $(TARGET_NAME)")
diff --git a/tests/tcg/aarch64/icivau.c b/tests/tcg/aarch64/icivau.c
new file mode 100644
index 00..e3e8569912
--- /dev/null
+++ b/tests/tcg/aarch64/icivau.c
@@ -0,0 +1,189 @@
+/*
+ * Tests the IC IVAU-driven workaround for catching changes made to dual-mapped
+ * code that would otherwise go unnoticed in user mode.
+ *
+ * Copyright (c) 2023 Ericsson AB
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MAX_CODE_SIZE 128
+
+typedef int (SelfModTest)(uint32_t, uint32_t*);
+typedef int (BasicTest)(int);
+
+static void mark_code_modified(const uint32_t *exec_data, size_t length)
+{
+int dc_required, ic_required;
+unsigned long ctr_el0;
+
+/*
+ * Clear the data/instruction cache, as indicated by the CTR_ELO.{DIC,IDC}
+ * flags.
+ *
+ * For completeness we might be tempted to assert that we should fail when
+ * the whole code update sequence is omitted, but that would make the test
+ * flaky as it can succeed by coincidence on actual hardware.
+ */
+asm ("mrs %0, ctr_el0\n" : "=r"(ctr_el0));
+
+/* CTR_EL0.IDC */
+dc_required = !((ctr_el0 >> 28) & 1);
+
+/* CTR_EL0.DIC */
+ic_required = !((ctr_el0 >> 29) & 1);
+
+if (dc_required) {
+size_t dcache_stride, i;
+
+/*
+ * Step according to the minimum cache size, as the cache maintenance
+ * instructions operate on the cache line of the given address.
+ *
+ * We assume that exec_data is properly aligned.
+ */
+dcache_stride = (4 << ((ctr_el0 >> 16) & 0xF));
+
+for (i = 0; i < length; i += dcache_stride) {
+const char *dc_addr = &((const char *)exec_data)[i];
+asm volatile ("dc cvau, %x[dc_addr]\n"
+  : /* no outputs */
+  : [dc_addr] "r"(dc_addr)
+  : "memory");
+}
+
+asm volatile ("dmb ish\n");
+}
+
+if (ic_required) {
+size_t icache_stride, i;
+
+icache_stride = (4 << (ctr_el0 & 0xF));
+
+for (i = 0; i < length; i += icache_stride) {
+const char *ic_addr = &((const char *)exec_data)[i];
+asm volatile ("ic ivau, %x[ic_addr]\n"
+  : /* no outputs */
+  : [ic_addr] "r"(ic_addr)
+  : "memory");
+}
+
+asm volatile ("dmb ish\n");
+}
+
+asm volatile ("isb sy\n");
+}
+
+static int basic_test(uint32_t *rw_data, const uint32_t *exec_data)
+{
+/*
+ * As user mode only misbehaved for dual-mapped code when previously
+ * translated code had been changed, we'll start off with this basic test
+ * function to ensure that there's already some translated code at
+ * exec_data before the next test. This should cause the next test to fail
+ * if `mark_code_modified` fails to invalidate the code.
+ *
+ * Note that the payload is in binary form instead of inline assembler
+ * because we cannot use __attribute__((naked)) on this platform and the
+ * workarounds are at least as ugly as this is.
+ */
+static const uint32_t basic_payload[] = {
+0xD65F03C0 /* 0x00: RET */
+};
+
+BasicTest *copied_ptr = (BasicTest *)exec_data;
+
+memcpy(rw_data, basic_payload, sizeof(basic_payload));
+mark_code_modified(exec_data, sizeof(basic_payload));
+
+return copied_ptr(1234) == 1234;
+}
+
+static int self_modification_test(uint32_t *rw_data, const uint32_t *exec_data)
+{
+/*
+ * This test is self-modifying in an attempt to cover an edge case where
+ * the IC IVAU instruction invalidates itself.
+ *
+ * Note that the IC IVAU instruction is 16 bytes into the function, in what
+ * will be the same cache line as the modifed instruction on machines with
+ * a cache line size >= 16 bytes.
+ */
+static const uint32_t self_mod_payload[] = {
+/* Overwrite the placeholder instruction with the new one. */
+0xB9001C20, /* 0x00: STR w0, [x1, 0x1C] */
+
+/* Get the