This patch set is to introduce a new itrace option 'T' for forcily use
timestamp trace for kernel time and support this option in cs-etm.
Some Arm platforms (either Arm CPUs prior to Armv8 or miss the FEAT_TRF
feature, currently the ETM driver cannot decide if the timestamp trace
is same with kernel time. This is why we introduce the itrace option
'T', we leave decision to users so users can specify this option to
forcily use the timestamp trace as the kernel time.
This patch set is tested on Arm Juno board.
Leo Yan (2):
perf auxtrace: Add 'T' itrace option for timestamp trace
perf cs-etm: Enable itrace option 'T'
tools/perf/Documentation/itrace.txt | 1 +
tools/perf/util/auxtrace.c | 3 +++
tools/perf/util/auxtrace.h | 3 +++
tools/perf/util/cs-etm.c | 21 ++++++++++++++++++---
4 files changed, 25 insertions(+), 3 deletions(-)
--
2.34.1
Prior to Armv8.4, the feature FEAT_TRF is not supported by Arm CPUs.
Consequently, the sysfs node 'ts_source' will not be set as 1 by the
CoreSight ETM driver. On the other hand, the perf tool relies on the
'ts_source' node to determine whether the kernel timestamp is traced.
Since the 'ts_source' is not set for Arm CPUs prior to Armv8.4,
platforms in this case cannot utilize the traced timestamp as the kernel
time.
This patch enables the 'T' itrace option, which forcibly utilizes the
traced timestamp as the kernel time. If users are aware that their
working platform's Arm CoreSight shares the same counter with the kernel
time, they can specify 'T' option to decode the traced timestamp as the
kernel time.
An usage example is:
# perf record -e cs_etm// -- test_program
# perf script --itrace=i10ibT
# perf report --itrace=i10ibT
Signed-off-by: Leo Yan <[email protected]>
---
tools/perf/util/cs-etm.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 9729d006550d..4a37fdeb1795 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -3322,12 +3322,27 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
etm->metadata = metadata;
etm->auxtrace_type = auxtrace_info->type;
- /* Use virtual timestamps if all ETMs report ts_source = 1 */
- etm->has_virtual_ts = cs_etm__has_virtual_ts(metadata, num_cpu);
+ if (etm->synth_opts.use_timestamp)
+ /*
+ * Prior to Armv8.4, Arm CPUs don't support FEAT_TRF feature,
+ * therefore the decoder cannot know if the timestamp trace is
+ * same with the kernel time.
+ *
+ * If a user has knowledge for the working platform and can
+ * specify itrace option 'T' to tell decoder to forcely use the
+ * traced timestamp as the kernel time.
+ */
+ etm->has_virtual_ts = true;
+ else
+ /* Use virtual timestamps if all ETMs report ts_source = 1 */
+ etm->has_virtual_ts = cs_etm__has_virtual_ts(metadata, num_cpu);
if (!etm->has_virtual_ts)
ui__warning("Virtual timestamps are not enabled, or not supported by the traced system.\n"
- "The time field of the samples will not be set accurately.\n\n");
+ "The time field of the samples will not be set accurately.\n"
+ "For Arm CPUs prior to Armv8.4 or without support FEAT_TRF,\n"
+ "you can specify the itrace option 'T' for timestamp decoding\n"
+ "if the Coresight timestamp on the platform is same with the kernel time.\n\n");
etm->auxtrace.process_event = cs_etm__process_event;
etm->auxtrace.process_auxtrace_event = cs_etm__process_auxtrace_event;
--
2.34.1
An AUX trace can contain timestamp, but in some situations, the hardware
trace module (e.g. Arm CoreSight) cannot decide the traced timestamp is
the same source with CPU's time, thus the decoder can not use the
timestamp trace for samples.
This patch introduces 'T' itrace option. If users know the platforms
they are working on have the same time counter with CPUs, users can
use this new option to tell a decoder for using timestamp trace as
kernel time.
Signed-off-by: Leo Yan <[email protected]>
---
tools/perf/Documentation/itrace.txt | 1 +
tools/perf/util/auxtrace.c | 3 +++
tools/perf/util/auxtrace.h | 3 +++
3 files changed, 7 insertions(+)
diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt
index a97f95825b14..19cc179be9a7 100644
--- a/tools/perf/Documentation/itrace.txt
+++ b/tools/perf/Documentation/itrace.txt
@@ -25,6 +25,7 @@
q quicker (less detailed) decoding
A approximate IPC
Z prefer to ignore timestamps (so-called "timeless" decoding)
+ T use the timestamp trace as kernel time
The default is all events i.e. the same as --itrace=iybxwpe,
except for perf script where it is --itrace=ce
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index a0368202a746..f528c4364d23 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -1638,6 +1638,9 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
case 'Z':
synth_opts->timeless_decoding = true;
break;
+ case 'T':
+ synth_opts->use_timestamp = true;
+ break;
case ' ':
case ',':
break;
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 29eb82dff574..55702215a82d 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -99,6 +99,7 @@ enum itrace_period_type {
* @remote_access: whether to synthesize remote access events
* @mem: whether to synthesize memory events
* @timeless_decoding: prefer "timeless" decoding i.e. ignore timestamps
+ * @use_timestamp: use the timestamp trace as kernel time
* @vm_time_correlation: perform VM Time Correlation
* @vm_tm_corr_dry_run: VM Time Correlation dry-run
* @vm_tm_corr_args: VM Time Correlation implementation-specific arguments
@@ -146,6 +147,7 @@ struct itrace_synth_opts {
bool remote_access;
bool mem;
bool timeless_decoding;
+ bool use_timestamp;
bool vm_time_correlation;
bool vm_tm_corr_dry_run;
char *vm_tm_corr_args;
@@ -678,6 +680,7 @@ bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
" q: quicker (less detailed) decoding\n" \
" A: approximate IPC\n" \
" Z: prefer to ignore timestamps (so-called \"timeless\" decoding)\n" \
+" T: use the timestamp trace as kernel time\n" \
" PERIOD[ns|us|ms|i|t]: specify period to sample stream\n" \
" concatenate multiple options. Default is iybxwpe or cewp\n"
--
2.34.1
On 14/10/2023 08:45, Leo Yan wrote:
> An AUX trace can contain timestamp, but in some situations, the hardware
> trace module (e.g. Arm CoreSight) cannot decide the traced timestamp is
> the same source with CPU's time, thus the decoder can not use the
> timestamp trace for samples.
>
> This patch introduces 'T' itrace option. If users know the platforms
> they are working on have the same time counter with CPUs, users can
> use this new option to tell a decoder for using timestamp trace as
> kernel time.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> tools/perf/Documentation/itrace.txt | 1 +
> tools/perf/util/auxtrace.c | 3 +++
> tools/perf/util/auxtrace.h | 3 +++
> 3 files changed, 7 insertions(+)
>
> diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt
> index a97f95825b14..19cc179be9a7 100644
> --- a/tools/perf/Documentation/itrace.txt
> +++ b/tools/perf/Documentation/itrace.txt
> @@ -25,6 +25,7 @@
> q quicker (less detailed) decoding
> A approximate IPC
> Z prefer to ignore timestamps (so-called "timeless" decoding)
> + T use the timestamp trace as kernel time
>
Maybe "Treat hardware timestamps as kernel time (trace and CPU time use
same clock source)" would be clearer.
And another point, although this isn't really related to this patch, but
why do we have the single letter arguments for itrace? It seems like it
massively restricts the available options and makes the command lines
hard to read because they don't have long forms. Why not just have them
as normal arguments?
If it's a backwards compatibility thing, would there be any objection to
adding this new option as a normal one rather than an itrace one?
> The default is all events i.e. the same as --itrace=iybxwpe,
> except for perf script where it is --itrace=ce
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index a0368202a746..f528c4364d23 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -1638,6 +1638,9 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
> case 'Z':
> synth_opts->timeless_decoding = true;
> break;
> + case 'T':
> + synth_opts->use_timestamp = true;
> + break;
> case ' ':
> case ',':
> break;
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 29eb82dff574..55702215a82d 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -99,6 +99,7 @@ enum itrace_period_type {
> * @remote_access: whether to synthesize remote access events
> * @mem: whether to synthesize memory events
> * @timeless_decoding: prefer "timeless" decoding i.e. ignore timestamps
> + * @use_timestamp: use the timestamp trace as kernel time
> * @vm_time_correlation: perform VM Time Correlation
> * @vm_tm_corr_dry_run: VM Time Correlation dry-run
> * @vm_tm_corr_args: VM Time Correlation implementation-specific arguments
> @@ -146,6 +147,7 @@ struct itrace_synth_opts {
> bool remote_access;
> bool mem;
> bool timeless_decoding;
> + bool use_timestamp;
And then this one could be like "hw_time_is_kernel_time", but I'm
stuggling to think of something shorter. Maybe your one is fine along
with the comment.
On 14/10/2023 08:45, Leo Yan wrote:
> Prior to Armv8.4, the feature FEAT_TRF is not supported by Arm CPUs.
> Consequently, the sysfs node 'ts_source' will not be set as 1 by the
> CoreSight ETM driver. On the other hand, the perf tool relies on the
> 'ts_source' node to determine whether the kernel timestamp is traced.
> Since the 'ts_source' is not set for Arm CPUs prior to Armv8.4,
> platforms in this case cannot utilize the traced timestamp as the kernel
> time.
>
> This patch enables the 'T' itrace option, which forcibly utilizes the
> traced timestamp as the kernel time. If users are aware that their
> working platform's Arm CoreSight shares the same counter with the kernel
> time, they can specify 'T' option to decode the traced timestamp as the
> kernel time.
>
> An usage example is:
>
> # perf record -e cs_etm// -- test_program
> # perf script --itrace=i10ibT
> # perf report --itrace=i10ibT
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> tools/perf/util/cs-etm.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 9729d006550d..4a37fdeb1795 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -3322,12 +3322,27 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
> etm->metadata = metadata;
> etm->auxtrace_type = auxtrace_info->type;
>
> - /* Use virtual timestamps if all ETMs report ts_source = 1 */
> - etm->has_virtual_ts = cs_etm__has_virtual_ts(metadata, num_cpu);
> + if (etm->synth_opts.use_timestamp)
> + /*
> + * Prior to Armv8.4, Arm CPUs don't support FEAT_TRF feature,
> + * therefore the decoder cannot know if the timestamp trace is
> + * same with the kernel time.
> + *
> + * If a user has knowledge for the working platform and can
> + * specify itrace option 'T' to tell decoder to forcely use the
> + * traced timestamp as the kernel time.
> + */
> + etm->has_virtual_ts = true;
> + else
> + /* Use virtual timestamps if all ETMs report ts_source = 1 */
> + etm->has_virtual_ts = cs_etm__has_virtual_ts(metadata, num_cpu);
>
> if (!etm->has_virtual_ts)
> ui__warning("Virtual timestamps are not enabled, or not supported by the traced system.\n"
> - "The time field of the samples will not be set accurately.\n\n");
> + "The time field of the samples will not be set accurately.\n"
> + "For Arm CPUs prior to Armv8.4 or without support FEAT_TRF,\n"
> + "you can specify the itrace option 'T' for timestamp decoding\n"
> + "if the Coresight timestamp on the platform is same with the kernel time.\n\n");
>
> etm->auxtrace.process_event = cs_etm__process_event;
> etm->auxtrace.process_auxtrace_event = cs_etm__process_auxtrace_event;
Reviewed-by: James Clark <[email protected]>
On 14/10/23 10:45, Leo Yan wrote:
> An AUX trace can contain timestamp, but in some situations, the hardware
> trace module (e.g. Arm CoreSight) cannot decide the traced timestamp is
> the same source with CPU's time, thus the decoder can not use the
> timestamp trace for samples.
>
> This patch introduces 'T' itrace option. If users know the platforms
"If users know" <- how would users know? Could the kernel
or tools also figure it out?
> they are working on have the same time counter with CPUs, users can
> use this new option to tell a decoder for using timestamp trace as
> kernel time.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> tools/perf/Documentation/itrace.txt | 1 +
> tools/perf/util/auxtrace.c | 3 +++
> tools/perf/util/auxtrace.h | 3 +++
> 3 files changed, 7 insertions(+)
>
> diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt
> index a97f95825b14..19cc179be9a7 100644
> --- a/tools/perf/Documentation/itrace.txt
> +++ b/tools/perf/Documentation/itrace.txt
> @@ -25,6 +25,7 @@
> q quicker (less detailed) decoding
> A approximate IPC
> Z prefer to ignore timestamps (so-called "timeless" decoding)
> + T use the timestamp trace as kernel time
>
> The default is all events i.e. the same as --itrace=iybxwpe,
> except for perf script where it is --itrace=ce
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index a0368202a746..f528c4364d23 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -1638,6 +1638,9 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
> case 'Z':
> synth_opts->timeless_decoding = true;
> break;
> + case 'T':
> + synth_opts->use_timestamp = true;
> + break;
> case ' ':
> case ',':
> break;
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 29eb82dff574..55702215a82d 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -99,6 +99,7 @@ enum itrace_period_type {
> * @remote_access: whether to synthesize remote access events
> * @mem: whether to synthesize memory events
> * @timeless_decoding: prefer "timeless" decoding i.e. ignore timestamps
> + * @use_timestamp: use the timestamp trace as kernel time
> * @vm_time_correlation: perform VM Time Correlation
> * @vm_tm_corr_dry_run: VM Time Correlation dry-run
> * @vm_tm_corr_args: VM Time Correlation implementation-specific arguments
> @@ -146,6 +147,7 @@ struct itrace_synth_opts {
> bool remote_access;
> bool mem;
> bool timeless_decoding;
> + bool use_timestamp;
> bool vm_time_correlation;
> bool vm_tm_corr_dry_run;
> char *vm_tm_corr_args;
> @@ -678,6 +680,7 @@ bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
> " q: quicker (less detailed) decoding\n" \
> " A: approximate IPC\n" \
> " Z: prefer to ignore timestamps (so-called \"timeless\" decoding)\n" \
> +" T: use the timestamp trace as kernel time\n" \
> " PERIOD[ns|us|ms|i|t]: specify period to sample stream\n" \
> " concatenate multiple options. Default is iybxwpe or cewp\n"
>
On 19/10/2023 11:47, Adrian Hunter wrote:
> On 14/10/23 10:45, Leo Yan wrote:
>> An AUX trace can contain timestamp, but in some situations, the hardware
>> trace module (e.g. Arm CoreSight) cannot decide the traced timestamp is
>> the same source with CPU's time, thus the decoder can not use the
>> timestamp trace for samples.
>>
>> This patch introduces 'T' itrace option. If users know the platforms
>
> "If users know" <- how would users know? Could the kernel
> or tools also figure it out?
>
The problem is this was only made into a discoverable feature with
Feat_TRF in Armv8.4. So this workaround is to support devices that
already had the right kind of timestamps before that feature.
It might be possible to list every device in the driver, but maybe there
could even be some firmware that disconnects the clock source on a
device that we thought would have it.
IMO adding this option is the best way to workaround it.
>> they are working on have the same time counter with CPUs, users can
>> use this new option to tell a decoder for using timestamp trace as
>> kernel time.
>>
>> Signed-off-by: Leo Yan <[email protected]>
>> ---
>> tools/perf/Documentation/itrace.txt | 1 +
>> tools/perf/util/auxtrace.c | 3 +++
>> tools/perf/util/auxtrace.h | 3 +++
>> 3 files changed, 7 insertions(+)
>>
>> diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt
>> index a97f95825b14..19cc179be9a7 100644
>> --- a/tools/perf/Documentation/itrace.txt
>> +++ b/tools/perf/Documentation/itrace.txt
>> @@ -25,6 +25,7 @@
>> q quicker (less detailed) decoding
>> A approximate IPC
>> Z prefer to ignore timestamps (so-called "timeless" decoding)
>> + T use the timestamp trace as kernel time
>>
>> The default is all events i.e. the same as --itrace=iybxwpe,
>> except for perf script where it is --itrace=ce
>> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
>> index a0368202a746..f528c4364d23 100644
>> --- a/tools/perf/util/auxtrace.c
>> +++ b/tools/perf/util/auxtrace.c
>> @@ -1638,6 +1638,9 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
>> case 'Z':
>> synth_opts->timeless_decoding = true;
>> break;
>> + case 'T':
>> + synth_opts->use_timestamp = true;
>> + break;
>> case ' ':
>> case ',':
>> break;
>> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
>> index 29eb82dff574..55702215a82d 100644
>> --- a/tools/perf/util/auxtrace.h
>> +++ b/tools/perf/util/auxtrace.h
>> @@ -99,6 +99,7 @@ enum itrace_period_type {
>> * @remote_access: whether to synthesize remote access events
>> * @mem: whether to synthesize memory events
>> * @timeless_decoding: prefer "timeless" decoding i.e. ignore timestamps
>> + * @use_timestamp: use the timestamp trace as kernel time
>> * @vm_time_correlation: perform VM Time Correlation
>> * @vm_tm_corr_dry_run: VM Time Correlation dry-run
>> * @vm_tm_corr_args: VM Time Correlation implementation-specific arguments
>> @@ -146,6 +147,7 @@ struct itrace_synth_opts {
>> bool remote_access;
>> bool mem;
>> bool timeless_decoding;
>> + bool use_timestamp;
>> bool vm_time_correlation;
>> bool vm_tm_corr_dry_run;
>> char *vm_tm_corr_args;
>> @@ -678,6 +680,7 @@ bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
>> " q: quicker (less detailed) decoding\n" \
>> " A: approximate IPC\n" \
>> " Z: prefer to ignore timestamps (so-called \"timeless\" decoding)\n" \
>> +" T: use the timestamp trace as kernel time\n" \
>> " PERIOD[ns|us|ms|i|t]: specify period to sample stream\n" \
>> " concatenate multiple options. Default is iybxwpe or cewp\n"
>>
>
>
On Thu, Oct 19, 2023 at 11:31:16AM +0100, James Clark wrote:
[...]
> > --- a/tools/perf/Documentation/itrace.txt
> > +++ b/tools/perf/Documentation/itrace.txt
> > @@ -25,6 +25,7 @@
> > q quicker (less detailed) decoding
> > A approximate IPC
> > Z prefer to ignore timestamps (so-called "timeless" decoding)
> > + T use the timestamp trace as kernel time
> >
>
> Maybe "Treat hardware timestamps as kernel time (trace and CPU time use
> same clock source)" would be clearer.
Agreed.
> And another point, although this isn't really related to this patch, but
> why do we have the single letter arguments for itrace? It seems like it
> massively restricts the available options and makes the command lines
> hard to read because they don't have long forms. Why not just have them
> as normal arguments?
TBH, I tried a bit for adding a normal argument, but I found it's not
nature like itrace option for passing the normal argument into the
decoder. And if we add a normal argument, we need to consider adding
into perf commands for 'perf report', 'perf script', etc; itrace options
can be shared by perf output commands.
I understand an advantage of using normal argument is the 'perf report'
command (e.g. perf report --aux-trace-kernel-time) doesn't need to
specify any extra itrace option.
If anyone still has concern for adding itrace optiona and prefer to add
normal argument, I am happy to rework for adding normal argument.
> If it's a backwards compatibility thing, would there be any objection to
> adding this new option as a normal one rather than an itrace one?
backward compatibility would not a problem - we add a new feature and
at meantime we don't break anything.
> > The default is all events i.e. the same as --itrace=iybxwpe,
> > except for perf script where it is --itrace=ce
> > diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> > index a0368202a746..f528c4364d23 100644
> > --- a/tools/perf/util/auxtrace.c
> > +++ b/tools/perf/util/auxtrace.c
> > @@ -1638,6 +1638,9 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
> > case 'Z':
> > synth_opts->timeless_decoding = true;
> > break;
> > + case 'T':
> > + synth_opts->use_timestamp = true;
> > + break;
> > case ' ':
> > case ',':
> > break;
> > diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> > index 29eb82dff574..55702215a82d 100644
> > --- a/tools/perf/util/auxtrace.h
> > +++ b/tools/perf/util/auxtrace.h
> > @@ -99,6 +99,7 @@ enum itrace_period_type {
> > * @remote_access: whether to synthesize remote access events
> > * @mem: whether to synthesize memory events
> > * @timeless_decoding: prefer "timeless" decoding i.e. ignore timestamps
> > + * @use_timestamp: use the timestamp trace as kernel time
> > * @vm_time_correlation: perform VM Time Correlation
> > * @vm_tm_corr_dry_run: VM Time Correlation dry-run
> > * @vm_tm_corr_args: VM Time Correlation implementation-specific arguments
> > @@ -146,6 +147,7 @@ struct itrace_synth_opts {
> > bool remote_access;
> > bool mem;
> > bool timeless_decoding;
> > + bool use_timestamp;
>
> And then this one could be like "hw_time_is_kernel_time", but I'm
> stuggling to think of something shorter. Maybe your one is fine along
> with the comment.
It's fine for me to rename as "hw_time_is_kernel_time" for more
readable. Will do in next spin.
Thanks,
Leo
Em Thu, Oct 19, 2023 at 01:47:15PM +0300, Adrian Hunter escreveu:
> On 14/10/23 10:45, Leo Yan wrote:
> > An AUX trace can contain timestamp, but in some situations, the hardware
> > trace module (e.g. Arm CoreSight) cannot decide the traced timestamp is
> > the same source with CPU's time, thus the decoder can not use the
> > timestamp trace for samples.
> >
> > This patch introduces 'T' itrace option. If users know the platforms
>
> "If users know" <- how would users know? Could the kernel
> or tools also figure it out?
Adrian, I'm trying to go all the outstanding patches, do you still have
any issues with this series?
- Arnaldo
> > they are working on have the same time counter with CPUs, users can
> > use this new option to tell a decoder for using timestamp trace as
> > kernel time.
> >
> > Signed-off-by: Leo Yan <[email protected]>
> > ---
> > tools/perf/Documentation/itrace.txt | 1 +
> > tools/perf/util/auxtrace.c | 3 +++
> > tools/perf/util/auxtrace.h | 3 +++
> > 3 files changed, 7 insertions(+)
> >
> > diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt
> > index a97f95825b14..19cc179be9a7 100644
> > --- a/tools/perf/Documentation/itrace.txt
> > +++ b/tools/perf/Documentation/itrace.txt
> > @@ -25,6 +25,7 @@
> > q quicker (less detailed) decoding
> > A approximate IPC
> > Z prefer to ignore timestamps (so-called "timeless" decoding)
> > + T use the timestamp trace as kernel time
> >
> > The default is all events i.e. the same as --itrace=iybxwpe,
> > except for perf script where it is --itrace=ce
> > diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> > index a0368202a746..f528c4364d23 100644
> > --- a/tools/perf/util/auxtrace.c
> > +++ b/tools/perf/util/auxtrace.c
> > @@ -1638,6 +1638,9 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
> > case 'Z':
> > synth_opts->timeless_decoding = true;
> > break;
> > + case 'T':
> > + synth_opts->use_timestamp = true;
> > + break;
> > case ' ':
> > case ',':
> > break;
> > diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> > index 29eb82dff574..55702215a82d 100644
> > --- a/tools/perf/util/auxtrace.h
> > +++ b/tools/perf/util/auxtrace.h
> > @@ -99,6 +99,7 @@ enum itrace_period_type {
> > * @remote_access: whether to synthesize remote access events
> > * @mem: whether to synthesize memory events
> > * @timeless_decoding: prefer "timeless" decoding i.e. ignore timestamps
> > + * @use_timestamp: use the timestamp trace as kernel time
> > * @vm_time_correlation: perform VM Time Correlation
> > * @vm_tm_corr_dry_run: VM Time Correlation dry-run
> > * @vm_tm_corr_args: VM Time Correlation implementation-specific arguments
> > @@ -146,6 +147,7 @@ struct itrace_synth_opts {
> > bool remote_access;
> > bool mem;
> > bool timeless_decoding;
> > + bool use_timestamp;
> > bool vm_time_correlation;
> > bool vm_tm_corr_dry_run;
> > char *vm_tm_corr_args;
> > @@ -678,6 +680,7 @@ bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
> > " q: quicker (less detailed) decoding\n" \
> > " A: approximate IPC\n" \
> > " Z: prefer to ignore timestamps (so-called \"timeless\" decoding)\n" \
> > +" T: use the timestamp trace as kernel time\n" \
> > " PERIOD[ns|us|ms|i|t]: specify period to sample stream\n" \
> > " concatenate multiple options. Default is iybxwpe or cewp\n"
> >
>
--
- Arnaldo
On 6/11/23 23:52, Arnaldo Carvalho de Melo wrote:
> Em Thu, Oct 19, 2023 at 01:47:15PM +0300, Adrian Hunter escreveu:
>> On 14/10/23 10:45, Leo Yan wrote:
>>> An AUX trace can contain timestamp, but in some situations, the hardware
>>> trace module (e.g. Arm CoreSight) cannot decide the traced timestamp is
>>> the same source with CPU's time, thus the decoder can not use the
>>> timestamp trace for samples.
>>>
>>> This patch introduces 'T' itrace option. If users know the platforms
>>
>> "If users know" <- how would users know? Could the kernel
>> or tools also figure it out?
>
> Adrian, I'm trying to go all the outstanding patches, do you still have
> any issues with this series?
No, although the question wasn't actually answered. I presume users
just have to try the 'T' option and see if it helps.
Em Tue, Nov 07, 2023 at 10:18:08PM +0800, Leo Yan escreveu:
> On Tue, Nov 07, 2023 at 12:16:25PM +0200, Adrian Hunter wrote:
>
> [...]
>
> > >>>> "If users know" <- how would users know? Could the kernel
> > >>>> or tools also figure it out?
> > >>>
> > >>> Adrian, I'm trying to go all the outstanding patches, do you still have
> > >>> any issues with this series?
> > >>
> > >> No, although the question wasn't actually answered. I presume users
> > >> just have to try the 'T' option and see if it helps.
> > >
> > > Sometimes, users are software developers in SoC companies, they can
> > > know well for the hardware design but are confused why current
> > > implementation cannot use timestamp trace. This is the main reason
> > > I sent this patch set.
> > >
> > > An example hardware platform is DB410c [1], we know its CoreSight can
> > > support timestamp trace, but if without this adding option 'T', we
> > > have no chance to use it due to it its CPU arch is prior to Armv8.4.
> >
> > perf config might be better than an itrace option, but you decide.
>
> I understand perf config is a better approach due to users don't need
> to bother inputting options after set it once. I will look at it and
> respin new patch set.
>
> Thanks for suggestion!
Thanks, applied.
- Arnaldo