public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL
@ 2026-02-20 19:52 Vidya Sagar
  2026-02-20 21:21 ` Dave Jiang
  2026-02-25 13:38 ` [PATCH V2] " Vidya Sagar
  0 siblings, 2 replies; 11+ messages in thread
From: Vidya Sagar @ 2026-02-20 19:52 UTC (permalink / raw)
  To: bhelgaas, dave.jiang, Jonathan.Cameron, alex.williamson,
	raphael.norwitz
  Cc: vsethi, sdonthineni, smadhavan, skancherla, vaslot, linux-pci,
	linux-cxl, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv

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@nvidia.com>
---
 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..57e24300d1c7 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 && cxl_sbr_masked(bridge))
 		return -ENOTTY;
-	}
 
 	rc = pci_dev_reset_iommu_prepare(dev);
 	if (rc) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-20 19:52 [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL Vidya Sagar
@ 2026-02-20 21:21 ` Dave Jiang
  2026-02-23 13:11   ` Vidya Sagar
  2026-02-25 13:38 ` [PATCH V2] " Vidya Sagar
  1 sibling, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2026-02-20 21:21 UTC (permalink / raw)
  To: Vidya Sagar, bhelgaas, Jonathan.Cameron, alex.williamson,
	raphael.norwitz, Dan Williams
  Cc: vsethi, sdonthineni, smadhavan, skancherla, vaslot, linux-pci,
	linux-cxl, linux-kernel, kthota, mmaddireddy, sagar.tv



On 2/20/26 12:52 PM, 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.


The "Unmask SBR" bit is a toggle bit. It does not give indicator whether the device is capable of SBR or not. The original thought was that if the user is issuing CXL SBR, you know what you are doing and the kernel will set that bit and issue the SBR.

DJ

> 
> 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@nvidia.com>
> ---
>  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..57e24300d1c7 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 && cxl_sbr_masked(bridge))
>  		return -ENOTTY;
> -	}
>  
>  	rc = pci_dev_reset_iommu_prepare(dev);
>  	if (rc) {


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-20 21:21 ` Dave Jiang
@ 2026-02-23 13:11   ` Vidya Sagar
  2026-02-23 15:52     ` Dave Jiang
  0 siblings, 1 reply; 11+ messages in thread
From: Vidya Sagar @ 2026-02-23 13:11 UTC (permalink / raw)
  To: Dave Jiang, bhelgaas@google.com, Jonathan.Cameron@huawei.com,
	alex.williamson@redhat.com, raphael.norwitz@nutanix.com,
	Dan Williams
  Cc: Vikram Sethi, Shanker Donthineni, Srirangan Madhavan,
	Sai Yashwanth Reddy Kancherla, Vishal Aslot,
	linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org, Krishna Thota, Manikanta Maddireddy,
	sagar.tv@gmail.com

On 21/02/26 02:51, Dave Jiang wrote:
> External email: Use caution opening links or attachments
>
>
> On 2/20/26 12:52 PM, 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.
>
> The "Unmask SBR" bit is a toggle bit. It does not give indicator whether the device is capable of SBR or not.

Not sure how is this point relevant here. Can you help me understand?

BTW, what do you mean by "whether the device is capable of SBR or not". My understanding is that each downstream port

must support SBR. The patch I made would ensure that 'bus' entry is not shown under 'reset_methods' if the downstream

port is a CXL capable port and 'Unmask SBR' is '0'.

> The original thought was that if the user is issuing CXL SBR, you know what you are doing and the kernel will set that bit and issue the SBR.
>
> DJ
>
>> 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@nvidia.com>
>> ---
>>  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..57e24300d1c7 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 && cxl_sbr_masked(bridge))
>>               return -ENOTTY;
>> -     }
>>
>>       rc = pci_dev_reset_iommu_prepare(dev);
>>       if (rc) {



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-23 13:11   ` Vidya Sagar
@ 2026-02-23 15:52     ` Dave Jiang
  2026-02-25 13:13       ` Vidya Sagar
  0 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2026-02-23 15:52 UTC (permalink / raw)
  To: Vidya Sagar, bhelgaas@google.com, Jonathan.Cameron@huawei.com,
	alex.williamson@redhat.com, raphael.norwitz@nutanix.com,
	Dan Williams
  Cc: Vikram Sethi, Shanker Donthineni, Srirangan Madhavan,
	Sai Yashwanth Reddy Kancherla, Vishal Aslot,
	linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org, Krishna Thota, Manikanta Maddireddy,
	sagar.tv@gmail.com



On 2/23/26 6:11 AM, Vidya Sagar wrote:
> On 21/02/26 02:51, Dave Jiang wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2/20/26 12:52 PM, 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.
>>
>> The "Unmask SBR" bit is a toggle bit. It does not give indicator whether the device is capable of SBR or not.
> 
> Not sure how is this point relevant here. Can you help me understand?

That means it's not a capability bit. It's a r/w toggle bit to mask or unmask SBR for CXL.

> 
> BTW, what do you mean by "whether the device is capable of SBR or not". My understanding is that each downstream port
> 
> must support SBR. The patch I made would ensure that 'bus' entry is not shown under 'reset_methods' if the downstream
> 
> port is a CXL capable port and 'Unmask SBR' is '0'.

The purpose of introducing the cxl bus reset method is, "you know what you are doing" because you selected this method and the kernel will set the bit to allow the reset. By default the Unmask SBR bit is clear. So if you hide the reset attribute then the user can't reset without going through other means to set the bit first (i.e. via setpci tool). This is intentionally setup to do this through total control of the kernel and not having to jump through hoops to toggle bit via a user tool first. Otherwise you can just toggle the bit with a user tool and use the standard PCI bus reset method.

Please see commit log of this commit:
53c49b6e6dd2 ("PCI/CXL: Add 'cxl_bus' reset method for devices below CXL Ports")

DJ

> 
>> The original thought was that if the user is issuing CXL SBR, you know what you are doing and the kernel will set that bit and issue the SBR.
>>
>> DJ
>>
>>> 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@nvidia.com>
>>> ---
>>>  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..57e24300d1c7 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 && cxl_sbr_masked(bridge))
>>>               return -ENOTTY;
>>> -     }
>>>
>>>       rc = pci_dev_reset_iommu_prepare(dev);
>>>       if (rc) {
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-23 15:52     ` Dave Jiang
@ 2026-02-25 13:13       ` Vidya Sagar
  2026-02-25 16:09         ` Dave Jiang
  0 siblings, 1 reply; 11+ messages in thread
From: Vidya Sagar @ 2026-02-25 13:13 UTC (permalink / raw)
  To: Dave Jiang, bhelgaas@google.com, Jonathan.Cameron@huawei.com,
	raphael.norwitz@nutanix.com, Dan Williams
  Cc: Vikram Sethi, Shanker Donthineni, Srirangan Madhavan,
	Sai Yashwanth Reddy Kancherla, Vishal Aslot,
	linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org, Krishna Thota, Manikanta Maddireddy,
	sagar.tv@gmail.com

On 23/02/26 21:22, Dave Jiang wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 2/23/26 6:11 AM, Vidya Sagar wrote:
>> On 21/02/26 02:51, Dave Jiang wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 2/20/26 12:52 PM, 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.
>>>
>>> The "Unmask SBR" bit is a toggle bit. It does not give indicator whether the device is capable of SBR or not.
>>
>> Not sure how is this point relevant here. Can you help me understand?
> 
> That means it's not a capability bit. It's a r/w toggle bit to mask or unmask SBR for CXL.
> 
>>
>> BTW, what do you mean by "whether the device is capable of SBR or not". My understanding is that each downstream port
>>
>> must support SBR. The patch I made would ensure that 'bus' entry is not shown under 'reset_methods' if the downstream
>>
>> port is a CXL capable port and 'Unmask SBR' is '0'.
> 
> The purpose of introducing the cxl bus reset method is, "you know what you are doing" because you selected this method and the kernel will set the bit to allow the reset.
I'm not really questioning the purpose of 'cxl_bus' reset method.
I'm only wondering why is 'bus' reset method shown if it can't be used.
> By default the Unmask SBR bit is clear. So if you hide the reset attribute then the user can't reset without going through other means to set the bit first (i.e. via setpci tool). This is intentionally setup to do this through total control of the
> kernel and not having to jump through hoops to toggle bit via a user tool first. Otherwise you can just toggle the bit with a user tool and use the standard PCI bus reset method.
This doesn't make much sense to me. Since the 'cxl_bus' method is anyway going to update the 'Unmask SBR' before applying the SBR through Bride Control register, what is the point in showing 'bus' which doesn't work by default and expecting the user to toggle the 'Unmask SBR' through tools like setpci? The user can very well use 'cxl_bus' directly which does the same thing, right?
BTW, I found one issue with my current patch where I try to hide 'bus' if 'Unmask SBR' is '0', but the CXL spec also says that for the SBR bit in Bridge Control Register to be ineffective, both 'Unmask SBR' should be '0' and also the link as such must be operating in the CXL mode. I'll push V2 patch to add this check as well.
> 
> Please see commit log of this commit:
> 53c49b6e6dd2 ("PCI/CXL: Add 'cxl_bus' reset method for devices below CXL Ports")
> 
> DJ
> 
>>
>>> The original thought was that if the user is issuing CXL SBR, you know what you are doing and the kernel will set that bit and issue the SBR.
>>>
>>> DJ
>>>
>>>> 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@nvidia.com>
>>>> ---
>>>>  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..57e24300d1c7 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 && cxl_sbr_masked(bridge))
>>>>               return -ENOTTY;
>>>> -     }
>>>>
>>>>       rc = pci_dev_reset_iommu_prepare(dev);
>>>>       if (rc) {
>>
>>
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH V2] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-20 19:52 [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL Vidya Sagar
  2026-02-20 21:21 ` Dave Jiang
@ 2026-02-25 13:38 ` Vidya Sagar
  2026-02-25 16:34   ` Dave Jiang
  2026-03-17 19:57   ` Bjorn Helgaas
  1 sibling, 2 replies; 11+ messages in thread
From: Vidya Sagar @ 2026-02-25 13:38 UTC (permalink / raw)
  To: bhelgaas, dave.jiang, Jonathan.Cameron, raphael.norwitz
  Cc: vsethi, sdonthineni, smadhavan, skancherla, vaslot, linux-pci,
	linux-cxl, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv

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@nvidia.com>
---
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) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-25 13:13       ` Vidya Sagar
@ 2026-02-25 16:09         ` Dave Jiang
  0 siblings, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2026-02-25 16:09 UTC (permalink / raw)
  To: Vidya Sagar, bhelgaas@google.com, Jonathan.Cameron@huawei.com,
	raphael.norwitz@nutanix.com, Dan Williams
  Cc: Vikram Sethi, Shanker Donthineni, Srirangan Madhavan,
	Sai Yashwanth Reddy Kancherla, Vishal Aslot,
	linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org, Krishna Thota, Manikanta Maddireddy,
	sagar.tv@gmail.com



On 2/25/26 6:13 AM, Vidya Sagar wrote:
> On 23/02/26 21:22, Dave Jiang wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2/23/26 6:11 AM, Vidya Sagar wrote:
>>> On 21/02/26 02:51, Dave Jiang wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 2/20/26 12:52 PM, 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.
>>>>
>>>> The "Unmask SBR" bit is a toggle bit. It does not give indicator whether the device is capable of SBR or not.
>>>
>>> Not sure how is this point relevant here. Can you help me understand?
>>
>> That means it's not a capability bit. It's a r/w toggle bit to mask or unmask SBR for CXL.
>>
>>>
>>> BTW, what do you mean by "whether the device is capable of SBR or not". My understanding is that each downstream port
>>>
>>> must support SBR. The patch I made would ensure that 'bus' entry is not shown under 'reset_methods' if the downstream
>>>
>>> port is a CXL capable port and 'Unmask SBR' is '0'.
>>
>> The purpose of introducing the cxl bus reset method is, "you know what you are doing" because you selected this method and the kernel will set the bit to allow the reset.
> I'm not really questioning the purpose of 'cxl_bus' reset method.
> I'm only wondering why is 'bus' reset method shown if it can't be used.
>> By default the Unmask SBR bit is clear. So if you hide the reset attribute then the user can't reset without going through other means to set the bit first (i.e. via setpci tool). This is intentionally setup to do this through total control of the
>> kernel and not having to jump through hoops to toggle bit via a user tool first. Otherwise you can just toggle the bit with a user tool and use the standard PCI bus reset method.
> This doesn't make much sense to me. Since the 'cxl_bus' method is anyway going to update the 'Unmask SBR' before applying the SBR through Bride Control register, what is the point in showing 'bus' which doesn't work by default and expecting the user to toggle the 'Unmask SBR' through tools like setpci? The user can very well use 'cxl_bus' directly which does the same thing, right?

