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

[Zhi Wang <zhiw@nvidia.com>]

On Wed, 30 Sep 2026 17:15:00 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:

> On Wed Sep 30, 2026 at 12:18 PM CEST, Zhi Wang wrote:

<snip>

> > +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.
> 

I think it is a great idea. I will split them in the next spin.

> > +#[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.