linux-kernel - Re: [PATCH v2 8/8] drivers/perf: Add Apple icestorm/firestorm CPU 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:   Fri, 3 Dec 2021 12:04:35 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Marc Zyngier <maz@...nel.org>
Cc:     linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, Will Deacon <will@...nel.org>,
        Hector Martin <marcan@...can.st>,
        Sven Peter <sven@...npeter.dev>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        Rob Herring <robh+dt@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Dougall <dougallj@...il.com>, kernel-team@...roid.com
Subject: Re: [PATCH v2 8/8] drivers/perf: Add Apple icestorm/firestorm CPU
 PMU driver

On Fri, Dec 03, 2021 at 11:22:53AM +0000, Marc Zyngier wrote:
> On Thu, 02 Dec 2021 16:14:01 +0000, Mark Rutland <mark.rutland@....com> wrote:
> > On Thu, Dec 02, 2021 at 03:39:46PM +0000, Marc Zyngier wrote:
> > > On Wed, 01 Dec 2021 16:58:10 +0000, Mark Rutland <mark.rutland@....com> wrote:
> > > > On Wed, Dec 01, 2021 at 01:49:09PM +0000, Marc Zyngier wrote:
> > > > > +	state = read_sysreg_s(SYS_IMP_APL_PMCR0_EL1);
> > > > > +	overflow = read_sysreg_s(SYS_IMP_APL_PMSR_EL1);
> > > > 
> > > > I assume the overflow behaviour is free-running rather than stopping?
> > > 
> > > Configurable, apparently. At the moment, I set it to stop on overflow.
> > > Happy to change the behaviour though.
> > 
> > The architected PMU continues counting upon overflow (which prevents
> > losing counts around the overlflow occurring), so I'd prefer that.
> > 
> > Is that behaviour per-counter, or for the PMU as a whole?
> 
> It is global. This will probably require some additional rework to
> clear bit 47 in overflowing counters, which we can't do atomically.

Ah; I see.

To calrify my comment above, the reason for wanting the counter to keep
counting is to count during the window between the IRQ being asserted and the
PMU IRQ handler being invoked, and it's fine for there to be a blackout period
*within* the PMU IRQ handler.

So for example it would be fine to have:

	irq_handler() 
	{
		if (!any_counter_overflowed())
			return IRQ_NONE;

		stop_all_counters();

		for_each_counter(c) {
			handle_counter(c);
		}
		
		start_all_counters();

		return IRQ_HANDLED;

	}

... and I think with that the regular per-counter period reprogramming would do
the right thing?

Really, all the PMU drivers should do that so that repgoramming is consistent
and we don't get skewed groups.

Thanks,
Mark.