Ok I think I see where I made a mistake. You are hiding the regular PCI bus reset method and not the cxl_bus reset method. Yes then I agree what you are doing is fine. This does not impact the cxl_bus reset method. Sorry about the noise. 

DJ

> BTW, I found one issue with my current patch where I try to hide 'bus' if 'Unmask SBR' is '0', but the CXL spec also says that for the SBR bit in Bridge Control Register to be ineffective, both 'Unmask SBR' should be '0' and also the link as such must be operating in the CXL mode. I'll push V2 patch to add this check as well.
>>
>> Please see commit log of this commit:
>> 53c49b6e6dd2 ("PCI/CXL: Add 'cxl_bus' reset method for devices below CXL Ports")
>>
>> DJ
>>
>>>
>>>> The original thought was that if the user is issuing CXL SBR, you know what you are doing and the kernel will set that bit and issue the SBR.
>>>>
>>>> DJ
>>>>
>>>>> 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@nvidia.com>
>>>>> ---
>>>>>  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..57e24300d1c7 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 && cxl_sbr_masked(bridge))
>>>>>               return -ENOTTY;
>>>>> -     }
>>>>>
>>>>>       rc = pci_dev_reset_iommu_prepare(dev);
>>>>>       if (rc) {
>>>
>>>
>>
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-25 13:38 ` [PATCH V2] " Vidya Sagar
@ 2026-02-25 16:34   ` Dave Jiang
  2026-02-27 13:34     ` Jonathan Cameron
  2026-03-17 17:34     ` Vidya Sagar
  2026-03-17 19:57   ` Bjorn Helgaas
  1 sibling, 2 replies; 11+ messages in thread
From: Dave Jiang @ 2026-02-25 16:34 UTC (permalink / raw)
  To: Vidya Sagar, bhelgaas, Jonathan.Cameron, raphael.norwitz
  Cc: vsethi, sdonthineni, smadhavan, skancherla, vaslot, linux-pci,
	linux-cxl, linux-kernel, kthota, mmaddireddy, sagar.tv



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@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
> 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) {


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-25 16:34   ` Dave Jiang
@ 2026-02-27 13:34     ` Jonathan Cameron
  2026-03-17 17:34     ` Vidya Sagar
  1 sibling, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-02-27 13:34 UTC (permalink / raw)
  To: Dave Jiang
  Cc: Vidya Sagar, bhelgaas, raphael.norwitz, vsethi, sdonthineni,
	smadhavan, skancherla, vaslot, linux-pci, linux-cxl, linux-kernel,
	kthota, mmaddireddy, sagar.tv

On Wed, 25 Feb 2026 09:34:00 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> 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@nvidia.com>  
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-25 16:34   ` Dave Jiang
  2026-02-27 13:34     ` Jonathan Cameron
@ 2026-03-17 17:34     ` Vidya Sagar
  1 sibling, 0 replies; 11+ messages in thread
From: Vidya Sagar @ 2026-03-17 17:34 UTC (permalink / raw)
  To: bhelgaas@google.com, Jonathan.Cameron@huawei.com
  Cc: Vikram Sethi, Shanker Donthineni, Srirangan Madhavan,
	Sai Yashwanth Reddy Kancherla, Vishal Aslot,
	linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org, Krishna Thota, Manikanta Maddireddy,
	sagar.tv@gmail.com, Dave Jiang, raphael.norwitz@nutanix.com

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@nvidia.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 
>> ---
>> 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?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] PCI: Hide SBR from reset_methods if masked by CXL
  2026-02-25 13:38 ` [PATCH V2] " Vidya Sagar
  2026-02-25 16:34   ` Dave Jiang
@ 2026-03-17 19:57   ` Bjorn Helgaas
  1 sibling, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2026-03-17 19:57 UTC (permalink / raw)
  To: Vidya Sagar
  Cc: bhelgaas, dave.jiang, Jonathan.Cameron, raphael.norwitz, vsethi,
	sdonthineni, smadhavan, skancherla, vaslot, linux-pci, linux-cxl,
	linux-kernel, kthota, mmaddireddy, sagar.tv

On Wed, Feb 25, 2026 at 07:08:01PM +0530, 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@nvidia.com>

Applied to pci/reset for v7.1, thanks!

I changed bridge->is_cxl to pcie_is_cxl(bridge) to match most other
tests.

> ---
> 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) {
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-03-17 19:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-20 19:52 [PATCH V1] PCI: Hide SBR from reset_methods if masked by CXL Vidya Sagar
2026-02-20 21:21 ` Dave Jiang
2026-02-23 13:11   ` Vidya Sagar
2026-02-23 15:52     ` Dave Jiang
2026-02-25 13:13       ` Vidya Sagar
2026-02-25 16:09         ` Dave Jiang
2026-02-25 13:38 ` [PATCH V2] " Vidya Sagar
2026-02-25 16:34   ` Dave Jiang
2026-02-27 13:34     ` Jonathan Cameron
2026-03-17 17:34     ` Vidya Sagar
2026-03-17 19:57   ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox