Re: [PATCH] thunderbolt: Make iommu_dma_protection more accurate

2022-03-18 Thread Lukas Wunner
On Fri, Mar 18, 2022 at 03:51:21PM +0100, Lukas Wunner wrote:
> On Fri, Mar 18, 2022 at 02:08:16PM +, Robin Murphy wrote:
> > OK, so do we have any realistic options for identifying the correct PCI
> > devices, if USB4 PCIe adapters might be anywhere relative to their
> > associated NHI? Short of maintaining a list of known IDs, the only thought I
> > have left is that if we walk the whole PCI segment looking specifically for
> > hotplug-capable Gen1 ports, any system modern enough to have Thunderbolt is
> > *probably* not going to have any real PCIe Gen1 hotplug slots, so maybe
> > false negatives might be tolerable, but it still feels like a bit of a
> > sketchy heuristic.
> 
> The Thunderbolt Device ROM contains the PCI slot number, so you can
> correlate the Thunderbolt switch ports with PCIe downstream ports
> and know exactly where PCIe tunnels are terminated.
[...]
> I implemented that in 2018, so it won't apply cleanly to current
> mainline.  But I kept forward-porting it on my private branch and
> could push that to GitHub if anyone is interested.

FWIW, here's the most recent forward-port I've done:

https://github.com/l1k/linux/commits/thunderbolt_correlate_5.13
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] thunderbolt: Make iommu_dma_protection more accurate

2022-03-18 Thread Lukas Wunner
On Fri, Mar 18, 2022 at 02:08:16PM +, Robin Murphy wrote:
> OK, so do we have any realistic options for identifying the correct PCI
> devices, if USB4 PCIe adapters might be anywhere relative to their
> associated NHI? Short of maintaining a list of known IDs, the only thought I
> have left is that if we walk the whole PCI segment looking specifically for
> hotplug-capable Gen1 ports, any system modern enough to have Thunderbolt is
> *probably* not going to have any real PCIe Gen1 hotplug slots, so maybe
> false negatives might be tolerable, but it still feels like a bit of a
> sketchy heuristic.

The Thunderbolt Device ROM contains the PCI slot number, so you can
correlate the Thunderbolt switch ports with PCIe downstream ports
and know exactly where PCIe tunnels are terminated.

Code is here:
* thunderbolt: Obtain PCI slot number from DROM
  https://github.com/l1k/linux/commit/756f7148bc10
* thunderbolt: Move upstream_port to struct tb
  https://github.com/l1k/linux/commit/58f16e7dd431
* thunderbolt: Correlate PCI devices with Thunderbolt ports
  https://github.com/l1k/linux/commit/f53ea40a7487

I implemented that in 2018, so it won't apply cleanly to current
mainline.  But I kept forward-porting it on my private branch and
could push that to GitHub if anyone is interested.

I don't know if this will work out-of-the-box for SoC-integrated
Thunderbolt controllers.  It was developed with the discrete
controllers in mind, which was the only thing available back then.

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] PCI / ACPI: Identify external PCI devices

2018-11-15 Thread Lukas Wunner
On Thu, Nov 15, 2018 at 09:10:26PM +0200, Mika Westerberg wrote:
> I was thinking we could cover all these with is_external filling them
> based on the _DSD or some other means in the kernel.
> 
> We would then deal all such devices as "untrusted" by default.

Tinfoil hat on, even internal devices could be malicious.
What's the downside of enabling the feature for everything?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] PCI / ACPI: Identify external PCI devices

2018-11-15 Thread Lukas Wunner
On Thu, Nov 15, 2018 at 01:37:37PM +0200, Mika Westerberg wrote:
> On Thu, Nov 15, 2018 at 11:13:56AM +, Lorenzo Pieralisi wrote:
> > I have strong objections to the way these bindings have been forced upon
> > everybody; if that's the way *generic* ACPI bindings are specified I
> > wonder why there still exists an ACPI specification and related working
> > group.
> > 
> > I personally (but that's Bjorn and Rafael choice) think that this is
> > not a change that belongs in PCI core, ACPI bindings are ill-defined
> > and device tree bindings are non-existing.
> 
> Any idea where should I put it then? These systems are already out there
> and we need to support them one way or another.

I suppose those are all Thunderbolt, so could be handled by the
existing ->is_thunderbolt bit?

It was said in this thread that ->is_external is more generic in
that it could also be used on PCIe slots, however that use case
doesn't appear to lend itself to the "plug in while laptop owner
is getting coffee" attack.  To access PCIe slots on a server you
normally need access to a data center.  On a desktop, you usually
have to open the case, by which time the coffee may already have
been fetched.  So frankly the binding seems a bit over-engineered
to me and yet another thing that BIOS writers may get wrong.

Well, just my 2 cents anyway.

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection

2018-11-12 Thread Lukas Wunner
On Mon, Nov 12, 2018 at 07:06:24PM +0300, Mika Westerberg wrote:
> Recent systems shipping with Windows 10 version 1803 or newer may be
> utilizing IOMMU to prevent DMA attacks via Thunderbolt ports. This is
> different from the previous security level based scheme because the
> connected device cannot access system memory outside of the regions
> allocated for it by the driver.
> 
> When enabled the BIOS makes sure no device can do DMA outside of RMRR
> (Reserved Memory Region Record) regions. This means that during OS boot,
> before it enables IOMMU, none of the connected devices can bypass DMA
> protection for instance by overwriting the data structures used by the
> IOMMU. The BIOS communicates support for this to the OS by setting a new
> bit in ACPI DMAR table [1].
> 
> Because these systems utilize an IOMMU to block possible DMA attacks,
> typically (but not always) the Thunderbolt security level is set to "none"
> which means that all PCIe devices are immediately usable. This also means
> that Linux needs to follow Windows 10 and enable IOMMU automatically when
> running on such system otherwise connected devices can read/write system
> memory pretty much without any restrictions.

What if the system is booted from a Thunderbolt-attached disk?
Won't this suddenly break with these patches?  That would seem like a
pretty significant regression.  What if the only GPU in the system is
Thunderbolt-attached?  Is it possible to recognize such scenarios and
automatically exempt affected devices from IOMMU blocking?

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] PCI / ACPI: Identify external PCI devices

