Re: [PATCH v7 2/4] perf/smmuv3: Add arm64 smmuv3 pmu driver
On Tue, Mar 26, 2019 at 03:17:51PM +, Shameer Kolothum wrote:
> From: Neil Leeder
>
> Adds a new driver to support the SMMUv3 PMU and add it into the
> perf events framework.
>
> Each SMMU node may have multiple PMUs associated with it, each of
> which may support different events.
>
> SMMUv3 PMCG devices are named as smmuv3_pmcg_ where
> is the physical page address of the SMMU PMCG
> wrapped to 4K boundary. For example, the PMCG at 0xff8884 is
> named smmuv3_pmcg_ff88840
>
> Filtering by stream id is done by specifying filtering parameters
> with the event. options are:
>filter_enable- 0 = no filtering, 1 = filtering enabled
>filter_span - 0 = exact match, 1 = pattern match
>filter_stream_id - pattern to filter against
>
> Example: perf stat -e smmuv3_pmcg_ff88840/transaction,filter_enable=1,
>filter_span=1,filter_stream_id=0x42/ -a netperf
>
> Applies filter pattern 0x42 to transaction events, which means events
> matching stream ids 0x42 & 0x43 are counted as only upper StreamID
> bits are required to match the given filter. Further filtering
> information is available in the SMMU documentation.
>
> SMMU events are not attributable to a CPU, so task mode and sampling
> are not supported.
>
> Signed-off-by: Neil Leeder
> Signed-off-by: Shameer Kolothum
> Reviewed-by: Robin Murphy
> ---
> drivers/perf/Kconfig | 9 +
> drivers/perf/Makefile | 1 +
> drivers/perf/arm_smmuv3_pmu.c | 776
> ++
> 3 files changed, 786 insertions(+)
> create mode 100644 drivers/perf/arm_smmuv3_pmu.c
[...]
> +static int smmu_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct smmu_pmu *smmu_pmu;
> + unsigned int target;
> +
> + smmu_pmu = hlist_entry_safe(node, struct smmu_pmu, node);
> + if (cpu != smmu_pmu->on_cpu)
> + return 0;
> +
> + target = cpumask_any_but(cpu_online_mask, cpu);
> + if (target >= nr_cpu_ids)
> + return 0;
> +
> + perf_pmu_migrate_context(_pmu->pmu, cpu, target);
> + smmu_pmu->on_cpu = target;
> + WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(target)));
I'm going to make this (and the other invocation) use
irq_set_affinity_hint() instead, so that we can build this driver as a
module with the appropriate Kconfig tweak.
Will
RE: [PATCH v7 2/4] perf/smmuv3: Add arm64 smmuv3 pmu driver
Hi Robin,
> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 26 March 2019 16:58
> To: Shameerali Kolothum Thodi ;
> lorenzo.pieral...@arm.com
> Cc: andrew.mur...@arm.com; jean-philippe.bruc...@arm.com;
> will.dea...@arm.com; mark.rutl...@arm.com; Guohanjun (Hanjun Guo)
> ; John Garry ;
> pa...@codeaurora.org; vkil...@codeaurora.org; rruig...@codeaurora.org;
> linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; Linuxarm ;
> neil.m.lee...@gmail.com
> Subject: Re: [PATCH v7 2/4] perf/smmuv3: Add arm64 smmuv3 pmu driver
>
> Hi Shameer,
>
> On 26/03/2019 15:17, Shameer Kolothum wrote:
> [...]
> > +static int smmu_pmu_apply_event_filter(struct smmu_pmu *smmu_pmu,
> > + struct perf_event *event, int idx)
> > +{
> > + u32 span, sid;
> > + unsigned int num_ctrs = smmu_pmu->num_counters;
> > + bool filter_en = !!get_filter_enable(event);
> > +
> > + span = filter_en ? get_filter_span(event) :
> > + SMMU_PMCG_DEFAULT_FILTER_SPAN;
> > + sid = filter_en ? get_filter_stream_id(event) :
> > + SMMU_PMCG_DEFAULT_FILTER_SID;
> > +
> > + /* Support individual filter settings */
> > + if (!smmu_pmu->global_filter) {
> > + smmu_pmu_set_event_filter(event, idx, span, sid);
> > + return 0;
> > + }
> > +
> > + /* Requested settings same as current global settings*/
> > + if (span == smmu_pmu->global_filter_span &&
> > + sid == smmu_pmu->global_filter_sid)
> > + return 0;
> > +
> > + if (!bitmap_empty(smmu_pmu->used_counters, num_ctrs))
> > + return -EAGAIN;
> > +
> > + if (idx == 0) {
> > + smmu_pmu_set_event_filter(event, idx, span, sid);
> > + smmu_pmu->global_filter_span = span;
> > + smmu_pmu->global_filter_sid = sid;
> > + return 0;
> > + }
>
> When I suggested dropping the check of idx, I did mean removing it
> entirely, not just moving it further down ;)
Ah..I must confess that I was slightly confused by that suggestion and
thought that you are making a case for code being more clear to read :)
> Nothing to worry about though, I'll just leave this here for Will to
> consider applying on top or squashing.
Thanks for that.
Cheers,
Shameer
> Thanks,
> Robin.
>
> ->8-
> From: Robin Murphy
> Subject: [PATCH] perf/smmuv3: Relax global filter constraint a little
>
> Although the current behaviour of smmu_pmu_get_event_idx() effectively
> ensures that the first-allocated counter will be counter 0, there's no
> need to strictly enforce that in smmu_pmu_apply_event_filter(). All that
> matters is that we only ever touch the global filter settings in
> SMMU_PMCG_SMR0 and SMMU_PMCG_EVTYPER0 while no counters are
> active.
>
> Signed-off-by: Robin Murphy
> ---
> drivers/perf/arm_smmuv3_pmu.c | 11 ---
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/perf/arm_smmuv3_pmu.c
> b/drivers/perf/arm_smmuv3_pmu.c
> index 6b3c0ed7ad71..23045ead6de1 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -286,14 +286,11 @@ static int smmu_pmu_apply_event_filter(struct
> smmu_pmu *smmu_pmu,
> if (!bitmap_empty(smmu_pmu->used_counters, num_ctrs))
> return -EAGAIN;
>
> - if (idx == 0) {
> - smmu_pmu_set_event_filter(event, idx, span, sid);
> - smmu_pmu->global_filter_span = span;
> - smmu_pmu->global_filter_sid = sid;
> - return 0;
> - }
> + smmu_pmu_set_event_filter(event, 0, span, sid);
> + smmu_pmu->global_filter_span = span;
> + smmu_pmu->global_filter_sid = sid;
>
> - return -EAGAIN;
> + return 0;
> }
>
> static int smmu_pmu_get_event_idx(struct smmu_pmu *smmu_pmu,
> --
> 2.20.1.dirty
Re: [PATCH v7 2/4] perf/smmuv3: Add arm64 smmuv3 pmu driver
Hi Shameer,
On 26/03/2019 15:17, Shameer Kolothum wrote:
[...]
+static int smmu_pmu_apply_event_filter(struct smmu_pmu *smmu_pmu,
+ struct perf_event *event, int idx)
+{
+ u32 span, sid;
+ unsigned int num_ctrs = smmu_pmu->num_counters;
+ bool filter_en = !!get_filter_enable(event);
+
+ span = filter_en ? get_filter_span(event) :
+ SMMU_PMCG_DEFAULT_FILTER_SPAN;
+ sid = filter_en ? get_filter_stream_id(event) :
+ SMMU_PMCG_DEFAULT_FILTER_SID;
+
+ /* Support individual filter settings */
+ if (!smmu_pmu->global_filter) {
+ smmu_pmu_set_event_filter(event, idx, span, sid);
+ return 0;
+ }
+
+ /* Requested settings same as current global settings*/
+ if (span == smmu_pmu->global_filter_span &&
+ sid == smmu_pmu->global_filter_sid)
+ return 0;
+
+ if (!bitmap_empty(smmu_pmu->used_counters, num_ctrs))
+ return -EAGAIN;
+
+ if (idx == 0) {
+ smmu_pmu_set_event_filter(event, idx, span, sid);
+ smmu_pmu->global_filter_span = span;
+ smmu_pmu->global_filter_sid = sid;
+ return 0;
+ }
When I suggested dropping the check of idx, I did mean removing it
entirely, not just moving it further down ;)
Nothing to worry about though, I'll just leave this here for Will to
consider applying on top or squashing.
Thanks,
Robin.
->8-
From: Robin Murphy
Subject: [PATCH] perf/smmuv3: Relax global filter constraint a little
Although the current behaviour of smmu_pmu_get_event_idx() effectively
ensures that the first-allocated counter will be counter 0, there's no
need to strictly enforce that in smmu_pmu_apply_event_filter(). All that
matters is that we only ever touch the global filter settings in
SMMU_PMCG_SMR0 and SMMU_PMCG_EVTYPER0 while no counters are active.
Signed-off-by: Robin Murphy
---
drivers/perf/arm_smmuv3_pmu.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index 6b3c0ed7ad71..23045ead6de1 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -286,14 +286,11 @@ static int smmu_pmu_apply_event_filter(struct
smmu_pmu *smmu_pmu,
if (!bitmap_empty(smmu_pmu->used_counters, num_ctrs))
return -EAGAIN;
- if (idx == 0) {
- smmu_pmu_set_event_filter(event, idx, span, sid);
- smmu_pmu->global_filter_span = span;
- smmu_pmu->global_filter_sid = sid;
- return 0;
- }
+ smmu_pmu_set_event_filter(event, 0, span, sid);
+ smmu_pmu->global_filter_span = span;
+ smmu_pmu->global_filter_sid = sid;
- return -EAGAIN;
+ return 0;
}
static int smmu_pmu_get_event_idx(struct smmu_pmu *smmu_pmu,
--
2.20.1.dirty
