Create VGPU changes with VFIO Framework by JunAr7112 · Pull Request #139 · NVIDIA/vgpu-device-manager

JunAr7112 · 2025-11-08T00:34:17Z

No description provided.

copy-pr-bot · 2025-11-08T00:34:20Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

cdesiniotis

Thanks @JunAr7112, this is a good start. As we make iterations on this and get more familiar with the internals here, it may be valuable to create a new internal/vgpu package that hides away the vfio vs mdev framework complexity. We need to think through what the right interface would be, but I imagine we will need methods for 1) getting all vGPU devices, 2) getting all parent devices (of which you can create a vGPU device on top of), 3) creating a vGPU device. The pkg/vgpu/config.go file, which is concerned with getting / setting a particular vGPU config, can invoke these methods without having to know what vfio / mdev is.

cdesiniotis · 2026-01-16T22:13:45Z

+		if ret != nvml.SUCCESS {
+			continue
+		}
+		vgpuConfig[typeName]++


Question -- does the name reported by NVML, e.g. vgpuTypeId.GetName(), align exactly with the name we were using before? (the names stored in vgpuDev.MDEVType)

I added a helper function to ensure they exactly align

So the name reported by NVML contains the product prefix? For example, is NVML returning NVIDIA A100-4C as the type name for the A100-4C device?

Yes I checked this manually earlier and added the parseVGPUTypeName(rawName string) to verify that we would only be getting A100-4C. The name reported by NVML included a prefix.

cdesiniotis · 2026-01-16T23:00:28Z

