Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On Wed, Oct 09, 2013 at 06:09:17AM -0400, Will Deacon wrote:
> On Tue, Oct 08, 2013 at 07:43:50PM +0100, Rob Herring wrote:
> > On 10/08/2013 11:59 AM, Will Deacon wrote:
> > > On Tue, Oct 08, 2013 at 05:40:21PM +0100, Andreas Herrmann wrote:
> > >> On Tue, Oct 08, 2013 at 05:20:08PM +0200, Andreas Herrmann wrote:
> > >> To be more specific: For SATA I'd need to specify 10 StreamIds. This
> > >> would
> > >>
> > >> (1) exceed MAX_MASTER_STREAMIDS (currently it's 8)
> > >>
> > >>(Can easily be fixed by adapting a macro.)
> > >>
> > >> (2) exceed number of available SMR groups to map the IDs to a context.
> > >>
> > >>This can be solved by caclulating an appropriate mask for the
> > >>mapping (but with a non-power-of-two number of StreamIds that's
> > >>already non-trivial -- for the trivial case I have some code to do
> > >>this).
> > >>
> > >> Both problems are avoided by introducing this patch -- use
> > >> smr_mask_bits to map all StreamIDs to the same context and be done
> > >> with it. (for the "single-master-SMMU" case)
> > >
> > > The problem is, this information *really* doesn't belong in the device
> > > tree,
> > > but I think computing the general case dynamically is incredibly difficult
> > > too (and requires *complete* topological information in the device-tree,
> > > so
> > > you don't accidentally pull in other devices).
> >
> > Couldn't this information be implied from the DT when you have no
> > streamID and only a single mmu-master?
>
> It's still fairly fragile though, since you (a) don't know that the stream
> IDs of the master are matchable by the SMRs (if we had the stream IDs, we
> can check this) and (b) you can still end up pulling devices into your
> address space that would otherwise happily operate using passthrough.
FYI, for SATA I'd have to specify below IDs (which requires some other
minor adaptions). I really would like to avoid specification of all
these StreamIDs and rather use stream matching with an appropriate
mask.
I can't end up pulling other devices into that mapping. It's just this
single master device at this SMMU.
And I also think that this "mask-all-stream-ids" option belongs to the
device tree as an hint to the driver that it is sane to use
smr_mask_bits as a mask for stream matching for this particular SMMU.
Andreas
---
ecx-2000, iommu/arm-smmu, of: Add all 10 StreamIDs for sata-smmu
Unfortunately this requires adaptions of MAX_MASTER_STREAMIDS and
MAX_PHANDLE_ARGS.
It still won't work with the current arm-smmu driver as it strictly
uses one SMR group for each StreamID. (But the corresponding SMMU has
just 4 SMR groups).
Signed-off-by: Andreas Herrmann
---
arch/arm/boot/dts/ecx-2000.dts|2 +-
arch/arm/boot/dts/ecx-common.dtsi |2 +-
drivers/iommu/arm-smmu.c |2 +-
include/linux/of.h|2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/ecx-2000.dts b/arch/arm/boot/dts/ecx-2000.dts
index 270f052..3904c2b 100644
--- a/arch/arm/boot/dts/ecx-2000.dts
+++ b/arch/arm/boot/dts/ecx-2000.dts
@@ -135,7 +135,7 @@
smmu_sata: smmu@92018 {
compatible = "arm,mmu-400";
reg = <0x0009 0x2018 0x1>;
- mmu-masters = <&sata>;
+ mmu-masters = <&sata 0 1 2 3 4 5 6 7 8 9>;
#global-interrupts = <1>;
interrupts = <0 114 4 0 114 4>;
arm,smmu-secure-config-access;
diff --git a/arch/arm/boot/dts/ecx-common.dtsi
b/arch/arm/boot/dts/ecx-common.dtsi
index 50e401e..961dc5b 100644
--- a/arch/arm/boot/dts/ecx-common.dtsi
+++ b/arch/arm/boot/dts/ecx-common.dtsi
@@ -35,7 +35,7 @@
&combophy0 3>;
calxeda,sgpio-gpio =<&gpioh 5 1 &gpioh 6 1 &gpioh 7 1>;
calxeda,led-order = <4 0 1 2 3>;
- #stream-id-cells = <0>;
+ #stream-id-cells = <10>;
};
sdhci@ffe0e000 {
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4e8ceab..cd370f7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -55,7 +55,7 @@
#define ARM_SMMU_OPT_SECURE_CONFIG_ACCESS (1 << 2)
/* Maximum number of stream IDs assigned to a single device */
-#define MAX_MASTER_STREAMIDS 8
+#define MAX_MASTER_STREAMIDS 10
/* Maximum number of context banks per SMMU */
#define ARM_SMMU_MAX_CBS 128
diff --git a/include/linux/of.h b/include/linux/of.h
index f95aee3..47f4857 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -67,7 +67,7 @@ struct device_node {
#endif
};
-#define MAX_PHANDLE_ARGS 8
+#define MAX_PHANDLE_ARGS 10
struct of_phandle_args {
struct device_node *np;
int args_count;
--
1.7.9.5
___
iommu mailing list
iommu@
Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On Tue, Oct 08, 2013 at 12:59:20PM -0400, Will Deacon wrote: > On Tue, Oct 08, 2013 at 05:40:21PM +0100, Andreas Herrmann wrote: > > On Tue, Oct 08, 2013 at 05:20:08PM +0200, Andreas Herrmann wrote: > > To be more specific: For SATA I'd need to specify 10 StreamIds. This > > would > > > > (1) exceed MAX_MASTER_STREAMIDS (currently it's 8) > > > >(Can easily be fixed by adapting a macro.) > > > > (2) exceed number of available SMR groups to map the IDs to a context. > > > >This can be solved by caclulating an appropriate mask for the > >mapping (but with a non-power-of-two number of StreamIds that's > >already non-trivial -- for the trivial case I have some code to do > >this). > > > > Both problems are avoided by introducing this patch -- use > > smr_mask_bits to map all StreamIDs to the same context and be done > > with it. (for the "single-master-SMMU" case) > > The problem is, this information *really* doesn't belong in the device tree, > but I think computing the general case dynamically is incredibly difficult > too (and requires *complete* topological information in the device-tree, so > you don't accidentally pull in other devices). So, if this kind of driver option does not belong to the device tree, what's the preferred solution to tell the driver to use smr_mask_bits as a mask for stream matching for a certain SMMU (that has only one master)? > > PS: I think (2) needs to be addressed sooner or later. We should use > > only as many SMR groups as really required -- ie. use masking of > > StreamIds if possible. If more than one StreamID is given for a > > master it might be possible to calculate a mask for a > > (power-of-two) number of adjacent StreamIds and then use only one > > SMR group to map these IDs to a context. (But I think that should > > only be done if multiple masters are attached to an SMMU.) > > I spent a few weeks looking at doing minimal SMR grouping whilst writing the > driver and ended up convincing myself that it's an NP-complete problem (I > tried a reduction involving Hamiltonian Cycles). Of course, if you have some > ideas here, we can try to implement something for a constrained instance of > the problem. > > For example, a simple solution is to xor all the IDs together and check no > other IDs fall under the resulting mask. However, this again relies on the > DT telling us complete topological information as well as the IDs being > organised in a particular way. Yes, it is an ugly problem. But assuming that the StreamIDs in many cases have a particular organsiation it should be possible to calculate a mask with reasonable effort. (I've attached a patch to do this for certain cases). I think it could be extended, such that in a case where StreamIDs 0, ... 9 are specified for a master the driver could set up SMR0 with mask 0x7, id 0 (mapping StreamIDs 0, ..., 7) SMR1 for StreamID 8 SMR2 for StreamID 9 Thus 3 SMR groups would be used instead of 10 (as it is done with the current implementation). Andreas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On Tue, Oct 08, 2013 at 07:43:50PM +0100, Rob Herring wrote: > On 10/08/2013 11:59 AM, Will Deacon wrote: > > On Tue, Oct 08, 2013 at 05:40:21PM +0100, Andreas Herrmann wrote: > >> On Tue, Oct 08, 2013 at 05:20:08PM +0200, Andreas Herrmann wrote: > >> To be more specific: For SATA I'd need to specify 10 StreamIds. This > >> would > >> > >> (1) exceed MAX_MASTER_STREAMIDS (currently it's 8) > >> > >>(Can easily be fixed by adapting a macro.) > >> > >> (2) exceed number of available SMR groups to map the IDs to a context. > >> > >>This can be solved by caclulating an appropriate mask for the > >>mapping (but with a non-power-of-two number of StreamIds that's > >>already non-trivial -- for the trivial case I have some code to do > >>this). > >> > >> Both problems are avoided by introducing this patch -- use > >> smr_mask_bits to map all StreamIDs to the same context and be done > >> with it. (for the "single-master-SMMU" case) > > > > The problem is, this information *really* doesn't belong in the device tree, > > but I think computing the general case dynamically is incredibly difficult > > too (and requires *complete* topological information in the device-tree, so > > you don't accidentally pull in other devices). > > Couldn't this information be implied from the DT when you have no > streamID and only a single mmu-master? It's still fairly fragile though, since you (a) don't know that the stream IDs of the master are matchable by the SMRs (if we had the stream IDs, we can check this) and (b) you can still end up pulling devices into your address space that would otherwise happily operate using passthrough. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On 10/08/2013 11:59 AM, Will Deacon wrote: > On Tue, Oct 08, 2013 at 05:40:21PM +0100, Andreas Herrmann wrote: >> On Tue, Oct 08, 2013 at 05:20:08PM +0200, Andreas Herrmann wrote: >> To be more specific: For SATA I'd need to specify 10 StreamIds. This >> would >> >> (1) exceed MAX_MASTER_STREAMIDS (currently it's 8) >> >>(Can easily be fixed by adapting a macro.) >> >> (2) exceed number of available SMR groups to map the IDs to a context. >> >>This can be solved by caclulating an appropriate mask for the >>mapping (but with a non-power-of-two number of StreamIds that's >>already non-trivial -- for the trivial case I have some code to do >>this). >> >> Both problems are avoided by introducing this patch -- use >> smr_mask_bits to map all StreamIDs to the same context and be done >> with it. (for the "single-master-SMMU" case) > > The problem is, this information *really* doesn't belong in the device tree, > but I think computing the general case dynamically is incredibly difficult > too (and requires *complete* topological information in the device-tree, so > you don't accidentally pull in other devices). Couldn't this information be implied from the DT when you have no streamID and only a single mmu-master? Rob >> PS: I think (2) needs to be addressed sooner or later. We should use >> only as many SMR groups as really required -- ie. use masking of >> StreamIds if possible. If more than one StreamID is given for a >> master it might be possible to calculate a mask for a >> (power-of-two) number of adjacent StreamIds and then use only one >> SMR group to map these IDs to a context. (But I think that should >> only be done if multiple masters are attached to an SMMU.) > > I spent a few weeks looking at doing minimal SMR grouping whilst writing the > driver and ended up convincing myself that it's an NP-complete problem (I > tried a reduction involving Hamiltonian Cycles). Of course, if you have some > ideas here, we can try to implement something for a constrained instance of > the problem. > > For example, a simple solution is to xor all the IDs together and check no > other IDs fall under the resulting mask. However, this again relies on the > DT telling us complete topological information as well as the IDs being > organised in a particular way. > > Will > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
> -Original Message- > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > boun...@lists.linux-foundation.org] On Behalf Of Will Deacon > Sent: Tuesday, October 08, 2013 10:29 PM > To: Andreas Herrmann > Cc: iommu@lists.linux-foundation.org; Rob Herring; linux-arm- > ker...@lists.infradead.org > Subject: Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking > > On Tue, Oct 08, 2013 at 05:40:21PM +0100, Andreas Herrmann wrote: > > On Tue, Oct 08, 2013 at 05:20:08PM +0200, Andreas Herrmann wrote: > > To be more specific: For SATA I'd need to specify 10 StreamIds. This > > would > > > > (1) exceed MAX_MASTER_STREAMIDS (currently it's 8) > > > >(Can easily be fixed by adapting a macro.) > > > > (2) exceed number of available SMR groups to map the IDs to a context. > > > >This can be solved by caclulating an appropriate mask for the > >mapping (but with a non-power-of-two number of StreamIds that's > >already non-trivial -- for the trivial case I have some code to do > >this). > > > > Both problems are avoided by introducing this patch -- use > > smr_mask_bits to map all StreamIDs to the same context and be done > > with it. (for the "single-master-SMMU" case) > > The problem is, this information *really* doesn't belong in the device > tree, but I think computing the general case dynamically is incredibly > difficult too (and requires *complete* topological information in the > device-tree, so you don't accidentally pull in other devices). > [Sethi Varun-B16395] I don't quiet follow the point about topological information. How do you determine from the topology if the stream ids should share the iommu domain? Another question, how would iommu group creation work for dma masters with multiple stream ids? -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On Tue, Oct 08, 2013 at 05:40:21PM +0100, Andreas Herrmann wrote: > On Tue, Oct 08, 2013 at 05:20:08PM +0200, Andreas Herrmann wrote: > To be more specific: For SATA I'd need to specify 10 StreamIds. This > would > > (1) exceed MAX_MASTER_STREAMIDS (currently it's 8) > >(Can easily be fixed by adapting a macro.) > > (2) exceed number of available SMR groups to map the IDs to a context. > >This can be solved by caclulating an appropriate mask for the >mapping (but with a non-power-of-two number of StreamIds that's >already non-trivial -- for the trivial case I have some code to do >this). > > Both problems are avoided by introducing this patch -- use > smr_mask_bits to map all StreamIDs to the same context and be done > with it. (for the "single-master-SMMU" case) The problem is, this information *really* doesn't belong in the device tree, but I think computing the general case dynamically is incredibly difficult too (and requires *complete* topological information in the device-tree, so you don't accidentally pull in other devices). > PS: I think (2) needs to be addressed sooner or later. We should use > only as many SMR groups as really required -- ie. use masking of > StreamIds if possible. If more than one StreamID is given for a > master it might be possible to calculate a mask for a > (power-of-two) number of adjacent StreamIds and then use only one > SMR group to map these IDs to a context. (But I think that should > only be done if multiple masters are attached to an SMMU.) I spent a few weeks looking at doing minimal SMR grouping whilst writing the driver and ended up convincing myself that it's an NP-complete problem (I tried a reduction involving Hamiltonian Cycles). Of course, if you have some ideas here, we can try to implement something for a constrained instance of the problem. For example, a simple solution is to xor all the IDs together and check no other IDs fall under the resulting mask. However, this again relies on the DT telling us complete topological information as well as the IDs being organised in a particular way. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On Tue, Oct 08, 2013 at 05:20:08PM +0200, Andreas Herrmann wrote: > On Tue, Oct 08, 2013 at 11:10:07AM -0400, Will Deacon wrote: > > On Tue, Oct 08, 2013 at 10:27:22AM +0100, Andreas Herrmann wrote: > > > Ie. use a mask based on smr_mask_bits to map all stream IDs of an SMMU > > > to one context. > > > > > > This behaviour is controlled per SMMU node with DT property > > > "arm,smmu-mask-stream-ids" and is only allowed if just a single master > > > is attached to an SMMU. If the option is specified, all stream-ids > > > that are provided in DT for the single master have no relevance. > > > > > > This is useful/convenient if a master has more than 8 stream-ids or if > > > not all stream-ids are known for a master device. > > > > Hmmm, this really scares me. What's the use-case? I worry about people > > inadvertently putting devices into an iommu domain because they happen to be > > included in a mask. Your solution seems to be targetting a single master > > with lots of IDs -- is this a PCI RC? > > No, it's the sata device. It has 0xf as smr_mask_bits. > > But it's just one device and it (usually) should be mapped to one > context -- no matter how many StreamIDs are really used. To be more specific: For SATA I'd need to specify 10 StreamIds. This would (1) exceed MAX_MASTER_STREAMIDS (currently it's 8) (Can easily be fixed by adapting a macro.) (2) exceed number of available SMR groups to map the IDs to a context. This can be solved by caclulating an appropriate mask for the mapping (but with a non-power-of-two number of StreamIds that's already non-trivial -- for the trivial case I have some code to do this). Both problems are avoided by introducing this patch -- use smr_mask_bits to map all StreamIDs to the same context and be done with it. (for the "single-master-SMMU" case) Andreas PS: I think (2) needs to be addressed sooner or later. We should use only as many SMR groups as really required -- ie. use masking of StreamIds if possible. If more than one StreamID is given for a master it might be possible to calculate a mask for a (power-of-two) number of adjacent StreamIds and then use only one SMR group to map these IDs to a context. (But I think that should only be done if multiple masters are attached to an SMMU.) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On Tue, Oct 08, 2013 at 11:10:07AM -0400, Will Deacon wrote: > On Tue, Oct 08, 2013 at 10:27:22AM +0100, Andreas Herrmann wrote: > > Ie. use a mask based on smr_mask_bits to map all stream IDs of an SMMU > > to one context. > > > > This behaviour is controlled per SMMU node with DT property > > "arm,smmu-mask-stream-ids" and is only allowed if just a single master > > is attached to an SMMU. If the option is specified, all stream-ids > > that are provided in DT for the single master have no relevance. > > > > This is useful/convenient if a master has more than 8 stream-ids or if > > not all stream-ids are known for a master device. > > Hmmm, this really scares me. What's the use-case? I worry about people > inadvertently putting devices into an iommu domain because they happen to be > included in a mask. Your solution seems to be targetting a single master > with lots of IDs -- is this a PCI RC? No, it's the sata device. It has 0xf as smr_mask_bits. But it's just one device and it (usually) should be mapped to one context -- no matter how many StreamIDs are really used. (PCI RC has also 0xf but there it's clear there can be multiple devices and the masking should not be done.) Andreas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
On Tue, Oct 08, 2013 at 10:27:22AM +0100, Andreas Herrmann wrote: > Ie. use a mask based on smr_mask_bits to map all stream IDs of an SMMU > to one context. > > This behaviour is controlled per SMMU node with DT property > "arm,smmu-mask-stream-ids" and is only allowed if just a single master > is attached to an SMMU. If the option is specified, all stream-ids > that are provided in DT for the single master have no relevance. > > This is useful/convenient if a master has more than 8 stream-ids or if > not all stream-ids are known for a master device. Hmmm, this really scares me. What's the use-case? I worry about people inadvertently putting devices into an iommu domain because they happen to be included in a mask. Your solution seems to be targetting a single master with lots of IDs -- is this a PCI RC? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
