Re: [PATCH] PCI/IOV: "virtfn4294967295\0" requires 17 bytes

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Sun, 2022-12-18 at 22:19 +0000, Matthew Wilcox wrote:
> On Sun, Dec 18, 2022 at 03:21:39PM +0300, Alexey V. Vissarionov wrote:
> > On 2022-12-18 19:57:02 +0900, Krzysztof Wilczyński wrote:
> > 
> >  > Thank you for sending the patch over! However, if possible,
> >  > can you send it as plain text without any multi-part MIME
> >  > involved?
> > 
> > ACK.
> > 
> >  > If possible, it would be nice to mention that this needed
> >  > to make sure that there is enough space to correctly
> >  > NULL-terminate the ID string.
> > 
> > ACK.
> > 
> > So, here goes the corrected text:
> > 
> > Although unlikely, the 'id' value may be as big as 4294967295
> > (uint32_max) and "virtfn4294967295\0" would require 17 bytes
> > instead of 16 to make sure that buffer has enough space to
> > properly NULL-terminate the ID string.
> 
> Wait, what?  How can we get to a number that large for the virtual
> function ID?  devfn is 8 bits, bus is a further 8 bits.  Sure, domain
> is an extra 16 bits on top of that but I'm pretty sure that virtual
> functions can't span multiple domains.  Unless that's changed recently?
> 
> Even if they can, we'd need to span 2^14 domains to get up to a billion
> IDs.  That's a hell of a system and I think overflowing here is the
> least of our problems.
> 
> So while this is typed as u32, I don't think it can get anywhere close.

While we can't realistically get such high IDs on s390 at the moment
it's important to note that we do use the PCI domains differently and
will use the entire 16 bit namespace. As s390 aka mainframes always has
a virtualization layer which does the PCI enumeration for us and just
presents us with single PCI functions from which we reconstruct a PCI
hiearchy. Historically the only PCI devices available were Virtual
Functions which then use bus and devfn 0 with either an incrementing or
user defined domain ID as virtual PCI domain. Meaning that
"ffff:00:00.0" can and does occur. Since commit d1379279f2d6b
("s390/pci: Handling multifunctions") we may also present multiple
functions under the same domain with the a local topology matching the
hardware, though currently limited to a single fixed bus 0, but still
with a virtual and user defined domain ID. So at least on s390 we would
"only" need 2^16 VFs to see ffff:ff:ff.f.

That said, as far as I can tell this wouldn't actually create a virtfn
ID of such a high value as I don't think the domain actually plays into
that. For example with the following (sub) topology of 2 PFs with 4 VFs
each:

 +-[0008:00]-+-00.0  Mellanox Technologies MT28800 Family [ConnectX-5 Ex]
 |           +-00.1  Mellanox Technologies MT28800 Family [ConnectX-5 Ex]
 |           +-00.2  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
 |           +-00.3  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
 |           +-00.4  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
 |           +-00.5  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
 |           +-08.2  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
 |           +-08.3  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
 |           +-08.4  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
 |           \-08.5  Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]

I actually get:

# ls -l /sys/bus/pci/devices/0008\:00\:00.0/virtfn*
... /sys/bus/pci/devices/0008:00:00.0/virtfn0 -> ../0008:00:00.2
... /sys/bus/pci/devices/0008:00:00.0/virtfn1 -> ../0008:00:00.3
... /sys/bus/pci/devices/0008:00:00.0/virtfn2 -> ../0008:00:00.4
... /sys/bus/pci/devices/0008:00:00.0/virtfn3 -> ../0008:00:00.5

# ls -l /sys/bus/pci/devices/0008\:00\:00.1/virtfn*
... /sys/bus/pci/devices/0008:00:00.1/virtfn0 -> ../0008:00:08.2
... /sys/bus/pci/devices/0008:00:00.1/virtfn1 -> ../0008:00:08.3
... /sys/bus/pci/devices/0008:00:00.1/virtfn2 -> ../0008:00:08.4
... /sys/bus/pci/devices/0008:00:00.1/virtfn3 -> ../0008:00:08.5

Thanks,
Niklas