1. Patchew
  2. linux
  3. [PATCH 00/17] coresight: Use per-sink trace ID maps for Perf sessions
  4. View patch

[PATCH 01/17] perf cs-etm: Print error for new PERF_RECORD_AUX_OUTPUT_HW_ID versions

James Clark posted 17 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH 01/17] perf cs-etm: Print error for new PERF_RECORD_AUX_OUTPUT_HW_ID versions
Posted by James Clark 1 year, 7 months ago 
The likely fix for this is to update Perf so print a helpful message.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index d65d7485886c..32818bd7cd17 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -335,8 +335,11 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session,
 	trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);
 
 	/* check that we can handle this version */
-	if (version > CS_AUX_HW_ID_CURR_VERSION)
+	if (version > CS_AUX_HW_ID_CURR_VERSION) {
+		pr_err("CS ETM Trace: PERF_RECORD_AUX_OUTPUT_HW_ID version %d not supported. Please update Perf.\n",
+		       version);
 		return -EINVAL;
+	}
 
 	/* get access to the etm metadata */
 	etm = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace);
-- 
2.34.1
Re: [PATCH 01/17] perf cs-etm: Print error for new PERF_RECORD_AUX_OUTPUT_HW_ID versions
Posted by Anshuman Khandual 1 year, 7 months ago 

On 4/29/24 20:51, James Clark wrote:
> The likely fix for this is to update Perf so print a helpful message.
> 
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/util/cs-etm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index d65d7485886c..32818bd7cd17 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -335,8 +335,11 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session,
>  	trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);
>  
>  	/* check that we can handle this version */
> -	if (version > CS_AUX_HW_ID_CURR_VERSION)
> +	if (version > CS_AUX_HW_ID_CURR_VERSION) {
> +		pr_err("CS ETM Trace: PERF_RECORD_AUX_OUTPUT_HW_ID version %d not supported. Please update Perf.\n",

Is not this bit misleading ? PERF_RECORD_AUX_OUTPUT_HW_ID is just the perf record
format identifier. The record version here, is derived from the platform specific
hardware ID information embedded in this perf record.

Should not this be just s/PERF_RECORD_AUX_OUTPUT_HW_ID/hardware ID/ instead ?

> +		       version);
>  		return -EINVAL;
> +	}
>  
>  	/* get access to the etm metadata */
>  	etm = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace);
Re: [PATCH 01/17] perf cs-etm: Print error for new PERF_RECORD_AUX_OUTPUT_HW_ID versions
Posted by James Clark 1 year, 7 months ago 

On 07/05/2024 04:47, Anshuman Khandual wrote:
> 
> 
> On 4/29/24 20:51, James Clark wrote:
>> The likely fix for this is to update Perf so print a helpful message.
>>
>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>>  tools/perf/util/cs-etm.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>> index d65d7485886c..32818bd7cd17 100644
>> --- a/tools/perf/util/cs-etm.c
>> +++ b/tools/perf/util/cs-etm.c
>> @@ -335,8 +335,11 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session,
>>  	trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);
>>  
>>  	/* check that we can handle this version */
>> -	if (version > CS_AUX_HW_ID_CURR_VERSION)
>> +	if (version > CS_AUX_HW_ID_CURR_VERSION) {
>> +		pr_err("CS ETM Trace: PERF_RECORD_AUX_OUTPUT_HW_ID version %d not supported. Please update Perf.\n",
> 
> Is not this bit misleading ? PERF_RECORD_AUX_OUTPUT_HW_ID is just the perf record
> format identifier. The record version here, is derived from the platform specific
> hardware ID information embedded in this perf record.

Not sure I follow what you mean here. 'version' is something that's
output by the kernel. It's saved into a perf.data file, and if Perf
can't handle version 2 for example, you need to update Perf.

> 
> Should not this be just s/PERF_RECORD_AUX_OUTPUT_HW_ID/hardware ID/ instead ?
> 

It's just a way to go from the error message to the part of the code or
docs that you need to look at. "hardware ID" wouldn't lead you anywhere
so I don't think it would be useful.

>> +		       version);
>>  		return -EINVAL;
>> +	}
>>  
>>  	/* get access to the etm metadata */
>>  	etm = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace);
>
Re: [PATCH 01/17] perf cs-etm: Print error for new PERF_RECORD_AUX_OUTPUT_HW_ID versions
Posted by Anshuman Khandual 1 year, 7 months ago 

