Re: [PATCH v3 01/10] rust: pci: add internal SR-IOV enable and disable helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] rust: conditional compilation gap for SR-IOV API forcing downstream #cfg usage
- [Low] rust: missing #[inline] on abstraction functions forwarding to C bindings
--

commit 3f2a7b7fc60a2b23673b53641eba3ae3ba445ce8
Author: Peter Colberg <peter@colberg.org>

rust: pci: add internal SR-IOV enable and disable helpers

Wrap pci_enable_sriov() and pci_disable_sriov() on Device<CoreInternal>
for use by the SR-IOV callback tokens and their rollback guard.
Keep these operations internal so drivers can only enable or disable
VFs through the callback-scoped API introduced later in this series.

> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 3ec897709e890..b11a1d32c5db5 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -34,6 +34,8 @@
>  
>  mod id;
>  mod io;
> +#[cfg(CONFIG_PCI_IOV)]
> +mod iov;

[Severity: Medium]
Does hiding the `iov` module unconditionally when `CONFIG_PCI_IOV` is disabled
break downstream driver compilation?

By gating the `iov` module behind `#[cfg(CONFIG_PCI_IOV)]`, this cascades into
missing types (`SriovEnable`) and trait methods (`Driver::sriov_enable`). This
forces Rust driver developers to either pollute their code with
`#[cfg(CONFIG_PCI_IOV)]` on trait methods or add `depends on PCI_IOV` to their
Kconfig to avoid compilation failures.

In the C PCI subsystem, `.sriov_configure` and `pci_enable_sriov()` are
unconditionally available and fall back to static inlines returning `-ENODEV`
when disabled, allowing drivers to compile seamlessly without `#ifdef`s.
Could we provide `!CONFIG_PCI_IOV` helpers in `rust/helpers/pci.c` to maintain
C parity and avoid breaking valid Kconfig combinations?

>  mod irq;
>  
>  pub use self::id::{
> diff --git a/rust/kernel/pci/iov.rs b/rust/kernel/pci/iov.rs
> new file mode 100644
> index 0000000000000..c37ce51346731
> --- /dev/null
> +++ b/rust/kernel/pci/iov.rs
> @@ -0,0 +1,46 @@
[ ... ]
> +impl Device<device::CoreInternal<'_>> {
> +    /// Enable the Single Root I/O Virtualization (SR-IOV) capability for this device,
> +    /// where `nr_virtfn` is number of Virtual Functions (VF) to enable.
> +    #[expect(dead_code)]
> +    pub(crate) fn enable_sriov(&self, nr_virtfn: c_int) -> Result {

[Severity: Low]
This isn't a bug, but should this function have an `#[inline]` annotation?

The rust.md guidelines recommend that small functions or functions forwarding
to a C binding call in abstractions (like the `kernel` crate) be annotated
with `#[inline]`. `enable_sriov()` directly forwards to
`bindings::pci_enable_sriov`.

> +        // SAFETY:
[ ... ]
> +        to_result(unsafe { bindings::pci_enable_sriov(self.as_raw(), nr_virtfn) })
> +    }
> +
> +    /// Disable the Single Root I/O Virtualization (SR-IOV) capability for this device.
> +    #[expect(dead_code)]
> +    pub(crate) fn disable_sriov(&self) {

[Severity: Low]
This isn't a bug, but should this function also have an `#[inline]`
annotation?

Similar to `enable_sriov()`, `disable_sriov()` directly forwards to
`bindings::pci_disable_sriov` and lacks the `#[inline]` annotation
recommended for abstraction methods.

> +        // SAFETY:
[ ... ]
> +        unsafe { bindings::pci_disable_sriov(self.as_raw()) };
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1790705494.git.zhiw@nvidia.com?part=1