Re: [RFC PATCH 0/9] pci: portdrv: Add auxiliary bus and register CXL PMUs (and aer)

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, May 29, 2024 at 05:40:54PM +0100, Jonathan Cameron wrote:
> Focus of this RFC is CXL Performance Monitoring Units in CXL Root Ports +
> Switch USP and DSPs.
> 
> The final patch moving AER to the aux bus is in the category of 'why
> not try this' rather than something I feel is a particularly good idea.
> I would like to get opinions on the various ways to avoid the double bus
> situation introduced here. Some comments on other options for those existing
> 'pci_express' bus devices at the end of this cover letter.
> 
> The primary problem this RFC tries to solve is providing a means to
> register the CXL PMUs found in devices which will be bound to the
> PCIe portdrv. The proposed solution is to avoid the limitations of
> the existing pcie service driver approach by bolting on support
> for devices registered on the auxiliary_bus.
> 
> Background
> ==========
> 
> When the CXL PMU driver was added, only the instances found in CXL type 3
> memory devices (end points) were supported. These PMUs also occur in CXL
> root ports, and CXL switch upstream and downstream ports.  Adding
> support for these was deliberately left for future work because theses
> ports are all bound to the pcie portdrv via the appropriate PCI class
> code.  Whilst some CXL support of functionality on RP and Switch Ports
> is handled by the CXL port driver, that is not bound to the PCIe device,
> is only instantiated as part of enumerating endpoints, and cannot use
> interrupts because those are in control of the PCIe portdrv. A PMU with
> out interrupts would be unfortunate so a different solution is needed.
> 
> Requirements
> ============
> 
> - Register multiple CXL PMUs (CPMUs) per portdrv instance.

What resources do CPMUs use?  BARs?  Config space registers in PCI
Capabilities?  Interrupts?  Are any of these resources
device-specific, or are they all defined by the CXL specs?

The "device" is a nice way to package those up and manage ownership
since there are often interdependencies.

I wish portdrv was directly implemented as part of the PCI core,
without binding to the device as a "driver".  We handle some other
pieces of functionality that way, e.g., the PCIe Capability itself,
PM, VPD, MSI/MSI-X, 

> - portdrv binds to the PCIe class code for PCI_CLASS_BRIDGE_PCI_NORMAL which
>   covers any PCI-Express port.
> - PCI MSI / MSIX message based interrupts must be registered by one driver -
>   in this case it's the portdrv.

The fact that AER/DPC/hotplug/etc (the portdrv services) use MSI/MSI-X
is a really annoying part of PCIe because bridges may have a BAR or
two and are allowed to have device-specific endpoint-like behavior, so
we may have to figure out how to share those interrupts between the
PCIe-architected stuff and the device-specific stuff.  I guess that's
messy no matter whether portdrv is its own separate driver or it's
integrated into the core.

> Side question.  Does anyone care if /sys/bus/pci_express goes away?
> - in theory it's ABI, but in practice it provides very little
>   actual ABI.

I would *love* to get rid of /sys/bus/pci_express, but I don't know
what, if anything, would break if we removed it.

Bjorn