2018-11-12 Thread Lukas Wunner
On Mon, Nov 12, 2018 at 07:06:25PM +0300, Mika Westerberg wrote:
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1378,6 +1378,27 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
>   }
>  }
>  
> +static void set_pcie_external(struct pci_dev *dev)
> +{
> + struct pci_dev *parent;
> +
> + /*
> +  * Walk up the device hierarchy and check for any upstream
> +  * bridge that has is_external_facing set to true. This means
> +  * the hierarchy is below PCIe port that is exposed externally
> +  * (such as Thunderbolt).
> +  */
> + parent = pci_upstream_bridge(dev);
> + while (parent) {
> + if (parent->is_external) {
> + dev->is_external = true;
> + break;
> + }
> +
> + parent = pci_upstream_bridge(parent);
> + }
> +}
> +

This looks pretty much like a duplication of the is_thunderbolt bit
in struct pci_dev and the pci_is_thunderbolt_attached() helper.

Why constrain the functionality to ports with the _DSD property
instead of making it available for *any* Thunderbolt port?

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v9 1/5] driver core: Find an existing link between two devices

2018-03-14 Thread Lukas Wunner
On Wed, Mar 14, 2018 at 12:14:15PM +, Robin Murphy wrote:
> >>On Wed, Mar 14, 2018 at 8:12 PM, Rafael J. Wysocki  
> >>wrote:
> >>>On Tuesday, March 13, 2018 12:23:34 PM CET Tomasz Figa wrote:
> On Tue, Mar 13, 2018 at 7:34 PM, Vivek Gautam 
>  wrote:
> >On Tue, Mar 13, 2018 at 3:45 PM, Tomasz Figa  wrote:
> >>On Tue, Mar 13, 2018 at 5:55 PM, Vivek Gautam 
> >> wrote:
> >>>The lists managing the device-links can be traversed to
> >>>find the link between two devices. The device_link_add() APIs
> >>>does traverse these lists to check if there's already a link
> >>>setup between the two devices.
> >>>So, add a new APIs, device_link_find(), to find an existing
> >>>device link between two devices - suppliers and consumers.
> >>
> >>I'm wondering if this API would be useful for anything else that the
> >>problem we're trying to solve with deleting links without storing them
> >>anywhere. Perhaps a device_link_del_dev(consumer, supplier) would be a
> >>better alternative?
> >
> >Yea, that sounds simpler i think. Will add this API instead of
> >find_link(). Thanks.
> 
> Perhaps let's wait for a moment to see if there are other opinions. :)
> 
> Rafael, Lucas, any thoughts?
> >>>
> >>>It is not clear to me what the device_link_del_dev(consumer, supplier)
> >>>would do.
> 
> Not quite - the issue here is that we have one supplier with an arbitrarily
> large number of consumers, and would prefer that supplier not to have to
> spend a whole bunch of memory to store all the struct device_link pointers
> for the sole reason of having something to give to device_link_del() at the
> end, given that the device links code is already keeping track of everything
> internally anyway.

Makes sense to me.  How about an additional flag which autoremoves the
link on provider unbind?

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v9 1/5] driver core: Find an existing link between two devices

2018-03-14 Thread Lukas Wunner
On Wed, Mar 14, 2018 at 12:12:05PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, March 13, 2018 12:23:34 PM CET Tomasz Figa wrote:
> > On Tue, Mar 13, 2018 at 7:34 PM, Vivek Gautam  
> > wrote:
> > > On Tue, Mar 13, 2018 at 3:45 PM, Tomasz Figa  wrote:
> > >> On Tue, Mar 13, 2018 at 5:55 PM, Vivek Gautam 
> > >>  wrote:
> > >>> The lists managing the device-links can be traversed to
> > >>> find the link between two devices. The device_link_add() APIs
> > >>> does traverse these lists to check if there's already a link
> > >>> setup between the two devices.
> > >>> So, add a new APIs, device_link_find(), to find an existing
> > >>> device link between two devices - suppliers and consumers.
> > >>
> > >> I'm wondering if this API would be useful for anything else that the
> > >> problem we're trying to solve with deleting links without storing them
> > >> anywhere. Perhaps a device_link_del_dev(consumer, supplier) would be a
> > >> better alternative?
> > >
> > > Yea, that sounds simpler i think. Will add this API instead of
> > > find_link(). Thanks.
> > 
> > Perhaps let's wait for a moment to see if there are other opinions. :)
> > 
> > Rafael, Lucas, any thoughts?
> 
> It is not clear to me what the device_link_del_dev(consumer, supplier)
> would do.

The point appears to be that the pointer to the device_link need not be
stored somewhere for later deletion.  The newly added function would
check if a device link exists and delete it if so.

However I don't understand why storing the pointer would be a problem?
Also, would using DL_FLAG_AUTOREMOVE avoid the need for the additional
function?

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 2/2] driver core: Silence device links sphinx warning

2016-12-04 Thread Lukas Wunner
Silence this warning emitted by sphinx:
include/linux/device.h:938: warning: No description found for parameter 'links'

While at it, fix typos in comments of device links code.

Cc: Rafael J. Wysocki <raf...@kernel.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Silvio Fricke <silvio.fri...@gmail.com>
Signed-off-by: Lukas Wunner <lu...@wunner.de>
---
 drivers/base/core.c| 4 ++--
 include/linux/device.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index d0c9df5..8c25e68 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -172,7 +172,7 @@ static int device_reorder_to_tail(struct device *dev, void 
*not_used)
  *
  * The supplier device is required to be registered when this function is 
called
  * and NULL will be returned if that is not the case.  The consumer device need
- * not be registerd, however.
+ * not be registered, however.
  */
 struct device_link *device_link_add(struct device *consumer,
struct device *supplier, u32 flags)
@@ -225,7 +225,7 @@ struct device_link *device_link_add(struct device *consumer,
INIT_LIST_HEAD(>c_node);
link->flags = flags;
 
-   /* Deterine the initial link state. */
+   /* Determine the initial link state. */
if (flags & DL_FLAG_STATELESS) {
link->status = DL_STATE_NONE;
} else {
diff --git a/include/linux/device.h b/include/linux/device.h
index 3896af4..87edfdf 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -813,6 +813,7 @@ struct dev_links_info {
  * on.  This shrinks the "Board Support Packages" (BSPs) and
  * minimizes board-specific #ifdefs in drivers.
  * @driver_data: Private pointer for driver specific info.
+ * @links: Links to suppliers and consumers of this device.
  * @power: For device power management.
  * See Documentation/power/admin-guide/devices.rst for details.
  * @pm_domain: Provide callbacks that are executed during system suspend,
-- 
2.10.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 1/2] Documentation/core-api/device_link: Add initial documentation

2016-12-04 Thread Lukas Wunner
Document device links as introduced in v4.10 with commits:
4bdb35506b89 ("driver core: Add a wrapper around
   __device_release_driver()")
9ed9895370ae ("driver core: Functional dependencies tracking
   support")
8c73b4288496 ("PM / sleep: Make async suspend/resume of devices use
   device links")
21d5c57b3726 ("PM / runtime: Use device links")
baa8809f6097 ("PM / runtime: Optimize the use of device links")

Cc: Rafael J. Wysocki <raf...@kernel.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Luis R. Rodriguez <mcg...@kernel.org>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Silvio Fricke <silvio.fri...@gmail.com>
Cc: Marek Szyprowski <m.szyprow...@samsung.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnier...@samsung.com>
Cc: Andrzej Hajda <a.ha...@samsung.com>
Cc: Mauro Carvalho Chehab <mche...@osg.samsung.com>
Cc: Inki Dae <inki@samsung.com>
Cc: Joerg Roedel <j...@8bytes.org>
Cc: Kukjin Kim <kg...@kernel.org>
Cc: Krzysztof Kozlowski <k...@kernel.org>
Cc: Mark Brown <broo...@kernel.org>
Cc: Tomeu Vizoso <tomeu.viz...@collabora.com>
Cc: Kevin Hilman <khil...@kernel.org>
Cc: Ulf Hansson <ulf.hans...@linaro.org>
Cc: Geert Uytterhoeven <ge...@linux-m68k.org>
Cc: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
Cc: Tomasz Figa <tomasz.f...@gmail.com>
Cc: Grant Likely <grant.lik...@secretlab.ca>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Lars-Peter Clausen <l...@metafoo.de>
Cc: Dmitry Torokhov <dmitry.torok...@gmail.com>
Cc: Christoph Hellwig <h...@infradead.org>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: Alan Stern <st...@rowland.harvard.edu>
Cc: Hanjun Guo <guohan...@huawei.com>
Cc: linux...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: iommu@lists.linux-foundation.org
Cc: linux-samsung-...@vger.kernel.org
Signed-off-by: Lukas Wunner <lu...@wunner.de>
---
 Documentation/core-api/device_link.rst | 279 +
 Documentation/core-api/index.rst   |   8 +
 2 files changed, 287 insertions(+)
 create mode 100644 Documentation/core-api/device_link.rst

diff --git a/Documentation/core-api/device_link.rst 
b/Documentation/core-api/device_link.rst
new file mode 100644
index 000..5f57134
--- /dev/null
+++ b/Documentation/core-api/device_link.rst
@@ -0,0 +1,279 @@
+
+Device links
+
+
+By default, the driver core only enforces dependencies between devices
+that are borne out of a parent/child relationship within the device
+hierarchy: When suspending, resuming or shutting down the system, devices
+are ordered based on this relationship, i.e. children are always suspended
+before their parent, and the parent is always resumed before its children.
+
+Sometimes there is a need to represent device dependencies beyond the
+mere parent/child relationship, e.g. between siblings, and have the
+driver core automatically take care of them.
+
+Secondly, the driver core by default does not enforce any driver presence
+dependencies, i.e. that one device must be bound to a driver before
+another one can probe or function correctly.
+
+Often these two dependency types come together, so a device depends on
+another one both with regards to driver presence *and* with regards to
+suspend/resume and shutdown ordering.
+
+Device links allow representation of such dependencies in the driver core.
+
+In its standard form, a device link combines *both* dependency types:
+It guarantees correct suspend/resume and shutdown ordering between a
+"supplier" device and its "consumer" devices, and it guarantees driver
+presence on the supplier.  The consumer devices are not probed before the
+supplier is bound to a driver, and they're unbound before the supplier
+is unbound.
+
+When driver presence on the supplier is irrelevant and only correct
+suspend/resume and shutdown ordering is needed, the device link may
+simply be set up with the ``DL_FLAG_STATELESS`` flag.  In other words,
+enforcing driver presence on the supplier is optional.
+
+Another optional feature is runtime PM integration:  By setting the
+``DL_FLAG_PM_RUNTIME`` flag on addition of the device link, the PM core
+is instructed to runtime resume the supplier and keep it active
+whenever and for as long as the consumer is runtime resumed.
+
+Usage
+=
+
+The earliest point in time when device links can be added is after
+:c:func:`device_add()` has been called for the supplier and
+:c:func:`device_initialize()` has been called for the consumer.
+
+It is legal to add them later, but care must be taken that the system
+remains in a consistent state:  E.g. a device link cannot be added in
+the midst of a suspend/resume transition, so either commencement of
+such a transition needs to be prevented wi

[PATCH 0/2] Device links documentation

2016-12-04 Thread Lukas Wunner
Here's my suggestion for initial documentation on the device links
feature that's queued for 4.10 on Greg KH's driver-core-next branch.
Please let me know if you have any additions or corrections.

To read this in rendered form:
http://wunner.de/kernel-doc/core-api/device_link.html

Patch [2/2] could go in via driver-core-next, but patch [1/2] would
need to go in via docs-next (because the core-api directory doesn't
exist yet in driver-core-next).

@Jonathan Corbet:  Is core-api the right place to put this? An
alternative would be Documentation/driver-api, but unlike core-api
it contains less prose text but mostly just bare API docs gleaned
from kernel-doc.


To render the documentation oneself, the device links patches need
to be merged from driver-core-next into docs-next.  Here's a branch
which contains all the necessary bits:
https://github.com/l1k/linux/commits/device_links_docs_v1

(The build is currently broken for docs-next, one needs to remove
80211.xml from Documentation/DocBook/Makefile and for some reason
$BUILDDIR/cec.h.rst is not found on the first build, but only on
the second build.)


About half of the documentation was distilled from the mailing list
discussion initiated by Luis Rodriguez after KS/LPC:
https://lkml.org/lkml/2016/11/7/790
https://lkml.org/lkml/2016/11/7/795
https://lkml.org/lkml/2016/11/8/899

The other half is based on the first draft I posted in September:
https://lkml.org/lkml/2016/9/27/215

Thanks,

Lukas


Lukas Wunner (2):
  Documentation/core-api/device_link: Add initial documentation
  driver core: Silence device links sphinx warning

 Documentation/core-api/device_link.rst | 279 +
 Documentation/core-api/index.rst   |   8 +
 drivers/base/core.c|   4 +-
 include/linux/device.h |   1 +
 4 files changed, 290 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/core-api/device_link.rst

-- 
2.10.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm

2016-11-19 Thread Lukas Wunner
On Tue, Nov 08, 2016 at 08:27:12AM +0100, Marek Szyprowski wrote:
> On 2016-11-07 22:47, Luis R. Rodriguez wrote:
> > If so
> > why? If this issue is present also on systems that only use ACPI is
> > this possibly due to an ACPI firmware bug  or the lack of some semantics
> > in ACPI to express ordering in a better way? If the issue is device
> > tree related only is this due to the lack of semantics in device tree
> > to express some more complex dependency ?
> 
> The main feature of device links that is used in this patch is enabling
> runtime pm dependency between Exynos SYSMMU controller (called it client
> device) and the device, for which it implements DMA address translation
> (called master device). The assumptions are following:
> 1. master device driver is completely unaware of the Exynos SYSMMU presence,
>IOMMU is transparently hooked up and managed by DMA-mapping framework
> 2. SYSMMU belongs to the same power domain as it's master device
> 3. SYSMMU is optional, master device can fully operate without it, with
>simple DMA address translation (DMA address == physical address)
> 4. Master device implements runtime pm, what in turn causes respective
>power domain to be turned on/off
> 5. DMA-mapping and IOMMU frameworks provides no calls to notify SYSMMU
>when its master device is performing DMA operations, so SYSMMU has
>to be runtime active
> 6. Currently SYSMMU always sets its runtime pm status to active after
>attaching to its master device to ensure proper hardware state. This
>prevents power domain to be turned off, even when master device sets
>its runtime pm status to suspended.
> 7. Exynos SYSMMU has to be runtime active at the same time when its
>master device is runtime active to it to perform DMA operations and
>allow the power domain to be turned off, when master device is
>runtime suspended.
> 8. The terms of device links, Exynos SYSMMU is a 'consumer' and master
>device is a 'supplier'.

You seem to have mixed up the consumer and supplier in point 8 above.
Your code is such that the SYSMMU is the supplier and the master device
is the consumer:

device_link_add(dev, data->sysmmu, DL_FLAG_PM_RUNTIME);

Prototype of device_link_add:

struct device_link *device_link_add(struct device *consumer,
struct device *supplier,
u32 flags);

Your code is correct, only point 8 above is wrong.

Best regards,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm

2016-11-08 Thread Lukas Wunner
On Tue, Nov 08, 2016 at 08:27:12AM +0100, Marek Szyprowski wrote:
> On 2016-11-07 22:47, Luis R. Rodriguez wrote:
> > Has there been any review of the existing similar solutions out there
> > such as the DRM / audio component framework? Would that help ?
> 
> Nope, none of that solution deals with runtime pm.

Well, they do.  Hybrid graphics laptops often have an HDA controller
on the discrete GPU and I assume that's what Luis meant. There's code
in drivers/gpu/vga/vga_switcheroo.c to make this (only sort of) work:

* When the GPU is powered up/down, the HDA controller's driver is
  instructed to pm_runtime_get/put the HDA device (see call to
  set_audio_state() in vga_switcheroo_set_dynamic_switch()).

* When a runtime PM ref is acquired on the HDA device, the
  GPU is powered up (see vga_switcheroo_runtime_resume_hdmi_audio()).


Unfortunately this is all fairly broken, e.g.:

* If a runtime PM ref on the HDA device is held for more than 5 sec
  (autosuspend delay of the GPU), the GPU will be powered down and
  the HDA device will become inaccessible, regardless of the runtime
  PM ref still being held (because vga_switcheroo_set_dynamic_switch()
  doesn't check the refcount of the HDA device).

* The DRM device is afforded direct-complete but the HDA device is not.
  If the GPU is handled earlier by dpm_suspend(), then runtime PM will
  have been disabled on the GPU and thus the HDA device will fail to
  runtime resume before system sleep.

Rafael's series allows representation of such inter-device dependencies
in the PM core and can thus replace kludgy and broken "solutions" like
the one above.

There's one thing that I haven't understood myself though:  In an e-mail
exchange in September Rafael has argued that the above-mentioned hybrid
graphics use case "isn't a good [example] IMO.  That clearly is a case
when two (or more) devices share power resources controlled by a single
on/off switch.  Which is a clear use case for a PM domain."

The same seems to apply to Marek's SYSMMU use case.  When applying device
links to SYSMMU or hybrid graphics, we select one of the devices in the
PM domain as master and have the other one depend on it as slave, i.e.
a synthetic hierarchical relationship is established.

I've responded to Rafael on September 18 that this can't be solved with
a struct dev_pm_domain, but haven't received a reply since:
https://lkml.org/lkml/2016/9/18/103

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)

2016-09-23 Thread Lukas Wunner
On Fri, Sep 23, 2016 at 02:49:20PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, September 20, 2016 10:51:13 AM Marek Szyprowski wrote:
> > On 2016-09-19 23:45, Tobias Jakobi wrote:
> > > I did some tests with the new version today. Sadly the reboot/shutdown
> > > issues are still present.
> > 
> > Thanks for the report. I've managed to reproduce this issue and it is again
> > caused by modifying device on devices_kset list before it will be finally
> > added by device_add(). I thought that the new patchset allows creating
> > links to a device, which has not been yet added to system device list.

Hm, Marek, why isn't it possible to set up the links from the consumer's
->probe hook in this case?


> > Should it be allowed to create a link to device, which
> > has not yet been added to system device list by device_add()?
> 
> While it would be easy to require both the consumer and producer devices to
> be registered for creating a link between them, that would just make it
> harder to use links in the first place.
> 
> So ideally, it should be possible to create links between devices before
> registering them, but since I didn't take that into account in the current
> patch series, some quite substantial changes are needed to cover that.
> 
> Additional link states come to mind, but then the "stateless" links are
> affected by this problem too.

device_link_add() could be changed to call device_reorder_to_tail()
only if device_is_registered(consumer) returns true.

That's an inline function defined in  which returns
dev->kobj.state_in_sysfs, a flag which is set in kobject_add().

Then device_add() would have to check if any links are already
set up and reorder the consumer behind the suppliers.

Doesn't seem to be *that* complex, but probably I'm missing something,
this is just off the cuff...

Best regards,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support

2016-07-20 Thread Lukas Wunner
On Thu, Jul 21, 2016 at 12:51:31AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 20, 2016 05:23:40 PM Lukas Wunner wrote:
> > On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote:
> > > On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote:
> > > > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote:
> > > > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote:
> > > > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote:
> > > > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner <lu...@wunner.de> 
> > > > > > > wrote:
> > > > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski 
> > > > > > > > wrote:
> > > > > > > > > From: "Rafael J. Wysocki" <rafael.j.wyso...@intel.com>
> > > > > > > > We also have such a functional dependency for Thunderbolt on 
> > > > > > > > Macs:
> > > > > > > > On resume from system sleep, the PCIe hotplug ports may not 
> > > > > > > > resume
> > > > > > > > before the thunderbolt driver has reestablished the PCI tunnels.
> > > > > > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt()
> > > > > > > > in drivers/pci/quirks.c. It would be good if we could represent
> > > > > > > > this dependency using something like Rafael's approach instead 
> > > > > > > > of
> > > > > > > > open coding it, however one detail in Rafael's patches is 
> > > > > > > > problematic:
> > > > > > > >
> > > > > > > > > New links are added by calling device_link_add() which may 
> > > > > > > > > happen
> > > > > > > > > either before the consumer device is probed or when probing 
> > > > > > > > > it, in
> > > > > > > > > which case the caller needs to ensure that the driver of the
> > > > > > > > > supplier device is present and functional and the 
> > > > > > > > > DEVICE_LINK_PROBE_TIME
> > > > > > > > > flag should be passed to device_link_add() to reflect that.
> > > > > > > >
> > > > > > > > The thunderbolt driver cannot call device_link_add() before the
> > > > > > > > PCIe hotplug ports are bound to a driver unless we amend portdrv
> > > > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs
> > > > > > > > if the thunderbolt driver isn't loaded.
> > > > > > > >
> > > > > > > > It would therefore be beneficial if device_link_add() can be
> > > > > > > > called even *after* the consumer is bound.
> > > > > > > 
> > > > > > > I don't quite follow.
> > > > > > > 
> > > > > > > Who's the provider and who's the consumer here?
> > > > > > 
> > > > > > thunderbolt.ko is the supplier.
> > > > > 
> > > > > But it binds to the children of the ports that are supposed to be its
> > > > > consumers?
> > > > > 
> > > > > Why is that even expected to work?
> > > > 
> > > > No, the consumers are aunts (or uncles) of the supplier, if you will. 
> > > > :-)
> > > > 
> > > > The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in
> > > > the drawing below). The supplier is the NHI:
> > > > 
> > > >   (Root Port)  Upstream Bridge --+-- Downstream Bridge 0  
> > > > NHI
> > > >  +-- Downstream Bridge 1 --
> > > >  +-- Downstream Bridge 2 --
> > > >  ...
> > > > 
> > > > We're calling pci_power_up() and pci_restore_state() from
> > > > pci_pm_resume_noirq(). And that will fail for devices below
> > > > the hotplug ports if the PCI tunnels haven't been re-established
> > > > yet by the NHI.
> > > 
> > > So the NHI is a PCIe device, right?
> > > 
> > > Does the Thunderbolt driver bind to that device?
> > 
> > The NHI is a PCI device but not a bridge. It has class 0x88000.
> > Yes, thunderbolt.ko binds to the NHI.
> > 
> > And portdrv binds to the upstream bridge and downstream bridges.
> > Those have class 0x60400.
> 
> OK, so why would there be a problem with creating links from the NHI
> (producer) to the ports (consumers) before binding portdrv to them?

Because the ordering in which drivers bind isn't guaranteed. At least
on my machine (Debian), portdrv always binds before thunderbolt.

I guess I could amend portdrv to return -EPROBE_DEFER on Macs if
no driver is bound to the NHI. Doesn't feel pretty to me though.

Ultimately this seems to be the same issue as with calling
dev_pm_domain_set() for a bound device. Perhaps device_link_add()
can likewise be allowed if a runtime PM ref is held for the devices
and the call happens under lock_system_sleep()?

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support

2016-07-20 Thread Lukas Wunner
On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote:
> > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote:
> > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote:
> > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote:
> > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner <lu...@wunner.de> 
> > > > > wrote:
> > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote:
> > > > > > > From: "Rafael J. Wysocki" <rafael.j.wyso...@intel.com>
> > > > > > We also have such a functional dependency for Thunderbolt on Macs:
> > > > > > On resume from system sleep, the PCIe hotplug ports may not resume
> > > > > > before the thunderbolt driver has reestablished the PCI tunnels.
> > > > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt()
> > > > > > in drivers/pci/quirks.c. It would be good if we could represent
> > > > > > this dependency using something like Rafael's approach instead of
> > > > > > open coding it, however one detail in Rafael's patches is 
> > > > > > problematic:
> > > > > >
> > > > > > > New links are added by calling device_link_add() which may happen
> > > > > > > either before the consumer device is probed or when probing it, in
> > > > > > > which case the caller needs to ensure that the driver of the
> > > > > > > supplier device is present and functional and the 
> > > > > > > DEVICE_LINK_PROBE_TIME
> > > > > > > flag should be passed to device_link_add() to reflect that.
> > > > > >
> > > > > > The thunderbolt driver cannot call device_link_add() before the
> > > > > > PCIe hotplug ports are bound to a driver unless we amend portdrv
> > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs
> > > > > > if the thunderbolt driver isn't loaded.
> > > > > >
> > > > > > It would therefore be beneficial if device_link_add() can be
> > > > > > called even *after* the consumer is bound.
> > > > > 
> > > > > I don't quite follow.
> > > > > 
> > > > > Who's the provider and who's the consumer here?
> > > > 
> > > > thunderbolt.ko is the supplier.
> > > 
> > > But it binds to the children of the ports that are supposed to be its
> > > consumers?
> > > 
> > > Why is that even expected to work?
> > 
> > No, the consumers are aunts (or uncles) of the supplier, if you will. :-)
> > 
> > The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in
> > the drawing below). The supplier is the NHI:
> > 
> >   (Root Port)  Upstream Bridge --+-- Downstream Bridge 0  NHI
> >  +-- Downstream Bridge 1 --
> >  +-- Downstream Bridge 2 --
> >  ...
> > 
> > We're calling pci_power_up() and pci_restore_state() from
> > pci_pm_resume_noirq(). And that will fail for devices below
> > the hotplug ports if the PCI tunnels haven't been re-established
> > yet by the NHI.
> 
> So the NHI is a PCIe device, right?
> 
> Does the Thunderbolt driver bind to that device?

The NHI is a PCI device but not a bridge. It has class 0x88000.
Yes, thunderbolt.ko binds to the NHI.

And portdrv binds to the upstream bridge and downstream bridges.
Those have class 0x60400.

Best regards,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support

2016-07-20 Thread Lukas Wunner
On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote:
> On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote:
> > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote:
> > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner <lu...@wunner.de> wrote:
> > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote:
> > > > > From: "Rafael J. Wysocki" <rafael.j.wyso...@intel.com>
> > > > We also have such a functional dependency for Thunderbolt on Macs:
> > > > On resume from system sleep, the PCIe hotplug ports may not resume
> > > > before the thunderbolt driver has reestablished the PCI tunnels.
> > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt()
> > > > in drivers/pci/quirks.c. It would be good if we could represent
> > > > this dependency using something like Rafael's approach instead of
> > > > open coding it, however one detail in Rafael's patches is problematic:
> > > >
> > > > > New links are added by calling device_link_add() which may happen
> > > > > either before the consumer device is probed or when probing it, in
> > > > > which case the caller needs to ensure that the driver of the
> > > > > supplier device is present and functional and the 
> > > > > DEVICE_LINK_PROBE_TIME
> > > > > flag should be passed to device_link_add() to reflect that.
> > > >
> > > > The thunderbolt driver cannot call device_link_add() before the
> > > > PCIe hotplug ports are bound to a driver unless we amend portdrv
> > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs
> > > > if the thunderbolt driver isn't loaded.
> > > >
> > > > It would therefore be beneficial if device_link_add() can be
> > > > called even *after* the consumer is bound.
> > > 
> > > I don't quite follow.
> > > 
> > > Who's the provider and who's the consumer here?
> > 
> > thunderbolt.ko is the supplier.
> 
> But it binds to the children of the ports that are supposed to be its
> consumers?
> 
> Why is that even expected to work?

No, the consumers are aunts (or uncles) of the supplier, if you will. :-)

The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in
the drawing below). The supplier is the NHI:

  (Root Port)  Upstream Bridge --+-- Downstream Bridge 0  NHI
 +-- Downstream Bridge 1 --
 +-- Downstream Bridge 2 --
 ...

We're calling pci_power_up() and pci_restore_state() from
pci_pm_resume_noirq(). And that will fail for devices below
the hotplug ports if the PCI tunnels haven't been re-established
yet by the NHI.

Currently we achieve that via quirk_apple_wait_for_thunderbolt() in
drivers/pci/quirks.c. It would be more elegant if we could make this
relationship explicit with "device links" and let the core handle it.

Or am I mistaken and this particular use case is not what "device links"
are intended for?

Thanks,

Lukas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support

2016-06-17 Thread Lukas Wunner
On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote:
> On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner <lu...@wunner.de> wrote:
> > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote:
> > > From: "Rafael J. Wysocki" <rafael.j.wyso...@intel.com>
> > We also have such a functional dependency for Thunderbolt on Macs:
> > On resume from system sleep, the PCIe hotplug ports may not resume
> > before the thunderbolt driver has reestablished the PCI tunnels.
> > Currently this is enforced by quirk_apple_wait_for_thunderbolt()
> > in drivers/pci/quirks.c. It would be good if we could represent
> > this dependency using something like Rafael's approach instead of
> > open coding it, however one detail in Rafael's patches is problematic:
> >
> > > New links are added by calling device_link_add() which may happen
> > > either before the consumer device is probed or when probing it, in
> > > which case the caller needs to ensure that the driver of the
> > > supplier device is present and functional and the DEVICE_LINK_PROBE_TIME
> > > flag should be passed to device_link_add() to reflect that.
> >
> > The thunderbolt driver cannot call device_link_add() before the
> > PCIe hotplug ports are bound to a driver unless we amend portdrv
> > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs
> > if the thunderbolt driver isn't loaded.
> >
> > It would therefore be beneficial if device_link_add() can be
> > called even *after* the consumer is bound.
> 
> I don't quite follow.
> 
> Who's the provider and who's the consumer here?

