diff mbox series

common/rc: confirm pcie hotplug capabilities

Message ID 20201125154952.2871261-1-kbusch@kernel.org (mailing list archive)
State New, archived
Headers show
Series common/rc: confirm pcie hotplug capabilities | expand

Commit Message

Keith Busch Nov. 25, 2020, 3:49 p.m. UTC
It turns out some PCIe slots report hotplug surprise but are not hotplug
capable. Despite the contridiction, the spec seems to allow that.

The linux pciehp driver needs hotplug capable to bind to the slot, and
the block/019 test requires hotplug surprise to handle the unannounced
link-down. Verify both bits in the slot capabilities register are set.

Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 common/rc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Shinichiro Kawasaki Dec. 2, 2020, 1:39 a.m. UTC | #1
On Nov 25, 2020 / 07:49, Keith Busch wrote:
> It turns out some PCIe slots report hotplug surprise but are not hotplug
> capable. Despite the contridiction, the spec seems to allow that.
> 
> The linux pciehp driver needs hotplug capable to bind to the slot, and
> the block/019 test requires hotplug surprise to handle the unannounced
> link-down. Verify both bits in the slot capabilities register are set.
> 
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  common/rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 0d7a3cd..d396fb5 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -269,7 +269,7 @@ _require_test_dev_in_hotplug_slot() {
>  
>  	local slt_cap
>  	slt_cap="$(setpci -s "${parent}" CAP_EXP+14.w)"
> -	if [[ $((0x${slt_cap} & 0x20)) -eq 0 ]]; then
> +	if [[ $((0x${slt_cap} & 0x60)) -ne 0x60 ]]; then
>  		SKIP_REASON="$TEST_DEV is not in a hot pluggable slot"
>  		return 1
>  	fi
> -- 
> 2.24.1
>

Thank you Keith.

I saw some mother boards report "hotplug surprise" bit to PCIe slots with
SAS-HBA, but do not report "hotplug capable" bit. Without the fix, block/019
was executed to the HDDs connected to the SAS-HBA, and the HDDs disappear after
the test case run (the test case emulates hotplug, but the PCIe slot is not
hotplug capable). I confirmed that this fix checks the both bits and skips
the test case for the HDDs.

Omar, please consider this fix to upstream.

Tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Omar Sandoval Jan. 4, 2021, 10:55 p.m. UTC | #2
On Wed, Nov 25, 2020 at 07:49:52AM -0800, Keith Busch wrote:
> It turns out some PCIe slots report hotplug surprise but are not hotplug
> capable. Despite the contridiction, the spec seems to allow that.
> 
> The linux pciehp driver needs hotplug capable to bind to the slot, and
> the block/019 test requires hotplug surprise to handle the unannounced
> link-down. Verify both bits in the slot capabilities register are set.
> 
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  common/rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied.
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 0d7a3cd..d396fb5 100644
--- a/common/rc
+++ b/common/rc
@@ -269,7 +269,7 @@  _require_test_dev_in_hotplug_slot() {
 
 	local slt_cap
 	slt_cap="$(setpci -s "${parent}" CAP_EXP+14.w)"
-	if [[ $((0x${slt_cap} & 0x20)) -eq 0 ]]; then
+	if [[ $((0x${slt_cap} & 0x60)) -ne 0x60 ]]; then
 		SKIP_REASON="$TEST_DEV is not in a hot pluggable slot"
 		return 1
 	fi