On 13 June 2017 at 02:55, Mike Leach <mike.leach@linaro.org> wrote:

Adds a compile-time option to print out the raw trace data bytes
when dumping the trace packets using the perf report --dump.

Unpacked raw trace data is output by default, but packed trace frame
output can be added.

Controlled by environment var "CSTRACE_RAW", checked in Makefile.config

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
tools/perf/Makefile.config | 6 ++++
tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 47 +++++++++++++++++++++++--
2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 432e25f..6a53e4b 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -586,6 +586,12 @@ ifdef CSTRACE_PATH
$(call detected,CSTRACE)
$(call detected_var,CSTRACE_PATH)
EXTLIBS += -L$(CSTRACE_LIB_PATH) $(LIBCSTRACE) -lstdc++
+ ifdef CSTRACE_RAW
+ CFLAGS += -DCS_DEBUG_RAW
+ ifeq (${CSTRACE_RAW}, packed)
+ CFLAGS += -DCS_RAW_PACKED
+ endif
+ endif
endif

ifdef NO_LIBPERL
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index bbe6923..d5d39c0 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -31,6 +31,16 @@

#define MAX_BUFFER 1024

+/* use raw logging */
+#ifdef CS_DEBUG_RAW
+#define CS_LOG_RAW_FRAMES
+#ifdef CS_RAW_PACKED
+#define CS_RAW_DEBUG_FLAGS (OCSD_DFRMTR_UNPACKED_RAW_OUT | OCSD_DFRMTR_PACKED_RAW_OUT)
+#else
+#define CS_RAW_DEBUG_FLAGS (OCSD_DFRMTR_UNPACKED_RAW_OUT)
+#endif
+#endif
+

The above two hunks won't be pleasant to upstream... Do we absolutely need packed and unpacked mode?

struct cs_etm_decoder;

struct cs_etm_channel {
@@ -269,8 +279,7 @@ static ocsd_datapath_resp_t cs_etm_decoder__etmv4i_packet_printer(
channel->cs_id);
break;
case OCSD_OP_RESET:
- sprintf(packet_str, "**** RESET DECODER id[%02X] ****",
- channel->cs_id);
+ sprintf(packet_str+offset, "**** RESET DECODER ****");

Code added in an earlier patch and deleted in a subsequent one is tolerated only when it shows a progression and ease review. Here the changes are small enough that is it now required - please put this in the previous patch.

break;
default:
break;
@@ -281,6 +290,16 @@ static ocsd_datapath_resp_t cs_etm_decoder__etmv4i_packet_printer(
return ret;
}

+#ifdef CS_LOG_RAW_FRAMES
+static void cs_etm_decoder__print_str_cb(const void *p_context,
+ const char *psz_msg_str,
+ const int str_len)
+{
+ if (p_context && str_len)
+ ((struct cs_etm_decoder *)p_context)->packet_printer(psz_msg_str);
+}
+#endif
+
static struct cs_etm_channel *cs_etm_decoder__create_channel_item(
struct cs_etm_decoder *decoder, unsigned char cs_id)
{
@@ -530,6 +549,30 @@ struct cs_etm_decoder *cs_etm_decoder__new(uint32_t num_cpu, struct cs_etm_decod
/* Create decode tree for the data source */
decoder->dcd_tree = ocsd_create_dcd_tree(format,flags);

+#ifdef CS_LOG_RAW_FRAMES
+ /* Only log these during a --dump operation */
+ if (d_params->operation == CS_ETM_OPERATION_PRINT) {
+ /* set up a library default logger to process the
+ * raw frame printer we add later
+ */
+ ocsd_def_errlog_init(OCSD_ERR_SEV_ERROR, 1);
+
+ /* no stdout / err / file output */
+ ocsd_def_errlog_config_output(C_API_MSGLOGOUT_FLG_NONE, NULL);
+
+ /* set the string CB for the default logger,
+ * passes strings to perf print logger.
+ */
+ ocsd_def_errlog_set_strprint_cb(decoder->dcd_tree,
+ (void *)decoder,
+ cs_etm_decoder__print_str_cb);
+
+ /* use the built in library printer for the raw frames */
+ ocsd_dt_set_raw_frame_printer(decoder->dcd_tree,
+ CS_RAW_DEBUG_FLAGS);
+ }
+#endif
+

The goal is to reduce the amount of #if/#endif in the code. Take this hunk, create a function with it and move it to the previous hunk. Then create an empty function with the same name and put it in and #else condition. That way said function can be called in cs_etm_decoder__new() without adding conditional defines.

Thanks for the code,

Mathieu

if (decoder->dcd_tree == 0) {
goto err_free_decoder;
}
--
2.7.4