* [PATCH 00/13] Introduce NVIDIA vGPU manager and VFIO variant driver
@ 2026-09-05 8:11 Zhi Wang
2026-09-05 8:11 ` [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU " Zhi Wang
0 siblings, 1 reply; 24+ messages in thread
From: Zhi Wang @ 2026-09-05 8:11 UTC (permalink / raw)
To: dakr, acourbot
Cc: alex, jgg, yishaih, skolothumtho, kevin.tian, airlied, simona,
ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin,
a.hindborg, aliceryhl, tmgross, jhubbard, ecourtney, cjia, smitra,
kjaju, alkumar, ankita, aniketa, kwankhede, targupta, nova-gpu,
linux-kernel, zhiwang, Zhi Wang, kvm
Add the nova-core support needed to create and manage NVIDIA vGPU
instances, together with the NVIDIA vGPU VFIO variant driver that consumes
the lifecycle interface.
The control path introduced by this series is:
+-----------------------+ +-----------------------+
| Linux guest | | Windows guest |
| NVIDIA guest driver | | NVIDIA guest driver |
+-----------+-----------+ +-----------+-----------+
| |
+-----------v-----------+ +-----------v-----------+
| QEMU (VFIO user) | | QEMU (VFIO user) |
+-----------+-----------+ +-----------+-----------+
+----------------+----------------+
|
+-------v-------+
| VFIO core |
+-------+-------+
|
+---------------------v---------------------+
| NVIDIA vGPU VFIO variant driver |
+---------------------+---------------------+
|
+---------------+----------------------+
| binds PCI VF | PF lifecycle API
| | open / close / reset
v v
+--------+--------+ +-------------+-------------+
| PCI VF(s) | | nova-core (PF driver) |
| (virtual GPU) | | vGPU Manager |
+--------+--------+ | VRAM / channels ... |
| +-------------+-------------+
| |
+-------------v--------------------------------------v----------------+
| NVIDIA physical GPU |
| VF resources PF / GSP / VRAM |
+---------------------------------------------------------------------+
On the nova-core side, the PF driver owns the GPU firmware-facing part
of vGPU management. It already detects vGPU mode before GSP boot and
supplies the SR-IOV topology to the GPU System Processor firmware
(GSP-RM). This series extends it to:
- Retain the firmware-reported VMMU segment size and FIFO engine
ordering after GSP_INIT, together with nova-core's channel capacity.
- Add VramBlock and VramRegion ownership plus Bar1Map for bounded CPU
mappings of VRAM-backed control structures.
- Query GSP-RM through its command queue for the vGPU type already
assigned to a VF and its properties.
- Build a layout-validated, profile-wide pool of paired framebuffer and
management-heap slots, reserve contiguous channel ranges, and keep
live allocations in a per-PF instance registry.
- Use the typed r000 firmware bindings for control and response layouts,
message IDs, and firmware-defined region sizes.
- Handle GMC transactions over the GSP queues, match GMC replies by
command and sequence, dispatch interleaved RM RPCs from the shared
GSP-to-CPU message queue, and consume stale GMC messages.
- Implement the GMCAPI bootload, shutdown, and cleanup operations with
typed channel and resource maps for each plugin instance.
- Establish the BAR1-backed PluginRpc channel, negotiate its protocol,
and send the VM configuration and plugin BME-state update.
- Use per-instance CeUtils channels to scrub guest framebuffer memory
on allocation, reset, and shutdown. If ownership or completion is
uncertain, keep the affected VRAM and channels out of the allocators.
- Expose the three per-instance plugin log buffers through debugfs with
the header needed by nvlog_decoder.
- Keep resource locking, teardown, and rollback inside nova-core.
- Select the firmware-defined 48-VM WPR2 heap when a device advertises
more than 32 VFs.
On the VFIO side, the NVIDIA vGPU VFIO variant driver:
- Binds only to an NVIDIA VF explicitly selected through
driver_override.
- Derives the guest function ID (GFID) from the SR-IOV VF index and
registers a vfio-pci-core device.
- Presents the vGPU PCI device and subsystem IDs returned by nova-core,
and limits the reported framebuffer BAR size.
- Delegates the remaining PCI and VFIO operations to vfio-pci-core and
uses the standard VFIO physical-device helpers for IOMMUFD.
The ownership boundary between the two drivers is visible at these
lifecycle points:
- Open: after vfio_pci_core_enable(), the variant driver passes the PF
returned by pci_physfn(vf), the GFID, the VF's PCI
domain/bus/device/function (DBDF), and the calling process TGID to
nvidia_vgpu_open(). nova-core validates the PF and GFID, creates and
activates the instance, and returns the guest-visible PCI IDs and
BAR1 size. On failure, the variant driver disables the vfio-pci-core
device; on success, it calls
vfio_pci_core_finish_enable().
- Reset: the variant driver first calls nvidia_vgpu_reset(). nova-core
sends the plugin reset RPC and scrubs the guest framebuffer. Only
after that succeeds is VFIO_DEVICE_RESET passed to vfio-pci-core for
the PCI function reset.
- Close: the variant driver invokes the void nvidia_vgpu_close()
interface to request plugin shutdown, framebuffer scrubbing, and
resource release from nova-core, then closes the vfio-pci-core
device.
The cross-module interface is therefore limited to three GPL-only
symbols in NOVA_CORE_VGPU: nvidia_vgpu_open(), nvidia_vgpu_close(), and
nvidia_vgpu_reset().
This is a ground-up rework of the earlier vGPU RFC [1]. In particular,
the vGPU manager has been moved from the VFIO driver into the nova-core
implementation, and the VFIO side no longer carries copies of RM firmware
headers. The VFIO driver attaches to the VF and uses its PF's nova-core
state for management, as discussed in that thread.
The patches are organized as follows:
1-4 Initialize VgpuManager and add the memory and firmware building
blocks.
5-8 Allocate instances and implement GMC, plugin boot, and RPC.
9-10 Scrub guest framebuffer memory and expose plugin diagnostics.
11-12 Export the lifecycle API and add its VFIO consumer.
13 Select the larger WPR2 heap for 48-VF devices.
The series is based on the nova-core r000 GSP, memory-management,
SR-IOV, Rust bitmap/id-pool, and PCI abstraction work. The exact base
commit is recorded below, and the complete prerequisite stack is
available in [2].
[1] https://lore.kernel.org/kvm/20250903221111.3866249-1-zhiw@nvidia.com/
[2] https://github.com/zhiwang-nvidia/nova-core/tree/zhi/nova-vgpu-wip-nova-gsp-20260902
Alok Kumar (1):
gpu: nova-core: vgpu: add VRAM slot allocator
Zhi Wang (12):
gpu: nova-core: vgpu: add post-GSP-boot vGPU initialization
gpu: nova-core: mm: add VramBlock and Bar1Map
gpu: nova-core: vgpu: add r000 plugin bindings
gpu: nova-core: vgpu: add instance create/destroy
gpu: nova-core: gsp: add GMC transaction helpers
gpu: nova-core: vgpu: add vGPU bootload
gpu: nova-core: vgpu: implement PluginRpc channel and config params
gpu: nova-core: vgpu: scrub guest framebuffer memory with CeUtils
gpu: nova-core: vgpu: export plugin log buffers via debugfs
gpu: nova-core: vgpu: export lifecycle operations to VFIO
vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
gpu: nova-core: reserve the 48-VM WPR2 heap
drivers/gpu/nova-core/driver.rs | 42 +-
drivers/gpu/nova-core/fb.rs | 2 +-
drivers/gpu/nova-core/gpu.rs | 184 +++--
drivers/gpu/nova-core/gpu/channel.rs | 3 +
drivers/gpu/nova-core/gsp.rs | 29 +-
drivers/gpu/nova-core/gsp/boot.rs | 13 +-
drivers/gpu/nova-core/gsp/cmdq.rs | 356 +++++++--
drivers/gpu/nova-core/gsp/commands.rs | 78 +-
drivers/gpu/nova-core/gsp/fw.rs | 63 +-
drivers/gpu/nova-core/gsp/fw/commands.rs | 185 ++++-
.../gpu/nova-core/gsp/fw/r000_00/bindings.rs | 175 +++++
drivers/gpu/nova-core/gsp/hal/gh100.rs | 2 +-
drivers/gpu/nova-core/gsp/hal/tu102.rs | 2 +-
drivers/gpu/nova-core/mm.rs | 6 +-
drivers/gpu/nova-core/mm/bar_user.rs | 183 ++++-
drivers/gpu/nova-core/mm/vram.rs | 187 +++++
drivers/gpu/nova-core/nova_core_exports.c | 5 +
drivers/gpu/nova-core/vgpu.rs | 91 ---
drivers/gpu/nova-core/vgpu/bootload.rs | 162 ++++
drivers/gpu/nova-core/vgpu/consts.rs | 33 +
drivers/gpu/nova-core/vgpu/fw.rs | 490 ++++++++++++
drivers/gpu/nova-core/vgpu/fw/commands.rs | 28 +
drivers/gpu/nova-core/vgpu/instance.rs | 705 ++++++++++++++++++
drivers/gpu/nova-core/vgpu/log.rs | 167 +++++
drivers/gpu/nova-core/vgpu/mod.rs | 170 +++++
drivers/gpu/nova-core/vgpu/plugin_rpc.rs | 274 +++++++
drivers/gpu/nova-core/vgpu/scrubber.rs | 474 ++++++++++++
drivers/gpu/nova-core/vgpu/vfio.rs | 282 +++++++
drivers/gpu/nova-core/vgpu/vram.rs | 140 ++++
drivers/vfio/pci/Kconfig | 2 +
drivers/vfio/pci/Makefile | 2 +
drivers/vfio/pci/nvidia-vgpu/Kconfig | 16 +
drivers/vfio/pci/nvidia-vgpu/Makefile | 2 +
drivers/vfio/pci/nvidia-vgpu/main.c | 253 +++++++
include/drm/nvidia_vgpu.h | 28 +
rust/bindings/bindings_helper.h | 1 +
36 files changed, 4536 insertions(+), 299 deletions(-)
create mode 100644 drivers/gpu/nova-core/mm/vram.rs
delete mode 100644 drivers/gpu/nova-core/vgpu.rs
create mode 100644 drivers/gpu/nova-core/vgpu/bootload.rs
create mode 100644 drivers/gpu/nova-core/vgpu/consts.rs
create mode 100644 drivers/gpu/nova-core/vgpu/fw.rs
create mode 100644 drivers/gpu/nova-core/vgpu/fw/commands.rs
create mode 100644 drivers/gpu/nova-core/vgpu/instance.rs
create mode 100644 drivers/gpu/nova-core/vgpu/log.rs
create mode 100644 drivers/gpu/nova-core/vgpu/mod.rs
create mode 100644 drivers/gpu/nova-core/vgpu/plugin_rpc.rs
create mode 100644 drivers/gpu/nova-core/vgpu/scrubber.rs
create mode 100644 drivers/gpu/nova-core/vgpu/vfio.rs
create mode 100644 drivers/gpu/nova-core/vgpu/vram.rs
create mode 100644 drivers/vfio/pci/nvidia-vgpu/Kconfig
create mode 100644 drivers/vfio/pci/nvidia-vgpu/Makefile
create mode 100644 drivers/vfio/pci/nvidia-vgpu/main.c
create mode 100644 include/drm/nvidia_vgpu.h
base-commit: a1dc9fdea9ab15e5df1bd6af1f6de2e40079f02d
--
2.53.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-05 8:11 [PATCH 00/13] Introduce NVIDIA vGPU manager and VFIO variant driver Zhi Wang
@ 2026-09-05 8:11 ` Zhi Wang
2026-09-09 3:00 ` Alex Williamson
2026-09-11 20:39 ` Danilo Krummrich
0 siblings, 2 replies; 24+ messages in thread
From: Zhi Wang @ 2026-09-05 8:11 UTC (permalink / raw)
To: dakr, acourbot
Cc: alex, jgg, yishaih, skolothumtho, kevin.tian, airlied, simona,
ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin,
a.hindborg, aliceryhl, tmgross, jhubbard, ecourtney, cjia, smitra,
kjaju, alkumar, ankita, aniketa, kwankhede, targupta, nova-gpu,
linux-kernel, zhiwang, Zhi Wang, kvm
NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
coordinated with the PF-side nova-core driver.
Add a VFIO PCI variant driver that binds NVIDIA devices only through
driver_override and rejects non-VFs. Delegate instance lifecycle
operations to nova-core, present the firmware-selected device and
subsystem IDs in configuration-space reads, and adjust the reported BAR1
aperture to the assigned profile. Use vfio-pci-core for the remaining
VFIO operations.
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
drivers/vfio/pci/Kconfig | 2 +
drivers/vfio/pci/Makefile | 2 +
drivers/vfio/pci/nvidia-vgpu/Kconfig | 16 ++
drivers/vfio/pci/nvidia-vgpu/Makefile | 2 +
drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
5 files changed, 275 insertions(+)
create mode 100644 drivers/vfio/pci/nvidia-vgpu/Kconfig
create mode 100644 drivers/vfio/pci/nvidia-vgpu/Makefile
create mode 100644 drivers/vfio/pci/nvidia-vgpu/main.c
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index 296bf01e185e..b48d8d1af42a 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -74,4 +74,6 @@ source "drivers/vfio/pci/qat/Kconfig"
source "drivers/vfio/pci/xe/Kconfig"
+source "drivers/vfio/pci/nvidia-vgpu/Kconfig"
+
endmenu
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index 6138f1bf241d..f3498e541555 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -24,3 +24,5 @@ obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu/
obj-$(CONFIG_QAT_VFIO_PCI) += qat/
obj-$(CONFIG_XE_VFIO_PCI) += xe/
+
+obj-$(CONFIG_NVIDIA_VGPU_VFIO_PCI) += nvidia-vgpu/
diff --git a/drivers/vfio/pci/nvidia-vgpu/Kconfig b/drivers/vfio/pci/nvidia-vgpu/Kconfig
new file mode 100644
index 000000000000..098822d32380
--- /dev/null
+++ b/drivers/vfio/pci/nvidia-vgpu/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config NVIDIA_VGPU_VFIO_PCI
+ tristate "VFIO support for the NVIDIA vGPU"
+ depends on NOVA_CORE && PCI_IOV
+ select VFIO_PCI_CORE
+ help
+ This option enables VFIO (Virtual Function I/O) support for
+ NVIDIA virtual GPUs (vGPU). It allows the assignment of a virtual
+ GPU instance to userspace applications via VFIO, typically used
+ with hypervisors such as KVM and device emulators like QEMU.
+
+ The NVIDIA vGPU allows a physical GPU to be partitioned into
+ multiple virtual GPUs, each of which can be passed to a virtual
+ machine as a PCI device using the standard VFIO infrastructure.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/pci/nvidia-vgpu/Makefile b/drivers/vfio/pci/nvidia-vgpu/Makefile
new file mode 100644
index 000000000000..193cc801a081
--- /dev/null
+++ b/drivers/vfio/pci/nvidia-vgpu/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_NVIDIA_VGPU_VFIO_PCI) += nvidia-vgpu-vfio-pci.o
+nvidia-vgpu-vfio-pci-y := main.o
diff --git a/drivers/vfio/pci/nvidia-vgpu/main.c b/drivers/vfio/pci/nvidia-vgpu/main.c
new file mode 100644
index 000000000000..d8626644f952
--- /dev/null
+++ b/drivers/vfio/pci/nvidia-vgpu/main.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/pci.h>
+#include <linux/pid.h>
+#include <linux/vfio_pci_core.h>
+#include <drm/nvidia_vgpu.h>
+
+static int nvidia_vgpu_fb_bar_index(struct pci_dev *pdev)
+{
+ if (pci_resource_flags(pdev, 0) & IORESOURCE_MEM_64)
+ return 2;
+ return 1;
+}
+
+struct nvidia_vgpu_pci_core_device {
+ struct vfio_pci_core_device core_device;
+ struct nvidia_vgpu_type_info type_info;
+ unsigned int gfid;
+};
+
+static inline unsigned int nvidia_vgpu_vf_dbdf(struct pci_dev *vf)
+{
+ return ((u32)pci_domain_nr(vf->bus) << 16) | pci_dev_id(vf);
+}
+
+static int nvidia_vgpu_open_device(struct vfio_device *core_vdev)
+{
+ struct nvidia_vgpu_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvidia_vgpu_pci_core_device, core_device.vdev);
+ struct pci_dev *vf = to_pci_dev(core_vdev->dev);
+ struct nvidia_vgpu_type_info type_info;
+ int ret;
+
+ if (!vf->is_virtfn)
+ return -ENODEV;
+
+ ret = vfio_pci_core_enable(&nvdev->core_device);
+ if (ret)
+ return ret;
+
+ ret = nvidia_vgpu_open(pci_physfn(vf), nvdev->gfid,
+ nvidia_vgpu_vf_dbdf(vf), task_tgid_nr(current),
+ &type_info);
+ if (ret) {
+ vfio_pci_core_disable(&nvdev->core_device);
+ return ret;
+ }
+
+ nvdev->type_info = type_info;
+ pci_dbg(vf, "vgpu open: dev_id=0x%x subsys_id=0x%x bar1_length=0x%llx\n",
+ type_info.pci_dev_id, type_info.pci_subsys_id,
+ type_info.bar1_length);
+ vfio_pci_core_finish_enable(&nvdev->core_device);
+ return 0;
+}
+
+static void nvidia_vgpu_close_device(struct vfio_device *core_vdev)
+{
+ struct nvidia_vgpu_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvidia_vgpu_pci_core_device, core_device.vdev);
+ struct pci_dev *vf = to_pci_dev(core_vdev->dev);
+
+ nvidia_vgpu_close(pci_physfn(vf), nvdev->gfid);
+ vfio_pci_core_close_device(core_vdev);
+}
+
+static ssize_t nvidia_vgpu_pci_read_config(struct vfio_device *core_vdev,
+ char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ struct nvidia_vgpu_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvidia_vgpu_pci_core_device, core_device.vdev);
+ struct nvidia_vgpu_type_info *ti = &nvdev->type_info;
+ loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
+ size_t register_offset;
+ loff_t copy_offset;
+ size_t copy_count;
+ __le16 val16;
+ int ret;
+
+ ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
+ if (ret < 0)
+ return ret;
+
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_DEVICE_ID,
+ sizeof(val16), ©_offset,
+ ©_count, ®ister_offset)) {
+ val16 = cpu_to_le16(ti->pci_dev_id);
+ if (copy_to_user(buf + copy_offset,
+ (void *)&val16 + register_offset, copy_count))
+ return -EFAULT;
+ }
+
+ if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_ID,
+ sizeof(val16), ©_offset,
+ ©_count, ®ister_offset)) {
+ val16 = cpu_to_le16(ti->pci_subsys_id);
+ if (copy_to_user(buf + copy_offset,
+ (void *)&val16 + register_offset, copy_count))
+ return -EFAULT;
+ }
+
+ return count;
+}
+
+static ssize_t nvidia_vgpu_pci_read(struct vfio_device *core_vdev,
+ char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+
+ if (index == VFIO_PCI_CONFIG_REGION_INDEX)
+ return nvidia_vgpu_pci_read_config(core_vdev, buf, count, ppos);
+
+ return vfio_pci_core_read(core_vdev, buf, count, ppos);
+}
+
+static int nvidia_vgpu_bar1_size(struct nvidia_vgpu_pci_core_device *nvdev,
+ u64 *size)
+{
+ if (check_shl_overflow(nvdev->type_info.bar1_length, 20, size))
+ return -EOVERFLOW;
+
+ return 0;
+}
+
+static int nvidia_vgpu_get_region_info(struct vfio_device *core_vdev,
+ struct vfio_region_info *info,
+ struct vfio_info_cap *caps)
+{
+ int ret;
+
+ ret = vfio_pci_ioctl_get_region_info(core_vdev, info, caps);
+ if (ret)
+ return ret;
+
+ if (info->index == nvidia_vgpu_fb_bar_index(
+ to_pci_dev(core_vdev->dev)) && info->size) {
+ struct nvidia_vgpu_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvidia_vgpu_pci_core_device,
+ core_device.vdev);
+ u64 vgpu_bar1;
+
+ ret = nvidia_vgpu_bar1_size(nvdev, &vgpu_bar1);
+ if (ret)
+ return ret;
+
+ if (vgpu_bar1 && vgpu_bar1 < info->size)
+ info->size = vgpu_bar1;
+ }
+
+ return 0;
+}
+
+static long nvidia_vgpu_pci_ioctl(struct vfio_device *core_vdev,
+ unsigned int cmd, unsigned long arg)
+{
+ if (cmd == VFIO_DEVICE_RESET) {
+ struct nvidia_vgpu_pci_core_device *nvdev = container_of(
+ core_vdev, struct nvidia_vgpu_pci_core_device,
+ core_device.vdev);
+ struct pci_dev *vf = to_pci_dev(core_vdev->dev);
+ int ret;
+
+ ret = nvidia_vgpu_reset(pci_physfn(vf), nvdev->gfid);
+ if (ret)
+ return ret;
+ }
+
+ return vfio_pci_core_ioctl(core_vdev, cmd, arg);
+}
+
+static const struct vfio_device_ops nvidia_vgpu_pci_ops = {
+ .name = "nvidia-vgpu-vfio-pci",
+ .init = vfio_pci_core_init_dev,
+ .release = vfio_pci_core_release_dev,
+ .open_device = nvidia_vgpu_open_device,
+ .close_device = nvidia_vgpu_close_device,
+ .ioctl = nvidia_vgpu_pci_ioctl,
+ .get_region_info_caps = nvidia_vgpu_get_region_info,
+ .device_feature = vfio_pci_core_ioctl_feature,
+ .read = nvidia_vgpu_pci_read,
+ .write = vfio_pci_core_write,
+ .mmap = vfio_pci_core_mmap,
+ .request = vfio_pci_core_request,
+ .match = vfio_pci_core_match,
+ .match_token_uuid = vfio_pci_core_match_token_uuid,
+ .bind_iommufd = vfio_iommufd_physical_bind,
+ .unbind_iommufd = vfio_iommufd_physical_unbind,
+ .attach_ioas = vfio_iommufd_physical_attach_ioas,
+ .detach_ioas = vfio_iommufd_physical_detach_ioas,
+};
+
+static int nvidia_vgpu_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *id)
+{
+ struct nvidia_vgpu_pci_core_device *nvdev;
+ int vf_id;
+ int ret;
+
+ if (!pdev->is_virtfn)
+ return -ENODEV;
+
+ vf_id = pci_iov_vf_id(pdev);
+ if (vf_id < 0)
+ return vf_id;
+
+ nvdev = vfio_alloc_device(nvidia_vgpu_pci_core_device, core_device.vdev,
+ &pdev->dev, &nvidia_vgpu_pci_ops);
+ if (IS_ERR(nvdev))
+ return PTR_ERR(nvdev);
+
+ nvdev->gfid = vf_id + 1;
+ dev_set_drvdata(&pdev->dev, &nvdev->core_device);
+ ret = vfio_pci_core_register_device(&nvdev->core_device);
+ if (ret)
+ goto out_put_vdev;
+
+ return 0;
+
+out_put_vdev:
+ vfio_put_device(&nvdev->core_device.vdev);
+ return ret;
+}
+
+static void nvidia_vgpu_pci_remove(struct pci_dev *pdev)
+{
+ struct vfio_pci_core_device *core_device = dev_get_drvdata(&pdev->dev);
+
+ vfio_pci_core_unregister_device(core_device);
+ vfio_put_device(&core_device->vdev);
+}
+
+static const struct pci_device_id nvidia_vgpu_pci_table[] = {
+ /* Placeholder: match all NVIDIA VFs (vendor 0x10de) */
+ { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) },
+ {}
+};
+MODULE_DEVICE_TABLE(pci, nvidia_vgpu_pci_table);
+
+static struct pci_driver nvidia_vgpu_pci_driver = {
+ .name = "nvidia-vgpu-vfio-pci",
+ .id_table = nvidia_vgpu_pci_table,
+ .probe = nvidia_vgpu_pci_probe,
+ .remove = nvidia_vgpu_pci_remove,
+ .driver_managed_dma = true,
+};
+module_pci_driver(nvidia_vgpu_pci_driver);
+
+MODULE_DESCRIPTION("NVIDIA vGPU vfio-pci driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("NOVA_CORE_VGPU");
--
2.53.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-05 8:11 ` [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU " Zhi Wang
@ 2026-09-09 3:00 ` Alex Williamson
2026-09-11 20:39 ` Danilo Krummrich
1 sibling, 0 replies; 24+ messages in thread
From: Alex Williamson @ 2026-09-09 3:00 UTC (permalink / raw)
To: Zhi Wang
Cc: dakr, acourbot, jgg, yishaih, skolothumtho, kevin.tian, airlied,
simona, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin,
a.hindborg, aliceryhl, tmgross, jhubbard, ecourtney, cjia, smitra,
kjaju, alkumar, ankita, aniketa, kwankhede, targupta, nova-gpu,
linux-kernel, zhiwang, kvm, alex
On Sat, 5 Sep 2026 11:11:15 +0300
Zhi Wang <zhiw@nvidia.com> wrote:
> NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
> coordinated with the PF-side nova-core driver.
>
> Add a VFIO PCI variant driver that binds NVIDIA devices only through
> driver_override and rejects non-VFs. Delegate instance lifecycle
> operations to nova-core, present the firmware-selected device and
> subsystem IDs in configuration-space reads, and adjust the reported BAR1
> aperture to the assigned profile. Use vfio-pci-core for the remaining
> VFIO operations.
>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
> drivers/vfio/pci/Kconfig | 2 +
> drivers/vfio/pci/Makefile | 2 +
> drivers/vfio/pci/nvidia-vgpu/Kconfig | 16 ++
> drivers/vfio/pci/nvidia-vgpu/Makefile | 2 +
> drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
> 5 files changed, 275 insertions(+)
> create mode 100644 drivers/vfio/pci/nvidia-vgpu/Kconfig
> create mode 100644 drivers/vfio/pci/nvidia-vgpu/Makefile
> create mode 100644 drivers/vfio/pci/nvidia-vgpu/main.c
>
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 296bf01e185e..b48d8d1af42a 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -74,4 +74,6 @@ source "drivers/vfio/pci/qat/Kconfig"
>
> source "drivers/vfio/pci/xe/Kconfig"
>
> +source "drivers/vfio/pci/nvidia-vgpu/Kconfig"
> +
> endmenu
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index 6138f1bf241d..f3498e541555 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -24,3 +24,5 @@ obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu/
> obj-$(CONFIG_QAT_VFIO_PCI) += qat/
>
> obj-$(CONFIG_XE_VFIO_PCI) += xe/
> +
> +obj-$(CONFIG_NVIDIA_VGPU_VFIO_PCI) += nvidia-vgpu/
> diff --git a/drivers/vfio/pci/nvidia-vgpu/Kconfig b/drivers/vfio/pci/nvidia-vgpu/Kconfig
> new file mode 100644
> index 000000000000..098822d32380
> --- /dev/null
> +++ b/drivers/vfio/pci/nvidia-vgpu/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config NVIDIA_VGPU_VFIO_PCI
> + tristate "VFIO support for the NVIDIA vGPU"
> + depends on NOVA_CORE && PCI_IOV
> + select VFIO_PCI_CORE
> + help
> + This option enables VFIO (Virtual Function I/O) support for
> + NVIDIA virtual GPUs (vGPU). It allows the assignment of a virtual
> + GPU instance to userspace applications via VFIO, typically used
> + with hypervisors such as KVM and device emulators like QEMU.
> +
> + The NVIDIA vGPU allows a physical GPU to be partitioned into
> + multiple virtual GPUs, each of which can be passed to a virtual
> + machine as a PCI device using the standard VFIO infrastructure.
> +
> + If you don't know what to do here, say N.
> diff --git a/drivers/vfio/pci/nvidia-vgpu/Makefile b/drivers/vfio/pci/nvidia-vgpu/Makefile
> new file mode 100644
> index 000000000000..193cc801a081
> --- /dev/null
> +++ b/drivers/vfio/pci/nvidia-vgpu/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_NVIDIA_VGPU_VFIO_PCI) += nvidia-vgpu-vfio-pci.o
> +nvidia-vgpu-vfio-pci-y := main.o
> diff --git a/drivers/vfio/pci/nvidia-vgpu/main.c b/drivers/vfio/pci/nvidia-vgpu/main.c
> new file mode 100644
> index 000000000000..d8626644f952
> --- /dev/null
> +++ b/drivers/vfio/pci/nvidia-vgpu/main.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/module.h>
> +#include <linux/overflow.h>
> +#include <linux/pci.h>
> +#include <linux/pid.h>
> +#include <linux/vfio_pci_core.h>
> +#include <drm/nvidia_vgpu.h>
> +
> +static int nvidia_vgpu_fb_bar_index(struct pci_dev *pdev)
> +{
> + if (pci_resource_flags(pdev, 0) & IORESOURCE_MEM_64)
> + return 2;
> + return 1;
> +}
> +
> +struct nvidia_vgpu_pci_core_device {
> + struct vfio_pci_core_device core_device;
> + struct nvidia_vgpu_type_info type_info;
> + unsigned int gfid;
> +};
> +
This is more commonly called an "sbdf". Also, consider some comments.
> +static inline unsigned int nvidia_vgpu_vf_dbdf(struct pci_dev *vf)
> +{
> + return ((u32)pci_domain_nr(vf->bus) << 16) | pci_dev_id(vf);
> +}
> +
> +static int nvidia_vgpu_open_device(struct vfio_device *core_vdev)
> +{
> + struct nvidia_vgpu_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvidia_vgpu_pci_core_device, core_device.vdev);
> + struct pci_dev *vf = to_pci_dev(core_vdev->dev);
> + struct nvidia_vgpu_type_info type_info;
> + int ret;
> +
> + if (!vf->is_virtfn)
> + return -ENODEV;
This is redundant to the probe check.
> +
> + ret = vfio_pci_core_enable(&nvdev->core_device);
> + if (ret)
> + return ret;
> +
> + ret = nvidia_vgpu_open(pci_physfn(vf), nvdev->gfid,
> + nvidia_vgpu_vf_dbdf(vf), task_tgid_nr(current),
> + &type_info);
> + if (ret) {
> + vfio_pci_core_disable(&nvdev->core_device);
> + return ret;
> + }
> +
> + nvdev->type_info = type_info;
> + pci_dbg(vf, "vgpu open: dev_id=0x%x subsys_id=0x%x bar1_length=0x%llx\n",
> + type_info.pci_dev_id, type_info.pci_subsys_id,
> + type_info.bar1_length);
> + vfio_pci_core_finish_enable(&nvdev->core_device);
> + return 0;
> +}
> +
> +static void nvidia_vgpu_close_device(struct vfio_device *core_vdev)
> +{
> + struct nvidia_vgpu_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvidia_vgpu_pci_core_device, core_device.vdev);
> + struct pci_dev *vf = to_pci_dev(core_vdev->dev);
> +
> + nvidia_vgpu_close(pci_physfn(vf), nvdev->gfid);
> + vfio_pci_core_close_device(core_vdev);
> +}
> +
> +static ssize_t nvidia_vgpu_pci_read_config(struct vfio_device *core_vdev,
> + char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + struct nvidia_vgpu_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvidia_vgpu_pci_core_device, core_device.vdev);
> + struct nvidia_vgpu_type_info *ti = &nvdev->type_info;
> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> + size_t register_offset;
> + loff_t copy_offset;
> + size_t copy_count;
> + __le16 val16;
> + int ret;
> +
> + ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
> + if (ret < 0)
> + return ret;
> +
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_DEVICE_ID,
> + sizeof(val16), ©_offset,
> + ©_count, ®ister_offset)) {
> + val16 = cpu_to_le16(ti->pci_dev_id);
> + if (copy_to_user(buf + copy_offset,
> + (void *)&val16 + register_offset, copy_count))
> + return -EFAULT;
> + }
Just stuff the device ID into vconfig, it's already read from there.
> +
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_ID,
> + sizeof(val16), ©_offset,
> + ©_count, ®ister_offset)) {
> + val16 = cpu_to_le16(ti->pci_subsys_id);
> + if (copy_to_user(buf + copy_offset,
> + (void *)&val16 + register_offset, copy_count))
> + return -EFAULT;
> + }
There's possibly an argument to be made that subsystem ID could be read
from vconfig by default too so it could be pre-filled after
vfio_config_init(), ie. after vfio_pci_core_enable(). The only reason
it might change would be if firmware was updated, but a firmware update
through vfio that changes the subsystem ID would be pretty sketchy
already.
> +
> + return count;
> +}
> +
> +static ssize_t nvidia_vgpu_pci_read(struct vfio_device *core_vdev,
> + char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> +
> + if (index == VFIO_PCI_CONFIG_REGION_INDEX)
> + return nvidia_vgpu_pci_read_config(core_vdev, buf, count, ppos);
> +
> + return vfio_pci_core_read(core_vdev, buf, count, ppos);
> +}
> +
> +static int nvidia_vgpu_bar1_size(struct nvidia_vgpu_pci_core_device *nvdev,
> + u64 *size)
> +{
> + if (check_shl_overflow(nvdev->type_info.bar1_length, 20, size))
> + return -EOVERFLOW;
> +
> + return 0;
> +}
> +
> +static int nvidia_vgpu_get_region_info(struct vfio_device *core_vdev,
> + struct vfio_region_info *info,
> + struct vfio_info_cap *caps)
> +{
> + int ret;
> +
> + ret = vfio_pci_ioctl_get_region_info(core_vdev, info, caps);
> + if (ret)
> + return ret;
> +
> + if (info->index == nvidia_vgpu_fb_bar_index(
> + to_pci_dev(core_vdev->dev)) && info->size) {
> + struct nvidia_vgpu_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvidia_vgpu_pci_core_device,
> + core_device.vdev);
> + u64 vgpu_bar1;
> +
> + ret = nvidia_vgpu_bar1_size(nvdev, &vgpu_bar1);
> + if (ret)
> + return ret;
> +
> + if (vgpu_bar1 && vgpu_bar1 < info->size)
> + info->size = vgpu_bar1;
> + }
So we're changing the reported BAR1 size, but just trusting that
userspace honors that size for read/write/mmap? Again, consider some
comments.
> +
> + return 0;
> +}
> +
> +static long nvidia_vgpu_pci_ioctl(struct vfio_device *core_vdev,
> + unsigned int cmd, unsigned long arg)
> +{
> + if (cmd == VFIO_DEVICE_RESET) {
> + struct nvidia_vgpu_pci_core_device *nvdev = container_of(
> + core_vdev, struct nvidia_vgpu_pci_core_device,
> + core_device.vdev);
> + struct pci_dev *vf = to_pci_dev(core_vdev->dev);
> + int ret;
> +
> + ret = nvidia_vgpu_reset(pci_physfn(vf), nvdev->gfid);
> + if (ret)
> + return ret;
> + }
> +
> + return vfio_pci_core_ioctl(core_vdev, cmd, arg);
What about reset invoked through FLR? Would this be better served
through .reset_prepare and .reset_done?
> +}
> +
> +static const struct vfio_device_ops nvidia_vgpu_pci_ops = {
> + .name = "nvidia-vgpu-vfio-pci",
> + .init = vfio_pci_core_init_dev,
> + .release = vfio_pci_core_release_dev,
> + .open_device = nvidia_vgpu_open_device,
> + .close_device = nvidia_vgpu_close_device,
> + .ioctl = nvidia_vgpu_pci_ioctl,
> + .get_region_info_caps = nvidia_vgpu_get_region_info,
> + .device_feature = vfio_pci_core_ioctl_feature,
> + .read = nvidia_vgpu_pci_read,
> + .write = vfio_pci_core_write,
> + .mmap = vfio_pci_core_mmap,
> + .request = vfio_pci_core_request,
> + .match = vfio_pci_core_match,
> + .match_token_uuid = vfio_pci_core_match_token_uuid,
> + .bind_iommufd = vfio_iommufd_physical_bind,
> + .unbind_iommufd = vfio_iommufd_physical_unbind,
> + .attach_ioas = vfio_iommufd_physical_attach_ioas,
> + .detach_ioas = vfio_iommufd_physical_detach_ioas,
> +};
> +
> +static int nvidia_vgpu_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + struct nvidia_vgpu_pci_core_device *nvdev;
> + int vf_id;
> + int ret;
> +
> + if (!pdev->is_virtfn)
> + return -ENODEV;
This driver needs to bind to what it matches in the id table, we don't
have a policy for userspace to pick a 2nd best variant driver.
hisi_acc handles a similar situation where only the VFs are supported
for the migration feature of the variant driver. The PF needs to be
supported here and bind to a vfio-pci-core passthrough ops structure.
We should probably define a PCI_DRIVER_OVERRIDE_DEVICE_VFIO variant
that allows a class code to be specified so we aren't using this for
all 10de: devices. I'm hoping that class code is for a 3D accelerator
or the like rather than VGA class (a VF can't technically support a
legacy endpoint anyway), but we need to consider what existing devices
that currently use vfio-pci would now be bound to this driver and what
module option features they might use. Thanks,
Alex
> +
> + vf_id = pci_iov_vf_id(pdev);
> + if (vf_id < 0)
> + return vf_id;
> +
> + nvdev = vfio_alloc_device(nvidia_vgpu_pci_core_device, core_device.vdev,
> + &pdev->dev, &nvidia_vgpu_pci_ops);
> + if (IS_ERR(nvdev))
> + return PTR_ERR(nvdev);
> +
> + nvdev->gfid = vf_id + 1;
> + dev_set_drvdata(&pdev->dev, &nvdev->core_device);
> + ret = vfio_pci_core_register_device(&nvdev->core_device);
> + if (ret)
> + goto out_put_vdev;
> +
> + return 0;
> +
> +out_put_vdev:
> + vfio_put_device(&nvdev->core_device.vdev);
> + return ret;
> +}
> +
> +static void nvidia_vgpu_pci_remove(struct pci_dev *pdev)
> +{
> + struct vfio_pci_core_device *core_device = dev_get_drvdata(&pdev->dev);
> +
> + vfio_pci_core_unregister_device(core_device);
> + vfio_put_device(&core_device->vdev);
> +}
> +
> +static const struct pci_device_id nvidia_vgpu_pci_table[] = {
> + /* Placeholder: match all NVIDIA VFs (vendor 0x10de) */
> + { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) },
> + {}
> +};
> +MODULE_DEVICE_TABLE(pci, nvidia_vgpu_pci_table);
> +
> +static struct pci_driver nvidia_vgpu_pci_driver = {
> + .name = "nvidia-vgpu-vfio-pci",
> + .id_table = nvidia_vgpu_pci_table,
> + .probe = nvidia_vgpu_pci_probe,
> + .remove = nvidia_vgpu_pci_remove,
> + .driver_managed_dma = true,
> +};
> +module_pci_driver(nvidia_vgpu_pci_driver);
> +
> +MODULE_DESCRIPTION("NVIDIA vGPU vfio-pci driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("NOVA_CORE_VGPU");
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-05 8:11 ` [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU " Zhi Wang
2026-09-09 3:00 ` Alex Williamson
@ 2026-09-11 20:39 ` Danilo Krummrich
2026-09-14 18:12 ` Alex Williamson
1 sibling, 1 reply; 24+ messages in thread
From: Danilo Krummrich @ 2026-09-11 20:39 UTC (permalink / raw)
To: Alex Williamson, Jason Gunthorpe, Zhi Wang
Cc: acourbot, yishaih, skolothumtho, kevin.tian, airlied, simona,
ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin,
a.hindborg, aliceryhl, tmgross, jhubbard, ecourtney, cjia, smitra,
kjaju, alkumar, ankita, aniketa, kwankhede, targupta, nova-gpu,
linux-kernel, zhiwang, kvm
Hi Alex, Jason, Zhi,
On Sat Sep 5, 2026 at 10:11 AM CEST, Zhi Wang wrote:
> NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
> coordinated with the PF-side nova-core driver.
[...]
> drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
This is going to be a longer response; sorry about this in advance.
Looking at the FFI boundary introduced in the previous patch, I'm concerned that
it translates the driver model relationships we've expressed through Rust's
ownership and lifetime model back into raw pointers and lifetime assumptions
that callers must uphold. It also introduces manual lifecycle management across
the boundary, rather than preserving nova-core's RAII-based ownership model.
I think implementing the NVIDIA vGPU driver in Rust would let us preserve those
relationships across the interface, make lifecycle management less error-prone,
and fit naturally alongside nova-core and nova-drm.
I did sketch up the necessary code for this in [1], which contains the VFIO/PCI
Rust infrastructure [2], a Rust implementation of the NVIDIA vGPU driver [3] and
the required PCI SR-IOV infrastructure [4].
This is PoC code; I focused on the general design, and I've tested the VFIO bits
to a point that I can poke the character device from userspace. But it still
needs a bit of cleanup and probably a few additional new type abstractions over
primitive types, etc. As conferences are approaching and I have a bunch of stuff
to prepare I could only finish it up after LPC, but I hope that Zhi is also
interested in picking it up. :)
I'm aware that you may have some concerns about Rust code in VFIO and one of
them might be maintainance. I already have a lot on my plate, but I'm happy to
offer to take responsibility. It would also be great if Zhi were interested in
helping maintain them.
In this context, I'd like to walk through the design and a few code examples
below. We can also follow up at LPC, perhaps as part of the Nova workshop. (Zhi,
would you be interested in preparing a brief session on this with me?)
In general, the VFIO/PCI Rust code is not much different from the FWCTL and DRM
code that we have upstream already. In the end they are all the same design in
terms of the Rust class device lifetime model.
Below is a PCI driver skeleton that implements a VFIO/PCI class device (stripped
down version of the NVIDIA vGPU driver), with a bunch of comments.
#[pin_data]
struct NvidiaVgpuData<'bound> {
_reg: vfio::pci::Registration<'bound, NvidiaVgpuOps>,
}
This is the driver's bus device private data. The driver core uses a RAII
approach for this, it creates the driver's bus device private data from the
initializer returned from probe() and calls the destructor of the driver's bus
device private data on remove(), which is the equivalent of a remove() callback
in C.
In this case it just contains the vfio::pci::Registration, which essentially is
a RAII type for vfio_pci_core_register_device() and
vfio_pci_core_unregister_device().
(I'm aware that that vfio_pci_core_register_device() currently requires the
driver's bus device private data to be set to a struct vfio_pci_core_device
pointer as a trick to support PCI bus callbacks, such as with
vfio_pci_core_aer_err_detected(). I fixed this up in [5].)
#[pin_data]
pub struct NvidiaVgpuRegData<'a> {
pdev: &'a pci::Device<Bound>,
api: NovaCoreVfApiHandle<'a>,
}
This is the private data attached to the vfio::pci::Registration, which is
accessible from the callbacks in struct vfio_device_ops.
You may wonder why the private data is on the vfio::pci::Registration rather
than the vfio::pci::Device.
The reason is that the callbacks in struct vfio_device_ops are lifetime wise
associated with the vfio::pci::Registration and not the vfio::pci::Device, so
the destructor of this data should be called when the destructor of
vfio::pci::Registration runs, i.e. directly after
vfio_pci_core_unregister_device().
Furthermore, it allows us to store device resources, such as a DMA coherent
allocation, within this data in the first place, as the Rust compiler will
ensure that a vfio::pci::Registration can't outlive driver unbind, since its
lifetime is bound to the bus driver's bus device private data.
(This is also why NvidiaVgpuRegData<'a> can store e.g. &'a pci::Device<Bound>;
the lifetime 'a is guaranteed to be shorter lived than the 'bound lifetime that
describes the lifetime of the driver being bound to a device.)
The vfio::pci::Device on the other hand is reference counted and has an
unbounded lifetime.
Note that in the end this is only a logical distinction for when the destructor
is called. The actual memory this data goes into does not matter too much; it
can be a new allocation, it can be the allocation of the struct
vfio_pci_core_device (as it is in C), since the struct vfio_pci_core_device
strictly outlives the vfio::pci::Registration, or it can also be the driver's
bus device private data allocation itself.
kernel::pci_device_table!(
PCI_TABLE,
<NvidiaVgpuDriver as pci::Driver>::IdInfo,
[
(
pci::DeviceId::from_class_and_vendor_vfio_override(
Class::DISPLAY_VGA,
ClassMask::ClassSubclass,
Vendor::NVIDIA
),
()
),
(
pci::DeviceId::from_class_and_vendor_vfio_override(
Class::DISPLAY_3D,
ClassMask::ClassSubclass,
Vendor::NVIDIA
),
()
),
]
);
The device ID table. It has some nice compile time guarantees as well, but it is
otherwise not very interesting in this context.
Let's look at the pci::Driver trait, which provides the callbacks (such as
probe()) and associated constants and types instead.
impl pci::Driver for NvidiaVgpuDriver {
type IdInfo = ();
type Data<'bound> = NvidiaVgpuData<'bound>;
const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
const DRIVER_MANAGED_DMA: bool = true;
fn probe<'bound>(
pdev: &'bound pci::Device<Core<'_>>,
_info: Option<&'bound Self::IdInfo>,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
Note the 'bound lifetime in the signature of probe(); it represents the lifetime
of the driver's bus device private data and hence the lifetime of the driver
being bound to a device.
let vdev = vfio::pci::Device::<NvidiaVgpuOps>::new(pdev)?;
This represents a struct vfio_pci_core_device and is typed over an
implementation of vfio::pci::Operations (i.e. a struct vfio_device_ops), but is
otherwise not very interesting.
try_pin_init!(Self::Data {
// SAFETY: The registration is dropped when the PCI driver is unbound.
_reg <- unsafe { vfio::pci::Registration::new(
pdev,
&vdev,
try_pin_init!(NvidiaVgpuRegData {
pdev,
api: NovaCoreVfApi::handle(pdev)?,
}),
)},
Here we create the vfio::pci::Registration (i.e. call
vfio_pci_core_register_device()).
It takes three arguments, a &vfio::pci::Device, the private data and a
&'bound pci::Device<Bound>.
The vfio::pci::Registration captures the lifetime ('bound) of the &'bound
pci::Device<Bound>, such that it can't outlive driver unbind. In practice this
is ensured as the compiler won't allow the vfio::pci::Registration to be stored
anywhere else as in a place that is either the bus device private data itself or
something else that is strictly shorter lived.
The call to
NovaCoreVfApi::handle(pdev)?
calls into nova-core, which will provide a handle to the nova-core API
representation. This handle ties back to nova-core private data that by itself
is shorter lived than nova-core's 'bound lifetime, but longer lived than vGPU's
'bound lifetime. IOW, the data is guaranteed to be valid for the full lifecycle
of vfio::pci::Registration and hence can be stored within its private data.
I will come back to how this guarantee is upheld below. For now, let's have a
look at how vfio::pci::Operations (i.e. struct vfio_device_ops) is represented.
})
}
}
#[pin_data]
struct NvidiaVgpuOpenData<'a> {
instance: VgpuInstance<'a>,
}
This type (yes, Rust loves new types :) represents data that lives from
open_device() until close_device().
Note that the implementation below does not have close_device() at all, as the
destructor of the OpenData already represents close_device() as a RAII type.
This is also what makes the API with nova-core much better, since...
impl vfio::pci::Operations for NvidiaVgpuOps {
const NAME: &'static CStr = c"nvidia-vgpu-vfio-pci";
type RegistrationData<'a> = NvidiaVgpuRegData<'a>;
type OpenData<'a> = NvidiaVgpuOpenData<'a>;
fn open_device<'a>(
_dev: &'a vfio::pci::Device<Self>,
rd: &'a Self::RegistrationData<'a>,
) -> impl PinInit<Self::OpenData<'a>, Error> + 'a {
try_pin_init!(NvidiaVgpuOpenData {
instance: rd.api.open(),
})
...here we can just call into nova-core via the API handle and obtain a
VgpuInstance<'a> struct from nova-core. Where nova-core can just store all the
objects that should be destructed on close_device() in the VgpuInstance struct.
This way we avoid an API contract where we have to translate a RAII based design
into procedural cleanup and vice versa.
Also note how we can represent that OpenData is strictly shorter lived as
RegistrationData and the driver's bus device private data, in a way that the
Rust compiler can ensure this.
}
fn ioctl<'a>(
dev: &vfio::pci::Device<Self, Ioctl>,
_rd: &Self::RegistrationData<'a>,
open_data: Pin<&Self::OpenData<'a>>,
cmd: u32,
arg: usize,
) -> Result<isize> {
// Handle driver ioctl.
dev.core_ioctl(cmd, arg)
}
fn read<'a>(
dev: &vfio::pci::Device<Self, Read>,
_rd: &Self::RegistrationData<'a>,
open_data: Pin<&Self::OpenData<'a>>,
buf: &mut vfio::UserBuf,
ppos: &mut vfio::pci::Position<'_>,
) -> Result<isize> {
Ok(0)
}
fn get_region_info<'a>(
dev: &vfio::pci::Device<Self, GetRegionInfo>,
rd: &Self::RegistrationData<'a>,
open_data: Pin<&Self::OpenData<'a>>,
info: &mut bindings::vfio_region_info,
caps: &mut vfio::InfoCap<'_>,
) -> Result {
Ok(())
}
}
The rest of the callbacks is not too interesting. The main thing to note is that
the type state on the &vfio::pci::Device in e.g. ioctl() allows us to ensure
that dev.core_ioctl() can only be called in ioctl() as it is only implemented
for &vfio::pci::Device<_, Ioctl> and we only ever give out a
&vfio::pci::Device<_, Ioctl> in ioctl().
In the VFIO/PCI code I only implemented the callbacks the NVIDIA vGPU driver
needs; all other callbacks can just remain the default trampolines for now.
Now, I promised to come back to how the following call works.
NovaCoreVfApi::handle(vf_pdev)?
As mentioned it provides a handle to the nova-core API representation, which is
shorter lived than nova-core's 'bound lifetime, but longer lived than vGPU's
'bound lifetime (and therefore always valid).
This is ensured by how I think we should implement the handling of the PF and VF
relationship on the PCI bus. Let's have a look at nova-core's probe() for this:
impl pci::Driver for NovaCoreDriver {
type IdInfo = ();
type Data<'bound> = NovaCore<'bound>;
const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
fn probe<'bound>(
pdev: &'bound pci::Device<Core<'_>>,
_info: Option<&'bound Self::IdInfo>,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
try_pin_init!(NovaCore {
_enable: {
let enable = pdev.enable_device()?;
pdev.set_master();
enable
},
gpu <- Gpu::new(pdev, pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?),
// SAFETY: `NovaCore` is dropped when the device is unbound.
_reg: unsafe {
auxiliary::Registration::new_with_lt(
pdev.as_ref(),
c"nova-drm",
AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
crate::MODULE_NAME,
NovaCoreApi { gpu: gpu.get_ref(), pdev },
)?
},
// SAFETY: `NovaCore` is dropped when the device is unbound.
_vf_reg <- unsafe {
let total_vfs = pdev.sriov_get_totalvfs().map_or(0, |v| v.get());
pci::VfRegistration::new(
pdev,
total_vfs,
total_vfs > 0,
NovaCoreVfApi { _gpu: gpu.get_ref(), pdev },
)
},
})
}
}
Similar to vfio::pci::Registration and auxiliary::Registration, we have a
pci::VfRegistration, which can only be constructed once by a PF; for VFs it
fails to construct.
This pci::VfRegistration returns an initializer and lives within the driver's
bus device private data allocation. The constructor of pci::VfRegistration takes
the private data type that should be shared with VFs.
IOW, we do not share the whole driver's bus device private data with the VFs,
but just a defined container within the driver's bus device private data.
This is also what we do for all other kinds of registrations, such as
irq::Registration, which has the advantage that it fundamentally prevents
ordering issues. For instance, when constructing an irq::Registration the IRQ
private data container is guaranteed to be fully initialized before the first
IRQ is received, whereas the rest of the driver's bus device private data may
not be initialized yet.
For the pci::VfRegistration this isn't a concern, as it would be valid to expose
the entire driver's bus device private data, but it is still cleaner if a PF
does not need to expose its whole bus device private data to the VFs, but just
the intended API type.
The required lifetime guarantee comes from the fact the pci::VfRegistration
lives in the driver's bus device private data, and serves as a guard that calls
pci_disable_sriov() in its destructor.
This way we also do not need the patch in [6]. I still think it would be
reasonable to have this, but the pci::VfRegistration approach is cleaner. In any
case, it's not an either-or, we can have both.
Coming back to nova-core's probe() above, we can see how this perfectly aligns
with how the API between nova-core and nova-drm works via the auxiliary bus.
Implementation wise the API on the nova-core side looks like this:
pub struct NovaCoreVfApi<'a> {
pub(crate) pdev: &'a pci::Device<device::Bound>,
pub(crate) _gpu: &'a Gpu<'a>,
}
/// Closure-based handle to the nova-core VF API.
pub struct NovaCoreVfApiHandle<'a> {
vf: &'a pci::Device<device::Bound>,
}
/// An active vGPU instance, closed on drop while the VF binding is still valid.
pub struct VgpuInstance<'a> {
api: &'a NovaCoreVfApiHandle<'a>,
}
impl NovaCoreVfApi<'_> {
/// Obtain a [`NovaCoreVfApiHandle`] from a VF registered by nova-core.
pub fn handle(vf: &pci::Device<device::Bound>) -> Result<NovaCoreVfApiHandle<'_>> {
NovaCoreVfApiHandle::of(vf)
}
}
impl<'a> NovaCoreVfApiHandle<'a> {
fn of(vf: &'a pci::Device<device::Bound>) -> Result<Self> {
vf.vf_registration_data_with::<ForLt!(NovaCoreVfApi<'_>), ()>(|_| ())?;
Ok(Self { vf })
}
/// Activate a vGPU instance, which is closed on drop.
pub fn open(&self) -> Result<VgpuInstance<'a>> {
VgpuInstance::new(self)
}
/// Access the [`NovaCoreVfApi`] through a closure.
pub fn with<R>(&self, f: impl for<'b> FnOnce(Pin<&NovaCoreVfApi<'b>>) -> R) -> R {
self.vf
.vf_registration_data_with::<ForLt!(NovaCoreVfApi<'_>), R>(f)
.expect("TypeId was validated in NovaCoreVfApiHandle::of()")
}
}
impl<'a> VgpuInstance<'a> {
fn new(api: NovaCoreVfApiHandle<'a>) -> Result<Self> {
// TODO: Create the vGPU instance object via `api.gpu`.
Ok(Self { api })
}
/// Reset this vGPU instance.
pub fn reset(&self) -> Result {
Ok(())
}
}
(Don't worry too much about the NovaCoreVfApiHandle::of() and
NovaCoreVfApiHandle::with() stuff. Those are helpers we also have in the
nova-drm API to deal with invariant or non-covariant types respectively.)
If you've made it this far, thanks for reading through this long write-up. I
hope you find it useful. Please let me know if you have any questions or
thoughts.
Thanks,
Danilo
[1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=poc/vgpu
[2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=484316d855e3d61873122c295feb9a7457eeca21
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=6292c1de7f0758dd31496a454b4034507f38bd40
[4] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=efac2cec97eab36edd01a2fe79aea67a04bd8842
[5] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=76b3bfd6386a01f338780e1a37b5bad3f5a48d31
[6] https://lore.kernel.org/lkml/20260303-rust-pci-sriov-v3-1-4443c35f0c88@redhat.com/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-11 20:39 ` Danilo Krummrich
@ 2026-09-14 18:12 ` Alex Williamson
2026-09-14 21:36 ` Danilo Krummrich
0 siblings, 1 reply; 24+ messages in thread
From: Alex Williamson @ 2026-09-14 18:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Jason Gunthorpe, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm, alex
Hi Danilo,
On Fri, 11 Sep 2026 22:39:16 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
> Hi Alex, Jason, Zhi,
>
> On Sat Sep 5, 2026 at 10:11 AM CEST, Zhi Wang wrote:
> > NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
> > coordinated with the PF-side nova-core driver.
>
> [...]
>
> > drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
>
> This is going to be a longer response; sorry about this in advance.
>
> Looking at the FFI boundary introduced in the previous patch, I'm concerned that
> it translates the driver model relationships we've expressed through Rust's
> ownership and lifetime model back into raw pointers and lifetime assumptions
> that callers must uphold. It also introduces manual lifecycle management across
> the boundary, rather than preserving nova-core's RAII-based ownership model.
>
> I think implementing the NVIDIA vGPU driver in Rust would let us preserve those
> relationships across the interface, make lifecycle management less error-prone,
> and fit naturally alongside nova-core and nova-drm.
[snip]
>
> If you've made it this far, thanks for reading through this long write-up. I
> hope you find it useful. Please let me know if you have any questions or
> thoughts.
I can't really say I made it this far with comprehension, but thanks
for the effort ;)
The one piece here that I can actually review is [5], where
dev_get_drvdata() is replaced with a vfio-pci-core struct pointer
embedded in the struct pci_dev, which is a non-starter as far as having
a common PCI-core shared by various drivers.
Maybe Dave Airlie can share some experience here with a Rust driver
growing up within a subsystem for a non-Rust-literate maintainer.
My concerns are of course who is going to review the Rust vfio-pci
variant drivers from a vfio perspective, not just a drm driver
viewpoint. Who is going to be responsive when the interfaces break and
monitor vfio proactively to prevent such breakages, and how do we avoid
derailing feature development in the core code base. Can a Rust
vfio-pci variant driver be self-contained, or to what extent does it
impose on the framework, such as the drvdata idiom.
FWIW, AI can only go so far to support reviews. Having the code
insight to ask the right questions is essential. A human in the loop
is a requirement.
Additionally, if we can't narrow the device matching to only the
Nova-core supported VFs, then this variant driver must immediately
provide feature parity of vfio-pci with pass-through to vfio-pci-core
for matched devices. Libvirt selects a best matching variant driver by
modalias with our override scheme.
I'd also like to hear from other core vfio contributors. Thanks,
Alex
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=poc/vgpu
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=484316d855e3d61873122c295feb9a7457eeca21
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=6292c1de7f0758dd31496a454b4034507f38bd40
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=efac2cec97eab36edd01a2fe79aea67a04bd8842
> [5] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=76b3bfd6386a01f338780e1a37b5bad3f5a48d31
> [6] https://lore.kernel.org/lkml/20260303-rust-pci-sriov-v3-1-4443c35f0c88@redhat.com/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-14 18:12 ` Alex Williamson
@ 2026-09-14 21:36 ` Danilo Krummrich
2026-09-15 18:01 ` Alex Williamson
0 siblings, 1 reply; 24+ messages in thread
From: Danilo Krummrich @ 2026-09-14 21:36 UTC (permalink / raw)
To: Alex Williamson
Cc: Jason Gunthorpe, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Mon Sep 14, 2026 at 8:12 PM CEST, Alex Williamson wrote:
> Hi Danilo,
>
> On Fri, 11 Sep 2026 22:39:16 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>
>> Hi Alex, Jason, Zhi,
>>
>> On Sat Sep 5, 2026 at 10:11 AM CEST, Zhi Wang wrote:
>> > NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
>> > coordinated with the PF-side nova-core driver.
>>
>> [...]
>>
>> > drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
>>
>> This is going to be a longer response; sorry about this in advance.
>>
>> Looking at the FFI boundary introduced in the previous patch, I'm concerned that
>> it translates the driver model relationships we've expressed through Rust's
>> ownership and lifetime model back into raw pointers and lifetime assumptions
>> that callers must uphold. It also introduces manual lifecycle management across
>> the boundary, rather than preserving nova-core's RAII-based ownership model.
>>
>> I think implementing the NVIDIA vGPU driver in Rust would let us preserve those
>> relationships across the interface, make lifecycle management less error-prone,
>> and fit naturally alongside nova-core and nova-drm.
> [snip]
>>
>> If you've made it this far, thanks for reading through this long write-up. I
>> hope you find it useful. Please let me know if you have any questions or
>> thoughts.
>
> I can't really say I made it this far with comprehension, but thanks
> for the effort ;)
>
> The one piece here that I can actually review is [5], where
> dev_get_drvdata() is replaced with a vfio-pci-core struct pointer
> embedded in the struct pci_dev, which is a non-starter as far as having
> a common PCI-core shared by various drivers.
Well, that was just a quick hack to get it out of the way. :)
I think there are a couple of options.
(1) Make the PM helpers take a struct vfio_pci_core_device * in the first
place and let the driver forward to the helpers in its own PM callbacks.
(2) Provide an (optional?) driver callback that translates a struct pci_dev to
struct vfio_pci_core_device.
(3) Provide a macro for drivers to define PM ops, letting drivers provide the
function that translates struct pci_dev to struct vfio_pci_core_device.
(4) Give struct vfio_pci_core_device its own PM domain (which is probably a
bit overkill :).
I understand that the idea is to hide the PM handling in the vfio-pci framwork,
but I think the existing implementation is a bit of a layering violation, since
class device implementations shouldn't impose requirements on the bus device
private data layout.
I also think that the approach to fully hide it in the framework is only really
worth if it doesn't otherwise impose subtle requirements on the driver (such as
the layout requirement of the bus device private data).
Thus, I'd personally just go with (1) as it is the most honest approach in terms
of driver layering. But I think (2) is a good alternative that is not more
invasive than asking drivers to set the bus device private data to
struct vfio_pci_core_device *.
> My concerns are of course who is going to review the Rust vfio-pci
> variant drivers from a vfio perspective, not just a drm driver
> viewpoint.
I did put some considerations about this; I think Zhi would be a great candidate
for this. :) And as mentioned I can also take some of the responsibility, but I
also want to be honest about the fact that I have a lot on my plate already.
> Who is going to be responsive when the interfaces break and
> monitor vfio proactively to prevent such breakages, and how do we avoid
> derailing feature development in the core code base.
In practice we shouldn't see any breakages with the Rust code that wouldn't also
break the C VFIO drivers. If this would be the case it would mean that the Rust
code relies on guarantees that none of the C drivers rely on, which would likely
mean that is was never a guarantee that was actually promised by the VFIO core
code.
There may be cases where Rust code breaks, and C code won't, but those should be
mechanical things, such as a type mismatch where Rust is more careful, e.g. if
we'd change a CPP define to an enum, etc.
> Can a Rust vfio-pci variant driver be self-contained, or to what extent does
> it impose on the framework, such as the drvdata idiom.
As mentioned above, I think this one is more of a layering violation in the
vfio-pci-core; class devices shouldn't impose layout requirements on bus device
private data.
The reason C drivers can get away with it more easily is e.g. that C relies on
procedural cleanup and that all responsibility for managing lifetimes sits on
the drivers themselves, so it is easier for them to adjust. But in general, it
wouldn't work out if all class device registrations or other core primitives
would have the same expectation.
For Rust specifically it is that the driver core controls the lifetime of the
bus device private data, which is a fundamental requirement to e.g. represent
registrations as RAII types. E.g. the vfio::pci::Registration has to be stored
in the bus device private data, such that it is guaranteed to be correctly
destroyed on driver unbind.
To get back to your question, a Rust vfio-pci variant driver should be
self-contained. The interface sits in the abstraction that translates the C
driver API to a Rust driver API. It sometimes can help quite significantly (e.g.
in terms of how complex the Rust code needs to get in order to actually be safe)
if the C code does a minor adjustment, but it shouldn't be necessary.
For instance, the only addition to the driver core we have is an additional
callback in struct device_driver, and in the future an additional pointer in
struct device_private; none of those couldn't be worked around in some way
though.
On the other hand there are examples where the Rust introduction motivated
design improvements on the C side, or bug fixes for issues that were caught
while writing a safe abstraction. For instance, we recently had some fixes
around dyn IDs in the PCI and USB core, which both were motivated by Rust code.
> FWIW, AI can only go so far to support reviews. Having the code
> insight to ask the right questions is essential. A human in the loop
> is a requirement.
>
> Additionally, if we can't narrow the device matching to only the
> Nova-core supported VFs,
Agreed, and I think that should be possible. Is there a particular case you
think of where this wouldn't hold?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-14 21:36 ` Danilo Krummrich
@ 2026-09-15 18:01 ` Alex Williamson
2026-09-15 21:19 ` Danilo Krummrich
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Alex Williamson @ 2026-09-15 18:01 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Jason Gunthorpe, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm, alex
On Mon, 14 Sep 2026 23:36:24 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
> On Mon Sep 14, 2026 at 8:12 PM CEST, Alex Williamson wrote:
> > Hi Danilo,
> >
> > On Fri, 11 Sep 2026 22:39:16 +0200
> > "Danilo Krummrich" <dakr@kernel.org> wrote:
> >
> >> Hi Alex, Jason, Zhi,
> >>
> >> On Sat Sep 5, 2026 at 10:11 AM CEST, Zhi Wang wrote:
> >> > NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
> >> > coordinated with the PF-side nova-core driver.
> >>
> >> [...]
> >>
> >> > drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
> >>
> >> This is going to be a longer response; sorry about this in advance.
> >>
> >> Looking at the FFI boundary introduced in the previous patch, I'm concerned that
> >> it translates the driver model relationships we've expressed through Rust's
> >> ownership and lifetime model back into raw pointers and lifetime assumptions
> >> that callers must uphold. It also introduces manual lifecycle management across
> >> the boundary, rather than preserving nova-core's RAII-based ownership model.
> >>
> >> I think implementing the NVIDIA vGPU driver in Rust would let us preserve those
> >> relationships across the interface, make lifecycle management less error-prone,
> >> and fit naturally alongside nova-core and nova-drm.
> > [snip]
> >>
> >> If you've made it this far, thanks for reading through this long write-up. I
> >> hope you find it useful. Please let me know if you have any questions or
> >> thoughts.
> >
> > I can't really say I made it this far with comprehension, but thanks
> > for the effort ;)
> >
> > The one piece here that I can actually review is [5], where
> > dev_get_drvdata() is replaced with a vfio-pci-core struct pointer
> > embedded in the struct pci_dev, which is a non-starter as far as having
> > a common PCI-core shared by various drivers.
>
> Well, that was just a quick hack to get it out of the way. :)
>
> I think there are a couple of options.
>
> (1) Make the PM helpers take a struct vfio_pci_core_device * in the first
> place and let the driver forward to the helpers in its own PM callbacks.
>
> (2) Provide an (optional?) driver callback that translates a struct pci_dev to
> struct vfio_pci_core_device.
>
> (3) Provide a macro for drivers to define PM ops, letting drivers provide the
> function that translates struct pci_dev to struct vfio_pci_core_device.
>
> (4) Give struct vfio_pci_core_device its own PM domain (which is probably a
> bit overkill :).
>
> I understand that the idea is to hide the PM handling in the vfio-pci framwork,
> but I think the existing implementation is a bit of a layering violation, since
> class device implementations shouldn't impose requirements on the bus device
> private data layout.
>
> I also think that the approach to fully hide it in the framework is only really
> worth if it doesn't otherwise impose subtle requirements on the driver (such as
> the layout requirement of the bus device private data).
>
> Thus, I'd personally just go with (1) as it is the most honest approach in terms
> of driver layering. But I think (2) is a good alternative that is not more
> invasive than asking drivers to set the bus device private data to
> struct vfio_pci_core_device *.
I'd position this more as a library convention than a class layering
violation. vfio-pci was originally one driver, vfio-pci-core was
pulled out to enable device specific support, ex. migration, in a more
manageable way. struct vfio_pci_core_device is not strictly a class,
it's the object used by the library that variant drivers opt to use
rather than re-implementing vfio-pci from the ground up.
The conventions of that library mean variant drivers get things like
VGA routing and power management for free, in adherence with how these
features are exported by the core, and can choose to opt-in to common
error handling.
The use of drvdata is part of that convention and audited by the core
such that failed compliance is rejected on registration. Clearly we
could allow variant drivers to provide ops for their own callbacks and
export core helpers they can use, but only a Rust driver requires this
and we need to figure out how to do this without degrading the audit in
the core.
Turning vfio-pci-core into a proper class to be able to have a real
layering violation claim seems like a much larger project.
> > My concerns are of course who is going to review the Rust vfio-pci
> > variant drivers from a vfio perspective, not just a drm driver
> > viewpoint.
>
> I did put some considerations about this; I think Zhi would be a great candidate
> for this. :) And as mentioned I can also take some of the responsibility, but I
> also want to be honest about the fact that I have a lot on my plate already.
And that's part of the problem, there are bandwidth issues all around
and neither you nor Zhi are, as of yet, core vfio contributors. Rust
may well be the better approach relative to the interface to Nova-core,
but it's not clear it's the right approach for a subsystem that can't
maintain or evaluate it and only has a sketch of what it looks like.
> > Who is going to be responsive when the interfaces break and
> > monitor vfio proactively to prevent such breakages, and how do we avoid
> > derailing feature development in the core code base.
>
> In practice we shouldn't see any breakages with the Rust code that wouldn't also
> break the C VFIO drivers. If this would be the case it would mean that the Rust
> code relies on guarantees that none of the C drivers rely on, which would likely
> mean that is was never a guarantee that was actually promised by the VFIO core
> code.
s/practice/theory/
> There may be cases where Rust code breaks, and C code won't, but those should be
> mechanical things, such as a type mismatch where Rust is more careful, e.g. if
> we'd change a CPP define to an enum, etc.
This seems like a very optimistic outlook. I don't have experience
with Rust to claim otherwise, but I do have general software experience
to be suspicious of anything sounding so clean.
> > Can a Rust vfio-pci variant driver be self-contained, or to what extent does
> > it impose on the framework, such as the drvdata idiom.
>
> As mentioned above, I think this one is more of a layering violation in the
> vfio-pci-core; class devices shouldn't impose layout requirements on bus device
> private data.
>
> The reason C drivers can get away with it more easily is e.g. that C relies on
> procedural cleanup and that all responsibility for managing lifetimes sits on
> the drivers themselves, so it is easier for them to adjust. But in general, it
> wouldn't work out if all class device registrations or other core primitives
> would have the same expectation.
>
> For Rust specifically it is that the driver core controls the lifetime of the
> bus device private data, which is a fundamental requirement to e.g. represent
> registrations as RAII types. E.g. the vfio::pci::Registration has to be stored
> in the bus device private data, such that it is guaranteed to be correctly
> destroyed on driver unbind.
I see, so driver core has its own convention for how Rust drivers must
use drvdata.
> To get back to your question, a Rust vfio-pci variant driver should be
> self-contained. The interface sits in the abstraction that translates the C
> driver API to a Rust driver API. It sometimes can help quite significantly (e.g.
> in terms of how complex the Rust code needs to get in order to actually be safe)
> if the C code does a minor adjustment, but it shouldn't be necessary.
It's not clear to me to what extent these abstractions hinder our
ability to evolve and refactor the C code. It may not "lock in" the
core API for variant drivers, but it seems it raises the bar that any
significant code refactor likely needs to refactor the abstraction
layer, potentially the Rust variant driver itself, which imposes a
burden on the vfio community that has so far not introduced Rust into
the code base.
> For instance, the only addition to the driver core we have is an additional
> callback in struct device_driver, and in the future an additional pointer in
> struct device_private; none of those couldn't be worked around in some way
> though.
>
> On the other hand there are examples where the Rust introduction motivated
> design improvements on the C side, or bug fixes for issues that were caught
> while writing a safe abstraction. For instance, we recently had some fixes
> around dyn IDs in the PCI and USB core, which both were motivated by Rust code.
I have no doubt that integration with a more structured language would
lead to various improvements. However, it doesn't seem there are
resources to support it in the short term.
> > FWIW, AI can only go so far to support reviews. Having the code
> > insight to ask the right questions is essential. A human in the loop
> > is a requirement.
> >
> > Additionally, if we can't narrow the device matching to only the
> > Nova-core supported VFs,
>
> Agreed, and I think that should be possible. Is there a particular case you
> think of where this wouldn't hold?
The modalias scheme only matches on vendor/device ID, subsystem IDs,
base/sub class, and interface. Nothing in the PCI spec requires that
the VF device ID is different from the PF device ID. Variant drivers
will often present a larger match surface to avoid the ongoing
maintenance overhead of listing explicit device IDs. Such an approach
here would put us in the position I noted in the previous reply where
the Rust variant driver needs to fully support these devices via
vfio-pci-core on day one. Binding GPU PFs to vfio-pci is a current,
valid use case (VFs for some vendors as well). Thanks,
Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-15 18:01 ` Alex Williamson
@ 2026-09-15 21:19 ` Danilo Krummrich
2026-09-15 22:55 ` Zhi Wang
2026-09-16 14:17 ` Jason Gunthorpe
2026-09-15 23:44 ` Dave Airlie
2026-09-17 7:12 ` Gary Guo
2 siblings, 2 replies; 24+ messages in thread
From: Danilo Krummrich @ 2026-09-15 21:19 UTC (permalink / raw)
To: Alex Williamson
Cc: Jason Gunthorpe, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Tue Sep 15, 2026 at 8:01 PM CEST, Alex Williamson wrote:
> On Mon, 14 Sep 2026 23:36:24 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>
>> On Mon Sep 14, 2026 at 8:12 PM CEST, Alex Williamson wrote:
>> > Hi Danilo,
>> >
>> > On Fri, 11 Sep 2026 22:39:16 +0200
>> > "Danilo Krummrich" <dakr@kernel.org> wrote:
>> >
>> >> Hi Alex, Jason, Zhi,
>> >>
>> >> On Sat Sep 5, 2026 at 10:11 AM CEST, Zhi Wang wrote:
>> >> > NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
>> >> > coordinated with the PF-side nova-core driver.
>> >>
>> >> [...]
>> >>
>> >> > drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
>> >>
>> >> This is going to be a longer response; sorry about this in advance.
>> >>
>> >> Looking at the FFI boundary introduced in the previous patch, I'm concerned that
>> >> it translates the driver model relationships we've expressed through Rust's
>> >> ownership and lifetime model back into raw pointers and lifetime assumptions
>> >> that callers must uphold. It also introduces manual lifecycle management across
>> >> the boundary, rather than preserving nova-core's RAII-based ownership model.
>> >>
>> >> I think implementing the NVIDIA vGPU driver in Rust would let us preserve those
>> >> relationships across the interface, make lifecycle management less error-prone,
>> >> and fit naturally alongside nova-core and nova-drm.
>> > [snip]
>> >>
>> >> If you've made it this far, thanks for reading through this long write-up. I
>> >> hope you find it useful. Please let me know if you have any questions or
>> >> thoughts.
>> >
>> > I can't really say I made it this far with comprehension, but thanks
>> > for the effort ;)
>> >
>> > The one piece here that I can actually review is [5], where
>> > dev_get_drvdata() is replaced with a vfio-pci-core struct pointer
>> > embedded in the struct pci_dev, which is a non-starter as far as having
>> > a common PCI-core shared by various drivers.
>>
>> Well, that was just a quick hack to get it out of the way. :)
>>
>> I think there are a couple of options.
>>
>> (1) Make the PM helpers take a struct vfio_pci_core_device * in the first
>> place and let the driver forward to the helpers in its own PM callbacks.
>>
>> (2) Provide an (optional?) driver callback that translates a struct pci_dev to
>> struct vfio_pci_core_device.
>>
>> (3) Provide a macro for drivers to define PM ops, letting drivers provide the
>> function that translates struct pci_dev to struct vfio_pci_core_device.
>>
>> (4) Give struct vfio_pci_core_device its own PM domain (which is probably a
>> bit overkill :).
>>
>> I understand that the idea is to hide the PM handling in the vfio-pci framwork,
>> but I think the existing implementation is a bit of a layering violation, since
>> class device implementations shouldn't impose requirements on the bus device
>> private data layout.
>>
>> I also think that the approach to fully hide it in the framework is only really
>> worth if it doesn't otherwise impose subtle requirements on the driver (such as
>> the layout requirement of the bus device private data).
>>
>> Thus, I'd personally just go with (1) as it is the most honest approach in terms
>> of driver layering. But I think (2) is a good alternative that is not more
>> invasive than asking drivers to set the bus device private data to
>> struct vfio_pci_core_device *.
>
> I'd position this more as a library convention than a class layering
> violation. vfio-pci was originally one driver, vfio-pci-core was
> pulled out to enable device specific support, ex. migration, in a more
> manageable way. struct vfio_pci_core_device is not strictly a class,
> it's the object used by the library that variant drivers opt to use
> rather than re-implementing vfio-pci from the ground up.
>
> The conventions of that library mean variant drivers get things like
> VGA routing and power management for free, in adherence with how these
> features are exported by the core, and can choose to opt-in to common
> error handling.
>
> The use of drvdata is part of that convention and audited by the core
> such that failed compliance is rejected on registration. Clearly we
> could allow variant drivers to provide ops for their own callbacks and
> export core helpers they can use, but only a Rust driver requires this
> and we need to figure out how to do this without degrading the audit in
> the core.
The driver_data pointer in struct device is defined to be a pointer where a
driver can store *arbitrary* data for the duration the driver is bound to this
device.
This is an API contract the driver core provides for the struct device structure
when used in a bus device (such as struct pci_dev), i.e. it is an API contract
between the driver core and drivers in general.
It is *not* the Rust driver core code breaking a convention here. The Rust
driver core code just relies on its own subsystem's API contract.
The fact that the vfio-pci-core forces vfio-pci drivers to only ever store a
struct vfio_pci_core_device pointer there re-defines the driver core's API
contract and therefore is a layering violation.
> Turning vfio-pci-core into a proper class to be able to have a real
> layering violation claim seems like a much larger project.
What you are describing rather sounds to me as if struct vfio_pci_core_device
wants to be two things: a bus device layered on top of a PCI device and a class
device around struct vfio_device. But in this case it should implement a real
bus on top of the PCI bus.
Currently it really is a wrapper around struct vfio_device (and therefore a
class device) which is bending the purpose of the driver's bus device private
data pointer to make a connection with the real bus device underneath.
>> > My concerns are of course who is going to review the Rust vfio-pci
>> > variant drivers from a vfio perspective, not just a drm driver
>> > viewpoint.
>>
>> I did put some considerations about this; I think Zhi would be a great candidate
>> for this. :) And as mentioned I can also take some of the responsibility, but I
>> also want to be honest about the fact that I have a lot on my plate already.
>
> And that's part of the problem, there are bandwidth issues all around and
> neither you nor Zhi are, as of yet, core vfio contributors.
That is true, but I suggest to also see it from the perspective that this effort
is a good chance to get more people to engage with the VFIO subsystem in the
first place and eventually become experts that also contribute to the C core.
For the Rust abstraction, there are two competencies needed: knowledge about
writing a VFIO driver (which Zhi clearly covers) and knowledge about the driver
model and its Rust implementation specifics as well as the Rust ecosystem in
general (which is why I offered myself to take responsibility).
Remember the VFIO abstractions are translating the C VFIO driver APIs to Rust
APIs for Rust drivers. They don't need to mess with VFIO internals.
>> > Who is going to be responsive when the interfaces break and
>> > monitor vfio proactively to prevent such breakages, and how do we avoid
>> > derailing feature development in the core code base.
>>
>> In practice we shouldn't see any breakages with the Rust code that wouldn't also
>> break the C VFIO drivers. If this would be the case it would mean that the Rust
>> code relies on guarantees that none of the C drivers rely on, which would likely
>> mean that is was never a guarantee that was actually promised by the VFIO core
>> code.
>
> s/practice/theory/
As mentioned above, the abstractions are translating the C VFIO driver APIs to
Rust APIs for Rust drivers. I.e. they don't use other API than the C drivers
use. Thus, code breakages should be limited to what I mentioned below.
>> There may be cases where Rust code breaks, and C code won't, but those should be
>> mechanical things, such as a type mismatch where Rust is more careful, e.g. if
>> we'd change a CPP define to an enum, etc.
>
> This seems like a very optimistic outlook. I don't have experience
> with Rust to claim otherwise, but I do have general software experience
> to be suspicious of anything sounding so clean.
I maintain some subsystems that have Rust code, and I also maintain Rust code of
subsystems I otherwise do not maintain. I haven't seen anything break so far
other than for the reasons already mentioned.
Of course, that doesn't mean that mistakes can't happen. For instance, we could
have the case that the Rust code makes assumptions that the subsystem never
guaranteed in the first place. But that'd be something that could also happen
for any other driver and it would be a bug in the Rust code that the maintainers
of the Rust code have to take responsibility for.
>> > Can a Rust vfio-pci variant driver be self-contained, or to what extent does
>> > it impose on the framework, such as the drvdata idiom.
>>
>> As mentioned above, I think this one is more of a layering violation in the
>> vfio-pci-core; class devices shouldn't impose layout requirements on bus device
>> private data.
>>
>> The reason C drivers can get away with it more easily is e.g. that C relies on
>> procedural cleanup and that all responsibility for managing lifetimes sits on
>> the drivers themselves, so it is easier for them to adjust. But in general, it
>> wouldn't work out if all class device registrations or other core primitives
>> would have the same expectation.
>>
>> For Rust specifically it is that the driver core controls the lifetime of the
>> bus device private data, which is a fundamental requirement to e.g. represent
>> registrations as RAII types. E.g. the vfio::pci::Registration has to be stored
>> in the bus device private data, such that it is guaranteed to be correctly
>> destroyed on driver unbind.
>
> I see, so driver core has its own convention for how Rust drivers must
> use drvdata.
As mentioned above, the driver_data pointer of struct device is specifically
reserved for drivers to store arbitrary data while they are bound to the device.
The only difference is that in C it is a convention and drivers call
dev_set_drvdata() themselves, whereas in Rust we do it in the driver core code.
Note that clearing this pointer is also done in the driver core in C in
device_unbind_cleanup().
>> To get back to your question, a Rust vfio-pci variant driver should be
>> self-contained. The interface sits in the abstraction that translates the C
>> driver API to a Rust driver API. It sometimes can help quite significantly (e.g.
>> in terms of how complex the Rust code needs to get in order to actually be safe)
>> if the C code does a minor adjustment, but it shouldn't be necessary.
>
> It's not clear to me to what extent these abstractions hinder our
> ability to evolve and refactor the C code. It may not "lock in" the
> core API for variant drivers, but it seems it raises the bar that any
> significant code refactor likely needs to refactor the abstraction
> layer, potentially the Rust variant driver itself, which imposes a
> burden on the vfio community that has so far not introduced Rust into
> the code base.
In general, if the refactor does not affect C drivers, it shouldn't affect the
Rust API either.
If the refactor affects C drivers, it will also affect Rust drivers and the
change in the Rust abstraction could also be more complicated than the change
required in the C drivers.
The reason is that we design the Rust APIs at minmum in a way that any arbitrary
usage can not produce undefined behavior. Beyond that, we also always try to
design them in a way that it also at least encourages correct semantical use.
So in theory, it could happen that we need to figure out a new way to abstract
the C API in way that can't produce undefined behavior.
But if that happens it should be something minor, e.g. a new callback which
requires a new type state, an argument that requires a new type carrying some
invariants, etc.
The biggest part of the abstraction is the integration in the driver lifecycle,
which is a repeating pattern. For instance, if you look at the fwctl Rust code
it looks almsot the same.
From my experience maintaining both C and Rust code I can also tell that good
design is pretty straight forward to abstract in Rust to produce a safe API; bad
design isn't.
Where good design means APIs with defined ownership, clear lifetime
relationships, and a consistent API surface. Whereas APIs that have unclear
ownership and lifetime relationships and inconsistent API surfaces are a huge
pain in the butt to abstract.
The reason is that Rust needs to get those things straight somehow in order to
provide a safe API surface. I.e. an underlying bad API design (as defined above)
needs the Rust implementation to make quite some stretches.
>> For instance, the only addition to the driver core we have is an additional
>> callback in struct device_driver, and in the future an additional pointer in
>> struct device_private; none of those couldn't be worked around in some way
>> though.
>>
>> On the other hand there are examples where the Rust introduction motivated
>> design improvements on the C side, or bug fixes for issues that were caught
>> while writing a safe abstraction. For instance, we recently had some fixes
>> around dyn IDs in the PCI and USB core, which both were motivated by Rust code.
>
> I have no doubt that integration with a more structured language would
> lead to various improvements. However, it doesn't seem there are
> resources to support it in the short term.
As mentoined above, there are people volunteering now. Without starting it, it
can't scale further than that. :)
>> > FWIW, AI can only go so far to support reviews. Having the code
>> > insight to ask the right questions is essential. A human in the loop
>> > is a requirement.
>> >
>> > Additionally, if we can't narrow the device matching to only the
>> > Nova-core supported VFs,
>>
>> Agreed, and I think that should be possible. Is there a particular case you
>> think of where this wouldn't hold?
>
> The modalias scheme only matches on vendor/device ID, subsystem IDs,
> base/sub class, and interface. Nothing in the PCI spec requires that
> the VF device ID is different from the PF device ID. Variant drivers
> will often present a larger match surface to avoid the ongoing
> maintenance overhead of listing explicit device IDs. Such an approach
> here would put us in the position I noted in the previous reply where
> the Rust variant driver needs to fully support these devices via
> vfio-pci-core on day one. Binding GPU PFs to vfio-pci is a current,
> valid use case (VFs for some vendors as well). Thanks,
My understanding is that in order to support PFs being bound to the variant
driver the only thing necessary would be to fall back to the vfio-pci-core
callbacks. If that's the case, my PoC code can already do that.
However, I don't really see the use-case; you can't load nvidia-vgpu without
nova-core in the first place, so it would require to unbind nova-core through
sysfs force unbind, no?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-15 21:19 ` Danilo Krummrich
@ 2026-09-15 22:55 ` Zhi Wang
2026-09-16 14:17 ` Jason Gunthorpe
1 sibling, 0 replies; 24+ messages in thread
From: Zhi Wang @ 2026-09-15 22:55 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alex Williamson, Jason Gunthorpe, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Tue, 15 Sep 2026 23:19:01 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
Hi folks:
Since my name has been mentioned several times in this loop, It would
be better I chime in besides re-spin my patches.
Danilo, thanks for putting the PoC together. I am interested in working
it thorough and some parts of idea has already been in my rust SR-IOV
rework patches [1]. I am already contributing to nova-core and working
on the vGPU manager and VFIO driver, so I can help with nova-core
integration and testing.
Alex, I take your concerns about review and ongoing maintenance
seriously. I'm happy to help evaluate the PoC while those questions are
being discussed and commit on the parts of what you think are
maintainable and reasonable. For now, I've updated the existing C
series to use the new SR-IOV support and incorporate your review
feedback [2]. I'll keep following up on the remaining comments.
[1] https://lore.kernel.org/all/20260915205659.76841-1-zhiw@nvidia.com/
[2] https://lore.kernel.org/all/20260915211811.84790-1-zhiw@nvidia.com/
> On Tue Sep 15, 2026 at 8:01 PM CEST, Alex Williamson wrote:
> > On Mon, 14 Sep 2026 23:36:24 +0200
> > "Danilo Krummrich" <dakr@kernel.org> wrote:
> >
> >> On Mon Sep 14, 2026 at 8:12 PM CEST, Alex Williamson wrote:
> >> > Hi Danilo,
> >> >
> >> > On Fri, 11 Sep 2026 22:39:16 +0200
> >> > "Danilo Krummrich" <dakr@kernel.org> wrote:
> >> >
> >> >> Hi Alex, Jason, Zhi,
> >> >>
> >> >> On Sat Sep 5, 2026 at 10:11 AM CEST, Zhi Wang wrote:
> >> >> > NVIDIA vGPU VFs require their open, reset, and close
> >> >> > lifecycle to be coordinated with the PF-side nova-core
> >> >> > driver.
> >> >>
> >> >> [...]
> >> >>
> >> >> > drivers/vfio/pci/nvidia-vgpu/main.c | 253
> >> >> > ++++++++++++++++++++++++++
> >> >>
> >> >> This is going to be a longer response; sorry about this in
> >> >> advance.
> >> >>
> >> >> Looking at the FFI boundary introduced in the previous patch,
> >> >> I'm concerned that it translates the driver model relationships
> >> >> we've expressed through Rust's ownership and lifetime model
> >> >> back into raw pointers and lifetime assumptions that callers
> >> >> must uphold. It also introduces manual lifecycle management
> >> >> across the boundary, rather than preserving nova-core's
> >> >> RAII-based ownership model.
> >> >>
> >> >> I think implementing the NVIDIA vGPU driver in Rust would let
> >> >> us preserve those relationships across the interface, make
> >> >> lifecycle management less error-prone, and fit naturally
> >> >> alongside nova-core and nova-drm.
> >> > [snip]
> >> >>
> >> >> If you've made it this far, thanks for reading through this
> >> >> long write-up. I hope you find it useful. Please let me know if
> >> >> you have any questions or thoughts.
> >> >
> >> > I can't really say I made it this far with comprehension, but
> >> > thanks for the effort ;)
> >> >
> >> > The one piece here that I can actually review is [5], where
> >> > dev_get_drvdata() is replaced with a vfio-pci-core struct pointer
> >> > embedded in the struct pci_dev, which is a non-starter as far as
> >> > having a common PCI-core shared by various drivers.
> >>
> >> Well, that was just a quick hack to get it out of the way. :)
> >>
> >> I think there are a couple of options.
> >>
> >> (1) Make the PM helpers take a struct vfio_pci_core_device * in
> >> the first place and let the driver forward to the helpers in its
> >> own PM callbacks.
> >>
> >> (2) Provide an (optional?) driver callback that translates a
> >> struct pci_dev to struct vfio_pci_core_device.
> >>
> >> (3) Provide a macro for drivers to define PM ops, letting
> >> drivers provide the function that translates struct pci_dev to
> >> struct vfio_pci_core_device.
> >>
> >> (4) Give struct vfio_pci_core_device its own PM domain (which is
> >> probably a bit overkill :).
> >>
> >> I understand that the idea is to hide the PM handling in the
> >> vfio-pci framwork, but I think the existing implementation is a
> >> bit of a layering violation, since class device implementations
> >> shouldn't impose requirements on the bus device private data
> >> layout.
> >>
> >> I also think that the approach to fully hide it in the framework
> >> is only really worth if it doesn't otherwise impose subtle
> >> requirements on the driver (such as the layout requirement of the
> >> bus device private data).
> >>
> >> Thus, I'd personally just go with (1) as it is the most honest
> >> approach in terms of driver layering. But I think (2) is a good
> >> alternative that is not more invasive than asking drivers to set
> >> the bus device private data to struct vfio_pci_core_device *.
> >
> > I'd position this more as a library convention than a class layering
> > violation. vfio-pci was originally one driver, vfio-pci-core was
> > pulled out to enable device specific support, ex. migration, in a
> > more manageable way. struct vfio_pci_core_device is not strictly a
> > class, it's the object used by the library that variant drivers opt
> > to use rather than re-implementing vfio-pci from the ground up.
> >
> > The conventions of that library mean variant drivers get things like
> > VGA routing and power management for free, in adherence with how
> > these features are exported by the core, and can choose to opt-in
> > to common error handling.
> >
> > The use of drvdata is part of that convention and audited by the
> > core such that failed compliance is rejected on registration.
> > Clearly we could allow variant drivers to provide ops for their own
> > callbacks and export core helpers they can use, but only a Rust
> > driver requires this and we need to figure out how to do this
> > without degrading the audit in the core.
>
> The driver_data pointer in struct device is defined to be a pointer
> where a driver can store *arbitrary* data for the duration the driver
> is bound to this device.
>
> This is an API contract the driver core provides for the struct
> device structure when used in a bus device (such as struct pci_dev),
> i.e. it is an API contract between the driver core and drivers in
> general.
>
> It is *not* the Rust driver core code breaking a convention here. The
> Rust driver core code just relies on its own subsystem's API contract.
>
> The fact that the vfio-pci-core forces vfio-pci drivers to only ever
> store a struct vfio_pci_core_device pointer there re-defines the
> driver core's API contract and therefore is a layering violation.
>
> > Turning vfio-pci-core into a proper class to be able to have a real
> > layering violation claim seems like a much larger project.
>
> What you are describing rather sounds to me as if struct
> vfio_pci_core_device wants to be two things: a bus device layered on
> top of a PCI device and a class device around struct vfio_device. But
> in this case it should implement a real bus on top of the PCI bus.
>
> Currently it really is a wrapper around struct vfio_device (and
> therefore a class device) which is bending the purpose of the
> driver's bus device private data pointer to make a connection with
> the real bus device underneath.
>
> >> > My concerns are of course who is going to review the Rust
> >> > vfio-pci variant drivers from a vfio perspective, not just a drm
> >> > driver viewpoint.
> >>
> >> I did put some considerations about this; I think Zhi would be a
> >> great candidate for this. :) And as mentioned I can also take some
> >> of the responsibility, but I also want to be honest about the fact
> >> that I have a lot on my plate already.
> >
> > And that's part of the problem, there are bandwidth issues all
> > around and neither you nor Zhi are, as of yet, core vfio
> > contributors.
>
> That is true, but I suggest to also see it from the perspective that
> this effort is a good chance to get more people to engage with the
> VFIO subsystem in the first place and eventually become experts that
> also contribute to the C core.
>
> For the Rust abstraction, there are two competencies needed:
> knowledge about writing a VFIO driver (which Zhi clearly covers) and
> knowledge about the driver model and its Rust implementation
> specifics as well as the Rust ecosystem in general (which is why I
> offered myself to take responsibility).
>
> Remember the VFIO abstractions are translating the C VFIO driver APIs
> to Rust APIs for Rust drivers. They don't need to mess with VFIO
> internals.
>
> >> > Who is going to be responsive when the interfaces break and
> >> > monitor vfio proactively to prevent such breakages, and how do
> >> > we avoid derailing feature development in the core code base.
> >>
> >> In practice we shouldn't see any breakages with the Rust code that
> >> wouldn't also break the C VFIO drivers. If this would be the case
> >> it would mean that the Rust code relies on guarantees that none of
> >> the C drivers rely on, which would likely mean that is was never a
> >> guarantee that was actually promised by the VFIO core code.
> >
> > s/practice/theory/
>
> As mentioned above, the abstractions are translating the C VFIO
> driver APIs to Rust APIs for Rust drivers. I.e. they don't use other
> API than the C drivers use. Thus, code breakages should be limited to
> what I mentioned below.
>
> >> There may be cases where Rust code breaks, and C code won't, but
> >> those should be mechanical things, such as a type mismatch where
> >> Rust is more careful, e.g. if we'd change a CPP define to an enum,
> >> etc.
> >
> > This seems like a very optimistic outlook. I don't have experience
> > with Rust to claim otherwise, but I do have general software
> > experience to be suspicious of anything sounding so clean.
>
> I maintain some subsystems that have Rust code, and I also maintain
> Rust code of subsystems I otherwise do not maintain. I haven't seen
> anything break so far other than for the reasons already mentioned.
>
> Of course, that doesn't mean that mistakes can't happen. For
> instance, we could have the case that the Rust code makes assumptions
> that the subsystem never guaranteed in the first place. But that'd be
> something that could also happen for any other driver and it would be
> a bug in the Rust code that the maintainers of the Rust code have to
> take responsibility for.
>
> >> > Can a Rust vfio-pci variant driver be self-contained, or to what
> >> > extent does it impose on the framework, such as the drvdata
> >> > idiom.
> >>
> >> As mentioned above, I think this one is more of a layering
> >> violation in the vfio-pci-core; class devices shouldn't impose
> >> layout requirements on bus device private data.
> >>
> >> The reason C drivers can get away with it more easily is e.g. that
> >> C relies on procedural cleanup and that all responsibility for
> >> managing lifetimes sits on the drivers themselves, so it is easier
> >> for them to adjust. But in general, it wouldn't work out if all
> >> class device registrations or other core primitives would have the
> >> same expectation.
> >>
> >> For Rust specifically it is that the driver core controls the
> >> lifetime of the bus device private data, which is a fundamental
> >> requirement to e.g. represent registrations as RAII types. E.g.
> >> the vfio::pci::Registration has to be stored in the bus device
> >> private data, such that it is guaranteed to be correctly destroyed
> >> on driver unbind.
> >
> > I see, so driver core has its own convention for how Rust drivers
> > must use drvdata.
>
> As mentioned above, the driver_data pointer of struct device is
> specifically reserved for drivers to store arbitrary data while they
> are bound to the device.
>
> The only difference is that in C it is a convention and drivers call
> dev_set_drvdata() themselves, whereas in Rust we do it in the driver
> core code.
>
> Note that clearing this pointer is also done in the driver core in C
> in device_unbind_cleanup().
>
> >> To get back to your question, a Rust vfio-pci variant driver
> >> should be self-contained. The interface sits in the abstraction
> >> that translates the C driver API to a Rust driver API. It
> >> sometimes can help quite significantly (e.g. in terms of how
> >> complex the Rust code needs to get in order to actually be safe)
> >> if the C code does a minor adjustment, but it shouldn't be
> >> necessary.
> >
> > It's not clear to me to what extent these abstractions hinder our
> > ability to evolve and refactor the C code. It may not "lock in" the
> > core API for variant drivers, but it seems it raises the bar that
> > any significant code refactor likely needs to refactor the
> > abstraction layer, potentially the Rust variant driver itself,
> > which imposes a burden on the vfio community that has so far not
> > introduced Rust into the code base.
>
> In general, if the refactor does not affect C drivers, it shouldn't
> affect the Rust API either.
>
> If the refactor affects C drivers, it will also affect Rust drivers
> and the change in the Rust abstraction could also be more complicated
> than the change required in the C drivers.
>
> The reason is that we design the Rust APIs at minmum in a way that
> any arbitrary usage can not produce undefined behavior. Beyond that,
> we also always try to design them in a way that it also at least
> encourages correct semantical use.
>
> So in theory, it could happen that we need to figure out a new way to
> abstract the C API in way that can't produce undefined behavior.
>
> But if that happens it should be something minor, e.g. a new callback
> which requires a new type state, an argument that requires a new type
> carrying some invariants, etc.
>
> The biggest part of the abstraction is the integration in the driver
> lifecycle, which is a repeating pattern. For instance, if you look at
> the fwctl Rust code it looks almsot the same.
>
> From my experience maintaining both C and Rust code I can also tell
> that good design is pretty straight forward to abstract in Rust to
> produce a safe API; bad design isn't.
>
> Where good design means APIs with defined ownership, clear lifetime
> relationships, and a consistent API surface. Whereas APIs that have
> unclear ownership and lifetime relationships and inconsistent API
> surfaces are a huge pain in the butt to abstract.
>
> The reason is that Rust needs to get those things straight somehow in
> order to provide a safe API surface. I.e. an underlying bad API
> design (as defined above) needs the Rust implementation to make quite
> some stretches.
>
> >> For instance, the only addition to the driver core we have is an
> >> additional callback in struct device_driver, and in the future an
> >> additional pointer in struct device_private; none of those
> >> couldn't be worked around in some way though.
> >>
> >> On the other hand there are examples where the Rust introduction
> >> motivated design improvements on the C side, or bug fixes for
> >> issues that were caught while writing a safe abstraction. For
> >> instance, we recently had some fixes around dyn IDs in the PCI and
> >> USB core, which both were motivated by Rust code.
> >
> > I have no doubt that integration with a more structured language
> > would lead to various improvements. However, it doesn't seem there
> > are resources to support it in the short term.
>
> As mentoined above, there are people volunteering now. Without
> starting it, it can't scale further than that. :)
>
> >> > FWIW, AI can only go so far to support reviews. Having the code
> >> > insight to ask the right questions is essential. A human in the
> >> > loop is a requirement.
> >> >
> >> > Additionally, if we can't narrow the device matching to only the
> >> > Nova-core supported VFs,
> >>
> >> Agreed, and I think that should be possible. Is there a particular
> >> case you think of where this wouldn't hold?
> >
> > The modalias scheme only matches on vendor/device ID, subsystem IDs,
> > base/sub class, and interface. Nothing in the PCI spec requires
> > that the VF device ID is different from the PF device ID. Variant
> > drivers will often present a larger match surface to avoid the
> > ongoing maintenance overhead of listing explicit device IDs. Such
> > an approach here would put us in the position I noted in the
> > previous reply where the Rust variant driver needs to fully support
> > these devices via vfio-pci-core on day one. Binding GPU PFs to
> > vfio-pci is a current, valid use case (VFs for some vendors as
> > well). Thanks,
>
> My understanding is that in order to support PFs being bound to the
> variant driver the only thing necessary would be to fall back to the
> vfio-pci-core callbacks. If that's the case, my PoC code can already
> do that.
>
> However, I don't really see the use-case; you can't load nvidia-vgpu
> without nova-core in the first place, so it would require to unbind
> nova-core through sysfs force unbind, no?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-15 18:01 ` Alex Williamson
2026-09-15 21:19 ` Danilo Krummrich
@ 2026-09-15 23:44 ` Dave Airlie
2026-09-16 14:25 ` Jason Gunthorpe
2026-09-17 7:12 ` Gary Guo
2 siblings, 1 reply; 24+ messages in thread
From: Dave Airlie @ 2026-09-15 23:44 UTC (permalink / raw)
To: Alex Williamson
Cc: Danilo Krummrich, Jason Gunthorpe, Zhi Wang, acourbot, yishaih,
skolothumtho, kevin.tian, simona, ojeda, alex.gaynor, boqun.feng,
gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Wed, 16 Sept 2026 at 04:01, Alex Williamson <alex@shazbot.org> wrote:
>
> On Mon, 14 Sep 2026 23:36:24 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>
> > On Mon Sep 14, 2026 at 8:12 PM CEST, Alex Williamson wrote:
> > > Hi Danilo,
> > >
> > > On Fri, 11 Sep 2026 22:39:16 +0200
> > > "Danilo Krummrich" <dakr@kernel.org> wrote:
> > >
> > >> Hi Alex, Jason, Zhi,
> > >>
> > >> On Sat Sep 5, 2026 at 10:11 AM CEST, Zhi Wang wrote:
> > >> > NVIDIA vGPU VFs require their open, reset, and close lifecycle to be
> > >> > coordinated with the PF-side nova-core driver.
> > >>
> > >> [...]
> > >>
> > >> > drivers/vfio/pci/nvidia-vgpu/main.c | 253 ++++++++++++++++++++++++++
> > >>
> > >> This is going to be a longer response; sorry about this in advance.
> > >>
> > >> Looking at the FFI boundary introduced in the previous patch, I'm concerned that
> > >> it translates the driver model relationships we've expressed through Rust's
> > >> ownership and lifetime model back into raw pointers and lifetime assumptions
> > >> that callers must uphold. It also introduces manual lifecycle management across
> > >> the boundary, rather than preserving nova-core's RAII-based ownership model.
> > >>
> > >> I think implementing the NVIDIA vGPU driver in Rust would let us preserve those
> > >> relationships across the interface, make lifecycle management less error-prone,
> > >> and fit naturally alongside nova-core and nova-drm.
> > > [snip]
> > >>
> > >> If you've made it this far, thanks for reading through this long write-up. I
> > >> hope you find it useful. Please let me know if you have any questions or
> > >> thoughts.
> > >
> > > I can't really say I made it this far with comprehension, but thanks
> > > for the effort ;)
> > >
> > > The one piece here that I can actually review is [5], where
> > > dev_get_drvdata() is replaced with a vfio-pci-core struct pointer
> > > embedded in the struct pci_dev, which is a non-starter as far as having
> > > a common PCI-core shared by various drivers.
> >
> > Well, that was just a quick hack to get it out of the way. :)
> >
> > I think there are a couple of options.
> >
> > (1) Make the PM helpers take a struct vfio_pci_core_device * in the first
> > place and let the driver forward to the helpers in its own PM callbacks.
> >
> > (2) Provide an (optional?) driver callback that translates a struct pci_dev to
> > struct vfio_pci_core_device.
> >
> > (3) Provide a macro for drivers to define PM ops, letting drivers provide the
> > function that translates struct pci_dev to struct vfio_pci_core_device.
> >
> > (4) Give struct vfio_pci_core_device its own PM domain (which is probably a
> > bit overkill :).
> >
> > I understand that the idea is to hide the PM handling in the vfio-pci framwork,
> > but I think the existing implementation is a bit of a layering violation, since
> > class device implementations shouldn't impose requirements on the bus device
> > private data layout.
> >
> > I also think that the approach to fully hide it in the framework is only really
> > worth if it doesn't otherwise impose subtle requirements on the driver (such as
> > the layout requirement of the bus device private data).
> >
> > Thus, I'd personally just go with (1) as it is the most honest approach in terms
> > of driver layering. But I think (2) is a good alternative that is not more
> > invasive than asking drivers to set the bus device private data to
> > struct vfio_pci_core_device *.
>
> I'd position this more as a library convention than a class layering
> violation. vfio-pci was originally one driver, vfio-pci-core was
> pulled out to enable device specific support, ex. migration, in a more
> manageable way. struct vfio_pci_core_device is not strictly a class,
> it's the object used by the library that variant drivers opt to use
> rather than re-implementing vfio-pci from the ground up.
Not to be critical of an evolved design, vfio-pci-core should probably
be structured as a series of helpers that drivers can use, rather than
a midlayer. Midlayers plagued us in drm, let drivers drive and provide
the VGA/PM interfaces using standard vfio-pci-core methods rather than
forcing vfio-pci-core to own the pci device data.
Just because C let's us take shortcuts with struct layouts and
pointers, it doesn't mean it's a clean pattern we want to continue
with indefinitely. You can look at the rust use of the subsystem as a
chance to clean up some of the corners where layers aren't clean.
I just threw opencode at the refactor and for $12 of my massive budget
it produced
the prompt was:
"in linux/drivers/vfio/pci the vfio pci core decides that drivers must
install a vfio_pci_core_device struct to drvdata, this feels like a
layering violation, I'd like the vfio pci core to be a set of helpers
the drivers use instead of midlayering itself, can you plan that out
and then execute"
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 54
++++++++++++++++++++++++++++++++++++++----------------
drivers/vfio/pci/ism/main.c | 45
++++++++++++++++++++++++++++++++++++---------
drivers/vfio/pci/mlx5/main.c | 39
++++++++++++++++++++++++++++++++-------
drivers/vfio/pci/nvgrace-gpu/main.c | 45
+++++++++++++++++++++++++++++++++++----------
drivers/vfio/pci/pds/pci_drv.c | 34
+++++++++++++++++++++++++++++++---
drivers/vfio/pci/pds/vfio_dev.c | 5 +----
drivers/vfio/pci/qat/main.c | 38
++++++++++++++++++++++++++++++++------
drivers/vfio/pci/vfio_pci.c | 34
++++++++++++++++++++++++++++++++--
drivers/vfio/pci/vfio_pci_core.c | 66
+++++++++++++++++++++++-------------------------------------------
drivers/vfio/pci/virtio/main.c | 34
+++++++++++++++++++++++++++++++---
drivers/vfio/pci/xe/main.c | 34
+++++++++++++++++++++++++++++++---
11 files changed, 322 insertions(+), 106 deletions(-)
It seems to be correct code, and makes things look more like drivers
owning the outer structs. I did look for this pattern elsewhere in the
kernel and it's not common at all, in fact my quick search couldn't
spot any other instances which again suggests it's not a pattern we
should be leaning into.
As for the rust stuff, I think for vfio your drivers are just not that
much of a horror show or refactoring often enough that rust will be
anything but a blip on your radar, having Zhi/Danilo and access to any
sort of LLM will generally make most refactors trivial, lots of
maintainers keep bringing up the same concerns about refactors and
interfaces and breakage, but I'm not seeing the problems, and most of
the refactors that are rust driven are make the C code clearer.
Dave.
>
> > > Can a Rust vfio-pci variant driver be self-contained, or to what extent does
> > > it impose on the framework, such as the drvdata idiom.
> >
> > As mentioned above, I think this one is more of a layering violation in the
> > vfio-pci-core; class devices shouldn't impose layout requirements on bus device
> > private data.
> >
> > The reason C drivers can get away with it more easily is e.g. that C relies on
> > procedural cleanup and that all responsibility for managing lifetimes sits on
> > the drivers themselves, so it is easier for them to adjust. But in general, it
> > wouldn't work out if all class device registrations or other core primitives
> > would have the same expectation.
> >
> > For Rust specifically it is that the driver core controls the lifetime of the
> > bus device private data, which is a fundamental requirement to e.g. represent
> > registrations as RAII types. E.g. the vfio::pci::Registration has to be stored
> > in the bus device private data, such that it is guaranteed to be correctly
> > destroyed on driver unbind.
>
> I see, so driver core has its own convention for how Rust drivers must
> use drvdata.
>
> > To get back to your question, a Rust vfio-pci variant driver should be
> > self-contained. The interface sits in the abstraction that translates the C
> > driver API to a Rust driver API. It sometimes can help quite significantly (e.g.
> > in terms of how complex the Rust code needs to get in order to actually be safe)
> > if the C code does a minor adjustment, but it shouldn't be necessary.
>
> It's not clear to me to what extent these abstractions hinder our
> ability to evolve and refactor the C code. It may not "lock in" the
> core API for variant drivers, but it seems it raises the bar that any
> significant code refactor likely needs to refactor the abstraction
> layer, potentially the Rust variant driver itself, which imposes a
> burden on the vfio community that has so far not introduced Rust into
> the code base.
>
> > For instance, the only addition to the driver core we have is an additional
> > callback in struct device_driver, and in the future an additional pointer in
> > struct device_private; none of those couldn't be worked around in some way
> > though.
> >
> > On the other hand there are examples where the Rust introduction motivated
> > design improvements on the C side, or bug fixes for issues that were caught
> > while writing a safe abstraction. For instance, we recently had some fixes
> > around dyn IDs in the PCI and USB core, which both were motivated by Rust code.
>
> I have no doubt that integration with a more structured language would
> lead to various improvements. However, it doesn't seem there are
> resources to support it in the short term.
>
> > > FWIW, AI can only go so far to support reviews. Having the code
> > > insight to ask the right questions is essential. A human in the loop
> > > is a requirement.
> > >
> > > Additionally, if we can't narrow the device matching to only the
> > > Nova-core supported VFs,
> >
> > Agreed, and I think that should be possible. Is there a particular case you
> > think of where this wouldn't hold?
>
> The modalias scheme only matches on vendor/device ID, subsystem IDs,
> base/sub class, and interface. Nothing in the PCI spec requires that
> the VF device ID is different from the PF device ID. Variant drivers
> will often present a larger match surface to avoid the ongoing
> maintenance overhead of listing explicit device IDs. Such an approach
> here would put us in the position I noted in the previous reply where
> the Rust variant driver needs to fully support these devices via
> vfio-pci-core on day one. Binding GPU PFs to vfio-pci is a current,
> valid use case (VFs for some vendors as well). Thanks,
>
> Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-15 21:19 ` Danilo Krummrich
2026-09-15 22:55 ` Zhi Wang
@ 2026-09-16 14:17 ` Jason Gunthorpe
2026-09-16 15:38 ` Danilo Krummrich
1 sibling, 1 reply; 24+ messages in thread
From: Jason Gunthorpe @ 2026-09-16 14:17 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Tue, Sep 15, 2026 at 11:19:01PM +0200, Danilo Krummrich wrote:
> The driver_data pointer in struct device is defined to be a pointer where a
> driver can store *arbitrary* data for the duration the driver is bound to this
> device.
There are places in the kernel where the drvdata of the bound device
ends up owned by the subsystem, not the end driver. Yes, an ideal
driver + subsystem should never even need drvdatab beyond remove. Yet,
things are not perfect..
The fundamental issue is some kernel API surfaces that the subystem
needs to work with only provide a struct device in their callbacks and
the subystem has no option but to use the drvdata for its own purpose
to recover the subsystem specific data.
For example VFIO hooks into this nasty API:
ret = vga_client_register(pdev, vfio_pci_set_decode);
if (ret)
return ret;
Which doesn't provide a void * token to pass the core code's
struct.
Another is all the PCI callbacks which assume the op behind them uses
drvdata to get its data.
It is not necessarily easy to fix. Rrouting all those PCI callbacks
through trampolines in every single driver is really not an appealing
design. There are alot of VFIO drivers.
Maybe it needs a dev->drvdata and dev->subsystem_data, maybe it needs
some PCI thing where the pm ops can get a void *, IDK.
This is not some philosophical thing about busses or classes, it is
just an accommodation for the way the kernel is now. Fix the above and
you can get rid of it.
> > I have no doubt that integration with a more structured language would
> > lead to various improvements. However, it doesn't seem there are
> > resources to support it in the short term.
>
> As mentoined above, there are people volunteering now. Without starting it, it
> can't scale further than that. :)
There are lots of other vfio patches that need attention too, and it
seems we are short of that more than anything. Now you need to do a
bunch of C refactoring patches as well just to get things ready to
show a bunch of rust code. It is a lot of work.
I don't really understand in a nutshell why we should do this for nova
the mails were so long... Can we not just ignore the lifetime
imperfection for this?
> However, I don't really see the use-case; you can't load nvidia-vgpu without
> nova-core in the first place, so it would require to unbind nova-core through
> sysfs force unbind, no?
nvidia gpu is a more unique scenario, if you are building a general
bindings it has to support the flows like this.
I've wanted to rework the way the common ops are shimmed in for a
while, you'd probably want to do that before rust bindings.
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-15 23:44 ` Dave Airlie
@ 2026-09-16 14:25 ` Jason Gunthorpe
0 siblings, 0 replies; 24+ messages in thread
From: Jason Gunthorpe @ 2026-09-16 14:25 UTC (permalink / raw)
To: Dave Airlie
Cc: Alex Williamson, Danilo Krummrich, Zhi Wang, acourbot, yishaih,
skolothumtho, kevin.tian, simona, ojeda, alex.gaynor, boqun.feng,
gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Wed, Sep 16, 2026 at 09:44:35AM +1000, Dave Airlie wrote:
> As for the rust stuff, I think for vfio your drivers are just not that
> much of a horror show or refactoring often enough that rust will be
> anything but a blip on your radar,
Uh? I've been sending a couple whole subsystem refactorings touching
every driver a couple times a year for what, 5 years now?
There are still more that are needed, like trying to redo the drvdata
apparently. As well as stuff on the ops side, and more and more.
Granted you are looking at it after all this work, but there are still
problematic areas. drvdata, some of the ops, some of the locking, I
think there will be alot of C work to fix things up enough rust
bindings can happen sanely.
It is not so easy, your LLM looks like it put PM trampolines in every
driver, which I would not like to see.. Not sure how it solved vga
without touching drm..
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-16 14:17 ` Jason Gunthorpe
@ 2026-09-16 15:38 ` Danilo Krummrich
2026-09-16 16:28 ` Jason Gunthorpe
0 siblings, 1 reply; 24+ messages in thread
From: Danilo Krummrich @ 2026-09-16 15:38 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Wed Sep 16, 2026 at 4:17 PM CEST, Jason Gunthorpe wrote:
> On Tue, Sep 15, 2026 at 11:19:01PM +0200, Danilo Krummrich wrote:
>
>> The driver_data pointer in struct device is defined to be a pointer where a
>> driver can store *arbitrary* data for the duration the driver is bound to this
>> device.
>
> There are places in the kernel where the drvdata of the bound device
> ends up owned by the subsystem, not the end driver. Yes, an ideal
> driver + subsystem should never even need drvdatab beyond remove. Yet,
> things are not perfect..
>
> The fundamental issue is some kernel API surfaces that the subystem
> needs to work with only provide a struct device in their callbacks and
> the subystem has no option but to use the drvdata for its own purpose
> to recover the subsystem specific data.
>
> For example VFIO hooks into this nasty API:
>
> ret = vga_client_register(pdev, vfio_pci_set_decode);
> if (ret)
> return ret;
>
> Which doesn't provide a void * token to pass the core code's
> struct.
>
> Another is all the PCI callbacks which assume the op behind them uses
> drvdata to get its data.
This is where the class (i.e. vfio-pci) should instead take a driver callback,
as only the driver really knowns about the layering details.
Usually, class device implementations can't make assumptions of the underlying
bus, because they have to work for any bus. I.e. there's no other way than
providing helpers and letting drivers do the glue code between the bus and the
class device.
> It is not necessarily easy to fix. Rrouting all those PCI callbacks
> through trampolines in every single driver is really not an appealing
> design. There are alot of VFIO drivers.
The reason this seems undesirable from a vfio-pci perspective is that it is
special in the sense that it is a class device that is specifically built to sit
on top of a spcific bus device (i.e. struct pci_dev).
Since this is a rare (maybe even unique?) edge case, the driver core has indeed
no infrastructure in place to represent this.
> Maybe it needs a dev->drvdata and dev->subsystem_data, maybe it needs
> some PCI thing where the pm ops can get a void *, IDK.
It still makes me think that there should be some closer integration of vfio-pci
with the PCI core, as it is specifically built for this bus.
In fact, what you say above is the generalization of my "hack" [1], which, the
more I think about this, seems actually less of a hack. :)
I would object a bit to the generalization with dev->subsystem_data as it
screams for abuse, but the thing in [1] seems more and more reasonable to me.
(Another option would be to really split it up and have a virtual vfio-pci bus
on top of PCI, which would also allow for custom match logic, but that also
seems pretty overkill.)
>> As mentoined above, there are people volunteering now. Without starting it, it
>> can't scale further than that. :)
>
> There are lots of other vfio patches that need attention too, and it
> seems we are short of that more than anything. Now you need to do a
> bunch of C refactoring patches as well just to get things ready to
> show a bunch of rust code. It is a lot of work.
It's only the drvdata thing, what else is missing?
> I don't really understand in a nutshell why we should do this for nova
> the mails were so long... Can we not just ignore the lifetime
> imperfection for this?
I mentioned some points in the first two paragraphs of [2]. Besides that, I
don't see a reason why we should spend time and effort for working out the
inferior solution, where the better alternative is even less effort, contributes
to better quality and stability of the whole driver project and also offers a
chance for the vfio subsystem to gain new contributors and gather experience
with the language that has proven itself in many areas already.
I mean, there'd still be the option to make it an experiment and say let's add
the abstractions and the nvidia-vgpu driver and see how it works out for a while
before allowing more Rust drivers. And if it really turns out to be bad, it
should also be easy to rip it out, replace nvidia-vgpu with a C driver and throw
in a crappy FFI layer. :)
>> However, I don't really see the use-case; you can't load nvidia-vgpu without
>> nova-core in the first place, so it would require to unbind nova-core through
>> sysfs force unbind, no?
>
> nvidia gpu is a more unique scenario, if you are building a general
> bindings it has to support the flows like this.
Sure, as mentioned, the code I sketched up should already be able to do this.
> I've wanted to rework the way the common ops are shimmed in for a
> while, you'd probably want to do that before rust bindings.
TBH, I don't think it makes a difference; having Rust abstractions is pretty
much the same as having another driver. I.e. it would be equivalent to saying
"before we accept another pci-vfio driver we need to do some rework".
Actually, I even think it can be an advantage, as you could think of the Rust
abstractions like a driver with built-in correctness checks, so it can help to
validate the changes.
Of course, this requires the responsibles of the Rust code to help with that and
I think we have this commitment.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?id=76b3bfd6386a01f338780e1a37b5bad3f5a48d31
[2] https://lore.kernel.org/nova-gpu/DLCRZLO06SIO.LS7TWQXIPZSQ@kernel.org/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-16 15:38 ` Danilo Krummrich
@ 2026-09-16 16:28 ` Jason Gunthorpe
2026-09-16 18:02 ` Danilo Krummrich
2026-09-17 1:49 ` Dave Airlie
0 siblings, 2 replies; 24+ messages in thread
From: Jason Gunthorpe @ 2026-09-16 16:28 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Wed, Sep 16, 2026 at 05:38:21PM +0200, Danilo Krummrich wrote:
> Usually, class device implementations can't make assumptions of the underlying
> bus, because they have to work for any bus. I.e. there's no other way than
> providing helpers and letting drivers do the glue code between the bus and the
> class device.
You should think of VFIO as a series of helper libraries. One of those
libraries is "here are all the PCI PM ops you need".
Drivers rarely need to customize these ops, they just need to wire it
up to the support library to avoid a bunch of code duplication.
Stated another way - the very point of vfio-pci is to remove
duplicated code from the drivers.
So if we wanted to push hard on removing drvdata, and don't want to
touch the struct device I would probably say to #define up a way for
the driver to build its unique trampolines. It wastes a bunch of .text
but at least it doesn't duplicate code.
> The reason this seems undesirable from a vfio-pci perspective is that it is
> special in the sense that it is a class device that is specifically built to sit
> on top of a spcific bus device (i.e. struct pci_dev).
It is a library, all these ideas to do things with the driver core to
implement a library make no architectural sense.
> It still makes me think that there should be some closer integration of vfio-pci
> with the PCI core, as it is specifically built for this bus.
It has such a basic need I don't see this as a reason to pollute pci
core with any vfio specific things. Like I would nak your [1], that's
completely wrong layering.
> > I don't really understand in a nutshell why we should do this for nova
> > the mails were so long... Can we not just ignore the lifetime
> > imperfection for this?
>
> I mentioned some points in the first two paragraphs of [2]. Besides that, I
> don't see a reason why we should spend time and effort for working out the
> inferior solution, where the better alternative is even less effort, contributes
> to better quality and stability of the whole driver project and also offers a
> chance for the vfio subsystem to gain new contributors and gather experience
> with the language that has proven itself in many areas already.
It seems to be quite a leap that it is less effort. IDK..
> TBH, I don't think it makes a difference; having Rust abstractions is pretty
> much the same as having another driver. I.e. it would be equivalent to saying
> "before we accept another pci-vfio driver we need to do some rework".
Well, it is, but thats the point when judging effort..
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-16 16:28 ` Jason Gunthorpe
@ 2026-09-16 18:02 ` Danilo Krummrich
2026-09-17 13:28 ` Jason Gunthorpe
2026-09-17 1:49 ` Dave Airlie
1 sibling, 1 reply; 24+ messages in thread
From: Danilo Krummrich @ 2026-09-16 18:02 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Wed Sep 16, 2026 at 6:28 PM CEST, Jason Gunthorpe wrote:
> On Wed, Sep 16, 2026 at 05:38:21PM +0200, Danilo Krummrich wrote:
>> Usually, class device implementations can't make assumptions of the underlying
>> bus, because they have to work for any bus. I.e. there's no other way than
>> providing helpers and letting drivers do the glue code between the bus and the
>> class device.
>
> You should think of VFIO as a series of helper libraries. One of those
> libraries is "here are all the PCI PM ops you need".
>
> Drivers rarely need to customize these ops, they just need to wire it
> up to the support library to avoid a bunch of code duplication.
>
> Stated another way - the very point of vfio-pci is to remove
> duplicated code from the drivers.
>
> So if we wanted to push hard on removing drvdata, and don't want to
> touch the struct device I would probably say to #define up a way for
> the driver to build its unique trampolines. It wastes a bunch of .text
> but at least it doesn't duplicate code.
>
>> The reason this seems undesirable from a vfio-pci perspective is that it is
>> special in the sense that it is a class device that is specifically built to sit
>> on top of a spcific bus device (i.e. struct pci_dev).
>
> It is a library, all these ideas to do things with the driver core to
> implement a library make no architectural sense.
Both is true, it is a library, and it is also a class device built on top of
another class device (struct vfio_device) that accomodates to a specific
underlying bus (PCI).
Which means that this library has the need to make a generic connection between
the class (VFIO) and the bus (PCI). And this is something that the kernel has no
generic solution for (and isn't represented by the driver core in any way).
Your dev->subsystem_data idea would accomodate this connection. However, I
object to this, as it'd be for a very special case and I'd be worried it is
abused by other subsystems in odd ways.
Besides that, I still think that the correct thing to do is to have the glue in
the driver and just provide the helpers in the best possible form. In fact, this
is what a library should do - provide the helpers, but do not directly
interfere with with other layers.
>> It still makes me think that there should be some closer integration of vfio-pci
>> with the PCI core, as it is specifically built for this bus.
>
> It has such a basic need I don't see this as a reason to pollute pci
> core with any vfio specific things. Like I would nak your [1], that's
> completely wrong layering.
As I said in the beginning, it is just a "hack"; and in fact it is the exact
same layering violation as abusing dev->driver_data or adding
dev->subsystem_data.
The reasons I called it "more reasonable" and "less of a hack" in my previous
reply are that it doesn't break an existing (driver core) API contract and it
only affects the the exact two layers this is about in the first place.
>> > I don't really understand in a nutshell why we should do this for nova
>> > the mails were so long... Can we not just ignore the lifetime
>> > imperfection for this?
>>
>> I mentioned some points in the first two paragraphs of [2]. Besides that, I
>> don't see a reason why we should spend time and effort for working out the
>> inferior solution, where the better alternative is even less effort, contributes
>> to better quality and stability of the whole driver project and also offers a
>> chance for the vfio subsystem to gain new contributors and gather experience
>> with the language that has proven itself in many areas already.
>
> It seems to be quite a leap that it is less effort. IDK..
>
>> TBH, I don't think it makes a difference; having Rust abstractions is pretty
>> much the same as having another driver. I.e. it would be equivalent to saying
>> "before we accept another pci-vfio driver we need to do some rework".
>
> Well, it is, but thats the point when judging effort..
So it seems that we agree that there isn't really a difference between adding a
new C or Rust driver in this regard.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-16 16:28 ` Jason Gunthorpe
2026-09-16 18:02 ` Danilo Krummrich
@ 2026-09-17 1:49 ` Dave Airlie
2026-09-17 7:01 ` Gary Guo
2026-09-17 13:20 ` Jason Gunthorpe
1 sibling, 2 replies; 24+ messages in thread
From: Dave Airlie @ 2026-09-17 1:49 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Danilo Krummrich, Alex Williamson, Zhi Wang, acourbot, yishaih,
skolothumtho, kevin.tian, simona, ojeda, alex.gaynor, boqun.feng,
gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Thu, 17 Sept 2026 at 02:28, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Sep 16, 2026 at 05:38:21PM +0200, Danilo Krummrich wrote:
> > Usually, class device implementations can't make assumptions of the underlying
> > bus, because they have to work for any bus. I.e. there's no other way than
> > providing helpers and letting drivers do the glue code between the bus and the
> > class device.
>
> You should think of VFIO as a series of helper libraries. One of those
> libraries is "here are all the PCI PM ops you need".
>
> Drivers rarely need to customize these ops, they just need to wire it
> up to the support library to avoid a bunch of code duplication.
It's currently a library being used as a midlayer. demidlayering it so
drivers control drvdata removes that.
I sent the diffstat that covers all the current vfio pci drivers and
that didn't seem excessive.
>
> Stated another way - the very point of vfio-pci is to remove
> duplicated code from the drivers.
>
> So if we wanted to push hard on removing drvdata, and don't want to
> touch the struct device I would probably say to #define up a way for
> the driver to build its unique trampolines. It wastes a bunch of .text
> but at least it doesn't duplicate code.
+static pci_ers_result_t hisi_acc_vfio_pci_aer_err_detected(
+ struct pci_dev *pdev, pci_channel_state_t state)
+{
+ struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
+
+ return vfio_pci_core_aer_err_detected(&hisi_acc_vdev->core_device,
state);
+}
+
+static int hisi_acc_vfio_pci_runtime_suspend(struct device *dev)
+{
+ struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
+
+ return vfio_pci_core_runtime_suspend(&hisi_acc_vdev->core_device);
+}
+
+static int hisi_acc_vfio_pci_runtime_resume(struct device *dev)
+{
+ struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
+
+ return vfio_pci_core_runtime_resume(&hisi_acc_vdev->core_device);
+}
+
+static const struct dev_pm_ops hisi_acc_vfio_pci_pm_ops = {
+ SET_RUNTIME_PM_OPS(hisi_acc_vfio_pci_runtime_suspend,
+ hisi_acc_vfio_pci_runtime_resume, NULL)
+};
is how much code it adds to current drivers, this doesn't seem
excessive for 10 drivers,
like you could obfuscate it a bit with some macros, but I really don't
see a lot of value in hiding
what is effectively just standard driver boilerplate.
Dave.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-17 1:49 ` Dave Airlie
@ 2026-09-17 7:01 ` Gary Guo
2026-09-17 13:20 ` Jason Gunthorpe
1 sibling, 0 replies; 24+ messages in thread
From: Gary Guo @ 2026-09-17 7:01 UTC (permalink / raw)
To: Dave Airlie, Jason Gunthorpe
Cc: Danilo Krummrich, Alex Williamson, Zhi Wang, acourbot, yishaih,
skolothumtho, kevin.tian, simona, ojeda, alex.gaynor, boqun.feng,
gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Thu Sep 17, 2026 at 2:49 AM BST, Dave Airlie wrote:
> On Thu, 17 Sept 2026 at 02:28, Jason Gunthorpe <jgg@nvidia.com> wrote:
>>
>> Stated another way - the very point of vfio-pci is to remove
>> duplicated code from the drivers.
>>
>> So if we wanted to push hard on removing drvdata, and don't want to
>> touch the struct device I would probably say to #define up a way for
>> the driver to build its unique trampolines. It wastes a bunch of .text
>> but at least it doesn't duplicate code.
>
> +static pci_ers_result_t hisi_acc_vfio_pci_aer_err_detected(
> + struct pci_dev *pdev, pci_channel_state_t state)
> +{
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
> +
> + return vfio_pci_core_aer_err_detected(&hisi_acc_vdev->core_device,
> state);
> +}
> +
> +static int hisi_acc_vfio_pci_runtime_suspend(struct device *dev)
> +{
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
> +
> + return vfio_pci_core_runtime_suspend(&hisi_acc_vdev->core_device);
> +}
> +
> +static int hisi_acc_vfio_pci_runtime_resume(struct device *dev)
> +{
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
> +
> + return vfio_pci_core_runtime_resume(&hisi_acc_vdev->core_device);
> +}
> +
> +static const struct dev_pm_ops hisi_acc_vfio_pci_pm_ops = {
> + SET_RUNTIME_PM_OPS(hisi_acc_vfio_pci_runtime_suspend,
> + hisi_acc_vfio_pci_runtime_resume, NULL)
> +};
>
> is how much code it adds to current drivers, this doesn't seem
> excessive for 10 drivers,
> like you could obfuscate it a bit with some macros, but I really don't
> see a lot of value in hiding
> what is effectively just standard driver boilerplate.
If it's just these callbacks, you could even have vfio-pci directly proxying these
callbacks by having a `struct device` -> `struct vfio_pci_core_device` hashmap.
(Not that I'm saying it's a good idea).
Best,
Gary
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-15 18:01 ` Alex Williamson
2026-09-15 21:19 ` Danilo Krummrich
2026-09-15 23:44 ` Dave Airlie
@ 2026-09-17 7:12 ` Gary Guo
2 siblings, 0 replies; 24+ messages in thread
From: Gary Guo @ 2026-09-17 7:12 UTC (permalink / raw)
To: Alex Williamson, Danilo Krummrich
Cc: Jason Gunthorpe, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Tue Sep 15, 2026 at 7:01 PM BST, Alex Williamson wrote:
> On Mon, 14 Sep 2026 23:36:24 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>> On Mon Sep 14, 2026 at 8:12 PM CEST, Alex Williamson wrote:
>> > The one piece here that I can actually review is [5], where
>> > dev_get_drvdata() is replaced with a vfio-pci-core struct pointer
>> > embedded in the struct pci_dev, which is a non-starter as far as having
>> > a common PCI-core shared by various drivers.
>>
>> Well, that was just a quick hack to get it out of the way. :)
>>
>> I think there are a couple of options.
>>
>> (1) Make the PM helpers take a struct vfio_pci_core_device * in the first
>> place and let the driver forward to the helpers in its own PM callbacks.
>>
>> (2) Provide an (optional?) driver callback that translates a struct pci_dev to
>> struct vfio_pci_core_device.
>>
>> (3) Provide a macro for drivers to define PM ops, letting drivers provide the
>> function that translates struct pci_dev to struct vfio_pci_core_device.
>>
>> (4) Give struct vfio_pci_core_device its own PM domain (which is probably a
>> bit overkill :).
>>
>> I understand that the idea is to hide the PM handling in the vfio-pci framwork,
>> but I think the existing implementation is a bit of a layering violation, since
>> class device implementations shouldn't impose requirements on the bus device
>> private data layout.
>>
>> I also think that the approach to fully hide it in the framework is only really
>> worth if it doesn't otherwise impose subtle requirements on the driver (such as
>> the layout requirement of the bus device private data).
>>
>> Thus, I'd personally just go with (1) as it is the most honest approach in terms
>> of driver layering. But I think (2) is a good alternative that is not more
>> invasive than asking drivers to set the bus device private data to
>> struct vfio_pci_core_device *.
>
> I'd position this more as a library convention than a class layering
> violation. vfio-pci was originally one driver, vfio-pci-core was
> pulled out to enable device specific support, ex. migration, in a more
> manageable way. struct vfio_pci_core_device is not strictly a class,
> it's the object used by the library that variant drivers opt to use
> rather than re-implementing vfio-pci from the ground up.
>
> The conventions of that library mean variant drivers get things like
> VGA routing and power management for free, in adherence with how these
> features are exported by the core, and can choose to opt-in to common
> error handling.
>
> The use of drvdata is part of that convention and audited by the core
> such that failed compliance is rejected on registration. Clearly we
> could allow variant drivers to provide ops for their own callbacks and
> export core helpers they can use, but only a Rust driver requires this
> and we need to figure out how to do this without degrading the audit in
> the core.
>
> Turning vfio-pci-core into a proper class to be able to have a real
> layering violation claim seems like a much larger project.
>
>> > My concerns are of course who is going to review the Rust vfio-pci
>> > variant drivers from a vfio perspective, not just a drm driver
>> > viewpoint.
FWIW, if we want vfio-pci to be a middle layer (looks like there're some
disagreements about this), we could quite simply achieve this by define
mod vfio_pci {
trait Driver {
/* callbacks here */
}
struct Adapter<D: Driver>(D);
impl pci::Driver for Adapter {
...
}
}
and then the way for people to be using this would to create the "Adapter" which
does the middle layering and register *that* as pci driver instead. Then
all PCI callbacks will first land in vfio-pci abstration's code before it
filters through things in the driver. This way, vfio-pci-core imlements the pci
driver then it controls its drvdata layout.
Best,
Gary
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-17 1:49 ` Dave Airlie
2026-09-17 7:01 ` Gary Guo
@ 2026-09-17 13:20 ` Jason Gunthorpe
1 sibling, 0 replies; 24+ messages in thread
From: Jason Gunthorpe @ 2026-09-17 13:20 UTC (permalink / raw)
To: Dave Airlie
Cc: Danilo Krummrich, Alex Williamson, Zhi Wang, acourbot, yishaih,
skolothumtho, kevin.tian, simona, ojeda, alex.gaynor, boqun.feng,
gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Thu, Sep 17, 2026 at 11:49:29AM +1000, Dave Airlie wrote:
> +static pci_ers_result_t hisi_acc_vfio_pci_aer_err_detected(
> + struct pci_dev *pdev, pci_channel_state_t state)
> +{
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
> +
> + return vfio_pci_core_aer_err_detected(&hisi_acc_vdev->core_device,
> state);
> +}
> +
> +static int hisi_acc_vfio_pci_runtime_suspend(struct device *dev)
> +{
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
> +
> + return vfio_pci_core_runtime_suspend(&hisi_acc_vdev->core_device);
> +}
> +
> +static int hisi_acc_vfio_pci_runtime_resume(struct device *dev)
> +{
> + struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
> +
> + return vfio_pci_core_runtime_resume(&hisi_acc_vdev->core_device);
> +}
> +
> +static const struct dev_pm_ops hisi_acc_vfio_pci_pm_ops = {
> + SET_RUNTIME_PM_OPS(hisi_acc_vfio_pci_runtime_suspend,
> + hisi_acc_vfio_pci_runtime_resume, NULL)
> +};
>
> is how much code it adds to current drivers, this doesn't seem
> excessive for 10 drivers,
Well, I think it is alot, and there it sucks if new ones are added.
> like you could obfuscate it a bit with some macros, but I really don't
> see a lot of value in hiding
> what is effectively just standard driver boilerplate.
This is exactly the kind of boilerplate people have been removing with
macros. I would macro it..
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-16 18:02 ` Danilo Krummrich
@ 2026-09-17 13:28 ` Jason Gunthorpe
2026-09-17 14:50 ` Danilo Krummrich
0 siblings, 1 reply; 24+ messages in thread
From: Jason Gunthorpe @ 2026-09-17 13:28 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Wed, Sep 16, 2026 at 08:02:17PM +0200, Danilo Krummrich wrote:
> >> TBH, I don't think it makes a difference; having Rust abstractions is pretty
> >> much the same as having another driver. I.e. it would be equivalent to saying
> >> "before we accept another pci-vfio driver we need to do some rework".
> >
> > Well, it is, but thats the point when judging effort..
>
> So it seems that we agree that there isn't really a difference between adding a
> new C or Rust driver in this regard.
To be clear I was trying to say I think you are are underestimating
how much work this is and how long it will take. Including all the
preconditon C reworks has to be included when considering how much
work is involved.
Why can't we just go forward with what Zhi already drafted? Do we
really need another side quest?
I cannot forsee rust bindings for vfio until sometime in 2027.
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-17 13:28 ` Jason Gunthorpe
@ 2026-09-17 14:50 ` Danilo Krummrich
2026-09-17 17:10 ` Jason Gunthorpe
0 siblings, 1 reply; 24+ messages in thread
From: Danilo Krummrich @ 2026-09-17 14:50 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Thu Sep 17, 2026 at 3:28 PM CEST, Jason Gunthorpe wrote:
> On Wed, Sep 16, 2026 at 08:02:17PM +0200, Danilo Krummrich wrote:
>> >> TBH, I don't think it makes a difference; having Rust abstractions is pretty
>> >> much the same as having another driver. I.e. it would be equivalent to saying
>> >> "before we accept another pci-vfio driver we need to do some rework".
>> >
>> > Well, it is, but thats the point when judging effort..
>>
>> So it seems that we agree that there isn't really a difference between adding a
>> new C or Rust driver in this regard.
>
> To be clear I was trying to say I think you are are underestimating
> how much work this is and how long it will take. Including all the
> preconditon C reworks has to be included when considering how much
> work is involved.
I'm not sure I follow what you mean by "preconditon C reworks".
Do you refer to the drvdata thing? That seems rather trivial to address?
Or do you refer to other C core reworks people are working on? If so, they are
only relevant if they affect the drivers. And if they affect the drivers,
there's not really a difference between adding a new C driver and adding a Rust
abstraction.
> Why can't we just go forward with what Zhi already drafted? Do we
> really need another side quest?
Because I don't think it is a side quest. As mentioned, the FFI layer is already
more complicated than the Rust abstraction we need, and I also don't see that
this work is progressed further than the Rust abstraction I drafted. So, why not
just go for the proper solution right away?
I still have to finish some talks, but I could probably finish that work until
LPC and then we can walk through it?
> I cannot forsee rust bindings for vfio until sometime in 2027.
What is different in 2027 than is now?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-17 14:50 ` Danilo Krummrich
@ 2026-09-17 17:10 ` Jason Gunthorpe
2026-09-17 20:09 ` Danilo Krummrich
0 siblings, 1 reply; 24+ messages in thread
From: Jason Gunthorpe @ 2026-09-17 17:10 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Thu, Sep 17, 2026 at 04:50:07PM +0200, Danilo Krummrich wrote:
> On Thu Sep 17, 2026 at 3:28 PM CEST, Jason Gunthorpe wrote:
> > On Wed, Sep 16, 2026 at 08:02:17PM +0200, Danilo Krummrich wrote:
> >> >> TBH, I don't think it makes a difference; having Rust abstractions is pretty
> >> >> much the same as having another driver. I.e. it would be equivalent to saying
> >> >> "before we accept another pci-vfio driver we need to do some rework".
> >> >
> >> > Well, it is, but thats the point when judging effort..
> >>
> >> So it seems that we agree that there isn't really a difference between adding a
> >> new C or Rust driver in this regard.
> >
> > To be clear I was trying to say I think you are are underestimating
> > how much work this is and how long it will take. Including all the
> > preconditon C reworks has to be included when considering how much
> > work is involved.
>
> I'm not sure I follow what you mean by "preconditon C reworks".
Rust bindings often seem to need C code rework for Rust, drvdata here
is an example.
> Or do you refer to other C core reworks people are working on? If so, they are
> only relevant if they affect the drivers. And if they affect the drivers,
> there's not really a difference between adding a new C driver and adding a Rust
> abstraction.
A new C driver should not need reworks?
> > I cannot forsee rust bindings for vfio until sometime in 2027.
>
> What is different in 2027 than is now?
I mean I think that is how long it will take to merge something like
that, given how many other vfio series there are, the lack of
expertise, and some general expectations. There are already lots of
series for vfio that are in front of something like this in the queue.
And then we end up with what will be one of the most complex drivers
in a form nobody familiar with vfio can review.
It just seems like a much bigger thing to me than whatever the ffi
problem is..
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-17 17:10 ` Jason Gunthorpe
@ 2026-09-17 20:09 ` Danilo Krummrich
2026-09-23 20:05 ` Alex Williamson
0 siblings, 1 reply; 24+ messages in thread
From: Danilo Krummrich @ 2026-09-17 20:09 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Alex Williamson, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm
On Thu Sep 17, 2026 at 7:10 PM CEST, Jason Gunthorpe wrote:
> On Thu, Sep 17, 2026 at 04:50:07PM +0200, Danilo Krummrich wrote:
>> On Thu Sep 17, 2026 at 3:28 PM CEST, Jason Gunthorpe wrote:
>> > On Wed, Sep 16, 2026 at 08:02:17PM +0200, Danilo Krummrich wrote:
>> >> >> TBH, I don't think it makes a difference; having Rust abstractions is pretty
>> >> >> much the same as having another driver. I.e. it would be equivalent to saying
>> >> >> "before we accept another pci-vfio driver we need to do some rework".
>> >> >
>> >> > Well, it is, but thats the point when judging effort..
>> >>
>> >> So it seems that we agree that there isn't really a difference between adding a
>> >> new C or Rust driver in this regard.
>> >
>> > To be clear I was trying to say I think you are are underestimating
>> > how much work this is and how long it will take. Including all the
>> > preconditon C reworks has to be included when considering how much
>> > work is involved.
>>
>> I'm not sure I follow what you mean by "preconditon C reworks".
>
> Rust bindings often seem to need C code rework for Rust, drvdata here
> is an example.
I can assure you we can set this concern aside. The driver core code, auxiliary,
platform, PCI, DMA, scatterlist, DRM, firmware loader, IRQ, I/O and fwctl, to
name just a few, did not need any changes. And from the top of my head I can't
think of any case where changes were needed.
For VFIO I already wrote the code, and no changes were required either, except
for the drvdata things of course.
But, as discussed, this is an existing layering violation. The Rust driver code
code simply relies on its own subsystem's API contract. Please let's not frame
this as a Rust issue.
Plus, it is trivial to address, so it luckily is not a huge problem anyway. :)
>> Or do you refer to other C core reworks people are working on? If so, they are
>> only relevant if they affect the drivers. And if they affect the drivers,
>> there's not really a difference between adding a new C driver and adding a Rust
>> abstraction.
>
> A new C driver should not need reworks?
If a new C driver is not affected the Rust abstraction won't be affected either.
They are just users of the same APIs the C drivers consume. So, that doesn't
seem to be a concern then.
>> > I cannot forsee rust bindings for vfio until sometime in 2027.
>>
>> What is different in 2027 than is now?
>
> I mean I think that is how long it will take to merge something like
> that, given how many other vfio series there are, the lack of
> expertise, and some general expectations. There are already lots of
> series for vfio that are in front of something like this in the queue.
The same would then be true for an nvidia-vgpu driver written in C, no? Again,
the Rust abstractions are nothing else than a driver in the end. They only
consume the driver facing APIs and translate them to a safe Rust API.
> And then we end up with what will be one of the most complex drivers
> in a form nobody familiar with vfio can review.
I think this isn't the case; the people you refer to can still review semantics
and vfio design specifics. It's not like everything is different.
For instance, there's still a get_region_info() callback on
vfio::pci::Operations where people can review whether this semantically makes
sense.
And for the language specifics we have a lot of Rust familiar people around who
will review the code and help to improve it.
Why not see this as a chance to scale on both ends? More people on both ends
will get familiar with VFIO and Rust at the same time.
Thanks,
Danilo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver
2026-09-17 20:09 ` Danilo Krummrich
@ 2026-09-23 20:05 ` Alex Williamson
0 siblings, 0 replies; 24+ messages in thread
From: Alex Williamson @ 2026-09-23 20:05 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Jason Gunthorpe, Zhi Wang, acourbot, yishaih, skolothumtho,
kevin.tian, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
ecourtney, cjia, smitra, kjaju, alkumar, ankita, aniketa,
kwankhede, targupta, nova-gpu, linux-kernel, zhiwang, kvm, alex,
Simon Song (SW-GPU)
On Thu, 17 Sep 2026 22:09:58 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
> On Thu Sep 17, 2026 at 7:10 PM CEST, Jason Gunthorpe wrote:
> > On Thu, Sep 17, 2026 at 04:50:07PM +0200, Danilo Krummrich wrote:
> >> On Thu Sep 17, 2026 at 3:28 PM CEST, Jason Gunthorpe wrote:
> >> > On Wed, Sep 16, 2026 at 08:02:17PM +0200, Danilo Krummrich wrote:
> >> >> >> TBH, I don't think it makes a difference; having Rust abstractions is pretty
> >> >> >> much the same as having another driver. I.e. it would be equivalent to saying
> >> >> >> "before we accept another pci-vfio driver we need to do some rework".
> >> >> >
> >> >> > Well, it is, but thats the point when judging effort..
> >> >>
> >> >> So it seems that we agree that there isn't really a difference between adding a
> >> >> new C or Rust driver in this regard.
> >> >
> >> > To be clear I was trying to say I think you are are underestimating
> >> > how much work this is and how long it will take. Including all the
> >> > preconditon C reworks has to be included when considering how much
> >> > work is involved.
> >>
> >> I'm not sure I follow what you mean by "preconditon C reworks".
> >
> > Rust bindings often seem to need C code rework for Rust, drvdata here
> > is an example.
>
> I can assure you we can set this concern aside. The driver core code, auxiliary,
> platform, PCI, DMA, scatterlist, DRM, firmware loader, IRQ, I/O and fwctl, to
> name just a few, did not need any changes. And from the top of my head I can't
> think of any case where changes were needed.
>
> For VFIO I already wrote the code, and no changes were required either, except
> for the drvdata things of course.
>
> But, as discussed, this is an existing layering violation. The Rust driver code
> code simply relies on its own subsystem's API contract. Please let's not frame
> this as a Rust issue.
>
> Plus, it is trivial to address, so it luckily is not a huge problem anyway. :)
>
> >> Or do you refer to other C core reworks people are working on? If so, they are
> >> only relevant if they affect the drivers. And if they affect the drivers,
> >> there's not really a difference between adding a new C driver and adding a Rust
> >> abstraction.
> >
> > A new C driver should not need reworks?
>
> If a new C driver is not affected the Rust abstraction won't be affected either.
> They are just users of the same APIs the C drivers consume. So, that doesn't
> seem to be a concern then.
>
> >> > I cannot forsee rust bindings for vfio until sometime in 2027.
> >>
> >> What is different in 2027 than is now?
> >
> > I mean I think that is how long it will take to merge something like
> > that, given how many other vfio series there are, the lack of
> > expertise, and some general expectations. There are already lots of
> > series for vfio that are in front of something like this in the queue.
>
> The same would then be true for an nvidia-vgpu driver written in C, no? Again,
> the Rust abstractions are nothing else than a driver in the end. They only
> consume the driver facing APIs and translate them to a safe Rust API.
>
> > And then we end up with what will be one of the most complex drivers
> > in a form nobody familiar with vfio can review.
>
> I think this isn't the case; the people you refer to can still review semantics
> and vfio design specifics. It's not like everything is different.
>
> For instance, there's still a get_region_info() callback on
> vfio::pci::Operations where people can review whether this semantically makes
> sense.
>
> And for the language specifics we have a lot of Rust familiar people around who
> will review the code and help to improve it.
>
> Why not see this as a chance to scale on both ends? More people on both ends
> will get familiar with VFIO and Rust at the same time.
This thread has gone cold for a few days and nothing really seems to be
dissuading from the direction of implementing the Nova interfacing
vfio-pci variant vGPU driver in Rust. Zhi has been investigating the
gaps and bugs in the PoC, and afaik, nothing stands out as a blocker.
I'll do my best to use LLM assisted reviews on my part, but the
expectation should be that I won't be merging any Rust code without
formal reviews on list. Over time I hope to be able to contribute from
that perspective, but the Rust community is going to need to hold their
end of the bargain as well. If meaningful reviews are not provided,
the driver can be orphaned and removed.
The Rust community also needs to be proactive in supporting the vfio
community such that abstractions and interfaces between C and Rust
don't become a barrier for C-based development. Keeping the Rust code
working when the C code changes is the Rust community's job, we
shouldn't expect more than "best effort" from the existing C-focused
community.
I also want to be clear, I'm not endorsing Rust as a pattern anywhere
beyond this individual vfio-pci variant driver. Any other proposals
would need to bring a comparably sized community to sponsor them.
Dave's approach to make vfio-pci-core a stateless helper with respect
to drvdata, with a dash of Jason's macro idea to minimize the driver
boilerplate for the common cases, seems like the right direction to
remove that convention. I've handed off my own LLM generated version
that does this and it should be refined and posted soon.
So, unless there's an aspect we haven't considered, it seems the Rust
variant driver should be the direction here. Thanks,
Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2026-09-23 20:05 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-05 8:11 [PATCH 00/13] Introduce NVIDIA vGPU manager and VFIO variant driver Zhi Wang
2026-09-05 8:11 ` [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU " Zhi Wang
2026-09-09 3:00 ` Alex Williamson
2026-09-11 20:39 ` Danilo Krummrich
2026-09-14 18:12 ` Alex Williamson
2026-09-14 21:36 ` Danilo Krummrich
2026-09-15 18:01 ` Alex Williamson
2026-09-15 21:19 ` Danilo Krummrich
2026-09-15 22:55 ` Zhi Wang
2026-09-16 14:17 ` Jason Gunthorpe
2026-09-16 15:38 ` Danilo Krummrich
2026-09-16 16:28 ` Jason Gunthorpe
2026-09-16 18:02 ` Danilo Krummrich
2026-09-17 13:28 ` Jason Gunthorpe
2026-09-17 14:50 ` Danilo Krummrich
2026-09-17 17:10 ` Jason Gunthorpe
2026-09-17 20:09 ` Danilo Krummrich
2026-09-23 20:05 ` Alex Williamson
2026-09-17 1:49 ` Dave Airlie
2026-09-17 7:01 ` Gary Guo
2026-09-17 13:20 ` Jason Gunthorpe
2026-09-15 23:44 ` Dave Airlie
2026-09-16 14:25 ` Jason Gunthorpe
2026-09-17 7:12 ` Gary Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox