[RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Matthew Wood]

Add a single sysfs read-only interface for reading PCIe device serial
numbers from userspace in a programmatic way. This device attribute
uses the same hexadecimal 1-byte dashed formatting as lspci serial number
capability output. If a device doesn't support the serial number
capability, the serial_number sysfs attribute will not be visible.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
Reviewed-by: Mario Limonciello <superm1@kernel.org>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
 drivers/pci/pci-sysfs.c                 | 27 ++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 69f952fffec7..d5251f4f3659 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -612,3 +612,12 @@ Description:
 
 		  # ls doe_features
 		  0001:01        0001:02        doe_discovery
+
+What:		/sys/bus/pci/devices/.../serial_number
+Date:		December 2025
+Contact:	Matthew Wood <thepacketgeek@gmail.com>
+Description:
+		This is visible only for PCIe devices that support the serial
+		number extended capability. The file is read only and due to
+		the possible sensitivity of accessible serial numbers, admin
+		only.
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 268c69daa4d5..1d26e4336f1b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -30,6 +30,7 @@
 #include <linux/msi.h>
 #include <linux/of.h>
 #include <linux/aperture.h>
+#include <linux/unaligned.h>
 #include "pci.h"
 
 #ifndef ARCH_PCI_DEV_GROUPS
@@ -239,6 +240,22 @@ static ssize_t current_link_width_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(current_link_width);
 
+static ssize_t serial_number_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pci_dev = to_pci_dev(dev);
+	u64 dsn;
+	u8 bytes[8];
+
+	dsn = pci_get_dsn(pci_dev);
+	if (!dsn)
+		return -EIO;
+	put_unaligned_be64(dsn, bytes);
+
+	return sysfs_emit(buf, "%8phD\n", bytes);
+}
+static DEVICE_ATTR_ADMIN_RO(serial_number);
+
 static ssize_t secondary_bus_number_show(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
@@ -660,6 +677,7 @@ static struct attribute *pcie_dev_attrs[] = {
 	&dev_attr_current_link_width.attr,
 	&dev_attr_max_link_width.attr,
 	&dev_attr_max_link_speed.attr,
+	&dev_attr_serial_number.attr,
 	NULL,
 };
 
@@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
 	struct device *dev = kobj_to_dev(kobj);
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	if (pci_is_pcie(pdev))
-		return a->mode;
+	if (!pci_is_pcie(pdev))
+		return 0;
 
-	return 0;
+	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
+		return 0;
+
+	return a->mode;
 }
 
 static const struct attribute_group pci_dev_group = {
-- 
2.50.1

[RESEND PATCH v7 0/1] PCI/sysfs: Expose PCIe device serial number

[Matthew Wood]

Add a single sysfs read-only, admin-only interface for reading PCIe device
serial numbers from userspace, using the same hexadecimal 1-byte dashed
formatting as lspci serial number capability output:

    sudo cat /sys/devices/pci0000:c0/0000:c0:01.1/0000:c1:00.0/0000:c2:1f.0/0000:ef:00.0/serial_number
    00-80-ee-00-00-00-41-80

If a device doesn't support the serial number capability, the
serial_number sysfs attribute will not be visible.

Comparing serial number format to lspci output:

    sudo lspci -vvv -s ef:00.0
        ef:00.0 Serial Attached SCSI controller: Broadcom / LSI PCIe Switch management endpoint (rev b0)
            Subsystem: Broadcom / LSI Device 0144
            ...
            Capabilities: [100 v1] Device Serial Number 00-80-ee-00-00-00-41-80
            ...

We have a specific use-case of needing to read the serial number
from an unpriviliged application; this patch exposes the serial number as a
file which means we can change the permissions from an admin context and
allow the unpriviliged userspace app to read the serial number. Otherwise
the serial number cannot be read as it requires both the admin capability
and parsing of the lspci text output which can be fragile and unreliable.


v7:
  Updated docs to change kernel introduction date to December 2025 (6.18)

Matthew Wood (1):
  PCI/sysfs: Expose PCIe device serial number

 Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
 drivers/pci/pci-sysfs.c                 | 27 ++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

-- 
2.50.1

[RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Matthew Wood]

Add a single sysfs read-only interface for reading PCIe device serial
numbers from userspace in a programmatic way. This device attribute
uses the same hexadecimal 1-byte dashed formatting as lspci serial number
capability output. If a device doesn't support the serial number
capability, the serial_number sysfs attribute will not be visible.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
Reviewed-by: Mario Limonciello <superm1@kernel.org>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
 drivers/pci/pci-sysfs.c                 | 27 ++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 69f952fffec7..d5251f4f3659 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -612,3 +612,12 @@ Description:
 
 		  # ls doe_features
 		  0001:01        0001:02        doe_discovery
+
+What:		/sys/bus/pci/devices/.../serial_number
+Date:		December 2025
+Contact:	Matthew Wood <thepacketgeek@gmail.com>
+Description:
+		This is visible only for PCIe devices that support the serial
+		number extended capability. The file is read only and due to
+		the possible sensitivity of accessible serial numbers, admin
+		only.
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 268c69daa4d5..1d26e4336f1b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -30,6 +30,7 @@
 #include <linux/msi.h>
 #include <linux/of.h>
 #include <linux/aperture.h>
+#include <linux/unaligned.h>
 #include "pci.h"
 
 #ifndef ARCH_PCI_DEV_GROUPS
@@ -239,6 +240,22 @@ static ssize_t current_link_width_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(current_link_width);
 
+static ssize_t serial_number_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pci_dev = to_pci_dev(dev);
+	u64 dsn;
+	u8 bytes[8];
+
+	dsn = pci_get_dsn(pci_dev);
+	if (!dsn)
+		return -EIO;
+	put_unaligned_be64(dsn, bytes);
+
+	return sysfs_emit(buf, "%8phD\n", bytes);
+}
+static DEVICE_ATTR_ADMIN_RO(serial_number);
+
 static ssize_t secondary_bus_number_show(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
@@ -660,6 +677,7 @@ static struct attribute *pcie_dev_attrs[] = {
 	&dev_attr_current_link_width.attr,
 	&dev_attr_max_link_width.attr,
 	&dev_attr_max_link_speed.attr,
+	&dev_attr_serial_number.attr,
 	NULL,
 };
 
@@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
 	struct device *dev = kobj_to_dev(kobj);
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	if (pci_is_pcie(pdev))
-		return a->mode;
+	if (!pci_is_pcie(pdev))
+		return 0;
 
-	return 0;
+	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
+		return 0;
+
+	return a->mode;
 }
 
 static const struct attribute_group pci_dev_group = {
-- 
2.50.1

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Keith Busch]

On Thu, Aug 21, 2025 at 04:22:38PM -0700, Matthew Wood wrote:
> Add a single sysfs read-only interface for reading PCIe device serial
> numbers from userspace in a programmatic way. This device attribute
> uses the same hexadecimal 1-byte dashed formatting as lspci serial number
> capability output. If a device doesn't support the serial number
> capability, the serial_number sysfs attribute will not be visible.

Looks good to me.

Reviewed-by: Keith Busch <kbusch@kernel.org>

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Hans Zhang]

On 2025/8/22 07:22, Matthew Wood wrote:
> EXTERNAL EMAIL
> 
> Add a single sysfs read-only interface for reading PCIe device serial
> numbers from userspace in a programmatic way. This device attribute
> uses the same hexadecimal 1-byte dashed formatting as lspci serial number
> capability output. If a device doesn't support the serial number
> capability, the serial_number sysfs attribute will not be visible.
> 
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
> Reviewed-by: Mario Limonciello <superm1@kernel.org>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Tested-by: Hans Zhang <hans.zhang@cixtech.com>

Best regards,
Hans

> ---
>   Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
>   drivers/pci/pci-sysfs.c                 | 27 ++++++++++++++++++++++---
>   2 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 69f952fffec7..d5251f4f3659 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -612,3 +612,12 @@ Description:
> 
>                    # ls doe_features
>                    0001:01        0001:02        doe_discovery
> +
> +What:          /sys/bus/pci/devices/.../serial_number
> +Date:          December 2025
> +Contact:       Matthew Wood <thepacketgeek@gmail.com>
> +Description:
> +               This is visible only for PCIe devices that support the serial
> +               number extended capability. The file is read only and due to
> +               the possible sensitivity of accessible serial numbers, admin
> +               only.
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 268c69daa4d5..1d26e4336f1b 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -30,6 +30,7 @@
>   #include <linux/msi.h>
>   #include <linux/of.h>
>   #include <linux/aperture.h>
> +#include <linux/unaligned.h>
>   #include "pci.h"
> 
>   #ifndef ARCH_PCI_DEV_GROUPS
> @@ -239,6 +240,22 @@ static ssize_t current_link_width_show(struct device *dev,
>   }
>   static DEVICE_ATTR_RO(current_link_width);
> 
> +static ssize_t serial_number_show(struct device *dev,
> +                                 struct device_attribute *attr, char *buf)
> +{
> +       struct pci_dev *pci_dev = to_pci_dev(dev);
> +       u64 dsn;
> +       u8 bytes[8];
> +
> +       dsn = pci_get_dsn(pci_dev);
> +       if (!dsn)
> +               return -EIO;
> +       put_unaligned_be64(dsn, bytes);
> +
> +       return sysfs_emit(buf, "%8phD\n", bytes);
> +}
> +static DEVICE_ATTR_ADMIN_RO(serial_number);
> +
>   static ssize_t secondary_bus_number_show(struct device *dev,
>                                           struct device_attribute *attr,
>                                           char *buf)
> @@ -660,6 +677,7 @@ static struct attribute *pcie_dev_attrs[] = {
>          &dev_attr_current_link_width.attr,
>          &dev_attr_max_link_width.attr,
>          &dev_attr_max_link_speed.attr,
> +       &dev_attr_serial_number.attr,
>          NULL,
>   };
> 
> @@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
>          struct device *dev = kobj_to_dev(kobj);
>          struct pci_dev *pdev = to_pci_dev(dev);
> 
> -       if (pci_is_pcie(pdev))
> -               return a->mode;
> +       if (!pci_is_pcie(pdev))
> +               return 0;
> 
> -       return 0;
> +       if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
> +               return 0;
> +
> +       return a->mode;
>   }
> 
>   static const struct attribute_group pci_dev_group = {
> --
> 2.50.1
> 
> 

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Keith Busch]

Hi Bjorn,

Can we get a ruling on this one? It's pretty straight forward
implementation exposing a useful attribute. This patch has got a good
amount of reviews and tests. Any objections?

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Keith Busch]

On Wed, Aug 27, 2025 at 01:39:41PM -0600, Keith Busch wrote:
> Can we get a ruling on this one? It's pretty straight forward
> implementation exposing a useful attribute. This patch has got a good
> amount of reviews and tests. Any objections?

I guess silence means acceptance. Unless I hear otherwise, I'll queue
this up in my tree to be sent out for the next merge window.

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Krzysztof Wilczyński]

Hello,

> Can we get a ruling on this one? It's pretty straight forward
> implementation exposing a useful attribute.

Who needs this?  Why is this useful?  Why hasn't there been a need for
exposing serial number in past decades, and suddenly we need it so
desperately?

We probably wouldn't want to add this if there is only a single user that
needs this, especially give that userspace tools like lspci already expose
this when someone needs it.

Also, we were reluctant to expose some types of information, like serial
numbers and such, via the VPD recently, so why exposing any serial numbers
via sysfs would be any different?

Thank you,

	Krzysztof

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Krzysztof Wilczyński]

Hello,

> @@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
>  	struct device *dev = kobj_to_dev(kobj);
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  
> -	if (pci_is_pcie(pdev))
> -		return a->mode;
> +	if (!pci_is_pcie(pdev))
> +		return 0;
>  
> -	return 0;
> +	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
> +		return 0;
> +
> +	return a->mode;

It would be fine to have this sysfs attribute present all the time, and
simply return error when the serial number is not available.  Not sure if
hiding it adds a lot of value.  This is how some of the existing attributes
currently behave.

But it does add extra code to pcie_dev_attrs_are_visible() where it is now
a special case, somewhat.

Thank you,

	Krzysztof

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Jonathan Cameron]

On Sat, 13 Sep 2025 15:17:20 +0900
Krzysztof Wilczyński <kw@linux.com> wrote:

> Hello,
> 
> > Can we get a ruling on this one? It's pretty straight forward
> > implementation exposing a useful attribute.  
> 
> Who needs this?  Why is this useful?  Why hasn't there been a need for
> exposing serial number in past decades, and suddenly we need it so
> desperately?
> 
> We probably wouldn't want to add this if there is only a single user that
> needs this, especially give that userspace tools like lspci already expose
> this when someone needs it.
> 
> Also, we were reluctant to expose some types of information, like serial
> numbers and such, via the VPD recently, so why exposing any serial numbers
> via sysfs would be any different?

I'll note that we already expose these serial numbers for CXL type3 devices
because they are really useful when you have a bunch of identical devices
as they turn in RAS error records and other places.

I pushed back on adding this same sysfs attribute to other CXL types
on basis we could just get it from the associated PCI device (assuming this
series lands).

https://elixir.bootlin.com/linux/v6.17-rc5/source/drivers/cxl/core/memdev.c#L113

Note that we don't do is_visible magic for this particular attribute in CXL
because it is mandated as present by the CXL spec.  We do that for a lot
of other stuff though as it keeps the interface clean.

Jonathan


> 
> Thank you,
> 
> 	Krzysztof

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Keith Busch]

On Sat, Sep 13, 2025 at 03:17:20PM +0900, Krzysztof Wilczy´nski wrote:
> Who needs this?  Why is this useful?  Why hasn't there been a need for
> exposing serial number in past decades,

I can't speak for other reviewers for their interest in having such an
attribute. Matt provided the reasoning here in the cover letter by
making it possible to access from unpriviledged applications that are
managing devices assigned to them without jumping through hoops to get
that information.

> and suddenly we need it so desperately?

"desperately" is a bit of a stretch. This simple patch has been out for
many months now, missed 2 merge windows with zero negative feedback, and
we're about to miss a 3rd. At this point, "frustration" would be a
better description.
 
> We probably wouldn't want to add this if there is only a single user that
> needs this, 

There's multiple reviewers representing different companies.

> especially give that userspace tools like lspci already expose
> this when someone needs it.

Only if you're root. This new attribute is admin-only by default too,
but can be changed as needed.

> Also, we were reluctant to expose some types of information, like serial
> numbers and such, via the VPD recently, so why exposing any serial numbers
> via sysfs would be any different?

It's a specification defined attribute of a device, and sysfs is the
interface that exports device attributes.

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Bjorn Helgaas]

On Thu, Aug 21, 2025 at 04:22:38PM -0700, Matthew Wood wrote:
> Add a single sysfs read-only interface for reading PCIe device serial
> numbers from userspace in a programmatic way. This device attribute
> uses the same hexadecimal 1-byte dashed formatting as lspci serial number
> capability output. If a device doesn't support the serial number
> capability, the serial_number sysfs attribute will not be visible.
> 
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
> Reviewed-by: Mario Limonciello <superm1@kernel.org>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Sorry for the delay, I have no excuse.

