[RFC v2 0/1] s390/pci: expose UID checking state in sysfs

[RFC v2 0/1] s390/pci: expose UID checking state in sysfs

[Niklas Schnelle]

Hi Bjorn,

this is a follow-up to my previous RFC of exposing our s390 specific UID
Checking attribute at /sys/bus/pci/zpci/unique_uids. As suggested by Greg
(thanks!) this version changes things to use named attributes directly without
resorting to any raw kobject handling, as a result the code moves to
drivers/pci/pci-sysfs.c with a CONFIG_S390 ifdef. Also I've changed the wording
to be more restrictive about what this attribute means. Instead of directly
calling out its current use to determine if UIDs are used as PCI domains it now
only explicitly claims that /sys/bus/pci/devices/<dev>/uid is guaranteed to be
a unique user-defined (in the zVM/KVM/LPAR configuration not Linux user-space)
identifier for the PCI function.

We've had some more internal discussion on this and are also considering to
instead put this attribute at /sys/firmware/zpci/unique_uids but as far as
I can see this strictly requires the use of raw kobject handling and loses us
the direct relation with PCI so I wanted to give this just one more shot and
get your opinion on it.

Thanks,
Niklas


Niklas Schnelle (1):
  PCI: Add s390 specific UID uniqueness attribute

 Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
 drivers/pci/pci-sysfs.c                 | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+)

-- 
2.17.1

[RFC v2 1/1] PCI: Add s390 specific UID uniqueness attribute

[Niklas Schnelle]

The global UID uniqueness attribute exposes whether the platform
guarantees that the user-defined per-device UID attribute values
(/sys/bus/pci/device/<dev>/uid) are unique and can thus be used as
a global identifier for the associated PCI device. With this commit
it is exposed at /sys/bus/pci/zpci/unique_uids

Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
 drivers/pci/pci-sysfs.c                 | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 25c9c39770c6..812dd9d3f80d 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -375,3 +375,12 @@ Description:
 		The value comes from the PCI kernel device state and can be one
 		of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold".
 		The file is read only.
+What:		/sys/bus/pci/zpci/unique_uids
+Date:		February 2021
+Contact:	Niklas Schnelle <schnelle@linux.ibm.com>
+Description:
+		This attribute exposes the global state of UID Uniqueness on an
+		s390 Linux system. If this file contains '1' the per-device UID
+		attribute is guaranteed to provide a unique user defined
+		identifier for that PCI device. If this file contains '0' UIDs
+		may collide and do not provide a unique identifier.
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index fb072f4b3176..61de66ab59cf 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -425,6 +425,24 @@ static ssize_t rescan_store(struct bus_type *bus, const char *buf, size_t count)
 }
 static BUS_ATTR_WO(rescan);
 
