| Message ID | 1384158718-4756-3-git-send-email-hdoyu@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 11, 2013 at 08:31:53AM +0000, Hiroshi Doyu wrote: > An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" > are done later. > > With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to > identify whether a device is IOMMU'able or not. If a device is > IOMMU'able, 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'able devices are > populated and configured as IOMMU'abled with help of the already > populated iommu device via iommu_ops->add_device(). > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> > --- > Update: > 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 This generally looks like the right idea to me, but it would be good to have the input from a DT maintainer on the use of "#stream-id-cells" as an indicator that a device is behind an IOMMU. (aside: I think "iommuable" is a horrible word!). What happens if you do the deferring at the bus level? E.g. defer all device probes on a bus, until the IOMMU is probed for that bus. That might fit better with future work, where we will almost certainly need to expose more of the bus topology to Linux. Of course, I can see that falling down for monolithic virtual buses like the platform bus. Will
On 11/11/2013 04:39 AM, Will Deacon wrote: > On Mon, Nov 11, 2013 at 08:31:53AM +0000, Hiroshi Doyu wrote: >> An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" >> are done later. >> >> With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to >> identify whether a device is IOMMU'able or not. If a device is >> IOMMU'able, 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'able devices are >> populated and configured as IOMMU'abled with help of the already >> populated iommu device via iommu_ops->add_device(). >> >> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> >> --- >> Update: >> 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 > > This generally looks like the right idea to me, but it would be good to have > the input from a DT maintainer on the use of "#stream-id-cells" as an indicator > that a device is behind an IOMMU. (aside: I think "iommuable" is a horrible > word!). > > What happens if you do the deferring at the bus level? E.g. defer all device > probes on a bus, until the IOMMU is probed for that bus. That might fit > better with future work, where we will almost certainly need to expose more > of the bus topology to Linux. Of course, I can see that falling down for > monolithic virtual buses like the platform bus. I don't think it's correct to think about "the IOMMU for the bus". There could easily be multiple different IOMMU slave-sides attached to a bus, and hence you'd need to defer probing until "all the IOMMs for the bus" to be available. Equally, I assume that dev->bus->iommu_ops rather assumes that bus masters always master transactions onto the same bus that their control registers are slaves upon. That also doesn't seem like a reasonable assumption. As such, I think an approach where each device waits for any IOMMUs that affect it (wherever/whatever and however many they may be) is better than one where we try to explicitly manage the probe order of devices based on their slave registers' location.
On 11/11/2013 01:31 AM, Hiroshi Doyu wrote: > An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" > are done later. > > With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to > identify whether a device is IOMMU'able or not. If a device is > IOMMU'able, 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'able devices are > populated and configured as IOMMU'abled with help of the already > populated iommu device via iommu_ops->add_device(). This looks fairly neat and clean. I'm still worried about using #stream-id-cells in DT nodes though. While I do understand that the *Linux* device model currently only allows each struct device to be affected by a single IOMMU, I worry that encoding that same restriction into DT is a mistake. I'd far rather see a property like: SMMU: smmu: smmu@xxxxxx { #smmu-cells = <1>; } Affected device: smmus = <&smmu 1>; (perhaps with smmu-names too) That would allow the DT to represent basically arbitrary HW configurations. The implementation of this patch would then be almost as trivial; you'd just need to walk the smmus property to find each phandle in turn, just like any other phandle+specifier property, and validate that the SMMU driver was already probe()d for each.
On Wed, 13 Nov 2013 00:34:20 +0100 Stephen Warren <swarren@wwwdotorg.org> wrote: > On 11/11/2013 01:31 AM, Hiroshi Doyu wrote: > > An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" > > are done later. > > > > With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to > > identify whether a device is IOMMU'able or not. If a device is > > IOMMU'able, 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'able devices are > > populated and configured as IOMMU'abled with help of the already > > populated iommu device via iommu_ops->add_device(). > > This looks fairly neat and clean. > > I'm still worried about using #stream-id-cells in DT nodes though. While > I do understand that the *Linux* device model currently only allows each > struct device to be affected by a single IOMMU, I worry that encoding > that same restriction into DT is a mistake. I'd far rather see a > property like: > > SMMU: > smmu: smmu@xxxxxx { > #smmu-cells = <1>; > } > > Affected device: > smmus = <&smmu 1>; > (perhaps with smmu-names too) > > That would allow the DT to represent basically arbitrary HW configurations. True, and also can solve multi IOMMU problem as well. > The implementation of this patch would then be almost as trivial; you'd > just need to walk the smmus property to find each phandle in turn, just > like any other phandle+specifier property, and validate that the SMMU > driver was already probe()d for each. This seems to be almost same as the previous v3 DT bindings, and if we introduce 64 bitmap newly, this DT bindings would be something like below: smmu: iommu@xxxxxx { #smmu-cells = <2>; ...... }; host1x { compatible = "nvidia,tegra30-host1x", "simple-bus"; nvidia,memory-clients = <&smmu 0x??????? 0x???????>; .... gr3d { compatible = "nvidia,tegra30-gr3d"; nvidia,memory-clients = <&smmu 0x??????? 0x???????>; } If a device is attached to multiple IOMMU H/Ws, gr3d { compatible = "nvidia,tegra30-gr3d"; nvidia,memory-clients = <&smmu 0x??????? 0x??????? &gart 0x???????>; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Then, the problem is the binding "name" and its "scope". This original patch works with "#stream-id-cells" in driver core because I assumed that "#stream-id-cells" can indicate globally that a device can be an IOMMU master. We may be able to have some kind of callback function which checks "#stream-id-cells" *in* SMMU driver, but at least this function needs to be registered in the bus at very early stage, iommu_ops->is_iommu_master(). But this cannot be done with bus->iommu_ops since bus->iommu_ops is set after IOMMU is populated. A kind of Chikin or the egg problem. Having a global bindings which indicates a device's IOMMU master'ability is quite convenient. For example, "iommu" and "#iommu-cells" without refering any local data. Then the above DT would be: smmu: iommu@xxxxxx { #iommu-cells = <2>; ^^^^^^^^^^^^^^^^^^ }; host1x { compatible = "nvidia,tegra30-host1x", "simple-bus"; iommu = <&smmu 0x??????? 0x???????>; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ gr3d { compatible = "nvidia,tegra30-gr3d"; iommu = <&smmu 0x??????? 0x???????>; } What do you think to have a global IOMMU DT bindings?
On Tue, Nov 12, 2013 at 11:34:20PM +0000, Stephen Warren wrote: > On 11/11/2013 01:31 AM, Hiroshi Doyu wrote: > > An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" > > are done later. > > > > With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to > > identify whether a device is IOMMU'able or not. If a device is > > IOMMU'able, 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'able devices are > > populated and configured as IOMMU'abled with help of the already > > populated iommu device via iommu_ops->add_device(). > > This looks fairly neat and clean. > > I'm still worried about using #stream-id-cells in DT nodes though. While > I do understand that the *Linux* device model currently only allows each > struct device to be affected by a single IOMMU, I worry that encoding > that same restriction into DT is a mistake. I'd far rather see a > property like: > > SMMU: > smmu: smmu@xxxxxx { > #smmu-cells = <1>; > } > > Affected device: > smmus = <&smmu 1>; > (perhaps with smmu-names too) > > That would allow the DT to represent basically arbitrary HW configurations. > > The implementation of this patch would then be almost as trivial; you'd > just need to walk the smmus property to find each phandle in turn, just > like any other phandle+specifier property, and validate that the SMMU > driver was already probe()d for each. There are a few problems with that: 1.) It assumes all devices sharing an SMMU have the same number of "smmu cells" 2.) It moves SMMU-specific data out to the device, which makes it impossible to describe more complicated topologies where IDs can be remapped/remastered, potentially by multiple SMMUs and/or bus bridges. When writing the binding for the ARM SMMU driver, I originally started with something similar to what you're suggesting, but was later forced down a different route when I realised what sort of systems were being built. We will have similar problems for PCIe RIDs and GIC DIDs (I spoke about this at the ARM mini-summit). They are not fixed across the system: they originate from a device, but can change as they traverse the system topology. Will
Will Deacon <will.deacon@arm.com> wrote @ Wed, 13 Nov 2013 15:38:04 +0100: > On Tue, Nov 12, 2013 at 11:34:20PM +0000, Stephen Warren wrote: > > On 11/11/2013 01:31 AM, Hiroshi Doyu wrote: > > > An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" > > > are done later. > > > > > > With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to > > > identify whether a device is IOMMU'able or not. If a device is > > > IOMMU'able, 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'able devices are > > > populated and configured as IOMMU'abled with help of the already > > > populated iommu device via iommu_ops->add_device(). > > > > This looks fairly neat and clean. > > > > I'm still worried about using #stream-id-cells in DT nodes though. While > > I do understand that the *Linux* device model currently only allows each > > struct device to be affected by a single IOMMU, I worry that encoding > > that same restriction into DT is a mistake. I'd far rather see a > > property like: > > > > SMMU: > > smmu: smmu@xxxxxx { > > #smmu-cells = <1>; > > } > > > > Affected device: > > smmus = <&smmu 1>; > > (perhaps with smmu-names too) > > > > That would allow the DT to represent basically arbitrary HW configurations. > > > > The implementation of this patch would then be almost as trivial; you'd > > just need to walk the smmus property to find each phandle in turn, just > > like any other phandle+specifier property, and validate that the SMMU > > driver was already probe()d for each. > > There are a few problems with that: > > 1.) It assumes all devices sharing an SMMU have the same number of > "smmu cells" This can be solved with introducing the fixed size of bitmap. The size of bitmap can be fixed even per SoC. In tegra we used 64(2 cells) which I expect at most. > 2.) It moves SMMU-specific data out to the device, which makes it > impossible to describe more complicated topologies where IDs can be > remapped/remastered, potentially by multiple SMMUs and/or bus bridges. > > When writing the binding for the ARM SMMU driver, I originally started with > something similar to what you're suggesting, but was later forced down a > different route when I realised what sort of systems were being built. > > We will have similar problems for PCIe RIDs and GIC DIDs (I spoke about this > at the ARM mini-summit). They are not fixed across the system: they > originate from a device, but can change as they traverse the system > topology. Is there any chance to overwrite SMMU driver specific params during setting up topologies?
On Wed, Nov 13, 2013 at 04:06:10PM +0000, Hiroshi Doyu wrote: > Will Deacon <will.deacon@arm.com> wrote @ Wed, 13 Nov 2013 15:38:04 +0100: > > > On Tue, Nov 12, 2013 at 11:34:20PM +0000, Stephen Warren wrote: > > > SMMU: > > > smmu: smmu@xxxxxx { > > > #smmu-cells = <1>; > > > } > > > > > > Affected device: > > > smmus = <&smmu 1>; > > > (perhaps with smmu-names too) > > > > > > That would allow the DT to represent basically arbitrary HW configurations. > > > > > > The implementation of this patch would then be almost as trivial; you'd > > > just need to walk the smmus property to find each phandle in turn, just > > > like any other phandle+specifier property, and validate that the SMMU > > > driver was already probe()d for each. > > > > There are a few problems with that: > > > > 1.) It assumes all devices sharing an SMMU have the same number of > > "smmu cells" > > This can be solved with introducing the fixed size of bitmap. The size > of bitmap can be fixed even per SoC. In tegra we used 64(2 cells) > which I expect at most. That really doesn't sound like a good idea where you have bridges (like a PCIe host controller) which could have a significant chunk of StreamID space. You'd also need to pad everything out with some dummy IDs for parsing purposes. Yuck! > > 2.) It moves SMMU-specific data out to the device, which makes it > > impossible to describe more complicated topologies where IDs can be > > remapped/remastered, potentially by multiple SMMUs and/or bus bridges. > > > > When writing the binding for the ARM SMMU driver, I originally started with > > something similar to what you're suggesting, but was later forced down a > > different route when I realised what sort of systems were being built. > > > > We will have similar problems for PCIe RIDs and GIC DIDs (I spoke about this > > at the ARM mini-summit). They are not fixed across the system: they > > originate from a device, but can change as they traverse the system > > topology. > > Is there any chance to overwrite SMMU driver specific params during > setting up topologies? Not sure I understand what you're getting at here. Could you elaborate please? Will
On 11/13/2013 07:38 AM, Will Deacon wrote: > On Tue, Nov 12, 2013 at 11:34:20PM +0000, Stephen Warren wrote: >> On 11/11/2013 01:31 AM, Hiroshi Doyu wrote: >>> An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" >>> are done later. >>> >>> With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to >>> identify whether a device is IOMMU'able or not. If a device is >>> IOMMU'able, 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'able devices are >>> populated and configured as IOMMU'abled with help of the already >>> populated iommu device via iommu_ops->add_device(). >> >> This looks fairly neat and clean. >> >> I'm still worried about using #stream-id-cells in DT nodes though. While >> I do understand that the *Linux* device model currently only allows each >> struct device to be affected by a single IOMMU, I worry that encoding >> that same restriction into DT is a mistake. I'd far rather see a >> property like: >> >> SMMU: >> smmu: smmu@xxxxxx { >> #smmu-cells = <1>; >> } >> >> Affected device: >> smmus = <&smmu 1>; >> (perhaps with smmu-names too) >> >> That would allow the DT to represent basically arbitrary HW configurations. >> >> The implementation of this patch would then be almost as trivial; you'd >> just need to walk the smmus property to find each phandle in turn, just >> like any other phandle+specifier property, and validate that the SMMU >> driver was already probe()d for each. > > There are a few problems with that: > > 1.) It assumes all devices sharing an SMMU have the same number of > "smmu cells" The SMMU HW defines how many cells are required to represent the client stream IDs. So, this isn't a problem. Are you thinking of cases where an SMMU client issues transactions with multiple different stream IDs? That is supported by having multiple entries in the smmus property. (Assuming that #smmu-cells in the SMMU is <1>) HW that issues with 1 stream ID: smmus = <&smmu 123>; HW that issues with 2 stream IDs: smmus = <&smmu 123 &smmu 345>; > 2.) It moves SMMU-specific data out to the device, which makes it Yes, but I think it's really SMMU-client-specific data not SMMU-specific. The SMMU doesn't dictate the stream IDs that "clients" include with their transactions; the "client" HW dictates that. > impossible to describe more complicated topologies where IDs can be > remapped/remastered, potentially by multiple SMMUs and/or bus bridges. I don't think so. The SMMU "clients" can indicate what stream IDs they use to issue transactions. The SMMU DT node or driver itself can still include a table that describes how it re-writes those stream IDs when forwarding transactions to the next step. I don't believe there's any requirement that the SMMU list all its clients and this mapping table in the same property. So, the SMMU could easily have: (entries are this SMMU's #stream-id-cells, assumed to be 1 here, then the next SMMU's #stream-id-cells, assumed to be 2 here) smmu-stream-id-translations = <123 1 987>, <345 0 765>; > When writing the binding for the ARM SMMU driver, I originally started with > something similar to what you're suggesting, but was later forced down a > different route when I realised what sort of systems were being built. > > We will have similar problems for PCIe RIDs and GIC DIDs (I spoke about this > at the ARM mini-summit). They are not fixed across the system: they > originate from a device, but can change as they traverse the system > topology.
On 11/13/2013 12:23 AM, Hiroshi Doyu wrote: > On Wed, 13 Nov 2013 00:34:20 +0100 > Stephen Warren <swarren@wwwdotorg.org> wrote: > >> On 11/11/2013 01:31 AM, Hiroshi Doyu wrote: >>> An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" >>> are done later. >>> >>> With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to >>> identify whether a device is IOMMU'able or not. If a device is >>> IOMMU'able, 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'able devices are >>> populated and configured as IOMMU'abled with help of the already >>> populated iommu device via iommu_ops->add_device(). >> >> This looks fairly neat and clean. >> >> I'm still worried about using #stream-id-cells in DT nodes though. While >> I do understand that the *Linux* device model currently only allows each >> struct device to be affected by a single IOMMU, I worry that encoding >> that same restriction into DT is a mistake. I'd far rather see a >> property like: >> >> SMMU: >> smmu: smmu@xxxxxx { >> #smmu-cells = <1>; >> } >> >> Affected device: >> smmus = <&smmu 1>; >> (perhaps with smmu-names too) >> >> That would allow the DT to represent basically arbitrary HW configurations. > > True, and also can solve multi IOMMU problem as well. > >> The implementation of this patch would then be almost as trivial; you'd >> just need to walk the smmus property to find each phandle in turn, just >> like any other phandle+specifier property, and validate that the SMMU >> driver was already probe()d for each. > > This seems to be almost same as the previous v3 DT bindings, and if we > introduce 64 bitmap newly, this DT bindings would be something like > below: > > smmu: iommu@xxxxxx { > #smmu-cells = <2>; > ...... > }; > > host1x { > compatible = "nvidia,tegra30-host1x", "simple-bus"; > nvidia,memory-clients = <&smmu 0x??????? 0x???????>; > .... > gr3d { > compatible = "nvidia,tegra30-gr3d"; > nvidia,memory-clients = <&smmu 0x??????? 0x???????>; > } > > If a device is attached to multiple IOMMU H/Ws, > > gr3d { > compatible = "nvidia,tegra30-gr3d"; > nvidia,memory-clients = <&smmu 0x??????? 0x??????? &gart 0x???????>; > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Then, the problem is the binding "name" and its "scope". This original > patch works with "#stream-id-cells" in driver core because I assumed > that "#stream-id-cells" can indicate globally that a device can be an > IOMMU master. For example, the pinctrl bindings have the same issue, since they're interpreted "globally", and by (code called from) the generic device probing code. We simply decided that the properties "pinctrl-names" and "pinctrl-n" (n=0...) were globally defined by the pinctrl subsystem, and hence could be parsed in any node. We could do the same with "smmus" and "smmu-names" in this case. > We may be able to have some kind of callback function which checks > "#stream-id-cells" *in* SMMU driver, but at least this function needs to > be registered in the bus at very early stage, iommu_ops->is_iommu_master(). > But this cannot be done with bus->iommu_ops since bus->iommu_ops is set > after IOMMU is populated. A kind of Chikin or the egg problem. I think this is simply the normal deferred probe. When device X attempts to probe, the core SMMU code (called from the core device probing code) iterates over the smmus property. If any of the phandles listed there don't have a registered SMMU driver, then defer probe of device X. Eventually, the SMMU driver will appear, and the driver core will attempt to re-probe device X, and all the SMMUs have devices probed already, and everything will work.
On 11/13/2013 10:31 AM, Will Deacon wrote: > On Wed, Nov 13, 2013 at 04:06:10PM +0000, Hiroshi Doyu wrote: >> Will Deacon <will.deacon@arm.com> wrote @ Wed, 13 Nov 2013 15:38:04 +0100: >> >>> On Tue, Nov 12, 2013 at 11:34:20PM +0000, Stephen Warren wrote: >>>> SMMU: >>>> smmu: smmu@xxxxxx { >>>> #smmu-cells = <1>; >>>> } >>>> >>>> Affected device: >>>> smmus = <&smmu 1>; >>>> (perhaps with smmu-names too) >>>> >>>> That would allow the DT to represent basically arbitrary HW configurations. >>>> >>>> The implementation of this patch would then be almost as trivial; you'd >>>> just need to walk the smmus property to find each phandle in turn, just >>>> like any other phandle+specifier property, and validate that the SMMU >>>> driver was already probe()d for each. >>> >>> There are a few problems with that: >>> >>> 1.) It assumes all devices sharing an SMMU have the same number of >>> "smmu cells" >> >> This can be solved with introducing the fixed size of bitmap. The size >> of bitmap can be fixed even per SoC. In tegra we used 64(2 cells) >> which I expect at most. > > That really doesn't sound like a good idea where you have bridges (like a > PCIe host controller) which could have a significant chunk of StreamID > space. You'd also need to pad everything out with some dummy IDs for parsing > purposes. Yuck! Can't you solve this by having the SMMU include a stream ID mapping table. In the case of stream IDs having a large "address" space, then just define the mapping table syntax to allow easy specification of ranges using start/length, start/end, or value/mask pairs. Similar to ranges or PCI's interrupt-map{,-mask}.
On Wed, Nov 13, 2013 at 05:53:36PM +0000, Stephen Warren wrote: > On 11/13/2013 10:31 AM, Will Deacon wrote: > > On Wed, Nov 13, 2013 at 04:06:10PM +0000, Hiroshi Doyu wrote: > >> This can be solved with introducing the fixed size of bitmap. The size > >> of bitmap can be fixed even per SoC. In tegra we used 64(2 cells) > >> which I expect at most. > > > > That really doesn't sound like a good idea where you have bridges (like a > > PCIe host controller) which could have a significant chunk of StreamID > > space. You'd also need to pad everything out with some dummy IDs for parsing > > purposes. Yuck! > > Can't you solve this by having the SMMU include a stream ID mapping > table. In the case of stream IDs having a large "address" space, then > just define the mapping table syntax to allow easy specification of > ranges using start/length, start/end, or value/mask pairs. Similar to > ranges or PCI's interrupt-map{,-mask}. That may work for windows, but the mapping from input stream IDs to output IDs can be arbitrary (I *really* hope people stick to windows for requester IDs though). Will
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) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index ee249bc..335bf6a 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -20,6 +20,8 @@ #include <linux/export.h> #include <linux/limits.h> #include <linux/of.h> +#include <linux/device.h> +#include <linux/iommu.h> /** * of_get_dma_window - Parse *dma-window property and returns 0 if found. @@ -88,3 +90,34 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, return 0; } EXPORT_SYMBOL_GPL(of_get_dma_window); + +static bool of_is_iommuable(struct device *dev) +{ + size_t bytes; + const __be32 *prop; + const char *propname = "#stream-id-cells"; + + prop = of_get_property(dev->of_node, propname, &bytes); + if (!prop || !bytes) + return false; + + pr_debug("%s=%d %s\n", propname, bytes, dev_name(dev)); + return true; +} + +int of_iommu_attach(struct device *dev) +{ + struct iommu_ops *ops; + + if (!of_is_iommuable(dev)) + return 0; + + ops = dev->bus->iommu_ops; + if (!ops) + return -EPROBE_DEFER; + + if (ops->add_device) + return ops->add_device(dev); + + return 0; +} diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h index 51a560f..3457489 100644 --- a/include/linux/of_iommu.h +++ b/include/linux/of_iommu.h @@ -7,6 +7,8 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix, int index, unsigned long *busno, dma_addr_t *addr, size_t *size); +extern int of_iommu_attach(struct device *dev); + #else static inline int of_get_dma_window(struct device_node *dn, const char *prefix, @@ -16,6 +18,11 @@ static inline int of_get_dma_window(struct device_node *dn, const char *prefix, return -EINVAL; } +static inline int of_iommu_attach(struct device *dev) +{ + return 0; +} + #endif /* CONFIG_OF_IOMMU */ #endif /* __OF_IOMMU_H */
An "IOMMU device" on the bus is poplulated first, "IOMMU'able devices" are done later. With CONFIG_OF_IOMMU, "#stream-id-cells" DT binding would be used to identify whether a device is IOMMU'able or not. If a device is IOMMU'able, 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'able devices are populated and configured as IOMMU'abled with help of the already populated iommu device via iommu_ops->add_device(). Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> --- Update: 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 | 33 +++++++++++++++++++++++++++++++++ include/linux/of_iommu.h | 7 +++++++ 3 files changed, 45 insertions(+)