* [PATCH] coresight: etm4x: Ensure default perf settings filter user/kernel
@ 2020-08-18 21:43 Mike Leach
2020-08-19 2:11 ` Leo Yan
0 siblings, 1 reply; 3+ messages in thread
From: Mike Leach @ 2020-08-18 21:43 UTC (permalink / raw)
To: leo.yan, linux-arm-kernel, coresight, mathieu.poirier
Cc: Mike Leach, suzuki.poulose
Moving from using an address filter to trace the default "all addresses"
range to no filtering to acheive the same result, has caused the perf
filtering of kernel/user address spaces from not working unless an
explicit address filter was used.
This due to the global TRCVICTLR exception level filtering not being set
correctly.
This patch fixes this.
Reported-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
drivers/hwtracing/coresight/coresight-etm4x.c | 11 +++++++++++
drivers/hwtracing/coresight/coresight-etm4x.h | 3 +++
2 files changed, 14 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 8b0634ebef77..d0f800753590 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -52,6 +52,7 @@ static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
static void etm4_set_default_config(struct etmv4_config *config);
static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
struct perf_event *event);
+static u64 etm4_get_access_type(struct etmv4_config *config);
static enum cpuhp_state hp_online;
@@ -785,6 +786,8 @@ static void etm4_init_arch_data(void *info)
static void etm4_set_default_config(struct etmv4_config *config)
{
+ u64 access_type;
+
/* disable all events tracing */
config->eventctrl0 = 0x0;
config->eventctrl1 = 0x0;
@@ -800,6 +803,14 @@ static void etm4_set_default_config(struct etmv4_config *config)
/* TRCVICTLR::EVENT = 0x01, select the always on logic */
config->vinst_ctrl = BIT(0);
+
+ /*
+ * TRCVICTLR::EXLEVEL_NS:EXLEVELS: Set kernel / user filtering
+ * bits in vinst, same bit pattern as address comparator acc values
+ * but offset in this register
+ */
+ access_type = etm4_get_access_type(config) << ETM_EXLEVEL_VINST_OFFSET;
+ config->vinst_ctrl |= (u32)access_type;
}
static u64 etm4_get_ns_access_type(struct etmv4_config *config)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index b8283e1d6d88..369abc25c597 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -192,6 +192,9 @@
#define ETM_EXLEVEL_NS_HYP BIT(14)
#define ETM_EXLEVEL_NS_NA BIT(15)
+/* access level control in VINST - same bits as TRCACATRn but offset */
+#define ETM_EXLEVEL_VINST_OFFSET 8
+
/* secure / non secure masks - TRCVICTLR, IDR3 */
#define ETM_EXLEVEL_S_VICTLR_MASK GENMASK(19, 16)
/* NS MON (EL3) mode never implemented */
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] coresight: etm4x: Ensure default perf settings filter user/kernel
2020-08-18 21:43 [PATCH] coresight: etm4x: Ensure default perf settings filter user/kernel Mike Leach
@ 2020-08-19 2:11 ` Leo Yan
2020-08-19 17:19 ` Mike Leach
0 siblings, 1 reply; 3+ messages in thread
From: Leo Yan @ 2020-08-19 2:11 UTC (permalink / raw)
To: Mike Leach; +Cc: coresight, mathieu.poirier, linux-arm-kernel, suzuki.poulose
Hi Mike,
On Tue, Aug 18, 2020 at 10:43:12PM +0100, Mike Leach wrote:
> Moving from using an address filter to trace the default "all addresses"
> range to no filtering to acheive the same result, has caused the perf
> filtering of kernel/user address spaces from not working unless an
> explicit address filter was used.
>
> This due to the global TRCVICTLR exception level filtering not being set
> correctly.
>
> This patch fixes this.
Very cool!
> Reported-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-etm4x.c | 11 +++++++++++
> drivers/hwtracing/coresight/coresight-etm4x.h | 3 +++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index 8b0634ebef77..d0f800753590 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -52,6 +52,7 @@ static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
> static void etm4_set_default_config(struct etmv4_config *config);
> static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
> struct perf_event *event);
> +static u64 etm4_get_access_type(struct etmv4_config *config);
>
> static enum cpuhp_state hp_online;
>
> @@ -785,6 +786,8 @@ static void etm4_init_arch_data(void *info)
>
> static void etm4_set_default_config(struct etmv4_config *config)
> {
> + u64 access_type;
> +
> /* disable all events tracing */
> config->eventctrl0 = 0x0;
> config->eventctrl1 = 0x0;
> @@ -800,6 +803,14 @@ static void etm4_set_default_config(struct etmv4_config *config)
>
> /* TRCVICTLR::EVENT = 0x01, select the always on logic */
> config->vinst_ctrl = BIT(0);
> +
> + /*
> + * TRCVICTLR::EXLEVEL_NS:EXLEVELS: Set kernel / user filtering
> + * bits in vinst, same bit pattern as address comparator acc values
> + * but offset in this register
> + */
> + access_type = etm4_get_access_type(config) << ETM_EXLEVEL_VINST_OFFSET;
> + config->vinst_ctrl |= (u32)access_type;
Just a minor comment, here the definition for ETM_EXLEVEL_VINST_OFFSET
is easily to introduce confusion. In the register TRCACATRn,
EXLEVEL_NS:EXLEVELS offset is 8, and in the registser TRCVICTLR the
EXLEVEL_NS:EXLEVELS offset is 16. So for more explict expression, we
need to define below two macros:
#define ETM_EXLEVEL_TRCVICTLR_OFFSET 16
#define ETM_EXLEVEL_TRCACATRn_OFFSET 8
access_type = etm4_get_access_type(config) >> ETM_EXLEVEL_TRCACATRn_OFFSET;
access_type = access_type << ETM_EXLEVEL_VINST_OFFSET;
config->vinst_ctrl |= (u32)access_type;
Otherwise, the change is good for me:
Reviewed-by: Leo Yan <leo.yan@linaro.org>
Tested-by: Leo Yan <leo.yan@linaro.org>
And thanks a lot for the quick fixing!
Leo
> static u64 etm4_get_ns_access_type(struct etmv4_config *config)
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index b8283e1d6d88..369abc25c597 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -192,6 +192,9 @@
> #define ETM_EXLEVEL_NS_HYP BIT(14)
> #define ETM_EXLEVEL_NS_NA BIT(15)
>
> +/* access level control in VINST - same bits as TRCACATRn but offset */
> +#define ETM_EXLEVEL_VINST_OFFSET 8
> +
> /* secure / non secure masks - TRCVICTLR, IDR3 */
> #define ETM_EXLEVEL_S_VICTLR_MASK GENMASK(19, 16)
> /* NS MON (EL3) mode never implemented */
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] coresight: etm4x: Ensure default perf settings filter user/kernel
2020-08-19 2:11 ` Leo Yan
@ 2020-08-19 17:19 ` Mike Leach
0 siblings, 0 replies; 3+ messages in thread
From: Mike Leach @ 2020-08-19 17:19 UTC (permalink / raw)
To: Leo Yan; +Cc: Coresight ML, Mathieu Poirier, linux-arm-kernel,
Suzuki K. Poulose
Hi Leo
On Wed, 19 Aug 2020 at 03:11, Leo Yan <leo.yan@linaro.org> wrote:
>
> Hi Mike,
>
> On Tue, Aug 18, 2020 at 10:43:12PM +0100, Mike Leach wrote:
> > Moving from using an address filter to trace the default "all addresses"
> > range to no filtering to acheive the same result, has caused the perf
> > filtering of kernel/user address spaces from not working unless an
> > explicit address filter was used.
> >
> > This due to the global TRCVICTLR exception level filtering not being set
> > correctly.
> >
> > This patch fixes this.
>
> Very cool!
>
> > Reported-by: Leo Yan <leo.yan@linaro.org>
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > ---
> > drivers/hwtracing/coresight/coresight-etm4x.c | 11 +++++++++++
> > drivers/hwtracing/coresight/coresight-etm4x.h | 3 +++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> > index 8b0634ebef77..d0f800753590 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> > @@ -52,6 +52,7 @@ static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
> > static void etm4_set_default_config(struct etmv4_config *config);
> > static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
> > struct perf_event *event);
> > +static u64 etm4_get_access_type(struct etmv4_config *config);
> >
> > static enum cpuhp_state hp_online;
> >
> > @@ -785,6 +786,8 @@ static void etm4_init_arch_data(void *info)
> >
> > static void etm4_set_default_config(struct etmv4_config *config)
> > {
> > + u64 access_type;
> > +
> > /* disable all events tracing */
> > config->eventctrl0 = 0x0;
> > config->eventctrl1 = 0x0;
> > @@ -800,6 +803,14 @@ static void etm4_set_default_config(struct etmv4_config *config)
> >
> > /* TRCVICTLR::EVENT = 0x01, select the always on logic */
> > config->vinst_ctrl = BIT(0);
> > +
> > + /*
> > + * TRCVICTLR::EXLEVEL_NS:EXLEVELS: Set kernel / user filtering
> > + * bits in vinst, same bit pattern as address comparator acc values
> > + * but offset in this register
> > + */
> > + access_type = etm4_get_access_type(config) << ETM_EXLEVEL_VINST_OFFSET;
> > + config->vinst_ctrl |= (u32)access_type;
>
> Just a minor comment, here the definition for ETM_EXLEVEL_VINST_OFFSET
> is easily to introduce confusion. In the register TRCACATRn,
> EXLEVEL_NS:EXLEVELS offset is 8, and in the registser TRCVICTLR the
> EXLEVEL_NS:EXLEVELS offset is 16. So for more explict expression, we
> need to define below two macros:
>
> #define ETM_EXLEVEL_TRCVICTLR_OFFSET 16
> #define ETM_EXLEVEL_TRCACATRn_OFFSET 8
>
> access_type = etm4_get_access_type(config) >> ETM_EXLEVEL_TRCACATRn_OFFSET;
> access_type = access_type << ETM_EXLEVEL_VINST_OFFSET;
> config->vinst_ctrl |= (u32)access_type;
>
> Otherwise, the change is good for me:
>
> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> Tested-by: Leo Yan <leo.yan@linaro.org>
>
Thanks for the review and test. I am not sure the above makes things
clearer. Looking at it, I think a better comment and more consistent
use of the TRCVICTLR name would make things better.
I 've spotted a second instance where this needs to be set, so the
next spin will have a helper fn updating config->vinst_ctrl.
Regards
Mike
> And thanks a lot for the quick fixing!
>
> Leo
>
> > static u64 etm4_get_ns_access_type(struct etmv4_config *config)
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> > index b8283e1d6d88..369abc25c597 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> > @@ -192,6 +192,9 @@
> > #define ETM_EXLEVEL_NS_HYP BIT(14)
> > #define ETM_EXLEVEL_NS_NA BIT(15)
> >
> > +/* access level control in VINST - same bits as TRCACATRn but offset */
> > +#define ETM_EXLEVEL_VINST_OFFSET 8
>
>
> > +
> > /* secure / non secure masks - TRCVICTLR, IDR3 */
> > #define ETM_EXLEVEL_S_VICTLR_MASK GENMASK(19, 16)
> > /* NS MON (EL3) mode never implemented */
> > --
> > 2.17.1
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-19 17:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-18 21:43 [PATCH] coresight: etm4x: Ensure default perf settings filter user/kernel Mike Leach
2020-08-19 2:11 ` Leo Yan
2020-08-19 17:19 ` Mike Leach
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox