* [Qemu-devel] [PULL 1/6] trace: fix misreporting of TCG access sizes for user-space
2018-06-27 12:58 [Qemu-devel] [PULL 0/6] Tracing patches Stefan Hajnoczi
@ 2018-06-27 12:58 ` Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 2/6] trace: simplify trace_mem functions Stefan Hajnoczi
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2018-06-27 12:58 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Dr. David Alan Gilbert, Richard Henderson,
Peter Crosthwaite, Stefan Hajnoczi, Juan Quintela,
Markus Armbruster, Michael Roth, Peter Maydell, Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
trace_mem_build_info expects a size_shift for its first argument. Fix it.
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-id: 1527028012-21888-2-git-send-email-cota@braap.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/exec/cpu_ldst_useronly_template.h | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/include/exec/cpu_ldst_useronly_template.h b/include/exec/cpu_ldst_useronly_template.h
index c168f31bba..e30e58ed4a 100644
--- a/include/exec/cpu_ldst_useronly_template.h
+++ b/include/exec/cpu_ldst_useronly_template.h
@@ -33,20 +33,24 @@
#define SUFFIX q
#define USUFFIX q
#define DATA_TYPE uint64_t
+#define SHIFT 3
#elif DATA_SIZE == 4
#define SUFFIX l
#define USUFFIX l
#define DATA_TYPE uint32_t
+#define SHIFT 2
#elif DATA_SIZE == 2
#define SUFFIX w
#define USUFFIX uw
#define DATA_TYPE uint16_t
#define DATA_STYPE int16_t
+#define SHIFT 1
#elif DATA_SIZE == 1
#define SUFFIX b
#define USUFFIX ub
#define DATA_TYPE uint8_t
#define DATA_STYPE int8_t
+#define SHIFT 0
#else
#error unsupported data size
#endif
@@ -63,7 +67,7 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
#if !defined(CODE_ACCESS)
trace_guest_mem_before_exec(
ENV_GET_CPU(env), ptr,
- trace_mem_build_info(DATA_SIZE, false, MO_TE, false));
+ trace_mem_build_info(SHIFT, false, MO_TE, false));
#endif
return glue(glue(ld, USUFFIX), _p)(g2h(ptr));
}
@@ -87,7 +91,7 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
#if !defined(CODE_ACCESS)
trace_guest_mem_before_exec(
ENV_GET_CPU(env), ptr,
- trace_mem_build_info(DATA_SIZE, true, MO_TE, false));
+ trace_mem_build_info(SHIFT, true, MO_TE, false));
#endif
return glue(glue(lds, SUFFIX), _p)(g2h(ptr));
}
@@ -113,7 +117,7 @@ glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr,
#if !defined(CODE_ACCESS)
trace_guest_mem_before_exec(
ENV_GET_CPU(env), ptr,
- trace_mem_build_info(DATA_SIZE, false, MO_TE, true));
+ trace_mem_build_info(SHIFT, false, MO_TE, true));
#endif
glue(glue(st, SUFFIX), _p)(g2h(ptr), v);
}
@@ -136,3 +140,4 @@ glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
#undef SUFFIX
#undef USUFFIX
#undef DATA_SIZE
+#undef SHIFT
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 2/6] trace: simplify trace_mem functions
2018-06-27 12:58 [Qemu-devel] [PULL 0/6] Tracing patches Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 1/6] trace: fix misreporting of TCG access sizes for user-space Stefan Hajnoczi
@ 2018-06-27 12:58 ` Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 3/6] trace: expand mem_info:size_shift to 3 bits Stefan Hajnoczi
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2018-06-27 12:58 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Dr. David Alan Gilbert, Richard Henderson,
Peter Crosthwaite, Stefan Hajnoczi, Juan Quintela,
Markus Armbruster, Michael Roth, Peter Maydell, Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
Add some defines for the mem_info bits, simplify
trace_mem_build_info, and also simplify trace_mem_get_info
by making it a wrapper around trace_mem_build_info.
This paves the way for increasing size_shift by one bit.
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-id: 1527028012-21888-3-git-send-email-cota@braap.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
trace/mem-internal.h | 46 ++++++++++++++++++++------------------------
trace/mem.h | 2 +-
2 files changed, 22 insertions(+), 26 deletions(-)
diff --git a/trace/mem-internal.h b/trace/mem-internal.h
index ddda934253..b684e2750c 100644
--- a/trace/mem-internal.h
+++ b/trace/mem-internal.h
@@ -10,37 +10,33 @@
#ifndef TRACE__MEM_INTERNAL_H
#define TRACE__MEM_INTERNAL_H
-static inline uint8_t trace_mem_get_info(TCGMemOp op, bool store)
-{
- uint8_t res = op;
- bool be = (op & MO_BSWAP) == MO_BE;
-
- /* remove untraced fields */
- res &= (1ULL << 4) - 1;
- /* make endianness absolute */
- res &= ~MO_BSWAP;
- if (be) {
- res |= 1ULL << 3;
- }
- /* add fields */
- if (store) {
- res |= 1ULL << 4;
- }
-
- return res;
-}
+#define TRACE_MEM_SZ_SHIFT_MASK 0x3 /* size shift mask */
+#define TRACE_MEM_SE (1ULL << 2) /* sign extended (y/n) */
+#define TRACE_MEM_BE (1ULL << 3) /* big endian (y/n) */
+#define TRACE_MEM_ST (1ULL << 4) /* store (y/n) */
static inline uint8_t trace_mem_build_info(
- TCGMemOp size, bool sign_extend, TCGMemOp endianness, bool store)
+ int size_shift, bool sign_extend, TCGMemOp endianness, bool store)
{
- uint8_t res = 0;
- res |= size;
- res |= (sign_extend << 2);
+ uint8_t res;
+
+ res = size_shift & TRACE_MEM_SZ_SHIFT_MASK;
+ if (sign_extend) {
+ res |= TRACE_MEM_SE;
+ }
if (endianness == MO_BE) {
- res |= (1ULL << 3);
+ res |= TRACE_MEM_BE;
+ }
+ if (store) {
+ res |= TRACE_MEM_ST;
}
- res |= (store << 4);
return res;
}
+static inline uint8_t trace_mem_get_info(TCGMemOp op, bool store)
+{
+ return trace_mem_build_info(op & MO_SIZE, !!(op & MO_SIGN),
+ op & MO_BSWAP, store);
+}
+
#endif /* TRACE__MEM_INTERNAL_H */
diff --git a/trace/mem.h b/trace/mem.h
index 9c88bcb4e6..2b58196e53 100644
--- a/trace/mem.h
+++ b/trace/mem.h
@@ -25,7 +25,7 @@ static uint8_t trace_mem_get_info(TCGMemOp op, bool store);
*
* Return a value for the 'info' argument in guest memory access traces.
*/
-static uint8_t trace_mem_build_info(TCGMemOp size, bool sign_extend,
+static uint8_t trace_mem_build_info(int size_shift, bool sign_extend,
TCGMemOp endianness, bool store);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 3/6] trace: expand mem_info:size_shift to 3 bits
2018-06-27 12:58 [Qemu-devel] [PULL 0/6] Tracing patches Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 1/6] trace: fix misreporting of TCG access sizes for user-space Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 2/6] trace: simplify trace_mem functions Stefan Hajnoczi
@ 2018-06-27 12:58 ` Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 4/6] trace: add trace_mem_build_info_no_se_be/le Stefan Hajnoczi
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2018-06-27 12:58 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Dr. David Alan Gilbert, Richard Henderson,
Peter Crosthwaite, Stefan Hajnoczi, Juan Quintela,
Markus Armbruster, Michael Roth, Peter Maydell, Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
This will allow us to trace 16B-long memory accesses.
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-id: 1527028012-21888-4-git-send-email-cota@braap.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
trace/mem-internal.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/trace/mem-internal.h b/trace/mem-internal.h
index b684e2750c..a9e408eb2f 100644
--- a/trace/mem-internal.h
+++ b/trace/mem-internal.h
@@ -10,10 +10,10 @@
#ifndef TRACE__MEM_INTERNAL_H
#define TRACE__MEM_INTERNAL_H
-#define TRACE_MEM_SZ_SHIFT_MASK 0x3 /* size shift mask */
-#define TRACE_MEM_SE (1ULL << 2) /* sign extended (y/n) */
-#define TRACE_MEM_BE (1ULL << 3) /* big endian (y/n) */
-#define TRACE_MEM_ST (1ULL << 4) /* store (y/n) */
+#define TRACE_MEM_SZ_SHIFT_MASK 0x7 /* size shift mask */
+#define TRACE_MEM_SE (1ULL << 3) /* sign extended (y/n) */
+#define TRACE_MEM_BE (1ULL << 4) /* big endian (y/n) */
+#define TRACE_MEM_ST (1ULL << 5) /* store (y/n) */
static inline uint8_t trace_mem_build_info(
int size_shift, bool sign_extend, TCGMemOp endianness, bool store)
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 4/6] trace: add trace_mem_build_info_no_se_be/le
2018-06-27 12:58 [Qemu-devel] [PULL 0/6] Tracing patches Stefan Hajnoczi
` (2 preceding siblings ...)
2018-06-27 12:58 ` [Qemu-devel] [PULL 3/6] trace: expand mem_info:size_shift to 3 bits Stefan Hajnoczi
@ 2018-06-27 12:58 ` Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 5/6] trace: enable tracing of TCG atomics Stefan Hajnoczi
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2018-06-27 12:58 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Dr. David Alan Gilbert, Richard Henderson,
Peter Crosthwaite, Stefan Hajnoczi, Juan Quintela,
Markus Armbruster, Michael Roth, Peter Maydell, Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
These will be used by the following commit.
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-id: 1527028012-21888-5-git-send-email-cota@braap.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
trace/mem-internal.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/trace/mem-internal.h b/trace/mem-internal.h
index a9e408eb2f..f6efaf6d6b 100644
--- a/trace/mem-internal.h
+++ b/trace/mem-internal.h
@@ -39,4 +39,16 @@ static inline uint8_t trace_mem_get_info(TCGMemOp op, bool store)
op & MO_BSWAP, store);
}
+static inline
+uint8_t trace_mem_build_info_no_se_be(int size_shift, bool store)
+{
+ return trace_mem_build_info(size_shift, false, MO_BE, store);
+}
+
+static inline
+uint8_t trace_mem_build_info_no_se_le(int size_shift, bool store)
+{
+ return trace_mem_build_info(size_shift, false, MO_LE, store);
+}
+
#endif /* TRACE__MEM_INTERNAL_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 5/6] trace: enable tracing of TCG atomics
2018-06-27 12:58 [Qemu-devel] [PULL 0/6] Tracing patches Stefan Hajnoczi
` (3 preceding siblings ...)
2018-06-27 12:58 ` [Qemu-devel] [PULL 4/6] trace: add trace_mem_build_info_no_se_be/le Stefan Hajnoczi
@ 2018-06-27 12:58 ` Stefan Hajnoczi
2018-06-27 12:58 ` [Qemu-devel] [PULL 6/6] trace: forbid floating point types Stefan Hajnoczi
2018-06-28 13:26 ` [Qemu-devel] [PULL 0/6] Tracing patches Peter Maydell
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2018-06-27 12:58 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Dr. David Alan Gilbert, Richard Henderson,
Peter Crosthwaite, Stefan Hajnoczi, Juan Quintela,
Markus Armbruster, Michael Roth, Peter Maydell, Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
We do not trace guest atomic accesses. Fix it.
Tested with a modified atomic_add-bench so that it executes
a deterministic number of instructions, i.e. fixed seeding,
no threading and fixed number of loop iterations instead
of running for a certain time.
Before:
- With parallel_cpus = false (no clone syscall so it is never set to true):
220070 memory accesses
- With parallel_cpus = true (hard-coded):
212105 memory accesses <-- we're not tracing the atomics!
After:
220070 memory accesses regardless of parallel_cpus.
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-id: 1527028012-21888-6-git-send-email-cota@braap.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
accel/tcg/atomic_template.h | 87 ++++++++++++++++++++++++++++++++++---
1 file changed, 81 insertions(+), 6 deletions(-)
diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 3f41ef2782..d751bcba48 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -18,30 +18,37 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "trace/mem.h"
+
#if DATA_SIZE == 16
# define SUFFIX o
# define DATA_TYPE Int128
# define BSWAP bswap128
+# define SHIFT 4
#elif DATA_SIZE == 8
# define SUFFIX q
# define DATA_TYPE uint64_t
# define SDATA_TYPE int64_t
# define BSWAP bswap64
+# define SHIFT 3
#elif DATA_SIZE == 4
# define SUFFIX l
# define DATA_TYPE uint32_t
# define SDATA_TYPE int32_t
# define BSWAP bswap32
+# define SHIFT 2
#elif DATA_SIZE == 2
# define SUFFIX w
# define DATA_TYPE uint16_t
# define SDATA_TYPE int16_t
# define BSWAP bswap16
+# define SHIFT 1
#elif DATA_SIZE == 1
# define SUFFIX b
# define DATA_TYPE uint8_t
# define SDATA_TYPE int8_t
# define BSWAP
+# define SHIFT 0
#else
# error unsupported data size
#endif
@@ -52,14 +59,37 @@
# define ABI_TYPE uint32_t
#endif
+#define ATOMIC_TRACE_RMW do { \
+ uint8_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, false); \
+ \
+ trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info); \
+ trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, \
+ info | TRACE_MEM_ST); \
+ } while (0)
+
+#define ATOMIC_TRACE_LD do { \
+ uint8_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, false); \
+ \
+ trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info); \
+ } while (0)
+
+# define ATOMIC_TRACE_ST do { \
+ uint8_t info = glue(trace_mem_build_info_no_se, MEND)(SHIFT, true); \
+ \
+ trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info); \
+ } while (0)
+
/* Define host-endian atomic operations. Note that END is used within
the ATOMIC_NAME macro, and redefined below. */
#if DATA_SIZE == 1
# define END
+# define MEND _be /* either le or be would be fine */
#elif defined(HOST_WORDS_BIGENDIAN)
# define END _be
+# define MEND _be
#else
# define END _le
+# define MEND _le
#endif
ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
@@ -67,7 +97,10 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
- DATA_TYPE ret = atomic_cmpxchg__nocheck(haddr, cmpv, newv);
+ DATA_TYPE ret;
+
+ ATOMIC_TRACE_RMW;
+ ret = atomic_cmpxchg__nocheck(haddr, cmpv, newv);
ATOMIC_MMU_CLEANUP;
return ret;
}
@@ -77,6 +110,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
{
ATOMIC_MMU_DECLS;
DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP;
+
+ ATOMIC_TRACE_LD;
__atomic_load(haddr, &val, __ATOMIC_RELAXED);
ATOMIC_MMU_CLEANUP;
return val;
@@ -87,6 +122,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
+
+ ATOMIC_TRACE_ST;
__atomic_store(haddr, &val, __ATOMIC_RELAXED);
ATOMIC_MMU_CLEANUP;
}
@@ -96,7 +133,10 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
- DATA_TYPE ret = atomic_xchg__nocheck(haddr, val);
+ DATA_TYPE ret;
+
+ ATOMIC_TRACE_RMW;
+ ret = atomic_xchg__nocheck(haddr, val);
ATOMIC_MMU_CLEANUP;
return ret;
}
@@ -107,7 +147,10 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
{ \
ATOMIC_MMU_DECLS; \
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \
- DATA_TYPE ret = atomic_##X(haddr, val); \
+ DATA_TYPE ret; \
+ \
+ ATOMIC_TRACE_RMW; \
+ ret = atomic_##X(haddr, val); \
ATOMIC_MMU_CLEANUP; \
return ret; \
}
@@ -126,6 +169,9 @@ GEN_ATOMIC_HELPER(xor_fetch)
/* These helpers are, as a whole, full barriers. Within the helper,
* the leading barrier is explicit and the trailing barrier is within
* cmpxchg primitive.
+ *
+ * Trace this load + RMW loop as a single RMW op. This way, regardless
+ * of CF_PARALLEL's value, we'll trace just a read and a write.
*/
#define GEN_ATOMIC_HELPER_FN(X, FN, XDATA_TYPE, RET) \
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
@@ -134,6 +180,8 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ATOMIC_MMU_DECLS; \
XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \
XDATA_TYPE cmp, old, new, val = xval; \
+ \
+ ATOMIC_TRACE_RMW; \
smp_mb(); \
cmp = atomic_read__nocheck(haddr); \
do { \
@@ -158,6 +206,7 @@ GEN_ATOMIC_HELPER_FN(umax_fetch, MAX, DATA_TYPE, new)
#endif /* DATA SIZE >= 16 */
#undef END
+#undef MEND
#if DATA_SIZE > 1
@@ -165,8 +214,10 @@ GEN_ATOMIC_HELPER_FN(umax_fetch, MAX, DATA_TYPE, new)
within the ATOMIC_NAME macro. */
#ifdef HOST_WORDS_BIGENDIAN
# define END _le
+# define MEND _le
#else
# define END _be
+# define MEND _be
#endif
ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
@@ -174,7 +225,10 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
- DATA_TYPE ret = atomic_cmpxchg__nocheck(haddr, BSWAP(cmpv), BSWAP(newv));
+ DATA_TYPE ret;
+
+ ATOMIC_TRACE_RMW;
+ ret = atomic_cmpxchg__nocheck(haddr, BSWAP(cmpv), BSWAP(newv));
ATOMIC_MMU_CLEANUP;
return BSWAP(ret);
}
@@ -184,6 +238,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
{
ATOMIC_MMU_DECLS;
DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP;
+
+ ATOMIC_TRACE_LD;
__atomic_load(haddr, &val, __ATOMIC_RELAXED);
ATOMIC_MMU_CLEANUP;
return BSWAP(val);
@@ -194,6 +250,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
+
+ ATOMIC_TRACE_ST;
val = BSWAP(val);
__atomic_store(haddr, &val, __ATOMIC_RELAXED);
ATOMIC_MMU_CLEANUP;
@@ -204,7 +262,10 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
{
ATOMIC_MMU_DECLS;
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
- ABI_TYPE ret = atomic_xchg__nocheck(haddr, BSWAP(val));
+ ABI_TYPE ret;
+
+ ATOMIC_TRACE_RMW;
+ ret = atomic_xchg__nocheck(haddr, BSWAP(val));
ATOMIC_MMU_CLEANUP;
return BSWAP(ret);
}
@@ -215,7 +276,10 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
{ \
ATOMIC_MMU_DECLS; \
DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \
- DATA_TYPE ret = atomic_##X(haddr, BSWAP(val)); \
+ DATA_TYPE ret; \
+ \
+ ATOMIC_TRACE_RMW; \
+ ret = atomic_##X(haddr, BSWAP(val)); \
ATOMIC_MMU_CLEANUP; \
return BSWAP(ret); \
}
@@ -232,6 +296,9 @@ GEN_ATOMIC_HELPER(xor_fetch)
/* These helpers are, as a whole, full barriers. Within the helper,
* the leading barrier is explicit and the trailing barrier is within
* cmpxchg primitive.
+ *
+ * Trace this load + RMW loop as a single RMW op. This way, regardless
+ * of CF_PARALLEL's value, we'll trace just a read and a write.
*/
#define GEN_ATOMIC_HELPER_FN(X, FN, XDATA_TYPE, RET) \
ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
@@ -240,6 +307,8 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \
ATOMIC_MMU_DECLS; \
XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \
XDATA_TYPE ldo, ldn, old, new, val = xval; \
+ \
+ ATOMIC_TRACE_RMW; \
smp_mb(); \
ldn = atomic_read__nocheck(haddr); \
do { \
@@ -271,11 +340,17 @@ GEN_ATOMIC_HELPER_FN(add_fetch, ADD, DATA_TYPE, new)
#endif /* DATA_SIZE >= 16 */
#undef END
+#undef MEND
#endif /* DATA_SIZE > 1 */
+#undef ATOMIC_TRACE_ST
+#undef ATOMIC_TRACE_LD
+#undef ATOMIC_TRACE_RMW
+
#undef BSWAP
#undef ABI_TYPE
#undef DATA_TYPE
#undef SDATA_TYPE
#undef SUFFIX
#undef DATA_SIZE
+#undef SHIFT
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 6/6] trace: forbid floating point types
2018-06-27 12:58 [Qemu-devel] [PULL 0/6] Tracing patches Stefan Hajnoczi
` (4 preceding siblings ...)
2018-06-27 12:58 ` [Qemu-devel] [PULL 5/6] trace: enable tracing of TCG atomics Stefan Hajnoczi
@ 2018-06-27 12:58 ` Stefan Hajnoczi
2018-06-28 13:26 ` [Qemu-devel] [PULL 0/6] Tracing patches Peter Maydell
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2018-06-27 12:58 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Dr. David Alan Gilbert, Richard Henderson,
Peter Crosthwaite, Stefan Hajnoczi, Juan Quintela,
Markus Armbruster, Michael Roth, Peter Maydell,
Daniel P . Berrangé
Only one existing trace event uses a floating point type. Unfortunately
float and double cannot be supported since SystemTap does not have
floating point types.
Remove float and double from the whitelist and document this limitation.
Update the migrate_transferred trace event to use uint64_t instead of
double.
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-id: 20180621150254.4922-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
docs/devel/tracing.txt | 5 +++++
migration/trace-events | 2 +-
qapi/trace-events | 2 +-
scripts/tracetool/__init__.py | 2 --
4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
index 07abbb345c..6f815ecbd7 100644
--- a/docs/devel/tracing.txt
+++ b/docs/devel/tracing.txt
@@ -104,6 +104,11 @@ Trace events should use types as follows:
* For everything else, use primitive scalar types (char, int, long) with the
appropriate signedness.
+ * Avoid floating point types (float and double) because SystemTap does not
+ support them. In most cases it is possible to round to an integer type
+ instead. This may require scaling the value first by multiplying it by 1000
+ or the like when digits after the decimal point need to be preserved.
+
Format strings should reflect the types defined in the trace event. Take
special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
respectively. This ensures portability between 32- and 64-bit platforms.
diff --git a/migration/trace-events b/migration/trace-events
index 3f67758893..7ea522e453 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -133,7 +133,7 @@ migrate_global_state_post_load(const char *state) "loaded state: %s"
migrate_global_state_pre_save(const char *state) "saved state: %s"
migration_thread_low_pending(uint64_t pending) "%" PRIu64
migrate_state_too_big(void) ""
-migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %g max_size %" PRId64
+migrate_transferred(uint64_t tranferred, uint64_t time_spent, uint64_t bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %" PRIu64 " max_size %" PRId64
process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
process_incoming_migration_co_postcopy_end_main(void) ""
migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
diff --git a/qapi/trace-events b/qapi/trace-events
index 9e9008a1dc..70e049ea80 100644
--- a/qapi/trace-events
+++ b/qapi/trace-events
@@ -29,6 +29,6 @@ visit_type_int64(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
visit_type_size(void *v, const char *name, uint64_t *obj) "v=%p name=%s obj=%p"
visit_type_bool(void *v, const char *name, bool *obj) "v=%p name=%s obj=%p"
visit_type_str(void *v, const char *name, char **obj) "v=%p name=%s obj=%p"
-visit_type_number(void *v, const char *name, double *obj) "v=%p name=%s obj=%p"
+visit_type_number(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
visit_type_any(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
visit_type_null(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index b20fac34a3..0e3c9e146c 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -53,8 +53,6 @@ ALLOWED_TYPES = [
"bool",
"unsigned",
"signed",
- "float",
- "double",
"int8_t",
"uint8_t",
"int16_t",
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PULL 0/6] Tracing patches
2018-06-27 12:58 [Qemu-devel] [PULL 0/6] Tracing patches Stefan Hajnoczi
` (5 preceding siblings ...)
2018-06-27 12:58 ` [Qemu-devel] [PULL 6/6] trace: forbid floating point types Stefan Hajnoczi
@ 2018-06-28 13:26 ` Peter Maydell
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2018-06-28 13:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Paolo Bonzini, Dr. David Alan Gilbert,
Richard Henderson, Peter Crosthwaite, Juan Quintela,
Markus Armbruster, Michael Roth
On 27 June 2018 at 13:58, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 00928a421d47f49691cace1207481b7aad31b1f1:
>
> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180626' into staging (2018-06-26 18:23:49 +0100)
>
> are available in the Git repository at:
>
> git://github.com/stefanha/qemu.git tags/tracing-pull-request
>
> for you to fetch changes up to ec09f877532210e28e1d4b6b12896d3eb6d8e8d1:
>
> trace: forbid floating point types (2018-06-27 11:09:29 +0100)
>
> ----------------------------------------------------------------
> Pull request
>
> * Trace TCG atomic memory accesses
> * Document that trace event arguments cannot be floating point
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread