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

Bjorn Helgaas
2024-06-05T18:04:11Z

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