Re: [PATCH v6 05/31] PCI: Refuse bus and slot reset below a CXL port with SBR masked

[Alex Williamson <alex@shazbot.org>]

On Fri, 16 Oct 2026 21:48:12 +0530
<mhonap@nvidia.com> wrote:

> From: Manish Honap <mhonap@nvidia.com>
> 
> A CXL port ignores the Secondary Bus Reset bit in Bridge Control unless
> Unmask SBR is set in its CXL Port DVSEC (CXL r4.0 sec 8.1.5.2).
> pci_reset_bus_function() already returns -ENOTTY in that case.
> pci_reset_bus(), pci_probe_reset_bus() and the slot variants do not, so
> they report success for a reset that reached no device below the port.
> 
> vfio-pci uses these for VFIO_DEVICE_GET_PCI_HOT_RESET_INFO and
> VFIO_DEVICE_PCI_HOT_RESET. A user is then told a hot reset happened while
> a CXL device below a masked port kept its state.
> 
> Return -ENOTTY from pci_probe_reset_slot() and pci_probe_reset_bus()
> when the bridge above the bus is a CXL port with SBR masked.
> pci_reset_bus() and pci_try_reset_bridge() probe before they reset, so
> they refuse too.
> 
> Users see this in three places:
> - the vfio hot reset ioctls fail (vfio reports -ENODEV when neither probe
>   succeeds),
> - the reset_subordinate sysfs attribute returns -ENOTTY,
> - vfio-pci groups a device in a hotplug slot below such a port by bus
>   instead of by slot.
> 
> Every other pci_reset_bus() caller, including the vfio bus reset at
> close, is refused the same way. With one device per CXL port both
> groupings have the same members.
> 
> Leave pci_bus_error_reset() as it is. AER and host bridge error recovery
> call it without saving or restoring state and only map its result to
> recovered or disconnect. Failing it below a masked port would turn
> recovery of every CXL device there, memory expanders included, into a
> disconnect; that is a CXL error handling decision, not part of this fix.
> 
> Assisted-by: LLM
> Signed-off-by: Manish Honap <mhonap@nvidia.com>
> ---
>  drivers/pci/pci.c | 31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index cecf8e681e69..87fe9de81acf 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4984,6 +4984,20 @@ static bool cxl_sbr_masked(struct pci_dev *dev)
>  	return true;
>  }
>  
> +/*
> + * A secondary bus reset below a CXL port with SBR masked does not reach the
> + * devices on the bus, so a bus or slot reset there would report success for
> + * a reset that did not happen. The probes and the save/restore resets that
> + * pci_reset_bus() and reset_subordinate use refuse it; error recovery
> + * (pci_bus_error_reset()) keeps its existing behavior.
> + */

Reads like AI over-explaining a fairly obvious concept.

> +static bool pci_bus_cxl_sbr_masked(struct pci_bus *bus)
> +{
> +	struct pci_dev *bridge = bus->self;
> +
> +	return bridge && pcie_is_cxl(bridge) && cxl_sbr_masked(bridge);
> +}
> +
>  static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
>  {
>  	struct pci_dev *bridge = pci_upstream_bridge(dev);
> @@ -5615,6 +5629,9 @@ static int pci_slot_reset(struct pci_slot *slot, bool probe)
>   */
>  int pci_probe_reset_slot(struct pci_slot *slot)
>  {
> +	if (slot && pci_bus_cxl_sbr_masked(slot->bus))
> +		return -ENOTTY;

Why would this be open coded here instead of pci_slot_resettable()?

> +
>  	return pci_slot_reset(slot, PCI_RESET_PROBE);
>  }
>  EXPORT_SYMBOL_GPL(pci_probe_reset_slot);
> @@ -5638,7 +5655,7 @@ static int pci_try_reset_slot(struct pci_slot *slot)
>  {
>  	int rc;
>  
> -	rc = pci_slot_reset(slot, PCI_RESET_PROBE);
> +	rc = pci_probe_reset_slot(slot);

Also suggests it's tested in the wrong place.

>  	if (rc)
>  		return rc;
>  
> @@ -5685,7 +5702,7 @@ static int pci_try_reset_bus(struct pci_bus *bus)
>  {
>  	int rc;
>  
> -	rc = pci_bus_reset(bus, PCI_RESET_PROBE);
> +	rc = pci_probe_reset_bus(bus);

And again.

>  	if (rc)
>  		return rc;
>  
> @@ -5726,8 +5743,13 @@ static int pci_reset_bridge(struct pci_dev *bridge, bool restore)
>  	if (list_empty(&bus->slots))
>  		goto bus_reset;
>  
> +	/*
> +	 * Probe without the CXL SBR mask check: error recovery keeps its
> +	 * reset path below a masked CXL port, and pci_try_reset_slot() and
> +	 * pci_try_reset_bus() refuse the restore path there.
> +	 */

More AI?  This function is not designated as exclusive to error
recovery.  If sbr is masked, slot reset doesn't generate an error and
doesn't effect a reset, regardless of who called it.  If we're including
a new condition for a bus/slot reset to be effective, it needs to be
applied universally.

>  	list_for_each_entry(slot, &bus->slots, list)
> -		if (pci_probe_reset_slot(slot))
> +		if (pci_slot_reset(slot, PCI_RESET_PROBE))
>  			goto bus_reset;
>  
>  	list_for_each_entry(slot, &bus->slots, list) {
> @@ -5773,6 +5795,9 @@ int pci_try_reset_bridge(struct pci_dev *bridge)
>   */
>  int pci_probe_reset_bus(struct pci_bus *bus)
>  {
> +	if (pci_bus_cxl_sbr_masked(bus))
> +		return -ENOTTY;
> +
>  	return pci_bus_reset(bus, PCI_RESET_PROBE);
>  }
>  EXPORT_SYMBOL_GPL(pci_probe_reset_bus);

Redux, pci_bus_resettable().  Thanks,

Alex