Re: [PATCH] PCI: hv: support reporting serial number as slot information

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Fri, Jul 13, 2018 at 02:38:19PM -0700, Stephen Hemminger wrote:
> On Wed, 11 Jul 2018 08:49:08 -0500
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> > On Wed, Jul 04, 2018 at 05:08:34PM +0100, Lorenzo Pieralisi wrote:
> > > On Tue, Jun 12, 2018 at 09:40:37AM -0700, Stephen Hemminger wrote:  
> > > > The Hyper-V host API for PCI provides a unique "serial number" which
> > > > can be used as the basis for sysfs PCI slot table. This can be useful
> > > > for cases where userspace wants to find the PCI device based on
> > > > serial number.
> > > > 
> > > > When an SR-IOV NIC is added, the host sends an attach message
> > > > with a serial number. The kernel doesn't use the serial number, but
> > > > it is useful when doing the same thing in a userspace driver such
> > > > as the DPDK. By having /sys/bus/pci/slots/N it provides a direct
> > > > way to find the matching PCI device.  
> > 
> > Is this essentially a way to expose the serial number to userspace?
> > 
> > I confess I'm not super familiar with what the slot infrastructure
> > gives us, and I'm trying to figure out if it's the right way to do
> > this.  Would a file directly in the device directory, e.g.,
> > 
> >   /sys/devices/pci0000:00/0000:00:00.0/serial
> > 
> > be a possibility?  There is an optional PCIe Device Serial Number
> > capability and also an optional serial number item in the VPD.  We
> > don't support either in the core yet, but maybe they could all be
> > exposed similarly?
> > 
> > Or is there something you need from the slot infrastructure in
> > addition to the serial number exposure?
> > 
> > You mentioned that this patch is along the lines of what KVM does.
> > Can you connect the dots a little bit for me as far as how this is
> > implemented for KVM?  I don't see KVM-related calls to
> > pci_create_slot() or pci_hp_register().
> > 
> > > > There may be some cases where the serial number is not unique such
> > > > as when using GPU's. But the PCI slot infrastructure will handle
> > > > that.
> > > > 
> > > > This also shortens the network device names generated by
> > > > systemd/udev. The new names use slot (ens2) rather than
> > > > PCI address (enP2p0s2).  
> > 
> > I don't recognize "enP2p0s2" as a PCI address.  Is that related to a
> > domain/bus/device/function address somehow, or is this some sort of
> > Windows-specific address?
> > 
> > > Hi Stephen,
> > > 
> > > I wanted to apply this patch but wanted to make sure all HV
> > > maintainers are in agreement first since this looks like
> > > a significant user-space ABI change.
> > > 
> > > I would also ask Bjorn's opinion on this since he has more
> > > insights into the slot interface history.
> > > 
> > > Thanks,
> > > Lorenzo
> > >   
> > > > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> 
> Yes, this is a way to expose serial number to userspace as well as a
> way to help udev with naming.

When you say "to help" it means "to fix" or to bring it on a par
with KVM ? This change is Hyper-V specific so I am not really against
it, still, I am not a big fan of making a serial number look like
a slot number to please udev either.

> Today, a VF device shows up as:
> $ lspci -n
> 0002:00:02.0 0200: 15b3:1014 (rev 80)
> 
> 
> And therefore is named: enP2p0s2 based on these rules:
> See: https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c#L20
> 
>  * Two character prefixes based on the type of interface:
>  *   en — Ethernet
>  *   sl — serial line IP (slip)
>  *   wl — wlan
>  *   ww — wwan
>  *
>  * Type of names:
> 
>  *   o<index>[n<phys_port_name>|d<dev_port>]
>  *                                         — on-board device index number
>  *   s<slot>[f<function>][n<phys_port_name>|d<dev_port>]
>  *                                         — hotplug slot index number
>  *   x<MAC>                                — MAC address
>  *   [P<domain>]p<bus>s<slot>[f<function>][n<phys_port_name>|d<dev_port>]
>  *                                         — PCI geographical location
> 
> I looked into several other options:
>   - put serial number into the device index number
> 
> 
> On KVM, the ACPI emulation provides PCI slot information so that is why
> the devices are named "ens3".
> 
> I looked into several other options (none of which worked well):
>   * provide sysfs /index file but that wants to be handled by smbios
>   * use VPD serial number but that is a GUID not an integer
>   * full hotplug (but Hyper-V doesn't support real PCI hotplug)
>   * new device_attribute serial, but that is not easy to hook
>     into existing code, and udev won't know about it.
> 
> Open to any ideas about how to best represent this.

By trawling PCI patchwork I found this series that was not merged, maybe
it is a starting point to implement the latter suggestion:

https://patchwork.ozlabs.org/patch/264610/

udev won't know about it but as I say above that's not a bug
we are fixing either AFAICS.

At the end of the day it is Hyper-V maintainers call, since that's
Hyper-V specific; exposing the serial attribute is something we
can do regardless, depending on what Bjorn thinks about it and you
can use it to provide userspace with the information you need.

> Thanks,
> Stephen
> 
> 
> 
> PS: it is  nuisance that when the PCI support was moved to
> drivers/pci/controllers it was not done in a way  that git correctly
> tracked the rename. Therefore all the commit history was broken :-(

Sorry about that.

Isn't "git log --follow" enough for this purpose ?

Thanks,
Lorenzo