* [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* 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 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
* [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* 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 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
* [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* 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 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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 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 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