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

On Sat, 6 Dec 2025 21:32:51 -0500
Joel Fernandes <joelagnelf at 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.
>

On Dec 9, 2025, at 10:41?PM, Zhi Wang <zhiw at nvidia.com> wrote:

?On Sat, 6 Dec 2025 21:32:51 -0500
Joel Fernandes <joelagnelf at 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.

Ah interesting. Let me also look into it on my side, see if I can find more
information. Thanks for mentioning the challenge.


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

Yes, I would say we definitely need to dig more into this ;) fun.


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 :)

 - Joel

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20251211/c6d43b1b/attachment-0001.htm>

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 at 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