Re: [PATCH] driver/perf: Add PMU driver for the ARM DMC-620 memory controller.

From: Mark Rutland <mark.rutland@arm.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>,
	Tuan Phan <tuanphan@os.amperecomputing.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Tuan Phan <tuanphan@amperemail.onmicrosoft.com>
Subject: Re: [PATCH] driver/perf: Add PMU driver for the ARM DMC-620 memory controller.
Date: Wed, 1 Apr 2020 12:27:39 +0100	[thread overview]
Message-ID: <20200401112739.GD17163@C02TD0UTHF1T.local> (raw)
In-Reply-To: <4d843ec7-ed74-4431-d8c7-d5aa6bd83c18@arm.com>

On Wed, Apr 01, 2020 at 12:12:23PM +0100, Robin Murphy wrote:
> On 2020-04-01 11:27 am, Will Deacon wrote:
> > On Wed, Apr 01, 2020 at 10:52:26AM +0100, Mark Rutland wrote:
> > > On Tue, Mar 31, 2020 at 03:14:59PM -0700, Tuan Phan wrote:
> > > > I looked at the SMMUv3 PMU driver and it also uses IRQF_SHARED. SMMUv3
> > > > PMU and DMC620 PMU are very much similar in which counters can be
> > > > accessed by any cores using memory map. Any special reasons
> > > > IRQF_SHARED works with SMMUv3 PMU driver?
> > > 
> > > No; I believe that is a bug in the SMMUv3 PMU driver. If the IRQ were
> > > shared, and another driver that held the IRQ changed the affinity,
> > > things would go very wrong.
> > 
> > I *think* the idea is that the SMMUv3 PMU driver manages multiple PMCG
> > devices, which may all share an irq line, rather than the irq line being
> > shared by some other driver that might change the affinity. So I suspect
> > dropping IRQF_SHARED will break things.
> 
> Each PMCG is conceptually a distinct PMU with its own interrupt - for
> instance, MMU-600 has one PMCG for its TCU and one for each TBU, each with a
> distinct interrupt output signal. Of course, integrators can and will mash
> them all together into a single SPI (particularly since they're all part of
> "the SMMU"), but that boils down to the same case as here.
> 
> This is going to continue to happen, so we could really do with figuring out
> a way to let MMIO system PMU drivers properly cope with shared interrupts in
> general :/

It does seem so, but I think we can only reasonably do that where it's
only being shared across instances of the same driver, rather than when
the IRQ is muxed across completely independent drivers. I'd like to
avoid that latter case if we can.

The driver would have to handle migration on a cross-instance basis.
e.g. all the contexts need to be migrated before the IRQ is, to avoid a
screaming IRQ on the target CPU, or the IRQ handler on the target racing
with migration from the source.

Is there a neat way to do that in a driver without using IRQF_SHARED, so
that we don't end up accidentally sharing with other drivers? We can
probably librify the code to handle this under drivers/pmu/.

Thanks,
Mark.