+		vfnum := 0
+		numVF := int(device.SriovInfo.PhysicalFunction.NumVFs)
+		for vfnum < numVF {
+			vfAddr := filepath.Join(HostPCIDevicesRoot, device.Address, "virtfn"+strconv.Itoa(vfnum), "nvidia")


I don't think the nvidia directory should be included in this path. The "path to the VF" is simply /sys/bus/pci/devices/<BDF>/virtfn<N>. Other parts of the code are not intuitive to me because vfAddr includes the nvidia directory.

local-agadiyar@ipp1-2284:/sys/bus/pci/devices/0000:41:00.0/virtfn0/nvidia$ cat current_vgpu_type
687
local-agadiyar@ipp1-2284:/sys/bus/pci/devices/0000:41:00.0/virtfn0/nvidia$ cat creatable_vgpu_types
ID : vGPU Name

The current_vgpu_type and creatable_vgpu_types files are located in the nvidia folder. This way we don't need to append nvidia onto another address variable

cdesiniotis · 2026-01-16T23:07:33Z

+				return nil, fmt.Errorf("virtual function %d at address %s does not exist", vfnum, vfAddr)
+			}
+			parentDevices = append(parentDevices, &ParentDevice{
+				NvidiaPCIDevice: device,


Question: Shouldn't NvidiaPCIDevice represent the VF (IIUC device is currently a PF)? If so, then I don't see the need to have VirtualFunctionaPath as a separate field. If we instead used the VF here, I think that would simplify the code in a few places and make this easier to read. Note, the nvpci.NvidiaPCIDevice type allows you to go from the VF to the backing PF via device.SriovInfo.VirtualFunction.PhysicalFunction.

I think the issue here is that nvpci doesn't have a built in way to get virtual functions from the physical function. That is why I am storing the physical device ( via nvdevices, err := m.nvlib.Nvpci.GetGPUs() ) as well as the path to the virtual function.

cdesiniotis · 2026-01-16T23:09:47Z

+	}
+	devices := []*Device{}
+	for _, parentDevice := range parentDevices {
+			vgpuTypeNumberBytes, err := os.ReadFile(filepath.Join(parentDevice.VirtualFunctionPath, "current_vgpu_type"))


As indicated in a prior comment, if parentDevice was just of type nvpci.NvidiaPCIDevice (and represented the VF), this code would be replaced by:

vgpuTypeNumberBytes, err := os.ReadFile(filepath.Join(parentDevice.NvidiaPCIDevice.Path, "current_vgpu_type"))

See above comment.

cdesiniotis · 2026-01-16T23:14:40Z

+// ParentDevice represents an NVIDIA parent PCI device.
+type ParentDevice struct {
+	*nvpci.NvidiaPCIDevice
+	VirtualFunctionPath string


Why is this field needed? Shouldn't NvidiaPCIDevice.Path represent the path to the virtual function?

See above comment. nvpci.NvidiaPCIDevice is storing the physical function

cdesiniotis · 2026-01-21T18:14:37Z

 }

 type nvlibVGPUConfigManager struct {
-	nvlib nvlib.Interface


Question -- does nvlib.Interface need to exist anymore?

No we are no longer using the nvlib.Interface. I don't think we need it for any of the other projects either

cdesiniotis · 2026-01-21T18:16:55Z

+		if ret != nvml.SUCCESS {
+			continue
+		}
+		vgpuConfig[typeName]++


So the name reported by NVML contains the product prefix? For example, is NVML returning NVIDIA A100-4C as the type name for the A100-4C device?

cdesiniotis · 2026-01-21T18:29:35Z

+		found := false
+		for _, vgpuTypeId := range supportedVGPUs {
+			rawName, ret := vgpuTypeId.GetName()
+			if ret != nvml.SUCCESS {
+				continue
+			}
+			name := parseVGPUTypeName(rawName)
+			if name == key {
+				found = true
+				sanitizedConfig[key] = val
+				break
+			}
+			if name == strippedKey {
+				found = true
+				sanitizedConfig[strippedKey] = val
+				break
+			}
+		}


To improve readability, what if we constructed a map, named supportedVgpuTypes, prior as such:

supportedVgpuTypes := map[string]bool{} for _, vgpu := range supportedVGPUs { name, ret := vgpu.GetName() if ret != nvml.SUCCESS { continue } name = parseVGPUTypeName(name) supportedVgpuNames[name] = true }

Then this for loop would simplify to

Suggested change

found := false

for _, vgpuTypeId := range supportedVGPUs {

rawName, ret := vgpuTypeId.GetName()

if ret != nvml.SUCCESS {

continue

}

name := parseVGPUTypeName(rawName)

if name == key {

found = true

sanitizedConfig[key] = val

break

}

if name == strippedKey {

found = true

sanitizedConfig[strippedKey] = val

break

}

}

if _, ok := supportedVgpuTypes[key]; ok {

sanitizedConfig[key] = val

} else if _, ok := supportedVgpuTypes[strippedKey]; ok {

sanitizedConfig[strippedKey] = val

} else {

return fmt.Errorf("vGPU type %s is not supported on GPU (index=%d, address=%s)", key, gpu, device.Address)

}

Ok I broke this into two for loops.

Signed-off-by: Arjun <[email protected]>

cdesiniotis · 2026-05-01T16:29:06Z

@JunAr7112 I am going to close this as not planned for now -- this will need to be re-evaluated if this feature gets prioritized again. We have these changes on your fork if we ever need to reference them.

JunAr7112 force-pushed the vfio_changes branch 2 times, most recently from cbe892c to 3f31828 Compare November 10, 2025 17:09

tariq1890 reviewed Nov 10, 2025

View reviewed changes

Comment thread pkg/vgpu/config.go Resolved

Comment thread pkg/vgpu/config.go Resolved

Comment thread pkg/vgpu/config.go Resolved

JunAr7112 force-pushed the vfio_changes branch from 1682d72 to 4a52cbe Compare November 10, 2025 18:58

JunAr7112 requested a review from cdesiniotis November 12, 2025 21:28

cdesiniotis reviewed Nov 18, 2025

View reviewed changes

JunAr7112 force-pushed the vfio_changes branch 6 times, most recently from 5daf473 to 89a1e62 Compare November 25, 2025 21:03

JunAr7112 force-pushed the vfio_changes branch 9 times, most recently from 50e2185 to 58c7cc6 Compare December 9, 2025 22:31

cdesiniotis reviewed Jan 13, 2026

View reviewed changes

Comment thread pkg/vgpu/config.go Resolved

Comment thread internal/vfio/vfio.go Resolved

Comment thread pkg/vgpu/config.go Resolved

JunAr7112 force-pushed the vfio_changes branch 3 times, most recently from d134551 to d49bf31 Compare January 16, 2026 00:25

cdesiniotis reviewed Jan 16, 2026

View reviewed changes

cdesiniotis reviewed Jan 17, 2026

View reviewed changes

JunAr7112 force-pushed the vfio_changes branch 2 times, most recently from d2246a9 to 2a5d547 Compare January 20, 2026 17:59

JunAr7112 force-pushed the vfio_changes branch 3 times, most recently from cfffa9f to 6eaa198 Compare January 20, 2026 22:51

cdesiniotis reviewed Jan 21, 2026

View reviewed changes

JunAr7112 force-pushed the vfio_changes branch 2 times, most recently from 7bc7d3d to 7e25ec0 Compare March 24, 2026 22:01

JunAr7112 force-pushed the vfio_changes branch from 7e25ec0 to 4309b52 Compare April 1, 2026 21:48

JunAr7112 added 12 commits April 1, 2026 22:31

created vfio framework

3729cc8

updated to one file

ab90719

updated setVGPUConfig

ff3fdf1

Signed-off-by: Arjun <[email protected]>

updated variables

885e7ea

updated isVFIOEnabled

ed0b482

Signed-off-by: Arjun <[email protected]>

updated with feedback

fe2965b

added vfio framework

b714105

Signed-off-by: Arjun <[email protected]>

added vgpu-combined

b0aad3e

Signed-off-by: Arjun <[email protected]>

updated to use nvml

dda947a

updated vfio to represent virtual function

a548e3e

refactored code to three interfaces

e4bf138

update March 2026

6c21213

Signed-off-by: Arjun <[email protected]>

JunAr7112 force-pushed the vfio_changes branch from 4309b52 to 6c21213 Compare April 1, 2026 22:32

cdesiniotis closed this May 1, 2026

karthikvetrivel mentioned this pull request Aug 31, 2026

feat(vgpu): support the vendor-specific VFIO framework on Ada/Hopper+ GPUs #195

Closed

Conversation

JunAr7112 commented Nov 8, 2025

Uh oh!

copy-pr-bot Bot commented Nov 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cdesiniotis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JunAr7112 Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JunAr7112 Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cdesiniotis commented May 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

JunAr7112 Mar 20, 2026 •

edited

Loading

JunAr7112 Jan 20, 2026 •

edited

Loading