gpu: nova-core: Enable booting GSP with vGPU enabled

[RFC 1/7] rust: pci: expose sriov_get_totalvfs() helper

Posted by Zhi Wang 1 week, 6 days ago

Add a wrapper for the `pci_sriov_get_totalvfs()` helper, allowing drivers
to query the number of total SR-IOV virtual functions a PCI device
supports.

This is useful for components that need to conditionally enable features
based on SR-IOV capability.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 rust/kernel/pci.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 7fcc5f6022c1..9a82e83dfd30 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -514,6 +514,18 @@ pub fn pci_class(&self) -> Class {
         // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
         Class::from_raw(unsafe { (*self.as_raw()).class })
     }
+
+    /// Returns total number of VFs, or `Err(ENODEV)` if none available.
+    pub fn sriov_get_totalvfs(&self) -> Result<i32> {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct pci_dev`.
+        let vfs = unsafe { bindings::pci_sriov_get_totalvfs(self.as_raw()) };
+
+        if vfs != 0 {
+            Ok(vfs)
+        } else {
+            Err(ENODEV)
+        }
+    }
 }
 
 impl Device<device::Bound> {
-- 
2.51.0

Re: [RFC 1/7] rust: pci: expose sriov_get_totalvfs() helper

Posted by Alexandre Courbot 1 week, 2 days ago

On Sat Dec 6, 2025 at 9:42 PM JST, Zhi Wang wrote:
> Add a wrapper for the `pci_sriov_get_totalvfs()` helper, allowing drivers
> to query the number of total SR-IOV virtual functions a PCI device
> supports.
>
> This is useful for components that need to conditionally enable features
> based on SR-IOV capability.
>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/kernel/pci.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 7fcc5f6022c1..9a82e83dfd30 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -514,6 +514,18 @@ pub fn pci_class(&self) -> Class {
>          // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>          Class::from_raw(unsafe { (*self.as_raw()).class })
>      }
> +
> +    /// Returns total number of VFs, or `Err(ENODEV)` if none available.
> +    pub fn sriov_get_totalvfs(&self) -> Result<i32> {
> +        // SAFETY: `self.as_raw()` is a valid pointer to a `struct pci_dev`.
> +        let vfs = unsafe { bindings::pci_sriov_get_totalvfs(self.as_raw()) };

This results in a build error if CONFIG_PCI_IOV is not set:

error[E0425]: cannot find function `pci_sriov_get_totalvfs` in crate `bindings`
   --> ../rust/kernel/pci.rs:521:38
    |
521 |         let vfs = unsafe { bindings::pci_sriov_get_totalvfs(self.as_raw()) };
    |                                      ^^^^^^^^^^^^^^^^^^^^^^ not found in `bindings`

error: aborting due to 1 previous error

Re: [RFC 1/7] rust: pci: expose sriov_get_totalvfs() helper

Posted by Alexandre Courbot 1 week, 3 days ago

On Sat Dec 6, 2025 at 9:42 PM JST, Zhi Wang wrote:
> Add a wrapper for the `pci_sriov_get_totalvfs()` helper, allowing drivers
> to query the number of total SR-IOV virtual functions a PCI device
> supports.
>
> This is useful for components that need to conditionally enable features
> based on SR-IOV capability.
>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/kernel/pci.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 7fcc5f6022c1..9a82e83dfd30 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -514,6 +514,18 @@ pub fn pci_class(&self) -> Class {
>          // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>          Class::from_raw(unsafe { (*self.as_raw()).class })
>      }
> +
> +    /// Returns total number of VFs, or `Err(ENODEV)` if none available.
> +    pub fn sriov_get_totalvfs(&self) -> Result<i32> {

The return type if `pci_sriov_get_totalvfs` is a bit unfortunate. It
returns `driver_max_VFs`, which is a `u16` [1], but not before
implicitly converting it to an `int` - even though it doesn't make use
of negative values to indicate errors!

Even its C callers eventually compare its returned value to a `u16` [2].

How about a cleanup patch to fix it to return `u16`, so we can make the
Rust wrapper return a `Result<NonZero<u16>>`?

[1] https://elixir.bootlin.com/linux/v6.18/source/drivers/pci/iov.c#L1281
[2] https://elixir.bootlin.com/linux/v6.18/source/drivers/pci/iov.c#L474

Re: [RFC 1/7] rust: pci: expose sriov_get_totalvfs() helper

Posted by Dirk Behme 1 week, 5 days ago

On 06.12.25 13:42, Zhi Wang wrote:
> Add a wrapper for the `pci_sriov_get_totalvfs()` helper, allowing drivers
> to query the number of total SR-IOV virtual functions a PCI device
> supports.
> 
> This is useful for components that need to conditionally enable features
> based on SR-IOV capability.
> 
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/kernel/pci.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 7fcc5f6022c1..9a82e83dfd30 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -514,6 +514,18 @@ pub fn pci_class(&self) -> Class {
>          // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>          Class::from_raw(unsafe { (*self.as_raw()).class })
>      }
> +
> +    /// Returns total number of VFs, or `Err(ENODEV)` if none available.
> +    pub fn sriov_get_totalvfs(&self) -> Result<i32> {
> +        // SAFETY: `self.as_raw()` is a valid pointer to a `struct pci_dev`.
> +        let vfs = unsafe { bindings::pci_sriov_get_totalvfs(self.as_raw()) };
> +
> +        if vfs != 0 {
> +            Ok(vfs)
> +        } else {
> +            Err(ENODEV)
> +        }

In the thread [1] there was some discussion about the `if {} else {}`
"style". From that discussion I "distilled" 6 options [2] which I
liked for having an overview :) Of course not all of these applied
there (const), neither will they here. And all have pros and cons. I
think in the end option #4 was selected.

What's about to do something similar here (and in the 2/7 patch as well)?

if vfs == 0 {
    return Err(ENODEV);
}

Ok(vfs)

Dirk

[1]
https://lore.kernel.org/rust-for-linux/CANiq72kiscT5euAUjcSzvxMzM9Hdj8aQGeUN_pVF-vHf3DhBuQ@mail.gmail.com/

[2] Options distilled from the thread [1]:

1.

if let Some(sum) = addr.checked_add(PAGE_SIZE - 1) {
    return Some(sum & PAGE_MASK);
}
None


2.

addr.checked_add(PAGE_SIZE - 1).map(|sum| sum & PAGE_MASK)


3.

if let Some(sum) = addr.checked_add(PAGE_SIZE - 1) {
   Some(sum & PAGE_MASK);
} else {
   None
}


4.

let Some(sum) = addr.checked_add(PAGE_SIZE - 1) else {
    return None;
};

Some(sum & PAGE_MASK)


5.

match addr.checked_add(PAGE_SIZE - 1) {
  Some(v) => Some(v & PAGE_MASK),
  None => None,
}

6.

Some(addr.checked_add(PAGE_SIZE - 1)? & PAGE_MASK)

Re: [RFC 1/7] rust: pci: expose sriov_get_totalvfs() helper

Posted by Zhi Wang 1 week, 2 days ago

On Sun, 7 Dec 2025 08:12:10 +0100
Dirk Behme <dirk.behme@gmail.com> wrote:

> On 06.12.25 13:42, Zhi Wang wrote:

snip

> In the thread [1] there was some discussion about the `if {} else {}`
> "style". From that discussion I "distilled" 6 options [2] which I
> liked for having an overview :) Of course not all of these applied
> there (const), neither will they here. And all have pros and cons. I
> think in the end option #4 was selected.
> 
> What's about to do something similar here (and in the 2/7 patch as
> well)?
> 
> if vfs == 0 {
>     return Err(ENODEV);
> }
> 
> Ok(vfs)
> 
> Dirk
>

Hey Dirk:

Thanks for the detailed summary! As a C programmer before, I tend to do
as above, because it improves a lot of readability on success path.
While playing with rust, I tend to use 5). Personally, I try to stay
away from if let chains, as some lifecycle changes happen in rust 2024
[1]. It wouldn't bite right now, but stay away from the bumps. :)

As Miguel's comment in another thread, I would improve this as
discussed.

[1]
https://doc.rust-lang.org/edition-guide/rust-2024/temporary-if-let-scope.html
 
> [1]
> https://lore.kernel.org/rust-for-linux/CANiq72kiscT5euAUjcSzvxMzM9Hdj8aQGeUN_pVF-vHf3DhBuQ@mail.gmail.com/
> 
> [2] Options distilled from the thread [1]:
> 
> 1.
> 
> if let Some(sum) = addr.checked_add(PAGE_SIZE - 1) {
>     return Some(sum & PAGE_MASK);
> }
> None
> 
> 
> 2.
> 
> addr.checked_add(PAGE_SIZE - 1).map(|sum| sum & PAGE_MASK)
> 
> 
> 3.
> 
> if let Some(sum) = addr.checked_add(PAGE_SIZE - 1) {
>    Some(sum & PAGE_MASK);
> } else {
>    None
> }
> 
> 
> 4.
> 
> let Some(sum) = addr.checked_add(PAGE_SIZE - 1) else {
>     return None;
> };
> 
> Some(sum & PAGE_MASK)
> 
> 
> 5.
> 
> match addr.checked_add(PAGE_SIZE - 1) {
>   Some(v) => Some(v & PAGE_MASK),
>   None => None,
> }
> 
> 6.
> 
> Some(addr.checked_add(PAGE_SIZE - 1)? & PAGE_MASK)
>

Re: [RFC 1/7] rust: pci: expose sriov_get_totalvfs() helper

Posted by Miguel Ojeda 1 week, 3 days ago

On Sun, Dec 7, 2025 at 9:52 AM Dirk Behme <dirk.behme@gmail.com> wrote:
>
> What's about to do something similar here (and in the 2/7 patch as well)?

In general, using early returns (especially for error and `None`
cases), i.e. keeping the happy path as the unindented/main one,
matches better the usual kernel style.

Whether that is with a simple `if` or `let ... else` etc. it depends
on the case, of course.

Cheers,
Miguel