thunderbolt.ko is the supplier. The PCIe hotplug ports of Thunderbolt
host controllers are the consumers.

If thunderbolt.ko is not loaded, no special precautions are needed for
the hotplug ports.

However *if* it is loaded and the system is suspended and there are
Thunderbolt devices plugged in, then on resume the hotplug ports must
wait for thunderbolt.ko to reestablish the PCI tunnels. If they do not
wait, things will go south because when pci_pm_resume_noirq() is called
for the devices *below* the hotplug port, we will try to put them into D0
(via pci_pm_default_resume_early()) and call pci_restore_state() even
though those hotplugged devices are not yet reachable.

So what I'd like to do is call device_link_add() when thunderbolt.ko
is loaded to shove a dependency onto the hotplug ports. But the hotplug
ports will long since have been bound at that point (to portdrv).
So it should be allowed to call device_link_add() ex post facto, after
the consumer has been bound.

Let me know if I still failed to convey the problem in an intelligible
way.

I had a similar problem with the Runtime PM for Thunderbolt series
(which BTW is still awaiting reviews):
http://www.spinics.net/lists/linux-pci/msg51158.html

The PM core allows calls to dev_pm_domain_set() only during ->probe or
if the device is unbound. Same constraint as with device_link_add().
This constraint was a major obstacle for me and I had to jump through
numerous additional hoops to work around it:

Thunderbolt controllers on Macs can be put into D3cold, but not with
standard ACPI methods, so platform_pci_power_manageable() is false for
these devices. Now this nothing unusual, the same applies to dual GPU
laptops (custom ACPI DSMs for Nvidia Optimus and AMD PowerXpress).

For such discrete GPUs, a struct dev_pm_domain is set during ->probe
(drivers/gpu/vga/vga_switcheroo.c: vga_switcheroo_init_domain_pm_ops()).
But I can't do that for Thunderbolt because the device in question is
a PCIe upstream port. Ideally I would shove a dev_pm_domain on the
upstream port when thunderbolt.ko loads, but again at that point
the port has long since been bound (to portdrv) so I can't call
dev_pm_domain_set().

The workaround I settled on is to attach to the upstream port as a
service driver, call down to the service driver's ->runtime_suspend
hooks when the port runtime suspends and power the controller down.
In addition, I had to make sure that saved_state isn't clobbered on
resume (patch [09/13]).

It would be good if someone who has a bird's eye view of the PM core
(i.e., you) could make a decision
(1) if it's possible and worthwhile to allow dev_pm_domain_set() for
already bound devices, and
(2) if the PCI core should be be able to deal with devices which are
runtime suspended to D3cold but not by the platform (because that's
what patch [09/13] does).

