[PATCH v1 0/2] perf cs-etm: Add support for itrace option 'T'

[PATCH v1 0/2] perf cs-etm: Add support for itrace option 'T'

[Leo Yan]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Leo Yan]

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 <leo.yan@linaro.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[James Clark]

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 <leo.yan@linaro.org>
> ---
>  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.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Leo Yan]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Adrian Hunter]

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 <leo.yan@linaro.org>
> ---
>  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"
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[James Clark]

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 <leo.yan@linaro.org>
>> ---
>>  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"
>>  
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Arnaldo Carvalho de Melo]

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 <leo.yan@linaro.org>
> > ---
> >  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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Adrian Hunter]

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.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[James Clark]

On 07/11/2023 07:19, Adrian Hunter wrote:
> 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.
> 

I suppose my previous answer about discoverability was more general. To 
answer the specific question "how would users know?", it would have to 
be mentioned in the reference manual of their device that this is how 
the timers are set up.

But I don't see this being a common use case, as I mentioned before, in 
newer arm platforms this is discoverable and just works.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Leo Yan]

Hi Adrian,

On Tue, Nov 07, 2023 at 09:19:10AM +0200, Adrian Hunter wrote:
> 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.

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.

@Arnaldo, since James gave comments in his replying, I will respin new
patch set and send out.  Thanks for popping up this patch set!

Leo

[1] https://developer.qualcomm.com/hardware/dragonboard-410c

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Adrian Hunter]

On 7/11/23 11:48, Leo Yan wrote:
> Hi Adrian,
> 
> On Tue, Nov 07, 2023 at 09:19:10AM +0200, Adrian Hunter wrote:
>> 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.
> 
> 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.

> 
> @Arnaldo, since James gave comments in his replying, I will respin new
> patch set and send out.  Thanks for popping up this patch set!
> 
> Leo
> 
> [1] https://developer.qualcomm.com/hardware/dragonboard-410c


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Leo Yan]

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!

Leo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

[Arnaldo Carvalho de Melo]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 2/2] perf cs-etm: Enable itrace option 'T'

[Leo Yan]

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 <leo.yan@linaro.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 2/2] perf cs-etm: Enable itrace option 'T'

[James Clark]

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 <leo.yan@linaro.org>
> ---
>  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 <james.clark@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel