[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <67de8faa7eca891c7c39ae83540f74369de5b783.camel@linux.ibm.com>
Date: Fri, 26 Sep 2025 20:36:58 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: ramesh@...ux.ibm.com, Bjorn Helgaas <bhelgaas@...gle.com>,
Lukas Wunner
<lukas@...ner.de>
Cc: Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
Christian Borntraeger
<borntraeger@...ux.ibm.com>,
Gerd Bayer <gbayer@...ux.ibm.com>,
Matthew
Rosato <mjrosato@...ux.ibm.com>,
Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev
<agordeev@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Julian
Ruess <julianr@...ux.ibm.com>,
Peter Oberparleiter <oberpar@...ux.ibm.com>,
linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org
Subject: Re: [PATCH] PCI: s390: Expose the UID as an arch specific PCI slot
attribute
On Fri, 2025-09-26 at 11:34 -0500, Ramesh Errabolu wrote:
> On 9/24/2025 8:14 AM, Niklas Schnelle wrote:
>
> > On s390, an individual PCI function can generally be identified by two
> > IDs, depending on the scope and the platform configuration.
> It would help to name the two IDs - FID and ???
How about:
"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."
And then reword the below without "so-called".
> > The first ID is the so-called FID, which 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 reuse the same configuration based on FIDs on two
> > different LPARs.
> >
> > Such matching LPAR configurations are useful, though, allowing
> > standardized setups and booting a Linux installation on different
> > machines. To allow this, a second user-defined identifier called UID was
> > introduced. It is only guaranteed to be unique within an LPAR and only
> > if the platform indicates so via the UID Checking flag.
> The paragraph as I read is not clear. Your intention is to highlight the
> need for UID to allow standardized setups.
Yes, that was my intention. Also here is where the second ID is
introduced so I'll reword this a bit if the name is already mentioned
in the first paragraph.
> >
> > 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 slot number 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@...ux.ibm.com>
> > ---
> > Note: I considered adding the UID as a generic "index" via the hotplug
> > slot driver but opted for a minimal solution to open the discussion. In
> > particular my concern with a generic attribute is that it would be hard
> > to find a format that fits everyone. For example on PCI devices we also
> > use the "index" attribute for UIDs analogous to SMBIOS but having it in
> > decimal is odd on s390 where these are usual in hexadecimal.
> > ---
> > 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 41f900f693d92522ff729829e446b581977ef3ff..23eed78d9dce72ef466679f31c78aca52ba00f99 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;
> >
>
> Will this not lead to linking error when the patch is built on non-s390
> architecture. You could refer to zpci_slot_attr_group using a
> CONFIG_..... and discard the #define ARCH_PCI_SLOT_GROUPS. I didn't find
> a relevant CONFIG_... that could be used.
This code is in arch/s390/ it will not be build on non-s390. For the
non s390 case ARCH_PCI_SLOT_GROUPS will be undefined and the #ifdef in
slot.c makes sure we're not trying to insert ARCH_PCI_SLOT_GROUPs in
the array as it is not defined.
Thanks,
Niklas
Powered by blists - more mailing lists