Cycle counting is enabled, when requested and supported but with a default
threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
representing the minimum interval between cycle count trace packets.
This makes cycle threshold user configurable, from the user space via perf
event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
in case no explicit request. As expected it creates a sysfs file as well.
/sys/bus/event_source/devices/cs_etm/format/cc_threshold
New 'cc_threshold' uses 'event->attr.config3' as no more space is available
in 'event->attr.config1' or 'event->attr.config2'.
Cc: Suzuki K Poulose <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: James Clark <[email protected]>
Cc: Leo Yan <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
Documentation/trace/coresight/coresight.rst | 2 ++
drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst
index 4a71ea6cb390..b88d83b59531 100644
--- a/Documentation/trace/coresight/coresight.rst
+++ b/Documentation/trace/coresight/coresight.rst
@@ -624,6 +624,8 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/
* - timestamp
- Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP
<coresight-timestamp>`
+ * - cc_treshold
+ - Cycle count treshhold value
How to use the STM module
-------------------------
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 5ca6278baff4..09f75dffae60 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset, "config:0-3");
PMU_FORMAT_ATTR(sinkid, "config2:0-31");
/* config ID - set if a system configuration is selected */
PMU_FORMAT_ATTR(configid, "config2:32-63");
+PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
/*
@@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
&format_attr_preset.attr,
&format_attr_configid.attr,
&format_attr_branch_broadcast.attr,
+ &format_attr_cc_threshold.attr,
NULL,
};
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 9d186af81ea0..9a2766f68416 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
struct etmv4_config *config = &drvdata->config;
struct perf_event_attr *attr = &event->attr;
unsigned long cfg_hash;
- int preset;
+ int preset, cc_threshold;
/* Clear configuration from previous run */
memset(config, 0, sizeof(struct etmv4_config));
@@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
if (attr->config & BIT(ETM_OPT_CYCACC)) {
config->cfg |= TRCCONFIGR_CCI;
/* TRM: Must program this for cycacc to work */
- config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
+ cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
+ if (cc_treshold) {
+ if (cc_treshold < drvdata->ccitmin)
+ config->ccctlr = drvdata->ccitmin;
+ else
+ config->ccctlr = cc_threshold;
+ } else {
+ config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
+ }
}
if (attr->config & BIT(ETM_OPT_TS)) {
/*
--
2.25.1
On 8/4/23 13:34, Al Grant wrote:
>
>
>> -----Original Message-----
>> From: Anshuman Khandual <[email protected]>
>> Sent: Friday, August 4, 2023 5:47 AM
>> To: [email protected]
>> Cc: Anshuman Khandual <[email protected]>; Mike Leach
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]
>> Subject: [PATCH] coresight: etm: Make cycle count threshold user configurable
>>
>> Cycle counting is enabled, when requested and supported but with a default
>> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into
>> TRCCCCTLR, representing the minimum interval between cycle count trace
>> packets.
>>
>> This makes cycle threshold user configurable, from the user space via perf event
>> attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT, in case
>> no explicit request. As expected it creates a sysfs file as well.
>>
>> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>>
>> New 'cc_threshold' uses 'event->attr.config3' as no more space is available in
>> 'event->attr.config1' or 'event->attr.config2'.
>>
>> Cc: Suzuki K Poulose <[email protected]>
>> Cc: Mike Leach <[email protected]>
>> Cc: James Clark <[email protected]>
>> Cc: Leo Yan <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Anshuman Khandual <[email protected]>
>> ---
>> Documentation/trace/coresight/coresight.rst | 2 ++
>> drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++
>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
>> 3 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/trace/coresight/coresight.rst
>> b/Documentation/trace/coresight/coresight.rst
>> index 4a71ea6cb390..b88d83b59531 100644
>> --- a/Documentation/trace/coresight/coresight.rst
>> +++ b/Documentation/trace/coresight/coresight.rst
>> @@ -624,6 +624,8 @@ They are also listed in the folder
>> /sys/bus/event_source/devices/cs_etm/format/
>> * - timestamp
>> - Session local version of the system wide setting:
>> :ref:`ETMv4_MODE_TIMESTAMP
>> <coresight-timestamp>`
>> + * - cc_treshold
>
> Spelling: cc_threshold
Will fix this, besides does it require some more description for
this new config option i.e cc_threshold ?
>
>> + - Cycle count treshhold value
>>
>> How to use the STM module
>> -------------------------
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index 5ca6278baff4..09f75dffae60 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset, "config:0-3");
>> PMU_FORMAT_ATTR(sinkid, "config2:0-31");
>> /* config ID - set if a system configuration is selected */
>> PMU_FORMAT_ATTR(configid, "config2:32-63");
>> +PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
>>
>>
>> /*
>> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
>> &format_attr_preset.attr,
>> &format_attr_configid.attr,
>> &format_attr_branch_broadcast.attr,
>> + &format_attr_cc_threshold.attr,
>> NULL,
>> };
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 9d186af81ea0..9a2766f68416 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct
>> coresight_device *csdev,
>> struct etmv4_config *config = &drvdata->config;
>> struct perf_event_attr *attr = &event->attr;
>> unsigned long cfg_hash;
>> - int preset;
>> + int preset, cc_threshold;
>>
>> /* Clear configuration from previous run */
>> memset(config, 0, sizeof(struct etmv4_config)); @@ -667,7 +667,15 @@
>> static int etm4_parse_event_config(struct coresight_device *csdev,
>> if (attr->config & BIT(ETM_OPT_CYCACC)) {
>> config->cfg |= TRCCONFIGR_CCI;
>> /* TRM: Must program this for cycacc to work */
>> - config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>> + cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
>
> Spelling again
Yikes, this does not even build. Seems like I had missed the applicable
config i.e CONFIG_CORESIGHT_SOURCE_ETM4X this time around. Apologies.
>
>> + if (cc_treshold) {
>> + if (cc_treshold < drvdata->ccitmin)
>> + config->ccctlr = drvdata->ccitmin;
>> + else
>> + config->ccctlr = cc_threshold;
>> + } else {
>> + config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>> + }
>
> Consider dropping the check against CCITMIN. There are CPUs where
> CCITMIN is incorrect, e.g. see published errata 1490853 where the
> value 0x100 should be 0b100 i.e. 4. On these ETMs it is possible to
> set the timing threshold to four cycles instead of 256 cycles, providing
> much better timing resolution. The kernel currently does not work
> around this errata and uses the incorrect value of ccitmin. If you drop
> the check, and trust the value provided by userspace, you allow
> userspace to work around it.
Why ? We could just work around the errata #1490853 while initializing
the drvdata->ccitmin if that is where the problem exists. I dont think
user space should be required to know about the erratas, and provide a
right value instead.
> -----Original Message-----
> From: Anshuman Khandual <[email protected]>
> Sent: Friday, August 4, 2023 5:47 AM
> To: [email protected]
> Cc: Anshuman Khandual <[email protected]>; Mike Leach
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: [PATCH] coresight: etm: Make cycle count threshold user configurable
>
> Cycle counting is enabled, when requested and supported but with a default
> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into
> TRCCCCTLR, representing the minimum interval between cycle count trace
> packets.
>
> This makes cycle threshold user configurable, from the user space via perf event
> attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT, in case
> no explicit request. As expected it creates a sysfs file as well.
>
> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>
> New 'cc_threshold' uses 'event->attr.config3' as no more space is available in
> 'event->attr.config1' or 'event->attr.config2'.
>
> Cc: Suzuki K Poulose <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: James Clark <[email protected]>
> Cc: Leo Yan <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> Documentation/trace/coresight/coresight.rst | 2 ++
> drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
> 3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/trace/coresight/coresight.rst
> b/Documentation/trace/coresight/coresight.rst
> index 4a71ea6cb390..b88d83b59531 100644
> --- a/Documentation/trace/coresight/coresight.rst
> +++ b/Documentation/trace/coresight/coresight.rst
> @@ -624,6 +624,8 @@ They are also listed in the folder
> /sys/bus/event_source/devices/cs_etm/format/
> * - timestamp
> - Session local version of the system wide setting:
> :ref:`ETMv4_MODE_TIMESTAMP
> <coresight-timestamp>`
> + * - cc_treshold
Spelling: cc_threshold
> + - Cycle count treshhold value
>
> How to use the STM module
> -------------------------
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c
> b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 5ca6278baff4..09f75dffae60 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset, "config:0-3");
> PMU_FORMAT_ATTR(sinkid, "config2:0-31");
> /* config ID - set if a system configuration is selected */
> PMU_FORMAT_ATTR(configid, "config2:32-63");
> +PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
>
>
> /*
> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
> &format_attr_preset.attr,
> &format_attr_configid.attr,
> &format_attr_branch_broadcast.attr,
> + &format_attr_cc_threshold.attr,
> NULL,
> };
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 9d186af81ea0..9a2766f68416 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct
> coresight_device *csdev,
> struct etmv4_config *config = &drvdata->config;
> struct perf_event_attr *attr = &event->attr;
> unsigned long cfg_hash;
> - int preset;
> + int preset, cc_threshold;
>
> /* Clear configuration from previous run */
> memset(config, 0, sizeof(struct etmv4_config)); @@ -667,7 +667,15 @@
> static int etm4_parse_event_config(struct coresight_device *csdev,
> if (attr->config & BIT(ETM_OPT_CYCACC)) {
> config->cfg |= TRCCONFIGR_CCI;
> /* TRM: Must program this for cycacc to work */
> - config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> + cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
Spelling again
> + if (cc_treshold) {
> + if (cc_treshold < drvdata->ccitmin)
> + config->ccctlr = drvdata->ccitmin;
> + else
> + config->ccctlr = cc_threshold;
> + } else {
> + config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> + }
Consider dropping the check against CCITMIN. There are CPUs where
CCITMIN is incorrect, e.g. see published errata 1490853 where the
value 0x100 should be 0b100 i.e. 4. On these ETMs it is possible to
set the timing threshold to four cycles instead of 256 cycles, providing
much better timing resolution. The kernel currently does not work
around this errata and uses the incorrect value of ccitmin. If you drop
the check, and trust the value provided by userspace, you allow
userspace to work around it.
Al
> }
> if (attr->config & BIT(ETM_OPT_TS)) {
> /*
> --
> 2.25.1
>
> _______________________________________________
> CoreSight mailing list -- [email protected] To unsubscribe send an email to
> [email protected]
On 04/08/2023 09:45, Anshuman Khandual wrote:
>
>
> On 8/4/23 13:34, Al Grant wrote:
>>
>>
>>> -----Original Message-----
>>> From: Anshuman Khandual <[email protected]>
>>> Sent: Friday, August 4, 2023 5:47 AM
>>> To: [email protected]
>>> Cc: Anshuman Khandual <[email protected]>; Mike Leach
>>> <[email protected]>; [email protected]; [email protected];
>>> [email protected]
>>> Subject: [PATCH] coresight: etm: Make cycle count threshold user configurable
>>>
>>> Cycle counting is enabled, when requested and supported but with a default
>>> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into
>>> TRCCCCTLR, representing the minimum interval between cycle count trace
>>> packets.
>>>
>>> This makes cycle threshold user configurable, from the user space via perf event
>>> attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT, in case
>>> no explicit request. As expected it creates a sysfs file as well.
>>>
>>> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>>>
>>> New 'cc_threshold' uses 'event->attr.config3' as no more space is available in
>>> 'event->attr.config1' or 'event->attr.config2'.
>>>
>>> Cc: Suzuki K Poulose <[email protected]>
>>> Cc: Mike Leach <[email protected]>
>>> Cc: James Clark <[email protected]>
>>> Cc: Leo Yan <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Signed-off-by: Anshuman Khandual <[email protected]>
>>> ---
>>> Documentation/trace/coresight/coresight.rst | 2 ++
>>> drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++
>>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
>>> 3 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/trace/coresight/coresight.rst
>>> b/Documentation/trace/coresight/coresight.rst
>>> index 4a71ea6cb390..b88d83b59531 100644
>>> --- a/Documentation/trace/coresight/coresight.rst
>>> +++ b/Documentation/trace/coresight/coresight.rst
>>> @@ -624,6 +624,8 @@ They are also listed in the folder
>>> /sys/bus/event_source/devices/cs_etm/format/
>>> * - timestamp
>>> - Session local version of the system wide setting:
>>> :ref:`ETMv4_MODE_TIMESTAMP
>>> <coresight-timestamp>`
>>> + * - cc_treshold
>>
>> Spelling: cc_threshold
>
> Will fix this, besides does it require some more description for
> this new config option i.e cc_threshold ?
>
>>
>>> + - Cycle count treshhold value
>>>
>>> How to use the STM module
>>> -------------------------
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> index 5ca6278baff4..09f75dffae60 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset, "config:0-3");
>>> PMU_FORMAT_ATTR(sinkid, "config2:0-31");
>>> /* config ID - set if a system configuration is selected */
>>> PMU_FORMAT_ATTR(configid, "config2:32-63");
>>> +PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
>>>
>>>
>>> /*
>>> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
>>> &format_attr_preset.attr,
>>> &format_attr_configid.attr,
>>> &format_attr_branch_broadcast.attr,
>>> + &format_attr_cc_threshold.attr,
>>> NULL,
>>> };
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> index 9d186af81ea0..9a2766f68416 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct
>>> coresight_device *csdev,
>>> struct etmv4_config *config = &drvdata->config;
>>> struct perf_event_attr *attr = &event->attr;
>>> unsigned long cfg_hash;
>>> - int preset;
>>> + int preset, cc_threshold;
>>>
>>> /* Clear configuration from previous run */
>>> memset(config, 0, sizeof(struct etmv4_config)); @@ -667,7 +667,15 @@
>>> static int etm4_parse_event_config(struct coresight_device *csdev,
>>> if (attr->config & BIT(ETM_OPT_CYCACC)) {
>>> config->cfg |= TRCCONFIGR_CCI;
>>> /* TRM: Must program this for cycacc to work */
>>> - config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>>> + cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
>>
>> Spelling again
>
> Yikes, this does not even build. Seems like I had missed the applicable
> config i.e CONFIG_CORESIGHT_SOURCE_ETM4X this time around. Apologies.
>
>>
>>> + if (cc_treshold) {
>>> + if (cc_treshold < drvdata->ccitmin)
>>> + config->ccctlr = drvdata->ccitmin;
>>> + else
>>> + config->ccctlr = cc_threshold;
>>> + } else {
>>> + config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>>> + }
>>
>> Consider dropping the check against CCITMIN. There are CPUs where
>> CCITMIN is incorrect, e.g. see published errata 1490853 where the
>> value 0x100 should be 0b100 i.e. 4. On these ETMs it is possible to
>> set the timing threshold to four cycles instead of 256 cycles, providing
>> much better timing resolution. The kernel currently does not work
>> around this errata and uses the incorrect value of ccitmin. If you drop
>> the check, and trust the value provided by userspace, you allow
>> userspace to work around it.
> Why ? We could just work around the errata #1490853 while initializing
> the drvdata->ccitmin if that is where the problem exists.
> I dont think
> user space should be required to know about the erratas, and provide a
I think that becomes less true for the tracing and PMU stuff. If you are
using it you likely need to know a lot about the platform you are
working on anyway.
For example right now I'm trying to upstream some metric formulas which
have a workaround where userspace needs to know the variant of the
processor. It's not possible for the kernel to do anything about it.
In this case as it's only one known errata we could add the workaround.
Unless we expect there to be the same issue again in the future? Or we
know there are already more CPUs than #1490853 mentions?
> right value instead.
> _______________________________________________
> CoreSight mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
> -----Original Message-----
> From: James Clark <[email protected]>
> Sent: Friday, August 4, 2023 10:23 AM
> To: Anshuman Khandual <[email protected]>; Al Grant
> <[email protected]>; [email protected]
> Cc: Mike Leach <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH] coresight: etm: Make cycle count threshold user
> configurable
>
>
>
> On 04/08/2023 09:45, Anshuman Khandual wrote:
> >
> >
> > On 8/4/23 13:34, Al Grant wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Anshuman Khandual <[email protected]>
> >>> Sent: Friday, August 4, 2023 5:47 AM
> >>> To: [email protected]
> >>> Cc: Anshuman Khandual <[email protected]>; Mike Leach
> >>> <[email protected]>; [email protected];
> >>> [email protected]; [email protected]
> >>> Subject: [PATCH] coresight: etm: Make cycle count threshold user
> >>> configurable
> >>>
> >>> Cycle counting is enabled, when requested and supported but with a
> >>> default threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting
> >>> into TRCCCCTLR, representing the minimum interval between cycle
> >>> count trace packets.
> >>>
> >>> This makes cycle threshold user configurable, from the user space
> >>> via perf event attributes. Although it falls back using
> >>> ETM_CYC_THRESHOLD_DEFAULT, in case no explicit request. As expected it
> creates a sysfs file as well.
> >>>
> >>> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
> >>>
> >>> New 'cc_threshold' uses 'event->attr.config3' as no more space is
> >>> available in 'event->attr.config1' or 'event->attr.config2'.
> >>>
> >>> Cc: Suzuki K Poulose <[email protected]>
> >>> Cc: Mike Leach <[email protected]>
> >>> Cc: James Clark <[email protected]>
> >>> Cc: Leo Yan <[email protected]>
> >>> Cc: [email protected]
> >>> Cc: [email protected]
> >>> Cc: [email protected]
> >>> Cc: [email protected]
> >>> Signed-off-by: Anshuman Khandual <[email protected]>
> >>> ---
> >>> Documentation/trace/coresight/coresight.rst | 2 ++
> >>> drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++
> >>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 12
> >>> ++++++++++--
> >>> 3 files changed, 14 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/trace/coresight/coresight.rst
> >>> b/Documentation/trace/coresight/coresight.rst
> >>> index 4a71ea6cb390..b88d83b59531 100644
> >>> --- a/Documentation/trace/coresight/coresight.rst
> >>> +++ b/Documentation/trace/coresight/coresight.rst
> >>> @@ -624,6 +624,8 @@ They are also listed in the folder
> >>> /sys/bus/event_source/devices/cs_etm/format/
> >>> * - timestamp
> >>> - Session local version of the system wide setting:
> >>> :ref:`ETMv4_MODE_TIMESTAMP
> >>> <coresight-timestamp>`
> >>> + * - cc_treshold
> >>
> >> Spelling: cc_threshold
> >
> > Will fix this, besides does it require some more description for this
> > new config option i.e cc_threshold ?
> >
> >>
> >>> + - Cycle count treshhold value
> >>>
> >>> How to use the STM module
> >>> -------------------------
> >>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c
> >>> b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >>> index 5ca6278baff4..09f75dffae60 100644
> >>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> >>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >>> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset, "config:0-3");
> >>> PMU_FORMAT_ATTR(sinkid, "config2:0-31");
> >>> /* config ID - set if a system configuration is selected */
> >>> PMU_FORMAT_ATTR(configid, "config2:32-63");
> >>> +PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
> >>>
> >>>
> >>> /*
> >>> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
> >>> &format_attr_preset.attr,
> >>> &format_attr_configid.attr,
> >>> &format_attr_branch_broadcast.attr,
> >>> + &format_attr_cc_threshold.attr,
> >>> NULL,
> >>> };
> >>>
> >>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >>> index 9d186af81ea0..9a2766f68416 100644
> >>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >>> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct
> >>> coresight_device *csdev,
> >>> struct etmv4_config *config = &drvdata->config;
> >>> struct perf_event_attr *attr = &event->attr;
> >>> unsigned long cfg_hash;
> >>> - int preset;
> >>> + int preset, cc_threshold;
> >>>
> >>> /* Clear configuration from previous run */
> >>> memset(config, 0, sizeof(struct etmv4_config)); @@ -667,7 +667,15
> >>> @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> >>> if (attr->config & BIT(ETM_OPT_CYCACC)) {
> >>> config->cfg |= TRCCONFIGR_CCI;
> >>> /* TRM: Must program this for cycacc to work */
> >>> - config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> >>> + cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
> >>
> >> Spelling again
> >
> > Yikes, this does not even build. Seems like I had missed the
> > applicable config i.e CONFIG_CORESIGHT_SOURCE_ETM4X this time around.
> Apologies.
> >
> >>
> >>> + if (cc_treshold) {
> >>> + if (cc_treshold < drvdata->ccitmin)
> >>> + config->ccctlr = drvdata->ccitmin;
> >>> + else
> >>> + config->ccctlr = cc_threshold;
> >>> + } else {
> >>> + config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> >>> + }
> >>
> >> Consider dropping the check against CCITMIN. There are CPUs where
> >> CCITMIN is incorrect, e.g. see published errata 1490853 where the
> >> value 0x100 should be 0b100 i.e. 4. On these ETMs it is possible to
> >> set the timing threshold to four cycles instead of 256 cycles,
> >> providing much better timing resolution. The kernel currently does
> >> not work around this errata and uses the incorrect value of ccitmin.
> >> If you drop the check, and trust the value provided by userspace, you
> >> allow userspace to work around it.
> > Why ? We could just work around the errata #1490853 while initializing
> > the drvdata->ccitmin if that is where the problem exists.
>
>
> > I dont think
> > user space should be required to know about the erratas, and provide a
>
> I think that becomes less true for the tracing and PMU stuff. If you are using it you
> likely need to know a lot about the platform you are working on anyway.
>
> For example right now I'm trying to upstream some metric formulas which have a
> workaround where userspace needs to know the variant of the processor. It's not
> possible for the kernel to do anything about it.
>
> In this case as it's only one known errata we could add the workaround.
>
> Unless we expect there to be the same issue again in the future? Or we know
> there are already more CPUs than #1490853 mentions?
It's a common-mode failure affecting several CPUs, each with their
own set of affected/fixed versions, so there would be several MIDRs
to check. The ones that have been published in errata notices include:
- #1490853: Neoverse N1, fixed r4p1
- #1490853: Cortex-A76, fixed r4p1 (n.b. same number as above)
- #1619801: Neoverse V1, fixed r1p0
- #1502854: Cortex-X1, fixed r1p0
- #1491015: Cortex-A77, fixed r1p1
Hopefully that's the lot.
For this issue you don't really need to check the fix version, as the
number you'd put in ccitmin is the same anyway.
Al
>
> > right value instead.
> > _______________________________________________
> > CoreSight mailing list -- [email protected] To unsubscribe
> > send an email to [email protected]
On 04/08/2023 11:10, Al Grant wrote:
>
>
>> -----Original Message-----
>> From: James Clark <[email protected]>
>> Sent: Friday, August 4, 2023 10:23 AM
>> To: Anshuman Khandual <[email protected]>; Al Grant
>> <[email protected]>; [email protected]
>> Cc: Mike Leach <[email protected]>; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: Re: [PATCH] coresight: etm: Make cycle count threshold user
>> configurable
>>
>>
>>
>> On 04/08/2023 09:45, Anshuman Khandual wrote:
>>>
>>>
>>> On 8/4/23 13:34, Al Grant wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Anshuman Khandual <[email protected]>
>>>>> Sent: Friday, August 4, 2023 5:47 AM
>>>>> To: [email protected]
>>>>> Cc: Anshuman Khandual <[email protected]>; Mike Leach
>>>>> <[email protected]>; [email protected];
>>>>> [email protected]; [email protected]
>>>>> Subject: [PATCH] coresight: etm: Make cycle count threshold user
>>>>> configurable
>>>>>
>>>>> Cycle counting is enabled, when requested and supported but with a
>>>>> default threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting
>>>>> into TRCCCCTLR, representing the minimum interval between cycle
>>>>> count trace packets.
>>>>>
>>>>> This makes cycle threshold user configurable, from the user space
>>>>> via perf event attributes. Although it falls back using
>>>>> ETM_CYC_THRESHOLD_DEFAULT, in case no explicit request. As expected it
>> creates a sysfs file as well.
>>>>>
>>>>> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>>>>>
>>>>> New 'cc_threshold' uses 'event->attr.config3' as no more space is
>>>>> available in 'event->attr.config1' or 'event->attr.config2'.
>>>>>
>>>>> Cc: Suzuki K Poulose <[email protected]>
>>>>> Cc: Mike Leach <[email protected]>
>>>>> Cc: James Clark <[email protected]>
>>>>> Cc: Leo Yan <[email protected]>
>>>>> Cc: [email protected]
>>>>> Cc: [email protected]
>>>>> Cc: [email protected]
>>>>> Cc: [email protected]
>>>>> Signed-off-by: Anshuman Khandual <[email protected]>
>>>>> ---
>>>>> Documentation/trace/coresight/coresight.rst | 2 ++
>>>>> drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++
>>>>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 12
>>>>> ++++++++++--
>>>>> 3 files changed, 14 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/trace/coresight/coresight.rst
>>>>> b/Documentation/trace/coresight/coresight.rst
>>>>> index 4a71ea6cb390..b88d83b59531 100644
>>>>> --- a/Documentation/trace/coresight/coresight.rst
>>>>> +++ b/Documentation/trace/coresight/coresight.rst
>>>>> @@ -624,6 +624,8 @@ They are also listed in the folder
>>>>> /sys/bus/event_source/devices/cs_etm/format/
>>>>> * - timestamp
>>>>> - Session local version of the system wide setting:
>>>>> :ref:`ETMv4_MODE_TIMESTAMP
>>>>> <coresight-timestamp>`
>>>>> + * - cc_treshold
>>>>
>>>> Spelling: cc_threshold
>>>
>>> Will fix this, besides does it require some more description for this
>>> new config option i.e cc_threshold ?
>>>
>>>>
>>>>> + - Cycle count treshhold value
>>>>>
>>>>> How to use the STM module
>>>>> -------------------------
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c
>>>>> b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>>>> index 5ca6278baff4..09f75dffae60 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>>>> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset, "config:0-3");
>>>>> PMU_FORMAT_ATTR(sinkid, "config2:0-31");
>>>>> /* config ID - set if a system configuration is selected */
>>>>> PMU_FORMAT_ATTR(configid, "config2:32-63");
>>>>> +PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
>>>>>
>>>>>
>>>>> /*
>>>>> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
>>>>> &format_attr_preset.attr,
>>>>> &format_attr_configid.attr,
>>>>> &format_attr_branch_broadcast.attr,
>>>>> + &format_attr_cc_threshold.attr,
>>>>> NULL,
>>>>> };
>>>>>
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>>> index 9d186af81ea0..9a2766f68416 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>>> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct
>>>>> coresight_device *csdev,
>>>>> struct etmv4_config *config = &drvdata->config;
>>>>> struct perf_event_attr *attr = &event->attr;
>>>>> unsigned long cfg_hash;
>>>>> - int preset;
>>>>> + int preset, cc_threshold;
>>>>>
>>>>> /* Clear configuration from previous run */
>>>>> memset(config, 0, sizeof(struct etmv4_config)); @@ -667,7 +667,15
>>>>> @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>>>>> if (attr->config & BIT(ETM_OPT_CYCACC)) {
>>>>> config->cfg |= TRCCONFIGR_CCI;
>>>>> /* TRM: Must program this for cycacc to work */
>>>>> - config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>>>>> + cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
>>>>
>>>> Spelling again
>>>
>>> Yikes, this does not even build. Seems like I had missed the
>>> applicable config i.e CONFIG_CORESIGHT_SOURCE_ETM4X this time around.
>> Apologies.
>>>
>>>>
>>>>> + if (cc_treshold) {
>>>>> + if (cc_treshold < drvdata->ccitmin)
>>>>> + config->ccctlr = drvdata->ccitmin;
>>>>> + else
>>>>> + config->ccctlr = cc_threshold;
>>>>> + } else {
>>>>> + config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>>>>> + }
>>>>
>>>> Consider dropping the check against CCITMIN. There are CPUs where
>>>> CCITMIN is incorrect, e.g. see published errata 1490853 where the
>>>> value 0x100 should be 0b100 i.e. 4. On these ETMs it is possible to
>>>> set the timing threshold to four cycles instead of 256 cycles,
>>>> providing much better timing resolution. The kernel currently does
>>>> not work around this errata and uses the incorrect value of ccitmin.
>>>> If you drop the check, and trust the value provided by userspace, you
>>>> allow userspace to work around it.
>>> Why ? We could just work around the errata #1490853 while initializing
>>> the drvdata->ccitmin if that is where the problem exists.
>>
>>
>>> I dont think
>>> user space should be required to know about the erratas, and provide a
>>
>> I think that becomes less true for the tracing and PMU stuff. If you are using it you
>> likely need to know a lot about the platform you are working on anyway.
>>
>> For example right now I'm trying to upstream some metric formulas which have a
>> workaround where userspace needs to know the variant of the processor. It's not
>> possible for the kernel to do anything about it.
>>
>> In this case as it's only one known errata we could add the workaround.
>>
>> Unless we expect there to be the same issue again in the future? Or we know
>> there are already more CPUs than #1490853 mentions?
>
> It's a common-mode failure affecting several CPUs, each with their
> own set of affected/fixed versions, so there would be several MIDRs
> to check. The ones that have been published in errata notices include:
>
> - #1490853: Neoverse N1, fixed r4p1
> - #1490853: Cortex-A76, fixed r4p1 (n.b. same number as above)
> - #1619801: Neoverse V1, fixed r1p0
> - #1502854: Cortex-X1, fixed r1p0
> - #1491015: Cortex-A77, fixed r1p1
>
> Hopefully that's the lot.
>
> For this issue you don't really need to check the fix version, as the
> number you'd put in ccitmin is the same anyway.
>
> Al
>
It could make sense to not clamp it then. Should we at least expose
CCITMIN in sysfs so userspace can attempt to put the right value if it's
not trying to work around something? If it's not already exposed.
>
>>
>>> right value instead.
>>> _______________________________________________
>>> CoreSight mailing list -- [email protected] To unsubscribe
>>> send an email to [email protected]
Hi Anshuman,
kernel test robot noticed the following build errors:
[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.5-rc4 next-20230804]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Anshuman-Khandual/coresight-etm-Make-cycle-count-threshold-user-configurable/20230804-124850
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20230804044720.1478900-1-anshuman.khandual%40arm.com
patch subject: [PATCH] coresight: etm: Make cycle count threshold user configurable
config: arm64-randconfig-r004-20230731 (https://download.01.org/0day-ci/archive/20230805/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230805/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
drivers/hwtracing/coresight/coresight-etm4x-core.c: In function 'etm4_parse_event_config':
>> drivers/hwtracing/coresight/coresight-etm4x-core.c:661:17: error: 'cc_treshold' undeclared (first use in this function); did you mean 'cc_threshold'?
661 | cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
| ^~~~~~~~~~~
| cc_threshold
drivers/hwtracing/coresight/coresight-etm4x-core.c:661:17: note: each undeclared identifier is reported only once for each function it appears in
vim +661 drivers/hwtracing/coresight/coresight-etm4x-core.c
629
630 static int etm4_parse_event_config(struct coresight_device *csdev,
631 struct perf_event *event)
632 {
633 int ret = 0;
634 struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
635 struct etmv4_config *config = &drvdata->config;
636 struct perf_event_attr *attr = &event->attr;
637 unsigned long cfg_hash;
638 int preset, cc_threshold;
639
640 /* Clear configuration from previous run */
641 memset(config, 0, sizeof(struct etmv4_config));
642
643 if (attr->exclude_kernel)
644 config->mode = ETM_MODE_EXCL_KERN;
645
646 if (attr->exclude_user)
647 config->mode = ETM_MODE_EXCL_USER;
648
649 /* Always start from the default config */
650 etm4_set_default_config(config);
651
652 /* Configure filters specified on the perf cmd line, if any. */
653 ret = etm4_set_event_filters(drvdata, event);
654 if (ret)
655 goto out;
656
657 /* Go from generic option to ETMv4 specifics */
658 if (attr->config & BIT(ETM_OPT_CYCACC)) {
659 config->cfg |= TRCCONFIGR_CCI;
660 /* TRM: Must program this for cycacc to work */
> 661 cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
662 if (cc_treshold) {
663 if (cc_treshold < drvdata->ccitmin)
664 config->ccctlr = drvdata->ccitmin;
665 else
666 config->ccctlr = cc_threshold;
667 } else {
668 config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
669 }
670 }
671 if (attr->config & BIT(ETM_OPT_TS)) {
672 /*
673 * Configure timestamps to be emitted at regular intervals in
674 * order to correlate instructions executed on different CPUs
675 * (CPU-wide trace scenarios).
676 */
677 ret = etm4_config_timestamp_event(drvdata);
678
679 /*
680 * No need to go further if timestamp intervals can't
681 * be configured.
682 */
683 if (ret)
684 goto out;
685
686 /* bit[11], Global timestamp tracing bit */
687 config->cfg |= TRCCONFIGR_TS;
688 }
689
690 /* Only trace contextID when runs in root PID namespace */
691 if ((attr->config & BIT(ETM_OPT_CTXTID)) &&
692 task_is_in_init_pid_ns(current))
693 /* bit[6], Context ID tracing bit */
694 config->cfg |= TRCCONFIGR_CID;
695
696 /*
697 * If set bit ETM_OPT_CTXTID2 in perf config, this asks to trace VMID
698 * for recording CONTEXTIDR_EL2. Do not enable VMID tracing if the
699 * kernel is not running in EL2.
700 */
701 if (attr->config & BIT(ETM_OPT_CTXTID2)) {
702 if (!is_kernel_in_hyp_mode()) {
703 ret = -EINVAL;
704 goto out;
705 }
706 /* Only trace virtual contextID when runs in root PID namespace */
707 if (task_is_in_init_pid_ns(current))
708 config->cfg |= TRCCONFIGR_VMID | TRCCONFIGR_VMIDOPT;
709 }
710
711 /* return stack - enable if selected and supported */
712 if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
713 /* bit[12], Return stack enable bit */
714 config->cfg |= TRCCONFIGR_RS;
715
716 /*
717 * Set any selected configuration and preset.
718 *
719 * This extracts the values of PMU_FORMAT_ATTR(configid) and PMU_FORMAT_ATTR(preset)
720 * in the perf attributes defined in coresight-etm-perf.c.
721 * configid uses bits 63:32 of attr->config2, preset uses bits 3:0 of attr->config.
722 * A zero configid means no configuration active, preset = 0 means no preset selected.
723 */
724 if (attr->config2 & GENMASK_ULL(63, 32)) {
725 cfg_hash = (u32)(attr->config2 >> 32);
726 preset = attr->config & 0xF;
727 ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset);
728 }
729
730 /* branch broadcast - enable if selected and supported */
731 if (attr->config & BIT(ETM_OPT_BRANCH_BROADCAST)) {
732 if (!drvdata->trcbb) {
733 /*
734 * Missing BB support could cause silent decode errors
735 * so fail to open if it's not supported.
736 */
737 ret = -EINVAL;
738 goto out;
739 } else {
740 config->cfg |= BIT(ETM4_CFG_BIT_BB);
741 }
742 }
743
744 out:
745 return ret;
746 }
747
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki