diff --git a/README.adoc b/README.adoc index 1bd8ad7..5826418 100644 --- a/README.adoc +++ b/README.adoc @@ -22969,6 +22969,10 @@ Examples: * link:userland/arch/aarch64/inline_asm/wfe_ldxr_stxr.cpp[] * link:userland/arch/aarch64/inline_asm/wfe_ldxr_str.cpp[] * link:userland/arch/aarch64/inline_asm/futex_ldxr_stxr.c[]: tests that ldxr and stxr do not interact with futexes. This was leading to problems in <> at one point: https://gem5.atlassian.net/browse/GEM5-537 ++ +Correct outcome: <>. ++ +Incorrect behaviour due to: https://gem5.atlassian.net/browse/GEM5-537[]: Exits successfully. */ SEV is not the only thing that can wake up a WFE, it is only an explicit software way to do it. diff --git a/lkmc/futex.h b/lkmc/futex.h index c2e6923..4301777 100644 --- a/lkmc/futex.h +++ b/lkmc/futex.h @@ -26,8 +26,6 @@ lkmc_futex(int *uaddr, int futex_op, int val, : "memory" ); return x0; - //return syscall(SYS_futex, uaddr, futex_op, val, - // timeout, uaddr2, val3); } #endif diff --git a/userland/arch/aarch64/inline_asm/futex_ldxr_stxr.c b/userland/arch/aarch64/inline_asm/futex_ldxr_stxr.c index 9fa9e14..6f4293f 100644 --- a/userland/arch/aarch64/inline_asm/futex_ldxr_stxr.c +++ b/userland/arch/aarch64/inline_asm/futex_ldxr_stxr.c @@ -12,11 +12,15 @@ #include +#define LDXR_OPS_SIZE 1024 static int futex1 = 1; static int futex2 = 1; -atomic_int ldxr_done = 0; -atomic_int stdr_wake_done = 0; -static uint64_t ldxr_var = 0; +/* We do this to ensure that those two varibles are well separated. + * If they are too close (same cache line?), then the str to ldxr_done + * can make CPU1 lose the lock. */ +static uint64_t ldxr_ops[LDXR_OPS_SIZE]; +static uint64_t *ldxr_done = ldxr_ops; +static uint64_t *ldxr_var = ldxr_ops + LDXR_OPS_SIZE - 1; void __attribute__ ((noinline)) busy_loop( unsigned long long max, @@ -32,11 +36,11 @@ void __attribute__ ((noinline)) busy_loop( void* thread_main(void *arg) { (void)arg; __asm__ __volatile__ ( - "ldxr x0, [%0]" - : - : "r" (&ldxr_var) : "x0", "memory" + "ldxr x0, [%[ldxr_var]];mov %[ldxr_done], 1" + : [ldxr_done] "=r" (*ldxr_done) + : [ldxr_var] "r" (ldxr_var) + : "x0", "memory" ); - ldxr_done = 1; lkmc_futex(&futex1, FUTEX_WAIT, futex1, NULL, NULL, 0); lkmc_futex(&futex2, FUTEX_WAIT, futex2, NULL, NULL, 0); return NULL; @@ -45,19 +49,21 @@ void* thread_main(void *arg) { int main(void) { pthread_t thread; pthread_create(&thread, NULL, thread_main, NULL); - while (!ldxr_done) {} + while (!*ldxr_done) {} /* Wait for thread1 to sleep on futex1. */ busy_loop(1000, 1); - /* Wrongly wake up the thread with a SEV. */ + /* Try to wake up the thread with an LLSC event. + * It should not wake up in a correct implementation, + * but it used to happen in gem5 before it was fixed. */ __asm__ __volatile__ ( "mov x0, 1;ldxr x0, [%0]; stxr w1, x0, [%0]" : - : "r" (&ldxr_var) : "x0", "x1", "memory" + : "r" (ldxr_var) : "x0", "x1", "memory" ); /* Wait for thread1 to sleep on futex2. */ busy_loop(1000, 1); - /* Wrongly wake thread from futex1 again. */ - /* But it is now sleeping on futex2, so this is wrong. */ + /* Before it was fixed in gem5, this would wrongly wake a futex2 + * because the previous futex1 was woken up via LLSC. */ lkmc_futex(&futex1, FUTEX_WAKE, 1, NULL, NULL, 0); assert(!pthread_join(thread, NULL)); }