> ---
>  Documentation/ABI/testing/sysfs-bus-pci |  9 +++++++++
>  drivers/pci/pci-sysfs.c                 | 27 ++++++++++++++++++++++---
>  2 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 69f952fffec7..d5251f4f3659 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -612,3 +612,12 @@ Description:
>  
>  		  # ls doe_features
>  		  0001:01        0001:02        doe_discovery
> +
> +What:		/sys/bus/pci/devices/.../serial_number
> +Date:		December 2025
> +Contact:	Matthew Wood <thepacketgeek@gmail.com>
> +Description:
> +		This is visible only for PCIe devices that support the serial
> +		number extended capability. The file is read only and due to
> +		the possible sensitivity of accessible serial numbers, admin
> +		only.
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 268c69daa4d5..1d26e4336f1b 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -30,6 +30,7 @@
>  #include <linux/msi.h>
>  #include <linux/of.h>
>  #include <linux/aperture.h>
> +#include <linux/unaligned.h>
>  #include "pci.h"
>  
>  #ifndef ARCH_PCI_DEV_GROUPS
> @@ -239,6 +240,22 @@ static ssize_t current_link_width_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(current_link_width);
>  
> +static ssize_t serial_number_show(struct device *dev,
> +				  struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pci_dev = to_pci_dev(dev);
> +	u64 dsn;
> +	u8 bytes[8];
> +
> +	dsn = pci_get_dsn(pci_dev);
> +	if (!dsn)
> +		return -EIO;
> +	put_unaligned_be64(dsn, bytes);
> +
> +	return sysfs_emit(buf, "%8phD\n", bytes);
> +}
> +static DEVICE_ATTR_ADMIN_RO(serial_number);
> +
>  static ssize_t secondary_bus_number_show(struct device *dev,
>  					 struct device_attribute *attr,
>  					 char *buf)
> @@ -660,6 +677,7 @@ static struct attribute *pcie_dev_attrs[] = {
>  	&dev_attr_current_link_width.attr,
>  	&dev_attr_max_link_width.attr,
>  	&dev_attr_max_link_speed.attr,
> +	&dev_attr_serial_number.attr,

I can see that the PCI r3.0 (conventional PCI) spec doesn't include
the Device Serial Number Capability and the PCIe spec does include it,
but this seems like it would fit better in the pci_dev_dev_attrs[],
and the visibility check would be parallel to the
dev_attr_boot_vga.attr check there.

>  	NULL,
>  };
>  
> @@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
>  	struct device *dev = kobj_to_dev(kobj);
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  
> -	if (pci_is_pcie(pdev))
> -		return a->mode;
> +	if (!pci_is_pcie(pdev))
> +		return 0;
>  
> -	return 0;
> +	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
> +		return 0;
> +
> +	return a->mode;
>  }
>  
>  static const struct attribute_group pci_dev_group = {
> -- 
> 2.50.1
> 

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Keith Busch]

On Sat, Sep 13, 2025 at 03:20:41PM +0900, Krzysztof Wilczy´nski wrote:
> Hello,
> 
> > @@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
> >  	struct device *dev = kobj_to_dev(kobj);
> >  	struct pci_dev *pdev = to_pci_dev(dev);
> >  
> > -	if (pci_is_pcie(pdev))
> > -		return a->mode;
> > +	if (!pci_is_pcie(pdev))
> > +		return 0;
> >  
> > -	return 0;
> > +	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
> > +		return 0;
> > +
> > +	return a->mode;
> 
> It would be fine to have this sysfs attribute present all the time, and
> simply return error when the serial number is not available.  Not sure if
> hiding it adds a lot of value.  This is how some of the existing attributes
> currently behave.
> 
> But it does add extra code to pcie_dev_attrs_are_visible() where it is now
> a special case, somewhat.

You bring up a good point, but I think it seems odd that the existing
pcie attributes are visible even if we know reading it will fail.
Perhaps the pcie link status visibility should be changed to follow this
patch's example to hide when they don't exist. Applications might notice
a different error, ENOENT vs EINVAL, if the device doesn't support the
capability, but that is a more accurate errno.

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Jonathan Cameron]

On Mon, 15 Sep 2025 16:23:07 -0600
Keith Busch <kbusch@kernel.org> wrote:

