| Message ID | 20241022105614.839199-8-alex.bennee@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | maintainer updates (testing, gdbstub, plugins) | expand |
On 10/22/24 03:56, Alex Bennée wrote: > From: Ilya Leoshkevich <iii@linux.ibm.com> > > 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@linux.ibm.com> > Message-Id: <20241001150617.9977-1-iii@linux.ibm.com> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > 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. > + } > + } > +} > + > +#define INITIAL 42 > +#define COUNT 1000000 > + > +int main(void) > +{ > + int command = THREAD_WAIT; > + pthread_t thread; > + long x = 0; > + int err; > + int i; > + > + err = pthread_create(&thread, NULL, &thread_func, &command); > + assert(err == 0); > + > + __atomic_store_n(&command, THREAD_PATCH, __ATOMIC_SEQ_CST); > + for (i = 0; i < COUNT; i++) { > + add1_or_nop(&x); > + } > + __atomic_store_n(&command, THREAD_STOP, __ATOMIC_SEQ_CST); > + > + err = pthread_join(thread, NULL); > + assert(err == 0); > + > + assert(x >= INITIAL); > + assert(x <= INITIAL + COUNT); > + > + return EXIT_SUCCESS; > +} > diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target > index 783ab5b21a..d6dff559c7 100644 > --- a/tests/tcg/x86_64/Makefile.target > +++ b/tests/tcg/x86_64/Makefile.target > @@ -17,6 +17,7 @@ X86_64_TESTS += cmpxchg > X86_64_TESTS += adox > X86_64_TESTS += test-1648 > X86_64_TESTS += test-2175 > +X86_64_TESTS += cross-modifying-code > TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64 > else > TESTS=$(MULTIARCH_TESTS) > @@ -27,6 +28,9 @@ adox: CFLAGS=-O2 > run-test-i386-ssse3: QEMU_OPTS += -cpu max > run-plugin-test-i386-ssse3-%: QEMU_OPTS += -cpu max > > +cross-modifying-code: CFLAGS+=-pthread > +cross-modifying-code: LDFLAGS+=-pthread > + > test-x86_64: LDFLAGS+=-lm -lc > test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h > $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS) With this change, Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
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(); + } + } +} + +#define INITIAL 42 +#define COUNT 1000000 + +int main(void) +{ + int command = THREAD_WAIT; + pthread_t thread; + long x = 0; + int err; + int i; + + err = pthread_create(&thread, NULL, &thread_func, &command); + assert(err == 0); + + __atomic_store_n(&command, THREAD_PATCH, __ATOMIC_SEQ_CST); + for (i = 0; i < COUNT; i++) { + add1_or_nop(&x); + } + __atomic_store_n(&command, THREAD_STOP, __ATOMIC_SEQ_CST); + + err = pthread_join(thread, NULL); + assert(err == 0); + + assert(x >= INITIAL); + assert(x <= INITIAL + COUNT); + + return EXIT_SUCCESS; +} diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target index 783ab5b21a..d6dff559c7 100644 --- a/tests/tcg/x86_64/Makefile.target +++ b/tests/tcg/x86_64/Makefile.target @@ -17,6 +17,7 @@ X86_64_TESTS += cmpxchg X86_64_TESTS += adox X86_64_TESTS += test-1648 X86_64_TESTS += test-2175 +X86_64_TESTS += cross-modifying-code TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64 else TESTS=$(MULTIARCH_TESTS) @@ -27,6 +28,9 @@ adox: CFLAGS=-O2 run-test-i386-ssse3: QEMU_OPTS += -cpu max run-plugin-test-i386-ssse3-%: QEMU_OPTS += -cpu max +cross-modifying-code: CFLAGS+=-pthread +cross-modifying-code: LDFLAGS+=-pthread + test-x86_64: LDFLAGS+=-lm -lc test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)