Re: [PATCH 7/9] gpu: nova-core: add vGPU preludes

["Alexandre Courbot" <acourbot@nvidia.com>]

On Thu Jun 4, 2026 at 8:43 PM JST, Zhi Wang wrote:
> The driver needs to detect vGPU capability before firmware loading so
> that the GSP boot flow can be adjusted accordingly.
>
> vGPU capability is determined by two sources: the PCI SR-IOV totalvfs
> count (whether the device advertises VFs) and the FSP PRC (Product
> Reconfiguration Control) knob (whether vGPU mode is actively enabled
> on this boot cycle). Both must agree for vGPU to proceed.
>
> Introduce VgpuManager to encapsulate vGPU state detection and tracking.
> On creation it queries totalvfs via sriov_get_totalvfs(); during GSP
> boot the PRC knob is read from FSP to refine the initial estimate.
> Extend GspBootContext with vgpu_requested and total_vfs fields to carry
> this state across the boot sequence. The FSP PRC read is performed
> inside the GH100 HAL boot method, where the FSP falcon is already
> available.
>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  drivers/gpu/nova-core/fsp.rs           |  1 -
>  drivers/gpu/nova-core/gpu.rs           | 24 +++++++++++--
>  drivers/gpu/nova-core/gsp.rs           |  5 +++
>  drivers/gpu/nova-core/gsp/hal/gh100.rs |  8 ++++-
>  drivers/gpu/nova-core/nova_core.rs     |  1 +
>  drivers/gpu/nova-core/vgpu.rs          | 47 ++++++++++++++++++++++++++
>  6 files changed, 82 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/nova-core/vgpu.rs
>
> diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
> index ce11efeba37e..c775e12c5451 100644
> --- a/drivers/gpu/nova-core/fsp.rs
> +++ b/drivers/gpu/nova-core/fsp.rs
> @@ -329,7 +329,6 @@ impl Fsp {
>      /// Queries FSP's Management Partition for the active vGPU mode knob value.
>      /// Returns [`VgpuMode::Enabled`] if vGPU support is active on this GPU,
>      /// [`VgpuMode::Disabled`] otherwise.
> -    #[expect(dead_code)]
>      pub(crate) fn read_vgpu_mode(
>          &mut self,
>          dev: &device::Device<device::Bound>,
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 69569e218d9b..6d8e60dd5292 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -9,7 +9,11 @@
>      io::Io,
>      num::Bounded,
>      pci,
> -    prelude::*, //
> +    prelude::*,
> +    sync::{
> +        new_mutex,
> +        Mutex, //
> +    },
>  };
>  
>  use crate::{
> @@ -27,6 +31,7 @@
>          GspBootContext, //
>      },
>      regs,
> +    vgpu::VgpuManager,
>  };
>  
>  mod hal;
> @@ -180,6 +185,13 @@ pub(crate) enum Architecture with TryFrom<Bounded<u32, 6>> {
>      }
>  }
>  
> +impl Architecture {
> +    /// Returns true for architectures that support vGPU (RTX PRO 6000 Blackwell Server Edition).
> +    pub(crate) const fn supports_vgpu(&self) -> bool {
> +        matches!(self, Self::BlackwellGB20x)
> +    }
> +}

Is this needed at all? It is only used in `VgpuManager::new` to
condition the call to `sriov_get_totalvfs`. But `sriov_get_totalvfs`
should return `0` if the device does not support virtual functions,
which is the value used on devices for which this method returns
`false`. So unless we can have virtual functions without vGPU support,
it seems like we can rely solely on the result of `sriov_get_totalvfs`.

Another potential gate is `read_vgpu_mode` - if this always returns
`false` on architectures that don't support vGPU, then we can also rely
on that instead of hardcoding supported archs in the driver.

In case we need to keep this method, it should use a HAL method (see
`pci_config_mirror_range` for an example). But I'd rather just remove it
if that's possible.

