Re: [PATCH v8 1/3] perf: cavium: Support memory controller PMU counters

[David Daney]

On 07/26/2017 07:29 PM, Greg KH wrote:
> On Wed, Jul 26, 2017 at 02:02:42PM -0700, David Daney wrote:
> > On 07/26/2017 01:08 PM, Greg KH wrote:
> > > On Wed, Jul 26, 2017 at 01:02:38PM -0700, David Daney wrote:
> > > > On 07/26/2017 10:33 AM, Greg KH wrote:
> > > > > On Wed, Jul 26, 2017 at 06:30:49PM +0200, Borislav Petkov wrote:
> > > > > > On Wed, Jul 26, 2017 at 09:19:49AM -0700, Greg KH wrote:
> > > > > > > On Wed, Jul 26, 2017 at 05:55:48PM +0200, Borislav Petkov wrote:
> > > > > > > > On Wed, Jul 26, 2017 at 05:45:15PM +0200, Jan Glauber wrote:
> > > > > > > > > The PMU/EDAC devices are all PCI devices do I need the 'struct pci_dev *'.
> > > > > > > > > I'm not aware of other ways to access these devices. Please enlighten
> > > > > > > > > me if I'm missing something.
> > > > > > > >
> > > > > > > > Me enlighten you on Cavium hardware?! You're funny.
> > > > > > > >
> > > > > > > > So I don't know whether the PCI hotplug code can run more than one
> > > > > > > > function upon PCI ID detection. Probably Greg will say, write a
> > > > > > > > multiplexer wrapper. :-)
> > > > > > >
> > > > > > > -ENOCONTEXT....
> > > > > > >
> > > > > > > Anyway, pci questions are best asked on the linux-pci@vger list.  And
> > > > > > > yes, all PCI devices end up with a 'struct pci_dev *' automatically.
> > > > > >
> > > > > > Simple: so they have a PCI ID of a memory contoller and want to hotplug
> > > > > > two drivers for it. And those two drivers should remain independent from
> > > > > > each other.
> > > > >
> > > > > Hahahahaha, no.  That's crazy, you were right in guessing what my answer
> > > > > was going to be :)
> > > >
> > > >
> > > > Just to be clear about the situation, the device is a memory controller.  It
> > > > has two main behaviors we are interested in:
> > > >
> > > > A) Error Detection And Correction (EDAC).  This should be connected to the
> > > > kernel's EDAC subsystem.  An existing driver (drivers/edac/thunderx_edac.c)
> > > > does exactly this.
> > > >
> > > > B) Performance Counters for actions taken in the corresponding memory. This
> > > > should be connected to the kernel's perf framework as an uncore-PMU (the
> > > > subject of this patch set).
> > > >
> > > > It is a single PCI device.  What should the driver architecture look like to
> > > > connect it to two different kernel subsystems?
> > >
> > > Modify the drivers/edac/thunderx_edac.c code to add support for
> > > performance counters.
> >
> > Thanks Greg.  This adds some clarity to the situation.
> >
> > This technique does slightly complicate the mapping of files and directories
> > in the kernel source tree to maintainers.
> >
> > Also, if a given configuration disables CONFIG_EDAC there is some hackery
> > needed to get the perf portion of the driver included.
>
> Well, you all deserve it for trying to have a single PCI device do
> multiple things at the same time.  There's no real good reason for
> creating hardware that way, PCI devices are "free", you should go throw
> a printed copy of the PCI spec at the firmware developers who did this
> to you.

The problem lies in something even more congealed than the "firmware". 
The PCI topology is etched in to the very fabric of the silicon of the SoC.


> Then get them to fix the firmware so you have multiple PCI devices...
>
> good luck!

We'll use all of that we can get!  Thanks.


> greg k-h