patches and low-level development discussion
 help / color / mirror / code / Atom feed
* [PATCH nixpkgs] spectrumPackages.spectrum-testhost: fix driver bind
@ 2021-04-21 10:16 Alyssa Ross
  2021-04-22  1:27 ` Cole Helbling
  0 siblings, 1 reply; 3+ messages in thread
From: Alyssa Ross @ 2021-04-21 10:16 UTC (permalink / raw)
  To: devel

The following operations are available to us in /sys/bus/pci/drivers/vfio-pci:
 bind, unbind, new_id, remove_id

bind and unbind control attachment of a device to a driver.
new_id and remove_id control whether a device should be considered
"supported" by a driver.

But when an unassigned device is newly supported by a driver, the
kernel will automatically bind it to that driver.  We were relying on
the new_id operation binding the device to the driver in this way.

But if the driver is in the supported list, but not bound to anything,
this won't happen.  new_id won't do anything, because it's already in
the supported list.  So we can't rely on new_id binding for us, and
need to try bind as well (at least if new_id fails).

To reproduce:

    echo 8086 1502 > /sys/bus/pci/drivers/vfio-pci/new_id
    echo 0000:00:19.0 > /sys/bus/pci/drivers/vfio-pci/unbind

(Use lspci -nn to find the right values for your hardware.)
This will leave you with a device that is supported by vfio-pci but
not bound to it, which would previously cause vm-net to fail to start,
but should not after this change.
---
 .../linux/spectrum/testhost/default.nix        | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/pkgs/os-specific/linux/spectrum/testhost/default.nix b/pkgs/os-specific/linux/spectrum/testhost/default.nix
index 7e1a973e8c6..21c585f1490 100644
--- a/pkgs/os-specific/linux/spectrum/testhost/default.nix
+++ b/pkgs/os-specific/linux/spectrum/testhost/default.nix
@@ -142,7 +142,9 @@ let
             printf "%s" $PCI_LOCATION
           }
 
-          # (Re)bind the device to the VFIO PCI driver.
+          # Tell the VFIO driver it should support our device.  This
+          # is allowed to fail because it might already know that, in
+          # which case it'll return EEXIST.
           if { modprobe vfio-pci }
           backtick -in device_id {
             if { dd bs=2 skip=1 count=2 status=none if=''${PCI_PATH}/vendor }
@@ -155,6 +157,20 @@ let
             printf "%s" $device_id
           }
 
+          # Bind the device to the VFIO driver.  This is allowed to
+          # fail because the new_id operation we just tried will have
+          # bound it automatically for us if it succeeded.  In such a
+          # case, the kernel will return ENODEV (conistency!).
+          foreground {
+            redirfd -w 1 /sys/bus/pci/drivers/vfio-pci/bind
+            printf "%s" $PCI_LOCATION
+          }
+
+          # Because we allow both new_id and bind to fail, we need to
+          # manually make sure now that at least one of them succeeded
+          # and the device is actually attached to the vfio-driver.
+          if { test -e /sys/bus/pci/drivers/vfio-pci/''${PCI_LOCATION} }
+
           foreground { mkdir env }
 
           ${cloud-hypervisor}/bin/cloud-hypervisor
-- 
2.31.0


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

* Re: [PATCH nixpkgs] spectrumPackages.spectrum-testhost: fix driver bind
  2021-04-21 10:16 [PATCH nixpkgs] spectrumPackages.spectrum-testhost: fix driver bind Alyssa Ross
@ 2021-04-22  1:27 ` Cole Helbling
  2021-04-22  8:02   ` Alyssa Ross
  0 siblings, 1 reply; 3+ messages in thread
From: Cole Helbling @ 2021-04-22  1:27 UTC (permalink / raw)
  To: Alyssa Ross, devel

On Wed Apr 21, 2021 at 3:16 AM PDT, Alyssa Ross wrote:

[snip]

> ---
>  .../linux/spectrum/testhost/default.nix        | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>

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

* Re: [PATCH nixpkgs] spectrumPackages.spectrum-testhost: fix driver bind
  2021-04-22  1:27 ` Cole Helbling
@ 2021-04-22  8:02   ` Alyssa Ross
  0 siblings, 0 replies; 3+ messages in thread
From: Alyssa Ross @ 2021-04-22  8:02 UTC (permalink / raw)
  To: Cole Helbling; +Cc: devel

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

> [snip]
>
>> ---
>>  .../linux/spectrum/testhost/default.nix        | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>

Pushed as 6234ff3dfb6.  Thanks Cole! :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

end of thread, other threads:[~2021-04-22  8:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 10:16 [PATCH nixpkgs] spectrumPackages.spectrum-testhost: fix driver bind Alyssa Ross
2021-04-22  1:27 ` Cole Helbling
2021-04-22  8:02   ` Alyssa Ross

Code repositories for project(s) associated with this public inbox

	https://spectrum-os.org/git/crosvm
	https://spectrum-os.org/git/doc
	https://spectrum-os.org/git/mktuntap
	https://spectrum-os.org/git/nixpkgs
	https://spectrum-os.org/git/spectrum
	https://spectrum-os.org/git/ucspi-vsock
	https://spectrum-os.org/git/www

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).