RE: [PATCH 1/1] s390/pci: expose a PCI device's UID as its index

[K, Narendra]

> -----Original Message-----
> From: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxx>
> Sent: Saturday, April 17, 2021 4:18 PM
> To: K, Narendra; Niklas Schnelle; Bjorn Helgaas
> Cc: Stefan Raspl; Peter Oberparleiter; linux-netdev@xxxxxxxxxxxxxxx; linux-
> pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> s390@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/1] s390/pci: expose a PCI device's UID as its index
> 
> 
> [EXTERNAL EMAIL]
> 
> 
> 
> On 4/16/21 7:58 PM, K, Narendra wrote:
> >> -----Original Message-----
> >> From: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> >> Sent: Thursday, April 15, 2021 12:55 PM
> >> To: Bjorn Helgaas
> >> Cc: K, Narendra; Viktor Mihajlovski; Stefan Raspl; Peter
> >> Oberparleiter; linux- netdev@xxxxxxxxxxxxxxx;
> >> linux-pci@xxxxxxxxxxxxxxx; linux- kernel@xxxxxxxxxxxxxxx;
> >> linux-s390@xxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH 1/1] s390/pci: expose a PCI device's UID as its
> >> index
> >>
> >>
> >> [EXTERNAL EMAIL]
> >>
> >> On Wed, 2021-04-14 at 15:17 -0500, Bjorn Helgaas wrote:
> >>> On Mon, Apr 12, 2021 at 03:59:05PM +0200, Niklas Schnelle wrote:
> >>>> On s390 each PCI device has a user-defined ID (UID) exposed under
> >>>> /sys/bus/pci/devices/<dev>/uid. This ID was designed to serve as
> >>>> the PCI device's primary index and to match the device within Linux
> >>>> to the device configured in the hypervisor. To serve as a primary
> >>>> identifier the UID must be unique within the Linux instance, this
> >>>> is guaranteed by the platform if and only if the UID Uniqueness
> >>>> Checking flag is set within the CLP List PCI Functions response.
> >>>>
> >>>> In this sense the UID serves an analogous function as the SMBIOS
> >>>> instance number or ACPI index exposed as the "index" respectively
> >>>> "acpi_index" device attributes and used by e.g. systemd to set
> >>>> interface names. As s390 does not use and will likely never use
> >>>> ACPI nor SMBIOS there is no conflict and we can just expose the UID
> >>>> under the
> >> "index"
> >>>> attribute whenever UID Uniqueness Checking is active and get
> >>>> systemd's interface naming support for free.
> >>>>
> >>>> Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> >>>> Acked-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxx>
> >>>
> >>> This seems like a nice solution to me.
> >>>
> >>> Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >>
> >> Thanks! Yes I agree it's a simple solution that also makes sense from
> >> a design point. I'll wait for Narendra's opinion of course.
> >
> > Hi Niklas,
> >
> > It seems like the UID and the index are not equivalent in their meaning.
> >
> Hi Narendra,
> 
> the reasoning behind the wish to reuse index is that we think it's similar in the
> sense that it provides a stable, firmware-reported interface identifier.
> Some background: s390 is a highly virtualized platform. There's no traditional
> bare metal mode of operation, neither for the computer system nor the I/O
> subsystem.
> The equivalent to a standalone system is a logical partition (LPAR) which in
> essence is a kind of virtual machine. LPARs access I/O devices in a virtualized
> fashion as well. E.g., for PCI devices the I/O subsystem is responsible for the
> management of PCI hardware and will provide the PCI functions to the LPARs.
> This is where UID and function_id come into play. Each PCI function will carry a
> function_id attribute which defines it uniquely within the entire s390 system. If
> a UID is configured for the function, it must be unique within an LPAR, but
> doesn't need to be unique system-wide.
> For instance, the admistrator may want to ensure that for every LPAR the
> primary ethernet interface has the same identifier, to allow miigration of Linux
> instances accross LPARs.
> This can't be achieved with a slot-based name.
> > The index SMBIOS type 41 device type instance indicates that
> >
> > 1. The device is an onboard device.
> > 2. A specific order of the onboard devices.
> >
> > The UID described seems to indicate that the PCI device is unique within the
> Linux instance (sufficient for naming).
> > But it does not indicate that PCI device is onboard and any order.
> >
> > As all devices with UID will be named as eno<UID> (Ethernet onboard),
> > names are not in sync with how each PCI function is exposed on a PCI slot
> (appears closer to SMBIOS type 9 record) as described below.
> >
> > https://urldefense.com/v3/__https://github.com/systemd/systemd/pull/19
> > 017__;!!LpKI!zDeT5hnnMp8tFNAzwNWtW3-
> C6w7p4FBLldAu705T3EPicJZNI7TewsdZa
> > LDBMQ$ [github[.]com]
> >
> https://urldefense.com/v3/__https://github.com/systemd/systemd/commit/
> >
> a496a238e8ee66ce25ad13a3f46549b2e2e979fc__;!!LpKI!zDeT5hnnMp8tFNAz
> wNWt
> > W3-C6w7p4FBLldAu705T3EPicJZNI7TewsfDTU8TaQ$ [github[.]com]
> >
> > In summary, it seems like the eno<UID> names on s390 will be unique
> names, but may not match the meaning of the index.
> >
> > Also,
> >
> > a) Will UID remain unique/same as initially exposed across reboots ?
> Yes, the UID is the primary identifier for a PCI function and part of the LPAR
> configuration. Even hotplug will not change the UID.
> >
> > b) Is the reuse of index related to the 'slots' based naming scheme described
> below ?
> >
> > https://urldefense.com/v3/__https://github.com/systemd/systemd/pull/19
> > 017__;!!LpKI!zDeT5hnnMp8tFNAzwNWtW3-
> C6w7p4FBLldAu705T3EPicJZNI7TewsdZa
> > LDBMQ$ [github[.]com]
> >
> https://urldefense.com/v3/__https://github.com/systemd/systemd/commit/
> >
> a496a238e8ee66ce25ad13a3f46549b2e2e979fc__;!!LpKI!zDeT5hnnMp8tFNAz
> wNWt
> > W3-C6w7p4FBLldAu705T3EPicJZNI7TewsfDTU8TaQ$ [github[.]com]
> >
> No, this is independent. The commit above fixes the slot name parsing.
> > c) If the slot based naming is fixed with the naming scheme from changes
> below, any thoughts on why is reuse of index needed ?
> >
> > https://urldefense.com/v3/__https://github.com/systemd/systemd/pull/19
> > 017__;!!LpKI!zDeT5hnnMp8tFNAzwNWtW3-
> C6w7p4FBLldAu705T3EPicJZNI7TewsdZa
> > LDBMQ$ [github[.]com]
> >
> https://urldefense.com/v3/__https://github.com/systemd/systemd/commit/
> >
> a496a238e8ee66ce25ad13a3f46549b2e2e979fc__;!!LpKI!zDeT5hnnMp8tFNAz
> wNWt
> > W3-C6w7p4FBLldAu705T3EPicJZNI7TewsfDTU8TaQ$ [github[.]com]
> As I wrote above, the slot describes the PCI function at the system level,
> whereas the uid/index does it in the context off the LPAR.

Hi Viktor,

Thank you for the context and clarification. 

I am not familiar with s390 and have not reviewed the patch. Please find the ack for reuse of  '/sys/bus/pci/devices/.../index'  sysfs attribute. 

Acked-by: Narendra K <narendra_k@xxxxxxxx>

With regards,
Narendra K