RE: [PATCH 1/7] powerpc: Add interface to get msi region information
> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, October 09, 2013 4:27 AM
> To: Bhushan Bharat-R65777
> Cc: alex.william...@redhat.com; j...@8bytes.org; b...@kernel.crashing.org;
> ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org; linux-...@vger.kernel.org; ag...@suse.de;
> iommu@lists.linux-foundation.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH 1/7] powerpc: Add interface to get msi region information
>
> On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
> > @@ -376,6 +405,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
> > int len;
> > u32 offset;
> > static const u32 all_avail[] = { 0, NR_MSI_IRQS };
> > + static int bank_index;
> >
> > match = of_match_device(fsl_of_msi_ids, &dev->dev);
> > if (!match)
> > @@ -419,8 +449,8 @@ static int fsl_of_msi_probe(struct platform_device *dev)
> > dev->dev.of_node->full_name);
> > goto error_out;
> > }
> > - msi->msiir_offset =
> > - features->msiir_offset + (res.start & 0xf);
> > + msi->msiir = res.start + features->msiir_offset;
> > + printk("msi->msiir = %llx\n", msi->msiir);
>
> dev_dbg or remove
Oops, sorry it was leftover of debugging :(
>
> > }
> >
> > msi->feature = features->fsl_pic_ip; @@ -470,6 +500,7 @@ static int
> > fsl_of_msi_probe(struct platform_device *dev)
> > }
> > }
> >
> > + msi->bank_index = bank_index++;
>
> What if multiple MSIs are boing probed in parallel?
Ohh, I have not thought that it can be called in parallel
> bank_index is not atomic.
Will declare bank_intex as atomic_t and use atomic_inc_return(&bank_index)
>
> > diff --git a/arch/powerpc/sysdev/fsl_msi.h
> > b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..6bd5cfc 100644
> > --- a/arch/powerpc/sysdev/fsl_msi.h
> > +++ b/arch/powerpc/sysdev/fsl_msi.h
> > @@ -29,12 +29,19 @@ struct fsl_msi {
> > struct irq_domain *irqhost;
> >
> > unsigned long cascade_irq;
> > -
> > - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
> > + dma_addr_t msiir; /* MSIIR Address in CCSR */
>
> Are you sure dma_addr_t is right here, versus phys_addr_t? It implies that
> it's
> the output of the DMA API, but I don't think the DMA API is used in the MSI
> driver. Perhaps it should be, but we still want the raw physical address to
> pass on to VFIO.
Looking through the conversation I will make this phys_addr_t
>
> > void __iomem *msi_regs;
> > u32 feature;
> > int msi_virqs[NR_MSI_REG];
> >
> > + /*
> > +* During probe each bank is assigned a index number.
> > +* index number ranges from 0 to 2^32.
> > +* Example MSI bank 1 = 0
> > +* MSI bank 2 = 1, and so on.
> > +*/
> > + int bank_index;
>
> 2^32 doesn't fit in "int" (nor does 2^32 - 1).
Right :(
>
> Just say that indices start at 0.
Will correct this
Thanks
-Bharat
>
> -Scott
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/7] powerpc: Add interface to get msi region information
On Tue, 2013-10-08 at 17:25 -0600, Bjorn Helgaas wrote: > >> - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */ > >> + dma_addr_t msiir; /* MSIIR Address in CCSR */ > > > > Are you sure dma_addr_t is right here, versus phys_addr_t? It implies > > that it's the output of the DMA API, but I don't think the DMA API is > > used in the MSI driver. Perhaps it should be, but we still want the raw > > physical address to pass on to VFIO. > > I don't know what "msiir" is used for, but if it's an address you > program into a PCI device, then it's a dma_addr_t even if you didn't > get it from the DMA API. Maybe "bus_addr_t" would have been a more > suggestive name than "dma_addr_t". That said, I have no idea how this > relates to VFIO. It's a bit awkward because it gets used both as something to program into a PCI device (and it's probably a bug that the DMA API doesn't get used), and also (if I understand the current plans correctly) as a physical address to give to VFIO to be a destination address in an IOMMU mapping. So I think the value we keep here should be a phys_addr_t (it comes straight from the MMIO address in the device tree), which gets trivially turned into a dma_addr_t by the non-VFIO code path because there's currently no translation there. -Scott ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/7] powerpc: Add interface to get msi region information
>> - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */ >> + dma_addr_t msiir; /* MSIIR Address in CCSR */ > > Are you sure dma_addr_t is right here, versus phys_addr_t? It implies > that it's the output of the DMA API, but I don't think the DMA API is > used in the MSI driver. Perhaps it should be, but we still want the raw > physical address to pass on to VFIO. I don't know what "msiir" is used for, but if it's an address you program into a PCI device, then it's a dma_addr_t even if you didn't get it from the DMA API. Maybe "bus_addr_t" would have been a more suggestive name than "dma_addr_t". That said, I have no idea how this relates to VFIO. Bjorn ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/7] powerpc: Add interface to get msi region information
On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
> @@ -376,6 +405,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
> int len;
> u32 offset;
> static const u32 all_avail[] = { 0, NR_MSI_IRQS };
> + static int bank_index;
>
> match = of_match_device(fsl_of_msi_ids, &dev->dev);
> if (!match)
> @@ -419,8 +449,8 @@ static int fsl_of_msi_probe(struct platform_device *dev)
> dev->dev.of_node->full_name);
> goto error_out;
> }
> - msi->msiir_offset =
> - features->msiir_offset + (res.start & 0xf);
> + msi->msiir = res.start + features->msiir_offset;
> + printk("msi->msiir = %llx\n", msi->msiir);
dev_dbg or remove
> }
>
> msi->feature = features->fsl_pic_ip;
> @@ -470,6 +500,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
> }
> }
>
> + msi->bank_index = bank_index++;
What if multiple MSIs are boing probed in parallel? bank_index is not
atomic.
> diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
> index 8225f86..6bd5cfc 100644
> --- a/arch/powerpc/sysdev/fsl_msi.h
> +++ b/arch/powerpc/sysdev/fsl_msi.h
> @@ -29,12 +29,19 @@ struct fsl_msi {
> struct irq_domain *irqhost;
>
> unsigned long cascade_irq;
> -
> - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
> + dma_addr_t msiir; /* MSIIR Address in CCSR */
Are you sure dma_addr_t is right here, versus phys_addr_t? It implies
that it's the output of the DMA API, but I don't think the DMA API is
used in the MSI driver. Perhaps it should be, but we still want the raw
physical address to pass on to VFIO.
> void __iomem *msi_regs;
> u32 feature;
> int msi_virqs[NR_MSI_REG];
>
> + /*
> + * During probe each bank is assigned a index number.
> + * index number ranges from 0 to 2^32.
> + * Example MSI bank 1 = 0
> + * MSI bank 2 = 1, and so on.
> + */
> + int bank_index;
2^32 doesn't fit in "int" (nor does 2^32 - 1).
Just say that indices start at 0.
-Scott
___
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 1/7] powerpc: Add interface to get msi region information
> -Original Message- > From: j...@8bytes.org [mailto:j...@8bytes.org] > Sent: Tuesday, October 08, 2013 10:32 PM > To: Bjorn Helgaas > Cc: Bhushan Bharat-R65777; alex.william...@redhat.com; > b...@kernel.crashing.org; > ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org; linux-...@vger.kernel.org; ag...@suse.de; Wood Scott- > B07421; iommu@lists.linux-foundation.org > Subject: Re: [PATCH 1/7] powerpc: Add interface to get msi region information > > On Tue, Oct 08, 2013 at 10:47:49AM -0600, Bjorn Helgaas wrote: > > I still have no idea what an "aperture type IOMMU" is, other than that > > it is "different." > > An aperture based IOMMU is basically any GART-like IOMMU which can only remap > a > small window (the aperture) of the DMA address space. DMA outside of that > window > is either blocked completly or passed through untranslated. It is completely blocked for Freescale PAMU. So for this type of iommu what we have to do is to create a MSI mapping just after guest physical address, Example: guest have a 512M of memory then we create window of 1G (because of power of 2 requirement), then we have to FIT MSI just after 512M of guest. And for that we need 1) to know the physical address of MSI's in interrupt controller (for that this patch was all about of). 2) When guest enable MSI interrupt then we write MSI-address and MSI-DATA in device. The discussion with Alex Williamson is about that interface. Thanks -Bharat > > > Joerg > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/7] powerpc: Add interface to get msi region information
On Tue, 2013-10-08 at 10:47 -0600, Bjorn Helgaas wrote: > On Thu, Oct 3, 2013 at 11:19 PM, Bhushan Bharat-R65777 > wrote: > > >> I don't know enough about VFIO to understand why these new interfaces are > >> needed. Is this the first VFIO IOMMU driver? I see > >> vfio_iommu_spapr_tce.c and > >> vfio_iommu_type1.c but I don't know if they're comparable to the Freescale > >> PAMU. > >> Do other VFIO IOMMU implementations support MSI? If so, do they handle the > >> problem of mapping the MSI regions in a different way? > > > > PAMU is an aperture type of IOMMU while other are paging type, So they are > > completely different from what PAMU is and handle that differently. > > This is not an explanation or a justification for adding new > interfaces. I still have no idea what an "aperture type IOMMU" is, > other than that it is "different." But I see that Alex is working on > this issue with you in a different thread, so I'm sure you guys will > sort it out. PAMU is a very constrained IOMMU that cannot do arbitrary page mappings. Due to these constraints, we cannot map the MSI I/O page at its normal address while also mapping RAM at the address we want. The address we can map it at depends on the addresses of other mappings, so it can't be hidden in the IOMMU driver -- the user needs to be in control. Another difference is that (if I understand correctly) PCs handle MSIs specially, via interrupt remapping, rather than being translated as a normal memory access through the IOMMU. -Scott ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/7] powerpc: Add interface to get msi region information
On Tue, Oct 08, 2013 at 10:47:49AM -0600, Bjorn Helgaas wrote: > I still have no idea what an "aperture type IOMMU" is, > other than that it is "different." An aperture based IOMMU is basically any GART-like IOMMU which can only remap a small window (the aperture) of the DMA address space. DMA outside of that window is either blocked completly or passed through untranslated. Joerg ___ 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 1/7] powerpc: Add interface to get msi region information
On Thu, Oct 3, 2013 at 11:19 PM, Bhushan Bharat-R65777 wrote: >> I don't know enough about VFIO to understand why these new interfaces are >> needed. Is this the first VFIO IOMMU driver? I see vfio_iommu_spapr_tce.c >> and >> vfio_iommu_type1.c but I don't know if they're comparable to the Freescale >> PAMU. >> Do other VFIO IOMMU implementations support MSI? If so, do they handle the >> problem of mapping the MSI regions in a different way? > > PAMU is an aperture type of IOMMU while other are paging type, So they are > completely different from what PAMU is and handle that differently. This is not an explanation or a justification for adding new interfaces. I still have no idea what an "aperture type IOMMU" is, other than that it is "different." But I see that Alex is working on this issue with you in a different thread, so I'm sure you guys will sort it out. Bjorn ___ 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 1/5] iommu/arm-smmu: Introduce driver option handling
On Tue, Oct 08, 2013 at 11:06:13AM -0400, Will Deacon wrote:
> On Tue, Oct 08, 2013 at 10:27:20AM +0100, Andreas Herrmann wrote:
> > Introduce handling of driver options. Options are set based on DT
> > information when probing an SMMU device. The first option introduced
> > is "arm,smmu-isolate-devices". (It will be used in the bus notifier
> > block.)
> >
> > Signed-off-by: Andreas Herrmann
> > ---
> > drivers/iommu/arm-smmu.c | 49
> > ++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index b632bcd..b127f0f 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -348,6 +348,7 @@ struct arm_smmu_device {
> > #define ARM_SMMU_FEAT_TRANS_S2 (1 << 3)
> > #define ARM_SMMU_FEAT_TRANS_NESTED (1 << 4)
> > u32 features;
> > + u32 options;
>
> This should be enum arm_smmu_option. Also, we should probably be consistent
> between the options and features (i.e. either use an enum for each of them,
> or just stick to #defines for both).
Ok, so I'd prefer to use macros then.
> > int version;
> >
> > u32 num_context_banks;
> > @@ -398,6 +399,52 @@ struct arm_smmu_domain {
> > static DEFINE_SPINLOCK(arm_smmu_devices_lock);
> > static LIST_HEAD(arm_smmu_devices);
> >
> > +/* driver options */
> > +enum arm_smmu_option {
> > + ISOLATE_DEVICES = 0,
> > + OPTION_MAX,
> > +};
> > +
> > +struct arm_smmu_option_prop {
> > + enum arm_smmu_option opt;
> > + const char *prop;
> > +};
> > +
> > +static struct arm_smmu_option_prop arm_smmu_options [] = {
> > + { ISOLATE_DEVICES, "arm,smmu-isolate-devices" },
> > + { OPTION_MAX, NULL},
> > +};
> > +
> > +static inline int arm_smmu_has_option(struct arm_smmu_device *smmu,
> > + enum arm_smmu_option opt)
> > +{
> > + return (smmu->options & (1 << opt));
> > +}
> > +
> > +static inline void arm_smmu_set_option(struct arm_smmu_device *smmu,
> > + enum arm_smmu_option opt)
> > +{
> > + smmu->options |= (1 << opt);
> > +}
> > +
> > +static inline void arm_smmu_clear_option(struct arm_smmu_device *smmu,
> > + enum arm_smmu_option opt)
> > +{
> > + smmu->options &= ~(1 << opt);
> > +}
>
> These three functions are a bit over-engineered! We have things like
> __set_bit if you really want to use helpers.
Right, will adapt it.
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
Re: [PATCH 1/5] iommu/arm-smmu: Introduce driver option handling
On Tue, Oct 08, 2013 at 10:27:20AM +0100, Andreas Herrmann wrote:
> Introduce handling of driver options. Options are set based on DT
> information when probing an SMMU device. The first option introduced
> is "arm,smmu-isolate-devices". (It will be used in the bus notifier
> block.)
>
> Signed-off-by: Andreas Herrmann
> ---
> drivers/iommu/arm-smmu.c | 49
> ++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index b632bcd..b127f0f 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -348,6 +348,7 @@ struct arm_smmu_device {
> #define ARM_SMMU_FEAT_TRANS_S2 (1 << 3)
> #define ARM_SMMU_FEAT_TRANS_NESTED (1 << 4)
> u32 features;
> + u32 options;
This should be enum arm_smmu_option. Also, we should probably be consistent
between the options and features (i.e. either use an enum for each of them,
or just stick to #defines for both).
> int version;
>
> u32 num_context_banks;
> @@ -398,6 +399,52 @@ struct arm_smmu_domain {
> static DEFINE_SPINLOCK(arm_smmu_devices_lock);
> static LIST_HEAD(arm_smmu_devices);
>
> +/* driver options */
> +enum arm_smmu_option {
> + ISOLATE_DEVICES = 0,
> + OPTION_MAX,
> +};
> +
> +struct arm_smmu_option_prop {
> + enum arm_smmu_option opt;
> + const char *prop;
> +};
> +
> +static struct arm_smmu_option_prop arm_smmu_options [] = {
> + { ISOLATE_DEVICES, "arm,smmu-isolate-devices" },
> + { OPTION_MAX, NULL},
> +};
> +
> +static inline int arm_smmu_has_option(struct arm_smmu_device *smmu,
> + enum arm_smmu_option opt)
> +{
> + return (smmu->options & (1 << opt));
> +}
> +
> +static inline void arm_smmu_set_option(struct arm_smmu_device *smmu,
> + enum arm_smmu_option opt)
> +{
> + smmu->options |= (1 << opt);
> +}
> +
> +static inline void arm_smmu_clear_option(struct arm_smmu_device *smmu,
> + enum arm_smmu_option opt)
> +{
> + smmu->options &= ~(1 << opt);
> +}
These three functions are a bit over-engineered! We have things like
__set_bit if you really want to use helpers.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/tegra-smmu: Staticize tegra_smmu_pm_ops
On Tue, 8 Oct 2013 12:51:04 +0200 Sachin Kamat wrote: > 'tegra_smmu_pm_ops' is used only in this file. Make it static. > > Signed-off-by: Sachin Kamat Acked-by: Hiroshi Doyu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu/tegra-gart: Staticize tegra_gart_pm_ops
On Tue, 8 Oct 2013 12:51:03 +0200 Sachin Kamat wrote: > 'tegra_gart_pm_ops' is local to this file. Make it static. > > Signed-off-by: Sachin Kamat Acked-by: Hiroshi Doyu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] iommu/tegra-gart: Staticize tegra_gart_pm_ops
'tegra_gart_pm_ops' is local to this file. Make it static.
Signed-off-by: Sachin Kamat
---
drivers/iommu/tegra-gart.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index f75483a..dba1a9f 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -411,7 +411,7 @@ static int tegra_gart_remove(struct platform_device *pdev)
return 0;
}
-const struct dev_pm_ops tegra_gart_pm_ops = {
+static const struct dev_pm_ops tegra_gart_pm_ops = {
.suspend= tegra_gart_suspend,
.resume = tegra_gart_resume,
};
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] iommu/tegra-smmu: Staticize tegra_smmu_pm_ops
'tegra_smmu_pm_ops' is used only in this file. Make it static.
Signed-off-by: Sachin Kamat
---
drivers/iommu/tegra-smmu.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 34374b3..605b5b4 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -1254,7 +1254,7 @@ static int tegra_smmu_remove(struct platform_device *pdev)
return 0;
}
-const struct dev_pm_ops tegra_smmu_pm_ops = {
+static const struct dev_pm_ops tegra_smmu_pm_ops = {
.suspend= tegra_smmu_suspend,
.resume = tegra_smmu_resume,
};
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu: arm-smmu: Use devm_ioremap_resource
On Tue, Oct 08, 2013 at 11:42:35AM +0100, Sachin Kamat wrote: > devm_request_and_ioremap is deprecated. Use devm_ioremap_resource > instead. I already have an identical patch from Julia, queued for 3.13. Thanks, Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] iommu: arm-smmu: Use devm_ioremap_resource
devm_request_and_ioremap is deprecated. Use devm_ioremap_resource
instead.
Signed-off-by: Sachin Kamat
---
drivers/iommu/arm-smmu.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 181c9ba..b564930 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1787,9 +1787,9 @@ static int arm_smmu_device_dt_probe(struct
platform_device *pdev)
}
smmu->size = resource_size(res);
- smmu->base = devm_request_and_ioremap(dev, res);
- if (!smmu->base)
- return -EADDRNOTAVAIL;
+ smmu->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(smmu->base))
+ return PTR_ERR(smmu->base);
if (of_property_read_u32(dev->of_node, "#global-interrupts",
&smmu->num_global_irqs)) {
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 7/9] iommu/arm-smmu: Add function that conditionally isolates all masters of all SMMUs
On Mon, Oct 07, 2013 at 04:42:27PM +0100, Andreas Herrmann wrote:
> On Fri, Sep 27, 2013 at 09:00:01AM -0400, Will Deacon wrote:
> > On Thu, Sep 26, 2013 at 11:36:19PM +0100, Andreas Herrmann wrote:
> > > + list_for_each_entry(smmu, &arm_smmu_devices, list) {
> > > + if (arm_smmu_disable_isolation ||
> > > + (!(smmu->features & ARM_SMMU_FEAT_ISOLATE_DEVICES)
> > > + && !arm_smmu_force_isolation))
> > > + continue;
> > > + rbn = rb_first(&smmu->masters);
> > > + while (rbn) {
> > > + master = container_of(rbn, struct arm_smmu_master,
> > > node);
> > > + pdev = of_find_device_by_node(master->of_node);
> > > + if (!pdev)
> > > + break;
> > > + dev = &pdev->dev;
> > > +
> > > + size = (size_t) dev->coherent_dma_mask;
> > > + size = size ? : (unsigned long) dev->dma_mask;
> >
> > Hmm, this could be *huge* with 64-bit capable DMA controllers (think LPAE).
>
> Yes, agreed.
> (And even for 32-bit DMA this requires a large bitmap_size for the
> mapping.)
>
> > Russell also has some pending dma mask cleanup, which might break some
> > assumptions here:
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/199397.html
> >
> > (namely that we're offsetting everything from zero).
>
> What do you think is a reasonable general value for the size of a
> mapping? (Do we need a DT property to specify this?)
>
> What about a size of 128 MB -- if I'm not mistaken this requires a
> bitmap_size of 4K.
I don't think we can reasonably pick a sane number for this. We *could* try
doing it lazily (i.e. driving the mapping off a fault handler) but that's
pretty scary and probably doesn't interact well with endpoints incompatible
with the stall model (e.g. PCIe).
So the right solution is probably to get this information from the device
drivers, but I don't know what the interaction is with the dma_mask...
You'll need to do some digging.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] documentation/iommu: Update description of ARM System MMU binding
This patch adds descriptions fore new properties of device tree
binding for the ARM SMMU architecture. These properties control
arm-smmu driver options.
Cc: Rob Herring
Cc: Grant Likely
Cc: Will Deacon
Signed-off-by: Andreas Herrmann
---
.../devicetree/bindings/iommu/arm,smmu.txt | 23
1 file changed, 23 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index e34c6cd..6415a88 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -48,6 +48,28 @@ conditions.
from the mmu-masters towards memory) node for this
SMMU.
+- arm,smmu-isolate-devices : Enable device isolation for all masters
+ of this SMMU. Ie. each master will be
+ attached to its own iommu domain.
+
+- arm,smmu-mask-stream-ids : Enable mapping of all StreamIDs to one
+ context for this SMMU. It is only allowed
+ if there is one single master. It saves
+ the need to know and specify all
+ StreamIDs in cases where just one master
+ is attached to an SMMU. If StreamIDs are
+ specified in the mmu-masters property
+ those are ignored. (The referred device
+ node must still have a "#stream-id-cells"
+ property. But it can be set to 0.)
+
+- arm,smmu-secure-config-access : Enable proper handling of buggy
+ implementations that always use
+ secure access to SMMU configuration
+ registers. In this case non-secure
+ aliases of secure registers have to
+ be used during SMMU configuration.
+
Example:
smmu {
@@ -67,4 +89,5 @@ Example:
*/
mmu-masters = <&dma0 0xd01d 0xd01e>,
<&dma1 0xd11c>;
+arm,smmu-isolate-devices;
};
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/5] iommu/arm-smmu: Misc modifications to support SMMUs on Calxeda ECX-2000
Hi Will, Here is a reworked patch set - to introduce device isolation (using bus-notifier). - to work around secure-only config access to SMMU registers - (new) to support masking of all streamIDs for an SMMU - add DT information for SMMUs on Calxeda ECX-2000 I've also separated out the introduction of driver options handling. (Just in case it doesn't please you...) I still need to add the documentation for the introduced SMMU options to Documentation/devicetree/bindings/iommu/arm,smmu.txt. But first I'd like to know whether addition of the 3 options is acceptable. Comments welcome. Thanks, Andreas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/5] iommu/arm-smmu: Support buggy implementations where all config accesses are secure
In such a case we have to use secure aliases of some non-secure
registers.
This handling is switched on by DT property
"arm,smmu-secure-config-access" for an SMMU node.
Signed-off-by: Andreas Herrmann
---
drivers/iommu/arm-smmu.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index e2dbcbb..de74273 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -61,6 +61,15 @@
#define ARM_SMMU_GR0(smmu) ((smmu)->base)
#define ARM_SMMU_GR1(smmu) ((smmu)->base + (smmu)->pagesize)
+/*
+ * SMMU global address space with conditional offset to access secure aliases
of
+ * non-secure registers (e.g. nsCR0: 0x400, nsGFSR: 0x448, nsGFSYNR0: 0x450)
+ */
+#define ARM_SMMU_GR0_NS(smmu) \
+ ((smmu)->base + \
+ (arm_smmu_has_option(smmu, SECURE_CONFIG_ACCESS) \
+ ? 0x400 : 0))
+
/* Page table bits */
#define ARM_SMMU_PTE_PAGE (((pteval_t)3) << 0)
#define ARM_SMMU_PTE_CONT (((pteval_t)1) << 52)
@@ -406,6 +415,7 @@ static LIST_HEAD(arm_smmu_devices);
enum arm_smmu_option {
ISOLATE_DEVICES = 0,
MASK_STREAM_IDS,
+ SECURE_CONFIG_ACCESS,
OPTION_MAX,
};
@@ -417,6 +427,7 @@ struct arm_smmu_option_prop {
static struct arm_smmu_option_prop arm_smmu_options [] = {
{ ISOLATE_DEVICES, "arm,smmu-isolate-devices" },
{ MASK_STREAM_IDS, "arm,smmu-mask-stream-ids" },
+ { SECURE_CONFIG_ACCESS, "arm,smmu-secure-config-access" },
{ OPTION_MAX, NULL},
};
@@ -675,16 +686,16 @@ static irqreturn_t arm_smmu_global_fault(int irq, void
*dev)
{
u32 gfsr, gfsynr0, gfsynr1, gfsynr2;
struct arm_smmu_device *smmu = dev;
- void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
+ void __iomem *gr0_base = ARM_SMMU_GR0_NS(smmu);
gfsr = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSR);
- if (!gfsr)
- return IRQ_NONE;
-
gfsynr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR0);
gfsynr1 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR1);
gfsynr2 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR2);
+ if (!gfsr)
+ return IRQ_NONE;
+
dev_err_ratelimited(smmu->dev,
"Unexpected global fault, this could be serious\n");
dev_err_ratelimited(smmu->dev,
@@ -1634,8 +1645,8 @@ static void arm_smmu_device_reset(struct arm_smmu_device
*smmu)
u32 reg;
/* clear global FSR */
- reg = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSR);
- writel(reg, gr0_base + ARM_SMMU_GR0_sGFSR);
+ reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR);
+ writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR);
/* Mark all SMRn as invalid and all S2CRn as bypass */
for (i = 0; i < smmu->num_mapping_groups; ++i) {
@@ -1655,7 +1666,7 @@ static void arm_smmu_device_reset(struct arm_smmu_device
*smmu)
writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH);
writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH);
- reg = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);
+ reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
/* Enable fault reporting */
reg |= (sCR0_GFRE | sCR0_GFIE | sCR0_GCFGFRE | sCR0_GCFGFIE);
@@ -1674,7 +1685,7 @@ static void arm_smmu_device_reset(struct arm_smmu_device
*smmu)
/* Push the button */
arm_smmu_tlb_sync(smmu);
- writel(reg, gr0_base + ARM_SMMU_GR0_sCR0);
+ writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
}
static int arm_smmu_id_size_to_bits(int size)
@@ -2012,7 +2023,8 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
free_irq(smmu->irqs[i], smmu);
/* Turn the thing off */
- writel(sCR0_CLIENTPD, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_sCR0);
+ writel(sCR0_CLIENTPD,
+ ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
return 0;
}
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/5] iommu/arm-smmu: Introduce bus notifier block
At the moment just handle BUS_NOTIFY_BIND_DRIVER to conditionally
isolate all master devices for an SMMU.
Depending on DT information each device is put into its own protection
domain (if possible). For configuration with one or just a few
masters per SMMU that is easy to achieve.
In case of many devices per SMMU (e.g. MMU-500 with it's distributed
translation support) isolation of each device might not be possible --
depending on number of available SMR groups and/or context banks.
Default is that device isolation is contolled per SMMU with SMMU node
property "arm,smmu-isolate-devices" in a DT. If this property is set
for an SMMU node, device isolation is performed.
W/o device isolation the driver detects SMMUs but no translation is
configured (transactions just bypass translation process).
Note that for device isolation dma_base and size are fixed as 0 and
SZ_128M at the moment.
Signed-off-by: Andreas Herrmann
---
drivers/iommu/arm-smmu.c | 53 ++
1 file changed, 53 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index b127f0f..9a73618 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -46,6 +46,7 @@
#include
#include
+#include
/* Maximum number of stream IDs assigned to a single device */
#define MAX_MASTER_STREAMIDS 8
@@ -1996,6 +1997,56 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
return 0;
}
+static int arm_smmu_device_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device *dev = data;
+ struct dma_iommu_mapping *mapping;
+ struct arm_smmu_device *smmu;
+ void __iomem *gr0_base;
+ u32 cr0;
+ int ret;
+
+ switch (action) {
+ case BUS_NOTIFY_BIND_DRIVER:
+
+ arm_smmu_add_device(dev);
+ smmu = dev->archdata.iommu;
+ if (!smmu || !(arm_smmu_has_option(smmu, ISOLATE_DEVICES)))
+ break;
+
+ mapping = arm_iommu_create_mapping(&platform_bus_type,
+ 0, SZ_128M, 0);
+ if (IS_ERR(mapping)) {
+ ret = PTR_ERR(mapping);
+ dev_info(dev, "arm_iommu_create_mapping failed\n");
+ break;
+ }
+
+ ret = arm_iommu_attach_device(dev, mapping);
+ if (ret < 0) {
+ dev_info(dev, "arm_iommu_attach_device failed\n");
+ arm_iommu_release_mapping(mapping);
+ }
+
+ gr0_base = ARM_SMMU_GR0_NS(smmu);
+ cr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);
+ cr0 |= sCR0_USFCFG;
+ writel(cr0, gr0_base + ARM_SMMU_GR0_sCR0);
+
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static struct notifier_block device_nb = {
+ .notifier_call = arm_smmu_device_notifier,
+};
+
#ifdef CONFIG_OF
static struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", },
@@ -2032,6 +2083,8 @@ static int __init arm_smmu_init(void)
if (!iommu_present(&amba_bustype))
bus_set_iommu(&amba_bustype, &arm_smmu_ops);
+ bus_register_notifier(&platform_bus_type, &device_nb);
+
return 0;
}
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/5] iommu/arm-smmu: Introduce driver option handling
Introduce handling of driver options. Options are set based on DT
information when probing an SMMU device. The first option introduced
is "arm,smmu-isolate-devices". (It will be used in the bus notifier
block.)
Signed-off-by: Andreas Herrmann
---
drivers/iommu/arm-smmu.c | 49 ++
1 file changed, 49 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index b632bcd..b127f0f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -348,6 +348,7 @@ struct arm_smmu_device {
#define ARM_SMMU_FEAT_TRANS_S2 (1 << 3)
#define ARM_SMMU_FEAT_TRANS_NESTED (1 << 4)
u32 features;
+ u32 options;
int version;
u32 num_context_banks;
@@ -398,6 +399,52 @@ struct arm_smmu_domain {
static DEFINE_SPINLOCK(arm_smmu_devices_lock);
static LIST_HEAD(arm_smmu_devices);
+/* driver options */
+enum arm_smmu_option {
+ ISOLATE_DEVICES = 0,
+ OPTION_MAX,
+};
+
+struct arm_smmu_option_prop {
+ enum arm_smmu_option opt;
+ const char *prop;
+};
+
+static struct arm_smmu_option_prop arm_smmu_options [] = {
+ { ISOLATE_DEVICES, "arm,smmu-isolate-devices" },
+ { OPTION_MAX, NULL},
+};
+
+static inline int arm_smmu_has_option(struct arm_smmu_device *smmu,
+ enum arm_smmu_option opt)
+{
+ return (smmu->options & (1 << opt));
+}
+
+static inline void arm_smmu_set_option(struct arm_smmu_device *smmu,
+ enum arm_smmu_option opt)
+{
+ smmu->options |= (1 << opt);
+}
+
+static inline void arm_smmu_clear_option(struct arm_smmu_device *smmu,
+ enum arm_smmu_option opt)
+{
+ smmu->options &= ~(1 << opt);
+}
+
+static void arm_smmu_check_options(struct arm_smmu_device *smmu)
+{
+ int i = 0;
+
+ do {
+ if (of_property_read_bool(smmu->dev->of_node,
+ arm_smmu_options[i].prop))
+ arm_smmu_set_option(smmu, arm_smmu_options[i].opt);
+ i++;
+ } while (arm_smmu_options[i].opt < OPTION_MAX);
+}
+
static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu,
struct device_node *dev_node)
{
@@ -1791,6 +1838,8 @@ static int arm_smmu_device_dt_probe(struct
platform_device *pdev)
}
smmu->dev = dev;
+ arm_smmu_check_options(smmu);
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(dev, "missing base address/size\n");
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/5] ARM: dts: Add nodes for SMMUs on Calxeda ECX-2000
Signed-off-by: Andreas Herrmann
---
arch/arm/boot/dts/ecx-2000.dts| 45 +++--
arch/arm/boot/dts/ecx-common.dtsi |9 +---
2 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/ecx-2000.dts b/arch/arm/boot/dts/ecx-2000.dts
index 139b40c..905 100644
--- a/arch/arm/boot/dts/ecx-2000.dts
+++ b/arch/arm/boot/dts/ecx-2000.dts
@@ -76,10 +76,11 @@
};
soc {
- ranges = <0x 0x 0x 0x>;
+ ranges = <0x0 0x0 0x0 0x>;
timer {
- compatible = "arm,cortex-a15-timer", "arm,armv7-timer";
interrupts = <1 13 0xf08>,
+ compatible = "arm,cortex-a15-timer", "arm,armv7-timer";
+ interrupts = <1 13 0xf08>,
<1 14 0xf08>,
<1 11 0xf08>,
<1 10 0xf08>;
@@ -103,6 +104,46 @@
interrupts = <0 76 4 0 75 4 0 74 4 0 73 4>;
};
};
+
+ soc@92000 {
+ ranges = <0x9 0x2000 0x9 0x2000 0x29>;
+ #address-cells = <2>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+ interrupt-parent = <&intc>;
+
+ smmu_mac0: smmu@92000 {
+ compatible = "arm,mmu-400";
+ reg = <0x9 0x2000 0x1>;
+ #global-interrupts = <1>;
+ interrupts = <0 106 4 0 106 4>;
+ mmu-masters = <&mac0 0x0 0x1>;
+ arm,smmu-secure-config-access;
+ arm,smmu-isolate-devices;
+ };
+
+ smmu_mac1: smmu@92008 {
+ compatible = "arm,mmu-400";
+ reg = <0x9 0x2008 0x1>;
+ #global-interrupts = <1>;
+ interrupts = <0 108 4 0 108 4>;
+ mmu-masters = <&mac1 0x0 0x1>;
+ arm,smmu-secure-config-access;
+ arm,smmu-isolate-devices;
+ };
+
+ smmu_sata: smmu@92018 {
+ compatible = "arm,mmu-400";
+ reg = <0x0009 0x2018 0x1>;
+ mmu-masters = <&sata>;
+ #global-interrupts = <1>;
+ interrupts = <0 114 4 0 114 4>;
+ arm,smmu-secure-config-access;
+ arm,smmu-isolate-devices;
+ arm,smmu-mask-stream-ids;
+ };
+ };
+
};
/include/ "ecx-common.dtsi"
diff --git a/arch/arm/boot/dts/ecx-common.dtsi
b/arch/arm/boot/dts/ecx-common.dtsi
index e8559b7..50e401e 100644
--- a/arch/arm/boot/dts/ecx-common.dtsi
+++ b/arch/arm/boot/dts/ecx-common.dtsi
@@ -25,7 +25,7 @@
compatible = "simple-bus";
interrupt-parent = <&intc>;
- sata@ffe08000 {
+ sata: sata@ffe08000 {
compatible = "calxeda,hb-ahci";
reg = <0xffe08000 0x1>;
interrupts = <0 83 4>;
@@ -35,6 +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>;
};
sdhci@ffe0e000 {
@@ -208,18 +209,20 @@
clock-names = "apb_pclk";
};
- ethernet@fff5 {
+ mac0: ethernet@fff5 {
compatible = "calxeda,hb-xgmac";
reg = <0xfff5 0x1000>;
interrupts = <0 77 4 0 78 4 0 79 4>;
dma-coherent;
+ #stream-id-cells = <2>;
};
- ethernet@fff51000 {
+ mac1: ethernet@fff51000 {
compatible = "calxeda,hb-xgmac";
reg = <0xfff51000 0x1000>;
interrupts = <0 80 4 0 81 4 0 82 4>;
dma-coherent;
+ #stream-id-cells = <2>;
};
combophy0: combo-phy@fff58000 {
--
1.7.9.5
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/5] iommu/arm-smmu: Introduce stream ID masking
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.
Signed-off-by: Andreas Herrmann
---
drivers/iommu/arm-smmu.c | 43 +++
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 9a73618..e2dbcbb 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -359,6 +359,7 @@ struct arm_smmu_device {
u32 num_mapping_groups;
DECLARE_BITMAP(smr_map, ARM_SMMU_MAX_SMRS);
+ u32 smr_mask_bits;
unsigned long input_size;
unsigned long s1_output_size;
@@ -370,6 +371,7 @@ struct arm_smmu_device {
struct list_headlist;
struct rb_root masters;
+ u32 num_masters;
};
struct arm_smmu_cfg {
@@ -403,6 +405,7 @@ static LIST_HEAD(arm_smmu_devices);
/* driver options */
enum arm_smmu_option {
ISOLATE_DEVICES = 0,
+ MASK_STREAM_IDS,
OPTION_MAX,
};
@@ -413,6 +416,7 @@ struct arm_smmu_option_prop {
static struct arm_smmu_option_prop arm_smmu_options [] = {
{ ISOLATE_DEVICES, "arm,smmu-isolate-devices" },
+ { MASK_STREAM_IDS, "arm,smmu-mask-stream-ids" },
{ OPTION_MAX, NULL},
};
@@ -488,6 +492,7 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
rb_link_node(&master->node, parent, new);
rb_insert_color(&master->node, &smmu->masters);
+ smmu->num_masters++;
return 0;
}
@@ -520,8 +525,19 @@ static int register_smmu_master(struct arm_smmu_device
*smmu,
master->of_node = masterspec->np;
master->num_streamids = masterspec->args_count;
- for (i = 0; i < master->num_streamids; ++i)
- master->streamids[i] = masterspec->args[i];
+ if (arm_smmu_has_option(smmu, MASK_STREAM_IDS)) {
+ if (smmu->num_masters) {
+ dev_err(dev, "option %s not supported with multiple
masters\n",
+ arm_smmu_options[MASK_STREAM_IDS].prop);
+ return -EINVAL;
+ }
+ /* set fixed streamid (0) that will be used for masking */
+ master->num_streamids = 1;
+ master->streamids[0] = 0;
+ } else {
+ for (i = 0; i < master->num_streamids; ++i)
+ master->streamids[i] = masterspec->args[i];
+ }
return insert_smmu_master(smmu, master);
}
@@ -1063,17 +1079,20 @@ static int arm_smmu_master_configure_smrs(struct
arm_smmu_device *smmu,
goto err_free_smrs;
}
- smrs[i] = (struct arm_smmu_smr) {
- .idx= idx,
- .mask = 0, /* We don't currently share SMRs */
- .id = master->streamids[i],
- };
+ smrs[i].idx = idx;
+ smrs[i].id = master->streamids[i];
+
+ if (arm_smmu_has_option(smmu, MASK_STREAM_IDS))
+ smrs[i].mask = smmu->smr_mask_bits;
+ else
+ smrs[i].mask = 0;
}
/* It worked! Now, poke the actual hardware */
for (i = 0; i < master->num_streamids; ++i) {
u32 reg = SMR_VALID | smrs[i].id << SMR_ID_SHIFT |
smrs[i].mask << SMR_MASK_SHIFT;
+ dev_dbg(smmu->dev, "SMR%d: 0x%x\n", smrs[i].idx, reg);
writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_SMR(smrs[i].idx));
}
@@ -1726,7 +1745,7 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
}
if (id & ID0_SMS) {
- u32 smr, sid, mask;
+ u32 smr, sid;
smmu->features |= ARM_SMMU_FEAT_STREAM_MATCH;
smmu->num_mapping_groups = (id >> ID0_NUMSMRG_SHIFT) &
@@ -1742,18 +1761,18 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
- mask = (smr >> SMR_MASK_SHIFT) & SMR_MASK_MASK;
+ smmu->smr_mask_bits = (smr >> SMR_MASK_SHIFT) & SMR_MASK_MASK;
sid = (smr >> SMR_ID_SHIFT) & SMR_ID_MASK;
- if ((mask & sid) != sid) {
+ if ((smmu->smr_mask_bits & sid) != sid) {
dev_er
