Re: [PATCH v7 3/9] PCI: Introduce pci_walk_bus_reverse(), for_each_pci_dev_reverse()

dan.j.williams
2025-10-30T19:36:29Z

Jonathan Cameron wrote:
> On Thu, 23 Oct 2025 19:04:12 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > PCI/TSM, the PCI core functionality for the PCIe TEE Device Interface
> > Security Protocol (TDISP), has a need to walk all subordinate functions of
> > a Device Security Manager (DSM) to setup a device security context. A DSM
> > is physical function 0 of multi-function or SR-IOV device endpoint, or it
> > is an upstream switch port.
> >
> > In error scenarios or when a TEE Security Manager (TSM) device is removed
> > it needs to unwind all established DSM contexts.
> >
> > Introduce reverse versions of PCI device iteration helpers to mirror the
> > setup path and ensure that dependent children are handled before parents.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> Bit of archaeology was needed as there are some existing oddities in the
> functions this is based on.
>
> My suggestions for this are don't use guard() and drop the void * cast that
> we should cleanup in the existing code.

I will keep cleanups that imply touching old functions for a potential
follow-on, but likely will let sleeping dogs lie.

[..]
> > +/**
> > + * pci_walk_bus_reverse - walk devices on/under bus, calling callback.
> > + * @top: bus whose devices should be walked
> > + * @cb: callback to be called for each device found
> > + * @userdata: arbitrary pointer to be passed to callback
> > + *
> > + * Same semantics as pci_walk_bus(), but walks the bus in reverse order.
> > + */
> > +void pci_walk_bus_reverse(struct pci_bus *top,
> > + int (*cb)(struct pci_dev *, void *), void *userdata)
> > +{
> > + guard(rwsem_read)(&pci_bus_sem);
>
> So this ends up different to pci_walk_bus. I'd be tempted to just
> not bother bringing a single guard() usage here. Gain is trivial and
> mixing and matching style in a file isn't particularly nice.

Yeah, and violates my earlier claim about being style-bug compatible.

[..]

3: f8e6f3d9ba81 ! 3: 5ae3e927d3ed PCI: Introduce pci_walk_bus_reverse(), for_each_pci_dev_reverse()
@@ drivers/pci/bus.c: void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_d
+void pci_walk_bus_reverse(struct pci_bus *top,
+ int (*cb)(struct pci_dev *, void *), void *userdata)
+{
-+ guard(rwsem_read)(&pci_bus_sem);
++ down_read(&pci_bus_sem);
+ __pci_walk_bus_reverse(top, cb, userdata);
++ up_read(&pci_bus_sem);
+}
+EXPORT_SYMBOL_GPL(pci_walk_bus_reverse);
+