linux-kernel - Re: [PATCH] coresight: etm4x: Modify core-commit of cpu to avoid the overflow of HiSilicon ETM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cfa91a72-6e01-85ce-583a-9a49093a875b@arm.com>
Date:   Mon, 23 Nov 2020 14:12:54 +0000
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     Qi Liu <liuqi115@...wei.com>, mathieu.poirier@...aro.org,
        mike.leach@...aro.org
Cc:     coresight@...ts.linaro.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linuxarm@...wei.com
Subject: Re: [PATCH] coresight: etm4x: Modify core-commit of cpu to avoid the
 overflow of HiSilicon ETM

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@...wei.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