On Thu, 17 Jul 2025 11:33:51 -0700
Dan Williams <dan.j.williams@xxxxxxxxx> 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.
>
> 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.
>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
A couple of trivial comments.
Probably want to +CC Greg KH on next version given bits in drivers/base
Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 69048869ef1c..d894c87ce1fd 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
include cleanup.h perhaps for access to guard()?
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index 53840634fbfc..7a4623f65256 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -282,6 +282,46 @@ static struct pci_dev *pci_get_dev_by_id(const struct pci_device_id *id,
> return pdev;
> }
>
> +static struct pci_dev *pci_get_dev_by_id_reverse(const struct pci_device_id *id,
> + struct pci_dev *from)
> +{
> + struct device *dev;
> + struct device *dev_start = NULL;
> + struct pci_dev *pdev = NULL;
> +
> + if (from)
> + dev_start = &from->dev;
> + dev = bus_find_device_reverse(&pci_bus_type, dev_start, (void *)id,
> + match_pci_dev_by_id);
> + if (dev)
> + pdev = to_pci_dev(dev);
> + pci_dev_put(from);
> + return pdev;
> +}
> +
> +enum pci_search_direction {
> + PCI_SEARCH_FORWARD,
> + PCI_SEARCH_REVERSE,
> +};
> +
I don't really care, but given there are only two sane directions maybe
a bool reverse as a parameter to __pci_get_subsys() would be sufficient?
> +static struct pci_dev *__pci_get_subsys(unsigned int vendor, unsigned int device,
> + unsigned int ss_vendor, unsigned int ss_device,
> + struct pci_dev *from, enum pci_search_direction dir)
> +{
> + struct pci_device_id id = {
> + .vendor = vendor,
> + .device = device,
> + .subvendor = ss_vendor,
> + .subdevice = ss_device,
> + };
> +
> + if (dir == PCI_SEARCH_FORWARD)
> + return pci_get_dev_by_id(&id, from);
> + else
> + return pci_get_dev_by_id_reverse(&id, from);
> +}
> +
This file seems to use 1 blank line only between functions.
> +
> /**
> * pci_get_subsys - begin or continue searching for a PCI device by vendor/subvendor/device/subdevice id
> * @vendor: PCI vendor id to match, or %PCI_ANY_ID to match all vendor ids
> @@ -302,14 +342,8 @@ struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
> unsigned int ss_vendor, unsigned int ss_device,
> struct pci_dev *from)
> {
> - struct pci_device_id id = {
> - .vendor = vendor,
> - .device = device,
> - .subvendor = ss_vendor,
> - .subdevice = ss_device,
> - };
> -
> - return pci_get_dev_by_id(&id, from);
> + return __pci_get_subsys(vendor, device, ss_vendor, ss_device, from,
> + PCI_SEARCH_FORWARD);
> }
> EXPORT_SYMBOL(pci_get_subsys);
>
> @@ -334,6 +368,19 @@ struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device,
> }
> EXPORT_SYMBOL(pci_get_device);
>
> +/*
> + * Same semantics as pci_get_device(), except walks the PCI device list
> + * in reverse discovery order.
> + */
> +struct pci_dev *pci_get_device_reverse(unsigned int vendor,
> + unsigned int device,
> + struct pci_dev *from)
> +{
> + return __pci_get_subsys(vendor, device, PCI_ANY_ID, PCI_ANY_ID, from,
> + PCI_SEARCH_REVERSE);
> +}
> +EXPORT_SYMBOL(pci_get_device_reverse);
> +
> /**
> * pci_get_class - begin or continue searching for a PCI device by class
> * @class: search for a PCI device with this class designation
> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> index f5a56efd2bd6..99b1002b3e31 100644
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -150,6 +150,9 @@ int bus_for_each_dev(const struct bus_type *bus, struct device *start,
> void *data, device_iter_t fn);
> struct device *bus_find_device(const struct bus_type *bus, struct device *start,
> const void *data, device_match_t match);
> +struct device *bus_find_device_reverse(const struct bus_type *bus,
> + struct device *start, const void *data,
> + device_match_t match);
> /**
> * bus_find_device_by_name - device iterator for locating a particular device
> * of a specific name.
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3fac811376b5..b8bca0711967 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -575,6 +575,8 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
>
> #define to_pci_dev(n) container_of(n, struct pci_dev, dev)
> #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
> +#define for_each_pci_dev_reverse(d) \
> + while ((d = pci_get_device_reverse(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
>
> static inline int pci_channel_offline(struct pci_dev *pdev)
> {
> @@ -1220,6 +1222,8 @@ u64 pci_get_dsn(struct pci_dev *dev);
>
> struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device,
> struct pci_dev *from);
> +struct pci_dev *pci_get_device_reverse(unsigned int vendor, unsigned int device,
> + struct pci_dev *from);
> struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
> unsigned int ss_vendor, unsigned int ss_device,
> struct pci_dev *from);
> @@ -1639,6 +1643,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
>
> void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
> void *userdata);
> +void pci_walk_bus_reverse(struct pci_bus *top,
> + int (*cb)(struct pci_dev *, void *), void *userdata);
> int pci_cfg_space_size(struct pci_dev *dev);
> unsigned char pci_bus_max_busnr(struct pci_bus *bus);
> resource_size_t pcibios_window_alignment(struct pci_bus *bus,
> @@ -2031,6 +2037,11 @@ static inline struct pci_dev *pci_get_device(unsigned int vendor,
> struct pci_dev *from)
> { return NULL; }
>
> +static inline struct pci_dev *pci_get_device_reverse(unsigned int vendor,
> + unsigned int device,
> + struct pci_dev *from)
> +{ return NULL; }
> +
> static inline struct pci_dev *pci_get_subsys(unsigned int vendor,
> unsigned int device,
> unsigned int ss_vendor,
