Re: [PATCH v3 10/10] samples: rust: add Rust SR-IOV VF driver sample

["Danilo Krummrich" <dakr@kernel.org>]

On Wed Sep 30, 2026 at 12:18 PM CEST, Zhi Wang wrote:
> +#[pin_data(PinnedDrop)]
> +struct PfDriverData<'bound> {
> +    // Keep the device alive until the registration stops exposing `PfApi::pdev`.

What does this mean?

> +    #[pin]
> +    _registration: pci::VfRegistration<'bound, PfApiForLt>,
> +    pdev: ARef<pci::Device>,

	pdev: &'bound pci::Device<Bound>,

> +}
> +
> +#[pin_data(PinnedDrop)]
> +struct VfDriverData {
> +    pdev: ARef<pci::Device>,

	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),
> +        ()
> +    )]
> +);
> +
> +kernel::pci_device_table!(
> +    VF_TABLE,
> +    <SampleVfDriver as pci::Driver>::IdInfo,
> +    [(
> +        // E1000_DEV_ID_82576_VF
> +        pci::DeviceId::from_id(pci::Vendor::INTEL, 0x10ca),
> +        ()
> +    )]
> +);

Since this is two drivers in the same module, can we please put everythng that
belongs to the PF driver first and then everything that belongs into the VF
driver second please? We could also make it two separate files.

> +#[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 {
> +        pin_init::pin_init_scope(move || {
> +            dev_info!(
> +                pdev,
> +                "Probe Rust SR-IOV PF sample (PCI ID: {}, 0x{:x}).\n",
> +                pdev.vendor_id(),
> +                pdev.device_id()
> +            );
> +
> +            pdev.enable_device_mem()?;
> +            pdev.set_master();
> +
> +            Ok(try_pin_init!(PfDriverData {
> +                // SAFETY:
> +                // - probe has exclusive access to this PF before SR-IOV is enabled;
> +                // - the registration is pinned in the PF driver data and dropped before `pdev`;
> +                // - no other registration is created for this PF; and
> +                // - VFs are enabled only after probe by `sriov_enable`.
> +                _registration <- unsafe {
> +                    pci::VfRegistration::new(
> +                        pdev,
> +                        try_pin_init!(PfApi {
> +                            pdev,
> +                            requests <- new_mutex!(0),
> +                        }),
> +                    )
> +                },
> +                pdev: pdev.into(),
> +            }))
> +        })

Let's put everything within a single try_pin_init!() block please.

> +#[vtable]
> +impl pci::Driver for SampleVfDriver {
> +    type IdInfo = ();
> +    type Data<'bound> = VfDriverData;
> +
> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &VF_TABLE;
> +
> +    fn probe<'bound>(
> +        pdev: &'bound pci::Device<Core<'_>>,
> +        _info: Option<&'bound Self::IdInfo>,
> +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> +        pin_init::pin_init_scope(move || {
> +            dev_info!(
> +                pdev,
> +                "Probe Rust SR-IOV VF sample (PCI ID: {}, 0x{:x}).\n",
> +                pdev.vendor_id(),
> +                pdev.device_id()
> +            );
> +
> +            let pdev_bound: &'bound pci::Device<Bound> = pdev;
> +            let pf_api = pdev_bound.vf_registration_data::<PfApiForLt>()?;
> +
> +            pdev.enable_device_mem()?;
> +            pdev.set_master();
> +
> +            let request = pf_api.submit(pdev)?;
> +            dev_info!(pdev, "Submitted request {} through PF data.\n", request);
> +
> +            Ok(try_pin_init!(VfDriverData { pdev: pdev.into() }))
> +        })

Same here. Also no need for pdev_bound, it derefs automatically, you can just
call pdev.vf_registration_data().

> +    }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for PfDriverData<'_> {
> +    fn drop(self: Pin<&mut Self>) {
> +        dev_info!(self.pdev, "Remove Rust SR-IOV PF sample.\n");
> +    }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for VfDriverData {
> +    fn drop(self: Pin<&mut Self>) {
> +        dev_info!(self.pdev, "Remove Rust SR-IOV VF sample.\n");
> +    }
> +}

Let's drop those, we don't really need them.

Here's the two probe() functions I came up with:

    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()
                );
            },

            _: {
                pdev.enable_device_mem()?;
                pdev.set_master();
            },

            // SAFETY:
            // - probe has exclusive access to this PF before SR-IOV is enabled;
            // - the registration is pinned in the PF driver data and dropped before `pdev`;
            // - no other registration is created for this PF; and
            // - VFs are enabled only after probe by `sriov_enable`.
            _registration <- unsafe {
                pci::VfRegistration::new(
                    pdev,
                    try_pin_init!(PfApi {
                        pdev,
                        requests <- new_mutex!(0),
                    }),
                )
            },

            pdev,
        })
    }

    fn probe<'bound>(
        pdev: &'bound pci::Device<Core<'_>>,
        _info: Option<&'bound Self::IdInfo>,
    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
        try_pin_init!(VfDriverData {
            _: {
                dev_info!(
                    pdev,
                    "Probe Rust SR-IOV VF sample (PCI ID: {}, 0x{:x}).\n",
                    pdev.vendor_id(),
                    pdev.device_id()
                );
            },

            _: {
                pdev.enable_device_mem()?;
                pdev.set_master();
            },

            pdev,

            _: {
                let pf_api = pdev.vf_registration_data::<PfApiForLt>()?;

                let request = pf_api.submit(pdev)?;
                dev_info!(pdev, "Submitted request {} through PF data.\n", request);
            },
        })
    }

Note that enable_device() and set_master() have their own block as they will be
replaced with a pci::DeviceEnableGuard soon.