+#if defined(CONFIG_S390)
+static ssize_t unique_uids_show(struct bus_type *bus, char *buf)
+{
+	return sprintf(buf, "%i\n", zpci_unique_uid);
+}
+static BUS_ATTR_RO(unique_uids);
+
+static struct attribute *zpci_bus_attrs[] = {
+	&bus_attr_unique_uids.attr,
+	NULL,
+};
+
+static struct attribute_group zpci_bus_group = {
+	.name = "zpci",
+	.attrs = zpci_bus_attrs,
+};
+#endif
+
 static struct attribute *pci_bus_attrs[] = {
 	&bus_attr_rescan.attr,
 	NULL,
@@ -436,6 +454,9 @@ static const struct attribute_group pci_bus_group = {
 
 const struct attribute_group *pci_bus_groups[] = {
 	&pci_bus_group,
+#if defined(CONFIG_S390)
+	&zpci_bus_group,
+#endif
 	NULL,
 };
 
-- 
2.17.1

Re: [RFC v2 1/1] PCI: Add s390 specific UID uniqueness attribute

[Greg Kroah-Hartman]

On Thu, Feb 04, 2021 at 10:43:53AM +0100, Niklas Schnelle wrote:
> The global UID uniqueness attribute exposes whether the platform
> guarantees that the user-defined per-device UID attribute values
> (/sys/bus/pci/device/<dev>/uid) are unique and can thus be used as
> a global identifier for the associated PCI device. With this commit
> it is exposed at /sys/bus/pci/zpci/unique_uids
> 
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
>  drivers/pci/pci-sysfs.c                 | 21 +++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 25c9c39770c6..812dd9d3f80d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -375,3 +375,12 @@ Description:
>  		The value comes from the PCI kernel device state and can be one
>  		of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold".
>  		The file is read only.
> +What:		/sys/bus/pci/zpci/unique_uids

No blank line before this new line?

And why "zpci"?

> +Date:		February 2021
> +Contact:	Niklas Schnelle <schnelle@linux.ibm.com>
> +Description:
> +		This attribute exposes the global state of UID Uniqueness on an
> +		s390 Linux system. If this file contains '1' the per-device UID
> +		attribute is guaranteed to provide a unique user defined
> +		identifier for that PCI device. If this file contains '0' UIDs
> +		may collide and do not provide a unique identifier.

What are they "colliding" with?  And where does the UID come from, the
device itself or somewhere else?

thanks,

greg k-h

Re: [RFC v2 1/1] PCI: Add s390 specific UID uniqueness attribute

[Niklas Schnelle]

On 2/4/21 11:40 AM, Greg Kroah-Hartman wrote:
> On Thu, Feb 04, 2021 at 10:43:53AM +0100, Niklas Schnelle wrote:
>> The global UID uniqueness attribute exposes whether the platform
>> guarantees that the user-defined per-device UID attribute values
>> (/sys/bus/pci/device/<dev>/uid) are unique and can thus be used as
>> a global identifier for the associated PCI device. With this commit
>> it is exposed at /sys/bus/pci/zpci/unique_uids
>>
>> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
>> ---
>>  Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
>>  drivers/pci/pci-sysfs.c                 | 21 +++++++++++++++++++++
>>  2 files changed, 30 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
>> index 25c9c39770c6..812dd9d3f80d 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-pci
>> +++ b/Documentation/ABI/testing/sysfs-bus-pci
>> @@ -375,3 +375,12 @@ Description:
>>  		The value comes from the PCI kernel device state and can be one
>>  		of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold".
>>  		The file is read only.
>> +What:		/sys/bus/pci/zpci/unique_uids
> 
> No blank line before this new line?

Good catch, thanks!

> 
> And why "zpci"?

There doesn't seem to be a precedent for arch specific attributes under
/sys/bus/pci so I went with a separate group for the RFC.
"zpci" since that's what we've been calling the s390 specific PCI.
I'm not attached to that name or having a separate group, from
my perspective /sys/bus/pci/unique_uids would actually be ideal
if Bjorn is okay with that, we don't currently foresee any additional
global attributes and no one else seems to have them but
one never knows and a separate group would then of course,
well group them.

> 
>> +Date:		February 2021
>> +Contact:	Niklas Schnelle <schnelle@linux.ibm.com>
>> +Description:
>> +		This attribute exposes the global state of UID Uniqueness on an
>> +		s390 Linux system. If this file contains '1' the per-device UID
>> +		attribute is guaranteed to provide a unique user defined
>> +		identifier for that PCI device. If this file contains '0' UIDs
>> +		may collide and do not provide a unique identifier.
> 
> What are they "colliding" with?  And where does the UID come from, the
> device itself or somewhere else?

If this attribute is 0 multiple PCI devices seen by Linux may have the same UID i.e.
they may collide with each other on the same Linux instance. The
UIDs are exposed under /sys/bus/pci/devices/<dev>/uid. Even if the attribute is 1
multiple Linux instances will often see the same UID. The main use case
we currently envision is naming PCI based network interfaces "eno<UID>"
which of course only works if the UIDs are unique for that Linux.
On the same mainframe multiple Linux instances may then e.g. use the same
UID for VFs from the same physical PCI network card or different cards
but the same physical network all defined by an administrator/management
system.

The UIDs come from the platform/firmware/hypervisor and are provided
for each device by the CLP List PCI Functions "instruction" that is used
on s390x where an OS can't probe the physical PCI bus but instead
that is done by firmware. On QEMU/KVM they can be set on the QEMU cli,
on our machine hypervisor they are defined by the machine administrator/management
software as part of the definition of VMs/Partitions on that mainframe which includes
everything from the number of CPUs, memory, I/O devices etc. With the exposed UID uniqueness
attribute the platform then certifies that it will ensure that a UID is set to
a unique non-zero value. I can of course add more of this explanation
in the documentation too.

Both the uniqueness guarantee (this attribute) as well as the UIDs themselves
are part of the Z (s390x) architecture so fall under the mainframe typical
backwards compatibility considerations.

Thanks,
Niklas

> 
> thanks,
> 
> greg k-h
> 

Re: [RFC v2 1/1] PCI: Add s390 specific UID uniqueness attribute

[Greg Kroah-Hartman]

On Thu, Feb 04, 2021 at 01:02:51PM +0100, Niklas Schnelle wrote:
> 
> 
> On 2/4/21 11:40 AM, Greg Kroah-Hartman wrote:
> > On Thu, Feb 04, 2021 at 10:43:53AM +0100, Niklas Schnelle wrote:
> >> The global UID uniqueness attribute exposes whether the platform
> >> guarantees that the user-defined per-device UID attribute values
> >> (/sys/bus/pci/device/<dev>/uid) are unique and can thus be used as
> >> a global identifier for the associated PCI device. With this commit
> >> it is exposed at /sys/bus/pci/zpci/unique_uids
> >>
> >> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> >> ---
> >>  Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
> >>  drivers/pci/pci-sysfs.c                 | 21 +++++++++++++++++++++
> >>  2 files changed, 30 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> >> index 25c9c39770c6..812dd9d3f80d 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-pci
> >> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> >> @@ -375,3 +375,12 @@ Description:
> >>  		The value comes from the PCI kernel device state and can be one
> >>  		of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold".
> >>  		The file is read only.
> >> +What:		/sys/bus/pci/zpci/unique_uids
> > 
> > No blank line before this new line?
> 
> Good catch, thanks!
> 
> > 
> > And why "zpci"?
> 
> There doesn't seem to be a precedent for arch specific attributes under
> /sys/bus/pci so I went with a separate group for the RFC.

Why?  There's nothing arch-specific here, right?  Either the file is
present or not.

> "zpci" since that's what we've been calling the s390 specific PCI.
> I'm not attached to that name or having a separate group, from
> my perspective /sys/bus/pci/unique_uids would actually be ideal
> if Bjorn is okay with that, we don't currently foresee any additional
> global attributes and no one else seems to have them but
> one never knows and a separate group would then of course,
> well group them.

Why not just a simple file, no need for arch-specific names if this
really can show up under other arches?

> >> +Date:		February 2021
> >> +Contact:	Niklas Schnelle <schnelle@linux.ibm.com>
> >> +Description:
> >> +		This attribute exposes the global state of UID Uniqueness on an
> >> +		s390 Linux system. If this file contains '1' the per-device UID
> >> +		attribute is guaranteed to provide a unique user defined
> >> +		identifier for that PCI device. If this file contains '0' UIDs
> >> +		may collide and do not provide a unique identifier.
> > 
> > What are they "colliding" with?  And where does the UID come from, the
> > device itself or somewhere else?
> 
> If this attribute is 0 multiple PCI devices seen by Linux may have the same UID i.e.
> they may collide with each other on the same Linux instance.

So can't userspace figure this out on its own?

> The
> UIDs are exposed under /sys/bus/pci/devices/<dev>/uid. Even if the attribute is 1
> multiple Linux instances will often see the same UID. The main use case
> we currently envision is naming PCI based network interfaces "eno<UID>"
> which of course only works if the UIDs are unique for that Linux.
> On the same mainframe multiple Linux instances may then e.g. use the same
> UID for VFs from the same physical PCI network card or different cards
> but the same physical network all defined by an administrator/management
> system.
> 
> The UIDs come from the platform/firmware/hypervisor and are provided
> for each device by the CLP List PCI Functions "instruction" that is used
> on s390x where an OS can't probe the physical PCI bus but instead
> that is done by firmware. On QEMU/KVM they can be set on the QEMU cli,
> on our machine hypervisor they are defined by the machine administrator/management
> software as part of the definition of VMs/Partitions on that mainframe which includes
> everything from the number of CPUs, memory, I/O devices etc. With the exposed UID uniqueness
> attribute the platform then certifies that it will ensure that a UID is set to
> a unique non-zero value. I can of course add more of this explanation
> in the documentation too.

Please explain it more, but why would userspace care about this?  If
userspace sees a UID "collision" then it just adds something else to the
end of the name to make it unique.

What is it supposed to do differently based on the value/presense of
this file?

> Both the uniqueness guarantee (this attribute) as well as the UIDs themselves
> are part of the Z (s390x) architecture so fall under the mainframe typical
> backwards compatibility considerations.

So what can userspace do with this?  What tool is going to rely on this?

thanks,

greg k-h

Re: [RFC v2 1/1] PCI: Add s390 specific UID uniqueness attribute

[Niklas Schnelle]

On 2/4/21 2:38 PM, Greg Kroah-Hartman wrote:
> On Thu, Feb 04, 2021 at 01:02:51PM +0100, Niklas Schnelle wrote:
>>
>>
>> On 2/4/21 11:40 AM, Greg Kroah-Hartman wrote:
>>> On Thu, Feb 04, 2021 at 10:43:53AM +0100, Niklas Schnelle wrote:
>>>> The global UID uniqueness attribute exposes whether the platform
>>>> guarantees that the user-defined per-device UID attribute values
>>>> (/sys/bus/pci/device/<dev>/uid) are unique and can thus be used as
>>>> a global identifier for the associated PCI device. With this commit
>>>> it is exposed at /sys/bus/pci/zpci/unique_uids
>>>>
>>>> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
>>>> ---
>>>>  Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
>>>>  drivers/pci/pci-sysfs.c                 | 21 +++++++++++++++++++++
>>>>  2 files changed, 30 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
>>>> index 25c9c39770c6..812dd9d3f80d 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-pci
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-pci
>>>> @@ -375,3 +375,12 @@ Description:
>>>>  		The value comes from the PCI kernel device state and can be one
>>>>  		of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold".
>>>>  		The file is read only.
>>>> +What:		/sys/bus/pci/zpci/unique_uids
>>>
>>> No blank line before this new line?
>>
>> Good catch, thanks!
>>
>>>
>>> And why "zpci"?
>>
>> There doesn't seem to be a precedent for arch specific attributes under
>> /sys/bus/pci so I went with a separate group for the RFC.
> 
> Why?  There's nothing arch-specific here, right?  Either the file is
> present or not.

Good point, will change this to /sys/bus/pci/unique_uids

> 
>> "zpci" since that's what we've been calling the s390 specific PCI.
>> I'm not attached to that name or having a separate group, from
>> my perspective /sys/bus/pci/unique_uids would actually be ideal
>> if Bjorn is okay with that, we don't currently foresee any additional
>> global attributes and no one else seems to have them but
>> one never knows and a separate group would then of course,
>> well group them.
> 
> Why not just a simple file, no need for arch-specific names if this
> really can show up under other arches?

I guess other arches would be free to implement the same UID concept
as above I'm happy to change this to a simple file.

> 
>>>> +Date:		February 2021
>>>> +Contact:	Niklas Schnelle <schnelle@linux.ibm.com>
>>>> +Description:
>>>> +		This attribute exposes the global state of UID Uniqueness on an
>>>> +		s390 Linux system. If this file contains '1' the per-device UID
>>>> +		attribute is guaranteed to provide a unique user defined
>>>> +		identifier for that PCI device. If this file contains '0' UIDs
>>>> +		may collide and do not provide a unique identifier.
>>>
>>> What are they "colliding" with?  And where does the UID come from, the
>>> device itself or somewhere else?
>>
>> If this attribute is 0 multiple PCI devices seen by Linux may have the same UID i.e.
>> they may collide with each other on the same Linux instance.
> 
> So can't userspace figure this out on its own?

Userspace can figure out that they do not currently collide but
without this attribute it doesn't know that the platform also guarantees
that no collision will be introduced by e.g. hotplug. We are however
pushing to have this always turned on so yes over time this attribute
will hopefully be always 1 but this will take years.

> 
>> The
>> UIDs are exposed under /sys/bus/pci/devices/<dev>/uid. Even if the attribute is 1
>> multiple Linux instances will often see the same UID. The main use case
>> we currently envision is naming PCI based network interfaces "eno<UID>"
>> which of course only works if the UIDs are unique for that Linux.
>> On the same mainframe multiple Linux instances may then e.g. use the same
>> UID for VFs from the same physical PCI network card or different cards
>> but the same physical network all defined by an administrator/management
>> system.
>>
>> The UIDs come from the platform/firmware/hypervisor and are provided
>> for each device by the CLP List PCI Functions "instruction" that is used
>> on s390x where an OS can't probe the physical PCI bus but instead
>> that is done by firmware. On QEMU/KVM they can be set on the QEMU cli,
>> on our machine hypervisor they are defined by the machine administrator/management
>> software as part of the definition of VMs/Partitions on that mainframe which includes
>> everything from the number of CPUs, memory, I/O devices etc. With the exposed UID uniqueness
>> attribute the platform then certifies that it will ensure that a UID is set to
>> a unique non-zero value. I can of course add more of this explanation
>> in the documentation too.
> 
> Please explain it more, but why would userspace care about this?  If
> userspace sees a UID "collision" then it just adds something else to the
> end of the name to make it unique.

The idea is that userspace can be sure that no collisions will ever
happen. Consequently we can tell users that if they have UID uniqueness
checking on in their partition definition and they add a PCIe NIC with UID
X the network device will be named "enoX". This is particularly important
since during installation users might have to specify the interface name
as part of the installer's boot parameters _before_ Linux even boots.
Note that in our usual environments a lot of customers do not use things
like DHCP and might have very strict rules which interfaces will be
used for what. Similarly this would allow users to know exact interface
names when specifying e.g a Cloud Init configuration.

> 
> What is it supposed to do differently based on the value/presense of
> this file?

When UIDs are not guaranteed to be unique they are often unset that
is 0 and then the most stable identifier for a PCI function that a user
can know before even booting Linux is /sys/bus/pci/function_id.
This Function ID (FID) is also used as the hotplug slot
number and thus the X in "ensX.." interface names.
Thus, this would instead be the preferred interface naming scheme that
still allows the user to relate the interface names inside Linux
to what they can see in their machine/hypervisor configuration.

It should be noted that in our environments often the Linux admin(s)
will be a different group with very different background then the machine/mainframe
admin and the FID and UID are ofent the only per device information both
sides can see to identify an interface. Currently not even
the MAC address can be seen from the machine configuration or
even when listing devices in the z/VM hypervisor so we really want to use
either the FID or the UID in interface names.

In the future similar considerations may apply to other PCI devices.

> 
>> Both the uniqueness guarantee (this attribute) as well as the UIDs themselves
>> are part of the Z (s390x) architecture so fall under the mainframe typical
>> backwards compatibility considerations.
> 
> So what can userspace do with this?  What tool is going to rely on this?

We have a systemd/udev patch waiting on this RFC discussion
that will use this attribute for the aforementioned decision of either
preferring the existing "ens<hotplug_slot_numnber>…" or "eno<UID>…"
interface names based on this value.

As I mentioned in the cover letter we are also
considering instead going for /sys/firmware/.. but as far as
I can see that currently really needs much uglier raw kobject handling and
would lose the direct relation to PCI and creates the question
why PCI code puts stuff in /sys/firmware/… or alternatively which
other code deals with a PCI specific value.

> 
> thanks,
> 
> greg k-h
>