[RFC 0/7] gpu: nova-core: Enable booting GSP with vGPU enabled

[RFC 0/7] gpu: nova-core: Enable booting GSP with vGPU enabled

[Zhi Wang]

The GSP boot support [1][2] has now been fully merged into drm-rust-next,
which puts us in a good position to start discussing the requirements for
bringing up GSP with vGPU enabled.

Booting up GSP with vGPU enabled is a key short-term milestone for upstream
vGPU support, allowing us to validate the basic GSP boot flow with vGPU
enabled even before the remaining nova-core dependencies are ready.

This RFC series builds on top of the latest drm-rust-next with kernel
module param support from [3]. A tree can be found [4].

[1] https://lore.kernel.org/all/20251114195552.739371-1-joelagnelf@nvidia.com/
[2] https://lore.kernel.org/all/20250827082015.959430-1-apopple@nvidia.com/
[3] https://lore.kernel.org/all/20250924-module-params-v3-v18-0-bf512c35d910@kernel.org/
[4] https://github.com/zhiwang-nvidia/nova-core/tree/zhi/rfc-boot-gsp-with-vgpu-enabled

Zhi Wang (7):
  rust: pci: expose sriov_get_totalvfs() helper
  [!UPSTREAM] rust: pci: support configuration space access
  gpu: nova-core: introduce vgpu_support module param.
  gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled
  gpu: nova-core: set RMSetSriovMode when NVIDIA vGPU is enabled
  gpu: nova-core: reserve a larger GSP WPR2 heap when vGPU is enabled.
  gpu: nova-core: load the scrubber ucode when vGPU support is enabled

 drivers/gpu/nova-core/fb.rs              | 19 ++++--
 drivers/gpu/nova-core/firmware.rs        |  1 +
 drivers/gpu/nova-core/firmware/booter.rs |  2 +
 drivers/gpu/nova-core/gpu.rs             |  6 +-
 drivers/gpu/nova-core/gsp.rs             |  8 ++-
 drivers/gpu/nova-core/gsp/boot.rs        | 38 +++++++++++-
 drivers/gpu/nova-core/gsp/commands.rs    | 31 ++++++----
 drivers/gpu/nova-core/gsp/fw.rs          | 75 ++++++++++++++++++++++++
 drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++-
 drivers/gpu/nova-core/nova_core.rs       | 15 +++++
 drivers/gpu/nova-core/regs.rs            | 11 ++++
 drivers/gpu/nova-core/vgpu.rs            | 26 ++++++++
 rust/kernel/pci.rs                       | 46 +++++++++++++++
 13 files changed, 267 insertions(+), 22 deletions(-)
 create mode 100644 drivers/gpu/nova-core/vgpu.rs

-- 
2.51.0

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

[Zhi Wang]

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

[RFC 2/7] [!UPSTREAM] rust: pci: support configuration space access

[Zhi Wang]

The work is WIP at [1].

Link: https://lore.kernel.org/all/20251119112117.116979-1-zhiw@nvidia.com/ [1]
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 rust/kernel/pci.rs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 9a82e83dfd30..e7fdded9d746 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -526,6 +526,40 @@ pub fn sriov_get_totalvfs(&self) -> Result<i32> {
             Err(ENODEV)
         }
     }
