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

Gerd Bayer
2025-12-04T13:32:45Z

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