[PATCH v3] PCI: s390: Expose the UID as an arch specific PCI slot attribute

Niklas Schnelle posted 1 patch 2 months ago
There is a newer version of this series
arch/s390/include/asm/pci.h |  4 ++++
arch/s390/pci/pci_sysfs.c   | 20 ++++++++++++++++++++
drivers/pci/slot.c          | 13 ++++++++++++-
3 files changed, 36 insertions(+), 1 deletion(-)
[PATCH v3] PCI: s390: Expose the UID as an arch specific PCI slot attribute
Posted by Niklas Schnelle 2 months ago
On s390, an individual PCI function can generally be identified by two
identifiers, the FID and the UID. Which identifier is used depends on
the scope and the platform configuration.

The first identifier, the FID, is always available and identifies a PCI
device uniquely within a machine. The FID may be virtualized by
hypervisors, but on the LPAR level, the machine scope makes it
impossible to create the same configuration based on FIDs on two
different LPARs of the same machine, and difficult to reuse across
machines.

Such matching LPAR configurations are useful, though, allowing
standardized setups and booting a Linux installation on different LPARs.
To this end the UID, or user-defined identifier, was introduced. While
it is only guaranteed to be unique within an LPAR and only if indicated
by firmware, it allows users to replicate PCI device setups.

On s390, which uses a machine hypervisor, a per PCI function hotplug
model is used. The shortcoming with the UID then is, that it is not
visible to the user without first attaching the PCI function and
accessing the "uid" device attribute. The FID, on the other hand, is
used as the slot name and is thus known even with the PCI function in
standby.

