Re: [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test

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(a)linux.ibm.com&gt; >> >> 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(a)linux.ibm.com&gt; >> Message-Id: <20241001150617.9977-1-iii(a)linux.ibm.com&gt; >> Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; >> --- >>   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. [...]