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. [...]