[PATCH] common/rc: confirm pcie hotplug capabilities

[PATCH] common/rc: confirm pcie hotplug capabilities

[Keith Busch]

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

Re: [PATCH] common/rc: confirm pcie hotplug capabilities

[Shinichiro Kawasaki]

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>

-- 
Best Regards,
Shin'ichiro Kawasaki

Re: [PATCH] common/rc: confirm pcie hotplug capabilities

[Omar Sandoval]

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.