linux-kernel - Re: [PATCH v2 4/6] perf cs-etm: Treat NO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181210225302.GC12152@xps15>
Date:   Mon, 10 Dec 2018 15:53:02 -0700
From:   Mathieu Poirier <mathieu.poirier@...aro.org>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Coresight ML <coresight@...ts.linaro.org>,
        Mike Leach <mike.leach@...aro.org>,
        Robert Walker <robert.walker@....com>
Subject: Re: [PATCH v2 4/6] perf cs-etm: Treat NO_SYNC element as trace
 discontinuity

On Mon, Dec 10, 2018 at 04:52:59PM +0800, Leo Yan wrote:
> CoreSight tracer driver might insert barrier packet between different
> buffers, thus the decoder can spot the boundaries based on the barrier
> packet; the decoder is possible to hit a barrier packet and emit a
> NO_SYNC element, then the decoder will find a periodic synchronisation
> point inside that next trace block that starts trace again but does not
> have the TRACE_ON element as indicator - usually because this block of
> trace has wrapped the buffer so we have lost the original point that
> trace was enabled.
> 
> In upper case, it results in the trace stream only inserts the
> OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but
> we don't handle NO_SYNC element properly and at the end users miss to
> see the info for trace discontinuity.
> 
> Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
> output from the decoder, but both of them indicate the trace data is
> discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace
> discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so
> cs-etm can handle discontinuity for this case, finally it saves the last
> trace data for previous trace block and restart samples for new block.
> 
> Credit to Mike Leach and Robert Walker who made me clear for underlying
> mechanism for NO_SYNC element, Mike also shared with me for detailed
> explanation for why we can treat NO_SYNC and TRACE_ON elements as the
> same, so this commit log reused most of his explanation.

Credit and thank you note such as this one go in the cover letter.  With this
change:

Reviewed-by: Mathieu Poirier <mathieu.poirier@...aro.org>

> 
> Cc: Mathieu Poirier <mathieu.poirier@...aro.org>
> Cc: Mike Leach <mike.leach@...aro.org>
> Cc: Robert Walker <robert.walker@....com>
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
> ---
>  tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> 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 a3994f1..46b67f1 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> @@ -411,6 +411,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
>  	case OCSD_GEN_TRC_ELEM_UNKNOWN:
>  		break;
>  	case OCSD_GEN_TRC_ELEM_NO_SYNC:
> +		resp = cs_etm_decoder__buffer_discontinuity(decoder,
> +							    trace_chan_id);
>  		decoder->trace_on = false;
>  		break;
>  	case OCSD_GEN_TRC_ELEM_TRACE_ON:
> -- 
> 2.7.4
>