> On Sat, Sep 13, 2025 at 03:20:41PM +0900, Krzysztof Wilczy´nski wrote:
> > Hello,
> >   
> > > @@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
> > >  	struct device *dev = kobj_to_dev(kobj);
> > >  	struct pci_dev *pdev = to_pci_dev(dev);
> > >  
> > > -	if (pci_is_pcie(pdev))
> > > -		return a->mode;
> > > +	if (!pci_is_pcie(pdev))
> > > +		return 0;
> > >  
> > > -	return 0;
> > > +	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
> > > +		return 0;
> > > +
> > > +	return a->mode;  
> > 
> > It would be fine to have this sysfs attribute present all the time, and
> > simply return error when the serial number is not available.  Not sure if
> > hiding it adds a lot of value.  This is how some of the existing attributes
> > currently behave.
> > 
> > But it does add extra code to pcie_dev_attrs_are_visible() where it is now
> > a special case, somewhat.  
> 
> You bring up a good point, but I think it seems odd that the existing
> pcie attributes are visible even if we know reading it will fail.

Perhaps historical. The is_visible infrastructure is I think somewhat
newer than a lot of that ABI.

> Perhaps the pcie link status visibility should be changed to follow this
> patch's example to hide when they don't exist. Applications might notice
> a different error, ENOENT vs EINVAL, if the device doesn't support the
> capability, but that is a more accurate errno.

As it is sysfs we can never be sure someone hasn't assumed existing files
are present even when they aren't useful.

So I doubt we can improve the existing cases without something breaking.
If we want to give it a go and see who screams I'm fine with that :)

Jonathan

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Keith Busch]

On Mon, Sep 15, 2025 at 02:39:04PM -0500, Bjorn Helgaas wrote:
> > @@ -660,6 +677,7 @@ static struct attribute *pcie_dev_attrs[] = {
> >  	&dev_attr_current_link_width.attr,
> >  	&dev_attr_max_link_width.attr,
> >  	&dev_attr_max_link_speed.attr,
> > +	&dev_attr_serial_number.attr,
> 
> I can see that the PCI r3.0 (conventional PCI) spec doesn't include
> the Device Serial Number Capability and the PCIe spec does include it,
> but this seems like it would fit better in the pci_dev_dev_attrs[],
> and the visibility check would be parallel to the
> dev_attr_boot_vga.attr check there.

I'm not sure I agree. The pci_dev_dev_attrs apply to all pci devices,
but DSN only exists in PCIe Extended Capability space. Conventional pci
config requests couldn't even describe it, so seems okay to fence it off
using the PCI-Express attribute group that already has that visibility
barrier.

I also don't like Krzysztof's suggestion to make it visible even if we
know you can't read it. The exisiting attributes that behave that way
shouldn't do that, IMO. It's a waste of resources to provide a handle
just to say the capability doesn't exist when the handle could just not
exist instead.

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Krzysztof Wilczyński]

Hello,

> > I can see that the PCI r3.0 (conventional PCI) spec doesn't include
> > the Device Serial Number Capability and the PCIe spec does include it,
> > but this seems like it would fit better in the pci_dev_dev_attrs[],
> > and the visibility check would be parallel to the
> > dev_attr_boot_vga.attr check there.
> 
> I'm not sure I agree. The pci_dev_dev_attrs apply to all pci devices,
> but DSN only exists in PCIe Extended Capability space. Conventional pci
> config requests couldn't even describe it, so seems okay to fence it off
> using the PCI-Express attribute group that already has that visibility
> barrier.

PCI-X 2.0 added Extended Configuration Space[1].  Perhaps why Bjorn had
different attributes group in mind here.

> I also don't like Krzysztof's suggestion to make it visible even if we
> know you can't read it.

I simply wanted to keep this new attribute and its behaviour aligned with
the existing ones.  Which we keep visible for historic reasons.

I don't like the special case it becomes within pcie_dev_attrs_are_visible(),
but it can't be helped without introducing an entirely new group for this
alone which would be an overkill, indeed. 

That said, no strong feelings about it.  I wish we could clean the existing
ones up a bit, though.

> The exisiting attributes that behave that way shouldn't do that, IMO.

A lot of then, as Jonathan also reminded us, predate kernfs' visibility
feature, sadly.  We probably won't change the existing ones, indeed, to
avoid potentially breaking something for some users.

This is why I was curious why do we need such an attribute to be added,
as when these new objects for PCI are added and gain users, then there
are often here to stay, with broken behaviour or not, sadly.  We don't
have any sensible way of properly deprecating things in this area.

