-
Notifications
You must be signed in to change notification settings - Fork 34
nvpassthrough: bind every PCI function of a GPU, not just the first auxiliary one #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jjacobelli
wants to merge
1
commit into
NVIDIA:main
Choose a base branch
from
jjacobelli:fix/bind-all-gpu-functions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ import ( | |
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "github.com/NVIDIA/go-nvlib/pkg/nvpci" | ||
|
|
@@ -162,8 +163,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 +222,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 +261,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 +275,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 +328,111 @@ 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 { | ||
| // The function is not present on this host; there is nothing to bind. | ||
| return nil | ||
| } | ||
| // 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) | ||
| if err != nil || !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) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also handle the |
||
| 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) { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.