Re: [PATCH V2] PCI: Hide SBR from reset_methods if masked by CXL

[Vidya Sagar]

On 25/02/26 22:04, Dave Jiang wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 2/25/26 6:38 AM, Vidya Sagar wrote:
>> The CXL specification (e.g., CXL r3.1 v1.0, sec 8.1.5.2) defines
>> the "Unmask SBR" bit in the Port Control Extensions Register.
>> When this bit is 0 (default), asserting the Secondary Bus Reset (SBR) bit
>> in the Bridge Control register has no effect on the downstream bus.
>>
>> Currently, the Linux PCI core checks this condition in
>> pci_reset_bus_function(). If SBR is masked, it returns -ENOTTY during the
>> execution of the reset. However, during the probe phase (when probe=true),
>> the function currently returns 0. This 0 return value incorrectly signals
>> to the PCI subsystem that SBR is a viable reset method for the device.
>>
>> As a result, 'bus' is listed in the device's
>> /sys/bus/pci/devices/.../reset_methods attribute, even though the hardware
>> is incapable of performing it. If a user attempts to write bus to reset
>> method or triggers a reset that falls back to SBR, the operation fails
>> with: "bash: echo: write error: Inappropriate ioctl for device" error.
>>
>> This patch modifies pci_reset_bus_function() to return -ENOTTY immediately
>> if cxl_sbr_masked() is true, regardless of the probe argument. This
>> ensures that 'bus' is not advertised in reset_methods when the hardware
>> prevents it, improving clarity for users and aligning the sysfs capability
>> report with actual hardware behavior.
>>
>> Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> 
> Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> 
>> ---
>> v2:
>> * Before deciding to hide 'bus' reset method, add an extra check to make sure
>>   that the link is indeed operating in the CXL mode and not in PCIe mode as
>>   the spec clearly says that a '0' in 'Unmask SBR' doesn't have any effect if
>>   the link is not operating in the CXL mode.
>>
>>  drivers/pci/pci.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index f3244630bfd0..a176566ba56f 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -4915,12 +4915,8 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
>>        * If "dev" is below a CXL port that has SBR control masked, SBR
>>        * won't do anything, so return error.
>>        */
>> -     if (bridge && cxl_sbr_masked(bridge)) {
>> -             if (probe)
>> -                     return 0;
>> -
>> +     if (bridge && bridge->is_cxl && cxl_sbr_masked(bridge))
>>               return -ENOTTY;
>> -     }
>>
>>       rc = pci_dev_reset_iommu_prepare(dev);
>>       if (rc) {
> 

Bjorn,
Do you have any comments for this patch?