linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/1] s390/pci: expose a PCI device's UID as its index
@ 2021-03-03  9:52 Niklas Schnelle
  2021-03-03  9:52 ` [RFC 1/1] " Niklas Schnelle
  0 siblings, 1 reply; 5+ messages in thread
From: Niklas Schnelle @ 2021-03-03  9:52 UTC (permalink / raw)
  To: Bjorn Helgaas, Narendra K, Greg Kroah-Hartman
  Cc: Viktor Mihajlovski, Stefan Raspl, Peter Oberparleiter, linux-pci,
	linux-kernel, linux-s390

Hi,

On s390 each PCI device has a user-defined ID (UID).  This ID was
designed to serve as the PCI device's primary index and to match the
device within Linux to the with the view in the hypervisor
configuration. 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 which is also currently used to determine whether the
UID is also used as the Domain part of the geographical PCI address of
the device.

As primary identifier of a PCI device, the UID serves an analogous
function as the SMBIOS instance number or ACPI index exposed as the
"index" respectively "acpi_index" device attributes. These attributes
are used by e.g. systemd/udev to set network interface names. As s390
does not use  ACPI nor SMBIOS there is no conflict and we can expose the
UID under the "index" attribute whenever UID Uniqueness Checking is
active and systemd/udev will then create "eno<UID>.." interface names.

Note: This is an evolution of an earlier patch I sent for exposing the
UID Uniqueness Checking flag directly. Thank you Greg for making me
realize that we were looking too much at just exposing platform details
instead of looking how existing interfaces could suit our purpose.

Thanks,
Niklas Schnelle

Niklas Schnelle (1):
  s390/pci: expose a PCI device's UID as its index

 Documentation/ABI/testing/sysfs-bus-pci | 11 +++++---
 arch/s390/pci/pci_sysfs.c               | 36 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 4 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC 1/1] s390/pci: expose a PCI device's UID as its index
  2021-03-03  9:52 [RFC 0/1] s390/pci: expose a PCI device's UID as its index Niklas Schnelle
@ 2021-03-03  9:52 ` Niklas Schnelle
  2021-03-07 20:46   ` Krzysztof Wilczyński
  0 siblings, 1 reply; 5+ messages in thread
From: Niklas Schnelle @ 2021-03-03  9:52 UTC (permalink / raw)
  To: Bjorn Helgaas, Narendra K, Greg Kroah-Hartman
  Cc: Viktor Mihajlovski, Stefan Raspl, Peter Oberparleiter, linux-pci,
	linux-kernel, linux-s390

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 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@linux.ibm.com>
Acked-by: Viktor Mihajlovski <mihajlov@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-bus-pci | 11 +++++---
 arch/s390/pci/pci_sysfs.c               | 36 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 25c9c39770c6..1241b6d11a52 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -195,10 +195,13 @@ What:		/sys/bus/pci/devices/.../index
 Date:		July 2010
 Contact:	Narendra K <narendra_k@dell.com>, linux-bugs@dell.com
 Description:
-		Reading this attribute will provide the firmware
-		given instance (SMBIOS type 41 device type instance) of the
-		PCI device. The attribute will be created only if the firmware
-		has given an instance number to the PCI device.
+		Reading this attribute will provide the firmware given instance
+		number of the PCI device.  Depending on the platform this can
+		be for example the SMBIOS type 41 device type instance or the
+		user-defined ID (UID) on s390. The attribute will be created
+		only if the firmware has given an instance number to the PCI
+		device and that number is guaranteed to uniquely identify the
+		device in the system.
 Users:
 		Userspace applications interested in knowing the
 		firmware assigned device type instance of the PCI
diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c
index 5c028bee91b9..30f48e8a1156 100644
--- a/arch/s390/pci/pci_sysfs.c
+++ b/arch/s390/pci/pci_sysfs.c
@@ -131,6 +131,38 @@ static ssize_t report_error_write(struct file *filp, struct kobject *kobj,
 }
 static BIN_ATTR(report_error, S_IWUSR, NULL, report_error_write, PAGE_SIZE);
 
+#ifndef CONFIG_DMI
+/* analogous to smbios index */
+static ssize_t index_show(struct device *dev,
+			  struct device_attribute *attr, char *buf)
+{
+	struct zpci_dev *zdev = to_zpci(to_pci_dev(dev));
+	u32 index = ~0;
+
+	if (zpci_unique_uid)
+		index = zdev->uid;
+
+	return sprintf(buf, "%u\n", index);
+}
+static DEVICE_ATTR_RO(index);
+
+static umode_t zpci_unique_uids(struct kobject *kobj,
+				struct attribute *attr, int n)
+{
+	return zpci_unique_uid ? attr->mode : 0;
+}
+
+static struct attribute *zpci_ident_attrs[] = {
+	&dev_attr_index.attr,
+	NULL,
+};
+
+static struct attribute_group zpci_ident_attr_group = {
+	.attrs = zpci_ident_attrs,
+	.is_visible = zpci_unique_uids,
+};
+#endif
+
 static struct bin_attribute *zpci_bin_attrs[] = {
 	&bin_attr_util_string,
 	&bin_attr_report_error,
@@ -150,6 +182,7 @@ static struct attribute *zpci_dev_attrs[] = {
 	&dev_attr_mio_enabled.attr,
 	NULL,
 };
+
 static struct attribute_group zpci_attr_group = {
 	.attrs = zpci_dev_attrs,
 	.bin_attrs = zpci_bin_attrs,
@@ -170,5 +203,8 @@ static struct attribute_group pfip_attr_group = {
 const struct attribute_group *zpci_attr_groups[] = {
 	&zpci_attr_group,
 	&pfip_attr_group,
+#ifndef CONFIG_DMI
+	&zpci_ident_attr_group,
+#endif
 	NULL,
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC 1/1] s390/pci: expose a PCI device's UID as its index
  2021-03-03  9:52 ` [RFC 1/1] " Niklas Schnelle