On 5/7/24 15:36, James Clark wrote:
> 
> 
> On 07/05/2024 04:47, Anshuman Khandual wrote:
>>
>>
>> On 4/29/24 20:51, James Clark wrote:
>>> The likely fix for this is to update Perf so print a helpful message.
>>>
>>> Signed-off-by: James Clark <james.clark@arm.com>
>>> ---
>>>  tools/perf/util/cs-etm.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>>> index d65d7485886c..32818bd7cd17 100644
>>> --- a/tools/perf/util/cs-etm.c
>>> +++ b/tools/perf/util/cs-etm.c
>>> @@ -335,8 +335,11 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session,
>>>  	trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);
>>>  
>>>  	/* check that we can handle this version */
>>> -	if (version > CS_AUX_HW_ID_CURR_VERSION)
>>> +	if (version > CS_AUX_HW_ID_CURR_VERSION) {
>>> +		pr_err("CS ETM Trace: PERF_RECORD_AUX_OUTPUT_HW_ID version %d not supported. Please update Perf.\n",
>>
>> Is not this bit misleading ? PERF_RECORD_AUX_OUTPUT_HW_ID is just the perf record
>> format identifier. The record version here, is derived from the platform specific
>> hardware ID information embedded in this perf record.
> 
> Not sure I follow what you mean here. 'version' is something that's
> output by the kernel. It's saved into a perf.data file, and if Perf
> can't handle version 2 for example, you need to update Perf.

Got it.

> 
>>
>> Should not this be just s/PERF_RECORD_AUX_OUTPUT_HW_ID/hardware ID/ instead ?
>>
> 
> It's just a way to go from the error message to the part of the code or
> docs that you need to look at. "hardware ID" wouldn't lead you anywhere
> so I don't think it would be useful.

Sure, fair enough.

> 
>>> +		       version);
>>>  		return -EINVAL;
>>> +	}
>>>  
>>>  	/* get access to the etm metadata */
>>>  	etm = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace);
>>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
Re: [PATCH 01/17] perf cs-etm: Print error for new PERF_RECORD_AUX_OUTPUT_HW_ID versions
Posted by Arnaldo Carvalho de Melo 1 year, 7 months ago 
On Tue, May 07, 2024 at 04:27:25PM +0530, Anshuman Khandual wrote:
> On 5/7/24 15:36, James Clark wrote:
> > On 07/05/2024 04:47, Anshuman Khandual wrote:
> >> On 4/29/24 20:51, James Clark wrote:
> >>> The likely fix for this is to update Perf so print a helpful message.
> >>>
> >>> Signed-off-by: James Clark <james.clark@arm.com>
> >>> ---
> >>>  tools/perf/util/cs-etm.c | 5 ++++-
> >>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> >>> index d65d7485886c..32818bd7cd17 100644
> >>> --- a/tools/perf/util/cs-etm.c
> >>> +++ b/tools/perf/util/cs-etm.c
> >>> @@ -335,8 +335,11 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session,
> >>>  	trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);
> >>>  
> >>>  	/* check that we can handle this version */
> >>> -	if (version > CS_AUX_HW_ID_CURR_VERSION)
> >>> +	if (version > CS_AUX_HW_ID_CURR_VERSION) {
> >>> +		pr_err("CS ETM Trace: PERF_RECORD_AUX_OUTPUT_HW_ID version %d not supported. Please update Perf.\n",
> >>
> >> Is not this bit misleading ? PERF_RECORD_AUX_OUTPUT_HW_ID is just the perf record
> >> format identifier. The record version here, is derived from the platform specific
> >> hardware ID information embedded in this perf record.
> > 
> > Not sure I follow what you mean here. 'version' is something that's
> > output by the kernel. It's saved into a perf.data file, and if Perf
> > can't handle version 2 for example, you need to update Perf.
 
> Got it.
 
> >> Should not this be just s/PERF_RECORD_AUX_OUTPUT_HW_ID/hardware ID/ instead ?
> >>
> > 
> > It's just a way to go from the error message to the part of the code or
> > docs that you need to look at. "hardware ID" wouldn't lead you anywhere
> > so I don't think it would be useful.
> 
> Sure, fair enough.

I'm taking this as an Acked-by, ok?

- Arnaldo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.