Re: [PATCH v7 8/9] PCI/IDE: Report available IDE streams

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Thu, 23 Oct 2025 19:04:17 -0700
Dan Williams <dan.j.williams@intel.com> 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.
> 
> Introduce a device-type, @pci_host_bridge_type, now that both a release
> method and sysfs attribute groups are being specified for all 'struct
> pci_host_bridge' instances.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

New day, new comments.  Nothing huge, but I would avoid the defining
an attr group to NULL as that feels like storing up bugs for the future.

> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index d3f16be40102..8b356dd09105 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -616,9 +616,12 @@ static inline void pci_doe_sysfs_teardown(struct pci_dev *pdev) { }
>  #ifdef CONFIG_PCI_IDE
>  void pci_ide_init(struct pci_dev *dev);
>  void pci_ide_init_host_bridge(struct pci_host_bridge *hb);
> +extern const struct attribute_group pci_ide_attr_group;
> +#define PCI_IDE_ATTR_GROUP (&pci_ide_attr_group)
>  #else
>  static inline void pci_ide_init(struct pci_dev *dev) { }
>  static inline void pci_ide_init_host_bridge(struct pci_host_bridge *hb) { }
> +#define PCI_IDE_ATTR_GROUP NULL

This only works if we assume nothing else ever ends up in pci_host_bridge_groups.
i.e. it's fragile - think of someone adding something after this.
Whilst I don't like ifdefs inline, there isn't a better option for this case that
I can think of.

>  #endif
>  
>  #ifdef CONFIG_PCI_TSM
> diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h
> index e638f9429bf9..85645b0a8620 100644
> --- a/include/linux/pci-ide.h
> +++ b/include/linux/pci-ide.h
> @@ -63,6 +63,7 @@ struct pci_ide {
>  	const char *name;
>  };
>  
> +void pci_ide_set_nr_streams(struct pci_host_bridge *hb, u16 nr);
>  struct pci_ide_partner *pci_ide_to_settings(struct pci_dev *pdev,
>  					    struct pci_ide *ide);
>  struct pci_ide *pci_ide_stream_alloc(struct pci_dev *pdev);
> diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
> index 302f7bae6c96..44f62da5e191 100644
> --- a/drivers/pci/ide.c
> +++ b/drivers/pci/ide.c

> +/**
> + * pci_ide_set_nr_streams() - sets size of the pool of IDE Stream resources
> + * @hb: host bridge boundary for the stream pool
> + * @nr: number of streams
> + *
> + * Platform PCI init and/or expert test module use only. Limit IDE
> + * Stream establishment by setting the number of stream resources
> + * available at the host bridge. Platform init code must set this before
> + * the first pci_ide_stream_alloc() call if the platform has less than the
> + * default of 256 streams per host-bridge.
> + *
> + * The "PCI_IDE" symbol namespace is required because this is typically
> + * a detail that is settled in early PCI init. I.e. this export is not
> + * for endpoint drivers.
> + */
> +void pci_ide_set_nr_streams(struct pci_host_bridge *hb, u16 nr)
> +{
> +	if (nr > 256)
> +		nr = 256;


hb->nr_ide_streams = min(nr, 256);
maybe

> +	hb->nr_ide_streams = nr;
> +	WARN_ON_ONCE(!ida_is_empty(&hb->ide_stream_ida));
> +	sysfs_update_group(&hb->dev.kobj, &pci_ide_attr_group);
> +}
> +EXPORT_SYMBOL_NS_GPL(pci_ide_set_nr_streams, "PCI_IDE");