Re: [PATCH 1/3] PCI: Add check for CXL Secondary Bus Reset

[Lukas Wunner <lukas@wunner.de>]

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