On Thu, Jul 17, 2025 at 11:33:51AM -0700, Dan Williams 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 SRIOV device endpoint, or it is
> an upstream switch port.
s/SRIOV/SR-IOV/
> 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.
I really don't like these search and iterator interfaces. I wish we
didn't need them like this because code that uses them becomes a
one-time thing that doesn't handle hotplug and has potential locking
and race issues. But I assume you really do need these.
> +++ b/drivers/base/bus.c
> +static struct device *prev_device(struct klist_iter *i)
> +{
> + struct klist_node *n = klist_prev(i);
> + struct device *dev = NULL;
> + struct device_private *dev_prv;
> +
> + if (n) {
> + dev_prv = to_device_private_bus(n);
> + dev = dev_prv->device;
> + }
> + return dev;
I think this would be simpler as:
if (!n)
return NULL;
dev_prv = to_device_private_bus(n);
return dev_prv->device;
> +++ b/drivers/pci/bus.c
> +static int __pci_walk_bus_reverse(struct pci_bus *top,
> + int (*cb)(struct pci_dev *, void *),
> + void *userdata)
> +{
> + struct pci_dev *dev;
> + int ret = 0;
> +
> + list_for_each_entry_reverse(dev, &top->devices, bus_list) {
> + if (dev->subordinate) {
> + ret = __pci_walk_bus_reverse(dev->subordinate, cb,
> + userdata);
> + if (ret)
> + break;
> + }
> + ret = cb(dev, userdata);
> + if (ret)
> + break;
> + }
> + return ret;
Why not:
list_for_each_entry_reverse(...) {
...
if (ret)
return ret;
}
return 0;
