On 10/22/24 17:16, Ilya Leoshkevich wrote:
On Tue, 2024-10-22 at 13:36 -0700, Pierrick Bouvier wrote:
On 10/22/24 03:56, Alex Bennée wrote:
From: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
commit f025692c992c ("accel/tcg: Clear PAGE_WRITE before
translation")
fixed cross-modifying code handling, but did not add a test. The
changed code was further improved recently [1], and I was not sure
whether these modifications were safe (spoiler: they were fine).
Add a test to make sure there are no regressions.
[1]
https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00034.html
Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
Message-Id: <20241001150617.9977-1-iii@xxxxxxxxxxxxx>
Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
---
tests/tcg/x86_64/cross-modifying-code.c | 80
+++++++++++++++++++++++++
tests/tcg/x86_64/Makefile.target | 4 ++
2 files changed, 84 insertions(+)
create mode 100644 tests/tcg/x86_64/cross-modifying-code.c
diff --git a/tests/tcg/x86_64/cross-modifying-code.c
b/tests/tcg/x86_64/cross-modifying-code.c
new file mode 100644
index 0000000000..2704df6061
--- /dev/null
+++ b/tests/tcg/x86_64/cross-modifying-code.c
@@ -0,0 +1,80 @@
+/*
+ * Test patching code, running in one thread, from another thread.
+ *
+ * Intel SDM calls this "cross-modifying code" and recommends a
special
+ * sequence, which requires both threads to cooperate.
+ *
+ * Linux kernel uses a different sequence that does not require
cooperation and
+ * involves patching the first byte with int3.
+ *
+ * Finally, there is user-mode software out there that simply uses
atomics, and
+ * that seems to be good enough in practice. Test that QEMU has no
problems
+ * with this as well.
+ */
+
+#include <assert.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <stdlib.h>
+
+void add1_or_nop(long *x);
+asm(".pushsection .rwx,\"awx\",@progbits\n"
+ ".globl add1_or_nop\n"
+ /* addq $0x1,(%rdi) */
+ "add1_or_nop: .byte 0x48, 0x83, 0x07, 0x01\n"
+ "ret\n"
+ ".popsection\n");
+
+#define THREAD_WAIT 0
+#define THREAD_PATCH 1
+#define THREAD_STOP 2
+
+static void *thread_func(void *arg)
+{
+ int val = 0x0026748d; /* nop */
+
+ while (true) {
+ switch (__atomic_load_n((int *)arg, __ATOMIC_SEQ_CST)) {
+ case THREAD_WAIT:
+ break;
+ case THREAD_PATCH:
+ val = __atomic_exchange_n((int *)&add1_or_nop, val,
+ __ATOMIC_SEQ_CST);
+ break;
+ case THREAD_STOP:
+ return NULL;
+ default:
+ assert(false);
+ __builtin_unreachable();
Use g_assert_not_reached() instead.
checkpatch emits an error for it now.
Is there an easy way to include glib from testcases?
It's located using meson, and I can't immediately see how to push the
respective compiler flags to the test Makefiles - this seems to be
currently handled by configure writing to $config_target_mak.
[...]
Sorry you're right, I missed the fact tests don't have the deps we have
in QEMU itself.
I don't think any test case include any extra dependency for now (and
would make it hard to cross compile them too), so it's not worth trying
to get the right glib header for this.
I don't now if it will be a problem when merging the series regarding
checkpatch, but if it is, we can always replace this by abort, or exit.
As it is,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@xxxxxxxxxx>
