linux-kernel - RE: [PATCH v7 2/4] perf/smmuv3: Add arm64 smmuv3 pmu driver

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]

Date:   Tue, 26 Mar 2019 17:02:26 +0000
From:   Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>
To:     Robin Murphy <robin.murphy@....com>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>
CC:     "andrew.murray@....com" <andrew.murray@....com>,
        "jean-philippe.brucker@....com" <jean-philippe.brucker@....com>,
        "will.deacon@....com" <will.deacon@....com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "Guohanjun (Hanjun Guo)" <guohanjun@...wei.com>,
        "John Garry" <john.garry@...wei.com>,
        "pabba@...eaurora.org" <pabba@...eaurora.org>,
        "vkilari@...eaurora.org" <vkilari@...eaurora.org>,
        "rruigrok@...eaurora.org" <rruigrok@...eaurora.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Linuxarm <linuxarm@...wei.com>,
        "neil.m.leeder@...il.com" <neil.m.leeder@...il.com>
Subject: RE: [PATCH v7 2/4] perf/smmuv3: Add arm64 smmuv3 pmu driver

Hi Robin,

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@....com]
> Sent: 26 March 2019 16:58
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>;
> lorenzo.pieralisi@....com
> Cc: andrew.murray@....com; jean-philippe.brucker@....com;
> will.deacon@....com; mark.rutland@....com; Guohanjun (Hanjun Guo)
> <guohanjun@...wei.com>; John Garry <john.garry@...wei.com>;
> pabba@...eaurora.org; vkilari@...eaurora.org; rruigrok@...eaurora.org;
> linux-acpi@...r.kernel.org; linux-kernel@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; Linuxarm <linuxarm@...wei.com>;
> neil.m.leeder@...il.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 <robin.murphy@....com>
> 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 <robin.murphy@....com>
> ---
>   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