* [PATCH v3 00/10] Atomic cleanup + clang-12 build fix
@ 2021-07-17 19:24 Richard Henderson
2021-07-17 19:24 ` [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64 Richard Henderson
` (10 more replies)
0 siblings, 11 replies; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, crobinso
This is intended to fix building with clang-12 on i386.
In the process, I found bugs wrt handling of guest memory in target/
with respect to atomics, fixed by unifying the api between softmmu
and user-only and removing some ifdefs under target/.
Unification of the api allowed some further cleanups.
I think that patches 1-5 fix all of the bugs, and that 6-10 are only
cleanup and could be left to next cycle.
Changes for v3:
* Dropped the typeof_strip_qual patch with broader testing.
* Squashed an include fix for trace/mem.h, with plugins enabled.
* Applied Phil's R-b.
r~
Richard Henderson (10):
qemu/atomic: Use macros for CONFIG_ATOMIC64
qemu/atomic: Remove pre-C11 atomic fallbacks
qemu/atomic: Add aligned_{int64,uint64}_t types
tcg: Rename helper_atomic_*_mmu and provide for user-only
accel/tcg: Standardize atomic helpers on softmmu api
accel/tcg: Fold EXTRA_ARGS into atomic_template.h
accel/tcg: Remove ATOMIC_MMU_DECLS
accel/tcg: Expand ATOMIC_MMU_LOOKUP_*
trace: Fold mem-internal.h into mem.h
accel/tcg: Push trace info building into atomic_common.c.inc
configure | 7 -
accel/tcg/atomic_template.h | 141 +++++++++-----------
accel/tcg/tcg-runtime.h | 46 -------
include/qemu/atomic.h | 243 ++++------------------------------
include/qemu/stats64.h | 2 +-
include/tcg/tcg.h | 78 +++++------
softmmu/timers-state.h | 2 +-
trace/mem-internal.h | 50 -------
trace/mem.h | 50 +++++--
accel/tcg/cputlb.c | 49 +------
accel/tcg/user-exec.c | 41 +++---
linux-user/hppa/cpu_loop.c | 2 +-
plugins/core.c | 2 +-
target/arm/helper-a64.c | 8 +-
target/i386/tcg/mem_helper.c | 15 +--
target/m68k/op_helper.c | 19 +--
target/ppc/mem_helper.c | 16 +--
target/s390x/tcg/mem_helper.c | 19 +--
tcg/tcg-op.c | 47 ++-----
util/qsp.c | 4 +-
accel/tcg/atomic_common.c.inc | 107 +++++++++++++--
21 files changed, 321 insertions(+), 627 deletions(-)
delete mode 100644 trace/mem-internal.h
--
2.25.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:01 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks Richard Henderson
` (9 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, Philippe Mathieu-Daudé, crobinso
Clang warnings about questionable atomic usage get localized
to the inline function in atomic.h. By using a macro, we get
the full traceback to the original use that caused the warning.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/atomic.h | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 3ccf84fd46..99d6030095 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -457,26 +457,11 @@
/* Abstractions to access atomically (i.e. "once") i64/u64 variables */
#ifdef CONFIG_ATOMIC64
-static inline int64_t qatomic_read_i64(const int64_t *ptr)
-{
- /* use __nocheck because sizeof(void *) might be < sizeof(u64) */
- return qatomic_read__nocheck(ptr);
-}
-
-static inline uint64_t qatomic_read_u64(const uint64_t *ptr)
-{
- return qatomic_read__nocheck(ptr);
-}
-
-static inline void qatomic_set_i64(int64_t *ptr, int64_t val)
-{
- qatomic_set__nocheck(ptr, val);
-}
-
-static inline void qatomic_set_u64(uint64_t *ptr, uint64_t val)
-{
- qatomic_set__nocheck(ptr, val);
-}
+/* Use __nocheck because sizeof(void *) might be < sizeof(u64) */
+#define qatomic_read_i64 qatomic_read__nocheck
+#define qatomic_read_u64 qatomic_read__nocheck
+#define qatomic_set_i64 qatomic_set__nocheck
+#define qatomic_set_u64 qatomic_set__nocheck
static inline void qatomic64_init(void)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
2021-07-17 19:24 ` [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64 Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:05 ` Peter Maydell
2021-07-19 15:40 ` Alex Bennée
2021-07-17 19:24 ` [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types Richard Henderson
` (8 subsequent siblings)
10 siblings, 2 replies; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, crobinso
We now require c11, so the fallbacks are now dead code
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
configure | 7 --
include/qemu/atomic.h | 204 +++---------------------------------------
2 files changed, 10 insertions(+), 201 deletions(-)
diff --git a/configure b/configure
index 49b5481139..eb8515ccca 100755
--- a/configure
+++ b/configure
@@ -3981,18 +3981,11 @@ cat > $TMPC << EOF
int main(void)
{
uint64_t x = 0, y = 0;
-#ifdef __ATOMIC_RELAXED
y = __atomic_load_n(&x, __ATOMIC_RELAXED);
__atomic_store_n(&x, y, __ATOMIC_RELAXED);
__atomic_compare_exchange_n(&x, &y, x, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
__atomic_exchange_n(&x, y, __ATOMIC_RELAXED);
__atomic_fetch_add(&x, y, __ATOMIC_RELAXED);
-#else
- typedef char is_host64[sizeof(void *) >= sizeof(uint64_t) ? 1 : -1];
- __sync_lock_test_and_set(&x, y);
- __sync_val_compare_and_swap(&x, y, 0);
- __sync_fetch_and_add(&x, y);
-#endif
return 0;
}
EOF
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 99d6030095..bf89855209 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -60,8 +60,9 @@
(unsigned short)1, \
(expr)+0))))))
-#ifdef __ATOMIC_RELAXED
-/* For C11 atomic ops */
+#ifndef __ATOMIC_RELAXED
+#error "Expecting C11 atomic ops"
+#endif
/* Manual memory barriers
*
@@ -239,193 +240,8 @@
#define qatomic_xor(ptr, n) \
((void) __atomic_fetch_xor(ptr, n, __ATOMIC_SEQ_CST))
-#else /* __ATOMIC_RELAXED */
-
-#ifdef __alpha__
-#define smp_read_barrier_depends() asm volatile("mb":::"memory")
-#endif
-
-#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__)
-
-/*
- * Because of the strongly ordered storage model, wmb() and rmb() are nops
- * here (a compiler barrier only). QEMU doesn't do accesses to write-combining
- * qemu memory or non-temporal load/stores from C code.
- */
-#define smp_mb_release() barrier()
-#define smp_mb_acquire() barrier()
-
-/*
- * __sync_lock_test_and_set() is documented to be an acquire barrier only,
- * but it is a full barrier at the hardware level. Add a compiler barrier
- * to make it a full barrier also at the compiler level.
- */
-#define qatomic_xchg(ptr, i) (barrier(), __sync_lock_test_and_set(ptr, i))
-
-#elif defined(_ARCH_PPC)
-
-/*
- * We use an eieio() for wmb() on powerpc. This assumes we don't
- * need to order cacheable and non-cacheable stores with respect to
- * each other.
- *
- * smp_mb has the same problem as on x86 for not-very-new GCC
- * (http://patchwork.ozlabs.org/patch/126184/, Nov 2011).
- */
-#define smp_wmb() ({ asm volatile("eieio" ::: "memory"); (void)0; })
-#if defined(__powerpc64__)
-#define smp_mb_release() ({ asm volatile("lwsync" ::: "memory"); (void)0; })
-#define smp_mb_acquire() ({ asm volatile("lwsync" ::: "memory"); (void)0; })
-#else
-#define smp_mb_release() ({ asm volatile("sync" ::: "memory"); (void)0; })
-#define smp_mb_acquire() ({ asm volatile("sync" ::: "memory"); (void)0; })
-#endif
-#define smp_mb() ({ asm volatile("sync" ::: "memory"); (void)0; })
-
-#endif /* _ARCH_PPC */
-
-/*
- * For (host) platforms we don't have explicit barrier definitions
- * for, we use the gcc __sync_synchronize() primitive to generate a
- * full barrier. This should be safe on all platforms, though it may
- * be overkill for smp_mb_acquire() and smp_mb_release().
- */
-#ifndef smp_mb
-#define smp_mb() __sync_synchronize()
-#endif
-
-#ifndef smp_mb_acquire
-#define smp_mb_acquire() __sync_synchronize()
-#endif
-
-#ifndef smp_mb_release
-#define smp_mb_release() __sync_synchronize()
-#endif
-
-#ifndef smp_read_barrier_depends
-#define smp_read_barrier_depends() barrier()
-#endif
-
-#ifndef signal_barrier
-#define signal_barrier() barrier()
-#endif
-
-/* These will only be atomic if the processor does the fetch or store
- * in a single issue memory operation
- */
-#define qatomic_read__nocheck(p) (*(__typeof__(*(p)) volatile*) (p))
-#define qatomic_set__nocheck(p, i) ((*(__typeof__(*(p)) volatile*) (p)) = (i))
-
-#define qatomic_read(ptr) qatomic_read__nocheck(ptr)
-#define qatomic_set(ptr, i) qatomic_set__nocheck(ptr,i)
-
-/**
- * qatomic_rcu_read - reads a RCU-protected pointer to a local variable
- * into a RCU read-side critical section. The pointer can later be safely
- * dereferenced within the critical section.
- *
- * This ensures that the pointer copy is invariant thorough the whole critical
- * section.
- *
- * Inserts memory barriers on architectures that require them (currently only
- * Alpha) and documents which pointers are protected by RCU.
- *
- * qatomic_rcu_read also includes a compiler barrier to ensure that
- * value-speculative optimizations (e.g. VSS: Value Speculation
- * Scheduling) does not perform the data read before the pointer read
- * by speculating the value of the pointer.
- *
- * Should match qatomic_rcu_set(), qatomic_xchg(), qatomic_cmpxchg().
- */
-#define qatomic_rcu_read(ptr) ({ \
- typeof(*ptr) _val = qatomic_read(ptr); \
- smp_read_barrier_depends(); \
- _val; \
-})
-
-/**
- * qatomic_rcu_set - assigns (publicizes) a pointer to a new data structure
- * meant to be read by RCU read-side critical sections.
- *
- * Documents which pointers will be dereferenced by RCU read-side critical
- * sections and adds the required memory barriers on architectures requiring
- * them. It also makes sure the compiler does not reorder code initializing the
- * data structure before its publication.
- *
- * Should match qatomic_rcu_read().
- */
-#define qatomic_rcu_set(ptr, i) do { \
- smp_wmb(); \
- qatomic_set(ptr, i); \
-} while (0)
-
-#define qatomic_load_acquire(ptr) ({ \
- typeof(*ptr) _val = qatomic_read(ptr); \
- smp_mb_acquire(); \
- _val; \
-})
-
-#define qatomic_store_release(ptr, i) do { \
- smp_mb_release(); \
- qatomic_set(ptr, i); \
-} while (0)
-
-#ifndef qatomic_xchg
-#if defined(__clang__)
-#define qatomic_xchg(ptr, i) __sync_swap(ptr, i)
-#else
-/* __sync_lock_test_and_set() is documented to be an acquire barrier only. */
-#define qatomic_xchg(ptr, i) (smp_mb(), __sync_lock_test_and_set(ptr, i))
-#endif
-#endif
-#define qatomic_xchg__nocheck qatomic_xchg
-
-/* Provide shorter names for GCC atomic builtins. */
-#define qatomic_fetch_inc(ptr) __sync_fetch_and_add(ptr, 1)
-#define qatomic_fetch_dec(ptr) __sync_fetch_and_add(ptr, -1)
-
-#define qatomic_fetch_add(ptr, n) __sync_fetch_and_add(ptr, n)
-#define qatomic_fetch_sub(ptr, n) __sync_fetch_and_sub(ptr, n)
-#define qatomic_fetch_and(ptr, n) __sync_fetch_and_and(ptr, n)
-#define qatomic_fetch_or(ptr, n) __sync_fetch_and_or(ptr, n)
-#define qatomic_fetch_xor(ptr, n) __sync_fetch_and_xor(ptr, n)
-
-#define qatomic_inc_fetch(ptr) __sync_add_and_fetch(ptr, 1)
-#define qatomic_dec_fetch(ptr) __sync_add_and_fetch(ptr, -1)
-#define qatomic_add_fetch(ptr, n) __sync_add_and_fetch(ptr, n)
-#define qatomic_sub_fetch(ptr, n) __sync_sub_and_fetch(ptr, n)
-#define qatomic_and_fetch(ptr, n) __sync_and_and_fetch(ptr, n)
-#define qatomic_or_fetch(ptr, n) __sync_or_and_fetch(ptr, n)
-#define qatomic_xor_fetch(ptr, n) __sync_xor_and_fetch(ptr, n)
-
-#define qatomic_cmpxchg(ptr, old, new) \
- __sync_val_compare_and_swap(ptr, old, new)
-#define qatomic_cmpxchg__nocheck(ptr, old, new) qatomic_cmpxchg(ptr, old, new)
-
-/* And even shorter names that return void. */
-#define qatomic_inc(ptr) ((void) __sync_fetch_and_add(ptr, 1))
-#define qatomic_dec(ptr) ((void) __sync_fetch_and_add(ptr, -1))
-#define qatomic_add(ptr, n) ((void) __sync_fetch_and_add(ptr, n))
-#define qatomic_sub(ptr, n) ((void) __sync_fetch_and_sub(ptr, n))
-#define qatomic_and(ptr, n) ((void) __sync_fetch_and_and(ptr, n))
-#define qatomic_or(ptr, n) ((void) __sync_fetch_and_or(ptr, n))
-#define qatomic_xor(ptr, n) ((void) __sync_fetch_and_xor(ptr, n))
-
-#endif /* __ATOMIC_RELAXED */
-
-#ifndef smp_wmb
#define smp_wmb() smp_mb_release()
-#endif
-#ifndef smp_rmb
#define smp_rmb() smp_mb_acquire()
-#endif
-
-/* This is more efficient than a store plus a fence. */
-#if !defined(__SANITIZE_THREAD__)
-#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__)
-#define qatomic_mb_set(ptr, i) ((void)qatomic_xchg(ptr, i))
-#endif
-#endif
/* qatomic_mb_read/set semantics map Java volatile variables. They are
* less expensive on some platforms (notably POWER) than fully
@@ -435,16 +251,16 @@
* use. See docs/devel/atomics.rst for more discussion.
*/
-#ifndef qatomic_mb_read
#define qatomic_mb_read(ptr) \
qatomic_load_acquire(ptr)
-#endif
-#ifndef qatomic_mb_set
-#define qatomic_mb_set(ptr, i) do { \
- qatomic_store_release(ptr, i); \
- smp_mb(); \
-} while(0)
+#if !defined(__SANITIZE_THREAD__) && \
+ (defined(__i386__) || defined(__x86_64__) || defined(__s390x__))
+/* This is more efficient than a store plus a fence. */
+# define qatomic_mb_set(ptr, i) ((void)qatomic_xchg(ptr, i))
+#else
+# define qatomic_mb_set(ptr, i) \
+ ({ qatomic_store_release(ptr, i); smp_mb(); })
#endif
#define qatomic_fetch_inc_nonzero(ptr) ({ \
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
2021-07-17 19:24 ` [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64 Richard Henderson
2021-07-17 19:24 ` [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:39 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only Richard Henderson
` (7 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, Philippe Mathieu-Daudé, crobinso
Use it to avoid some clang-12 -Watomic-alignment errors,
forcing some structures to be aligned and as a pointer when
we have ensured that the address is aligned.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/atomic_template.h | 4 ++--
include/qemu/atomic.h | 14 +++++++++++++-
include/qemu/stats64.h | 2 +-
softmmu/timers-state.h | 2 +-
linux-user/hppa/cpu_loop.c | 2 +-
util/qsp.c | 4 ++--
6 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index afa8a9daf3..d347462af5 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -28,8 +28,8 @@
# define SHIFT 4
#elif DATA_SIZE == 8
# define SUFFIX q
-# define DATA_TYPE uint64_t
-# define SDATA_TYPE int64_t
+# define DATA_TYPE aligned_uint64_t
+# define SDATA_TYPE aligned_int64_t
# define BSWAP bswap64
# define SHIFT 3
#elif DATA_SIZE == 4
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index bf89855209..f8f159052f 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -271,7 +271,19 @@
_oldn; \
})
-/* Abstractions to access atomically (i.e. "once") i64/u64 variables */
+/*
+ * Abstractions to access atomically (i.e. "once") i64/u64 variables.
+ *
+ * The i386 abi is odd in that by default members are only aligned to
+ * 4 bytes, which means that 8-byte types can wind up mis-aligned.
+ * Clang will then warn about this, and emit a call into libatomic.
+ *
+ * Use of these types in structures when they will be used with atomic
+ * operations can avoid this.
+ */
+typedef int64_t aligned_int64_t __attribute__((aligned(8)));
+typedef uint64_t aligned_uint64_t __attribute__((aligned(8)));
+
#ifdef CONFIG_ATOMIC64
/* Use __nocheck because sizeof(void *) might be < sizeof(u64) */
#define qatomic_read_i64 qatomic_read__nocheck
diff --git a/include/qemu/stats64.h b/include/qemu/stats64.h
index fdd3d1b8f9..802402254b 100644
--- a/include/qemu/stats64.h
+++ b/include/qemu/stats64.h
@@ -21,7 +21,7 @@
typedef struct Stat64 {
#ifdef CONFIG_ATOMIC64
- uint64_t value;
+ aligned_uint64_t value;
#else
uint32_t low, high;
uint32_t lock;
diff --git a/softmmu/timers-state.h b/softmmu/timers-state.h
index 8c262ce139..94bb7394c5 100644
--- a/softmmu/timers-state.h
+++ b/softmmu/timers-state.h
@@ -47,7 +47,7 @@ typedef struct TimersState {
int64_t last_delta;
/* Compensate for varying guest execution speed. */
- int64_t qemu_icount_bias;
+ aligned_int64_t qemu_icount_bias;
int64_t vm_clock_warp_start;
int64_t cpu_clock_offset;
diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index 3aaaf3337c..82d8183821 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -82,7 +82,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
o64 = *(uint64_t *)g2h(cs, old);
n64 = *(uint64_t *)g2h(cs, new);
#ifdef CONFIG_ATOMIC64
- r64 = qatomic_cmpxchg__nocheck((uint64_t *)g2h(cs, addr),
+ r64 = qatomic_cmpxchg__nocheck((aligned_uint64_t *)g2h(cs, addr),
o64, n64);
ret = r64 != o64;
#else
diff --git a/util/qsp.c b/util/qsp.c
index bacc5fa2f6..8562b14a87 100644
--- a/util/qsp.c
+++ b/util/qsp.c
@@ -83,8 +83,8 @@ typedef struct QSPCallSite QSPCallSite;
struct QSPEntry {
void *thread_ptr;
const QSPCallSite *callsite;
- uint64_t n_acqs;
- uint64_t ns;
+ aligned_uint64_t n_acqs;
+ aligned_uint64_t ns;
unsigned int n_objs; /* count of coalesced objs; only used for reporting */
};
typedef struct QSPEntry QSPEntry;
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (2 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:17 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 05/10] accel/tcg: Standardize atomic helpers on softmmu api Richard Henderson
` (6 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, crobinso
Always provide the atomic interface using TCGMemOpIdx oi
and uintptr_t retaddr. Rename from helper_* to cpu_* so
as to (mostly) match the exec/cpu_ldst.h functions, and
to emphasize that they are not callable from TCG directly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/tcg/tcg.h | 78 ++++++++++++++++-------------------
accel/tcg/cputlb.c | 8 ++--
accel/tcg/user-exec.c | 59 ++++++++++++++++----------
target/arm/helper-a64.c | 8 ++--
target/i386/tcg/mem_helper.c | 15 +------
target/m68k/op_helper.c | 19 +++------
target/ppc/mem_helper.c | 16 +++----
target/s390x/tcg/mem_helper.c | 19 ++++-----
8 files changed, 104 insertions(+), 118 deletions(-)
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 25dd19d6e1..44ccd86f3e 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -1341,31 +1341,32 @@ void helper_be_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
# define helper_ret_stl_mmu helper_le_stl_mmu
# define helper_ret_stq_mmu helper_le_stq_mmu
#endif
+#endif /* CONFIG_SOFTMMU */
-uint32_t helper_atomic_cmpxchgb_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cpu_atomic_cmpxchgb_mmu(CPUArchState *env, target_ulong addr,
+ uint32_t cmpv, uint32_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgw_le_mmu(CPUArchState *env, target_ulong addr,
uint32_t cmpv, uint32_t newv,
TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgw_le_mmu(CPUArchState *env, target_ulong addr,
- uint32_t cmpv, uint32_t newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgl_le_mmu(CPUArchState *env, target_ulong addr,
- uint32_t cmpv, uint32_t newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
-uint64_t helper_atomic_cmpxchgq_le_mmu(CPUArchState *env, target_ulong addr,
- uint64_t cmpv, uint64_t newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgw_be_mmu(CPUArchState *env, target_ulong addr,
- uint32_t cmpv, uint32_t newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgl_be_mmu(CPUArchState *env, target_ulong addr,
- uint32_t cmpv, uint32_t newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
-uint64_t helper_atomic_cmpxchgq_be_mmu(CPUArchState *env, target_ulong addr,
- uint64_t cmpv, uint64_t newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgl_le_mmu(CPUArchState *env, target_ulong addr,
+ uint32_t cmpv, uint32_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+uint64_t cpu_atomic_cmpxchgq_le_mmu(CPUArchState *env, target_ulong addr,
+ uint64_t cmpv, uint64_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgw_be_mmu(CPUArchState *env, target_ulong addr,
+ uint32_t cmpv, uint32_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgl_be_mmu(CPUArchState *env, target_ulong addr,
+ uint32_t cmpv, uint32_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+uint64_t cpu_atomic_cmpxchgq_be_mmu(CPUArchState *env, target_ulong addr,
+ uint64_t cmpv, uint64_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
#define GEN_ATOMIC_HELPER(NAME, TYPE, SUFFIX) \
-TYPE helper_atomic_ ## NAME ## SUFFIX ## _mmu \
+TYPE cpu_atomic_ ## NAME ## SUFFIX ## _mmu \
(CPUArchState *env, target_ulong addr, TYPE val, \
TCGMemOpIdx oi, uintptr_t retaddr);
@@ -1411,31 +1412,22 @@ GEN_ATOMIC_HELPER_ALL(xchg)
#undef GEN_ATOMIC_HELPER_ALL
#undef GEN_ATOMIC_HELPER
-#endif /* CONFIG_SOFTMMU */
-/*
- * These aren't really a "proper" helpers because TCG cannot manage Int128.
- * However, use the same format as the others, for use by the backends.
- *
- * The cmpxchg functions are only defined if HAVE_CMPXCHG128;
- * the ld/st functions are only defined if HAVE_ATOMIC128,
- * as defined by <qemu/atomic128.h>.
- */
-Int128 helper_atomic_cmpxchgo_le_mmu(CPUArchState *env, target_ulong addr,
- Int128 cmpv, Int128 newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
-Int128 helper_atomic_cmpxchgo_be_mmu(CPUArchState *env, target_ulong addr,
- Int128 cmpv, Int128 newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
+Int128 cpu_atomic_cmpxchgo_le_mmu(CPUArchState *env, target_ulong addr,
+ Int128 cmpv, Int128 newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+Int128 cpu_atomic_cmpxchgo_be_mmu(CPUArchState *env, target_ulong addr,
+ Int128 cmpv, Int128 newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
-Int128 helper_atomic_ldo_le_mmu(CPUArchState *env, target_ulong addr,
- TCGMemOpIdx oi, uintptr_t retaddr);
-Int128 helper_atomic_ldo_be_mmu(CPUArchState *env, target_ulong addr,
- TCGMemOpIdx oi, uintptr_t retaddr);
-void helper_atomic_sto_le_mmu(CPUArchState *env, target_ulong addr, Int128 val,
- TCGMemOpIdx oi, uintptr_t retaddr);
-void helper_atomic_sto_be_mmu(CPUArchState *env, target_ulong addr, Int128 val,
- TCGMemOpIdx oi, uintptr_t retaddr);
+Int128 cpu_atomic_ldo_le_mmu(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+Int128 cpu_atomic_ldo_be_mmu(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+void cpu_atomic_sto_le_mmu(CPUArchState *env, target_ulong addr, Int128 val,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+void cpu_atomic_sto_be_mmu(CPUArchState *env, target_ulong addr, Int128 val,
+ TCGMemOpIdx oi, uintptr_t retaddr);
#ifdef CONFIG_DEBUG_TCG
void tcg_assert_listed_vecop(TCGOpcode);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index b4e15b6aad..63da1cc96f 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2686,12 +2686,14 @@ void cpu_stq_le_data(CPUArchState *env, target_ulong ptr, uint64_t val)
cpu_stq_le_data_ra(env, ptr, val, 0);
}
-/* First set of helpers allows passing in of OI and RETADDR. This makes
- them callable from other helpers. */
+/*
+ * First set of functions passes in OI and RETADDR.
+ * This makes them callable from other helpers.
+ */
#define EXTRA_ARGS , TCGMemOpIdx oi, uintptr_t retaddr
#define ATOMIC_NAME(X) \
- HELPER(glue(glue(glue(atomic_ ## X, SUFFIX), END), _mmu))
+ glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
#define ATOMIC_MMU_DECLS
#define ATOMIC_MMU_LOOKUP_RW \
atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ | PAGE_WRITE, retaddr)
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index ba09fd0413..82dbe06f08 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -1234,19 +1234,23 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
return ret;
}
-/* Macro to call the above, with local variables from the use context. */
-#define ATOMIC_MMU_DECLS do {} while (0)
-#define ATOMIC_MMU_LOOKUP_RW atomic_mmu_lookup(env, addr, DATA_SIZE, GETPC())
+#include "atomic_common.c.inc"
+
+/*
+ * First set of functions passes in OI and RETADDR.
+ * This makes them callable from other helpers.
+ */
+
+#define EXTRA_ARGS , TCGMemOpIdx oi, uintptr_t retaddr
+#define ATOMIC_NAME(X) \
+ glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
+#define ATOMIC_MMU_DECLS
+#define ATOMIC_MMU_LOOKUP_RW atomic_mmu_lookup(env, addr, DATA_SIZE, retaddr)
#define ATOMIC_MMU_LOOKUP_R ATOMIC_MMU_LOOKUP_RW
#define ATOMIC_MMU_LOOKUP_W ATOMIC_MMU_LOOKUP_RW
#define ATOMIC_MMU_CLEANUP do { clear_helper_retaddr(); } while (0)
#define ATOMIC_MMU_IDX MMU_USER_IDX
-#define ATOMIC_NAME(X) HELPER(glue(glue(atomic_ ## X, SUFFIX), END))
-#define EXTRA_ARGS
-
-#include "atomic_common.c.inc"
-
#define DATA_SIZE 1
#include "atomic_template.h"
@@ -1261,20 +1265,33 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
#include "atomic_template.h"
#endif
-/* The following is only callable from other helpers, and matches up
- with the softmmu version. */
-
#if HAVE_ATOMIC128 || HAVE_CMPXCHG128
-
-#undef EXTRA_ARGS
-#undef ATOMIC_NAME
-#undef ATOMIC_MMU_LOOKUP_RW
-
-#define EXTRA_ARGS , TCGMemOpIdx oi, uintptr_t retaddr
-#define ATOMIC_NAME(X) \
- HELPER(glue(glue(glue(atomic_ ## X, SUFFIX), END), _mmu))
-#define ATOMIC_MMU_LOOKUP_RW atomic_mmu_lookup(env, addr, DATA_SIZE, retaddr)
-
#define DATA_SIZE 16
#include "atomic_template.h"
#endif
+
+/*
+ * Second set of functions is directly callable from TCG.
+ */
+
+#undef EXTRA_ARGS
+#undef ATOMIC_NAME
+#undef ATOMIC_MMU_DECLS
+
+#define ATOMIC_NAME(X) HELPER(glue(glue(atomic_ ## X, SUFFIX), END))
+#define EXTRA_ARGS
+#define ATOMIC_MMU_DECLS uintptr_t retaddr = GETPC()
+
+#define DATA_SIZE 1
+#include "atomic_template.h"
+
+#define DATA_SIZE 2
+#include "atomic_template.h"
+
+#define DATA_SIZE 4
+#include "atomic_template.h"
+
+#ifdef CONFIG_ATOMIC64
+#define DATA_SIZE 8
+#include "atomic_template.h"
+#endif
diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index ac5c4452d5..26f79f9141 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -564,7 +564,7 @@ uint64_t HELPER(paired_cmpxchg64_le_parallel)(CPUARMState *env, uint64_t addr,
cmpv = int128_make128(env->exclusive_val, env->exclusive_high);
newv = int128_make128(new_lo, new_hi);
- oldv = helper_atomic_cmpxchgo_le_mmu(env, addr, cmpv, newv, oi, ra);
+ oldv = cpu_atomic_cmpxchgo_le_mmu(env, addr, cmpv, newv, oi, ra);
success = int128_eq(oldv, cmpv);
return !success;
@@ -638,7 +638,7 @@ uint64_t HELPER(paired_cmpxchg64_be_parallel)(CPUARMState *env, uint64_t addr,
*/
cmpv = int128_make128(env->exclusive_high, env->exclusive_val);
newv = int128_make128(new_hi, new_lo);
- oldv = helper_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
+ oldv = cpu_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
success = int128_eq(oldv, cmpv);
return !success;
@@ -660,7 +660,7 @@ void HELPER(casp_le_parallel)(CPUARMState *env, uint32_t rs, uint64_t addr,
cmpv = int128_make128(env->xregs[rs], env->xregs[rs + 1]);
newv = int128_make128(new_lo, new_hi);
- oldv = helper_atomic_cmpxchgo_le_mmu(env, addr, cmpv, newv, oi, ra);
+ oldv = cpu_atomic_cmpxchgo_le_mmu(env, addr, cmpv, newv, oi, ra);
env->xregs[rs] = int128_getlo(oldv);
env->xregs[rs + 1] = int128_gethi(oldv);
@@ -681,7 +681,7 @@ void HELPER(casp_be_parallel)(CPUARMState *env, uint32_t rs, uint64_t addr,
cmpv = int128_make128(env->xregs[rs + 1], env->xregs[rs]);
newv = int128_make128(new_lo, new_hi);
- oldv = helper_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
+ oldv = cpu_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
env->xregs[rs + 1] = int128_getlo(oldv);
env->xregs[rs] = int128_gethi(oldv);
diff --git a/target/i386/tcg/mem_helper.c b/target/i386/tcg/mem_helper.c
index 591f512bff..2da3cd14b6 100644
--- a/target/i386/tcg/mem_helper.c
+++ b/target/i386/tcg/mem_helper.c
@@ -64,22 +64,12 @@ void helper_cmpxchg8b(CPUX86State *env, target_ulong a0)
cmpv = deposit64(env->regs[R_EAX], 32, 32, env->regs[R_EDX]);
newv = deposit64(env->regs[R_EBX], 32, 32, env->regs[R_ECX]);
-#ifdef CONFIG_USER_ONLY
- {
- uint64_t *haddr = g2h(env_cpu(env), a0);
- cmpv = cpu_to_le64(cmpv);
- newv = cpu_to_le64(newv);
- oldv = qatomic_cmpxchg__nocheck(haddr, cmpv, newv);
- oldv = le64_to_cpu(oldv);
- }
-#else
{
uintptr_t ra = GETPC();
int mem_idx = cpu_mmu_index(env, false);
TCGMemOpIdx oi = make_memop_idx(MO_TEQ, mem_idx);
- oldv = helper_atomic_cmpxchgq_le_mmu(env, a0, cmpv, newv, oi, ra);
+ oldv = cpu_atomic_cmpxchgq_le_mmu(env, a0, cmpv, newv, oi, ra);
}
-#endif
if (oldv == cmpv) {
eflags |= CC_Z;
@@ -147,8 +137,7 @@ void helper_cmpxchg16b(CPUX86State *env, target_ulong a0)
int mem_idx = cpu_mmu_index(env, false);
TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
- Int128 oldv = helper_atomic_cmpxchgo_le_mmu(env, a0, cmpv,
- newv, oi, ra);
+ Int128 oldv = cpu_atomic_cmpxchgo_le_mmu(env, a0, cmpv, newv, oi, ra);
if (int128_eq(oldv, cmpv)) {
eflags |= CC_Z;
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index ae1ba4b437..d006d1cb3e 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -22,6 +22,7 @@
#include "exec/exec-all.h"
#include "exec/cpu_ldst.h"
#include "semihosting/semihost.h"
+#include "tcg/tcg.h"
#if defined(CONFIG_USER_ONLY)
@@ -782,9 +783,9 @@ static void do_cas2l(CPUM68KState *env, uint32_t regs, uint32_t a1, uint32_t a2,
uint32_t u2 = env->dregs[Du2];
uint32_t l1, l2;
uintptr_t ra = GETPC();
-#if defined(CONFIG_ATOMIC64) && !defined(CONFIG_USER_ONLY)
+#if defined(CONFIG_ATOMIC64)
int mmu_idx = cpu_mmu_index(env, 0);
- TCGMemOpIdx oi;
+ TCGMemOpIdx oi = make_memop_idx(MO_BEQ, mmu_idx);
#endif
if (parallel) {
@@ -794,23 +795,13 @@ static void do_cas2l(CPUM68KState *env, uint32_t regs, uint32_t a1, uint32_t a2,
if ((a1 & 7) == 0 && a2 == a1 + 4) {
c = deposit64(c2, 32, 32, c1);
u = deposit64(u2, 32, 32, u1);
-#ifdef CONFIG_USER_ONLY
- l = helper_atomic_cmpxchgq_be(env, a1, c, u);
-#else
- oi = make_memop_idx(MO_BEQ, mmu_idx);
- l = helper_atomic_cmpxchgq_be_mmu(env, a1, c, u, oi, ra);
-#endif
+ l = cpu_atomic_cmpxchgq_be_mmu(env, a1, c, u, oi, ra);
l1 = l >> 32;
l2 = l;
} else if ((a2 & 7) == 0 && a1 == a2 + 4) {
c = deposit64(c1, 32, 32, c2);
u = deposit64(u1, 32, 32, u2);
-#ifdef CONFIG_USER_ONLY
- l = helper_atomic_cmpxchgq_be(env, a2, c, u);
-#else
- oi = make_memop_idx(MO_BEQ, mmu_idx);
- l = helper_atomic_cmpxchgq_be_mmu(env, a2, c, u, oi, ra);
-#endif
+ l = cpu_atomic_cmpxchgq_be_mmu(env, a2, c, u, oi, ra);
l2 = l >> 32;
l1 = l;
} else
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index 444b2a30ef..e2282baa8d 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -376,7 +376,7 @@ uint64_t helper_lq_le_parallel(CPUPPCState *env, target_ulong addr,
/* We will have raised EXCP_ATOMIC from the translator. */
assert(HAVE_ATOMIC128);
- ret = helper_atomic_ldo_le_mmu(env, addr, opidx, GETPC());
+ ret = cpu_atomic_ldo_le_mmu(env, addr, opidx, GETPC());
env->retxh = int128_gethi(ret);
return int128_getlo(ret);
}
@@ -388,7 +388,7 @@ uint64_t helper_lq_be_parallel(CPUPPCState *env, target_ulong addr,
/* We will have raised EXCP_ATOMIC from the translator. */
assert(HAVE_ATOMIC128);
- ret = helper_atomic_ldo_be_mmu(env, addr, opidx, GETPC());
+ ret = cpu_atomic_ldo_be_mmu(env, addr, opidx, GETPC());
env->retxh = int128_gethi(ret);
return int128_getlo(ret);
}
@@ -401,7 +401,7 @@ void helper_stq_le_parallel(CPUPPCState *env, target_ulong addr,
/* We will have raised EXCP_ATOMIC from the translator. */
assert(HAVE_ATOMIC128);
val = int128_make128(lo, hi);
- helper_atomic_sto_le_mmu(env, addr, val, opidx, GETPC());
+ cpu_atomic_sto_le_mmu(env, addr, val, opidx, GETPC());
}
void helper_stq_be_parallel(CPUPPCState *env, target_ulong addr,
@@ -412,7 +412,7 @@ void helper_stq_be_parallel(CPUPPCState *env, target_ulong addr,
/* We will have raised EXCP_ATOMIC from the translator. */
assert(HAVE_ATOMIC128);
val = int128_make128(lo, hi);
- helper_atomic_sto_be_mmu(env, addr, val, opidx, GETPC());
+ cpu_atomic_sto_be_mmu(env, addr, val, opidx, GETPC());
}
uint32_t helper_stqcx_le_parallel(CPUPPCState *env, target_ulong addr,
@@ -429,8 +429,8 @@ uint32_t helper_stqcx_le_parallel(CPUPPCState *env, target_ulong addr,
cmpv = int128_make128(env->reserve_val2, env->reserve_val);
newv = int128_make128(new_lo, new_hi);
- oldv = helper_atomic_cmpxchgo_le_mmu(env, addr, cmpv, newv,
- opidx, GETPC());
+ oldv = cpu_atomic_cmpxchgo_le_mmu(env, addr, cmpv, newv,
+ opidx, GETPC());
success = int128_eq(oldv, cmpv);
}
env->reserve_addr = -1;
@@ -451,8 +451,8 @@ uint32_t helper_stqcx_be_parallel(CPUPPCState *env, target_ulong addr,
cmpv = int128_make128(env->reserve_val2, env->reserve_val);
newv = int128_make128(new_lo, new_hi);
- oldv = helper_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv,
- opidx, GETPC());
+ oldv = cpu_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv,
+ opidx, GETPC());
success = int128_eq(oldv, cmpv);
}
env->reserve_addr = -1;
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 9bae13ecf0..21a4de4067 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -1811,7 +1811,7 @@ void HELPER(cdsg_parallel)(CPUS390XState *env, uint64_t addr,
mem_idx = cpu_mmu_index(env, false);
oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
- oldv = helper_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
+ oldv = cpu_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
fail = !int128_eq(oldv, cmpv);
env->cc_op = fail;
@@ -1884,7 +1884,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
ov = qatomic_cmpxchg__nocheck(haddr, cv, nv);
#else
TCGMemOpIdx oi = make_memop_idx(MO_TEUL | MO_ALIGN, mem_idx);
- ov = helper_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
+ ov = cpu_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
#endif
} else {
ov = cpu_ldl_data_ra(env, a1, ra);
@@ -1903,13 +1903,8 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
if (parallel) {
#ifdef CONFIG_ATOMIC64
-# ifdef CONFIG_USER_ONLY
- uint64_t *haddr = g2h(env_cpu(env), a1);
- ov = qatomic_cmpxchg__nocheck(haddr, cv, nv);
-# else
TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN, mem_idx);
- ov = helper_atomic_cmpxchgq_be_mmu(env, a1, cv, nv, oi, ra);
-# endif
+ ov = cpu_atomic_cmpxchgq_be_mmu(env, a1, cv, nv, oi, ra);
#else
/* Note that we asserted !parallel above. */
g_assert_not_reached();
@@ -1945,7 +1940,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
cpu_stq_data_ra(env, a1 + 8, int128_getlo(nv), ra);
} else if (HAVE_CMPXCHG128) {
TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
- ov = helper_atomic_cmpxchgo_be_mmu(env, a1, cv, nv, oi, ra);
+ ov = cpu_atomic_cmpxchgo_be_mmu(env, a1, cv, nv, oi, ra);
cc = !int128_eq(ov, cv);
} else {
/* Note that we asserted !parallel above. */
@@ -1985,7 +1980,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
} else if (HAVE_ATOMIC128) {
TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
Int128 sv = int128_make128(svl, svh);
- helper_atomic_sto_be_mmu(env, a2, sv, oi, ra);
+ cpu_atomic_sto_be_mmu(env, a2, sv, oi, ra);
} else {
/* Note that we asserted !parallel above. */
g_assert_not_reached();
@@ -2486,7 +2481,7 @@ uint64_t HELPER(lpq_parallel)(CPUS390XState *env, uint64_t addr)
mem_idx = cpu_mmu_index(env, false);
oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
- v = helper_atomic_ldo_be_mmu(env, addr, oi, ra);
+ v = cpu_atomic_ldo_be_mmu(env, addr, oi, ra);
hi = int128_gethi(v);
lo = int128_getlo(v);
@@ -2518,7 +2513,7 @@ void HELPER(stpq_parallel)(CPUS390XState *env, uint64_t addr,
mem_idx = cpu_mmu_index(env, false);
oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
v = int128_make128(low, high);
- helper_atomic_sto_be_mmu(env, addr, v, oi, ra);
+ cpu_atomic_sto_be_mmu(env, addr, v, oi, ra);
}
/* Execute instruction. This instruction executes an insn modified with
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 05/10] accel/tcg: Standardize atomic helpers on softmmu api
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (3 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:19 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h Richard Henderson
` (5 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, crobinso
Reduce the amount of code duplication by always passing
the TCGMemOpIdx argument to helper_atomic_*. This is not
currently used for user-only, but it's easy to ignore.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/tcg-runtime.h | 46 -----------------------
accel/tcg/cputlb.c | 32 ----------------
accel/tcg/user-exec.c | 26 -------------
tcg/tcg-op.c | 47 ++++-------------------
accel/tcg/atomic_common.c.inc | 70 +++++++++++++++++++++++++++++++++++
5 files changed, 78 insertions(+), 143 deletions(-)
diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
index 91a5b7e85f..37cbd722bf 100644
--- a/accel/tcg/tcg-runtime.h
+++ b/accel/tcg/tcg-runtime.h
@@ -39,8 +39,6 @@ DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, env)
DEF_HELPER_FLAGS_3(memset, TCG_CALL_NO_RWG, ptr, ptr, int, ptr)
#endif /* IN_HELPER_PROTO */
-#ifdef CONFIG_SOFTMMU
-
DEF_HELPER_FLAGS_5(atomic_cmpxchgb, TCG_CALL_NO_WG,
i32, env, tl, i32, i32, i32)
DEF_HELPER_FLAGS_5(atomic_cmpxchgw_be, TCG_CALL_NO_WG,
@@ -88,50 +86,6 @@ DEF_HELPER_FLAGS_5(atomic_cmpxchgq_le, TCG_CALL_NO_WG,
TCG_CALL_NO_WG, i32, env, tl, i32, i32)
#endif /* CONFIG_ATOMIC64 */
-#else
-
-DEF_HELPER_FLAGS_4(atomic_cmpxchgb, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgw_be, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgw_le, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgl_be, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgl_le, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-#ifdef CONFIG_ATOMIC64
-DEF_HELPER_FLAGS_4(atomic_cmpxchgq_be, TCG_CALL_NO_WG, i64, env, tl, i64, i64)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgq_le, TCG_CALL_NO_WG, i64, env, tl, i64, i64)
-#endif
-
-#ifdef CONFIG_ATOMIC64
-#define GEN_ATOMIC_HELPERS(NAME) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), b), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_le), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_be), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_le), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_be), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), q_le), \
- TCG_CALL_NO_WG, i64, env, tl, i64) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), q_be), \
- TCG_CALL_NO_WG, i64, env, tl, i64)
-#else
-#define GEN_ATOMIC_HELPERS(NAME) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), b), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_le), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_be), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_le), \
- TCG_CALL_NO_WG, i32, env, tl, i32) \
- DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_be), \
- TCG_CALL_NO_WG, i32, env, tl, i32)
-#endif /* CONFIG_ATOMIC64 */
-
-#endif /* CONFIG_SOFTMMU */
-
GEN_ATOMIC_HELPERS(fetch_add)
GEN_ATOMIC_HELPERS(fetch_and)
GEN_ATOMIC_HELPERS(fetch_or)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 63da1cc96f..842cf4b572 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2725,38 +2725,6 @@ void cpu_stq_le_data(CPUArchState *env, target_ulong ptr, uint64_t val)
#include "atomic_template.h"
#endif
-/* Second set of helpers are directly callable from TCG as helpers. */
-
-#undef EXTRA_ARGS
-#undef ATOMIC_NAME
-#undef ATOMIC_MMU_LOOKUP_RW
-#undef ATOMIC_MMU_LOOKUP_R
-#undef ATOMIC_MMU_LOOKUP_W
-
-#define EXTRA_ARGS , TCGMemOpIdx oi
-#define ATOMIC_NAME(X) HELPER(glue(glue(atomic_ ## X, SUFFIX), END))
-#define ATOMIC_MMU_LOOKUP_RW \
- atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ | PAGE_WRITE, GETPC())
-#define ATOMIC_MMU_LOOKUP_R \
- atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ, GETPC())
-#define ATOMIC_MMU_LOOKUP_W \
- atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_WRITE, GETPC())
-
-#define DATA_SIZE 1
-#include "atomic_template.h"
-
-#define DATA_SIZE 2
-#include "atomic_template.h"
-
-#define DATA_SIZE 4
-#include "atomic_template.h"
-
-#ifdef CONFIG_ATOMIC64
-#define DATA_SIZE 8
-#include "atomic_template.h"
-#endif
-#undef ATOMIC_MMU_IDX
-
/* Code access functions. */
static uint64_t full_ldub_code(CPUArchState *env, target_ulong addr,
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 82dbe06f08..7e92d6b875 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -1269,29 +1269,3 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
#define DATA_SIZE 16
#include "atomic_template.h"
#endif
-
-/*
- * Second set of functions is directly callable from TCG.
- */
-
-#undef EXTRA_ARGS
-#undef ATOMIC_NAME
-#undef ATOMIC_MMU_DECLS
-
-#define ATOMIC_NAME(X) HELPER(glue(glue(atomic_ ## X, SUFFIX), END))
-#define EXTRA_ARGS
-#define ATOMIC_MMU_DECLS uintptr_t retaddr = GETPC()
-
-#define DATA_SIZE 1
-#include "atomic_template.h"
-
-#define DATA_SIZE 2
-#include "atomic_template.h"
-
-#define DATA_SIZE 4
-#include "atomic_template.h"
-
-#ifdef CONFIG_ATOMIC64
-#define DATA_SIZE 8
-#include "atomic_template.h"
-#endif
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 0c561fb253..66829be6ba 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -3084,7 +3084,6 @@ static void tcg_gen_ext_i64(TCGv_i64 ret, TCGv_i64 val, MemOp opc)
}
}
-#ifdef CONFIG_SOFTMMU
typedef void (*gen_atomic_cx_i32)(TCGv_i32, TCGv_env, TCGv,
TCGv_i32, TCGv_i32, TCGv_i32);
typedef void (*gen_atomic_cx_i64)(TCGv_i64, TCGv_env, TCGv,
@@ -3093,12 +3092,6 @@ typedef void (*gen_atomic_op_i32)(TCGv_i32, TCGv_env, TCGv,
TCGv_i32, TCGv_i32);
typedef void (*gen_atomic_op_i64)(TCGv_i64, TCGv_env, TCGv,
TCGv_i64, TCGv_i32);
-#else
-typedef void (*gen_atomic_cx_i32)(TCGv_i32, TCGv_env, TCGv, TCGv_i32, TCGv_i32);
-typedef void (*gen_atomic_cx_i64)(TCGv_i64, TCGv_env, TCGv, TCGv_i64, TCGv_i64);
-typedef void (*gen_atomic_op_i32)(TCGv_i32, TCGv_env, TCGv, TCGv_i32);
-typedef void (*gen_atomic_op_i64)(TCGv_i64, TCGv_env, TCGv, TCGv_i64);
-#endif
#ifdef CONFIG_ATOMIC64
# define WITH_ATOMIC64(X) X,
@@ -3144,14 +3137,8 @@ void tcg_gen_atomic_cmpxchg_i32(TCGv_i32 retv, TCGv addr, TCGv_i32 cmpv,
gen = table_cmpxchg[memop & (MO_SIZE | MO_BSWAP)];
tcg_debug_assert(gen != NULL);
-#ifdef CONFIG_SOFTMMU
- {
- TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
- gen(retv, cpu_env, addr, cmpv, newv, tcg_constant_i32(oi));
- }
-#else
- gen(retv, cpu_env, addr, cmpv, newv);
-#endif
+ TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
+ gen(retv, cpu_env, addr, cmpv, newv, tcg_constant_i32(oi));
if (memop & MO_SIGN) {
tcg_gen_ext_i32(retv, retv, memop);
@@ -3188,14 +3175,8 @@ void tcg_gen_atomic_cmpxchg_i64(TCGv_i64 retv, TCGv addr, TCGv_i64 cmpv,
gen = table_cmpxchg[memop & (MO_SIZE | MO_BSWAP)];
tcg_debug_assert(gen != NULL);
-#ifdef CONFIG_SOFTMMU
- {
- TCGMemOpIdx oi = make_memop_idx(memop, idx);
- gen(retv, cpu_env, addr, cmpv, newv, tcg_constant_i32(oi));
- }
-#else
- gen(retv, cpu_env, addr, cmpv, newv);
-#endif
+ TCGMemOpIdx oi = make_memop_idx(memop, idx);
+ gen(retv, cpu_env, addr, cmpv, newv, tcg_constant_i32(oi));
#else
gen_helper_exit_atomic(cpu_env);
/* Produce a result, so that we have a well-formed opcode stream
@@ -3251,14 +3232,8 @@ static void do_atomic_op_i32(TCGv_i32 ret, TCGv addr, TCGv_i32 val,
gen = table[memop & (MO_SIZE | MO_BSWAP)];
tcg_debug_assert(gen != NULL);
-#ifdef CONFIG_SOFTMMU
- {
- TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
- gen(ret, cpu_env, addr, val, tcg_constant_i32(oi));
- }
-#else
- gen(ret, cpu_env, addr, val);
-#endif
+ TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
+ gen(ret, cpu_env, addr, val, tcg_constant_i32(oi));
if (memop & MO_SIGN) {
tcg_gen_ext_i32(ret, ret, memop);
@@ -3296,14 +3271,8 @@ static void do_atomic_op_i64(TCGv_i64 ret, TCGv addr, TCGv_i64 val,
gen = table[memop & (MO_SIZE | MO_BSWAP)];
tcg_debug_assert(gen != NULL);
-#ifdef CONFIG_SOFTMMU
- {
- TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
- gen(ret, cpu_env, addr, val, tcg_constant_i32(oi));
- }
-#else
- gen(ret, cpu_env, addr, val);
-#endif
+ TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
+ gen(ret, cpu_env, addr, val, tcg_constant_i32(oi));
#else
gen_helper_exit_atomic(cpu_env);
/* Produce a result, so that we have a well-formed opcode stream
diff --git a/accel/tcg/atomic_common.c.inc b/accel/tcg/atomic_common.c.inc
index 344525b0bb..a668cf0d6f 100644
--- a/accel/tcg/atomic_common.c.inc
+++ b/accel/tcg/atomic_common.c.inc
@@ -52,3 +52,73 @@ void atomic_trace_st_post(CPUArchState *env, target_ulong addr, uint16_t info)
{
qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, info);
}
+
+/*
+ * Atomic helpers callable from TCG.
+ * These have a common interface and all defer to cpu_atomic_*
+ * using the host return address from GETPC().
+ */
+
+#define CMPXCHG_HELPER(OP, TYPE) \
+ TYPE HELPER(atomic_##OP)(CPUArchState *env, target_ulong addr, \
+ TYPE oldv, TYPE newv, uint32_t oi) \
+ { return cpu_atomic_##OP##_mmu(env, addr, oldv, newv, oi, GETPC()); }
+
+CMPXCHG_HELPER(cmpxchgb, uint32_t)
+CMPXCHG_HELPER(cmpxchgw_be, uint32_t)
+CMPXCHG_HELPER(cmpxchgw_le, uint32_t)
+CMPXCHG_HELPER(cmpxchgl_be, uint32_t)
+CMPXCHG_HELPER(cmpxchgl_le, uint32_t)
+
+#ifdef CONFIG_ATOMIC64
+CMPXCHG_HELPER(cmpxchgq_be, uint64_t)
+CMPXCHG_HELPER(cmpxchgq_le, uint64_t)
+#endif
+
+#undef CMPXCHG_HELPER
+
+#define ATOMIC_HELPER(OP, TYPE) \
+ TYPE HELPER(glue(atomic_,OP))(CPUArchState *env, target_ulong addr, \
+ TYPE val, uint32_t oi) \
+ { return glue(glue(cpu_atomic_,OP),_mmu)(env, addr, val, oi, GETPC()); }
+
+#ifdef CONFIG_ATOMIC64
+#define GEN_ATOMIC_HELPERS(OP) \
+ ATOMIC_HELPER(glue(OP,b), uint32_t) \
+ ATOMIC_HELPER(glue(OP,w_be), uint32_t) \
+ ATOMIC_HELPER(glue(OP,w_le), uint32_t) \
+ ATOMIC_HELPER(glue(OP,l_be), uint32_t) \
+ ATOMIC_HELPER(glue(OP,l_le), uint32_t) \
+ ATOMIC_HELPER(glue(OP,q_be), uint64_t) \
+ ATOMIC_HELPER(glue(OP,q_le), uint64_t)
+#else
+#define GEN_ATOMIC_HELPERS(OP) \
+ ATOMIC_HELPER(glue(OP,b), uint32_t) \
+ ATOMIC_HELPER(glue(OP,w_be), uint32_t) \
+ ATOMIC_HELPER(glue(OP,w_le), uint32_t) \
+ ATOMIC_HELPER(glue(OP,l_be), uint32_t) \
+ ATOMIC_HELPER(glue(OP,l_le), uint32_t)
+#endif
+
+GEN_ATOMIC_HELPERS(fetch_add)
+GEN_ATOMIC_HELPERS(fetch_and)
+GEN_ATOMIC_HELPERS(fetch_or)
+GEN_ATOMIC_HELPERS(fetch_xor)
+GEN_ATOMIC_HELPERS(fetch_smin)
+GEN_ATOMIC_HELPERS(fetch_umin)
+GEN_ATOMIC_HELPERS(fetch_smax)
+GEN_ATOMIC_HELPERS(fetch_umax)
+
+GEN_ATOMIC_HELPERS(add_fetch)
+GEN_ATOMIC_HELPERS(and_fetch)
+GEN_ATOMIC_HELPERS(or_fetch)
+GEN_ATOMIC_HELPERS(xor_fetch)
+GEN_ATOMIC_HELPERS(smin_fetch)
+GEN_ATOMIC_HELPERS(umin_fetch)
+GEN_ATOMIC_HELPERS(smax_fetch)
+GEN_ATOMIC_HELPERS(umax_fetch)
+
+GEN_ATOMIC_HELPERS(xchg)
+
+#undef ATOMIC_HELPER
+#undef GEN_ATOMIC_HELPERS
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (4 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 05/10] accel/tcg: Standardize atomic helpers on softmmu api Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:19 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS Richard Henderson
` (4 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, Philippe Mathieu-Daudé, crobinso
All instances of EXTRA_ARGS are now identical.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/atomic_template.h | 36 ++++++++++++++++++++----------------
accel/tcg/cputlb.c | 1 -
accel/tcg/user-exec.c | 1 -
3 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index d347462af5..52fb26a274 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -71,7 +71,8 @@
#endif
ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
- ABI_TYPE cmpv, ABI_TYPE newv EXTRA_ARGS)
+ ABI_TYPE cmpv, ABI_TYPE newv,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -92,7 +93,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
#if DATA_SIZE >= 16
#if HAVE_ATOMIC128
-ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
@@ -106,8 +108,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
return val;
}
-void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
@@ -121,8 +123,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
}
#endif
#else
-ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -139,7 +141,7 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
#define GEN_ATOMIC_HELPER(X) \
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
- ABI_TYPE val EXTRA_ARGS) \
+ ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
ATOMIC_MMU_DECLS; \
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
@@ -173,7 +175,7 @@ GEN_ATOMIC_HELPER(xor_fetch)
*/
#define GEN_ATOMIC_HELPER_FN(X, FN, XDATA_TYPE, RET) \
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
- ABI_TYPE xval EXTRA_ARGS) \
+ ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
ATOMIC_MMU_DECLS; \
XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
@@ -218,7 +220,8 @@ GEN_ATOMIC_HELPER_FN(umax_fetch, MAX, DATA_TYPE, new)
#endif
ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
- ABI_TYPE cmpv, ABI_TYPE newv EXTRA_ARGS)
+ ABI_TYPE cmpv, ABI_TYPE newv,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -239,7 +242,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
#if DATA_SIZE >= 16
#if HAVE_ATOMIC128
-ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
@@ -253,8 +257,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
return BSWAP(val);
}
-void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
@@ -270,8 +274,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
}
#endif
#else
-ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -288,7 +292,7 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
#define GEN_ATOMIC_HELPER(X) \
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
- ABI_TYPE val EXTRA_ARGS) \
+ ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
ATOMIC_MMU_DECLS; \
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
@@ -320,7 +324,7 @@ GEN_ATOMIC_HELPER(xor_fetch)
*/
#define GEN_ATOMIC_HELPER_FN(X, FN, XDATA_TYPE, RET) \
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
- ABI_TYPE xval EXTRA_ARGS) \
+ ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
ATOMIC_MMU_DECLS; \
XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 842cf4b572..cc0e673222 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2691,7 +2691,6 @@ void cpu_stq_le_data(CPUArchState *env, target_ulong ptr, uint64_t val)
* This makes them callable from other helpers.
*/
-#define EXTRA_ARGS , TCGMemOpIdx oi, uintptr_t retaddr
#define ATOMIC_NAME(X) \
glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
#define ATOMIC_MMU_DECLS
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 7e92d6b875..f6f8ddeb60 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -1241,7 +1241,6 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
* This makes them callable from other helpers.
*/
-#define EXTRA_ARGS , TCGMemOpIdx oi, uintptr_t retaddr
#define ATOMIC_NAME(X) \
glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
#define ATOMIC_MMU_DECLS
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (5 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:20 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_* Richard Henderson
` (3 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, Philippe Mathieu-Daudé, crobinso
All definitions are now empty.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/atomic_template.h | 12 ------------
accel/tcg/cputlb.c | 1 -
accel/tcg/user-exec.c | 1 -
3 files changed, 14 deletions(-)
diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 52fb26a274..ae6b6a03be 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -74,7 +74,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE cmpv, ABI_TYPE newv,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
DATA_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
@@ -96,7 +95,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
ATOMIC_MMU_IDX);
@@ -111,7 +109,6 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
ATOMIC_MMU_IDX);
@@ -126,7 +123,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
DATA_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
@@ -143,7 +139,6 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- ATOMIC_MMU_DECLS; \
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
DATA_TYPE ret; \
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
@@ -177,7 +172,6 @@ GEN_ATOMIC_HELPER(xor_fetch)
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- ATOMIC_MMU_DECLS; \
XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
XDATA_TYPE cmp, old, new, val = xval; \
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
@@ -223,7 +217,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE cmpv, ABI_TYPE newv,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
DATA_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
@@ -245,7 +238,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
ATOMIC_MMU_IDX);
@@ -260,7 +252,6 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, true,
ATOMIC_MMU_IDX);
@@ -277,7 +268,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
ABI_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
@@ -294,7 +284,6 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- ATOMIC_MMU_DECLS; \
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
DATA_TYPE ret; \
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, \
@@ -326,7 +315,6 @@ GEN_ATOMIC_HELPER(xor_fetch)
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- ATOMIC_MMU_DECLS; \
XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
XDATA_TYPE ldo, ldn, old, new, val = xval; \
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, \
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index cc0e673222..dc646e964a 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2693,7 +2693,6 @@ void cpu_stq_le_data(CPUArchState *env, target_ulong ptr, uint64_t val)
#define ATOMIC_NAME(X) \
glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
-#define ATOMIC_MMU_DECLS
#define ATOMIC_MMU_LOOKUP_RW \
atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ | PAGE_WRITE, retaddr)
#define ATOMIC_MMU_LOOKUP_R \
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index f6f8ddeb60..bc4a38b4df 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -1243,7 +1243,6 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
#define ATOMIC_NAME(X) \
glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
-#define ATOMIC_MMU_DECLS
#define ATOMIC_MMU_LOOKUP_RW atomic_mmu_lookup(env, addr, DATA_SIZE, retaddr)
#define ATOMIC_MMU_LOOKUP_R ATOMIC_MMU_LOOKUP_RW
#define ATOMIC_MMU_LOOKUP_W ATOMIC_MMU_LOOKUP_RW
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_*
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (6 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:22 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 09/10] trace: Fold mem-internal.h into mem.h Richard Henderson
` (2 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, crobinso
Unify the parameters of atomic_mmu_lookup between cputlb.c and
user-exec.c. Call the function directly, and remove the macros.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/atomic_template.h | 41 +++++++++++++++++++++++++------------
accel/tcg/cputlb.c | 7 +------
accel/tcg/user-exec.c | 12 ++++++-----
3 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index ae6b6a03be..6ee0158c5f 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -74,7 +74,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE cmpv, ABI_TYPE newv,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
DATA_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
ATOMIC_MMU_IDX);
@@ -95,7 +96,9 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ, retaddr);
+ DATA_TYPE val;
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
ATOMIC_MMU_IDX);
@@ -109,7 +112,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_WRITE, retaddr);
uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
ATOMIC_MMU_IDX);
@@ -123,7 +127,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
DATA_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
ATOMIC_MMU_IDX);
@@ -139,7 +144,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
+ PAGE_READ | PAGE_WRITE, retaddr); \
DATA_TYPE ret; \
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
ATOMIC_MMU_IDX); \
@@ -161,7 +167,8 @@ GEN_ATOMIC_HELPER(xor_fetch)
#undef GEN_ATOMIC_HELPER
-/* These helpers are, as a whole, full barriers. Within the helper,
+/*
+ * These helpers are, as a whole, full barriers. Within the helper,
* the leading barrier is explicit and the trailing barrier is within
* cmpxchg primitive.
*
@@ -172,7 +179,8 @@ GEN_ATOMIC_HELPER(xor_fetch)
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
+ XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
+ PAGE_READ | PAGE_WRITE, retaddr); \
XDATA_TYPE cmp, old, new, val = xval; \
uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
ATOMIC_MMU_IDX); \
@@ -217,7 +225,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE cmpv, ABI_TYPE newv,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
DATA_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
ATOMIC_MMU_IDX);
@@ -238,7 +247,9 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ, retaddr);
+ DATA_TYPE val;
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
ATOMIC_MMU_IDX);
@@ -252,7 +263,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_WRITE, retaddr);
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, true,
ATOMIC_MMU_IDX);
@@ -268,7 +280,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
{
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
ABI_TYPE ret;
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
ATOMIC_MMU_IDX);
@@ -284,7 +297,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
+ DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
+ PAGE_READ | PAGE_WRITE, retaddr); \
DATA_TYPE ret; \
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, \
false, ATOMIC_MMU_IDX); \
@@ -315,7 +329,8 @@ GEN_ATOMIC_HELPER(xor_fetch)
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
{ \
- XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW; \
+ XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
+ PAGE_READ | PAGE_WRITE, retaddr); \
XDATA_TYPE ldo, ldn, old, new, val = xval; \
uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, \
false, ATOMIC_MMU_IDX); \
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index dc646e964a..b1e5471f94 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2693,12 +2693,7 @@ void cpu_stq_le_data(CPUArchState *env, target_ulong ptr, uint64_t val)
#define ATOMIC_NAME(X) \
glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
-#define ATOMIC_MMU_LOOKUP_RW \
- atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ | PAGE_WRITE, retaddr)
-#define ATOMIC_MMU_LOOKUP_R \
- atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ, retaddr)
-#define ATOMIC_MMU_LOOKUP_W \
- atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_WRITE, retaddr)
+
#define ATOMIC_MMU_CLEANUP
#define ATOMIC_MMU_IDX get_mmuidx(oi)
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index bc4a38b4df..90d1a2d327 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -1221,9 +1221,14 @@ uint64_t cpu_ldq_code(CPUArchState *env, abi_ptr ptr)
return ret;
}
-/* Do not allow unaligned operations to proceed. Return the host address. */
+/*
+ * Do not allow unaligned operations to proceed. Return the host address.
+ *
+ * @prot may be PAGE_READ, PAGE_WRITE, or PAGE_READ|PAGE_WRITE.
+ */
static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
- int size, uintptr_t retaddr)
+ TCGMemOpIdx oi, int size, int prot,
+ uintptr_t retaddr)
{
/* Enforce qemu required alignment. */
if (unlikely(addr & (size - 1))) {
@@ -1243,9 +1248,6 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
#define ATOMIC_NAME(X) \
glue(glue(glue(cpu_atomic_ ## X, SUFFIX), END), _mmu)
-#define ATOMIC_MMU_LOOKUP_RW atomic_mmu_lookup(env, addr, DATA_SIZE, retaddr)
-#define ATOMIC_MMU_LOOKUP_R ATOMIC_MMU_LOOKUP_RW
-#define ATOMIC_MMU_LOOKUP_W ATOMIC_MMU_LOOKUP_RW
#define ATOMIC_MMU_CLEANUP do { clear_helper_retaddr(); } while (0)
#define ATOMIC_MMU_IDX MMU_USER_IDX
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 09/10] trace: Fold mem-internal.h into mem.h
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (7 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_* Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:26 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 10/10] accel/tcg: Push trace info building into atomic_common.c.inc Richard Henderson
2021-07-18 22:33 ` [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Cole Robinson
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, Philippe Mathieu-Daudé, crobinso
Since the last thing that mem.h does is include mem-internal.h,
the symbols are not actually private.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
trace/mem-internal.h | 50 --------------------------------------------
trace/mem.h | 50 ++++++++++++++++++++++++++++++++++----------
plugins/core.c | 2 +-
3 files changed, 40 insertions(+), 62 deletions(-)
delete mode 100644 trace/mem-internal.h
diff --git a/trace/mem-internal.h b/trace/mem-internal.h
deleted file mode 100644
index 8b72b678fa..0000000000
--- a/trace/mem-internal.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Helper functions for guest memory tracing
- *
- * Copyright (C) 2016 Lluís Vilanova <vilanova@ac.upc.edu>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- */
-
-#ifndef TRACE__MEM_INTERNAL_H
-#define TRACE__MEM_INTERNAL_H
-
-#define TRACE_MEM_SZ_SHIFT_MASK 0xf /* size shift mask */
-#define TRACE_MEM_SE (1ULL << 4) /* sign extended (y/n) */
-#define TRACE_MEM_BE (1ULL << 5) /* big endian (y/n) */
-#define TRACE_MEM_ST (1ULL << 6) /* store (y/n) */
-#define TRACE_MEM_MMU_SHIFT 8 /* mmu idx */
-
-static inline uint16_t trace_mem_build_info(
- int size_shift, bool sign_extend, MemOp endianness,
- bool store, unsigned int mmu_idx)
-{
- uint16_t res;
-
- res = size_shift & TRACE_MEM_SZ_SHIFT_MASK;
- if (sign_extend) {
- res |= TRACE_MEM_SE;
- }
- if (endianness == MO_BE) {
- res |= TRACE_MEM_BE;
- }
- if (store) {
- res |= TRACE_MEM_ST;
- }
-#ifdef CONFIG_SOFTMMU
- res |= mmu_idx << TRACE_MEM_MMU_SHIFT;
-#endif
- return res;
-}
-
-static inline uint16_t trace_mem_get_info(MemOp op,
- unsigned int mmu_idx,
- bool store)
-{
- return trace_mem_build_info(op & MO_SIZE, !!(op & MO_SIGN),
- op & MO_BSWAP, store,
- mmu_idx);
-}
-
-#endif /* TRACE__MEM_INTERNAL_H */
diff --git a/trace/mem.h b/trace/mem.h
index 9644f592b4..2f27e7bdf0 100644
--- a/trace/mem.h
+++ b/trace/mem.h
@@ -12,24 +12,52 @@
#include "tcg/tcg.h"
-
-/**
- * trace_mem_get_info:
- *
- * Return a value for the 'info' argument in guest memory access traces.
- */
-static uint16_t trace_mem_get_info(MemOp op, unsigned int mmu_idx, bool store);
+#define TRACE_MEM_SZ_SHIFT_MASK 0xf /* size shift mask */
+#define TRACE_MEM_SE (1ULL << 4) /* sign extended (y/n) */
+#define TRACE_MEM_BE (1ULL << 5) /* big endian (y/n) */
+#define TRACE_MEM_ST (1ULL << 6) /* store (y/n) */
+#define TRACE_MEM_MMU_SHIFT 8 /* mmu idx */
/**
* trace_mem_build_info:
*
* Return a value for the 'info' argument in guest memory access traces.
*/
-static uint16_t trace_mem_build_info(int size_shift, bool sign_extend,
- MemOp endianness, bool store,
- unsigned int mmuidx);
+static inline uint16_t trace_mem_build_info(int size_shift, bool sign_extend,
+ MemOp endianness, bool store,
+ unsigned int mmu_idx)
+{
+ uint16_t res;
+
+ res = size_shift & TRACE_MEM_SZ_SHIFT_MASK;
+ if (sign_extend) {
+ res |= TRACE_MEM_SE;
+ }
+ if (endianness == MO_BE) {
+ res |= TRACE_MEM_BE;
+ }
+ if (store) {
+ res |= TRACE_MEM_ST;
+ }
+#ifdef CONFIG_SOFTMMU
+ res |= mmu_idx << TRACE_MEM_MMU_SHIFT;
+#endif
+ return res;
+}
-#include "trace/mem-internal.h"
+/**
+ * trace_mem_get_info:
+ *
+ * Return a value for the 'info' argument in guest memory access traces.
+ */
+static inline uint16_t trace_mem_get_info(MemOp op,
+ unsigned int mmu_idx,
+ bool store)
+{
+ return trace_mem_build_info(op & MO_SIZE, !!(op & MO_SIGN),
+ op & MO_BSWAP, store,
+ mmu_idx);
+}
#endif /* TRACE__MEM_H */
diff --git a/plugins/core.c b/plugins/core.c
index e1bcdb570d..474db287cb 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -27,7 +27,7 @@
#include "exec/helper-proto.h"
#include "tcg/tcg.h"
#include "tcg/tcg-op.h"
-#include "trace/mem-internal.h" /* mem_info macros */
+#include "trace/mem.h" /* mem_info macros */
#include "plugin.h"
#include "qemu/compiler.h"
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 10/10] accel/tcg: Push trace info building into atomic_common.c.inc
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (8 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 09/10] trace: Fold mem-internal.h into mem.h Richard Henderson
@ 2021-07-17 19:24 ` Richard Henderson
2021-07-19 12:25 ` Peter Maydell
2021-07-18 22:33 ` [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Cole Robinson
10 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2021-07-17 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, pbonzini, crobinso
Use trace_mem_get_info instead of trace_mem_build_info,
using the TCGMemOpIdx that we already have. Do this in
the atomic_trace_*_pre function as common subroutines.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/atomic_template.h | 48 +++++++++--------------------------
accel/tcg/atomic_common.c.inc | 37 ++++++++++++++++++---------
2 files changed, 37 insertions(+), 48 deletions(-)
diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 6ee0158c5f..d89af4cc1e 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -77,10 +77,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_READ | PAGE_WRITE, retaddr);
DATA_TYPE ret;
- uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
- atomic_trace_rmw_pre(env, addr, info);
#if DATA_SIZE == 16
ret = atomic16_cmpxchg(haddr, cmpv, newv);
#else
@@ -99,10 +97,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_READ, retaddr);
DATA_TYPE val;
- uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_ld_pre(env, addr, oi);
- atomic_trace_ld_pre(env, addr, info);
val = atomic16_read(haddr);
ATOMIC_MMU_CLEANUP;
atomic_trace_ld_post(env, addr, info);
@@ -114,10 +110,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
{
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_WRITE, retaddr);
- uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_st_pre(env, addr, oi);
- atomic_trace_st_pre(env, addr, info);
atomic16_set(haddr, val);
ATOMIC_MMU_CLEANUP;
atomic_trace_st_post(env, addr, info);
@@ -130,10 +124,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_READ | PAGE_WRITE, retaddr);
DATA_TYPE ret;
- uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
- atomic_trace_rmw_pre(env, addr, info);
ret = qatomic_xchg__nocheck(haddr, val);
ATOMIC_MMU_CLEANUP;
atomic_trace_rmw_post(env, addr, info);
@@ -147,9 +139,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
PAGE_READ | PAGE_WRITE, retaddr); \
DATA_TYPE ret; \
- uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
- ATOMIC_MMU_IDX); \
- atomic_trace_rmw_pre(env, addr, info); \
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi); \
ret = qatomic_##X(haddr, val); \
ATOMIC_MMU_CLEANUP; \
atomic_trace_rmw_post(env, addr, info); \
@@ -182,9 +172,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
PAGE_READ | PAGE_WRITE, retaddr); \
XDATA_TYPE cmp, old, new, val = xval; \
- uint16_t info = trace_mem_build_info(SHIFT, false, 0, false, \
- ATOMIC_MMU_IDX); \
- atomic_trace_rmw_pre(env, addr, info); \
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi); \
smp_mb(); \
cmp = qatomic_read__nocheck(haddr); \
do { \
@@ -228,10 +216,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_READ | PAGE_WRITE, retaddr);
DATA_TYPE ret;
- uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
- atomic_trace_rmw_pre(env, addr, info);
#if DATA_SIZE == 16
ret = atomic16_cmpxchg(haddr, BSWAP(cmpv), BSWAP(newv));
#else
@@ -250,10 +236,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_READ, retaddr);
DATA_TYPE val;
- uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_ld_pre(env, addr, oi);
- atomic_trace_ld_pre(env, addr, info);
val = atomic16_read(haddr);
ATOMIC_MMU_CLEANUP;
atomic_trace_ld_post(env, addr, info);
@@ -265,11 +249,9 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
{
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_WRITE, retaddr);
- uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, true,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_st_pre(env, addr, oi);
val = BSWAP(val);
- atomic_trace_st_pre(env, addr, info);
val = BSWAP(val);
atomic16_set(haddr, val);
ATOMIC_MMU_CLEANUP;
@@ -283,10 +265,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
PAGE_READ | PAGE_WRITE, retaddr);
ABI_TYPE ret;
- uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
- ATOMIC_MMU_IDX);
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
- atomic_trace_rmw_pre(env, addr, info);
ret = qatomic_xchg__nocheck(haddr, BSWAP(val));
ATOMIC_MMU_CLEANUP;
atomic_trace_rmw_post(env, addr, info);
@@ -300,9 +280,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
PAGE_READ | PAGE_WRITE, retaddr); \
DATA_TYPE ret; \
- uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, \
- false, ATOMIC_MMU_IDX); \
- atomic_trace_rmw_pre(env, addr, info); \
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi); \
ret = qatomic_##X(haddr, BSWAP(val)); \
ATOMIC_MMU_CLEANUP; \
atomic_trace_rmw_post(env, addr, info); \
@@ -332,9 +310,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
PAGE_READ | PAGE_WRITE, retaddr); \
XDATA_TYPE ldo, ldn, old, new, val = xval; \
- uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, \
- false, ATOMIC_MMU_IDX); \
- atomic_trace_rmw_pre(env, addr, info); \
+ uint16_t info = atomic_trace_rmw_pre(env, addr, oi); \
smp_mb(); \
ldn = qatomic_read__nocheck(haddr); \
do { \
diff --git a/accel/tcg/atomic_common.c.inc b/accel/tcg/atomic_common.c.inc
index a668cf0d6f..6c0339f610 100644
--- a/accel/tcg/atomic_common.c.inc
+++ b/accel/tcg/atomic_common.c.inc
@@ -13,45 +13,58 @@
* See the COPYING file in the top-level directory.
*/
-static inline
-void atomic_trace_rmw_pre(CPUArchState *env, target_ulong addr, uint16_t info)
+static uint16_t atomic_trace_rmw_pre(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi)
{
CPUState *cpu = env_cpu(env);
+ uint16_t info = trace_mem_get_info(get_memop(oi), get_mmuidx(oi), false);
trace_guest_mem_before_exec(cpu, addr, info);
trace_guest_mem_before_exec(cpu, addr, info | TRACE_MEM_ST);
+
+ return info;
}
-static inline void
-atomic_trace_rmw_post(CPUArchState *env, target_ulong addr, uint16_t info)
+static void atomic_trace_rmw_post(CPUArchState *env, target_ulong addr,
+ uint16_t info)
{
qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, info);
qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, info | TRACE_MEM_ST);
}
-static inline
-void atomic_trace_ld_pre(CPUArchState *env, target_ulong addr, uint16_t info)
+#if HAVE_ATOMIC128
+static uint16_t atomic_trace_ld_pre(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi)
{
+ uint16_t info = trace_mem_get_info(get_memop(oi), get_mmuidx(oi), false);
+
trace_guest_mem_before_exec(env_cpu(env), addr, info);
+
+ return info;
}
-static inline
-void atomic_trace_ld_post(CPUArchState *env, target_ulong addr, uint16_t info)
+static void atomic_trace_ld_post(CPUArchState *env, target_ulong addr,
+ uint16_t info)
{
qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, info);
}
-static inline
-void atomic_trace_st_pre(CPUArchState *env, target_ulong addr, uint16_t info)
+static uint16_t atomic_trace_st_pre(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi)
{
+ uint16_t info = trace_mem_get_info(get_memop(oi), get_mmuidx(oi), true);
+
trace_guest_mem_before_exec(env_cpu(env), addr, info);
+
+ return info;
}
-static inline
-void atomic_trace_st_post(CPUArchState *env, target_ulong addr, uint16_t info)
+static void atomic_trace_st_post(CPUArchState *env, target_ulong addr,
+ uint16_t info)
{
qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, info);
}
+#endif
/*
* Atomic helpers callable from TCG.
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 00/10] Atomic cleanup + clang-12 build fix
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
` (9 preceding siblings ...)
2021-07-17 19:24 ` [PATCH v3 10/10] accel/tcg: Push trace info building into atomic_common.c.inc Richard Henderson
@ 2021-07-18 22:33 ` Cole Robinson
10 siblings, 0 replies; 25+ messages in thread
From: Cole Robinson @ 2021-07-18 22:33 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: pbonzini, peter.maydell
On 7/17/21 3:24 PM, Richard Henderson wrote:
> This is intended to fix building with clang-12 on i386.
> In the process, I found bugs wrt handling of guest memory in target/
> with respect to atomics, fixed by unifying the api between softmmu
> and user-only and removing some ifdefs under target/.
>
> Unification of the api allowed some further cleanups.
>
> I think that patches 1-5 fix all of the bugs, and that 6-10 are only
> cleanup and could be left to next cycle.
>
> Changes for v3:
> * Dropped the typeof_strip_qual patch with broader testing.
> * Squashed an include fix for trace/mem.h, with plugins enabled.
> * Applied Phil's R-b.
>
>
> r~
>
>
> Richard Henderson (10):
> qemu/atomic: Use macros for CONFIG_ATOMIC64
> qemu/atomic: Remove pre-C11 atomic fallbacks
> qemu/atomic: Add aligned_{int64,uint64}_t types
> tcg: Rename helper_atomic_*_mmu and provide for user-only
> accel/tcg: Standardize atomic helpers on softmmu api
> accel/tcg: Fold EXTRA_ARGS into atomic_template.h
> accel/tcg: Remove ATOMIC_MMU_DECLS
> accel/tcg: Expand ATOMIC_MMU_LOOKUP_*
> trace: Fold mem-internal.h into mem.h
> accel/tcg: Push trace info building into atomic_common.c.inc
>
> configure | 7 -
> accel/tcg/atomic_template.h | 141 +++++++++-----------
> accel/tcg/tcg-runtime.h | 46 -------
> include/qemu/atomic.h | 243 ++++------------------------------
> include/qemu/stats64.h | 2 +-
> include/tcg/tcg.h | 78 +++++------
> softmmu/timers-state.h | 2 +-
> trace/mem-internal.h | 50 -------
> trace/mem.h | 50 +++++--
> accel/tcg/cputlb.c | 49 +------
> accel/tcg/user-exec.c | 41 +++---
> linux-user/hppa/cpu_loop.c | 2 +-
> plugins/core.c | 2 +-
> target/arm/helper-a64.c | 8 +-
> target/i386/tcg/mem_helper.c | 15 +--
> target/m68k/op_helper.c | 19 +--
> target/ppc/mem_helper.c | 16 +--
> target/s390x/tcg/mem_helper.c | 19 +--
> tcg/tcg-op.c | 47 ++-----
> util/qsp.c | 4 +-
> accel/tcg/atomic_common.c.inc | 107 +++++++++++++--
> 21 files changed, 321 insertions(+), 627 deletions(-)
> delete mode 100644 trace/mem-internal.h
>
This series fixes the clang i686 build issue for me. Thanks Richard!
Tested-by: Cole Robinson <crobinso@redhat.com>
- Cole
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64
2021-07-17 19:24 ` [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64 Richard Henderson
@ 2021-07-19 12:01 ` Peter Maydell
2021-07-20 20:38 ` Richard Henderson
0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:01 UTC (permalink / raw)
To: Richard Henderson
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Clang warnings about questionable atomic usage get localized
> to the inline function in atomic.h. By using a macro, we get
> the full traceback to the original use that caused the warning.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/atomic.h | 25 +++++--------------------
> 1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index 3ccf84fd46..99d6030095 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -457,26 +457,11 @@
>
> /* Abstractions to access atomically (i.e. "once") i64/u64 variables */
> #ifdef CONFIG_ATOMIC64
> -static inline int64_t qatomic_read_i64(const int64_t *ptr)
> -{
> - /* use __nocheck because sizeof(void *) might be < sizeof(u64) */
> - return qatomic_read__nocheck(ptr);
> -}
> -
> -static inline uint64_t qatomic_read_u64(const uint64_t *ptr)
> -{
> - return qatomic_read__nocheck(ptr);
> -}
> -
> -static inline void qatomic_set_i64(int64_t *ptr, int64_t val)
> -{
> - qatomic_set__nocheck(ptr, val);
> -}
> -
> -static inline void qatomic_set_u64(uint64_t *ptr, uint64_t val)
> -{
> - qatomic_set__nocheck(ptr, val);
> -}
> +/* Use __nocheck because sizeof(void *) might be < sizeof(u64) */
> +#define qatomic_read_i64 qatomic_read__nocheck
> +#define qatomic_read_u64 qatomic_read__nocheck
> +#define qatomic_set_i64 qatomic_set__nocheck
> +#define qatomic_set_u64 qatomic_set__nocheck
Previously if you tried to do qatomic_set_i64() etc on something
that wasn't an int64_t*, the compiler would complain. Now it will
silently store a different-sized value, I think. Is there a way
to retain the typechecking in the macro versions?
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks
2021-07-17 19:24 ` [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks Richard Henderson
@ 2021-07-19 12:05 ` Peter Maydell
2021-07-19 15:40 ` Alex Bennée
1 sibling, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:05 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We now require c11, so the fallbacks are now dead code
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> configure | 7 --
> include/qemu/atomic.h | 204 +++---------------------------------------
> 2 files changed, 10 insertions(+), 201 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only
2021-07-17 19:24 ` [PATCH v3 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only Richard Henderson
@ 2021-07-19 12:17 ` Peter Maydell
0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:17 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Always provide the atomic interface using TCGMemOpIdx oi
> and uintptr_t retaddr. Rename from helper_* to cpu_* so
> as to (mostly) match the exec/cpu_ldst.h functions, and
> to emphasize that they are not callable from TCG directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 05/10] accel/tcg: Standardize atomic helpers on softmmu api
2021-07-17 19:24 ` [PATCH v3 05/10] accel/tcg: Standardize atomic helpers on softmmu api Richard Henderson
@ 2021-07-19 12:19 ` Peter Maydell
0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:19 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Reduce the amount of code duplication by always passing
> the TCGMemOpIdx argument to helper_atomic_*. This is not
> currently used for user-only, but it's easy to ignore.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> @@ -3144,14 +3137,8 @@ void tcg_gen_atomic_cmpxchg_i32(TCGv_i32 retv, TCGv addr, TCGv_i32 cmpv,
> gen = table_cmpxchg[memop & (MO_SIZE | MO_BSWAP)];
> tcg_debug_assert(gen != NULL);
>
> -#ifdef CONFIG_SOFTMMU
> - {
> - TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
> - gen(retv, cpu_env, addr, cmpv, newv, tcg_constant_i32(oi));
> - }
> -#else
> - gen(retv, cpu_env, addr, cmpv, newv);
> -#endif
> + TCGMemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
> + gen(retv, cpu_env, addr, cmpv, newv, tcg_constant_i32(oi));
In all of these, our coding style says 'oi' should be declared at the top
of the function, not in the middle like this.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h
2021-07-17 19:24 ` [PATCH v3 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h Richard Henderson
@ 2021-07-19 12:19 ` Peter Maydell
0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:19 UTC (permalink / raw)
To: Richard Henderson
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> All instances of EXTRA_ARGS are now identical.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> accel/tcg/atomic_template.h | 36 ++++++++++++++++++++----------------
> accel/tcg/cputlb.c | 1 -
> accel/tcg/user-exec.c | 1 -
> 3 files changed, 20 insertions(+), 18 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS
2021-07-17 19:24 ` [PATCH v3 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS Richard Henderson
@ 2021-07-19 12:20 ` Peter Maydell
0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:20 UTC (permalink / raw)
To: Richard Henderson
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> All definitions are now empty.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_*
2021-07-17 19:24 ` [PATCH v3 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_* Richard Henderson
@ 2021-07-19 12:22 ` Peter Maydell
0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:22 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Unify the parameters of atomic_mmu_lookup between cputlb.c and
> user-exec.c. Call the function directly, and remove the macros.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 10/10] accel/tcg: Push trace info building into atomic_common.c.inc
2021-07-17 19:24 ` [PATCH v3 10/10] accel/tcg: Push trace info building into atomic_common.c.inc Richard Henderson
@ 2021-07-19 12:25 ` Peter Maydell
0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Use trace_mem_get_info instead of trace_mem_build_info,
> using the TCGMemOpIdx that we already have. Do this in
> the atomic_trace_*_pre function as common subroutines.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> accel/tcg/atomic_template.h | 48 +++++++++--------------------------
> accel/tcg/atomic_common.c.inc | 37 ++++++++++++++++++---------
> 2 files changed, 37 insertions(+), 48 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 09/10] trace: Fold mem-internal.h into mem.h
2021-07-17 19:24 ` [PATCH v3 09/10] trace: Fold mem-internal.h into mem.h Richard Henderson
@ 2021-07-19 12:26 ` Peter Maydell
0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:26 UTC (permalink / raw)
To: Richard Henderson
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Since the last thing that mem.h does is include mem-internal.h,
> the symbols are not actually private.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types
2021-07-17 19:24 ` [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types Richard Henderson
@ 2021-07-19 12:39 ` Peter Maydell
2021-07-20 20:52 ` Richard Henderson
0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2021-07-19 12:39 UTC (permalink / raw)
To: Richard Henderson
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
QEMU Developers, Cole Robinson
On Sat, 17 Jul 2021 at 20:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Use it to avoid some clang-12 -Watomic-alignment errors,
> forcing some structures to be aligned and as a pointer when
> we have ensured that the address is aligned.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> accel/tcg/atomic_template.h | 4 ++--
> include/qemu/atomic.h | 14 +++++++++++++-
> include/qemu/stats64.h | 2 +-
> softmmu/timers-state.h | 2 +-
> linux-user/hppa/cpu_loop.c | 2 +-
> util/qsp.c | 4 ++--
> 6 files changed, 20 insertions(+), 8 deletions(-)
> diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
> index 3aaaf3337c..82d8183821 100644
> --- a/linux-user/hppa/cpu_loop.c
> +++ b/linux-user/hppa/cpu_loop.c
> @@ -82,7 +82,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
> o64 = *(uint64_t *)g2h(cs, old);
> n64 = *(uint64_t *)g2h(cs, new);
> #ifdef CONFIG_ATOMIC64
> - r64 = qatomic_cmpxchg__nocheck((uint64_t *)g2h(cs, addr),
> + r64 = qatomic_cmpxchg__nocheck((aligned_uint64_t *)g2h(cs, addr),
> o64, n64);
This cast is OK, but it took me a while to verify that:
* we check that 'addr' is 8-aligned further up in this function
* we check that guest_base is at least page-aligned in
probe_guest_base(), and there's no way to avoid that function
getting called if you specify a guest-base value on the command line
Is it worth asserting that the value we get back from g2h() really
is 8-aligned before casting ?
Anyway,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks
2021-07-17 19:24 ` [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks Richard Henderson
2021-07-19 12:05 ` Peter Maydell
@ 2021-07-19 15:40 ` Alex Bennée
1 sibling, 0 replies; 25+ messages in thread
From: Alex Bennée @ 2021-07-19 15:40 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, crobinso, qemu-devel, pbonzini
Richard Henderson <richard.henderson@linaro.org> writes:
> We now require c11, so the fallbacks are now dead code
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64
2021-07-19 12:01 ` Peter Maydell
@ 2021-07-20 20:38 ` Richard Henderson
0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2021-07-20 20:38 UTC (permalink / raw)
To: Peter Maydell
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
QEMU Developers, Cole Robinson
On 7/19/21 2:01 AM, Peter Maydell wrote:
>> -static inline void qatomic_set_u64(uint64_t *ptr, uint64_t val)
>> -{
>> - qatomic_set__nocheck(ptr, val);
>> -}
...
>> +#define qatomic_set_u64 qatomic_set__nocheck
>
> Previously if you tried to do qatomic_set_i64() etc on something
> that wasn't an int64_t*, the compiler would complain. Now it will
> silently store a different-sized value, I think. Is there a way
> to retain the typechecking in the macro versions?
I can add a trivial _Generic wrapper.
r~
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types
2021-07-19 12:39 ` Peter Maydell
@ 2021-07-20 20:52 ` Richard Henderson
0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2021-07-20 20:52 UTC (permalink / raw)
To: Peter Maydell
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
QEMU Developers, Cole Robinson
On 7/19/21 2:39 AM, Peter Maydell wrote:
> This cast is OK, but it took me a while to verify that:
> * we check that 'addr' is 8-aligned further up in this function
> * we check that guest_base is at least page-aligned in
> probe_guest_base(), and there's no way to avoid that function
> getting called if you specify a guest-base value on the command line
Yep.
> Is it worth asserting that the value we get back from g2h() really
> is 8-aligned before casting ?
No, I don't think so. I think the check on the guest address is sufficient, and Just Know
that guest_base is not going to misalign anything.
r~
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2021-07-20 20:52 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-17 19:24 [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Richard Henderson
2021-07-17 19:24 ` [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64 Richard Henderson
2021-07-19 12:01 ` Peter Maydell
2021-07-20 20:38 ` Richard Henderson
2021-07-17 19:24 ` [PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks Richard Henderson
2021-07-19 12:05 ` Peter Maydell
2021-07-19 15:40 ` Alex Bennée
2021-07-17 19:24 ` [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types Richard Henderson
2021-07-19 12:39 ` Peter Maydell
2021-07-20 20:52 ` Richard Henderson
2021-07-17 19:24 ` [PATCH v3 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only Richard Henderson
2021-07-19 12:17 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 05/10] accel/tcg: Standardize atomic helpers on softmmu api Richard Henderson
2021-07-19 12:19 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h Richard Henderson
2021-07-19 12:19 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS Richard Henderson
2021-07-19 12:20 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_* Richard Henderson
2021-07-19 12:22 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 09/10] trace: Fold mem-internal.h into mem.h Richard Henderson
2021-07-19 12:26 ` Peter Maydell
2021-07-17 19:24 ` [PATCH v3 10/10] accel/tcg: Push trace info building into atomic_common.c.inc Richard Henderson
2021-07-19 12:25 ` Peter Maydell
2021-07-18 22:33 ` [PATCH v3 00/10] Atomic cleanup + clang-12 build fix Cole Robinson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.