Remedy this shortcoming by providing the UID as an attribute on the slot
allowing the user to identify a PCI function based on the UID without
having to first attach it. Do this via a macro mechanism analogous to
what was introduced by commit 265baca69a07 ("s390/pci: Stop usurping
pdev->dev.groups") for the PCI device attributes.

Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Note: I considered adding the UID as a generic "index" via the hotplug
slot driver but felt like there is probably too little commonality on
format and usage patterns.

v2->v3:
- Rebase on v6.18-rc1 and resolve conflict with recent s390 PCI sysfs change
- Link to v2: https://lore.kernel.org/r/20251008-uid_slot-v2-1-ef22cef27741@linux.ibm.com
---
 arch/s390/include/asm/pci.h |  4 ++++
 arch/s390/pci/pci_sysfs.c   | 20 ++++++++++++++++++++
 drivers/pci/slot.c          | 13 ++++++++++++-
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index 6890925d5587b1825cc51ac7e2be2003132244da..ffe63be00484fc9b38ce2f2437cddcaced621b03 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -207,6 +207,10 @@ extern const struct attribute_group zpci_ident_attr_group;
 			    &pfip_attr_group,		 \
 			    &zpci_ident_attr_group,
 
+extern const struct attribute_group zpci_slot_attr_group;
+
+#define ARCH_PCI_SLOT_GROUPS (&zpci_slot_attr_group)
+
 extern unsigned int s390_pci_force_floating __initdata;
 extern unsigned int s390_pci_no_rid;
 
diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c
index 12060870e2aa167c58a690d584f0268b1347fbfe..f85e297a6e6ccdc841524ca2611a361012148a0a 100644
--- a/arch/s390/pci/pci_sysfs.c
+++ b/arch/s390/pci/pci_sysfs.c
@@ -188,6 +188,17 @@ static ssize_t index_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(index);
 
+static ssize_t zpci_uid_slot_show(struct pci_slot *slot, char *buf)
+{
+	struct zpci_dev *zdev = container_of(slot->hotplug, struct zpci_dev,
+					     hotplug_slot);
+
+	return sysfs_emit(buf, "0x%x\n", zdev->uid);
+}
+
+static struct pci_slot_attribute zpci_slot_attr_uid =
+	__ATTR(uid, 0444, zpci_uid_slot_show, NULL);
+
 static umode_t zpci_index_is_visible(struct kobject *kobj,
 				     struct attribute *attr, int n)
 {
@@ -244,6 +255,15 @@ const struct attribute_group pfip_attr_group = {
 	.attrs = pfip_attrs,
 };
 
+static struct attribute *zpci_slot_attrs[] = {
+	&zpci_slot_attr_uid.attr,
+	NULL,
+};
+
+const struct attribute_group zpci_slot_attr_group = {
+	.attrs = zpci_slot_attrs,
+};
+
 static struct attribute *clp_fw_attrs[] = {
 	&uid_checking_attr.attr,
 	NULL,
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index 50fb3eb595fe65e271b6b339d43c9677c61b1e45..b09e7852c33ed4957432ac73b36d181ecd8283a1 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -96,7 +96,18 @@ static struct attribute *pci_slot_default_attrs[] = {
 	&pci_slot_attr_cur_speed.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(pci_slot_default);
+
+static const struct attribute_group pci_slot_default_group = {
+	.attrs = pci_slot_default_attrs,
+};
+
+static const struct attribute_group *pci_slot_default_groups[] = {
+	&pci_slot_default_group,
+#ifdef ARCH_PCI_SLOT_GROUPS
+	ARCH_PCI_SLOT_GROUPS,
+#endif
+	NULL,
+};
 
 static const struct kobj_type pci_slot_ktype = {
 	.sysfs_ops = &pci_slot_sysfs_ops,

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20250923-uid_slot-e3559cf5ca30

Best regards,
-- 
Niklas Schnelle
Re: [PATCH v3] PCI: s390: Expose the UID as an arch specific PCI slot attribute
Posted by Gerd Bayer 2 weeks, 1 day ago
On Wed, 2025-10-15 at 15:42 +0200, Niklas Schnelle wrote:
> On s390, an individual PCI function can generally be identified by two
> identifiers, the FID and the UID. Which identifier is used depends on
> the scope and the platform configuration.
> 
> The first identifier, the FID, is always available and identifies a PCI
> device uniquely within a machine. The FID may be virtualized by
> hypervisors, but on the LPAR level, the machine scope makes it
> impossible to create the same configuration based on FIDs on two
> different LPARs of the same machine, and difficult to reuse across
> machines.
> 
> Such matching LPAR configurations are useful, though, allowing
> standardized setups and booting a Linux installation on different LPARs.
> To this end the UID, or user-defined identifier, was introduced. While
> it is only guaranteed to be unique within an LPAR and only if indicated
> by firmware, it allows users to replicate PCI device setups.
> 
> On s390, which uses a machine hypervisor, a per PCI function hotplug
> model is used. The shortcoming with the UID then is, that it is not
> visible to the user without first attaching the PCI function and
> accessing the "uid" device attribute. The FID, on the other hand, is
> used as the slot name and is thus known even with the PCI function in
> standby.
> 
> Remedy this shortcoming by providing the UID as an attribute on the slot
> allowing the user to identify a PCI function based on the UID without
> having to first attach it. Do this via a macro mechanism analogous to
> what was introduced by commit 265baca69a07 ("s390/pci: Stop usurping
> pdev->dev.groups") for the PCI device attributes.

Hi Niklas,

I like this addition a lot. Also, Lukas' method to add arch-specific
attributes to sysfs. Is there a reason why you didn't apply that
mechanism 1-to-1?

> 
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
> Note: I considered adding the UID as a generic "index" via the hotplug
> slot driver but felt like there is probably too little commonality on
> format and usage patterns

Sorry for my ignorance but how is the hotplug slot driver defining an
"index" or how is used?


> v2->v3:
> - Rebase on v6.18-rc1 and resolve conflict with recent s390 PCI sysfs change
> - Link to v2: https://lore.kernel.org/r/20251008-uid_slot-v2-1-ef22cef27741@linux.ibm.com
> ---
>  arch/s390/include/asm/pci.h |  4 ++++
>  arch/s390/pci/pci_sysfs.c   | 20 ++++++++++++++++++++
>  drivers/pci/slot.c          | 13 ++++++++++++-
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 

  [ ... snip ... ]

> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 50fb3eb595fe65e271b6b339d43c9677c61b1e45..b09e7852c33ed4957432ac73b36d181ecd8283a1 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
> @@ -96,7 +96,18 @@ static struct attribute *pci_slot_default_attrs[] = {
>  	&pci_slot_attr_cur_speed.attr,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(pci_slot_default);
> +
> +static const struct attribute_group pci_slot_default_group = {
> +	.attrs = pci_slot_default_attrs,
> +};
> +
> +static const struct attribute_group *pci_slot_default_groups[] = {
> +	&pci_slot_default_group,
> +#ifdef ARCH_PCI_SLOT_GROUPS
> +	ARCH_PCI_SLOT_GROUPS,
> +#endif
> +	NULL,
> +};
>  

With the following diff you could avoid the #ifdef directive in the
middle of the definition of the attribute_group - shamelessly stolen
from Lukas' patch


diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index 8e5e0f995e91..1d8105c4d2a6 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -208,7 +208,7 @@ extern const struct attribute_group
zpci_ident_attr_group;
 
 extern const struct attribute_group zpci_slot_attr_group;
 
-#define ARCH_PCI_SLOT_GROUPS (&zpci_slot_attr_group)
+#define ARCH_PCI_SLOT_GROUPS &zpci_slot_attr_group,
 
 extern unsigned int s390_pci_force_floating __initdata;
 extern unsigned int s390_pci_no_rid;
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index b09e7852c33e..9ba7fc0066bf 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -11,6 +11,10 @@
 #include <linux/err.h>
 #include "pci.h"
 
+#ifndef ARCH_PCI_SLOT_GROUPS
+#define ARCH_PCI_SLOT_GROUPS
+#endif
+
 struct kset *pci_slots_kset;
 EXPORT_SYMBOL_GPL(pci_slots_kset);
 
@@ -103,9 +107,7 @@ static const struct attribute_group
pci_slot_default_group = {
 
 static const struct attribute_group *pci_slot_default_groups[] = {
        &pci_slot_default_group,
-#ifdef ARCH_PCI_SLOT_GROUPS
-       ARCH_PCI_SLOT_GROUPS,
-#endif
+       ARCH_PCI_SLOT_GROUPS
        NULL,
 };

Thanks,
Gerd
Re: [PATCH v3] PCI: s390: Expose the UID as an arch specific PCI slot attribute
Posted by Niklas Schnelle 2 weeks, 1 day ago
On Thu, 2025-12-04 at 13:45 +0100, Gerd Bayer wrote:
> On Wed, 2025-10-15 at 15:42 +0200, Niklas Schnelle wrote:
> > On s390, an individual PCI function can generally be identified by two
> > identifiers, the FID and the UID. Which identifier is used depends on
> > the scope and the platform configuration.
> > 
> > The first identifier, the FID, is always available and identifies a PCI
> > device uniquely within a machine. The FID may be virtualized by
> > hypervisors, but on the LPAR level, the machine scope makes it
> > impossible to create the same configuration based on FIDs on two
> > different LPARs of the same machine, and difficult to reuse across
> > machines.
> > 
> > Such matching LPAR configurations are useful, though, allowing
> > standardized setups and booting a Linux installation on different LPARs.
> > To this end the UID, or user-defined identifier, was introduced. While
> > it is only guaranteed to be unique within an LPAR and only if indicated
> > by firmware, it allows users to replicate PCI device setups.
> > 
> > On s390, which uses a machine hypervisor, a per PCI function hotplug
> > model is used. The shortcoming with the UID then is, that it is not
> > visible to the user without first attaching the PCI function and
> > accessing the "uid" device attribute. The FID, on the other hand, is
> > used as the slot name and is thus known even with the PCI function in
> > standby.
> > 
> > Remedy this shortcoming by providing the UID as an attribute on the slot
> > allowing the user to identify a PCI function based on the UID without
> > having to first attach it. Do this via a macro mechanism analogous to
> > what was introduced by commit 265baca69a07 ("s390/pci: Stop usurping
> > pdev->dev.groups") for the PCI device attributes.
> 
> Hi Niklas,
> 
> I like this addition a lot. Also, Lukas' method to add arch-specific
> attributes to sysfs. Is there a reason why you didn't apply that
> mechanism 1-to-1?

I considered that and actually had basically the same as your diff in
an intermediate version. To me it looked cleaner with the #ifdef in the
attribute_group array initialization as I feel that it is more obvious
if the macroed elements are used. This may be because my editor grays
out the code between unused #ifdef or just because "it's in one place"
either way just a matter of taste. Also the comma gave me checkpatch
complaints.

> 
> > 
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> > Note: I considered adding the UID as a generic "index" via the hotplug
> > slot driver but felt like there is probably too little commonality on
> > format and usage patterns
> 
> Sorry for my ignorance but how is the hotplug slot driver defining an
> "index" or how is used?
> 

It's not. I meant that I considered that the UID could be turned into a
generic "index" attribute in the same way that the UID is used as
/sys/bus/pci/devices/<dev>/index on s390 while the same attribute is an
SMBIOS index thing on x86 and others.
Re: [PATCH v3] PCI: s390: Expose the UID as an arch specific PCI slot attribute
Posted by Gerd Bayer 2 weeks, 1 day ago
On Thu, 2025-12-04 at 14:09 +0100, Niklas Schnelle wrote:
> On Thu, 2025-12-04 at 13:45 +0100, Gerd Bayer wrote:
> > On Wed, 2025-10-15 at 15:42 +0200, Niklas Schnelle wrote:
> > > On s390, an individual PCI function can generally be identified by two
> > > identifiers, the FID and the UID. Which identifier is used depends on
> > > the scope and the platform configuration.
> > > 
> > > The first identifier, the FID, is always available and identifies a PCI
> > > device uniquely within a machine. The FID may be virtualized by
> > > hypervisors, but on the LPAR level, the machine scope makes it
> > > impossible to create the same configuration based on FIDs on two
> > > different LPARs of the same machine, and difficult to reuse across
> > > machines.
> > > 
> > > Such matching LPAR configurations are useful, though, allowing
> > > standardized setups and booting a Linux installation on different LPARs.
> > > To this end the UID, or user-defined identifier, was introduced. While
> > > it is only guaranteed to be unique within an LPAR and only if indicated
> > > by firmware, it allows users to replicate PCI device setups.
> > > 
> > > On s390, which uses a machine hypervisor, a per PCI function hotplug
> > > model is used. The shortcoming with the UID then is, that it is not
> > > visible to the user without first attaching the PCI function and
> > > accessing the "uid" device attribute. The FID, on the other hand, is
> > > used as the slot name and is thus known even with the PCI function in
> > > standby.
> > > 
> > > Remedy this shortcoming by providing the UID as an attribute on the slot
> > > allowing the user to identify a PCI function based on the UID without
> > > having to first attach it. Do this via a macro mechanism analogous to
> > > what was introduced by commit 265baca69a07 ("s390/pci: Stop usurping
> > > pdev->dev.groups") for the PCI device attributes.
> > 
> > Hi Niklas,
> > 
> > I like this addition a lot. Also, Lukas' method to add arch-specific
> > attributes to sysfs. Is there a reason why you didn't apply that
> > mechanism 1-to-1?
> 
> I considered that and actually had basically the same as your diff in
> an intermediate version. To me it looked cleaner with the #ifdef in the
> attribute_group array initialization as I feel that it is more obvious
> if the macroed elements are used. This may be because my editor grays
> out the code between unused #ifdef or just because "it's in one place"
> either way just a matter of taste. Also the comma gave me checkpatch
> complaints.

I agree, for me it's more a question of taste than anything else. If
there's a preference towards commonality, Lukas or Bjorn may want to
chime in.


> > 
> > > 
> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > ---
> > > Note: I considered adding the UID as a generic "index" via the hotplug
> > > slot driver but felt like there is probably too little commonality on
> > > format and usage patterns
> > 
> > Sorry for my ignorance but how is the hotplug slot driver defining an
> > "index" or how is used?
> > 
> 
> It's not. I meant that I considered that the UID could be turned into a
> generic "index" attribute in the same way that the UID is used as
> /sys/bus/pci/devices/<dev>/index on s390 while the same attribute is an
> SMBIOS index thing on x86 and others.

I see and agree, "index" by itself lacks the definition of a
"dimension" on which you mark individual slots.

Having no further comments, feel free to add my
Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>

Thanks,
Gerd
Re: [PATCH v3] PCI: s390: Expose the UID as an arch specific PCI slot attribute
Posted by Julian Ruess 2 weeks, 1 day ago
On Wed Oct 15, 2025 at 3:42 PM CEST, Niklas Schnelle wrote:
> On s390, an individual PCI function can generally be identified by two
> identifiers, the FID and the UID. Which identifier is used depends on
> the scope and the platform configuration.
>
> The first identifier, the FID, is always available and identifies a PCI
> device uniquely within a machine. The FID may be virtualized by
> hypervisors, but on the LPAR level, the machine scope makes it
> impossible to create the same configuration based on FIDs on two
> different LPARs of the same machine, and difficult to reuse across
> machines.
>
> Such matching LPAR configurations are useful, though, allowing
> standardized setups and booting a Linux installation on different LPARs.
> To this end the UID, or user-defined identifier, was introduced. While
> it is only guaranteed to be unique within an LPAR and only if indicated
> by firmware, it allows users to replicate PCI device setups.
>
> On s390, which uses a machine hypervisor, a per PCI function hotplug
> model is used. The shortcoming with the UID then is, that it is not
> visible to the user without first attaching the PCI function and
> accessing the "uid" device attribute. The FID, on the other hand, is
> used as the slot name and is thus known even with the PCI function in
> standby.
>
> Remedy this shortcoming by providing the UID as an attribute on the slot
> allowing the user to identify a PCI function based on the UID without
> having to first attach it. Do this via a macro mechanism analogous to
> what was introduced by commit 265baca69a07 ("s390/pci: Stop usurping
> pdev->dev.groups") for the PCI device attributes.
>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
> Note: I considered adding the UID as a generic "index" via the hotplug
> slot driver but felt like there is probably too little commonality on
> format and usage patterns.
>
> v2->v3:
> - Rebase on v6.18-rc1 and resolve conflict with recent s390 PCI sysfs change
> - Link to v2: https://lore.kernel.org/r/20251008-uid_slot-v2-1-ef22cef27741@linux.ibm.com
> ---

-- snip --

Feel free to add my
Reviewed-by: Julian Ruess <julianr@linux.ibm.com>

Thanks,
Julian