> +
>  #[derive(Clone, Copy)]
>  pub(crate) struct Revision {
>      major: Bounded<u8, 4>,
> @@ -278,9 +290,12 @@ pub(crate) struct Gpu<'gpu> {
>      gsp_falcon: Falcon<GspFalcon>,
>      /// SEC2 falcon instance, used for GSP boot up and cleanup.
>      sec2_falcon: Falcon<Sec2Falcon>,
> -    /// GSP runtime data. Temporarily an empty placeholder.
> +    /// GSP runtime data.
>      #[pin]
>      gsp: Gsp,
> +    /// vGPU state (SR-IOV / FSP PRC), behind Mutex for FFI concurrency.
> +    #[pin]
> +    vgpu: Mutex<VgpuManager>,

The `Mutex` should not be needed - afaict `vgpu` is not modified, and we
can get a mutable reference to it while we are contructing the GPU
anyway.

>      /// GSP unload firmware bundle, if any.
>      unload_bundle: Option<gsp::UnloadBundle>,
>  }
> @@ -319,18 +334,23 @@ pub(crate) fn new(
>  
>              sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset)?,
>  
> +            vgpu <- new_mutex!(VgpuManager::new(pdev, spec.chipset.arch())?, "vgpu_manager"),
> +
>              gsp <- Gsp::new(pdev),
>  
>              // This member must be initialized last, so the `UnloadBundle` can never be dropped from
>              // outside of the constructed `Gpu`, ensuring that the unload sequence is properly run
>              // in case of failure.
>              unload_bundle: {
> +                let mgr = vgpu.lock();
>                  let ctx = GspBootContext {
>                      pdev,
>                      bar,
>                      chipset: spec.chipset,
>                      gsp_falcon,
>                      sec2_falcon,
> +                    vgpu_requested: core::cell::Cell::new(mgr.vgpu_requested),
> +                    total_vfs: mgr.total_vfs,
>                  };
>                  gsp.boot(&ctx)?
>              },
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index f0901bf86258..94cd4a784b79 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -3,6 +3,8 @@
>  mod boot;
>  mod hal;
>  
> +use core::cell::Cell;
> +
>  use kernel::{
>      debugfs,
>      device,
> @@ -56,6 +58,9 @@ pub(crate) struct GspBootContext<'a> {
>      pub(crate) chipset: Chipset,
>      pub(crate) gsp_falcon: &'a Falcon<GspFalcon>,
>      pub(crate) sec2_falcon: &'a Falcon<Sec2Falcon>,
> +    pub(crate) vgpu_requested: Cell<bool>,

I think we can do without the `Cell`. Since we own the `GspBootContext`,
we can pass it by value to `Gsp::boot`, which can them modify it, and
pass mutable references to it to the other methods is calls. But it
would be even better if we didn't need to modify this at all.

The problem seems to come from the fact that we only build `Fsp` in the
GSP boot code. But vGPU would ideally query it outside of the boot
sequence. So a better design would probably be to make `Fsp` a member of
`Gpu` (behind an `Option`, as all GPUs don't have one) so we can call
`read_vgpu_mode` outside of the GSP boot sequence and get the definitive
enabled state of vGPU earlier in the GPU construction.

I think this is going to be needed anyway because patch 9 also wants to
pass the vGPU enabled status to build `FbLayout`, but can only do so
*before* we query the FSP about the PRC knob. So it doesn't necessarily
have the correct state.

Another benefit of having the `Fsp` moved to `Gpu` will be that you can
then pass a reference to it to the `VgpuManager::new` and have all the
vGPU stuff done in `vgpu.rs`.

Once we have that, we should also be able to add a reference to the (now
read-only) `VgpuManager` to `GspBootContext`, and get the parameters
directly from the source instead of duplicating them.

I can work on a patch to move `Fsp` if my suggestion is not clear, or
let me know if you prefer to do it yourself.

> +    #[expect(dead_code)]
> +    pub(crate) total_vfs: u16,
>  }
>  
>  impl<'a> GspBootContext<'a> {
> diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
> index c9fdc8cacedc..e133a92fb67f 100644
> --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
> +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
> @@ -24,7 +24,8 @@
>      },
>      fsp::{
>          FmcBootArgs,
> -        Fsp, //
> +        Fsp,
> +        VgpuMode, //
>      },
>      gsp::{
>          boot::BootUnloadGuard,
> @@ -174,6 +175,11 @@ fn boot<'a>(
>  
>          let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset, fsp_fw)?;
>  
> +        let vgpu_mode = fsp.read_vgpu_mode(dev, bar)?;
> +        dev_dbg!(dev, "vGPU mode: {:?}\n", vgpu_mode);
> +        ctx.vgpu_requested
> +            .set(ctx.vgpu_requested.get() && vgpu_mode == VgpuMode::Enabled);
> +
>          let args = FmcBootArgs::new(
>              dev,
>              chipset,
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index 9f0199f7b38c..6f55a9242027 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -26,6 +26,7 @@
>  mod regs;
>  mod sbuffer;
>  mod vbios;
> +mod vgpu;
>  
>  pub(crate) const MODULE_NAME: &core::ffi::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>  
> diff --git a/drivers/gpu/nova-core/vgpu.rs b/drivers/gpu/nova-core/vgpu.rs
> new file mode 100644
> index 000000000000..c8f1b74037c7
> --- /dev/null
> +++ b/drivers/gpu/nova-core/vgpu.rs
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> +    device,
> +    pci,
> +    prelude::*, //
> +};
> +
> +use crate::gpu::Architecture;
> +
> +/// vGPU manager.
> +///
> +/// On creation, performs platform detection to determine whether vGPU is
> +/// requested (PRC knob + totalvfs for Blackwell). The `vgpu_requested`
> +/// flag may be further refined during boot (e.g. FSP PRC knob read).
> +pub(crate) struct VgpuManager {
> +    pub(crate) vgpu_requested: bool,
> +    pub(crate) vgpu_enabled: bool,
> +    pub(crate) total_vfs: u16,
> +}
> +
> +impl VgpuManager {
> +    pub(crate) fn new(
> +        pdev: &pci::Device<device::Bound>,
> +        arch: Architecture,
> +    ) -> Result<VgpuManager> {
> +        let total_vfs: u16 = if arch.supports_vgpu() {
> +            pdev.sriov_get_totalvfs()
> +                .ok()
> +                .and_then(|n| n.try_into().ok())
> +                .unwrap_or(0)
> +        } else {
> +            0
> +        };
> +
> +        Ok(VgpuManager {
> +            vgpu_requested: total_vfs > 0,
> +            vgpu_enabled: false,
> +            total_vfs,
> +        })
> +    }
> +
> +    #[expect(dead_code)]
> +    pub(crate) fn set_vgpu_enabled(&mut self, enabled: bool) {
> +        self.vgpu_enabled = enabled;
> +    }

This method doesn't seem to be called anywhere in the series - let's
remove it if it is not needed.