> It's a waste of resources to provide a handle just to say the capability
> doesn't exist when the handle could just not exist instead.

I haven't checked how the kernfs side looks like, admittedly, but I think
whether an attribute is visible or not, it does not unload and/or de-allocate
any space for the accompanying kernfs object...  So, the resources saving
here might not be in any way significant.

1. https://en.m.wikipedia.org/wiki/PCI_configuration_space

Thank you,

	Krzysztof

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Krzysztof Wilczyński]

> > Add a single sysfs read-only interface for reading PCIe device serial
> > numbers from userspace in a programmatic way. This device attribute
> > uses the same hexadecimal 1-byte dashed formatting as lspci serial number
> > capability output. If a device doesn't support the serial number
> > capability, the serial_number sysfs attribute will not be visible.
> > 
> > Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
> > Reviewed-by: Mario Limonciello <superm1@kernel.org>
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> 
> Sorry for the delay, I have no excuse.

I also didn't noticed it has been a while... This series has fallen through
the cracks completely, indeed.  Sorry about this.

Thank you,

	Krzysztof

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Thomas Weißschuh]

On Wed, Sep 17, 2025 at 05:34:22PM +0900, Krzysztof Wilczyński wrote:

(...)

> > It's a waste of resources to provide a handle just to say the capability
> > doesn't exist when the handle could just not exist instead.
> 
> I haven't checked how the kernfs side looks like, admittedly, but I think
> whether an attribute is visible or not, it does not unload and/or de-allocate
> any space for the accompanying kernfs object...  So, the resources saving
> here might not be in any way significant.

If I read the sysfs code correctly (create_files() in fs/sysfs/group.c),
the kernfs node should not even be allocated for invisible files.


Thomas

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Matthew Wood]

Thank you all for the helpful responses!

> Hello,
>
> > > I can see that the PCI r3.0 (conventional PCI) spec doesn't include
> > > the Device Serial Number Capability and the PCIe spec does include it,
> > > but this seems like it would fit better in the pci_dev_dev_attrs[],
> > > and the visibility check would be parallel to the
> > > dev_attr_boot_vga.attr check there.
> >
> > I'm not sure I agree. The pci_dev_dev_attrs apply to all pci devices,
> > but DSN only exists in PCIe Extended Capability space. Conventional pci
> > config requests couldn't even describe it, so seems okay to fence it off
> > using the PCI-Express attribute group that already has that visibility
> > barrier.
>
> PCI-X 2.0 added Extended Configuration Space[1].  Perhaps why Bjorn had
> different attributes group in mind here.

Looking more at pci_get_dsn, I see that there's no check for pci_is_pcie so I
understand the thoughts here and can see how it would make sense to move
to pci_dev_dev_attrs.

I also agree that the visibility check for dsn would match the
existing dev_attr_boot_vga
check, with the additional advantage of not making the existing attrs in
pcie_dev_attrs_are_visible oddballs without visibility checks.

I will prepare a v8 with that change, thanks again!

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Krzysztof Wilczyński]

Hi Thomas,

> > > It's a waste of resources to provide a handle just to say the capability
> > > doesn't exist when the handle could just not exist instead.
> > 
> > I haven't checked how the kernfs side looks like, admittedly, but I think
> > whether an attribute is visible or not, it does not unload and/or de-allocate
> > any space for the accompanying kernfs object...  So, the resources saving
> > here might not be in any way significant.
> 
> If I read the sysfs code correctly (create_files() in fs/sysfs/group.c),
> the kernfs node should not even be allocated for invisible files.

Good to know!  I wasn't sure and did look...  I stand corrected now.

Thank you for taking the time to check this.  Much appreciated. :)

Thank you!

	Krzysztof

Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

[Krzysztof Wilczyński]

Hello,

> Thank you all for the helpful responses!

Thank you for working on this! 

I am sorry that we took time to get this merged.

[...]
> I will prepare a v8 with that change, thanks again!

Feel free to add:

Reviewed-by: Krzysztof Wilczyński <kwilczynski@kernel.org>

While at it.

Thank you!

	Krzysztof