diff mbox series

[v2,1/2] iommu: add support for drivers that manage iommu explicitly

Message ID 20190906214409.26677-2-robdclark@gmail.com (mailing list archive)
State New, archived
Headers show
Series iommu: handle drivers that manage iommu directly | expand

Commit Message

Rob Clark Sept. 6, 2019, 9:44 p.m. UTC 
From: Rob Clark <robdclark@chromium.org>

Avoid attaching any non-driver managed domain if the driver indicates
that it manages the iommu directly.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/iommu/iommu.c    | 2 +-
 drivers/iommu/of_iommu.c | 3 +++
 include/linux/device.h   | 3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)

Comments

Joerg Roedel Sept. 10, 2019, 8:14 a.m. UTC | #1 
On Fri, Sep 06, 2019 at 02:44:01PM -0700, Rob Clark wrote:
> @@ -674,7 +674,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
>  
>  	mutex_lock(&group->mutex);
>  	list_add_tail(&device->list, &group->devices);
> -	if (group->domain)
> +	if (group->domain && !(dev->driver && dev->driver->driver_manages_iommu))

Hmm, this code usually runs at enumeration time when no driver is
attached to the device. Actually it would be pretty dangerous when this
code runs while a driver is attached to the device. How does that change
make things work for you?

Regards,

	Joerg
Rob Clark Sept. 10, 2019, 3:34 p.m. UTC | #2 
On Tue, Sep 10, 2019 at 1:14 AM Joerg Roedel <joro@8bytes.org> wrote:
>
> On Fri, Sep 06, 2019 at 02:44:01PM -0700, Rob Clark wrote:
> > @@ -674,7 +674,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
> >
> >       mutex_lock(&group->mutex);
> >       list_add_tail(&device->list, &group->devices);
> > -     if (group->domain)
> > +     if (group->domain && !(dev->driver && dev->driver->driver_manages_iommu))
>
> Hmm, this code usually runs at enumeration time when no driver is
> attached to the device. Actually it would be pretty dangerous when this
> code runs while a driver is attached to the device. How does that change
> make things work for you?
>

I was seeing this get called via the path driver_probe_device() ->
platform_dma_configure() -> of_dma_configure() -> of_iommu_configure()
-> iommu_probe_device() -> ...

The only cases I was seeing where dev->driver is NULL where a few
places that drivers call of_dma_configure() on their own sub-devices.
But maybe there are some other paths that I did not notice?

BR,
-R
Robin Murphy Sept. 10, 2019, 4:51 p.m. UTC | #3 
On 10/09/2019 16:34, Rob Clark wrote:
> On Tue, Sep 10, 2019 at 1:14 AM Joerg Roedel <joro@8bytes.org> wrote:
>>
>> On Fri, Sep 06, 2019 at 02:44:01PM -0700, Rob Clark wrote:
>>> @@ -674,7 +674,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
>>>
>>>        mutex_lock(&group->mutex);
>>>        list_add_tail(&device->list, &group->devices);
>>> -     if (group->domain)
>>> +     if (group->domain && !(dev->driver && dev->driver->driver_manages_iommu))
>>
>> Hmm, this code usually runs at enumeration time when no driver is
>> attached to the device. Actually it would be pretty dangerous when this
>> code runs while a driver is attached to the device. How does that change
>> make things work for you?
>>
> 
> I was seeing this get called via the path driver_probe_device() ->
> platform_dma_configure() -> of_dma_configure() -> of_iommu_configure()
> -> iommu_probe_device() -> ...
> 
> The only cases I was seeing where dev->driver is NULL where a few
> places that drivers call of_dma_configure() on their own sub-devices.
> But maybe there are some other paths that I did not notice?

For the of_iommu flow, it very much depends on your DT layout and driver 
probe order as to whether you catch the "proper" call from 
iommu_bus_notifier()/iommu_bus_init() or the late "replay" from 
of_iommu_configure(). I wouldn't make any assumptions of consistency.

Robin.
diff mbox series

Patch

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0c674d80c37f..2ac5e8d48cae 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -674,7 +674,7 @@  int iommu_group_add_device(struct iommu_group *group, struct device *dev)
 
 	mutex_lock(&group->mutex);
 	list_add_tail(&device->list, &group->devices);
-	if (group->domain)
+	if (group->domain && !(dev->driver && dev->driver->driver_manages_iommu))
 		ret = __iommu_attach_device(group->domain, dev);
 	mutex_unlock(&group->mutex);
 	if (ret)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 614a93aa5305..62b47e384a77 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -221,6 +221,9 @@  const struct iommu_ops *of_iommu_configure(struct device *dev,
 	} else if (err < 0) {
 		dev_dbg(dev, "Adding to IOMMU failed: %d\n", err);
 		ops = NULL;
+	} else if (dev->driver && dev->driver->driver_manages_iommu) {
+		dev_dbg(dev, "Driver manages IOMMU\n");
+		ops = NULL;
 	}
 
 	return ops;
diff --git a/include/linux/device.h b/include/linux/device.h
index 1aa341b2a0db..b77a11b8d9bb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -284,7 +284,8 @@  struct device_driver {
 	struct module		*owner;
 	const char		*mod_name;	/* used for built-in modules */
 
-	bool suppress_bind_attrs;	/* disables bind/unbind via sysfs */
+	bool suppress_bind_attrs:1;	/* disables bind/unbind via sysfs */
+	bool driver_manages_iommu:1;	/* driver manages IOMMU explicitly */
 	enum probe_type probe_type;
 
 	const struct of_device_id	*of_match_table;