Re: [PATCH v5 9/9] samples: rust: add Rust SR-IOV PF and VF driver samples

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sat Oct 31, 2026 at 2:12 AM JST, Simon Song wrote:
> From: Peter Colberg <peter@colberg.org>
>
> Add separate PF and VF driver modules that demonstrate SR-IOV using
> QEMU's 82576 (igb) emulation.
>
> The PF publishes a pinned API object in its private data and exposes a
> submit_request() helper. The VF calls this helper with its Bound device
> borrow. The helper performs the typed registration lookup in the PF
> crate, keeping the API object and its lifetime encoding private.
>
> Adapt the nova-core/nova-drm rules in drivers/gpu/Makefile, preserving
> their metadata, export-selection and MODVERSIONS recipes with the sample
> object names and paths. Set the PF crate name explicitly because its Rust
> root file has a _main suffix.
>
> Keep the helper out of line so the VF module has a real symbol dependency
> on the PF module. The module loader therefore holds the PF module while
> the VF module is loaded, and modprobe loads the dependency automatically.
>
> Each driver has its own module registration. The VF module can be unloaded
> and reloaded while the PF remains bound. Unbinding the PF disables SR-IOV
> and removes its VFs before releasing the registered data. The shared
> Kconfig option builds both drivers as modules or both into the kernel.

I agree with Danilo that having two modules is unneeded here, especially
if loading the vf module ends up automatically loading the pf one
anyway. Let's keep sample code simple.

<...>
> diff --git a/samples/rust/rust_driver_sriov_pf_main.rs b/samples/rust/rust_driver_sriov_pf_main.rs
> new file mode 100644
> index 00000000000..b0795663aea
> --- /dev/null
> +++ b/samples/rust/rust_driver_sriov_pf_main.rs
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust SR-IOV driver sample based on QEMU's 82576 ([igb]) emulation.
> +//!
> +//! To make this driver probe, QEMU must be run with `-device igb`.
> +//!
> +//! Further, enable [vIOMMU] with interrupt remapping using, e.g.,
> +//!
> +//! `-M q35,accel=kvm,kernel-irqchip=split -device intel-iommu,intremap=on,caching-mode=on`
> +//!
> +//! and append `intel_iommu=on` to the guest kernel arguments.
> +//!
> +//! Load `rust_driver_sriov_vf` with `modprobe` to load both the PF and VF drivers.
> +//! The VF module depends on the PF module through [`submit_request()`].
> +//!
> +//! [igb]: https://www.qemu.org/docs/master/system/devices/igb.html
> +//! [vIOMMU]: https://wiki.qemu.org/Features/VT-d
> +
> +use kernel::{
> +    device::{
> +        Bound,
> +        Core, //
> +    },
> +    new_mutex,
> +    pci,
> +    prelude::*,
> +    sync::Mutex,
> +    types::CovariantForLt, //
> +};
> +
> +struct SamplePfDriver;
> +
> +#[pin_data]
> +struct PfApi<'bound> {
> +    pdev: &'bound pci::Device<Bound>,
> +    #[pin]
> +    requests: Mutex<u64>,
> +}
> +
> +type PfApiForLt = CovariantForLt!(PfApi<'_>);
> +
> +impl PfApi<'_> {
> +    fn submit(self: Pin<&Self>, vf: &pci::Device<Bound>) -> Result<u64> {
> +        let mut requests = self.requests.lock();
> +        let request = requests.checked_add(1).ok_or(EOVERFLOW)?;
> +        *requests = request;
> +        drop(requests);

Would it make sense to just use an atomic type here?

> +
> +        dev_info!(
> +            self.pdev,
> +            "Handle PF request {} from VF devfn {:#x}.\n",
> +            request,
> +            vf.dev_id()
> +        );
> +
> +        Ok(request)
> +    }
> +}
> +
> +/// Submits a request to the PF associated with a bound VF.
> +// Keep this call out of line so the VF module has a symbol dependency on the PF module.
> +#[inline(never)]
> +pub fn submit_request(vf: &pci::Device<Bound>) -> Result<u64> {
> +    vf.vf_registration_data::<PfApiForLt>()?.submit(vf)
> +}
> +
> +#[pin_data]
> +struct PfDriverData<'bound> {
> +    #[pin]
> +    _registration: pci::VfRegistration<'bound, PfApiForLt>,
> +    pdev: &'bound pci::Device<Bound>,
> +}
> +
> +kernel::pci_device_table!(
> +    PF_TABLE,
> +    <SamplePfDriver as pci::Driver>::IdInfo,
> +    [(
> +        // E1000_DEV_ID_82576
> +        pci::DeviceId::from_id(pci::Vendor::INTEL, 0x10c9),
> +        ()
> +    )]
> +);
> +
> +#[vtable]
> +impl pci::Driver for SamplePfDriver {
> +    type IdInfo = ();
> +    type Data<'bound> = PfDriverData<'bound>;
> +
> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PF_TABLE;
> +
> +    fn probe<'bound>(
> +        pdev: &'bound pci::Device<Core<'_>>,
> +        _info: Option<&'bound Self::IdInfo>,
> +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> +        try_pin_init!(PfDriverData {
> +            _: {
> +                dev_info!(
> +                    pdev,
> +                    "Probe Rust SR-IOV PF sample (PCI ID: {}, 0x{:x}).\n",
> +                    pdev.vendor_id(),
> +                    pdev.device_id()
> +                );
> +            },
> +            // SAFETY: The registration is embedded in the PF driver's private data.
> +            _registration <- unsafe {
> +                pci::VfRegistration::new(
> +                    pdev,
> +                    try_pin_init!(PfApi {
> +                        pdev,
> +                        requests <- new_mutex!(0),
> +                    }),
> +                )
> +            },
> +            _: {
> +                pdev.enable_device_mem()?;
> +                pdev.set_master();
> +            },
> +            pdev,
> +        })
> +    }
> +
> +    fn sriov_enable<'a>(
> +        dev: &'a pci::Device<Core<'_>>,
> +        data: Pin<&Self::Data<'a>>,
> +        token: pci::SriovEnable<'a>,
> +    ) -> Result<pci::SriovEnabled<'a>> {
> +        dev_info!(
> +            data.pdev,
> +            "Enable SR-IOV (PCI ID: {}, 0x{:x}).\n",
> +            data.pdev.vendor_id(),
> +            data.pdev.device_id()
> +        );
> +
> +        let num_vfs = token.num_vfs();
> +        let enabled = token.enable(num_vfs)?;
> +        assert_eq!(dev.num_vf(), num_vfs);

