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

dan.j.williams
2025-10-30T20:48:44Z

Jonathan Cameron wrote:
> 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.

Fair enough, a comment explaining the hazard is about as ugly as the
ifdef.

[..]
> > +/**
> > + * 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

Sure.

8: 8bcdc030d445 ! 8: 37a609df4dd5 PCI/IDE: Report available IDE streams
@@ drivers/pci/pci.h: static inline void pci_doe_sysfs_teardown(struct pci_dev *pde
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
- #endif
-
- #ifdef CONFIG_PCI_TSM

## include/linux/pci-ide.h ##
@@ include/linux/pci-ide.h: struct pci_ide {
@@ drivers/pci/ide.c: void pci_ide_init_host_bridge(struct pci_host_bridge *hb)
+ */
+void pci_ide_set_nr_streams(struct pci_host_bridge *hb, u16 nr)
+{
-+ if (nr > 256)
-+ nr = 256;
-+ hb->nr_ide_streams = nr;
++ hb->nr_ide_streams = min(nr, 256);
+ WARN_ON_ONCE(!ida_is_empty(&hb->ide_stream_ida));
+ sysfs_update_group(&hb->dev.kobj, &pci_ide_attr_group);
+}
@@ drivers/pci/probe.c: static void pci_release_host_bridge_dev(struct device *dev)
}

+static const struct attribute_group *pci_host_bridge_groups[] = {
-+ PCI_IDE_ATTR_GROUP,
++#ifdef CONFIG_PCI_IDE
++ &pci_ide_attr_group,
++#endif
+ NULL
+};
+