linux-kernel - Re: [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20131122074111.155E2C40753@trevor.secretlab.ca>
Date:	Fri, 22 Nov 2013 07:41:11 +0000
From:	Grant Likely <grant.likely@...aro.org>
To:	Stephen Warren <swarren@...dotorg.org>,
	Hiroshi Doyu <hdoyu@...dia.com>, swarren@...dia.com,
	will.deacon@....com, thierry.reding@...il.com, galak@...eaurora.org
Cc:	mark.rutland@....com, devicetree@...r.kernel.org,
	iommu@...ts.linux-foundation.org, linux-tegra@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, lorenzo.pieralisi@....com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs

On Thu, 21 Nov 2013 12:04:18 -0700, Stephen Warren <swarren@...dotorg.org> wrote:
> On 11/21/2013 06:15 AM, Grant Likely wrote:
> > On Tue, 19 Nov 2013 11:33:06 +0200, Hiroshi Doyu <hdoyu@...dia.com> wrote:
> >> IOMMU devices on the bus need to be poplulated first, then iommu
> >> master devices are done later.
> >>
> >> With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
> >> whether a device can be an iommu msater or not. If a device can, we'll
> >> defer to populate that device till an iommu device is populated. Once
> >> an iommu device is populated, "dev->bus->iommu_ops" is set in the
> >> bus. Then, those defered iommu master devices are populated and
> >> configured for IOMMU with help of the already populated iommu device
> >> via iommu_ops->add_device(). Multiple IOMMUs can be listed on this
> >> "iommus" binding so that a device can have multiple IOMMUs attached.
> >>
> >> Signed-off-by: Hiroshi Doyu <hdoyu@...dia.com>
> >> ---
> >> v5:
> >> Use "iommus=" binding instread of arm,smmu's "#stream-id-cells".
> >>
> >> v4:
> >> This is newly added, and the successor of the following RFC:
> >>   [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach()
> >>   http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html
> >> ---
> >>  drivers/base/dd.c        |  5 +++++
> >>  drivers/iommu/of_iommu.c | 22 ++++++++++++++++++++++
> >>  include/linux/of_iommu.h |  7 +++++++
> >>  3 files changed, 34 insertions(+)
> >>
> >> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> >> index 35fa368..6e892d4 100644
> >> --- a/drivers/base/dd.c
> >> +++ b/drivers/base/dd.c
> >> @@ -25,6 +25,7 @@
> >>  #include <linux/async.h>
> >>  #include <linux/pm_runtime.h>
> >>  #include <linux/pinctrl/devinfo.h>
> >> +#include <linux/of_iommu.h>
> >>  
> >>  #include "base.h"
> >>  #include "power/power.h"
> >> @@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> >>  
> >>  	dev->driver = drv;
> >>  
> >> +	ret = of_iommu_attach(dev);
> >> +	if (ret)
> >> +		goto probe_failed;
> >> +
> >>  	/* If using pinctrl, bind pins now before probing */
> >>  	ret = pinctrl_bind_pins(dev);
> >>  	if (ret)
> > 
> > I'm very concerned about this approach. Hooking into the core probe
> > path for things like this is not going to scale well. I'm not thrilled
> > with the pinctrl hook being here either, but that is already merged. :-(
> > Also, hooking in here is going affect every single device device driver
> > probe path, and a large number of them are never, ever, going to have
> > iommu interactions.
> > 
> > There needs to be a less invasive way of doing what you want. I still
> > feel like the individual device drivers themselves need to be aware that
> > they might be hooking through an IOMMU. Or, if they are hooking through
> > a bus like PCIe, then have the PCIe bus defer creating child devices
> > until the IOMMU is configured and in place.
> 
> I general though, couldn't any MMIO on-SoC device potentially be
> affected by an IOMMU? The point of putting the call to of_iommu_attach()
> here rather than inside individual driver's probe() is to avoid
> requiring "every" driver having to paste more boiler-plate into probe().

It seems more that IOMMU attachment is closer to being a property of the
bus rather than a property of the device itself. In that context it
would make more sense for the bus device to hold off child device
registration or probing until the IOMMU is available. That keeps the
logic out of both the core code and the individual device drivers.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/