diff --git a/pkg/nvpassthrough/nvpassthrough.go b/pkg/nvpassthrough/nvpassthrough.go index 469b5ed..578fec0 100644 --- a/pkg/nvpassthrough/nvpassthrough.go +++ b/pkg/nvpassthrough/nvpassthrough.go @@ -17,9 +17,12 @@ package nvpassthrough import ( + "errors" "fmt" + "io/fs" "os" "path/filepath" + "strconv" "strings" "github.com/NVIDIA/go-nvlib/pkg/nvpci" @@ -162,8 +165,8 @@ func (n *nvpassthrough) FindBestVFIOVariant(address string) (string, error) { // BindToVFIODriver binds the provided NVIDIA PCI device to the // vfio-pci driver (or a variant VFIO driver if one is preferred). // This function takes care of additional logic, like making sure -// the vfio-pci driver is loaded first and that an auxiliary graphics -// device also get bound to the vfio-pci driver. +// the vfio-pci driver is loaded first and that every other PCI function +// of the same card also gets bound to the vfio-pci driver. func (n *nvpassthrough) BindToVFIODriver(address string) error { device, err := n.nvpciLib.GetNvidiaDeviceByPciBusID(address) if err != nil { @@ -221,25 +224,25 @@ func (n *nvpassthrough) BindToVFIODriver(address string) error { } } - // For graphics mode, bind the auxiliary device as well - auxDev, err := getGraphicsAuxDev(device) + // Bind every other function of the same card. The whole IOMMU group must be bound to a + // vfio driver (or to nothing) before VFIO will hand it to a guest. + auxDevs, err := getAuxDevices(pciDevicesRoot, device) if err != nil { - return fmt.Errorf("failed to get graphics auxiliary device for %s: %w", device.Address, err) - } - if auxDev == nil { - return nil - } - if auxDev.Driver == vfioDriverName { - return nil + return fmt.Errorf("failed to get auxiliary devices for %s: %w", device.Address, err) } + for _, auxDev := range auxDevs { + if auxDev.Driver == vfioDriverName { + continue + } - n.logger.Infof("Binding graphics auxiliary device %s to driver: %s", auxDev.Address, vfioDriverName) + n.logger.Infof("Binding auxiliary device %s to driver: %s", auxDev.Address, vfioDriverName) - if err := unbind(auxDev.Address); err != nil { - return fmt.Errorf("failed to unbind graphics auxiliary device %s: %w", auxDev.Address, err) - } - if err := bind(auxDev.Address, vfioDriverName); err != nil { - return fmt.Errorf("failed to bind graphics auxiliary device %s to %s: %w", auxDev, vfioDriverName, err) + if err := unbind(auxDev.Address); err != nil { + return fmt.Errorf("failed to unbind auxiliary device %s: %w", auxDev.Address, err) + } + if err := bind(auxDev.Address, vfioDriverName); err != nil { + return fmt.Errorf("failed to bind auxiliary device %s to %s: %w", auxDev.Address, vfioDriverName, err) + } } return nil @@ -260,7 +263,7 @@ func (n *nvpassthrough) BindToDriver(address string, driver string) error { // Unbind unbinds the provided NVIDIA PCI Device from // any driver it is currently bound to. This function also ensures -// an auxiliary graphics device is also unbound. +// that every other PCI function of the same card is unbound. func (n *nvpassthrough) Unbind(address string) error { device, err := n.nvpciLib.GetNvidiaDeviceByPciBusID(address) if err != nil { @@ -274,14 +277,14 @@ func (n *nvpassthrough) Unbind(address string) error { return fmt.Errorf("failed to unbind device %s: %w", address, err) } - // For graphics mode, unbind the auxiliary device as well - auxDev, err := getGraphicsAuxDev(device) + // Unbind every other function of the same card, mirroring BindToVFIODriver. + auxDevs, err := getAuxDevices(pciDevicesRoot, device) if err != nil { - return fmt.Errorf("failed to get graphics auxiliary device for %s: %w", address, err) + return fmt.Errorf("failed to get auxiliary devices for %s: %w", address, err) } - if auxDev != nil { + for _, auxDev := range auxDevs { if err := unbind(auxDev.Address); err != nil { - return fmt.Errorf("failed to unbind graphics auxiliary device %s: %w", auxDev.Address, err) + return fmt.Errorf("failed to unbind auxiliary device %s: %w", auxDev.Address, err) } } @@ -327,46 +330,120 @@ func unbind(address string) error { return nil } -func getGraphicsAuxDev(device *nvpci.NvidiaPCIDevice) (*nvidiaPCIAuxDevice, error) { - if device.Class != nvpci.PCIVgaControllerClass { +// getAuxDevices returns every other PCI function that belongs to the same physical card as +// device. +// +// VFIO assigns an entire IOMMU group to a guest, and refuses the group unless every device in +// it is bound to a vfio driver or to no driver at all. A discrete NVIDIA GPU is a +// multi-function PCI device: .0 VGA/3D, .1 HDMI audio, and on Turing-era boards .2 +// (VirtualLink USB xHCI) and .3 (USB-C UCSI). If any of those is left bound to a host driver +// such as xhci_hcd, the group is not viable and passthrough fails with +// "vfio: group N is not viable". +// +// Functions are discovered two ways, and the results are unioned: +// +// - Sibling PCI functions sharing the same domain:bus:device. This is the only way to find +// the VirtualLink xHCI and USB-C UCSI functions, which do not create a device link back +// to the GPU. +// - "consumer:pci:" device links, which the HDA audio function does create. +// +// Siblings are scoped to the physical card rather than to the IOMMU group on purpose: where +// ACS is unavailable a group can span an entire root port, and unbinding unrelated devices +// from their drivers would be harmful. +func getAuxDevices(devicesRoot string, device *nvpci.NvidiaPCIDevice) ([]*nvidiaPCIAuxDevice, error) { + if !device.IsGPU() { return nil, nil } - // Look for consumer symlink - entries, err := os.ReadDir(device.Path) - if err != nil { - return nil, err - } + seen := map[string]bool{device.Address: true} + var auxDevs []*nvidiaPCIAuxDevice - for _, entry := range entries { - if strings.HasPrefix(entry.Name(), "consumer") { - // Extract aux device name from consumer:pci:XXXX:XX:XX.X format - _, address, ok := strings.Cut(entry.Name(), consumerPrefix) - if !ok || address == "" { - continue + add := func(address string) error { + if address == "" || seen[address] { + return nil + } + path := filepath.Join(devicesRoot, address) + if _, err := os.Stat(path); err != nil { + if errors.Is(err, fs.ErrNotExist) { + // The function is not present on this host; there is nothing to bind. + return nil } + return fmt.Errorf("failed to stat auxiliary device %s: %w", address, err) + } + // Auxiliary functions of a GPU are by definition the same vendor. Checking guards + // against acting on a device we did not intend to touch. + isNvidia, err := isNvidiaVendor(path) + switch { + case errors.Is(err, fs.ErrNotExist): + // The function was removed between the stat above and this read. + return nil + case err != nil: + return fmt.Errorf("failed to check vendor for auxiliary device %s: %w", address, err) + case !isNvidia: + return nil + } + driver, err := getDriver(path) + if err != nil { + return fmt.Errorf("failed to get driver for auxiliary device %s: %w", address, err) + } + seen[address] = true + auxDevs = append(auxDevs, &nvidiaPCIAuxDevice{ + Path: path, + Address: address, + Driver: driver, + }) + return nil + } - // Check if aux device exists - path := filepath.Join(pciDevicesRoot, address) - if _, err := os.Stat(path); err != nil { + // Sibling functions: same domain:bus:device, differing only in function number. + if slotPrefix, _, ok := strings.Cut(device.Address, "."); ok { + slotPrefix += "." + entries, err := os.ReadDir(devicesRoot) + if err != nil { + return nil, fmt.Errorf("failed to list PCI devices in %s: %w", devicesRoot, err) + } + for _, entry := range entries { + if !strings.HasPrefix(entry.Name(), slotPrefix) { continue } - - auxDev := &nvidiaPCIAuxDevice{ - Path: path, - Address: address, + if err := add(entry.Name()); err != nil { + return nil, err } + } + } - driver, err := getDriver(path) - if err != nil { - return nil, fmt.Errorf("failed to get driver for graphics auxiliary device %s: %w", address, err) - } - auxDev.Driver = driver - return auxDev, nil + // Consumer device links, e.g. the HDA audio function. + entries, err := os.ReadDir(device.Path) + if err != nil { + return nil, fmt.Errorf("failed to list %s: %w", device.Path, err) + } + for _, entry := range entries { + if !strings.HasPrefix(entry.Name(), "consumer") { + continue + } + _, address, ok := strings.Cut(entry.Name(), consumerPrefix) + if !ok { + continue + } + if err := add(address); err != nil { + return nil, err } } - return nil, nil + return auxDevs, nil +} + +// isNvidiaVendor reports whether the PCI device at devicePath is an NVIDIA device. +func isNvidiaVendor(devicePath string) (bool, error) { + contents, err := os.ReadFile(filepath.Join(devicePath, "vendor")) + if err != nil { + return false, fmt.Errorf("failed to read vendor of %s: %w", devicePath, err) + } + vendorID, err := strconv.ParseUint(strings.TrimPrefix(strings.TrimSpace(string(contents)), "0x"), 16, 16) + if err != nil { + return false, fmt.Errorf("failed to parse vendor of %s: %w", devicePath, err) + } + return uint16(vendorID) == nvpci.PCINvidiaVendorID, nil } func getDriver(devicePath string) (string, error) { diff --git a/pkg/nvpassthrough/nvpassthrough_test.go b/pkg/nvpassthrough/nvpassthrough_test.go new file mode 100644 index 0000000..64bdf04 --- /dev/null +++ b/pkg/nvpassthrough/nvpassthrough_test.go @@ -0,0 +1,261 @@ +/* + * Copyright (c) NVIDIA CORPORATION. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nvpassthrough + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/NVIDIA/go-nvlib/pkg/nvpci" +) + +// pciFunc describes one PCI function to materialise in the fake sysfs tree. +type pciFunc struct { + address string + vendor uint16 + driver string // empty means unbound +} + +// newFakePCITree builds a minimal /sys/bus/pci/devices layout and returns its path. +func newFakePCITree(t *testing.T, funcs []pciFunc, consumerLinksOn string, consumers []string) string { + t.Helper() + root := t.TempDir() + devicesRoot := filepath.Join(root, "devices") + driversRoot := filepath.Join(root, "drivers") + require.NoError(t, os.MkdirAll(devicesRoot, 0755)) + require.NoError(t, os.MkdirAll(driversRoot, 0755)) + + for _, f := range funcs { + devPath := filepath.Join(devicesRoot, f.address) + require.NoError(t, os.MkdirAll(devPath, 0755)) + vendor := fmt.Sprintf("0x%04x\n", f.vendor) + require.NoError(t, os.WriteFile(filepath.Join(devPath, "vendor"), []byte(vendor), 0644)) + if f.driver != "" { + drvPath := filepath.Join(driversRoot, f.driver) + require.NoError(t, os.MkdirAll(drvPath, 0755)) + require.NoError(t, os.Symlink(drvPath, filepath.Join(devPath, "driver"))) + } + } + + // "consumer:pci:" entries live in the GPU's own sysfs directory. + for _, c := range consumers { + require.NoError(t, os.MkdirAll( + filepath.Join(devicesRoot, consumerLinksOn, consumerPrefix+c), 0755)) + } + return devicesRoot +} + +func addressesOf(devs []*nvidiaPCIAuxDevice) []string { + out := make([]string, 0, len(devs)) + for _, d := range devs { + out = append(out, d.Address) + } + sort.Strings(out) + return out +} + +func TestGetAuxDevices(t *testing.T) { + const nvidia = nvpci.PCINvidiaVendorID + + testCases := []struct { + description string + funcs []pciFunc + consumers []string + gpu string + class uint32 // defaults to PCIVgaControllerClass + expected []string + }{ + { + description: "turing board: audio, VirtualLink xHCI and USB-C UCSI are all returned", + // .2/.3 have no consumer link -- they are only reachable via sibling enumeration + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:16:00.1", nvidia, "vfio-pci"}, + {"0000:16:00.2", nvidia, "xhci_hcd"}, + {"0000:16:00.3", nvidia, ""}, + }, + consumers: []string{"0000:16:00.1"}, + gpu: "0000:16:00.0", + expected: []string{"0000:16:00.1", "0000:16:00.2", "0000:16:00.3"}, + }, + { + description: "single-function GPU has no auxiliary devices", + funcs: []pciFunc{{"0000:9b:00.0", nvidia, "vfio-pci"}}, + gpu: "0000:9b:00.0", + expected: []string{}, + }, + { + description: "a sibling function from another vendor is not touched", + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:16:00.1", 0x8086, "snd_hda_intel"}, + }, + gpu: "0000:16:00.0", + expected: []string{}, + }, + { + description: "functions on a different slot are not siblings", + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:17:00.0", nvidia, "vfio-pci"}, + }, + gpu: "0000:16:00.0", + expected: []string{}, + }, + { + description: "GPU with a single HDA audio function is still returned", + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:16:00.1", nvidia, "snd_hda_intel"}, + }, + consumers: []string{"0000:16:00.1"}, + gpu: "0000:16:00.0", + expected: []string{"0000:16:00.1"}, + }, + { + description: "auxiliary device reachable only via a consumer link is still found", + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:20:00.0", nvidia, "snd_hda_intel"}, + }, + consumers: []string{"0000:20:00.0"}, + gpu: "0000:16:00.0", + expected: []string{"0000:20:00.0"}, + }, + { + description: "3D-controller-class GPU has its auxiliary functions returned", + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:16:00.1", nvidia, "snd_hda_intel"}, + }, + gpu: "0000:16:00.0", + class: nvpci.PCI3dControllerClass, + expected: []string{"0000:16:00.1"}, + }, + { + description: "NVSwitch has no auxiliary functions handled", + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:16:00.1", nvidia, "snd_hda_intel"}, + }, + gpu: "0000:16:00.0", + class: nvpci.PCINvSwitchClass, + expected: []string{}, + }, + { + description: "a function found both as a sibling and via a consumer link appears once", + funcs: []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:16:00.1", nvidia, "snd_hda_intel"}, + }, + consumers: []string{"0000:16:00.1"}, + gpu: "0000:16:00.0", + expected: []string{"0000:16:00.1"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + devicesRoot := newFakePCITree(t, tc.funcs, tc.gpu, tc.consumers) + class := tc.class + if class == 0 { + class = nvpci.PCIVgaControllerClass + } + dev := &nvpci.NvidiaPCIDevice{ + Address: tc.gpu, + Path: filepath.Join(devicesRoot, tc.gpu), + Class: class, + } + + auxDevs, err := getAuxDevices(devicesRoot, dev) + require.NoError(t, err) + require.ElementsMatch(t, tc.expected, addressesOf(auxDevs)) + }) + } +} + +// The driver currently bound to each auxiliary function must be reported, since +// BindToVFIODriver skips functions already on the vfio driver and unbinds the rest. +func TestGetAuxDevicesReportsCurrentDriver(t *testing.T) { + const nvidia = nvpci.PCINvidiaVendorID + funcs := []pciFunc{ + {"0000:16:00.0", nvidia, "vfio-pci"}, + {"0000:16:00.1", nvidia, "vfio-pci"}, + {"0000:16:00.2", nvidia, "xhci_hcd"}, + {"0000:16:00.3", nvidia, ""}, + } + devicesRoot := newFakePCITree(t, funcs, "0000:16:00.0", nil) + dev := &nvpci.NvidiaPCIDevice{ + Address: "0000:16:00.0", + Path: filepath.Join(devicesRoot, "0000:16:00.0"), + Class: nvpci.PCIVgaControllerClass, + } + + auxDevs, err := getAuxDevices(devicesRoot, dev) + require.NoError(t, err) + + got := map[string]string{} + for _, d := range auxDevs { + got[d.Address] = d.Driver + } + require.Equal(t, map[string]string{ + "0000:16:00.1": "vfio-pci", + "0000:16:00.2": "xhci_hcd", + "0000:16:00.3": "", + }, got) +} + +func TestGetAuxDevicesMalformedVendorIsAnError(t *testing.T) { + funcs := []pciFunc{ + {"0000:16:00.0", nvpci.PCINvidiaVendorID, "vfio-pci"}, + {"0000:16:00.1", nvpci.PCINvidiaVendorID, "snd_hda_intel"}, + } + devicesRoot := newFakePCITree(t, funcs, "0000:16:00.0", nil) + require.NoError(t, os.WriteFile( + filepath.Join(devicesRoot, "0000:16:00.1", "vendor"), []byte("not-a-vendor-id\n"), 0644)) + + dev := &nvpci.NvidiaPCIDevice{ + Address: "0000:16:00.0", + Path: filepath.Join(devicesRoot, "0000:16:00.0"), + Class: nvpci.PCIVgaControllerClass, + } + + _, err := getAuxDevices(devicesRoot, dev) + require.ErrorContains(t, err, "0000:16:00.1") +} + +// A consumer link may name a function that is not present, and a function may be +// removed while the tree is being walked. Both are tolerated and simply skipped. +func TestGetAuxDevicesToleratesAbsentFunction(t *testing.T) { + funcs := []pciFunc{{"0000:16:00.0", nvpci.PCINvidiaVendorID, "vfio-pci"}} + devicesRoot := newFakePCITree(t, funcs, "0000:16:00.0", []string{"0000:16:00.1"}) + + dev := &nvpci.NvidiaPCIDevice{ + Address: "0000:16:00.0", + Path: filepath.Join(devicesRoot, "0000:16:00.0"), + Class: nvpci.PCIVgaControllerClass, + } + + auxDevs, err := getAuxDevices(devicesRoot, dev) + require.NoError(t, err) + require.Empty(t, auxDevs) +}