x86-64: Implement/use load_cbne_load_add_load_add_store

Change the current load_add_load_load_add_store for a
load_cbne_load_add_load_add_store, which allows validating that the
percpu pointer did not change since it was loaded from C.

This allows precomputing the address of the percpu memory area in C and
provide it as a second pointer argument.

The comparison approach is prefered to the offset-from-pointer approach
because it does not leak implementation details of the percpu allocator.

Add missing rseq_after_asm_goto() in the static inline function.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I3fe9d57d13f7507d5af95ef37391ad36fe2221fe

diff --git a/include/rseq/arch/x86/bits.h b/include/rseq/arch/x86/bits.h
index df30fc6d6c6dbe01256c97170e88fb14eb361078..2bb493654f0a91dc5c0310f76aa95942dd0d0798 100644 (file)
--- a/include/rseq/arch/x86/bits.h
+++ b/include/rseq/arch/x86/bits.h
@@ -196,10 +196,13 @@ error1:
 #endif
 }
 
-#define rseq_arch_has_load_add_load_load_add_store
+#define rseq_arch_has_load_cbne_load_add_load_add_store
 
 static inline __attribute__((always_inline))
-int RSEQ_TEMPLATE_IDENTIFIER(rseq_load_add_load_load_add_store__ptr)(intptr_t *ptr, long off, intptr_t inc, int cpu)
+int
+RSEQ_TEMPLATE_IDENTIFIER(rseq_load_cbne_load_add_load_add_store__ptr)(intptr_t *ptr,
+               intptr_t expect, intptr_t *ptr2, ptrdiff_t offset,
+               intptr_t inc, int cpu)
 {
        RSEQ_INJECT_C(9)
 
@@ -215,36 +218,44 @@ int RSEQ_TEMPLATE_IDENTIFIER(rseq_load_add_load_load_add_store__ptr)(intptr_t *p
 #ifdef RSEQ_COMPARE_TWICE
                RSEQ_ASM_CBNE_CPU_ID(cpu_id, RSEQ_ASM_TP_SEGMENT:RSEQ_TEMPLATE_INDEX_CPU_ID_OFFSET(%[rseq_offset]), %l[error1])
 #endif
-               /* get p+v */
                "movq %[ptr], %%rbx\n\t"
-               "addq %[off], %%rbx\n\t"
-               /* get pv */
-               "movq (%%rbx), %%rcx\n\t"
-               /* *pv += inc */
-               "addq %[inc], (%%rcx)\n\t"
-               "2:\n\t"
+               "cmpq %%rbx, %[expect]\n\t"
+               "jne %l[ne]\n\t"
                RSEQ_INJECT_ASM(4)
+               "movq %[ptr2], %%rbx\n\t"
+               "addq %[offset], %%rbx\n\t"
+               "addq %[inc], (%%rbx)\n\t"
+               "2:\n\t"
+               RSEQ_INJECT_ASM(5)
                RSEQ_ASM_DEFINE_ABORT(4, "", abort)
                : /* gcc asm goto does not allow outputs */
                : [cpu_id]              "r" (cpu),
                  [rseq_offset]         "r" (rseq_offset),
                  /* final store input */
                  [ptr]                 "m" (*ptr),
-                 [off]                 "er" (off),
+                 [expect]              "r" (expect),
+                 [ptr2]                "m" (*ptr2),
+                 [offset]              "er" (offset),
                  [inc]                 "er" (inc)
-               : "memory", "cc", "rax", "rbx", "rcx"
+               : "memory", "cc", "rax", "rbx"
                  RSEQ_INJECT_CLOBBER
-               : abort
+               : abort, ne
 #ifdef RSEQ_COMPARE_TWICE
                  , error1
 #endif
        );
+       rseq_after_asm_goto();
        return 0;
 abort:
+       rseq_after_asm_goto();
        RSEQ_INJECT_FAILED
        return -1;
+ne:
+       rseq_after_asm_goto();
+       return 1;
 #ifdef RSEQ_COMPARE_TWICE
 error1:
+       rseq_after_asm_goto();
        rseq_bug("cpu_id comparison failed");
 #endif
 }

