* 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; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread