linux-kernel - Re: [PATCH v4 4/5] arm64/perf: Enable PMCR long cycle counter bit

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: <20160301072106.GA6326@hardcore>
Date:	Tue, 1 Mar 2016 08:21:06 +0100
From:	Jan Glauber <jan.glauber@...iumnetworks.com>
To:	Will Deacon <will.deacon@....com>
CC:	Mark Rutland <mark.rutland@....com>,
	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <marc.zyngier@....com>
Subject: Re: [PATCH v4 4/5] arm64/perf: Enable PMCR long cycle counter bit

On Mon, Feb 29, 2016 at 03:39:35PM +0000, Will Deacon wrote:
> Hi Jan,
> 
> I've queued this lot on my perf/updates branch, but I just noticed an
> oddity whilst dealing with some potential conflicts with the kvm tree.
> 
> On Thu, Feb 18, 2016 at 05:50:13PM +0100, Jan Glauber wrote:
> > With the long cycle counter bit (LC) disabled the cycle counter is not
> > working on ThunderX SOC (ThunderX only implements Aarch64).
> > Also, according to documentation LC == 0 is deprecated.
> > 
> > To keep the code simple the patch does not introduce 64 bit wide counter
> > functions. Instead writing the cycle counter always sets the upper
> > 32 bits so overflow interrupts are generated as before.
> > 
> > Original patch from Andrew Pinksi <Andrew.Pinksi@...iumnetworks.com>
> > 
> > Signed-off-by: Jan Glauber <jglauber@...ium.com>
> > ---
> >  arch/arm64/kernel/perf_event.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > index 0ed05f6..c68fa98 100644
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -405,6 +405,7 @@ static const struct attribute_group *armv8_pmuv3_attr_groups[] = {
> >  #define ARMV8_PMCR_D		(1 << 3) /* CCNT counts every 64th cpu cycle */
> >  #define ARMV8_PMCR_X		(1 << 4) /* Export to ETM */
> >  #define ARMV8_PMCR_DP		(1 << 5) /* Disable CCNT if non-invasive debug*/
> > +#define ARMV8_PMCR_LC		(1 << 6) /* Overflow on 64 bit cycle counter */
> >  #define	ARMV8_PMCR_N_SHIFT	11	 /* Number of counters supported */
> >  #define	ARMV8_PMCR_N_MASK	0x1f
> >  #define	ARMV8_PMCR_MASK		0x3f	 /* Mask for writable bits */
> 
> You haven't extended this mask to cover the LC bit, so it will be ignored
> by armv8pmu_pmcr_write afaict.

This is weird. I've double checked and I missed this mask. Annoying.
Nevertheless it works for me without the LC bit set.

> How did you test this? I can easily update the mask, but it would be
> good to know that it doesn't end up cause a breakage.
 
For testing I used:
- perf top and perf record & report
- looked at interrupt numbers in /proc/interrupts

Without the patch _no_ samples at all are recorded and the interrupt does
not occur. With the patch I get samples and see a reasonable number of
interrupts.

Extending the mask so the LC bit is covered would make sense, I'm going
to test this now.

Jan

> Will