Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Friday 30 May 2014 22:29:13 Hiroshi Doyu wrote:
>
> IIUC the original problem, "a master with 8 streamIDs" means something
> like below, where some devices have multiple IDs but some have a
> single. A sinle #address-cells cannot afford those 2 masters at once.
>
>iommu {
>/* the specifier represents the ID of the master */
>#address-cells = <1>;
>#size-cells = <0>;
>};
>
> master@a {
> ...
> iommus = <&smmu 1 2 3>; # 3 IDs
> };
>
> master@b {
> ...
> iommus = <&smmu 4>; # 1 ID
> };
This would not be the usual format really. It should instead be
iommus = <&smmu 1>, <&smmu 2>, <&smmu 3>;
which can be tedious to type.
> Tegra,SMMU has a similar problem and we have used a fixed size bitmap(64
> bit) to afford 64 stream IDs so that a single device can hold multiple
> IDs. If we apply the same bitmap to the above exmaple:
>
>iommu {
>/* the specifier represents the ID of the master */
>#address-cells = <1>;
>#size-cells = <0>;
>};
>
> master@a {
> ...
> iommus = <&smmu (BIT(1) | BIT(2) | BIT(3))>; # IDs 1 2 3
> };
>
> master@b {
> ...
> iommus = <&smmu BIT(4)>; # ID 4
> };
>
> The disadvantage of this is that this limits the max number of streamIDs
> to support. If # of streamID is increased later more than 64, this
> format cannot cover any more. You have to predict the max # of streamIDs
> in advance if steamID is statically assigned.
>
Well, the iommu specific binding could allow a variable #address-cells.
That way, you just need to know the number of stream IDs for that instance
of the iommu.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Friday 30 May 2014 14:31:55 Rob Herring wrote:
> On Fri, May 30, 2014 at 2:06 PM, Arnd Bergmann wrote:
> > On Friday 30 May 2014 08:16:05 Rob Herring wrote:
> >> On Fri, May 23, 2014 at 3:33 PM, Thierry Reding
> >> wrote:
> >> > From: Thierry Reding
> >> > +IOMMU master node:
> >> > +==
> >> > +
> >> > +Devices that access memory through an IOMMU are called masters. A
> >> > device can
> >> > +have multiple master interfaces (to one or more IOMMU devices).
> >> > +
> >> > +Required properties:
> >> > +
> >> > +- iommus: A list of phandle and IOMMU specifier pairs that describe the
> >> > IOMMU
> >> > + master interfaces of the device. One entry in the list describes one
> >> > master
> >> > + interface of the device.
> >> > +
> >> > +When an "iommus" property is specified in a device tree node, the IOMMU
> >> > will
> >> > +be used for address translation. If a "dma-ranges" property exists in
> >> > the
> >> > +device's parent node it will be ignored. An exception to this rule is
> >> > if the
> >> > +referenced IOMMU is disabled, in which case the "dma-ranges" property
> >> > of the
> >> > +parent shall take effect.
> >>
> >> Just thinking out loud, could you have dma-ranges in the iommu node
> >> for the case when the iommu is enabled rather than putting the DMA
> >> window information into the iommus property?
> >>
> >> This would probably mean that you need both #iommu-cells and
> >> #address-cells.
> >
> > The reason for doing like this was that you may need a different window
> > for each device, while there can only be one dma-ranges property in
> > an iommu node.
>
> My suggestion was that you also put the IDs in the dma-ranges as the
> first cell much as ranges for PCI encodes other information in the
> first cell. Then you can have an entry for each ID. The downside is
> another special case like PCI.
>
> The argument for using #address-cells and #size-cells seems to be to
> align with how ranges work. If that's the route we want to go, then I
> say we should not stop there and actually use dma-ranges as well.
> Otherwise, I don't see the advantage over #iommu-cells.
I can see how dma-ranges in bus nodes work, it just doesn't seem to
have any reasonable meaning in the iommu node itself.
> > I don't understand the problem. If you have stream IDs 0 through 7,
> > you would have
> >
> > master@a {
> > ...
> > iommus = <&smmu 0>;
> > };
> >
> > master@b {
> > ...
> > iommus = <&smmu 1;
> > };
> >
> > ...
> >
> > master@12 {
> > ...
> > iommus = <&smmu 7;
> > };
> >
> > and you don't need a window at all. Why would you need a mask of
> > some sort?
>
> 1 master with 7 IDs like this:
>
> master@a {
> ...
> iommus = <&smmu 0> <&smmu 1> <&smmu 2> <&smmu 3>
> <&smmu 4> <&smmu 5> <&smmu 6> <&smmu 7>;
> };
>
> If there was any sort of window, then it is almost certainly the same
> window for each ID.
Ok, I see. In that case you'd probably want to have #address-cells = <1>
and #size-cells = <1> and give a range of IDs like
iommus = <&smmu 0 8>;
Do you think that ranges can have a meaningful definition with the ARM
SMMU stream IDs?
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Fri, May 30, 2014 at 2:06 PM, Arnd Bergmann wrote:
> On Friday 30 May 2014 08:16:05 Rob Herring wrote:
>> On Fri, May 23, 2014 at 3:33 PM, Thierry Reding
>> wrote:
>> > From: Thierry Reding
>> > +IOMMU master node:
>> > +==
>> > +
>> > +Devices that access memory through an IOMMU are called masters. A device
>> > can
>> > +have multiple master interfaces (to one or more IOMMU devices).
>> > +
>> > +Required properties:
>> > +
>> > +- iommus: A list of phandle and IOMMU specifier pairs that describe the
>> > IOMMU
>> > + master interfaces of the device. One entry in the list describes one
>> > master
>> > + interface of the device.
>> > +
>> > +When an "iommus" property is specified in a device tree node, the IOMMU
>> > will
>> > +be used for address translation. If a "dma-ranges" property exists in the
>> > +device's parent node it will be ignored. An exception to this rule is if
>> > the
>> > +referenced IOMMU is disabled, in which case the "dma-ranges" property of
>> > the
>> > +parent shall take effect.
>>
>> Just thinking out loud, could you have dma-ranges in the iommu node
>> for the case when the iommu is enabled rather than putting the DMA
>> window information into the iommus property?
>>
>> This would probably mean that you need both #iommu-cells and #address-cells.
>
> The reason for doing like this was that you may need a different window
> for each device, while there can only be one dma-ranges property in
> an iommu node.
My suggestion was that you also put the IDs in the dma-ranges as the
first cell much as ranges for PCI encodes other information in the
first cell. Then you can have an entry for each ID. The downside is
another special case like PCI.
The argument for using #address-cells and #size-cells seems to be to
align with how ranges work. If that's the route we want to go, then I
say we should not stop there and actually use dma-ranges as well.
Otherwise, I don't see the advantage over #iommu-cells.
>> > +
>> > +Optional properties:
>> > +
>> > +- iommu-names: A list of names identifying each entry in the "iommus"
>> > + property.
>>
>> Do we really need a name here? I would not expect that you have
>> clearly documented names here from the datasheet like you would for
>> interrupts or clocks, so you'd just be making up names. Sorry, but I'm
>> not a fan of names properties in general.
>
> Good point, this was really overdesign by modeling it after other
> subsystems that can have a use for names.
>
>> > +Multiple-master IOMMU:
>> > +--
>> > +
>> > + iommu {
>> > + /* the specifier represents the ID of the master */
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > + };
>> > +
>> > + master {
>> > + /* device has master ID 42 in the IOMMU */
>> > + iommus = <&/iommu 42>;
>> > + };
>>
>> Presumably the ID would be the streamID on ARM's SMMU. How would a
>> master with 8 streamIDs be described? This is what Calxeda midway has
>> for SATA and I would expect that to be somewhat common. Either you
>> need some ID masking or you'll have lots of duplication when you have
>> windows.
>
> I don't understand the problem. If you have stream IDs 0 through 7,
> you would have
>
> master@a {
> ...
> iommus = <&smmu 0>;
> };
>
> master@b {
> ...
> iommus = <&smmu 1;
> };
>
> ...
>
> master@12 {
> ...
> iommus = <&smmu 7;
> };
>
> and you don't need a window at all. Why would you need a mask of
> some sort?
1 master with 7 IDs like this:
master@a {
...
iommus = <&smmu 0> <&smmu 1> <&smmu 2> <&smmu 3>
<&smmu 4> <&smmu 5> <&smmu 6> <&smmu 7>;
};
If there was any sort of window, then it is almost certainly the same
window for each ID.
Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
Arnd Bergmann writes:
>> > +Multiple-master IOMMU:
>> > +--
>> > +
>> > + iommu {
>> > + /* the specifier represents the ID of the master */
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > + };
>> > +
>> > + master {
>> > + /* device has master ID 42 in the IOMMU */
>> > + iommus = <&/iommu 42>;
>> > + };
>>
>> Presumably the ID would be the streamID on ARM's SMMU. How would a
>> master with 8 streamIDs be described? This is what Calxeda midway has
>> for SATA and I would expect that to be somewhat common. Either you
>> need some ID masking or you'll have lots of duplication when you have
>> windows.
>
> I don't understand the problem. If you have stream IDs 0 through 7,
> you would have
>
> master@a {
> ...
> iommus = <&smmu 0>;
> };
>
> master@b {
> ...
> iommus = <&smmu 1;
> };
>
> ...
>
> master@12 {
> ...
> iommus = <&smmu 7;
> };
>
> and you don't need a window at all. Why would you need a mask of
> some sort?
IIUC the original problem, "a master with 8 streamIDs" means something
like below, where some devices have multiple IDs but some have a
single. A sinle #address-cells cannot afford those 2 masters at once.
iommu {
/* the specifier represents the ID of the master */
#address-cells = <1>;
#size-cells = <0>;
};
master@a {
...
iommus = <&smmu 1 2 3>; # 3 IDs
};
master@b {
...
iommus = <&smmu 4>; # 1 ID
};
Tegra,SMMU has a similar problem and we have used a fixed size bitmap(64
bit) to afford 64 stream IDs so that a single device can hold multiple
IDs. If we apply the same bitmap to the above exmaple:
iommu {
/* the specifier represents the ID of the master */
#address-cells = <1>;
#size-cells = <0>;
};
master@a {
...
iommus = <&smmu (BIT(1) | BIT(2) | BIT(3))>; # IDs 1 2 3
};
master@b {
...
iommus = <&smmu BIT(4)>; # ID 4
};
The disadvantage of this is that this limits the max number of streamIDs
to support. If # of streamID is increased later more than 64, this
format cannot cover any more. You have to predict the max # of streamIDs
in advance if steamID is statically assigned.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Friday 30 May 2014 12:27:28 Dave Martin wrote: > On Fri, May 30, 2014 at 08:30:08AM +0100, Thierry Reding wrote: > > On Thu, May 29, 2014 at 09:52:22AM -0600, Stephen Warren wrote: > > > On 05/23/2014 02:36 PM, Thierry Reding wrote: > > > I think this is a mistake. address-cells/size-cells are for transactions > > > flowing down the bus (from the CPU to date). Describing a connection > > > from a device to an IOMMU is something completely different, and should > > > therefore simply use an iommu-cells property to describe any necessary > > > information. If we start re-using properties for different things in > > > different contexts, how is anyone going to know what they mean, and how > > > will conflicts be resolved. For example, what if there's a single HW > > > module that both acts as a regular register bus with children (where > > > address-cells/size-cells defines how transactions reach the children > > > from the parent), and is also an IOMMU (where according to this binding > > > proposal, address-cells/size-cells represent some aspect of the IOMMU > > > feature). Using different properties for different things is the only > > > sane way to keep different concepts separate. Another alternative would > > > be to represent the single HW module as separate nodes in DT, but I > > > think that will only make our lives harder, and where I've done that in > > > the past, I've regretted it. > > > > There was some back-and-forth on this topic and the latest concensus > > when I wrote the second version was that #address-cells and #size-cells > > were to be used. > > > > But there was some bore back-and-forth after that, and it seems like > > Arnd no longer thinks that using #address-cells and #size-cells is a > > good idea either[0]. > > Mistake or not, ePAPR already (ab)uses the address concept for PCI. > Unless ePAPR is wrong, I don't think it makes sense to argue that > the address concept cannot be repurposed. > > The reason why this is abused for PCI is the same as our reason here: > different masters really are treated as distinct even when accessing > the same destination address, and DT has no general native way to > describe that. > > One clear advantage of using #address-cells etc. is that ePAPR already > has very clear and well-defined ways of how to specify range mappings. > This gives us a ready-made way to describe windowed IOMMUs and 1:1 > remappings of whole blocks of master IDs, which are the most obvious > cases other than having a small-integer set of explicit IDs. > > That said, the PCI pseudo-address thing is not something we _necessarily_ > want to repeat. I'm really impartial to the question of whether to use #address-cells or #iommus now. It's possible that we can use the addresses in some meaningful way, but it's not clear to me that this will help make the representation cleaner or that we will actually want it for SMMU. > > Arnd, can you take another look at this binding and see if there's > > anything else missing? If not I'll go through the document again and > > update all #address-cells/#size-cells references with #iommu-cells as > > appropriate and submit v3. > > How do you envisage propagation of the master ID bits downstream of the > IOMMU would be described? > > We will definitely need a way to describe this for GICv3. How those > values are propagated is likely to vary between related SoCs and doesn't > feel like it should be baked into a driver, especially for the ARM SMMU > which may get reused in radically different SoC families from different > vendors. > > The most likely types of remapping are the adding of a base offset or > some extra bits to the ID -- because not all MSIs to the GIC will > necessarily pass through the IOMMU. It's also possible that we might > see ID squashing or folding in some systems. > > > For types of remapping which mix the ID and address together, I now > do tend to agree that any flexibility arising from describing that > in a general way that is unlikely to repay the cost of trying to > interpret and analyse the DT. Defining a few sterotypical kinds > of mapping explicitly, as needed, looks more sensible for now. The > windowed-IOMMU case is one example. For MSI, my feeling is that we'd just be best off doing an end-to-end description. Any device using an MSI would have an interrupt specifier that is sufficient for the interrupt controller to understand where the IRQ comes from, and return an address/value pair back to the driver to program into some register. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Friday 30 May 2014 08:16:05 Rob Herring wrote:
> On Fri, May 23, 2014 at 3:33 PM, Thierry Reding
> wrote:
> > From: Thierry Reding
> > +IOMMU master node:
> > +==
> > +
> > +Devices that access memory through an IOMMU are called masters. A device
> > can
> > +have multiple master interfaces (to one or more IOMMU devices).
> > +
> > +Required properties:
> > +
> > +- iommus: A list of phandle and IOMMU specifier pairs that describe the
> > IOMMU
> > + master interfaces of the device. One entry in the list describes one
> > master
> > + interface of the device.
> > +
> > +When an "iommus" property is specified in a device tree node, the IOMMU
> > will
> > +be used for address translation. If a "dma-ranges" property exists in the
> > +device's parent node it will be ignored. An exception to this rule is if
> > the
> > +referenced IOMMU is disabled, in which case the "dma-ranges" property of
> > the
> > +parent shall take effect.
>
> Just thinking out loud, could you have dma-ranges in the iommu node
> for the case when the iommu is enabled rather than putting the DMA
> window information into the iommus property?
>
> This would probably mean that you need both #iommu-cells and #address-cells.
The reason for doing like this was that you may need a different window
for each device, while there can only be one dma-ranges property in
an iommu node.
> > +
> > +Optional properties:
> > +
> > +- iommu-names: A list of names identifying each entry in the "iommus"
> > + property.
>
> Do we really need a name here? I would not expect that you have
> clearly documented names here from the datasheet like you would for
> interrupts or clocks, so you'd just be making up names. Sorry, but I'm
> not a fan of names properties in general.
Good point, this was really overdesign by modeling it after other
subsystems that can have a use for names.
> > +Multiple-master IOMMU:
> > +--
> > +
> > + iommu {
> > + /* the specifier represents the ID of the master */
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > +
> > + master {
> > + /* device has master ID 42 in the IOMMU */
> > + iommus = <&/iommu 42>;
> > + };
>
> Presumably the ID would be the streamID on ARM's SMMU. How would a
> master with 8 streamIDs be described? This is what Calxeda midway has
> for SATA and I would expect that to be somewhat common. Either you
> need some ID masking or you'll have lots of duplication when you have
> windows.
I don't understand the problem. If you have stream IDs 0 through 7,
you would have
master@a {
...
iommus = <&smmu 0>;
};
master@b {
...
iommus = <&smmu 1;
};
...
master@12 {
...
iommus = <&smmu 7;
};
and you don't need a window at all. Why would you need a mask of
some sort?
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Friday 30 May 2014 12:22:32 Dave Martin wrote:
> > +
> > +Examples:
> > +=
> > +
> > +Single-master IOMMU:
> > +
> > +
> > + iommu {
> > + #address-cells = <0>;
> > + #size-cells = <0>;
> > + };
> > +
> > + master {
> > + iommus = <&/iommu>;
> > + };
> > +
> > +Multiple-master IOMMU with fixed associations:
> > +--
> > +
> > + /* multiple-master IOMMU */
> > + iommu {
> > + /*
> > +* Masters are statically associated with this IOMMU and
> > +* address translation is always enabled.
> > +*/
> > + #address-cells = <0>;
> > + #size-cells = <0>;
>
> In this example, can different translations be set up for the different
> masters?
>
> With no cells available to contain any sort of ID, it looks like this
> is not possible.
Correct, this example is for an IOMMU that does not use IDs but has a
shared address space for all devices.
> > +Multiple-master IOMMU with configurable DMA window:
> > +---
> > +
> > + / {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + iommu {
> > + /* master ID, address of DMA window */
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + };
> > +
> > + master {
> > + /* master ID 42, 4 GiB DMA window starting at 0 */
> > + iommus = <&/iommu 42 0 0x1 0x0>;
>
> I'm still concerned that in order to deal with future cases we will have
> to invent multiple ways to parse the "iommus" property. For example, if
> we have a PCEe RC mastering through an IOMMU, it will pass a huge set
> of possible master IDs to the IOMMU, not just noe or two.
>
> Do you have a solution in mind for that which doesn't break backwards
> compatibility?
I think we can treat PCI as a special case here and have an interface
that gets used by the PCI core code to talk to the IOMMU core code
when setting up a the dma_map_ops for a PCI function. As long as the
IOMMU driver understands what PCI is, we don't have to describe the
mapping in detail.
> One option is to include an extra cell to the IOMMUs property
> that indicates how to parse it. For now, only a single value would
> be defined. For example:
>
> iommus = <&/iommu IOMMU_SIMPLE 42>;
>
> Then maybe later
>
> iommus = <&/iommu IOMMU_RANGE 0x1 0x1>;
>
> (I'm not suggesting what IOMMU_RANGE might mean.)
>
This can really be left up to the specific IOMMU driver itself.
We can have drivers that support both #address-cells=<1>
and #address-cells=<2> and behave differently based on that.
I don't see a reason to define that across IOMMU implementations.
> Other options are to introduce a new property name
>
> range-iommus = <&/iommu 0x1 0x1>;
>
> or control the parsing of incompatible iommus properties via a compatible
> string somewhere.
Introducing a new compatible string is always an option as the last resort.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: msm: use devm_ioremap_resource to simplify code
On Tue, May 27, 2014 at 06:18:42PM +0800, Kefeng Wang wrote: > Use devm_ioremap_resource() to make the code simpler, drop unused variable, > redundant return value check, and error-handing code. > > Signed-off-by: Kefeng Wang > --- > drivers/iommu/msm_iommu_dev.c | 38 +++--- > 1 files changed, 7 insertions(+), 31 deletions(-) Applied to arm/msm, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] remove duplicate checking code
On Thu, May 29, 2014 at 03:45:47PM +0200, Paul Bolle wrote:
> On Mon, 2014-05-26 at 11:39 +0200, Joerg Roedel wrote:
> > On Fri, May 16, 2014 at 03:39:40PM +0800, Vaughan Cao wrote:
> > > amd_iommu_rlookup_table[devid] != NULL is already guaranteed by
> > > check_device
> > > called before, it's fine to attach device at this point.
> > >
> > > Signed-off-by: Vaughan Cao
> > > ---
> > > drivers/iommu/amd_iommu.c | 6 --
> > > 1 file changed, 6 deletions(-)
> >
> > Applied, thanks.
>
> This one entered linux-next, as of next-20142527, in commit ecef115d4540
> ("iommu/amd: Remove duplicate checking code"). It triggers an obviously
> correct warning:
> drivers/iommu/amd_iommu.c: In function ‘amd_iommu_init_passthrough’:
> drivers/iommu/amd_iommu.c:3503:6: warning: unused variable ‘devid’
> [-Wunused-variable]
> u16 devid;
> ^
> drivers/iommu/amd_iommu.c:3502:20: warning: unused variable ‘iommu’
> [-Wunused-variable]
> struct amd_iommu *iommu;
> ^
>
> Is the trivial fix for this queued somewhere?
Just queued one to my x86/amd branch. Thanks for the report.
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Fri, May 23, 2014 at 3:33 PM, Thierry Reding wrote: > From: Thierry Reding > > This commit introduces a generic device tree binding for IOMMU devices. > Only a very minimal subset is described here, but it is enough to cover > the requirements of both the Exynos System MMU and Tegra SMMU as > discussed here: > > https://lkml.org/lkml/2014/4/27/346 > > Signed-off-by: Thierry Reding > --- > Changes in v2: > - add notes about "dma-ranges" property (drop note from commit message) > - document priorities of "iommus" property vs. "dma-ranges" property > - drop #iommu-cells in favour of #address-cells and #size-cells > - remove multiple-master device example > > Documentation/devicetree/bindings/iommu/iommu.txt | 167 > ++ > 1 file changed, 167 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt > > diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt > b/Documentation/devicetree/bindings/iommu/iommu.txt > new file mode 100644 > index ..6ce759afcc94 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/iommu.txt > @@ -0,0 +1,167 @@ > +This document describes the generic device tree binding for IOMMUs and their > +master(s). > + > + > +IOMMU device node: > +== > + > +An IOMMU can provide the following services: > + > +* Remap address space to allow devices to access physical memory ranges that > + they otherwise wouldn't be capable of accessing. > + > + Example: 32-bit DMA to 64-bit physical addresses > + > +* Implement scatter-gather at page level granularity so that the device does > + not have to. > + > +* Provide system protection against "rogue" DMA by forcing all accesses to go > + through the IOMMU and faulting when encountering accesses to unmapped > + address regions. > + > +* Provide address space isolation between multiple contexts. > + > + Example: Virtualization > + > +Device nodes compatible with this binding represent hardware with some of the > +above capabilities. > + > +IOMMUs can be single-master or multiple-master. Single-master IOMMU devices > +typically have a fixed association to the master device, whereas multiple- > +master IOMMU devices can translate accesses from more than one master. > + > +The device tree node of the IOMMU device's parent bus must contain a valid > +"dma-ranges" property that describes how the physical address space of the > +IOMMU maps to memory. An empty "dma-ranges" property means that there is a > +1:1 mapping from IOMMU to memory. > + > +Required properties: > + > +- #address-cells: The number of cells in an IOMMU specifier needed to encode > + an address. > +- #size-cells: The number of cells in an IOMMU specifier needed to represent > + the length of an address range. > + > +Typical values for the above include: > +- #address-cells = <0>, size-cells = <0>: Single master IOMMU devices are not > + configurable and therefore no additional information needs to be encoded in > + the specifier. This may also apply to multiple master IOMMU devices that do > + not allow the association of masters to be configured. > +- #address-cells = <1>, size-cells = <0>: Multiple master IOMMU devices may > + need to be configured in order to enable translation for a given master. In > + such cases the single address cell corresponds to the master device's ID. > +- #address-cells = <2>, size-cells = <2>: Some IOMMU devices allow the DMA > + window for masters to be configured. The first cell of the address in this > + may contain the master device's ID for example, while the second cell could > + contain the start of the DMA window for the given device. The length of the > + DMA window is specified by two additional cells. > + > + > +IOMMU master node: > +== > + > +Devices that access memory through an IOMMU are called masters. A device can > +have multiple master interfaces (to one or more IOMMU devices). > + > +Required properties: > + > +- iommus: A list of phandle and IOMMU specifier pairs that describe the IOMMU > + master interfaces of the device. One entry in the list describes one master > + interface of the device. > + > +When an "iommus" property is specified in a device tree node, the IOMMU will > +be used for address translation. If a "dma-ranges" property exists in the > +device's parent node it will be ignored. An exception to this rule is if the > +referenced IOMMU is disabled, in which case the "dma-ranges" property of the > +parent shall take effect. Just thinking out loud, could you have dma-ranges in the iommu node for the case when the iommu is enabled rather than putting the DMA window information into the iommus property? This would probably mean that you need both #iommu-cells and #address-cells. > + > +Optional properties: > + > +- iommu-names: A list of names identifying each entry in the "iommus" > + property. Do we really need a name here? I woul
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Fri, May 30, 2014 at 08:30:08AM +0100, Thierry Reding wrote: > On Thu, May 29, 2014 at 09:52:22AM -0600, Stephen Warren wrote: > > On 05/23/2014 02:36 PM, Thierry Reding wrote: > > > From: Thierry Reding > > > > > > This commit introduces a generic device tree binding for IOMMU devices. > > > Only a very minimal subset is described here, but it is enough to cover > > > the requirements of both the Exynos System MMU and Tegra SMMU as > > > discussed here: > > > > > > https://lkml.org/lkml/2014/4/27/346 > > > > > > Signed-off-by: Thierry Reding > > > --- > > > Apologies for the noise, but apparently I mistyped one of the email > > > addresses, should be fixed now. > > > > > > Changes in v2: > > > - add notes about "dma-ranges" property (drop note from commit message) > > > - document priorities of "iommus" property vs. "dma-ranges" property > > > - drop #iommu-cells in favour of #address-cells and #size-cells > > > > I think this is a mistake. address-cells/size-cells are for transactions > > flowing down the bus (from the CPU to date). Describing a connection > > from a device to an IOMMU is something completely different, and should > > therefore simply use an iommu-cells property to describe any necessary > > information. If we start re-using properties for different things in > > different contexts, how is anyone going to know what they mean, and how > > will conflicts be resolved. For example, what if there's a single HW > > module that both acts as a regular register bus with children (where > > address-cells/size-cells defines how transactions reach the children > > from the parent), and is also an IOMMU (where according to this binding > > proposal, address-cells/size-cells represent some aspect of the IOMMU > > feature). Using different properties for different things is the only > > sane way to keep different concepts separate. Another alternative would > > be to represent the single HW module as separate nodes in DT, but I > > think that will only make our lives harder, and where I've done that in > > the past, I've regretted it. > > There was some back-and-forth on this topic and the latest concensus > when I wrote the second version was that #address-cells and #size-cells > were to be used. > > But there was some bore back-and-forth after that, and it seems like > Arnd no longer thinks that using #address-cells and #size-cells is a > good idea either[0]. Mistake or not, ePAPR already (ab)uses the address concept for PCI. Unless ePAPR is wrong, I don't think it makes sense to argue that the address concept cannot be repurposed. The reason why this is abused for PCI is the same as our reason here: different masters really are treated as distinct even when accessing the same destination address, and DT has no general native way to describe that. One clear advantage of using #address-cells etc. is that ePAPR already has very clear and well-defined ways of how to specify range mappings. This gives us a ready-made way to describe windowed IOMMUs and 1:1 remappings of whole blocks of master IDs, which are the most obvious cases other than having a small-integer set of explicit IDs. That said, the PCI pseudo-address thing is not something we _necessarily_ want to repeat. > Arnd, can you take another look at this binding and see if there's > anything else missing? If not I'll go through the document again and > update all #address-cells/#size-cells references with #iommu-cells as > appropriate and submit v3. How do you envisage propagation of the master ID bits downstream of the IOMMU would be described? We will definitely need a way to describe this for GICv3. How those values are propagated is likely to vary between related SoCs and doesn't feel like it should be baked into a driver, especially for the ARM SMMU which may get reused in radically different SoC families from different vendors. The most likely types of remapping are the adding of a base offset or some extra bits to the ID -- because not all MSIs to the GIC will necessarily pass through the IOMMU. It's also possible that we might see ID squashing or folding in some systems. For types of remapping which mix the ID and address together, I now do tend to agree that any flexibility arising from describing that in a general way that is unlikely to repay the cost of trying to interpret and analyse the DT. Defining a few sterotypical kinds of mapping explicitly, as needed, looks more sensible for now. The windowed-IOMMU case is one example. Cheers ---Dave ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Fri, May 23, 2014 at 10:36:35PM +0200, Thierry Reding wrote: > From: Thierry Reding > > This commit introduces a generic device tree binding for IOMMU devices. > Only a very minimal subset is described here, but it is enough to cover > the requirements of both the Exynos System MMU and Tegra SMMU as > discussed here: > > https://lkml.org/lkml/2014/4/27/346 > > Signed-off-by: Thierry Reding > --- > Apologies for the noise, but apparently I mistyped one of the email > addresses, should be fixed now. > > Changes in v2: > - add notes about "dma-ranges" property (drop note from commit message) > - document priorities of "iommus" property vs. "dma-ranges" property > - drop #iommu-cells in favour of #address-cells and #size-cells > - remove multiple-master device example > > Documentation/devicetree/bindings/iommu/iommu.txt | 167 > ++ > 1 file changed, 167 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt > > diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt > b/Documentation/devicetree/bindings/iommu/iommu.txt > new file mode 100644 > index ..6ce759afcc94 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/iommu.txt > @@ -0,0 +1,167 @@ > +This document describes the generic device tree binding for IOMMUs and their > +master(s). > + > + > +IOMMU device node: > +== > + > +An IOMMU can provide the following services: > + > +* Remap address space to allow devices to access physical memory ranges that > + they otherwise wouldn't be capable of accessing. > + > + Example: 32-bit DMA to 64-bit physical addresses > + > +* Implement scatter-gather at page level granularity so that the device does > + not have to. > + > +* Provide system protection against "rogue" DMA by forcing all accesses to go > + through the IOMMU and faulting when encountering accesses to unmapped > + address regions. > + > +* Provide address space isolation between multiple contexts. > + > + Example: Virtualization > + > +Device nodes compatible with this binding represent hardware with some of the > +above capabilities. > + > +IOMMUs can be single-master or multiple-master. Single-master IOMMU devices > +typically have a fixed association to the master device, whereas multiple- > +master IOMMU devices can translate accesses from more than one master. > + > +The device tree node of the IOMMU device's parent bus must contain a valid > +"dma-ranges" property that describes how the physical address space of the > +IOMMU maps to memory. An empty "dma-ranges" property means that there is a > +1:1 mapping from IOMMU to memory. > + > +Required properties: > + > +- #address-cells: The number of cells in an IOMMU specifier needed to encode > + an address. > +- #size-cells: The number of cells in an IOMMU specifier needed to represent > + the length of an address range. > + > +Typical values for the above include: > +- #address-cells = <0>, size-cells = <0>: Single master IOMMU devices are not > + configurable and therefore no additional information needs to be encoded in > + the specifier. This may also apply to multiple master IOMMU devices that do > + not allow the association of masters to be configured. > +- #address-cells = <1>, size-cells = <0>: Multiple master IOMMU devices may > + need to be configured in order to enable translation for a given master. In > + such cases the single address cell corresponds to the master device's ID. > +- #address-cells = <2>, size-cells = <2>: Some IOMMU devices allow the DMA > + window for masters to be configured. The first cell of the address in this > + may contain the master device's ID for example, while the second cell could > + contain the start of the DMA window for the given device. The length of the > + DMA window is specified by two additional cells. > + > + > +IOMMU master node: > +== > + > +Devices that access memory through an IOMMU are called masters. A device can > +have multiple master interfaces (to one or more IOMMU devices). > + > +Required properties: > + > +- iommus: A list of phandle and IOMMU specifier pairs that describe the IOMMU > + master interfaces of the device. One entry in the list describes one master > + interface of the device. > + > +When an "iommus" property is specified in a device tree node, the IOMMU will > +be used for address translation. If a "dma-ranges" property exists in the > +device's parent node it will be ignored. An exception to this rule is if the > +referenced IOMMU is disabled, in which case the "dma-ranges" property of the > +parent shall take effect. > + > +Optional properties: > + > +- iommu-names: A list of names identifying each entry in the "iommus" > + property. > + > + > +Notes: > +== > + > +One possible extension to the above is to use an "iommus" property along with > +a "dma-ranges" property in a bus device node (such as PCI host
RE: [PATCH v4 13/16] iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
> -Original Message-
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Friday, May 23, 2014 4:39 AM
> To: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org
> Cc: linux-ker...@vger.kernel.org; aco...@gmail.com; li...@horizon.com;
> bhelg...@google.com; Sethi Varun-B16395; eddy0...@gmail.com
> Subject: [PATCH v4 13/16] iommu/fsl: Use iommu_group_get_for_dev() for
> IOMMU groups
>
> Drop custom code and use IOMMU provided grouping support for PCI.
>
> Signed-off-by: Alex Williamson
> Cc: Varun Sethi
> ---
> drivers/iommu/fsl_pamu_domain.c | 66 +
> --
> 1 file changed, 1 insertion(+), 65 deletions(-)
>
> diff --git a/drivers/iommu/fsl_pamu_domain.c
> b/drivers/iommu/fsl_pamu_domain.c index 93072ba..d02d668 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -38,7 +38,6 @@
> #include
>
> #include "fsl_pamu_domain.h"
> -#include "pci.h"
>
> /*
> * Global spinlock that needs to be held while @@ -892,8 +891,6 @@
> static int fsl_pamu_get_domain_attr(struct iommu_domain *domain,
> return ret;
> }
>
> -#define REQ_ACS_FLAGS(PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR |
> PCI_ACS_UF)
> -
> static struct iommu_group *get_device_iommu_group(struct device *dev) {
> struct iommu_group *group;
> @@ -950,74 +947,13 @@ static struct iommu_group
> *get_pci_device_group(struct pci_dev *pdev)
> struct pci_controller *pci_ctl;
> bool pci_endpt_partioning;
> struct iommu_group *group = NULL;
> - struct pci_dev *bridge, *dma_pdev = NULL;
>
> pci_ctl = pci_bus_to_host(pdev->bus);
> pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl);
> /* We can partition PCIe devices so assign device group to the
> device */
> if (pci_endpt_partioning) {
> - bridge = pci_find_upstream_pcie_bridge(pdev);
> - if (bridge) {
> - if (pci_is_pcie(bridge))
> - dma_pdev = pci_get_domain_bus_and_slot(
> - pci_domain_nr(pdev->bus),
> - bridge->subordinate->number, 0);
> - if (!dma_pdev)
> - dma_pdev = pci_dev_get(bridge);
> - } else
> - dma_pdev = pci_dev_get(pdev);
> -
> - /* Account for quirked devices */
> - swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
> -
> - /*
> - * If it's a multifunction device that does not support our
> - * required ACS flags, add to the same group as lowest
> numbered
> - * function that also does not suport the required ACS flags.
> - */
> - if (dma_pdev->multifunction &&
> - !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) {
> - u8 i, slot = PCI_SLOT(dma_pdev->devfn);
> -
> - for (i = 0; i < 8; i++) {
> - struct pci_dev *tmp;
> -
> - tmp = pci_get_slot(dma_pdev->bus,
> PCI_DEVFN(slot,
> i));
> - if (!tmp)
> - continue;
> -
> - if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) {
> - swap_pci_ref(&dma_pdev, tmp);
> - break;
> - }
> - pci_dev_put(tmp);
> - }
> - }
> -
> - /*
> - * Devices on the root bus go through the iommu. If that's
> not us,
> - * find the next upstream device and test ACS up to the root
> bus.
> - * Finding the next device may require skipping virtual
> buses.
> - */
> - while (!pci_is_root_bus(dma_pdev->bus)) {
> - struct pci_bus *bus = dma_pdev->bus;
> -
> - while (!bus->self) {
> - if (!pci_is_root_bus(bus))
> - bus = bus->parent;
> - else
> - goto root_bus;
> - }
> -
> - if (pci_acs_path_enabled(bus->self, NULL,
> REQ_ACS_FLAGS))
> - break;
> -
> - swap_pci_ref(&dma_pdev, pci_dev_get(bus->self));
> - }
> + group = iommu_group_get_for_dev(&pdev->dev);
>
> -root_bus:
> - group = get_device_iommu_group(&dma_pdev->dev);
> - pci_dev_put(dma_pdev);
> /*
>* PCIe controller is not a paritionable entity
>* free the controller device iommu_group.
Acknowledged-by: Varun Sethi
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mai
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Thu, May 29, 2014 at 09:52:22AM -0600, Stephen Warren wrote: > On 05/23/2014 02:36 PM, Thierry Reding wrote: > > From: Thierry Reding > > > > This commit introduces a generic device tree binding for IOMMU devices. > > Only a very minimal subset is described here, but it is enough to cover > > the requirements of both the Exynos System MMU and Tegra SMMU as > > discussed here: > > > > https://lkml.org/lkml/2014/4/27/346 > > > > Signed-off-by: Thierry Reding > > --- > > Apologies for the noise, but apparently I mistyped one of the email > > addresses, should be fixed now. > > > > Changes in v2: > > - add notes about "dma-ranges" property (drop note from commit message) > > - document priorities of "iommus" property vs. "dma-ranges" property > > - drop #iommu-cells in favour of #address-cells and #size-cells > > I think this is a mistake. address-cells/size-cells are for transactions > flowing down the bus (from the CPU to date). Describing a connection > from a device to an IOMMU is something completely different, and should > therefore simply use an iommu-cells property to describe any necessary > information. If we start re-using properties for different things in > different contexts, how is anyone going to know what they mean, and how > will conflicts be resolved. For example, what if there's a single HW > module that both acts as a regular register bus with children (where > address-cells/size-cells defines how transactions reach the children > from the parent), and is also an IOMMU (where according to this binding > proposal, address-cells/size-cells represent some aspect of the IOMMU > feature). Using different properties for different things is the only > sane way to keep different concepts separate. Another alternative would > be to represent the single HW module as separate nodes in DT, but I > think that will only make our lives harder, and where I've done that in > the past, I've regretted it. There was some back-and-forth on this topic and the latest concensus when I wrote the second version was that #address-cells and #size-cells were to be used. But there was some bore back-and-forth after that, and it seems like Arnd no longer thinks that using #address-cells and #size-cells is a good idea either[0]. Arnd, can you take another look at this binding and see if there's anything else missing? If not I'll go through the document again and update all #address-cells/#size-cells references with #iommu-cells as appropriate and submit v3. Thanks, Thierry [0]: https://lkml.org/lkml/2014/5/20/609 pgpRXa58012Ya.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
