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

[Dan Williams <dan.j.williams@xxxxxxxxx>]

Dave Jiang wrote:
> Per CXL spec r3.1 8.1.5.2, secondary bus reset is masked unless the
> "Unmask SBR" bit is set. Add a check to the PCI secondary bus reset
> path to fail the CXL SBR request if the "Unmask SBR" bit is clear in
> the CXL Port Control Extensions register by returning -ENOTTY.
> 
> With the current behavior, the bus_reset would appear to have executed
> successfully, however the operation is actually masked if the "Unmask
> SBR" bit is set with the default value. The intention is to inform the
> user that SBR for the CXL device is masked and will not go through.
> 
> The expectation is that if a user overrides the "Unmask SBR" via a
> user tool such as setpci then they can trigger a bus reset successfully.
> 
> Link: https://lore.kernel.org/linux-cxl/20240220203956.GA1502351@bhelgaas/
> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> ---
[..]
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e5f243dd4288..259e5d6538bb 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4927,10 +4927,55 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
>  	return pci_reset_hotplug_slot(dev->slot->hotplug, probe);
>  }
>  
> +static bool pci_is_cxl(struct pci_dev *dev)
> +{
> +	return pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL,
> +					 CXL_DVSEC_PCIE_DEVICE);
> +}
> +
> +static bool is_cxl_port_sbr_masked(struct pci_dev *dev)
> +{
> +	int dvsec;
> +	u16 reg;
> +	int rc;
> +
> +	/*
> +	 * No DVSEC found, must not be CXL port.
> +	 */

This comment is unfortunately not correct. Per CXL 9.12.3 "Enumerating
CXL RPs and DSPs", the CXL_DVSEC_PCIE_PORT disappears when no endpoint
is connected. So the comment should be:

/*
 * No DVSEC found, either is not a CXL port, or not connected in which
 * case mask state is a nop (CXL 3.1 9.12.3 "Enumerating CXL RPs and DSPs") 
 */

> +	dvsec = pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL,
> +					  CXL_DVSEC_PCIE_PORT);
> +	if (!dvsec)
> +		return false;
> +
> +	rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_PORT_CONTROL, &reg);
> +	if (rc)
> +		return true;

If the link gets disconnected after the above check but before reading
the register the value returned *should* be 0xffff which should
naturally indicate that reset is not masked.