If the answer to (2) is yes then there's some more stuff to fix:
Discrete GPUs on dual GPU laptops can be manually suspended to D3cold
behind the PM core's back by loading nouveau / radeon / amdgpu with
runpm=0 and writing OFF to the vga_switcheroo debugfs interface.
This is currently broken in conjunction with system 

Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support

2016-06-17 Thread Lukas Wunner
Hi Marek,

On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote:
> From: "Rafael J. Wysocki" 
> 
> Currently, there is a problem with handling cases where functional
> dependencies between devices are involved.
> 
> What I mean by a "functional dependency" is when the driver of device
> B needs both device A and its driver to be present and functional to
> be able to work.  This implies that the driver of A needs to be
> working for B to be probed successfully and it cannot be unbound from
> the device before the B's driver.  This also has certain consequences
> for power management of these devices (suspend/resume and runtime PM
> ordering).
> 
> Add support for representing those functional dependencies between
> devices to allow the driver core to track them and act on them in
> certain cases where they matter.

Rafael has indicated that he intends to respin this series:
https://lkml.org/lkml/2016/6/8/1061

We also have such a functional dependency for Thunderbolt on Macs:
On resume from system sleep, the PCIe hotplug ports may not resume
before the thunderbolt driver has reestablished the PCI tunnels.
Currently this is enforced by quirk_apple_wait_for_thunderbolt()
in drivers/pci/quirks.c. It would be good if we could represent
this dependency using something like Rafael's approach instead of
open coding it, however one detail in Rafael's patches is problematic:

