On Thu, 15 May 2025 22:47:28 -0700
Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> The limited number of link-encryption (IDE) streams that a given set of
> host bridges supports is a platform specific detail. Provide
> pci_ide_init_nr_streams() as a generic facility for either platform TSM
> drivers, or PCI core native IDE, to report the number available streams.
> After invoking pci_ide_init_nr_streams() an "available_secure_streams"
> attribute appears in PCI host bridge sysfs to convey that count.
>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: Samuel Ortiz <sameo@xxxxxxxxxxxx>
> Cc: Alexey Kardashevskiy <aik@xxxxxxx>
> Cc: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
Trivial stuff inline.
> ---
> .../ABI/testing/sysfs-devices-pci-host-bridge | 13 ++++
> drivers/pci/ide.c | 59 +++++++++++++++++++
> drivers/pci/pci.h | 3 +
> drivers/pci/probe.c | 12 +++-
> include/linux/pci.h | 8 +++
> 5 files changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-pci-host-bridge b/Documentation/ABI/testing/sysfs-devices-pci-host-bridge
> index d592b68c7333..382866a21703 100644
> --- a/Documentation/ABI/testing/sysfs-devices-pci-host-bridge
> +++ b/Documentation/ABI/testing/sysfs-devices-pci-host-bridge
> @@ -36,3 +36,16 @@ Description:
> stream resources shared by the Root Ports in a host bridge. See
> /sys/devices/pciDDDD:BB entry for details about the DDDD:BB
> format.
> +
> +What: pciDDDD:BB/available_secure_streams
> +Date: December, 2024
> +Contact: linux-pci@xxxxxxxxxxxxxxx
> +Description:
> + (RO) When a host bridge has Root Ports that support PCIe IDE
> + (link encryption and integrity protection) there may be a
> + limited number of streams that can be used for establishing new
> + secure links. This attribute decrements upon secure link setup,
> + and increments upon secure link teardown. The in-use stream
> + count is determined by counting stream symlinks. See
> + /sys/devices/pciDDDD:BB entry for details about the DDDD:BB
> + format.
Is this name specific enough given mix of link and selective streams, both of which are
limited? Nice to use generic terms but this one feels too generic!
> diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
> index a529926647f4..b7561ac03405 100644
> --- a/drivers/pci/ide.c
> +++ b/drivers/pci/ide.c
> +static struct attribute *pci_ide_attrs[] = {
> + &dev_attr_available_secure_streams.attr,
> + NULL,
As below. No trailing comma here.
> +};
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 56704e851224..93be55321537 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -640,6 +640,16 @@ static void pci_release_host_bridge_dev(struct device *dev)
> kfree(bridge);
> }
>
> +static const struct attribute_group *pci_host_bridge_groups[] = {
> + PCI_IDE_ATTR_GROUP,
> + NULL,
One of my favourite comments :) No comma on terminators. Let's
not make it easy to accidentally put something after them.
> +};
> +
> +static const struct device_type pci_host_bridge_type = {
> + .groups = pci_host_bridge_groups,
> + .release = pci_release_host_bridge_dev,
I've no problem with this as a clean up but you could have just
used the bridge->dev.groups instead I think? If you are clearing
that out for some other use alter, mention that in the patch description.
> +};
> +
> static void pci_init_host_bridge(struct pci_host_bridge *bridge)
> {
> INIT_LIST_HEAD(&bridge->windows);
> @@ -659,6 +669,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)
> bridge->native_dpc = 1;
> bridge->domain_nr = PCI_DOMAIN_NR_NOT_SET;
> bridge->native_cxl_error = 1;
> + bridge->dev.type = &pci_host_bridge_type;
>
> device_initialize(&bridge->dev);
> }
> @@ -672,7 +683,6 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
> return NULL;
>
> pci_init_host_bridge(bridge);
> - bridge->dev.release = pci_release_host_bridge_dev;
>
> return bridge;
> }
