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@intel.com>
---
drivers/base/bus.c | 38 +++++++++++++++++++++++
drivers/pci/bus.c | 37 ++++++++++++++++++++++
drivers/pci/search.c | 63 +++++++++++++++++++++++++++++++++-----
include/linux/device/bus.h | 3 ++
include/linux/pci.h | 11 +++++++
5 files changed, 144 insertions(+), 8 deletions(-)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 5e75e1bce551..d19dae8f9d1b 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -334,6 +334,19 @@ static struct device *next_device(struct klist_iter *i)
return dev;
}
+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;
+}
+
/**
* bus_for_each_dev - device iterator.
* @bus: bus type.
@@ -414,6 +427,31 @@ struct device *bus_find_device(const struct bus_type *bus,
}
EXPORT_SYMBOL_GPL(bus_find_device);
+struct device *bus_find_device_reverse(const struct bus_type *bus,
+ struct device *start, const void *data,
+ device_match_t match)
+{
+ struct subsys_private *sp = bus_to_subsys(bus);
+ struct klist_iter i;
+ struct device *dev;
+
+ if (!sp)
+ return NULL;
+
+ klist_iter_init_node(&sp->klist_devices, &i,
+ (start ? &start->p->knode_bus : NULL));
+ while ((dev = prev_device(&i))) {
+ if (match(dev, data)) {
+ get_device(dev);
+ break;
+ }
+ }
+ klist_iter_exit(&i);
+ subsys_put(sp);
+ return dev;
+}
+EXPORT_SYMBOL_GPL(bus_find_device_reverse);
+
static struct device_driver *next_driver(struct klist_iter *i)
{
struct klist_node *n = klist_next(i);
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
@@ -428,6 +428,27 @@ static int __pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void
return ret;
}
+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;
+}
+
/**
* pci_walk_bus - walk devices on/under bus, calling callback.
* @top: bus whose devices should be walked
@@ -449,6 +470,22 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), void
}
EXPORT_SYMBOL_GPL(pci_walk_bus);
+/**
+ * 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);
+ __pci_walk_bus_reverse(top, cb, userdata);
+}
+EXPORT_SYMBOL_GPL(pci_walk_bus_reverse);
+
void pci_walk_bus_locked(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), void *userdata)
{
lockdep_assert_held(&pci_bus_sem);
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,
+};
+
+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);
+}
+
+
/**
* 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,
--
2.50.1On 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;
Bjorn Helgaas wrote:
> 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/
ack
> > 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.
The underlying assumption is that the first generation of TDISP capable
devices will have a Device Security Manager (DSM) for all the SR-IOV
virtual functions of the device, or the card will have an embedded PCIe
switch where the Upstream Switch Port has a Device Security Manager for
integrated Dowstream Endpoint functions in the card.
The expectation is that physical hotplug for these cases never happens
*within* a security domain. The entire physical function is removed and
by implication all the functions the DSM watches over.
However, this does highlight a miss for logical hotplug of VFs. This
enabling wants to have sriov_init() check if the PF is connected to a
TSM and if so perform a late pdev->tsm->ops->probe() to setup any
context needed to allow the VF to go through secure-device-assignment. I
will add that for the next version.
The reverse is already there... any TSM context for to-be-removed VFs is
cleaned up.
>
> > +++ 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;
Agree, in isolation, but next to next_device() the style looks odd. So,
go back and style-fix code from 2008, or make 2025 code look like 2008
code is the choice.
>
> > +++ 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;
Again, for conformance to existing style of __pci_walk_bus(). Want a
lead-in cleanup for that?
On Thu, Aug 07, 2025 at 04:17:54PM -0700, dan.j.williams@intel.com wrote:
> Bjorn Helgaas wrote:
> > On Thu, Jul 17, 2025 at 11:33:51AM -0700, Dan Williams wrote:
> > > +++ 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;
>
> Agree, in isolation, but next to next_device() the style looks odd. So,
> go back and style-fix code from 2008, or make 2025 code look like 2008
> code is the choice.
Good point, I didn't look around at that code. Following the existing
style seems right to me.
> > > +++ 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;
>
> Again, for conformance to existing style of __pci_walk_bus(). Want a
> lead-in cleanup for that?
Don't bother. Maybe some janitor will show up and do it eventually.
Bjorn
On Thu, 17 Jul 2025 11:33:51 -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 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@intel.com>
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@huawei.com>
> 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,
Jonathan Cameron wrote:
> On Thu, 17 Jul 2025 11:33:51 -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 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@intel.com>
>
> A couple of trivial comments.
>
> Probably want to +CC Greg KH on next version given bits in drivers/base
Oh, true. On last revision I copied him on whole series. Missed that this
time.
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
> > 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()?
Sure.
> > 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?
I dislike reading:
return __pci_get_subsys(vendor, device, ss_vendor, ss_device, from, false);
...in isolation where I must walk the symbol to the function to figure
out what that parameter means vs:
return __pci_get_subsys(vendor, device, ss_vendor, ss_device, from,
PCI_SEARCH_FORWARD);
...which is immediately clear.
>
> > +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.
ok.
> > > +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?
>
> I dislike reading:
>
> return __pci_get_subsys(vendor, device, ss_vendor, ss_device, from, false);
>
> ...in isolation where I must walk the symbol to the function to figure
> out what that parameter means vs:
>
> return __pci_get_subsys(vendor, device, ss_vendor, ss_device, from,
> PCI_SEARCH_FORWARD);
>
> ...which is immediately clear.
Fair enough.
>
> >
© 2016 - 2025 Red Hat, Inc.