+
+    /// Find the extended capability
+    pub fn find_ext_capability(&self, cap: i32) -> Option<u16> {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct pci_dev`.
+        let offset = unsafe { bindings::pci_find_ext_capability(self.as_raw(), cap) };
+        if offset != 0 {
+            Some(offset as u16)
+        } else {
+            None
+        }
+    }
+
+    /// Read configuration space by word
+    pub fn config_read_word(&self, where_: i32) -> Result<u16, Error> {
+        let mut val: u16 = 0;
+
+        // SAFETY: `self.as_raw()` is a valid pointer to `struct pci_dev`,
+        // and `&mut val` is a valid pointer to writable memory.
+        to_result(unsafe {
+            bindings::pci_read_config_word(self.as_raw(), where_, &mut val)
+        })?;
+
+        Ok(val)
+    }
+
+    /// Read configuration space by dword
+    pub fn config_read_dword(&self, where_: i32) -> Result<u32, Error> {
+        let mut val: u32 = 0;
+        // SAFETY: `self.as_raw()` is a valid pointer to `struct pci_dev`,
+        // and `&mut val` is a valid pointer to writable memory.
+        to_result(unsafe { bindings::pci_read_config_dword(self.as_raw(), where_, &mut val) })?;
+
+        Ok(val)
+    }
 }
 
 impl Device<device::Bound> {
-- 
2.51.0

[RFC 3/7] gpu: nova-core: introduce vgpu_support module param.

[Zhi Wang]

Introduce a kernel module param to set vGPU support in nova-core.

vgpu_support = 1 (default): automatic

The driver automatically enables or disables vGPU support based on if the
GPU advertises SRIOV caps.

vgpu_support = 0: disabled

Explicitly disables vGPU support. The driver will not enable vGPU support
regardless.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/gpu/nova-core/gpu.rs       |  4 ++++
 drivers/gpu/nova-core/nova_core.rs | 15 +++++++++++++++
 drivers/gpu/nova-core/vgpu.rs      | 26 ++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 drivers/gpu/nova-core/vgpu.rs

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 629c9d2dc994..10c5ae07a891 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -20,6 +20,7 @@
     gfw,
     gsp::Gsp,
     regs,
+    vgpu::Vgpu, //
 };
 
 macro_rules! define_chipset {
@@ -252,6 +253,7 @@ pub(crate) struct Gpu {
     /// GSP runtime data. Temporarily an empty placeholder.
     #[pin]
     gsp: Gsp,
+    vgpu: Vgpu,
 }
 
 impl Gpu {
@@ -271,6 +273,8 @@ pub(crate) fn new<'a>(
                     .inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did not complete"))?;
             },
 
+            vgpu: Vgpu::new(pdev)?,
+
             sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,
 
             gsp_falcon: Falcon::new(
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b98a1c03f13d..e034353f69ab 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -18,6 +18,7 @@
 mod sbuffer;
 mod util;
 mod vbios;
+mod vgpu;
 
 pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 
@@ -28,6 +29,20 @@
     description: "Nova Core GPU driver",
     license: "GPL v2",
     firmware: [],
+    params: {
+        // vgpu_support = 1 (default): automatic
+        //
+        // The driver automatically enables or disables vGPU support based on if the GPU
+        // advertises SRIOV caps.
+        //
+        // vgpu_support = 0: disabled
+        //
+        // Explicitly disables vGPU support. The driver will not enable vGPU support regardless.
+        vgpu_support: u32 {
+            default: 1,
+            description: "Enable vGPU support - (1 = auto (default), 0 = disable)",
+        },
+    },
 }
 
 kernel::module_firmware!(firmware::ModInfoBuilder);
diff --git a/drivers/gpu/nova-core/vgpu.rs b/drivers/gpu/nova-core/vgpu.rs
new file mode 100644
index 000000000000..9701b97bf6bf
--- /dev/null
+++ b/drivers/gpu/nova-core/vgpu.rs
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+use kernel::{
+    device,
+    pci,
+    prelude::*, //
+};
+
+use crate::{
+    module_parameters, //
+};
+
+pub(crate) struct Vgpu {
+    pub vgpu_support: bool,
+}
+
+impl Vgpu {
+    pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<Vgpu> {
+        Ok(Vgpu {
+            vgpu_support: match *module_parameters::vgpu_support.value() {
+                0 => false,
+                _ => pdev.sriov_get_totalvfs().is_ok(),
+            },
+        })
+    }
+}
-- 
2.51.0

[RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

[Zhi Wang]

GSP firmware needs to know the VF BAR offsets to correctly calculate the
VF events.

The VF BAR information is stored in GSP_VF_INFO, which needs to be
initialized and uploaded together with the GSP_SYSTEM_INFO.

Populate GSP_VF_INFO when nova-core uploads the GSP_SYSTEM_INFO if NVIDIA
vGPU is enabled.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/gpu/nova-core/gpu.rs             |  2 +-
 drivers/gpu/nova-core/gsp.rs             |  8 ++-
 drivers/gpu/nova-core/gsp/boot.rs        |  6 +-
 drivers/gpu/nova-core/gsp/commands.rs    |  8 ++-
 drivers/gpu/nova-core/gsp/fw.rs          | 75 ++++++++++++++++++++++++
 drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++-
 6 files changed, 102 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 10c5ae07a891..08a41e7bd982 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -285,7 +285,7 @@ pub(crate) fn new<'a>(
 
             sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset)?,
 
-            gsp <- Gsp::new(pdev)?,
+            gsp <- Gsp::new(pdev, vgpu.vgpu_support)?,
 
             _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? },
 
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index fb6f74797178..2d9352740c28 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -115,11 +115,16 @@ pub(crate) struct Gsp {
     pub(crate) cmdq: Cmdq,
     /// RM arguments.
     rmargs: CoherentAllocation<GspArgumentsCached>,
+    /// Support vGPU.
+    vgpu_support: bool,
 }
 
 impl Gsp {
     // Creates an in-place initializer for a `Gsp` manager for `pdev`.
-    pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self, Error>> {
+    pub(crate) fn new(
+        pdev: &pci::Device<device::Bound>,
+        vgpu_support: bool,
+    ) -> Result<impl PinInit<Self, Error>> {
         let dev = pdev.as_ref();
         let libos = CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
             dev,
@@ -156,6 +161,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
             logrm,
             rmargs,
             cmdq,
+            vgpu_support,
         }))
     }
 }
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 54937606b5b0..5016c630cec3 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -33,6 +33,7 @@
     gpu::Chipset,
     gsp::{
         commands,
+        fw::GspVfInfo,
         sequencer::{
             GspSequencer,
             GspSequencerParams, //
@@ -136,6 +137,7 @@ pub(crate) fn boot(
         sec2_falcon: &Falcon<Sec2>,
     ) -> Result {
         let dev = pdev.as_ref();
+        let vgpu_support = self.vgpu_support;
 
         let bios = Vbios::new(dev, bar)?;
 
@@ -162,8 +164,10 @@ pub(crate) fn boot(
             CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
         dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
 
+        let vf_info = GspVfInfo::new(pdev, bar, vgpu_support)?;
+
         self.cmdq
-            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
+            .send_command(bar, commands::SetSystemInfo::new(pdev, vf_info))?;
         self.cmdq.send_command(bar, commands::SetRegistry::new())?;
 
         gsp_falcon.reset(bar)?;
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 0425c65b5d6f..1d519c4ed232 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -26,6 +26,7 @@
         },
         fw::{
             commands::*,
+            GspVfInfo,
             MsgFunction, //
         },
     },
@@ -36,12 +37,13 @@
 /// The `GspSetSystemInfo` command.
 pub(crate) struct SetSystemInfo<'a> {
     pdev: &'a pci::Device<device::Bound>,
+    vf_info: GspVfInfo,
 }
 
 impl<'a> SetSystemInfo<'a> {
     /// Creates a new `GspSetSystemInfo` command using the parameters of `pdev`.
-    pub(crate) fn new(pdev: &'a pci::Device<device::Bound>) -> Self {
-        Self { pdev }
+    pub(crate) fn new(pdev: &'a pci::Device<device::Bound>, vf_info: GspVfInfo) -> Self {
+        Self { pdev, vf_info }
     }
 }
 
@@ -51,7 +53,7 @@ impl<'a> CommandToGsp for SetSystemInfo<'a> {
     type InitError = Error;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
-        GspSetSystemInfo::init(self.pdev)
+        GspSetSystemInfo::init(self.pdev, self.vf_info)
     }
 }
 
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index abffd6beec65..a0581ac34586 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -9,8 +9,10 @@
 use core::ops::Range;
 
 use kernel::{
+    device,
     dma::CoherentAllocation,
     fmt,
+    pci,
     prelude::*,
     ptr::{
         Alignable,
@@ -27,6 +29,7 @@
 };
 
 use crate::{
+    driver::Bar0,
     fb::FbLayout,
     firmware::gsp::GspFirmware,
     gpu::Chipset,
@@ -926,3 +929,75 @@ fn new(cmdq: &Cmdq) -> Self {
         })
     }
 }
+
+/// VF information - gspVFInfo in SetSystemInfo.
+#[derive(Clone, Copy, Zeroable)]
+#[repr(transparent)]
+pub(crate) struct GspVfInfo {
+    inner: bindings::GSP_VF_INFO,
+}
+
+impl GspVfInfo {
+    /// Creates a new GspVfInfo structure.
+    pub(crate) fn new<'a>(
+        pdev: &'a pci::Device<device::Bound>,
+        bar: &Bar0,
+        vgpu_support: bool,
+    ) -> Result<GspVfInfo> {
+        let mut vf_info = GspVfInfo::zeroed();
+        let info = &mut vf_info.inner;
+
+        if vgpu_support {
+            let val = pdev.sriov_get_totalvfs()?;
+            info.totalVFs = u32::try_from(val)?;
+
+            let pos = pdev
+                .find_ext_capability(kernel::bindings::PCI_EXT_CAP_ID_SRIOV as i32)
+                .ok_or(ENODEV)?;
+
+            let val = pdev.config_read_word(
+                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_VF_OFFSET as i32),
+            )?;
+            info.firstVFOffset = u32::from(val);
+
+            let val = pdev.config_read_dword(
+                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32),
+            )?;
+            info.FirstVFBar0Address = u64::from(val);
+
+            let bar1_lo = pdev.config_read_dword(
+                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 4),
+            )?;
+            let bar1_hi = pdev.config_read_dword(
+                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 8),
+            )?;
+
+            let addr_mask = u64::try_from(kernel::bindings::PCI_BASE_ADDRESS_MEM_MASK)?;
+
+            info.FirstVFBar1Address =
+                (u64::from(bar1_hi) << 32) | ((u64::from(bar1_lo)) & addr_mask);
+
+            let bar2_lo = pdev.config_read_dword(
+                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 12),
+            )?;
+            let bar2_hi = pdev.config_read_dword(
+                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 16),
+            )?;
+
+            info.FirstVFBar2Address = (u64::from(bar2_hi) << 32) | (u64::from(bar2_lo) & addr_mask);
+
+            let val = bar.read32(0x88000 + 0xbf4);
+            info.b64bitBar1 = u8::from((val & 0x00000006) == 0x00000004);
+
+            let val = bar.read32(0x88000 + 0xbfc);
+            info.b64bitBar2 = u8::from((val & 0x00000006) == 0x00000004);
+        }
+        Ok(vf_info)
+    }
+}
+
+// SAFETY: Padding is explicit and does not contain uninitialized data.
+unsafe impl AsBytes for GspVfInfo {}
+
+// SAFETY: This struct only contains integer types for which all bit patterns are valid.
+unsafe impl FromBytes for GspVfInfo {}
diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
index 21be44199693..3b5c05704b2d 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -4,7 +4,10 @@
 use kernel::transmute::{AsBytes, FromBytes};
 use kernel::{device, pci};
 
-use crate::gsp::GSP_PAGE_SIZE;
+use crate::gsp::{
+    fw::GspVfInfo,
+    GSP_PAGE_SIZE, //
+};
 
 use super::bindings;
 
@@ -18,7 +21,10 @@ pub(crate) struct GspSetSystemInfo {
 impl GspSetSystemInfo {
     /// Returns an in-place initializer for the `GspSetSystemInfo` command.
     #[allow(non_snake_case)]
-    pub(crate) fn init<'a>(dev: &'a pci::Device<device::Bound>) -> impl Init<Self, Error> + 'a {
+    pub(crate) fn init<'a>(
+        dev: &'a pci::Device<device::Bound>,
+        info: GspVfInfo,
+    ) -> impl Init<Self, Error> + 'a {
         type InnerGspSystemInfo = bindings::GspSystemInfo;
         let init_inner = try_init!(InnerGspSystemInfo {
             gpuPhysAddr: dev.resource_start(0)?,
@@ -38,6 +44,7 @@ pub(crate) fn init<'a>(dev: &'a pci::Device<device::Bound>) -> impl Init<Self, E
             PCIRevisionID: u32::from(dev.revision_id()),
             bIsPrimary: 0,
             bPreserveVideoMemoryAllocations: 0,
+            gspVFInfo: info.inner,
             ..Zeroable::init_zeroed()
         });
 
-- 
2.51.0

[RFC 5/7] gpu: nova-core: set RMSetSriovMode when NVIDIA vGPU is enabled

[Zhi Wang]

The registry object "RMSetSriovMode" is required to be set when vGPU is
enabled.

Set "RMSetSriovMode" to 1 when nova-core is loading the GSP firmware and
initialize the GSP registry objects, if vGPU is enabled.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs     |  3 ++-
 drivers/gpu/nova-core/gsp/commands.rs | 23 +++++++++++++++--------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 5016c630cec3..847ce550eccf 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -168,7 +168,8 @@ pub(crate) fn boot(
 
         self.cmdq
             .send_command(bar, commands::SetSystemInfo::new(pdev, vf_info))?;
-        self.cmdq.send_command(bar, commands::SetRegistry::new())?;
+        self.cmdq
+            .send_command(bar, commands::SetRegistry::new(vgpu_support))?;
 
         gsp_falcon.reset(bar)?;
         let libos_handle = self.libos.dma_handle();
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 1d519c4ed232..00ba48a25444 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -64,16 +64,18 @@ struct RegistryEntry {
 
 /// The `SetRegistry` command.
 pub(crate) struct SetRegistry {
-    entries: [RegistryEntry; Self::NUM_ENTRIES],
+    entries: [RegistryEntry; Self::MAX_NUM_ENTRIES],
+    num_entries: usize,
 }
 
 impl SetRegistry {
     // For now we hard-code the registry entries. Future work will allow others to
     // be added as module parameters.
-    const NUM_ENTRIES: usize = 3;
+    const MAX_NUM_ENTRIES: usize = 4;
 
     /// Creates a new `SetRegistry` command, using a set of hardcoded entries.
-    pub(crate) fn new() -> Self {
+    pub(crate) fn new(vgpu_support: bool) -> Self {
+        let num_entries = if vgpu_support { 4 } else { 3 };
         Self {
             entries: [
                 // RMSecBusResetEnable - enables PCI secondary bus reset
@@ -93,7 +95,12 @@ pub(crate) fn new() -> Self {
                     key: "RMDevidCheckIgnore",
                     value: 1,
                 },
+                RegistryEntry {
+                    key: "RMSetSriovMode",
+                    value: 1,
+                },
             ],
+            num_entries,
         }
     }
 }
@@ -104,15 +111,15 @@ impl CommandToGsp for SetRegistry {
     type InitError = Infallible;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
-        PackedRegistryTable::init(Self::NUM_ENTRIES as u32, self.variable_payload_len() as u32)
+        PackedRegistryTable::init(self.num_entries as u32, self.variable_payload_len() as u32)
     }
 
     fn variable_payload_len(&self) -> usize {
         let mut key_size = 0;
-        for i in 0..Self::NUM_ENTRIES {
+        for i in 0..self.num_entries {
             key_size += self.entries[i].key.len() + 1; // +1 for NULL terminator
         }
-        Self::NUM_ENTRIES * size_of::<PackedRegistryEntry>() + key_size
+        self.num_entries * size_of::<PackedRegistryEntry>() + key_size
     }
 
     fn init_variable_payload(
@@ -120,12 +127,12 @@ fn init_variable_payload(
         dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
     ) -> Result {
         let string_data_start_offset =
-            size_of::<PackedRegistryTable>() + Self::NUM_ENTRIES * size_of::<PackedRegistryEntry>();
+            size_of::<PackedRegistryTable>() + self.num_entries * size_of::<PackedRegistryEntry>();
 
         // Array for string data.
         let mut string_data = KVec::new();
 
-        for entry in self.entries.iter().take(Self::NUM_ENTRIES) {
+        for entry in self.entries.iter().take(self.num_entries) {
             dst.write_all(
                 PackedRegistryEntry::new(
                     (string_data_start_offset + string_data.len()) as u32,
-- 
2.51.0

[RFC 6/7] gpu: nova-core: reserve a larger GSP WPR2 heap when vGPU is enabled.

[Zhi Wang]

To support the maximum vGPUs on devices that support vGPU, a larger
WPR2 heap size is required. On Ada with vGPU supported, the size should
be set to at least 581MB.

When vGPU support is enabled, reserve a large WPR2 heap size up to
581MB, set the max supported VF to max in WPR2 meta.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs       | 19 +++++++++++++++----
 drivers/gpu/nova-core/gsp/boot.rs |  2 +-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 3c9cf151786c..9a5c40029f3a 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -119,7 +119,12 @@ pub(crate) struct FbLayout {
 
 impl FbLayout {
     /// Computes the FB layout for `chipset` required to run the `gsp_fw` GSP firmware.
-    pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<Self> {
+    pub(crate) fn new(
+        chipset: Chipset,
+        bar: &Bar0,
+        gsp_fw: &GspFirmware,
+        vgpu_support: bool,
+    ) -> Result<Self> {
         let hal = hal::fb_hal(chipset);
 
         let fb = {
@@ -181,8 +186,12 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
 
         let wpr2_heap = {
             const WPR2_HEAP_DOWN_ALIGN: Alignment = Alignment::new::<SZ_1M>();
-            let wpr2_heap_size =
-                gsp::LibosParams::from_chipset(chipset).wpr_heap_size(chipset, fb.end);
+            let wpr2_heap_size = if !vgpu_support {
+                gsp::LibosParams::from_chipset(chipset).wpr_heap_size(chipset, fb.end)
+            } else {
+                581 * usize_as_u64(SZ_1M)
+            };
+
             let wpr2_heap_addr = (elf.start - wpr2_heap_size).align_down(WPR2_HEAP_DOWN_ALIGN);
 
             wpr2_heap_addr..(elf.start).align_down(WPR2_HEAP_DOWN_ALIGN)
@@ -202,6 +211,8 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
             wpr2.start - HEAP_SIZE..wpr2.start
         };
 
+        let vf_partition_count = if vgpu_support { 32 } else { 0 };
+
         Ok(Self {
             fb,
             vga_workspace,
@@ -211,7 +222,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
             wpr2_heap,
             wpr2,
             heap,
-            vf_partition_count: 0,
+            vf_partition_count,
         })
     }
 }
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 847ce550eccf..ec006c26f19f 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -146,7 +146,7 @@ pub(crate) fn boot(
             GFP_KERNEL,
         )?;
 
-        let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
+        let fb_layout = FbLayout::new(chipset, bar, &gsp_fw, vgpu_support)?;
         dev_dbg!(dev, "{:#x?}\n", fb_layout);
 
         Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;
-- 
2.51.0

[RFC 7/7] gpu: nova-core: load the scrubber ucode when vGPU support is enabled

[Zhi Wang]

To support the maximum vGPUs on the device that support vGPU, a larger
WPR2 heap size is required. By setting the WPR2 heap size larger than 256MB
the scrubber ucode image is required to scrub the FB memory before any
other ucode image is executed.

If not, the GSP firmware hangs when booting.

When vGPU support is enabled, execute the scrubber ucode image to scrub the
FB memory before executing any other ucode images.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs        |  1 +
 drivers/gpu/nova-core/firmware/booter.rs |  2 ++
 drivers/gpu/nova-core/gsp/boot.rs        | 27 ++++++++++++++++++++++++
 drivers/gpu/nova-core/regs.rs            | 11 ++++++++++
 4 files changed, 41 insertions(+)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 2d2008b33fb4..5ae1ab262d57 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -226,6 +226,7 @@ const fn make_entry_chipset(self, chipset: &str) -> Self {
             .make_entry_file(chipset, "booter_unload")
             .make_entry_file(chipset, "bootloader")
             .make_entry_file(chipset, "gsp")
+            .make_entry_file(chipset, "scrubber")
     }
 
     pub(crate) const fn create(
diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index f107f753214a..f622c9b960de 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -269,6 +269,7 @@ fn new_booter(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self>
 
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub(crate) enum BooterKind {
+    Scrubber,
     Loader,
     #[expect(unused)]
     Unloader,
@@ -286,6 +287,7 @@ pub(crate) fn new(
         bar: &Bar0,
     ) -> Result<Self> {
         let fw_name = match kind {
+            BooterKind::Scrubber => "scrubber",
             BooterKind::Loader => "booter_load",
             BooterKind::Unloader => "booter_unload",
         };
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index ec006c26f19f..8ef79433f017 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -151,6 +151,33 @@ pub(crate) fn boot(
 
         Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;
 
+        if vgpu_support {
+            let scrubber = BooterFirmware::new(
+                dev,
+                BooterKind::Scrubber,
+                chipset,
+                FIRMWARE_VERSION,
+                sec2_falcon,
+                bar,
+            )?;
+
+            sec2_falcon.reset(bar)?;
+            sec2_falcon.dma_load(bar, &scrubber)?;
+
+            let (mbox0, mbox1) = sec2_falcon.boot(bar, None, None)?;
+
+            dev_dbg!(
+                pdev.as_ref(),
+                "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n",
+                mbox0,
+                mbox1
+            );
+
+            if !regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed() {
+                return Err(ETIMEDOUT);
+            }
+        }
+
         let booter_loader = BooterFirmware::new(
             dev,
             BooterKind::Loader,
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 82cc6c0790e5..9f3a52ca014f 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -173,6 +173,17 @@ pub(crate) fn higher_bound(self) -> u64 {
     26:26   boot_stage_3_handoff as bool;
 });
 
+register!(NV_PGC6_BSI_SECURE_SCRATCH_15 @ 0x001180fc {
+    31:29   scrubber_handoff as u8;
+});
+
+impl NV_PGC6_BSI_SECURE_SCRATCH_15 {
+    /// Returns `true` if scrubber is completed.
+    pub(crate) fn scrubber_completed(self) -> bool {
+        self.scrubber_handoff() >= 0x3
+    }
+}
+
 // Privilege level mask register. It dictates whether the host CPU has privilege to access the
 // `PGC6_AON_SECURE_SCRATCH_GROUP_05` register (which it needs to read GFW_BOOT).
 register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK @ 0x00118128,
-- 
2.51.0

Re: [RFC 7/7] gpu: nova-core: load the scrubber ucode when vGPU support is enabled

[Joel Fernandes]

Hi Zhi,

On 12/6/2025 7:42 AM, Zhi Wang wrote:
> To support the maximum vGPUs on the device that support vGPU, a larger
> WPR2 heap size is required. By setting the WPR2 heap size larger than 256MB
> the scrubber ucode image is required to scrub the FB memory before any
> other ucode image is executed.
> 
> If not, the GSP firmware hangs when booting.
> 
> When vGPU support is enabled, execute the scrubber ucode image to scrub the
> FB memory before executing any other ucode images.
> 
[..]
>      pub(crate) const fn create(
> diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
> index f107f753214a..f622c9b960de 100644
> --- a/drivers/gpu/nova-core/firmware/booter.rs
> +++ b/drivers/gpu/nova-core/firmware/booter.rs
> @@ -269,6 +269,7 @@ fn new_booter(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self>
>  
>  #[derive(Copy, Clone, Debug, PartialEq)]
>  pub(crate) enum BooterKind {
> +    Scrubber,
>      Loader,
>      #[expect(unused)]
>      Unloader,
> @@ -286,6 +287,7 @@ pub(crate) fn new(
>          bar: &Bar0,
>      ) -> Result<Self> {
>          let fw_name = match kind {
> +            BooterKind::Scrubber => "scrubber",
>              BooterKind::Loader => "booter_load",
>              BooterKind::Unloader => "booter_unload",
>          };
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index ec006c26f19f..8ef79433f017 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -151,6 +151,33 @@ pub(crate) fn boot(
>  
>          Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;

Could you elaborate on how the timeout below works? See comment below.

>  
> +        if vgpu_support {
> +            let scrubber = BooterFirmware::new(
> +                dev,
> +                BooterKind::Scrubber,
> +                chipset,
> +                FIRMWARE_VERSION,
> +                sec2_falcon,
> +                bar,
> +            )?;
> +
> +            sec2_falcon.reset(bar)?;
> +            sec2_falcon.dma_load(bar, &scrubber)?;
> +
> +            let (mbox0, mbox1) = sec2_falcon.boot(bar, None, None)?;

boot() already returns -ETIMEDOUT via wait_till_halted()->read_poll_timeout().

The wait there is 2 seconds. I assume the scrubber would have completed by then.

> +
> +            dev_dbg!(
> +                pdev.as_ref(),
> +                "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n",
> +                mbox0,
> +                mbox1
> +            );
> +
> +            if !regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed() {
> +                return Err(ETIMEDOUT);

So under which situation do you get to this point (!scrubber_completed) ?
Basically I am not sure if ETIMEDOUT is the right error to return here, because
boot() already returns ETIMEDOUT by waiting for the halt.

If you still want return ETIMEDOUT here, then it sounds like you're waiting for
scrubbing beyond the waiting already done by boot(). If so, then shouldn't you
need to use read_poll_timeout() here?

perhaps something like:

 read_poll_timeout(
     || Ok(regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed()),
     |val: &bool| *val,
     Delta::from_millis(10),
     Delta::from_secs(5),
 )?;

Thanks.

Re: [RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

[Joel Fernandes]

Hi Zhi,

On 12/6/2025 7:42 AM, Zhi Wang wrote:
[...]
> +/// VF information - gspVFInfo in SetSystemInfo.
> +#[derive(Clone, Copy, Zeroable)]
> +#[repr(transparent)]
> +pub(crate) struct GspVfInfo {
> +    inner: bindings::GSP_VF_INFO,
> +}
> +
> +impl GspVfInfo {
> +    /// Creates a new GspVfInfo structure.
> +    pub(crate) fn new<'a>(
> +        pdev: &'a pci::Device<device::Bound>,
> +        bar: &Bar0,
> +        vgpu_support: bool,
> +    ) -> Result<GspVfInfo> {
> +        let mut vf_info = GspVfInfo::zeroed();
> +        let info = &mut vf_info.inner;
> +
> +        if vgpu_support {
> +            let val = pdev.sriov_get_totalvfs()?;
> +            info.totalVFs = u32::try_from(val)?;
> +
> +            let pos = pdev
> +                .find_ext_capability(kernel::bindings::PCI_EXT_CAP_ID_SRIOV as i32)
> +                .ok_or(ENODEV)?;
> +
> +            let val = pdev.config_read_word(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_VF_OFFSET as i32),
> +            )?;
> +            info.firstVFOffset = u32::from(val);
> +
> +            let val = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32),
> +            )?;
> +            info.FirstVFBar0Address = u64::from(val);
> +
> +            let bar1_lo = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 4),
> +            )?;
> +            let bar1_hi = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 8),
> +            )?;
> +
> +            let addr_mask = u64::try_from(kernel::bindings::PCI_BASE_ADDRESS_MEM_MASK)?;
> +
> +            info.FirstVFBar1Address =
> +                (u64::from(bar1_hi) << 32) | ((u64::from(bar1_lo)) & addr_mask);
> +
> +            let bar2_lo = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 12),
> +            )?;
> +            let bar2_hi = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 16),
> +            )?;
> +
> +            info.FirstVFBar2Address = (u64::from(bar2_hi) << 32) | (u64::from(bar2_lo) & addr_mask);
> +
> +            let val = bar.read32(0x88000 + 0xbf4);
> +            info.b64bitBar1 = u8::from((val & 0x00000006) == 0x00000004);
> +
> +            let val = bar.read32(0x88000 + 0xbfc);
> +            info.b64bitBar2 = u8::from((val & 0x00000006) == 0x00000004);

Please no magic numbers, please use proper named constants with documentation
comments explaining the values.

Also BAR reads here need proper register macro definitions/access.

Also the above code is lacking in comments. All the steps above need proper
comments IMO.

General philosophy of Nova is it is a well documented, cleanly written driver
with minimal/no magic numbers and abundant comments. :)

Thanks.

Re: [RFC 7/7] gpu: nova-core: load the scrubber ucode when vGPU support is enabled

[Dirk Behme]

On 06.12.25 13:42, Zhi Wang wrote:
> To support the maximum vGPUs on the device that support vGPU, a larger
> WPR2 heap size is required. By setting the WPR2 heap size larger than 256MB
> the scrubber ucode image is required to scrub the FB memory before any
> other ucode image is executed.
> 
> If not, the GSP firmware hangs when booting.
> 
> When vGPU support is enabled, execute the scrubber ucode image to scrub the
> FB memory before executing any other ucode images.
> 
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware.rs        |  1 +
>  drivers/gpu/nova-core/firmware/booter.rs |  2 ++
>  drivers/gpu/nova-core/gsp/boot.rs        | 27 ++++++++++++++++++++++++
>  drivers/gpu/nova-core/regs.rs            | 11 ++++++++++
>  4 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index 2d2008b33fb4..5ae1ab262d57 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -226,6 +226,7 @@ const fn make_entry_chipset(self, chipset: &str) -> Self {
>              .make_entry_file(chipset, "booter_unload")
>              .make_entry_file(chipset, "bootloader")
>              .make_entry_file(chipset, "gsp")
> +            .make_entry_file(chipset, "scrubber")
>      }
>  
>      pub(crate) const fn create(
> diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
> index f107f753214a..f622c9b960de 100644
> --- a/drivers/gpu/nova-core/firmware/booter.rs
> +++ b/drivers/gpu/nova-core/firmware/booter.rs
> @@ -269,6 +269,7 @@ fn new_booter(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self>
>  
>  #[derive(Copy, Clone, Debug, PartialEq)]
>  pub(crate) enum BooterKind {
> +    Scrubber,
>      Loader,
>      #[expect(unused)]
>      Unloader,
> @@ -286,6 +287,7 @@ pub(crate) fn new(
>          bar: &Bar0,
>      ) -> Result<Self> {
>          let fw_name = match kind {
> +            BooterKind::Scrubber => "scrubber",
>              BooterKind::Loader => "booter_load",
>              BooterKind::Unloader => "booter_unload",
>          };
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index ec006c26f19f..8ef79433f017 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -151,6 +151,33 @@ pub(crate) fn boot(
>  
>          Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;
>  
> +        if vgpu_support {
> +            let scrubber = BooterFirmware::new(
> +                dev,
> +                BooterKind::Scrubber,
> +                chipset,
> +                FIRMWARE_VERSION,
> +                sec2_falcon,
> +                bar,
> +            )?;
> +
> +            sec2_falcon.reset(bar)?;
> +            sec2_falcon.dma_load(bar, &scrubber)?;
> +
> +            let (mbox0, mbox1) = sec2_falcon.boot(bar, None, None)?;
> +
> +            dev_dbg!(
> +                pdev.as_ref(),


I think you can use `dev` here?

Dirk


> +                "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n",
> +                mbox0,
> +                mbox1
> +            );
> +
> +            if !regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed() {
> +                return Err(ETIMEDOUT);
> +            }
> +        }
> +
>          let booter_loader = BooterFirmware::new(
>              dev,
>              BooterKind::Loader,
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 82cc6c0790e5..9f3a52ca014f 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -173,6 +173,17 @@ pub(crate) fn higher_bound(self) -> u64 {
>      26:26   boot_stage_3_handoff as bool;
>  });
>  
> +register!(NV_PGC6_BSI_SECURE_SCRATCH_15 @ 0x001180fc {
> +    31:29   scrubber_handoff as u8;
> +});
> +
> +impl NV_PGC6_BSI_SECURE_SCRATCH_15 {
> +    /// Returns `true` if scrubber is completed.
> +    pub(crate) fn scrubber_completed(self) -> bool {
> +        self.scrubber_handoff() >= 0x3
> +    }
> +}
> +
>  // Privilege level mask register. It dictates whether the host CPU has privilege to access the
>  // `PGC6_AON_SECURE_SCRATCH_GROUP_05` register (which it needs to read GFW_BOOT).
>  register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK @ 0x00118128,

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

[Dirk Behme]

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 5/7] gpu: nova-core: set RMSetSriovMode when NVIDIA vGPU is enabled

[Timur Tabi]

On Sat, 2025-12-06 at 12:42 +0000, Zhi Wang wrote:
> -    pub(crate) fn new() -> Self {
> +    pub(crate) fn new(vgpu_support: bool) -> Self {
> +        let num_entries = if vgpu_support { 4 } else { 3 };

Instead of passing a bool, and then hard-coding the length based on that bool (which would
require that RMSetSriovMode always be the last entry in the array), you need to do what Nouveau
does: if VGPU is enabled, then dynamically append the entry to the array.

Re: [RFC 5/7] gpu: nova-core: set RMSetSriovMode when NVIDIA vGPU is enabled

[Joel Fernandes]

> On Dec 7, 2025, at 10:55 AM, Timur Tabi <ttabi@nvidia.com> wrote:
> 
>> On Sat, 2025-12-06 at 12:42 +0000, Zhi Wang wrote:
>> -    pub(crate) fn new() -> Self {
>> +    pub(crate) fn new(vgpu_support: bool) -> Self {
>> +        let num_entries = if vgpu_support { 4 } else { 3 };
> 
> Instead of passing a bool, and then hard-coding the length based on that bool (which would
> require that RMSetSriovMode always be the last entry in the array), you need to do what Nouveau
> does: if VGPU is enabled, then dynamically append the entry to the array.

Yeah, I agree with Timur. 

Thanks.

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

[Miguel Ojeda]

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

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

[Alexandre Courbot]

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 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

[Zhi Wang]

On Sat, 6 Dec 2025 21:32:51 -0500
Joel Fernandes <joelagnelf@nvidia.com> wrote:

> Hi Zhi,
> 
> On 12/6/2025 7:42 AM, Zhi Wang wrote:

snip

 ==
> > 0x00000004); +
> > +            let val = bar.read32(0x88000 + 0xbfc);
> > +            info.b64bitBar2 = u8::from((val & 0x00000006) ==
> > 0x00000004);
> 
> Please no magic numbers, please use proper named constants with
> documentation comments explaining the values.
> 
> Also BAR reads here need proper register macro definitions/access.
> 

That is true. :) But this is because there is no register definition in
the OpenRM code/non OpenRM code as well. I have no idea about the name
and bit definitions of this register.

Suppose I will have to find some clues from some folks then document
them here when going to patches request for merged. :)

> Also the above code is lacking in comments. All the steps above need
> proper comments IMO.
> 
> General philosophy of Nova is it is a well documented, cleanly
> written driver with minimal/no magic numbers and abundant comments. :)
> 

Agree. :)

> Thanks.
> 

Re: [RFC 7/7] gpu: nova-core: load the scrubber ucode when vGPU support is enabled

[Zhi Wang]

On Sat, 6 Dec 2025 21:26:12 -0500
Joel Fernandes <joelagnelf@nvidia.com> wrote:

> Hi Zhi,
> 
> On 12/6/2025 7:42 AM, Zhi Wang wrote:

snip

> 
> boot() already returns -ETIMEDOUT via
> wait_till_halted()->read_poll_timeout().
> 
> The wait there is 2 seconds. I assume the scrubber would have
> completed by then.
> 1
> > +
> > +            dev_dbg!(
> > +                pdev.as_ref(),
> > +                "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n",
> > +                mbox0,
> > +                mbox1
> > +            );
> > +
> > +            if
> > !regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed()
> > {
> > +                return Err(ETIMEDOUT);  
> 
> So under which situation do you get to this point
> (!scrubber_completed) ? Basically I am not sure if ETIMEDOUT is the
> right error to return here, because boot() already returns ETIMEDOUT
> by waiting for the halt.
>
> If you still want return ETIMEDOUT here, then it sounds like you're
> waiting for scrubbing beyond the waiting already done by boot(). If
> so, then shouldn't you need to use read_poll_timeout() here?
> 
> perhaps something like:
> 
>  read_poll_timeout(
>      ||
> Ok(regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed()),
> |val: &bool| *val, Delta::from_millis(10),
>      Delta::from_secs(5),
>  )?;
> 

This is the identical implementation to OpenRM [1]. According to that
parts of code, I think the scrubber runs in the binary booting process.
When it signals the firmware booting successfully, the scrubbing should
be done. Let me change to another errno.

[1]https://github.com/NVIDIA/open-gpu-kernel-modules/blob/a5bfb10e75a4046c5d991c65f49b5d29151e68cf/src/nvidia/src/kernel/gpu/gsp/arch/ada/kernel_gsp_ad102.c#L49
> Thanks.
> 

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

[Zhi Wang]

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 5/7] gpu: nova-core: set RMSetSriovMode when NVIDIA vGPU is enabled

[Zhi Wang]

On Sun, 7 Dec 2025 16:57:01 +0000
Joel Fernandes <joelagnelf@nvidia.com> wrote:

> 
> 
> > On Dec 7, 2025, at 10:55 AM, Timur Tabi <ttabi@nvidia.com> wrote:
> > 
> >> On Sat, 2025-12-06 at 12:42 +0000, Zhi Wang wrote:
> >> -    pub(crate) fn new() -> Self {
> >> +    pub(crate) fn new(vgpu_support: bool) -> Self {
> >> +        let num_entries = if vgpu_support { 4 } else { 3 };
> > 
> > Instead of passing a bool, and then hard-coding the length based on
> > that bool (which would require that RMSetSriovMode always be the
> > last entry in the array), you need to do what Nouveau does: if VGPU
> > is enabled, then dynamically append the entry to the array.
> 
> Yeah, I agree with Timur. 
> 

Hey Timur and Joe:

Let me see how this could be solved dynamically. Probably need more
changes on other items as well.

Apart from this, I felt that we might need a struct GspBootConfig to
pass around the GSP booting path, while writing these patches. As we
already had coming items, like reserved memory size when vGPU is
enabled or not, vGPU enabled switch, also reserved memory size on
Hopper/Blackwell in John's patch.

It seems we need a central object to host these tuning for GSP booting
up. 

Z.

> Thanks.
> 
> 

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

[Alexandre Courbot]

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 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

[Alexandre Courbot]

On Sat Dec 6, 2025 at 9:42 PM JST, Zhi Wang wrote:
> GSP firmware needs to know the VF BAR offsets to correctly calculate the
> VF events.
>
> The VF BAR information is stored in GSP_VF_INFO, which needs to be
> initialized and uploaded together with the GSP_SYSTEM_INFO.
>
> Populate GSP_VF_INFO when nova-core uploads the GSP_SYSTEM_INFO if NVIDIA
> vGPU is enabled.
>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  drivers/gpu/nova-core/gpu.rs             |  2 +-
>  drivers/gpu/nova-core/gsp.rs             |  8 ++-
>  drivers/gpu/nova-core/gsp/boot.rs        |  6 +-
>  drivers/gpu/nova-core/gsp/commands.rs    |  8 ++-
>  drivers/gpu/nova-core/gsp/fw.rs          | 75 ++++++++++++++++++++++++
>  drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++-
>  6 files changed, 102 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 10c5ae07a891..08a41e7bd982 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -285,7 +285,7 @@ pub(crate) fn new<'a>(
>  
>              sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset)?,
>  
> -            gsp <- Gsp::new(pdev)?,
> +            gsp <- Gsp::new(pdev, vgpu.vgpu_support)?,

This seems like it is making the whole `Vgpu` structure introduced in
the previous patch superfluous, since its sole purpose is to pass the
`vgpu_support` value to `Gsp::new` - we could just extract that value in
`Gsp::new`, or better in `Gsp::boot`, where we actually use it, and
avoid storing it in 3 different places.

>  
>              _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? },
>  
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index fb6f74797178..2d9352740c28 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -115,11 +115,16 @@ pub(crate) struct Gsp {
>      pub(crate) cmdq: Cmdq,
>      /// RM arguments.
>      rmargs: CoherentAllocation<GspArgumentsCached>,
> +    /// Support vGPU.
> +    vgpu_support: bool,
>  }
>  
>  impl Gsp {
>      // Creates an in-place initializer for a `Gsp` manager for `pdev`.
> -    pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self, Error>> {
> +    pub(crate) fn new(
> +        pdev: &pci::Device<device::Bound>,
> +        vgpu_support: bool,
> +    ) -> Result<impl PinInit<Self, Error>> {
>          let dev = pdev.as_ref();
>          let libos = CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
>              dev,
> @@ -156,6 +161,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
>              logrm,
>              rmargs,
>              cmdq,
> +            vgpu_support,
>          }))
>      }
>  }
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index 54937606b5b0..5016c630cec3 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -33,6 +33,7 @@
>      gpu::Chipset,
>      gsp::{
>          commands,
> +        fw::GspVfInfo,
>          sequencer::{
>              GspSequencer,
>              GspSequencerParams, //
> @@ -136,6 +137,7 @@ pub(crate) fn boot(
>          sec2_falcon: &Falcon<Sec2>,
>      ) -> Result {
>          let dev = pdev.as_ref();
> +        let vgpu_support = self.vgpu_support;
>  
>          let bios = Vbios::new(dev, bar)?;
>  
> @@ -162,8 +164,10 @@ pub(crate) fn boot(
>              CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
>          dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>  
> +        let vf_info = GspVfInfo::new(pdev, bar, vgpu_support)?;

This is a strange constructor. It initializes the `GspVfInfo` if
`vgpu_support` is true, and returns a zeroed structure otherwise. I'd
rather have an unconditional constructor that is only called if
`vgpu_support` is true, and store the result in an `Option`:

    let vf_info = if vgpu_support {
        Some(GspVfInfo::new(pdev, bar)?)
    } else {
        None
    };

It will become clearer later why this is a better design.

> +
>          self.cmdq
> -            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
> +            .send_command(bar, commands::SetSystemInfo::new(pdev, vf_info))?;

As a result of the previous comment, `SetSystemInfo::new` now takes an
`Option<GspVfInfo>`...

>          self.cmdq.send_command(bar, commands::SetRegistry::new())?;
>  
>          gsp_falcon.reset(bar)?;
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index 0425c65b5d6f..1d519c4ed232 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -26,6 +26,7 @@
>          },
>          fw::{
>              commands::*,
> +            GspVfInfo,
>              MsgFunction, //
>          },
>      },
> @@ -36,12 +37,13 @@
>  /// The `GspSetSystemInfo` command.
>  pub(crate) struct SetSystemInfo<'a> {
>      pdev: &'a pci::Device<device::Bound>,
> +    vf_info: GspVfInfo,

... and this becomes an `Option` as well.

>  }
>  
>  impl<'a> SetSystemInfo<'a> {
>      /// Creates a new `GspSetSystemInfo` command using the parameters of `pdev`.
> -    pub(crate) fn new(pdev: &'a pci::Device<device::Bound>) -> Self {
> -        Self { pdev }
> +    pub(crate) fn new(pdev: &'a pci::Device<device::Bound>, vf_info: GspVfInfo) -> Self {
> +        Self { pdev, vf_info }
>      }
>  }
>  
> @@ -51,7 +53,7 @@ impl<'a> CommandToGsp for SetSystemInfo<'a> {
>      type InitError = Error;
>  
>      fn init(&self) -> impl Init<Self::Command, Self::InitError> {
> -        GspSetSystemInfo::init(self.pdev)
> +        GspSetSystemInfo::init(self.pdev, self.vf_info)

And here things become interesting: you can leave the constructor if
`GspSetSystemInfo` unchanged, since vgpu support is not strictly
required to produce a valid value. But you can also chain the
initializer to add the vgpu information when relevant:

    GspSetSystemInfo::init(self.pdev).chain(|info| {
        if let Some(vf_info) = &self.vf_info {
            info.set_vf_info(vf_info);
        }

        Ok(())
    })

(more on `set_vf_info` below)

This results in less code overall, and better conveys the fact that vgpu
support is technically optional.

>      }
>  }
>  
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index abffd6beec65..a0581ac34586 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -9,8 +9,10 @@
>  use core::ops::Range;
>  
>  use kernel::{
> +    device,
>      dma::CoherentAllocation,
>      fmt,
> +    pci,
>      prelude::*,
>      ptr::{
>          Alignable,
> @@ -27,6 +29,7 @@
>  };
>  
>  use crate::{
> +    driver::Bar0,
>      fb::FbLayout,
>      firmware::gsp::GspFirmware,
>      gpu::Chipset,
> @@ -926,3 +929,75 @@ fn new(cmdq: &Cmdq) -> Self {
>          })
>      }
>  }
> +
> +/// VF information - gspVFInfo in SetSystemInfo.
> +#[derive(Clone, Copy, Zeroable)]
> +#[repr(transparent)]
> +pub(crate) struct GspVfInfo {
> +    inner: bindings::GSP_VF_INFO,
> +}

You can use a tuple struct here (i.e. `struct
GspVfInfo(bindings::GSP_VF_INFO`).

Also none of the derives should be needed eventually.

> +
> +impl GspVfInfo {
> +    /// Creates a new GspVfInfo structure.
> +    pub(crate) fn new<'a>(
> +        pdev: &'a pci::Device<device::Bound>,
> +        bar: &Bar0,
> +        vgpu_support: bool,

As mentioned before, we can drop this bool argument.

> +    ) -> Result<GspVfInfo> {
> +        let mut vf_info = GspVfInfo::zeroed();
> +        let info = &mut vf_info.inner;

It is generally considered better practice to avoid mutating values we
initialize. By starting with a zeroed state and then initializing the
members, you are at risk of forgetting to initialize some.

What you should do is initialize your `GspVfInfo` in its entirety in a
final statement like the other command structures do, storing
intermediate values in temporary variables if needed.

> +
> +        if vgpu_support {
> +            let val = pdev.sriov_get_totalvfs()?;
> +            info.totalVFs = u32::try_from(val)?;

We should be able to avoid this `try_from` once `sriov_get_totalvfs`
returns the correct `u16` type as suggested on the first patch. :)

> +
> +            let pos = pdev
> +                .find_ext_capability(kernel::bindings::PCI_EXT_CAP_ID_SRIOV as i32)
> +                .ok_or(ENODEV)?;

`pos` also seems to be the wrong type - it seems to me that it should be unsigned...

> +
> +            let val = pdev.config_read_word(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_VF_OFFSET as i32),
> +            )?;

... but I guess this comes from here - I understand that
`config_read_word` takes an `int` for its offset parameter, but when I
read the C code I also see checks that return errors if that offset is
bigger than 4095. What does the PCI spec says? It seems we can go with a
`u16` for the offset, which would simplify this code quite a bit.

Also please don't use `as` whenever possible, there are utility
functions in `crate::num` to do the conversions infallibly. You will
probably be interested in `u32_into_u16` for this method.

> +            info.firstVFOffset = u32::from(val);
> +
> +            let val = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32),
> +            )?;
> +            info.FirstVFBar0Address = u64::from(val);
> +
> +            let bar1_lo = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 4),
> +            )?;
> +            let bar1_hi = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 8),
> +            )?;
> +
> +            let addr_mask = u64::try_from(kernel::bindings::PCI_BASE_ADDRESS_MEM_MASK)?;

This `try_from` will always fail as `PCI_BASE_ADDRESS_MEM_MASK` is
negative. This is a case for a legit use of `as` (with a CAST: comment),
although this is also a case for us generating better bindings. :)

> +
> +            info.FirstVFBar1Address =
> +                (u64::from(bar1_hi) << 32) | ((u64::from(bar1_lo)) & addr_mask);
> +
> +            let bar2_lo = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 12),
> +            )?;
> +            let bar2_hi = pdev.config_read_dword(
> +                i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 16),
> +            )?;
> +
> +            info.FirstVFBar2Address = (u64::from(bar2_hi) << 32) | (u64::from(bar2_lo) & addr_mask);
> +
> +            let val = bar.read32(0x88000 + 0xbf4);
> +            info.b64bitBar1 = u8::from((val & 0x00000006) == 0x00000004);
> +
> +            let val = bar.read32(0x88000 + 0xbfc);
> +            info.b64bitBar2 = u8::from((val & 0x00000006) == 0x00000004);

Magic numbers baaaaaaad. Let's define these as proper registers.

Re: [RFC 2/7] [!UPSTREAM] rust: pci: support configuration space access

[Ewan CHORYNSKI]

On Sat, Dec 06, 2025 at 12:42:03PM +0000, Zhi Wang wrote:
> +    /// Find the extended capability
> +    pub fn find_ext_capability(&self, cap: i32) -> Option<u16> {
> +        // SAFETY: `self.as_raw()` is a valid pointer to a `struct pci_dev`.
> +        let offset = unsafe { bindings::pci_find_ext_capability(self.as_raw(), cap) };
> +        if offset != 0 {
> +            Some(offset as u16)
> +        } else {
> +            None
> +        }
> +    }

This is a good candidate to use `Option<NonZeroU16>` for the return
type as you don't expect Some(0) to be valid. It will also enable niche
optimisation for the Option. The whole if statement can even be replaced
by `NonZeroU16::new(offset as 16)` which return an option directly.

Re: [RFC 7/7] gpu: nova-core: load the scrubber ucode when vGPU support is enabled

[Joel Fernandes]

> On Dec 9, 2025, at 11:05 PM, Zhi Wang <zhiw@nvidia.com> wrote:
> [..]
>>> +
>>> +            dev_dbg!(
>>> +                pdev.as_ref(),
>>> +                "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n",
>>> +                mbox0,
>>> +                mbox1
>>> +            );
>>> +
>>> +            if
>>> !regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed()
>>> {
>>> +                return Err(ETIMEDOUT);  
>> 
>> So under which situation do you get to this point
>> (!scrubber_completed) ? Basically I am not sure if ETIMEDOUT is the
>> right error to return here, because boot() already returns ETIMEDOUT
>> by waiting for the halt.
>> 
>> If you still want return ETIMEDOUT here, then it sounds like you're
>> waiting for scrubbing beyond the waiting already done by boot(). If
>> so, then shouldn't you need to use read_poll_timeout() here?
>> 
>> perhaps something like:
>> 
>> read_poll_timeout(
>>     ||
>> Ok(regs::NV_PGC6_BSI_SECURE_SCRATCH_15::read(bar).scrubber_completed()),
>> |val: &bool| *val, Delta::from_millis(10),
>>     Delta::from_secs(5),
>> )?;
>> 
> 
> This is the identical implementation to OpenRM [1]. According to that
> parts of code, I think the scrubber runs in the binary booting process.
> When it signals the firmware booting successfully, the scrubbing should
> be done. Let me change to another errno.
> 
> [1]https://github.com/NVIDIA/open-gpu-kernel-modules/blob/a5bfb10e75a4046c5d991c65f49b5d29151e68cf/src/nvidia/src/kernel/gpu/gsp/arch/ada/kernel_gsp_ad102.c#L49

Sure, it was just misleading in the patch that we’re returning a timeout error, when the error is something else (like scrubber failed). Thanks for correcting it.

 - Joel

Re: [RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

[Joel Fernandes]

Hi Zhi,

On Tue, Dec 09, 2025 at 03:41:14PM +0200, Zhi Wang wrote:
> On Sat, 6 Dec 2025 21:32:51 -0500
> Joel Fernandes <joelagnelf@nvidia.com> wrote:
[..]
> > > 0x00000004); +
> > > +            let val = bar.read32(0x88000 + 0xbfc);
> > > +            info.b64bitBar2 = u8::from((val & 0x00000006) ==
> > > 0x00000004);
> > 
> > Please no magic numbers, please use proper named constants with
> > documentation comments explaining the values.
> > 
> > Also BAR reads here need proper register macro definitions/access.
> > 
> 
> That is true. :) But this is because there is no register definition in
> the OpenRM code/non OpenRM code as well. I have no idea about the name
> and bit definitions of this register.
> 
> Suppose I will have to find some clues from some folks then document
> them here when going to patches request for merged. :)


I think these magic numbers are PCIe config space related. I found a couple of references [1] [2] [3]

[1]
In Open GPU docs, I see 0x00088000 is NV_PCFG but this is on Turing, lets
confirm what it is on other architectures (if not common, should it go
through a HAL?).

https://github.com/NVIDIA/open-gpu-kernel-modules/blob/a5bfb10e75a4046c5d991c65f49b5d29151e68cf/src/common/inc/swref/published/turing/tu102/dev_nv_xve.h#L4

and 0xbf4 is SRIOV capability headers, per the same header file:
NV_XVE_SRIOV_CAP_HDR10

Also the bit definition is not documented in that public header, but I find
from internal sources that what you're trying to do with the "& 0x6" is
determine whether the VF BAR is capable of 64-bit addressing:

 Bits [2:1] is VF_BAR1_ADR_TYPE and = 2 means the BAR is capable of 64-bit
 addressing, and = 0 means 32-bit.

I wonder if the format of these capability headers are present in the PCI
specification? It is worth checking, I find some very similar mentions of the
value 2 being 64-bit in https://wiki.osdev.org/PCI as well.

[2]
In Nouveau I found the 0x88000
  drivers/gpu/drm/nouveau/nouveau_reg.h +684

With a bunch of ids and such which is typical of what is in config space:

#    define NV50_PBUS_PCI_ID                                0x00088000
#        define NV50_PBUS_PCI_ID_VENDOR_ID                  0x0000ffff
#        define NV50_PBUS_PCI_ID_VENDOR_ID__SHIFT                    0
#        define NV50_PBUS_PCI_ID_DEVICE_ID                  0xffff0000
#        define NV50_PBUS_PCI_ID_DEVICE_ID__SHIFT                   16

Perhaps this is something pdev.config_read_dword() should be giving?

[3] This one I am not sure off, but the link
https://envytools.readthedocs.io/en/latest/hw/bus/pci.html says that on NV40+
cards, all 0x1000 bytes of PCIE config space are mapped to MMIO register
space at addresses 0x88000-0x88fff. This matches exactly the magic number in
your patch.

Also, I wonder if we need to determine if the BARs can be 64-bit addressed, do
we have requirements for BAR sizes > 4GB for vGPU and if not, do we need to
determine the BAR size addressability?

Also, shouldn't the PCI core subsystem be automatically determining if the
BARs are 64-bit addressable? Not sure if that belongs in the driver. It would
be good to understand how this is supposed to work.

thanks,

 - Joel

Re: [RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

[John Hubbard]

On 12/11/25 5:36 PM, Joel Fernandes wrote:
> Hi Zhi,
> 
> On Tue, Dec 09, 2025 at 03:41:14PM +0200, Zhi Wang wrote:
>> On Sat, 6 Dec 2025 21:32:51 -0500
>> Joel Fernandes <joelagnelf@nvidia.com> wrote:
> [..]
>>>> 0x00000004); +
>>>> +            let val = bar.read32(0x88000 + 0xbfc);
>>>> +            info.b64bitBar2 = u8::from((val & 0x00000006) ==
>>>> 0x00000004);
>>>
>>> Please no magic numbers, please use proper named constants with
>>> documentation comments explaining the values.
>>>
>>> Also BAR reads here need proper register macro definitions/access.
>>>
>>
>> That is true. :) But this is because there is no register definition in
>> the OpenRM code/non OpenRM code as well. I have no idea about the name
>> and bit definitions of this register.
>>
>> Suppose I will have to find some clues from some folks then document
>> them here when going to patches request for merged. :)
> 
> 
> I think these magic numbers are PCIe config space related. I found a couple of references [1] [2] [3]
> 
> [1]
> In Open GPU docs, I see 0x00088000 is NV_PCFG but this is on Turing, lets
> confirm what it is on other architectures (if not common, should it go
> through a HAL?).

It changed on Hopper. My Hopper/Blackwell series handles this.

thanks,
John Hubbard

> 
> https://github.com/NVIDIA/open-gpu-kernel-modules/blob/a5bfb10e75a4046c5d991c65f49b5d29151e68cf/src/common/inc/swref/published/turing/tu102/dev_nv_xve.h#L4
> 
> and 0xbf4 is SRIOV capability headers, per the same header file:
> NV_XVE_SRIOV_CAP_HDR10
> 
> Also the bit definition is not documented in that public header, but I find
> from internal sources that what you're trying to do with the "& 0x6" is
> determine whether the VF BAR is capable of 64-bit addressing:
> 
>   Bits [2:1] is VF_BAR1_ADR_TYPE and = 2 means the BAR is capable of 64-bit
>   addressing, and = 0 means 32-bit.
> 
> I wonder if the format of these capability headers are present in the PCI
> specification? It is worth checking, I find some very similar mentions of the
> value 2 being 64-bit in https://wiki.osdev.org/PCI as well.
> 
> [2]
> In Nouveau I found the 0x88000
>    drivers/gpu/drm/nouveau/nouveau_reg.h +684
> 
> With a bunch of ids and such which is typical of what is in config space:
> 
> #    define NV50_PBUS_PCI_ID                                0x00088000
> #        define NV50_PBUS_PCI_ID_VENDOR_ID                  0x0000ffff
> #        define NV50_PBUS_PCI_ID_VENDOR_ID__SHIFT                    0
> #        define NV50_PBUS_PCI_ID_DEVICE_ID                  0xffff0000
> #        define NV50_PBUS_PCI_ID_DEVICE_ID__SHIFT                   16
> 
> Perhaps this is something pdev.config_read_dword() should be giving?
> 
> [3] This one I am not sure off, but the link
> https://envytools.readthedocs.io/en/latest/hw/bus/pci.html says that on NV40+
> cards, all 0x1000 bytes of PCIE config space are mapped to MMIO register
> space at addresses 0x88000-0x88fff. This matches exactly the magic number in
> your patch.
> 
> Also, I wonder if we need to determine if the BARs can be 64-bit addressed, do
> we have requirements for BAR sizes > 4GB for vGPU and if not, do we need to
> determine the BAR size addressability?
> 
> Also, shouldn't the PCI core subsystem be automatically determining if the
> BARs are 64-bit addressable? Not sure if that belongs in the driver. It would
> be good to understand how this is supposed to work.
> 
> thanks,
> 
>   - Joel
> 

Re: [RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

[Joel Fernandes]

> On Dec 12, 2025, at 9:17 AM, John Hubbard <jhubbard@nvidia.com> wrote:
> 
> On 12/11/25 5:36 PM, Joel Fernandes wrote:
>> Hi Zhi,
>>> On Tue, Dec 09, 2025 at 03:41:14PM +0200, Zhi Wang wrote:
>>> On Sat, 6 Dec 2025 21:32:51 -0500
>>> Joel Fernandes <joelagnelf@nvidia.com> wrote:
>> [..]
>>>>> 0x00000004); +
>>>>> +            let val = bar.read32(0x88000 + 0xbfc);
>>>>> +            info.b64bitBar2 = u8::from((val & 0x00000006) ==
>>>>> 0x00000004);
>>>> 
>>>> Please no magic numbers, please use proper named constants with
>>>> documentation comments explaining the values.
>>>> 
>>>> Also BAR reads here need proper register macro definitions/access.
>>>> 
>>> 
>>> That is true. :) But this is because there is no register definition in
>>> the OpenRM code/non OpenRM code as well. I have no idea about the name
>>> and bit definitions of this register.
>>> 
>>> Suppose I will have to find some clues from some folks then document
>>> them here when going to patches request for merged. :)
>> I think these magic numbers are PCIe config space related. I found a couple of references [1] [2] [3]
>> [1]
>> In Open GPU docs, I see 0x00088000 is NV_PCFG but this is on Turing, lets
>> confirm what it is on other architectures (if not common, should it go
>> through a HAL?).
> 
> It changed on Hopper. My Hopper/Blackwell series handles this.

Great, thanks for confirming, John. Zhi, you should probably rebase on the series from John then.

thanks,

 - Joel



> 
> thanks,
> John Hubbard
> 
>> https://github.com/NVIDIA/open-gpu-kernel-modules/blob/a5bfb10e75a4046c5d991c65f49b5d29151e68cf/src/common/inc/swref/published/turing/tu102/dev_nv_xve.h#L4
>> and 0xbf4 is SRIOV capability headers, per the same header file:
>> NV_XVE_SRIOV_CAP_HDR10
>> Also the bit definition is not documented in that public header, but I find
>> from internal sources that what you're trying to do with the "& 0x6" is
>> determine whether the VF BAR is capable of 64-bit addressing:
>>  Bits [2:1] is VF_BAR1_ADR_TYPE and = 2 means the BAR is capable of 64-bit
>>  addressing, and = 0 means 32-bit.
>> I wonder if the format of these capability headers are present in the PCI
>> specification? It is worth checking, I find some very similar mentions of the
>> value 2 being 64-bit in https://wiki.osdev.org/PCI as well.
>> [2]
>> In Nouveau I found the 0x88000
>>   drivers/gpu/drm/nouveau/nouveau_reg.h +684
>> With a bunch of ids and such which is typical of what is in config space:
>> #    define NV50_PBUS_PCI_ID                                0x00088000
>> #        define NV50_PBUS_PCI_ID_VENDOR_ID                  0x0000ffff
>> #        define NV50_PBUS_PCI_ID_VENDOR_ID__SHIFT                    0
>> #        define NV50_PBUS_PCI_ID_DEVICE_ID                  0xffff0000
>> #        define NV50_PBUS_PCI_ID_DEVICE_ID__SHIFT                   16
>> Perhaps this is something pdev.config_read_dword() should be giving?
>> [3] This one I am not sure off, but the link
>> https://envytools.readthedocs.io/en/latest/hw/bus/pci.html says that on NV40+
>> cards, all 0x1000 bytes of PCIE config space are mapped to MMIO register
>> space at addresses 0x88000-0x88fff. This matches exactly the magic number in
>> your patch.
>> Also, I wonder if we need to determine if the BARs can be 64-bit addressed, do
>> we have requirements for BAR sizes > 4GB for vGPU and if not, do we need to
>> determine the BAR size addressability?
>> Also, shouldn't the PCI core subsystem be automatically determining if the
>> BARs are 64-bit addressable? Not sure if that belongs in the driver. It would
>> be good to understand how this is supposed to work.
>> thanks,
>>  - Joel
> 

Re: [RFC 5/7] gpu: nova-core: set RMSetSriovMode when NVIDIA vGPU is enabled

[Alexandre Courbot]

On Mon Dec 8, 2025 at 12:55 AM JST, Timur Tabi wrote:
> On Sat, 2025-12-06 at 12:42 +0000, Zhi Wang wrote:
>> -    pub(crate) fn new() -> Self {
>> +    pub(crate) fn new(vgpu_support: bool) -> Self {
>> +        let num_entries = if vgpu_support { 4 } else { 3 };
>
> Instead of passing a bool, and then hard-coding the length based on that bool (which would
> require that RMSetSriovMode always be the last entry in the array), you need to do what Nouveau
> does: if VGPU is enabled, then dynamically append the entry to the array.

Yup, as we will add more stuff to the registry depending on various
conditions it would be great to make it more flexible.

The way for this would be to make `SetRegistry` wrap a `KVec` of
`RegistryEntry` - that way we can add what we need according to the
runtime options.

The current design of `SetRegistry` (which does not directly wraps the
type to write to the command queue, but instead implements
`CommandToGsp` to create it as the command is sent) should make this
rather trivial.

Re: [RFC 6/7] gpu: nova-core: reserve a larger GSP WPR2 heap when vGPU is enabled.

[Alexandre Courbot]

On Sat Dec 6, 2025 at 9:42 PM JST, Zhi Wang wrote:
> To support the maximum vGPUs on devices that support vGPU, a larger
> WPR2 heap size is required. On Ada with vGPU supported, the size should
> be set to at least 581MB.
>
> When vGPU support is enabled, reserve a large WPR2 heap size up to
> 581MB, set the max supported VF to max in WPR2 meta.

This patch also sets the number of partitions, this should be
mentioned in the commit log as well.

>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  drivers/gpu/nova-core/fb.rs       | 19 +++++++++++++++----
>  drivers/gpu/nova-core/gsp/boot.rs |  2 +-
>  2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 3c9cf151786c..9a5c40029f3a 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
> @@ -119,7 +119,12 @@ pub(crate) struct FbLayout {
>  
>  impl FbLayout {
>      /// Computes the FB layout for `chipset` required to run the `gsp_fw` GSP firmware.
> -    pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<Self> {
> +    pub(crate) fn new(
> +        chipset: Chipset,
> +        bar: &Bar0,
> +        gsp_fw: &GspFirmware,
> +        vgpu_support: bool,
> +    ) -> Result<Self> {
>          let hal = hal::fb_hal(chipset);
>  
>          let fb = {
> @@ -181,8 +186,12 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>  
>          let wpr2_heap = {
>              const WPR2_HEAP_DOWN_ALIGN: Alignment = Alignment::new::<SZ_1M>();
> -            let wpr2_heap_size =
> -                gsp::LibosParams::from_chipset(chipset).wpr_heap_size(chipset, fb.end);
> +            let wpr2_heap_size = if !vgpu_support {
> +                gsp::LibosParams::from_chipset(chipset).wpr_heap_size(chipset, fb.end)
> +            } else {
> +                581 * usize_as_u64(SZ_1M)

Do we have a constant defined somewhere in OpenRM for this 581 size? If
so, let's regenerate the bindings to add it and use it here.

> +            };
> +
>              let wpr2_heap_addr = (elf.start - wpr2_heap_size).align_down(WPR2_HEAP_DOWN_ALIGN);
>  
>              wpr2_heap_addr..(elf.start).align_down(WPR2_HEAP_DOWN_ALIGN)
> @@ -202,6 +211,8 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>              wpr2.start - HEAP_SIZE..wpr2.start
>          };
>  
> +        let vf_partition_count = if vgpu_support { 32 } else { 0 };

Same question for the `32` magic number here. I suspect this will evolve
with future chips, so we need to have a good source of truth so we can
abstract this when needed.

Re: [RFC 7/7] gpu: nova-core: load the scrubber ucode when vGPU support is enabled

[Alexandre Courbot]

On Sat Dec 6, 2025 at 9:42 PM JST, Zhi Wang wrote:
> To support the maximum vGPUs on the device that support vGPU, a larger
> WPR2 heap size is required. By setting the WPR2 heap size larger than 256MB
> the scrubber ucode image is required to scrub the FB memory before any
> other ucode image is executed.

So the trigger for running the scrubber ucode is not as much "vGPU is in
use" as it is "the WPR2 heap size is larger than 256MB".

Can we change the code to reflect that? We have the `fb_layout`
structure available at this time, so it should be easy IIUC.