diff --git a/include/rseq/pseudocode.h b/include/rseq/pseudocode.h
index c6fb9494475ee06d425adcf7bc1aa47678bd38cf..426dbf5d7e7c4b434da84295da4460f1fc35d401 100644 (file)
--- a/include/rseq/pseudocode.h
+++ b/include/rseq/pseudocode.h
@@ -77,12 +77,13 @@
  */
 
 /*
- * rseq_load_add_load_load_add_store(ptr, off, inc)
+ * rseq_load_cbne_load_add_load_add_store(ptr, expect, ptr2, offset, inc)
  *
  * Pseudo-code:
  *   load(r1, [ptr])
- *   add(r1, [off])
- *   load(r2, r1)
+ *   cbne(r1, [expect])
+ *   load(r2, [ptr2])
+ *   add(r2, [offset])
  *   load(r3, r2)
  *   add(r3, [inc])
  *   store(r3, r2)

diff --git a/include/rseq/rseq.h b/include/rseq/rseq.h
index ea795c4cdea74142a9419628e65ca448efb4cc1c..8a0444f3ec29a97d116cfa9be6194cdc1c296659 100644 (file)
--- a/include/rseq/rseq.h
+++ b/include/rseq/rseq.h
@@ -273,18 +273,19 @@ int rseq_load_add_store__ptr(enum rseq_mo rseq_mo, enum rseq_percpu_mode percpu_
        }
 }
 
-#ifdef rseq_arch_has_load_add_load_load_add_store
+#ifdef rseq_arch_has_load_cbne_load_add_load_add_store
 static inline __attribute__((always_inline))
-int rseq_load_add_load_load_add_store__ptr(enum rseq_mo rseq_mo, enum rseq_percpu_mode percpu_mode,
-                          intptr_t *ptr, long off, intptr_t inc, int cpu)
+int rseq_load_cbne_load_add_load_add_store__ptr(enum rseq_mo rseq_mo, enum rseq_percpu_mode percpu_mode,
+                          intptr_t *ptr, intptr_t expect, intptr_t *ptr2, ptrdiff_t offset,
+                          intptr_t inc, int cpu)
 {
        if (rseq_mo != RSEQ_MO_RELAXED)
                return -1;
        switch (percpu_mode) {
        case RSEQ_PERCPU_CPU_ID:
-               return rseq_load_add_load_load_add_store__ptr_relaxed_cpu_id(ptr, off, inc, cpu);
+               return rseq_load_cbne_load_add_load_add_store__ptr_relaxed_cpu_id(ptr, expect, ptr2, offset, inc, cpu);
        case RSEQ_PERCPU_MM_CID:
-               return rseq_load_add_load_load_add_store__ptr_relaxed_mm_cid(ptr, off, inc, cpu);
+               return rseq_load_cbne_load_add_load_add_store__ptr_relaxed_mm_cid(ptr, expect, ptr2, offset, inc, cpu);
        default:
                return -1;
        }

diff --git a/tests/param_test.c b/tests/param_test.c
index c8ac956d699dc08ace060766993ed3ab33c7c27a..644d6387c7d5cc9d680d3bf539107cc93544c950 100644 (file)
--- a/tests/param_test.c
+++ b/tests/param_test.c
@@ -297,7 +297,7 @@ static int sys_membarrier(int cmd, int flags, int cpu_id)
        return syscall(__NR_membarrier, cmd, flags, cpu_id);
 }
 
-#ifdef rseq_arch_has_load_add_load_load_add_store
+#ifdef rseq_arch_has_load_cbne_load_add_load_add_store
 #define TEST_MEMBARRIER
 #endif
 
@@ -1386,12 +1386,12 @@ void *test_membarrier_worker_thread(void *arg)
 
                do {
                        int cpu = get_current_cpu_id();
-                       ptrdiff_t mempool_offset = rseq_percpu_pool_ptr_offset(args->mempool, cpu);
+                       struct percpu_list __rseq_percpu *list = RSEQ_READ_ONCE(args->percpu_list_ptr);
+                       struct percpu_list *cpulist = rseq_percpu_ptr(list, cpu);
 
-                       ret = rseq_load_add_load_load_add_store__ptr(RSEQ_MO_RELAXED, RSEQ_PERCPU,
+                       ret = rseq_load_cbne_load_add_load_add_store__ptr(RSEQ_MO_RELAXED, RSEQ_PERCPU,
                                (intptr_t *) &args->percpu_list_ptr,
-                               mempool_offset + offsetof(struct percpu_list, head),
-                               1, cpu);
+                               (intptr_t) list, (intptr_t *) &cpulist->head, 0, 1, cpu);
                } while (rseq_unlikely(ret));
        }
 
@@ -1627,7 +1627,7 @@ void test_membarrier(void)
                                "Skipping membarrier test.\n");
                return;
        }
-       fprintf(stderr, "rseq_load_add_load_load_add_store__ptr is not implemented on this architecture. "
+       fprintf(stderr, "rseq_load_cbne_load_add_load_add_store__ptr is not implemented on this architecture. "
                        "Skipping membarrier test.\n");
 }
 #endif