From 1f59abaebf9cb1437d59e2b8cf879c6b86976d45 Mon Sep 17 00:00:00 2001 From: Michal Berger Date: Thu, 28 Jul 2022 16:30:26 +0200 Subject: [PATCH] scripts/setup: Use driver_override to bind devices Using new_id attribute is global in scope, meaning that depending on the kernel's setup seen prior running setup.sh, single write to it may re-bind ALL matching devices. This doesn't play well with our PCI_{ALLOWED,BLOCKED} options as they can't be enforced in such a case. Consider the following example: > modprobe -r nvme # all nvme ctrls are detached from the kernel > echo 0xdead 0xbeef >/sys/bus/pci/drivers/uio_pci_generic/new_id # setup.sh-wise > modprobe -r nvme > PCI_BLOCKED=some:dead:beef.bdf setup.sh # PCI_BLOCKED device still ends up bound to userspace driver. After this single write, ALL matching devices will end up bound to uio_pci_generic. To avoid this, we should override preferred driver on per-bdf basis. Signed-off-by: Michal Berger Change-Id: Ic4613e33321303b92b47ce3f4d7e1f29ecca3036 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13813 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Kamil Godzwon Reviewed-by: Konrad Sztyber --- scripts/setup.sh | 11 ++++------- test/nvme/sw_hotplug.sh | 8 ++++---- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/scripts/setup.sh b/scripts/setup.sh index b0d948582..f925ef077 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -134,7 +134,6 @@ function linux_bind_driver() { bdf="$1" driver_name="$2" old_driver_name=${drivers_d["$bdf"]:-no driver} - ven_dev_id="${pci_ids_vendor["$bdf"]#0x} ${pci_ids_device["$bdf"]#0x}" if [[ $driver_name == "$old_driver_name" ]]; then pci_dev_echo "$bdf" "Already using the $old_driver_name driver" @@ -142,7 +141,6 @@ function linux_bind_driver() { fi if [[ $old_driver_name != "no driver" ]]; then - echo "$ven_dev_id" > "/sys/bus/pci/devices/$bdf/driver/remove_id" 2> /dev/null || true echo "$bdf" > "/sys/bus/pci/devices/$bdf/driver/unbind" fi @@ -152,8 +150,9 @@ function linux_bind_driver() { return 0 fi - echo "$ven_dev_id" > "/sys/bus/pci/drivers/$driver_name/new_id" 2> /dev/null || true - echo "$bdf" > "/sys/bus/pci/drivers/$driver_name/bind" 2> /dev/null || true + echo "$driver_name" > "/sys/bus/pci/devices/$bdf/driver_override" + echo "$bdf" > "/sys/bus/pci/drivers_probe" + echo "" > "/sys/bus/pci/devices/$bdf/driver_override" if [[ $driver_name == uio_pci_generic ]] && ! check_for_driver igb_uio; then # Check if the uio_pci_generic driver is broken as it might be in @@ -177,8 +176,6 @@ function linux_bind_driver() { function linux_unbind_driver() { local bdf="$1" - local ven_dev_id - ven_dev_id="${pci_ids_vendor["$bdf"]#0x} ${pci_ids_device["$bdf"]#0x}" local old_driver_name=${drivers_d["$bdf"]:-no driver} if [[ $old_driver_name == "no driver" ]]; then @@ -187,8 +184,8 @@ function linux_unbind_driver() { fi if [[ -e /sys/bus/pci/drivers/$old_driver_name ]]; then - echo "$ven_dev_id" > "/sys/bus/pci/drivers/$old_driver_name/remove_id" 2> /dev/null || true echo "$bdf" > "/sys/bus/pci/drivers/$old_driver_name/unbind" + echo "" > "/sys/bus/pci/devices/$bdf/driver_override" fi pci_dev_echo "$bdf" "$old_driver_name -> no driver" diff --git a/test/nvme/sw_hotplug.sh b/test/nvme/sw_hotplug.sh index 02803d403..43ed89170 100755 --- a/test/nvme/sw_hotplug.sh +++ b/test/nvme/sw_hotplug.sh @@ -56,12 +56,12 @@ remove_attach_helper() { done fi - # setup.sh masks failures while writing to sysfs interfaces so let's do - # this on our own to make sure there's anything for the hotplug to reattach - # to. + # Avoid setup.sh as it does some extra work which is not relevant for this test. for dev in "${nvmes[@]}"; do + echo "${pci_bus_driver["$dev"]}" > "/sys/bus/pci/devices/$dev/driver_override" echo "$dev" > "/sys/bus/pci/devices/$dev/driver/unbind" - echo "$dev" > "/sys/bus/pci/drivers/${pci_bus_driver["$dev"]}/bind" + echo "$dev" > "/sys/bus/pci/drivers_probe" + echo "" > "/sys/bus/pci/devices/$dev/driver_override" done # Wait now for hotplug to reattach to the devices