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

[Ilya Leoshkevich <iii@linux.ibm.com>]

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@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.

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.

[...]