On Mon, Mar 11, 2024 at 01:39:53PM -0700, Dave Jiang wrote:
> +static bool is_cxl_device(struct pci_dev *dev)
> +{
> + return pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL,
> + CXL_DVSEC_PCIE_DEVICE);
> +}
If this was my bikeshed, I'd call it pci_is_cxl() to match pci_is_pcie().
> +static bool is_cxl_port_sbr_masked(struct pci_dev *dev)
> +{
> + int dvsec;
> + int rc;
> + u16 reg;
Nit: Inverse Christmas tree?
> static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
> {
> int rc;
>
> + /* If it's a CXL port and the SBR control is masked, fail the SBR */
> + if (is_cxl_device(dev) && dev->bus->self &&
> + is_cxl_port_sbr_masked(dev->bus->self)) {
> + if (probe)
> + return 0;
> +
> + return -EPERM;
> + }
> +
Is this also necessary if CONFIG_CXL_PCI=n?
Return code on non-availability of a reset method is generally -ENOTTY.
Or is the choice deliberate to expose this reset method despite the bit
being set and thus allow user space to unmask it in the first place?
Also, we mostly use pci_upstream_bridge(dev) in lieu of dev->bus->self.
Is the choice to use the latter deliberate because maybe is_virtfn is
never set and the device can never be on the root bus? (What about
RCiEP CXL devices?)
Thanks,
Lukas
