Re: [PATCH 3/8] rust: pci: add {enable,disable}_sriov(), to control SR-IOV capability

[Peter Colberg <pcolberg@redhat.com>]

On Fri, Nov 21, 2025 at 07:28:33PM -0400, Jason Gunthorpe wrote:
> On Wed, Nov 19, 2025 at 05:19:07PM -0500, Peter Colberg wrote:
> > Add methods to enable and disable the Single Root I/O Virtualization
> > (SR-IOV) capability for a PCI device. The wrapped C methods take care
> > of validating whether the device is a Physical Function (PF), whether
> > SR-IOV is currently disabled (or enabled), and whether the number of
> > requested VFs does not exceed the total number of supported VFs.
> > 
> > Suggested-by: Danilo Krummrich <dakr@kernel.org>
> > Signed-off-by: Peter Colberg <pcolberg@redhat.com>
> > ---
> >  rust/kernel/pci.rs | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> > index 814990d386708fe2ac652ccaa674c10a6cf390cb..556a01ed9bc3b1300a3340a3d2383e08ceacbfe5 100644
> > --- a/rust/kernel/pci.rs
> > +++ b/rust/kernel/pci.rs
> > @@ -454,6 +454,36 @@ pub fn set_master(&self) {
> >          // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> >          unsafe { bindings::pci_set_master(self.as_raw()) };
> >      }
> > +
> > +    /// Enable the Single Root I/O Virtualization (SR-IOV) capability for this device,
> > +    /// where `nr_virtfn` is number of Virtual Functions (VF) to enable.
> > +    #[cfg(CONFIG_PCI_IOV)]
> > +    pub fn enable_sriov(&self, nr_virtfn: i32) -> Result {
> > +        // SAFETY:
> > +        // `self.as_raw` returns a valid pointer to a `struct pci_dev`.
> > +        //
> > +        // `pci_enable_sriov()` checks that the enable operation is valid:
> > +        // - the device is a Physical Function (PF),
> > +        // - SR-IOV is currently disabled, and
> > +        // - `nr_virtfn` does not exceed the total number of supported VFs.
> > +        let ret = unsafe { bindings::pci_enable_sriov(self.as_raw(), nr_virtfn) };
> > +        if ret != 0 {
> > +            return Err(crate::error::Error::from_errno(ret));
> > +        }
> > +        Ok(())
> > +    }
> > +
> > +    /// Disable the Single Root I/O Virtualization (SR-IOV) capability for this device.
> > +    #[cfg(CONFIG_PCI_IOV)]
> > +    pub fn disable_sriov(&self) {
> > +        // SAFETY:
> > +        // `self.as_raw` returns a valid pointer to a `struct pci_dev`.
> > +        //
> > +        // `pci_disable_sriov()` checks that the disable operation is valid:
> > +        // - the device is a Physical Function (PF), and
> > +        // - SR-IOV is currently enabled.
> > +        unsafe { bindings::pci_disable_sriov(self.as_raw()) };
> > +    }
> 
> Both these functions should only be called on bound devices - the
> safety statement should call it out, does the code require it?

Yes, these functions are in the Core device context that inherits from
the Bound device context, which guarantees that the PF device is bound
to a driver. I have added a note to the SAFETY comments.

> 
> Also per my other email SRIOV should be disabled before a driver can
> be unbound, this patch should take care of it to not introduce an
> dangerous enable_sriov().

Thanks for your review. This has been addressed in v2, which disables
SR-IOV before remove() when a PF driver opts in using a new flag
sriov_disable_on_remove, which is set by default for a Rust driver.

Further, enable_sriov() is prevented during remove() using a new
flag inhibit_enable in the pci_sriov structure that is set before
and cleared after the PF driver is unbound from the device.

Thanks,
Peter

> 
> Jason
>