> New links are added by calling device_link_add() which may happen
> either before the consumer device is probed or when probing it, in
> which case the caller needs to ensure that the driver of the
> supplier device is present and functional and the DEVICE_LINK_PROBE_TIME
> flag should be passed to device_link_add() to reflect that.

The thunderbolt driver cannot call device_link_add() before the
PCIe hotplug ports are bound to a driver unless we amend portdrv
to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs
if the thunderbolt driver isn't loaded.

It would therefore be beneficial if device_link_add() can be
called even *after* the consumer is bound.

Thanks,

Lukas

> 
> Link objects are deleted either explicitly, by calling
> device_link_del() on the link object in question, or automatically,
> when the consumer device is unbound from its driver or when one
> of the target devices is deleted, depending on the link type.
> 
> There are two types of link objects, persistent and non-persistent.
> The difference between them is that the persistent links stay around
> until one of the target devices is deleted, which the non-persistent
> ones are deleted when the consumer driver is unbound from its device
> (ie. they are assumed to be valid only as long as the consumer device
> has a driver bound to it).  The DEVICE_LINK_PERSISTENT flag has to
> be passed to device_link_add() so as to create a persistent link.
> 
> One of the actions carried out by device_link_add() is to reorder
> the lists used for device shutdown and system suspend/resume to
> put the consumer device along with all of its children and all of
> its consumers (and so on, recursively) to the ends of those list
> in order to ensure the right ordering between the all of the supplier
> and consumer devices.
> 
> Signed-off-by: Rafael J. Wysocki 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/base/base.h|  11 ++
>  drivers/base/core.c| 386 
> +
>  drivers/base/dd.c  |  42 +-
>  include/linux/device.h |  36 +
>  4 files changed, 470 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index e05db388bd1c..cccb1d211541 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -107,6 +107,9 @@ extern void bus_remove_device(struct device *dev);
>  
>  extern int bus_add_driver(struct device_driver *drv);
>  extern void bus_remove_driver(struct device_driver *drv);
> +extern void device_release_driver_internal(struct device *dev,
> +struct device_driver *drv,
> +struct device *parent);
>  
>  extern void driver_detach(struct device_driver *drv);
>  extern int driver_probe_device(struct device_driver *drv, struct device 
> *dev);
> @@ -152,3 +155,11 @@ extern int devtmpfs_init(void);
>  #else
>  static inline int devtmpfs_init(void) { return 0; }
>  #endif
> +
> +/* Device links */
> +extern int device_links_check_suppliers(struct device *dev);
> +extern void device_links_driver_bound(struct device *dev);
> +extern void device_links_driver_gone(struct device *dev);
> +extern void device_links_no_driver(struct device *dev);
> +extern bool device_links_busy(struct device *dev);
> +extern void device_links_unbind_consumers(struct device *dev);
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 0a8bdade53f2..416341df3268 100644
> --- a/drivers/base/core.c
> +++