Re: [PATCH v2 1/1] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled

On Wed Aug 5, 2026 at 2:00 AM JST, Zhi Wang wrote:
> GSP firmware requires VF BAR layout information to correctly route
> interrupts and doorbell events to individual virtual functions.
> Without this data the firmware cannot set up per-VF event routing.
>
> When vGPU mode is enabled, read the VF topology from the typed PCI
> SR-IOV extended capability and populate the GSP_VF_INFO embedded in
> the GSP_SYSTEM_INFO command. Keep the VF information builder associated
> with GspSetSystemInfo since it is only used to construct that command.
>
> Link: https://lore.kernel.org/nova-gpu/[email protected]/
> Signed-off-by: Zhi Wang <[email protected]>

This is looking good IMHO, and actually I would suggest including it
with your PCI extended capabilities' v9: that way, the added SR-IOV
capability gets a user immediately and we can merge both patches in one
go. It is also helpful for review to see the code being used in
practice.

Just one question: we are passing `VgpuState` by value everywhere, which
is ok for now, but is it expected to grow further in the future? Its
name suggests that it could end up storing much more things.

<...>
> diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
> index 6dc31d1bf5ae..71feda9c1e69 100644
> --- a/drivers/gpu/nova-core/gsp/fw/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
> @@ -16,7 +16,8 @@
>  use crate::{
>      gpu::Chipset,
>      gsp::GSP_PAGE_SIZE,
> -    num::IntoSafeCast, //
> +    num::IntoSafeCast,
> +    vgpu::VgpuState, //
>  };
>  
>  use super::bindings;
> @@ -29,10 +30,43 @@ pub(crate) struct GspSetSystemInfo {
>  static_assert!(size_of::<GspSetSystemInfo>() < GSP_PAGE_SIZE);
>  
>  impl GspSetSystemInfo {
> +    /// Builds the VF topology passed to GSP-RM as part of the system information.
> +    fn build_vf_info(
> +        dev: &pci::Device<device::Bound>,
> +        vgpu_state: VgpuState,
> +    ) -> Result<bindings::GSP_VF_INFO> {
> +        let VgpuState::Enabled { total_vfs } = vgpu_state else {
> +            return Ok(Default::default());

Maybe `Zeroable::zeroed()` for consistency with the `Enabled` path below?