@ 2021-03-07 20:46   ` Krzysztof Wilczyński
  2021-03-08  7:02     ` Niklas Schnelle
  0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Wilczyński @ 2021-03-07 20:46 UTC (permalink / raw)
  To: Niklas Schnelle
  Cc: Bjorn Helgaas, Narendra K, Greg Kroah-Hartman,
	Viktor Mihajlovski, Stefan Raspl, Peter Oberparleiter, linux-pci,
	linux-kernel, linux-s390

Hi Niklas,

[...]
> +static ssize_t index_show(struct device *dev,
> +			  struct device_attribute *attr, char *buf)
> +{
> +	struct zpci_dev *zdev = to_zpci(to_pci_dev(dev));
> +	u32 index = ~0;
> +
> +	if (zpci_unique_uid)
> +		index = zdev->uid;
> +
> +	return sprintf(buf, "%u\n", index);
[...]

Would it be possible to use the new sysfs_emit() rather than sprintf()
even though the zpci_attr macro and still use mio_enabled_show() still
would use sprintf().  What do you think?

See https://www.kernel.org/doc/html/latest/filesystems/sysfs.html for
the changes in the internal API.

Krzysztof

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC 1/1] s390/pci: expose a PCI device's UID as its index
  2021-03-07 20:46   ` Krzysztof Wilczyński
@ 2021-03-08  7:02     ` Niklas Schnelle
  2021-03-08  8:42       ` Viktor Mihajlovski
  0 siblings, 1 reply; 5+ messages in thread
From: Niklas Schnelle @ 2021-03-08  7:02 UTC (permalink / raw)
  To: Krzysztof Wilczyński
  Cc: Bjorn Helgaas, Narendra K, Greg Kroah-Hartman,
	Viktor Mihajlovski, Stefan Raspl, Peter Oberparleiter, linux-pci,
	linux-kernel, linux-s390



On 3/7/21 9:46 PM, Krzysztof Wilczyński wrote:
> Hi Niklas,
> 
> [...]
>> +static ssize_t index_show(struct device *dev,
>> +			  struct device_attribute *attr, char *buf)
>> +{
>> +	struct zpci_dev *zdev = to_zpci(to_pci_dev(dev));
>> +	u32 index = ~0;
>> +
>> +	if (zpci_unique_uid)
>> +		index = zdev->uid;
>> +
>> +	return sprintf(buf, "%u\n", index);
> [...]
> 
> Would it be possible to use the new sysfs_emit() rather than sprintf()
> even though the zpci_attr macro and still use mio_enabled_show() still
> would use sprintf().  What do you think?
> 
> See https://www.kernel.org/doc/html/latest/filesystems/sysfs.html for
> the changes in the internal API.
> 
> Krzysztof
> 

Of course that makes sense and thanks for pointing me to this API!
@Viktor, may I carry your R-b over?

I'll also update the other attributes in a clean up patch.

Thanks,
Niklas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC 1/1] s390/pci: expose a PCI device's UID as its index
  2021-03-08  7:02     ` Niklas Schnelle
@ 2021-03-08  8:42       ` Viktor Mihajlovski
  0 siblings, 0 replies; 5+ messages in thread
From: Viktor Mihajlovski @ 2021-03-08  8:42 UTC (permalink / raw)
  To: Niklas Schnelle, Krzysztof Wilczyński
  Cc: Bjorn Helgaas, Narendra K, Greg Kroah-Hartman, Stefan Raspl,
	Peter Oberparleiter, linux-pci, linux-kernel, linux-s390



On 3/8/21 8:02 AM, Niklas Schnelle wrote:
> 
> 
> On 3/7/21 9:46 PM, Krzysztof Wilczyński wrote:
>> Hi Niklas,
>>
>> [...]
>>> +static ssize_t index_show(struct device *dev,
>>> +			  struct device_attribute *attr, char *buf)
>>> +{
>>> +	struct zpci_dev *zdev = to_zpci(to_pci_dev(dev));
>>> +	u32 index = ~0;
>>> +
>>> +	if (zpci_unique_uid)
>>> +		index = zdev->uid;
>>> +
>>> +	return sprintf(buf, "%u\n", index);
>> [...]
>>
>> Would it be possible to use the new sysfs_emit() rather than sprintf()
>> even though the zpci_attr macro and still use mio_enabled_show() still
>> would use sprintf().  What do you think?
>>
>> See https://www.kernel.org/doc/html/latest/filesystems/sysfs.html for
>> the changes in the internal API.
>>
>> Krzysztof
>>
> 
> Of course that makes sense and thanks for pointing me to this API!
> @Viktor, may I carry your R-b over?
> 
Sure, please go ahead.
> I'll also update the other attributes in a clean up patch.
> 
> Thanks,
> Niklas
> 

-- 
Kind Regards,
    Viktor

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-08  8:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03  9:52 [RFC 0/1] s390/pci: expose a PCI device's UID as its index Niklas Schnelle
2021-03-03  9:52 ` [RFC 1/1] " Niklas Schnelle
2021-03-07 20:46   ` Krzysztof Wilczyński
2021-03-08  7:02     ` Niklas Schnelle
2021-03-08  8:42       ` Viktor Mihajlovski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).