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

[Joel Fernandes <joelagnelf@nvidia.com>]

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.