Is this assert useful?

> +        Ok(enabled)
> +    }
> +
> +    fn sriov_disable<'a>(
> +        dev: &'a pci::Device<Core<'_>>,
> +        data: Pin<&Self::Data<'a>>,
> +        token: pci::SriovDisable<'a>,
> +    ) -> Result {
> +        dev_info!(
> +            data.pdev,
> +            "Disable SR-IOV (PCI ID: {}, 0x{:x}).\n",
> +            data.pdev.vendor_id(),
> +            data.pdev.device_id()
> +        );
> +
> +        token.disable();
> +        assert_eq!(dev.num_vf(), 0);

Same here.

> +        Ok(())
> +    }
> +}
> +
> +kernel::module_pci_driver! {
> +    type: SamplePfDriver,
> +    name: "rust_driver_sriov_pf",
> +    authors: ["Peter Colberg"],
> +    description: "Rust SR-IOV PF driver",
> +    license: "GPL v2",
> +}
> diff --git a/samples/rust/rust_driver_sriov_vf.rs b/samples/rust/rust_driver_sriov_vf.rs
> new file mode 100644
> index 00000000000..bc8e5fdc8ce
> --- /dev/null
> +++ b/samples/rust/rust_driver_sriov_vf.rs
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Virtual Function driver for the Rust SR-IOV sample.
> +
> +use kernel::{
> +    device::{
> +        Bound,
> +        Core, //
> +    },
> +    pci,
> +    prelude::*, //
> +};
> +
> +struct SampleVfDriver;
> +
> +#[pin_data]
> +struct VfDriverData<'bound> {
> +    pdev: &'bound pci::Device<Bound>,

Is `pdev` ever used for something? It looks like the intent was to use
that to call `submit_request`, but this only happens in the `probe`
method at the moment.