* [PATCH] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-04 4:47 Anshuman Khandual
2023-08-04 8:04 ` Al Grant
2023-08-05 3:18 ` kernel test robot
0 siblings, 2 replies; 10+ messages in thread
From: Anshuman Khandual @ 2023-08-04 4:47 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Anshuman Khandual, Suzuki K Poulose, Mike Leach, James Clark,
Leo Yan, coresight, linux-doc, linux-kernel
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 <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
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
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-04 4:47 [PATCH] coresight: etm: Make cycle count threshold user configurable Anshuman Khandual
@ 2023-08-04 8:04 ` Al Grant
2023-08-04 8:45 ` Anshuman Khandual
2023-08-05 3:18 ` kernel test robot
1 sibling, 1 reply; 10+ messages in thread
From: Al Grant @ 2023-08-04 8:04 UTC (permalink / raw)
To: Anshuman Khandual, linux-arm-kernel
Cc: Anshuman Khandual, Mike Leach, coresight, linux-doc, linux-kernel
> -----Original Message-----
> From: Anshuman Khandual <anshuman.khandual@arm.com>
> Sent: Friday, August 4, 2023 5:47 AM
> To: linux-arm-kernel@lists.infradead.org
> Cc: Anshuman Khandual <Anshuman.Khandual@arm.com>; Mike Leach
> <mike.leach@linaro.org>; coresight@lists.linaro.org; linux-doc@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 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 <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> 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 -- coresight@lists.linaro.org To unsubscribe send an email to
> coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-04 8:04 ` Al Grant
@ 2023-08-04 8:45 ` Anshuman Khandual
2023-08-04 9:23 ` James Clark
0 siblings, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2023-08-04 8:45 UTC (permalink / raw)
To: Al Grant, linux-arm-kernel; +Cc: Mike Leach, coresight, linux-doc, linux-kernel
On 8/4/23 13:34, Al Grant wrote:
>
>
>> -----Original Message-----
>> From: Anshuman Khandual <anshuman.khandual@arm.com>
>> Sent: Friday, August 4, 2023 5:47 AM
>> To: linux-arm-kernel@lists.infradead.org
>> Cc: Anshuman Khandual <Anshuman.Khandual@arm.com>; Mike Leach
>> <mike.leach@linaro.org>; coresight@lists.linaro.org; linux-doc@vger.kernel.org;
>> linux-kernel@vger.kernel.org
>> 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 <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-04 8:45 ` Anshuman Khandual
@ 2023-08-04 9:23 ` James Clark
2023-08-04 10:10 ` Al Grant
0 siblings, 1 reply; 10+ messages in thread
From: James Clark @ 2023-08-04 9:23 UTC (permalink / raw)
To: Anshuman Khandual, Al Grant, linux-arm-kernel
Cc: Mike Leach, coresight, linux-doc, linux-kernel
On 04/08/2023 09:45, Anshuman Khandual wrote:
>
>
> On 8/4/23 13:34, Al Grant wrote:
>>
>>
>>> -----Original Message-----
>>> From: Anshuman Khandual <anshuman.khandual@arm.com>
>>> Sent: Friday, August 4, 2023 5:47 AM
>>> To: linux-arm-kernel@lists.infradead.org
>>> Cc: Anshuman Khandual <Anshuman.Khandual@arm.com>; Mike Leach
>>> <mike.leach@linaro.org>; coresight@lists.linaro.org; linux-doc@vger.kernel.org;
>>> linux-kernel@vger.kernel.org
>>> 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 <suzuki.poulose@arm.com>
>>> Cc: Mike Leach <mike.leach@linaro.org>
>>> Cc: James Clark <james.clark@arm.com>
>>> Cc: Leo Yan <leo.yan@linaro.org>
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>> 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 -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-04 9:23 ` James Clark
@ 2023-08-04 10:10 ` Al Grant
2023-08-04 11:07 ` James Clark
0 siblings, 1 reply; 10+ messages in thread
From: Al Grant @ 2023-08-04 10:10 UTC (permalink / raw)
To: James Clark, Anshuman Khandual, linux-arm-kernel
Cc: Mike Leach, coresight, linux-doc, linux-kernel
> -----Original Message-----
> From: James Clark <james.clark@arm.com>
> Sent: Friday, August 4, 2023 10:23 AM
> To: Anshuman Khandual <Anshuman.Khandual@arm.com>; Al Grant
> <Al.Grant@arm.com>; linux-arm-kernel@lists.infradead.org
> Cc: Mike Leach <mike.leach@linaro.org>; coresight@lists.linaro.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org
> 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 <anshuman.khandual@arm.com>
> >>> Sent: Friday, August 4, 2023 5:47 AM
> >>> To: linux-arm-kernel@lists.infradead.org
> >>> Cc: Anshuman Khandual <Anshuman.Khandual@arm.com>; Mike Leach
> >>> <mike.leach@linaro.org>; coresight@lists.linaro.org;
> >>> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org
> >>> 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 <suzuki.poulose@arm.com>
> >>> Cc: Mike Leach <mike.leach@linaro.org>
> >>> Cc: James Clark <james.clark@arm.com>
> >>> Cc: Leo Yan <leo.yan@linaro.org>
> >>> Cc: coresight@lists.linaro.org
> >>> Cc: linux-arm-kernel@lists.infradead.org
> >>> Cc: linux-doc@vger.kernel.org
> >>> Cc: linux-kernel@vger.kernel.org
> >>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >>> ---
> >>> 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 -- coresight@lists.linaro.org To unsubscribe
> > send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-04 10:10 ` Al Grant
@ 2023-08-04 11:07 ` James Clark
0 siblings, 0 replies; 10+ messages in thread
From: James Clark @ 2023-08-04 11:07 UTC (permalink / raw)
To: Al Grant, Anshuman Khandual, linux-arm-kernel
Cc: Mike Leach, coresight, linux-doc, linux-kernel
On 04/08/2023 11:10, Al Grant wrote:
>
>
>> -----Original Message-----
>> From: James Clark <james.clark@arm.com>
>> Sent: Friday, August 4, 2023 10:23 AM
>> To: Anshuman Khandual <Anshuman.Khandual@arm.com>; Al Grant
>> <Al.Grant@arm.com>; linux-arm-kernel@lists.infradead.org
>> Cc: Mike Leach <mike.leach@linaro.org>; coresight@lists.linaro.org; linux-
>> doc@vger.kernel.org; linux-kernel@vger.kernel.org
>> 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 <anshuman.khandual@arm.com>
>>>>> Sent: Friday, August 4, 2023 5:47 AM
>>>>> To: linux-arm-kernel@lists.infradead.org
>>>>> Cc: Anshuman Khandual <Anshuman.Khandual@arm.com>; Mike Leach
>>>>> <mike.leach@linaro.org>; coresight@lists.linaro.org;
>>>>> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org
>>>>> 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 <suzuki.poulose@arm.com>
>>>>> Cc: Mike Leach <mike.leach@linaro.org>
>>>>> Cc: James Clark <james.clark@arm.com>
>>>>> Cc: Leo Yan <leo.yan@linaro.org>
>>>>> Cc: coresight@lists.linaro.org
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: linux-doc@vger.kernel.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>> ---
>>>>> 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 -- coresight@lists.linaro.org To unsubscribe
>>> send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-04 4:47 [PATCH] coresight: etm: Make cycle count threshold user configurable Anshuman Khandual
2023-08-04 8:04 ` Al Grant
@ 2023-08-05 3:18 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2023-08-05 3:18 UTC (permalink / raw)
To: Anshuman Khandual, linux-arm-kernel
Cc: oe-kbuild-all, Anshuman Khandual, Suzuki K Poulose, Mike Leach,
James Clark, Leo Yan, coresight, linux-doc, linux-kernel
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/202308051014.Uzl7XY32-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230805/202308051014.Uzl7XY32-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308051014.Uzl7XY32-lkp@intel.com/
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-08 7:45 Anshuman Khandual
2023-08-08 10:19 ` Mike Leach
0 siblings, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2023-08-08 7:45 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Al.Grant, Anshuman Khandual, Suzuki K Poulose, Mike Leach,
James Clark, Leo Yan, coresight, linux-doc, linux-kernel
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 <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
Changes in V2:
- s/treshhold/threshhold
Changes in V1:
https://lore.kernel.org/all/20230804044720.1478900-1-anshuman.khandual@arm.com/
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..a698b07206b5 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_threshold
+ - Cycle count threshold 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..a353c0784bab 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_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
+ if (cc_threshold) {
+ if (cc_threshold < 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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-08 7:45 Anshuman Khandual
@ 2023-08-08 10:19 ` Mike Leach
2023-08-09 4:42 ` Anshuman Khandual
0 siblings, 1 reply; 10+ messages in thread
From: Mike Leach @ 2023-08-08 10:19 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-arm-kernel, Al.Grant, Suzuki K Poulose, James Clark,
Leo Yan, coresight, linux-doc, linux-kernel
Hi Anshuman
On Tue, 8 Aug 2023 at 08:45, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> 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 <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> Changes in V2:
>
Please ensure that the e-mail title reflects the "v2" nature of this.
e.g. [PATCH v2] ....
> - s/treshhold/threshhold
>
> Changes in V1:
>
> https://lore.kernel.org/all/20230804044720.1478900-1-anshuman.khandual@arm.com/
>
> 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..a698b07206b5 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_threshold
> + - Cycle count threshold value
>
> How to use the STM module
> -------------------------
The doc elements here need to be in a separate patch in the set and
additionally sent to doc list and maintainer
(linux-doc@vger.kernel.org and corbet@lwn.net) to be picked up.
> 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..a353c0784bab 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_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
> + if (cc_threshold) {
> + if (cc_threshold < 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
>
As we are using ccitmin - which I think we must - then we need an
additional patch in the set that implements the errata workaround
mentioned by Al, when setting ccitmin in etm4_init_arch_data().
Perhaps a helper function called from there to read the ID reg unless
one of the errata cores in which case set to 0x4.
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] coresight: etm: Make cycle count threshold user configurable
2023-08-08 10:19 ` Mike Leach
@ 2023-08-09 4:42 ` Anshuman Khandual
0 siblings, 0 replies; 10+ messages in thread
From: Anshuman Khandual @ 2023-08-09 4:42 UTC (permalink / raw)
To: Mike Leach
Cc: linux-arm-kernel, Al.Grant, Suzuki K Poulose, James Clark,
Leo Yan, coresight, linux-doc, linux-kernel
On 8/8/23 15:49, Mike Leach wrote:
> Hi Anshuman
>
> On Tue, 8 Aug 2023 at 08:45, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> 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 <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Changes in V2:
>>
>
> Please ensure that the e-mail title reflects the "v2" nature of this.
> e.g. [PATCH v2] ....
Right, missed that.
>
>
>> - s/treshhold/threshhold
>>
>> Changes in V1:
>>
>> https://lore.kernel.org/all/20230804044720.1478900-1-anshuman.khandual@arm.com/
>>
>> 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..a698b07206b5 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_threshold
>> + - Cycle count threshold value
>>
>> How to use the STM module
>> -------------------------
>
> The doc elements here need to be in a separate patch in the set and
> additionally sent to doc list and maintainer
> (linux-doc@vger.kernel.org and corbet@lwn.net) to be picked up.
Sure, will do that.
>
>> 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..a353c0784bab 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_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
>> + if (cc_threshold) {
>> + if (cc_threshold < 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
>>
>
> As we are using ccitmin - which I think we must - then we need an
> additional patch in the set that implements the errata workaround
> mentioned by Al, when setting ccitmin in etm4_init_arch_data().
Agreed, I am working on that.
> Perhaps a helper function called from there to read the ID reg unless
> one of the errata cores in which case set to 0x4.
Even on the errata cores, first will read the ID register TRCIDR3 to
ascertain CCITMIN field to be 0x100 (256) before overriding with 0x4.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-08-09 4:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-04 4:47 [PATCH] coresight: etm: Make cycle count threshold user configurable Anshuman Khandual
2023-08-04 8:04 ` Al Grant
2023-08-04 8:45 ` Anshuman Khandual
2023-08-04 9:23 ` James Clark
2023-08-04 10:10 ` Al Grant
2023-08-04 11:07 ` James Clark
2023-08-05 3:18 ` kernel test robot
2023-08-08 7:45 Anshuman Khandual
2023-08-08 10:19 ` Mike Leach
2023-08-09 4:42 ` Anshuman Khandual
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®