diff mbox series

coresight: etm4x: Modify core-commit of cpu to avoid the overflow of HiSilicon ETM

Message ID 1606138167-8076-1-git-send-email-liuqi115@huawei.com (mailing list archive)
State New, archived
Headers show
Series coresight: etm4x: Modify core-commit of cpu to avoid the overflow of HiSilicon ETM | expand

Commit Message

liuqi (BA) Nov. 23, 2020, 1:29 p.m. UTC
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 <liuqi115@huawei.com>
---
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(+)

--
2.8.1

Comments

Suzuki K Poulose Nov. 23, 2020, 2:12 p.m. UTC | #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 <liuqi115@huawei.com>


> ---
> 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
liuqi (BA) Nov. 26, 2020, 1:38 p.m. UTC | #2
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 <liuqi115@huawei.com>
> 
> 
>> ---
>> 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
> 
> .
>
diff mbox series

Patch

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 */