The ETM device can't keep up with the core pipeline when cpu core
is at full speed. This may cause overflow within core and its ETM.
This is a common phenomenon on ETM devices.
On HiSilicon Hip08 platform, a specific feature is added to set
core pipeline. So commit rate can be reduced manually to avoid ETM
overflow.
Signed-off-by: Qi Liu <[email protected]>
---
Change since v1:
- add CONFIG_ETM4X_IMPDEF_FEATURE and CONFIG_ETM4X_IMPDEF_HISILICON
to keep specific feature off platforms which don't use it.
Change since v2:
- remove some unused variable.
Change since v3:
- use read/write_sysreg_s() to access register.
drivers/hwtracing/coresight/Kconfig | 9 +++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 84 ++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-etm4x.h | 12 ++++
3 files changed, 105 insertions(+)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index c119824..1cc3601 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -110,6 +110,15 @@ config CORESIGHT_SOURCE_ETM4X
To compile this driver as a module, choose M here: the
module will be called coresight-etm4x.
+config ETM4X_IMPDEF_FEATURE
+ bool "Control overflow impdef support in CoreSight ETM 4.x driver "
+ depends on CORESIGHT_SOURCE_ETM4X
+ help
+ This control provides overflow implement define for CoreSight
+ ETM 4.x tracer module which could not reduce commit race
+ automatically, and could avoid overflow within ETM tracer module
+ and its cpu core.
+
config CORESIGHT_STM
tristate "CoreSight System Trace Macrocell driver"
depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index abd706b..a1cf215 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -3,6 +3,7 @@
* Copyright (c) 2014, The Linux Foundation. All rights reserved.
*/
+#include <linux/bitops.h>
#include <linux/kernel.h>
#include <linux/moduleparam.h>
#include <linux/init.h>
@@ -28,7 +29,9 @@
#include <linux/perf_event.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
+
#include <asm/sections.h>
+#include <asm/sysreg.h>
#include <asm/local.h>
#include <asm/virt.h>
@@ -103,6 +106,83 @@ struct etm4_enable_arg {
int rc;
};
+#ifdef CONFIG_ETM4X_IMPDEF_FEATURE
+
+#define HISI_HIP08_AMBA_ID 0x000b6d01
+#define ETM4_AMBA_MASK 0xfffff
+#define HISI_HIP08_CORE_COMMIT_CLEAR 0x3000
+#define HISI_HIP08_CORE_COMMIT_SHIFT 12
+#define HISI_HIP08_CORE_COMMIT_REG sys_reg(3, 1, 15, 2, 5)
+
+bool etm4_hisi_match_pid(unsigned int id)
+{
+ return (id & ETM4_AMBA_MASK) == HISI_HIP08_AMBA_ID;
+}
+
+static void etm4_hisi_config_core_commit(bool enable)
+{
+ u64 val;
+
+ val = read_sysreg_s(HISI_HIP08_CORE_COMMIT_REG);
+ val &= ~HISI_HIP08_CORE_COMMIT_CLEAR;
+ val |= enable << HISI_HIP08_CORE_COMMIT_SHIFT;
+ write_sysreg_s(val, HISI_HIP08_CORE_COMMIT_REG);
+}
+
+static struct etm4_arch_features etm4_features[] = {
+ [ETM4_IMPDEF_HISI_CORE_COMMIT] = {
+ .set_commit = etm4_hisi_config_core_commit,
+ },
+ {},
+};
+
+static void etm4_enable_arch_specific(struct etmv4_drvdata *drvdata)
+{
+ struct etm4_arch_features *ftr;
+ int bit;
+
+ for_each_set_bit(bit, drvdata->arch_features, ETM4_IMPDEF_FEATURE_MAX) {
+ ftr = &etm4_features[bit];
+
+ if (ftr->set_commit)
+ ftr->set_commit(true);
+ }
+}
+
+static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata)
+{
+ struct etm4_arch_features *ftr;
+ int bit;
+
+ for_each_set_bit(bit, drvdata->arch_features, ETM4_IMPDEF_FEATURE_MAX) {
+ ftr = &etm4_features[bit];
+
+ if (ftr->set_commit)
+ ftr->set_commit(false);
+ }
+}
+
+static void etm4x_check_arch_features(struct etmv4_drvdata *drvdata,
+ unsigned int id)
+{
+ if (etm4_hisi_match_pid(id))
+ set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features);
+}
+#else
+static void etm4_enable_arch_specific(struct etmv4_drvdata *drvdata)
+{
+}
+
+static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata)
+{
+}
+
+static void etm4_check_arch_features(struct etmv4_drvdata drvdata,
+ unsigned int id)
+{
+}
+#endif /* CONFIG_ETM4X_IMPDEF_FEATURE */
+
static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
{
int i, rc;
@@ -110,6 +190,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
struct device *etm_dev = &drvdata->csdev->dev;
CS_UNLOCK(drvdata->base);
+ etm4_enable_arch_specific(drvdata);
etm4_os_unlock(drvdata);
@@ -476,6 +557,7 @@ static void etm4_disable_hw(void *info)
int i;
CS_UNLOCK(drvdata->base);
+ etm4_disable_arch_specific(drvdata);
if (!drvdata->skip_power_up) {
/* power can be removed from the trace unit now */
@@ -1547,6 +1629,8 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
drvdata->boot_enable = true;
}
+ etm4x_check_arch_features(drvdata, id->id);
+
return 0;
}
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index eefc737..1784975 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -8,6 +8,7 @@
#include <asm/local.h>
#include <linux/spinlock.h>
+#include <linux/types.h>
#include "coresight-priv.h"
/*
@@ -203,6 +204,11 @@
/* Interpretation of resource numbers change at ETM v4.3 architecture */
#define ETM4X_ARCH_4V3 0x43
+enum etm_impdef_type {
+ ETM4_IMPDEF_HISI_CORE_COMMIT,
+ ETM4_IMPDEF_FEATURE_MAX,
+};
+
/**
* struct etmv4_config - configuration information related to an ETMv4
* @mode: Controls various modes supported by this ETM.
@@ -415,6 +421,7 @@ struct etmv4_save_state {
* @state_needs_restore: True when there is context to restore after PM exit
* @skip_power_up: Indicates if an implementation can skip powering up
* the trace unit.
+ * @arch_features: Bitmap of arch features of etmv4 devices.
*/
struct etmv4_drvdata {
void __iomem *base;
@@ -463,6 +470,11 @@ struct etmv4_drvdata {
struct etmv4_save_state *save_state;
bool state_needs_restore;
bool skip_power_up;
+ DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);
+};
+
+struct etm4_arch_features {
+ void (*set_commit)(bool enable);
};
/* Address comparator access types */
--
2.8.1
Hi Qi
Thanks for the changes. Mostly looks good to me, except for the
name of the call back.
On 11/23/20 1:29 PM, Qi Liu wrote:
> The ETM device can't keep up with the core pipeline when cpu core
> is at full speed. This may cause overflow within core and its ETM.
> This is a common phenomenon on ETM devices.
>
> On HiSilicon Hip08 platform, a specific feature is added to set
> core pipeline. So commit rate can be reduced manually to avoid ETM
> overflow.
>
> Signed-off-by: Qi Liu <[email protected]>
> ---
> Change since v1:
> - add CONFIG_ETM4X_IMPDEF_FEATURE and CONFIG_ETM4X_IMPDEF_HISILICON
> to keep specific feature off platforms which don't use it.
> Change since v2:
> - remove some unused variable.
> Change since v3:
> - use read/write_sysreg_s() to access register.
>
> drivers/hwtracing/coresight/Kconfig | 9 +++
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 84 ++++++++++++++++++++++
> drivers/hwtracing/coresight/coresight-etm4x.h | 12 ++++
> 3 files changed, 105 insertions(+)
>
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index eefc737..1784975 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -8,6 +8,7 @@
>
> #include <asm/local.h>
> #include <linux/spinlock.h>
> +#include <linux/types.h>
> #include "coresight-priv.h"
>
> /*
> @@ -203,6 +204,11 @@
> /* Interpretation of resource numbers change at ETM v4.3 architecture */
> #define ETM4X_ARCH_4V3 0x43
>
> +enum etm_impdef_type {
> + ETM4_IMPDEF_HISI_CORE_COMMIT,
> + ETM4_IMPDEF_FEATURE_MAX,
> +};
> +
> /**
> * struct etmv4_config - configuration information related to an ETMv4
> * @mode: Controls various modes supported by this ETM.
> @@ -415,6 +421,7 @@ struct etmv4_save_state {
> * @state_needs_restore: True when there is context to restore after PM exit
> * @skip_power_up: Indicates if an implementation can skip powering up
> * the trace unit.
> + * @arch_features: Bitmap of arch features of etmv4 devices.
> */
> struct etmv4_drvdata {
> void __iomem *base;
> @@ -463,6 +470,11 @@ struct etmv4_drvdata {
> struct etmv4_save_state *save_state;
> bool state_needs_restore;
> bool skip_power_up;
> + DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);
> +};
> +
> +struct etm4_arch_features {
> + void (*set_commit)(bool enable);
The set_commit is too hisilicon specific :-). Could we please rename
this to soemthing more generic. The callback for hisilicon etms, could still
be xx_commit". May be simply call it
callback() ?
or may be even
arch_callback() ?
> };
nit: This need not be part of the header file, as it is not used
outside the etm4x-core.c
Suzuki
Hi Qi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.10-rc5 next-20201123]
[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]
url: https://github.com/0day-ci/linux/commits/Qi-Liu/coresight-etm4x-Modify-core-commit-of-cpu-to-avoid-the-overflow-of-HiSilicon-ETM/20201123-213732
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 418baf2c28f3473039f2f7377760bd8f6897ae18
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e21b077bb0d120583deb7cd6f1654d7c356175af
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Qi-Liu/coresight-etm4x-Modify-core-commit-of-cpu-to-avoid-the-overflow-of-HiSilicon-ETM/20201123-213732
git checkout e21b077bb0d120583deb7cd6f1654d7c356175af
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/hwtracing/coresight/coresight-etm4x-core.c:117:6: warning: no previous prototype for 'etm4_hisi_match_pid' [-Wmissing-prototypes]
117 | bool etm4_hisi_match_pid(unsigned int id)
| ^~~~~~~~~~~~~~~~~~~
vim +/etm4_hisi_match_pid +117 drivers/hwtracing/coresight/coresight-etm4x-core.c
116
> 117 bool etm4_hisi_match_pid(unsigned int id)
118 {
119 return (id & ETM4_AMBA_MASK) == HISI_HIP08_AMBA_ID;
120 }
121
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On 2020/11/23 22:12, Suzuki K Poulose wrote:
> Hi Qi
>
> Thanks for the changes. Mostly looks good to me, except for the
> name of the call back.
>
>
Hi Suzuki,
ok, I'll send a new patch to change the name of call back.
Thanks
Qi
> On 11/23/20 1:29 PM, Qi Liu wrote:
>> The ETM device can't keep up with the core pipeline when cpu core
>> is at full speed. This may cause overflow within core and its ETM.
>> This is a common phenomenon on ETM devices.
>>
>> On HiSilicon Hip08 platform, a specific feature is added to set
>> core pipeline. So commit rate can be reduced manually to avoid ETM
>> overflow.
>>
>> Signed-off-by: Qi Liu <[email protected]>
>
>
>> ---
>> Change since v1:
>> - add CONFIG_ETM4X_IMPDEF_FEATURE and CONFIG_ETM4X_IMPDEF_HISILICON
>> to keep specific feature off platforms which don't use it.
>> Change since v2:
>> - remove some unused variable.
>> Change since v3:
>> - use read/write_sysreg_s() to access register.
>>
>> drivers/hwtracing/coresight/Kconfig | 9 +++
>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 84 ++++++++++++++++++++++
>> drivers/hwtracing/coresight/coresight-etm4x.h | 12 ++++
>> 3 files changed, 105 insertions(+)
>>
>
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index eefc737..1784975 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -8,6 +8,7 @@
>>
>> #include <asm/local.h>
>> #include <linux/spinlock.h>
>> +#include <linux/types.h>
>> #include "coresight-priv.h"
>>
>> /*
>> @@ -203,6 +204,11 @@
>> /* Interpretation of resource numbers change at ETM v4.3 architecture */
>> #define ETM4X_ARCH_4V3 0x43
>>
>> +enum etm_impdef_type {
>> + ETM4_IMPDEF_HISI_CORE_COMMIT,
>> + ETM4_IMPDEF_FEATURE_MAX,
>> +};
>> +
>> /**
>> * struct etmv4_config - configuration information related to an ETMv4
>> * @mode: Controls various modes supported by this ETM.
>> @@ -415,6 +421,7 @@ struct etmv4_save_state {
>> * @state_needs_restore: True when there is context to restore after PM exit
>> * @skip_power_up: Indicates if an implementation can skip powering up
>> * the trace unit.
>> + * @arch_features: Bitmap of arch features of etmv4 devices.
>> */
>> struct etmv4_drvdata {
>> void __iomem *base;
>> @@ -463,6 +470,11 @@ struct etmv4_drvdata {
>> struct etmv4_save_state *save_state;
>> bool state_needs_restore;
>> bool skip_power_up;
>> + DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);
>> +};
>> +
>> +struct etm4_arch_features {
>> + void (*set_commit)(bool enable);
>
> The set_commit is too hisilicon specific :-). Could we please rename
> this to soemthing more generic. The callback for hisilicon etms, could still
> be xx_commit". May be simply call it
>
> callback() ?
>
> or may be even
> arch_callback() ?
>
>
>> };
>
> nit: This need not be part of the header file, as it is not used
> outside the etm4x-core.c
>
> Suzuki
>
> .
>