From: Ramesh Gupta <[email protected]>
This patch is to flush the iommu page table entries from L1 and L2
caches using dma_map_single. This also simplifies the implementation
by removing the functions flush_iopgd_range/flush_iopte_range.
Change-Id: I19c0bf437d75c79084b2fa28c7da50a4c84b91a0
Signed-off-by: Ramesh Gupta <[email protected]>
Signed-off-by: Hari Kanigeri <[email protected]>
Signed-off-by: Fernando Guzman Lugo <[email protected]>
---
arch/arm/plat-omap/iommu.c | 41 ++++++++++-------------------------------
1 files changed, 10 insertions(+), 31 deletions(-)
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 8a51fd5..1fb5e41 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -18,6 +18,7 @@
#include <linux/ioport.h>
#include <linux/clk.h>
#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
#include <asm/cacheflush.h>
@@ -466,29 +467,6 @@ EXPORT_SYMBOL_GPL(foreach_iommu_device);
#endif /* CONFIG_OMAP_IOMMU_DEBUG_MODULE */
-/*
- * H/W pagetable operations
- */
-static void flush_iopgd_range(u32 *first, u32 *last)
-{
- /* FIXME: L2 cache should be taken care of if it exists */
- do {
- asm("mcr p15, 0, %0, c7, c10, 1 @ flush_pgd"
- : : "r" (first));
- first += L1_CACHE_BYTES / sizeof(*first);
- } while (first <= last);
-}
-
-static void flush_iopte_range(u32 *first, u32 *last)
-{
- /* FIXME: L2 cache should be taken care of if it exists */
- do {
- asm("mcr p15, 0, %0, c7, c10, 1 @ flush_pte"
- : : "r" (first));
- first += L1_CACHE_BYTES / sizeof(*first);
- } while (first <= last);
-}
-
static void iopte_free(u32 *iopte)
{
/* Note: freed iopte's must be clean ready for re-use */
@@ -515,7 +493,7 @@ static u32 *iopte_alloc(struct iommu *obj, u32 *iopgd, u32 da)
return ERR_PTR(-ENOMEM);
*iopgd = virt_to_phys(iopte) | IOPGD_TABLE;
- flush_iopgd_range(iopgd, iopgd);
+ dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
dev_vdbg(obj->dev, "%s: a new pte:%p\n", __func__, iopte);
} else {
@@ -544,7 +522,7 @@ static int iopgd_alloc_section(struct iommu *obj, u32 da, u32 pa, u32 prot)
}
*iopgd = (pa & IOSECTION_MASK) | prot | IOPGD_SECTION;
- flush_iopgd_range(iopgd, iopgd);
+ dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
return 0;
}
@@ -561,7 +539,7 @@ static int iopgd_alloc_super(struct iommu *obj, u32 da, u32 pa, u32 prot)
for (i = 0; i < 16; i++)
*(iopgd + i) = (pa & IOSUPER_MASK) | prot | IOPGD_SUPER;
- flush_iopgd_range(iopgd, iopgd + 15);
+ dma_map_single(obj->dev, iopgd, 16, DMA_TO_DEVICE);
return 0;
}
@@ -574,7 +552,7 @@ static int iopte_alloc_page(struct iommu *obj, u32 da, u32 pa, u32 prot)
return PTR_ERR(iopte);
*iopte = (pa & IOPAGE_MASK) | prot | IOPTE_SMALL;
- flush_iopte_range(iopte, iopte);
+ dma_map_single(obj->dev, iopte, 1, DMA_TO_DEVICE);
dev_vdbg(obj->dev, "%s: da:%08x pa:%08x pte:%p *pte:%08x\n",
__func__, da, pa, iopte, *iopte);
@@ -599,7 +577,7 @@ static int iopte_alloc_large(struct iommu *obj, u32 da, u32 pa, u32 prot)
for (i = 0; i < 16; i++)
*(iopte + i) = (pa & IOLARGE_MASK) | prot | IOPTE_LARGE;
- flush_iopte_range(iopte, iopte + 15);
+ dma_map_single(obj->dev, iopte, 15, DMA_TO_DEVICE);
return 0;
}
@@ -703,7 +681,8 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
}
bytes *= nent;
memset(iopte, 0, nent * sizeof(*iopte));
- flush_iopte_range(iopte, iopte + (nent - 1) * sizeof(*iopte));
+ dma_map_single(obj->dev, iopte, nent * sizeof(*iopte),
+ DMA_TO_DEVICE);
/*
* do table walk to check if this table is necessary or not
@@ -725,7 +704,7 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
bytes *= nent;
}
memset(iopgd, 0, nent * sizeof(*iopgd));
- flush_iopgd_range(iopgd, iopgd + (nent - 1) * sizeof(*iopgd));
+ dma_map_single(obj->dev, iopgd, nent * sizeof(*iopgd), DMA_TO_DEVICE);
out:
return bytes;
}
@@ -770,7 +749,7 @@ static void iopgtable_clear_entry_all(struct iommu *obj)
iopte_free(iopte_offset(iopgd, 0));
*iopgd = 0;
- flush_iopgd_range(iopgd, iopgd);
+ dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
}
flush_iotlb_all(obj);
--
1.7.0.4
On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> From: Ramesh Gupta <[email protected]>
>
> This patch is to flush the iommu page table entries from L1 and L2
> caches using dma_map_single. This also simplifies the implementation
> by removing the functions flush_iopgd_range/flush_iopte_range.
No. This usage is just wrong. If you're going to use the DMA API then
unmap it, otherwise the DMA API debugging will go awol.
Hi, Russell.
I think we need cache maintenance operations that effect on inner and
outer caches at the same time.
Even though the DMA APIs are not for cache maintenance but for IO mapping,
they are useful for cache maint'
because they operate on inner and outer caches.
As you know, inner cache of Cortex-A is logical cache and outer cache
is physical cache in the programmer's point of view.
We need logical address and physical address at the same time to clean
or invalidate inner and outer cache.
That means we need to translate logical to physical address and it is
sometimes not trivial.
Finally, the kernel will contain many similar routines that do same thing.
On Fri, Apr 15, 2011 at 7:30 AM, Russell King - ARM Linux
<[email protected]> wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> From: Ramesh Gupta <[email protected]>
>>
>> This patch is to flush the iommu page table entries from L1 and L2
>> caches using dma_map_single. This also simplifies the implementation
>> by removing the functions ?flush_iopgd_range/flush_iopte_range.
>
> No. ?This usage is just wrong. ?If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>
On Fri, Apr 15, 2011 at 11:24:16AM +0900, KyongHo Cho wrote:
> That means we need to translate logical to physical address and it is
> sometimes not trivial.
What do you mean "sometimes not trivial" ? The DMA does nothing more
than virt_to_phys(virt) to get the physical address. It's _that_ simple.
If virt_to_phys(virt) is likely to fail, there's protection in the DMA API
to BUG_ON() in that case.
> Finally, the kernel will contain many similar routines that do same thing.
So when we get coherent DMA, you won't care that the DMA API functions
start doing nothing with caches?
Russell,
On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
<[email protected]> wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> From: Ramesh Gupta <[email protected]>
>>
>> This patch is to flush the iommu page table entries from L1 and L2
>> caches using dma_map_single. This also simplifies the implementation
>> by removing the functions ?flush_iopgd_range/flush_iopte_range.
>
> No. ?This usage is just wrong. ?If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.
>
Thank you for the comments, this particular memory is always a write
from the A9 for MMU programming and
only read from the slave processor, that is the reason for not calling
the unmap. I can re-look into the changes to call
unmap in a proper way as this impacts the DMA API.
Are there any other ways to perform only flush the memory from L1/L2 caches?
regards
Ramesh Gupta G
On Friday 15 April 2011, Russell King - ARM Linux wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > From: Ramesh Gupta <[email protected]>
> >
> > This patch is to flush the iommu page table entries from L1 and L2
> > caches using dma_map_single. This also simplifies the implementation
> > by removing the functions flush_iopgd_range/flush_iopte_range.
>
> No. This usage is just wrong. If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.
It's also completely upside-down: The iommu support should provide interfaces
using the dma-mapping API, not use that API to provide a machine specific
version of the generic interface.
As far as I can tell, nothing actually uses these drivers, maybe we should just
remove them before we get any code in the mainline kernel that depends on it.
Arnd
* Arnd Bergmann <[email protected]> [110418 10:26]:
> On Friday 15 April 2011, Russell King - ARM Linux wrote:
> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > > From: Ramesh Gupta <[email protected]>
> > >
> > > This patch is to flush the iommu page table entries from L1 and L2
> > > caches using dma_map_single. This also simplifies the implementation
> > > by removing the functions flush_iopgd_range/flush_iopte_range.
> >
> > No. This usage is just wrong. If you're going to use the DMA API then
> > unmap it, otherwise the DMA API debugging will go awol.
>
>
> It's also completely upside-down: The iommu support should provide interfaces
> using the dma-mapping API, not use that API to provide a machine specific
> version of the generic interface.
>
> As far as I can tell, nothing actually uses these drivers, maybe we should just
> remove them before we get any code in the mainline kernel that depends on it.
There is drivers/media/video/omap3isp/isp.c. But if we now
have a generic replacement for this code we should start using it.
Hiroshi, any comments on that?
Regards,
Tony
From: ext Tony Lindgren <[email protected]>
Subject: Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache
Date: Mon, 18 Apr 2011 14:05:08 +0300
> * Arnd Bergmann <[email protected]> [110418 10:26]:
>> On Friday 15 April 2011, Russell King - ARM Linux wrote:
>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> > > From: Ramesh Gupta <[email protected]>
>> > >
>> > > This patch is to flush the iommu page table entries from L1 and L2
>> > > caches using dma_map_single. This also simplifies the implementation
>> > > by removing the functions flush_iopgd_range/flush_iopte_range.
>> >
>> > No. This usage is just wrong. If you're going to use the DMA API then
>> > unmap it, otherwise the DMA API debugging will go awol.
>>
>>
>> It's also completely upside-down: The iommu support should provide interfaces
>> using the dma-mapping API, not use that API to provide a machine specific
>> version of the generic interface.
>>
>> As far as I can tell, nothing actually uses these drivers, maybe we should just
>> remove them before we get any code in the mainline kernel that depends on it.
>
> There is drivers/media/video/omap3isp/isp.c. But if we now
Yes, and "dspbridge" has introduced this too, IIRC.
> have a generic replacement for this code we should start using it.
I'm afraid that there's no general IOMMU APIs yet, or already? If
there is, migrating to those general IOMMU API is the way, but still
SoC dependent parts remain, anyway. I guess that more or less general
IOMMU API is composed of common set of client APIs(like IOVMM) and the
registration of H/W dependent functions(like omap iommu), I guess.
> Hiroshi, any comments on that?
This patch is not about (1)general buffer handling between CPU cores
but just about (2)page table entry coherency. (2) is quite same to
what ARM does for its pagetable in cpu_*_set_pte_ext. I think that
using dma api to make pte entry coherent may make sense for general
solution, as Russell suggested.
For (1), I agree that IOVMM layer should be refactored by introducing
dma-mapping API in any case. Any patches are appreciated.
On Monday 18 April 2011, Tony Lindgren wrote:
> * Arnd Bergmann <[email protected]> [110418 10:26]:
> > On Friday 15 April 2011, Russell King - ARM Linux wrote:
> > > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > > > From: Ramesh Gupta <[email protected]>
> > > >
> > > > This patch is to flush the iommu page table entries from L1 and L2
> > > > caches using dma_map_single. This also simplifies the implementation
> > > > by removing the functions flush_iopgd_range/flush_iopte_range.
> > >
> > > No. This usage is just wrong. If you're going to use the DMA API then
> > > unmap it, otherwise the DMA API debugging will go awol.
> >
> >
> > It's also completely upside-down: The iommu support should provide interfaces
> > using the dma-mapping API, not use that API to provide a machine specific
> > version of the generic interface.
> >
> > As far as I can tell, nothing actually uses these drivers, maybe we should just
> > remove them before we get any code in the mainline kernel that depends on it.
>
> There is drivers/media/video/omap3isp/isp.c.
Ah, I didn't see that, was looking at an older version.
> But if we now have a generic replacement for this code we should start
> using it.
To give some background:
Historically, the dma-mapping API has been used for all IOMMUs on
architectures that need it. This interface is rather flexible,
but ARM currently only uses it for managing static mappings.
One thing that it cannot do is mapping memory to an arbitrary
(driver-chosen) bus address. The generic iommu API was added in order
to do that, and is mostly used for virtual machines with KVM.
The MSM platform in ARM also added support for the generic iommu
API, and now the exynos4 is gaining support for it as well.
One missing piece is still a way for a platform to provide both
the iommu and the dma-mapping API in a unified driver. Right now,
you have to export both interface for a generic solution.
On ARM, we don't yet use include/asm-generic/dma-mapping-common.h,
which allows overriding the dma-mapping API per device. This would
have to change if you want to export the IOMMU functionality using
the dma-mapping API, but that would also allow abstracting the
various ways we currently have (dmabounce, swiotlb, linear map,
custom __arch_page_to_dma, iommu, coherent or noncoherent DMA)
in a nicer way per device.
Arnd
On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <[email protected]> wrote:
> On Monday 18 April 2011, Tony Lindgren wrote:
>> * Arnd Bergmann <[email protected]> [110418 10:26]:
>> > On Friday 15 April 2011, Russell King - ARM Linux wrote:
>> > > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> > > > From: Ramesh Gupta <[email protected]>
>> > > >
>> > > > This patch is to flush the iommu page table entries from L1 and L2
>> > > > caches using dma_map_single. This also simplifies the implementation
>> > > > by removing the functions ?flush_iopgd_range/flush_iopte_range.
>> > >
>> > > No. ?This usage is just wrong. ?If you're going to use the DMA API then
>> > > unmap it, otherwise the DMA API debugging will go awol.
>> >
>> >
>> > It's also completely upside-down: The iommu support should provide interfaces
>> > using the dma-mapping API, not use that API to provide a machine specific
>> > version of the generic interface.
>> >
>> > As far as I can tell, nothing actually uses these drivers, maybe we should just
>> > remove them before we get any code in the mainline kernel that depends on it.
>>
>> There is drivers/media/video/omap3isp/isp.c.
>
> Ah, I didn't see that, was looking at an older version.
>
>> But if we now have a generic replacement for this code we should start
>> using it.
>
> To give some background:
>
> Historically, the dma-mapping API has been used for all IOMMUs on
> architectures that need it. This interface is rather flexible,
> but ARM currently only uses it for managing static mappings.
> One thing that it cannot do is mapping memory to an arbitrary
> (driver-chosen) bus address. The generic iommu API was added in order
> to do that, and is mostly used for virtual machines with KVM.
>
> The MSM platform in ARM also added support for the generic iommu
> API, and now the exynos4 is gaining support for it as well.
You can find a exynos4 patches.
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/31613
>
> One missing piece is still a way for a platform to provide both
> the iommu and the dma-mapping API in a unified driver. Right now,
> you have to export both interface for a generic solution.
Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
implementation into mm, but MM did't accept it.
So now it's implemented at each SoCs. I think it's good chance to make
a generic IOMMU feature for ARM consolidation.
>
> On ARM, we don't yet use include/asm-generic/dma-mapping-common.h,
> which allows overriding the dma-mapping API per device. This would
> have to change if you want to export the IOMMU functionality using
> the dma-mapping API, but that would also allow abstracting the
> various ways we currently have (dmabounce, swiotlb, linear map,
> custom __arch_page_to_dma, iommu, coherent or noncoherent DMA)
> in a nicer way per device.
Before this idea, can you review our implementation at above URL?
Thank you,
Kyungmin Park
>
> ? ? ? ?Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
On Monday 18 April 2011, Hiroshi DOYU wrote:
> From: ext Tony Lindgren <[email protected]>
> > have a generic replacement for this code we should start using it.
>
> I'm afraid that there's no general IOMMU APIs yet, or already? If
> there is, migrating to those general IOMMU API is the way, but still
> SoC dependent parts remain, anyway. I guess that more or less general
> IOMMU API is composed of common set of client APIs(like IOVMM) and the
> registration of H/W dependent functions(like omap iommu), I guess.
As I explained, we have a surplus of generic iommu APIs
already, see include/linux/dma-mapping.h and include/linux/iommu.h
The dma-mapping interface handles IOMMUs on all other architectures
that have them.
Arnd
On Monday 18 April 2011, Kyungmin Park wrote:
> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <[email protected]> wrote:
> >
> > One missing piece is still a way for a platform to provide both
> > the iommu and the dma-mapping API in a unified driver. Right now,
> > you have to export both interface for a generic solution.
>
> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> implementation into mm, but MM did't accept it.
I'm confused. What do you mean with MM?
> So now it's implemented at each SoCs. I think it's good chance to make
> a generic IOMMU feature for ARM consolidation.
>
> Before this idea, can you review our implementation at above URL?
I've commented on the main implementation for the IOMMU now.
Arnd
On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <[email protected]> wrote:
> On Monday 18 April 2011, Kyungmin Park wrote:
>> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <[email protected]> wrote:
>> >
>> > One missing piece is still a way for a platform to provide both
>> > the iommu and the dma-mapping API in a unified driver. Right now,
>> > you have to export both interface for a generic solution.
>>
>> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
>> implementation into mm, but MM did't accept it.
>
> I'm confused. What do you mean with MM?
linux/mm, Memory Management.
Thank you,
Kyungmin Park
>
>> So now it's implemented at each SoCs. I think it's good chance to make
>> a generic IOMMU feature for ARM consolidation.
>>
>> Before this idea, can you review our implementation at above URL?
>
> I've commented on the main implementation for the IOMMU now.
>
> ? ? ? ?Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
On Tuesday 19 April 2011, Kyungmin Park wrote:
> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <[email protected]> wrote:
> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <[email protected]> wrote:
> >> >
> >> > One missing piece is still a way for a platform to provide both
> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> > you have to export both interface for a generic solution.
> >>
> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> implementation into mm, but MM did't accept it.
> >
> > I'm confused. What do you mean with MM?
> linux/mm, Memory Management.
I'm still confused. What were you suggesting to merge in there?
Do you have a link to a mailing list discussion?
Arnd
On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <[email protected]> wrote:
> On Tuesday 19 April 2011, Kyungmin Park wrote:
>> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <[email protected]> wrote:
>> > On Monday 18 April 2011, Kyungmin Park wrote:
>> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <[email protected]> wrote:
>> >> >
>> >> > One missing piece is still a way for a platform to provide both
>> >> > the iommu and the dma-mapping API in a unified driver. Right now,
>> >> > you have to export both interface for a generic solution.
>> >>
>> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
>> >> implementation into mm, but MM did't accept it.
>> >
>> > I'm confused. What do you mean with MM?
>> linux/mm, Memory Management.
>
> I'm still confused. What were you suggesting to merge in there?
> Do you have a link to a mailing list discussion?
First, Zach Pfeffer <[email protected]> sent the patch
https://patchwork.kernel.org/patch/108736/
Second, Michal tried it.
http://lkml.org/lkml/2010/9/6/41
http://lwn.net/Articles/403643/
https://patchwork.kernel.org/patch/157451/
Thank you,
Kyungmin Park
>
> ? ? ? ?Arnd
>
On Tue, Apr 19, 2011 at 09:35:56PM +0900, Kyungmin Park wrote:
> On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <[email protected]> wrote:
> > On Tuesday 19 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <[email protected]> wrote:
> >> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <[email protected]> wrote:
> >> >> >
> >> >> > One missing piece is still a way for a platform to provide both
> >> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> >> > you have to export both interface for a generic solution.
> >> >>
> >> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> >> implementation into mm, but MM did't accept it.
> >> >
> >> > I'm confused. What do you mean with MM?
> >> linux/mm, Memory Management.
> >
> > I'm still confused. What were you suggesting to merge in there?
> > Do you have a link to a mailing list discussion?
>
> First, Zach Pfeffer <[email protected]> sent the patch
> https://patchwork.kernel.org/patch/108736/
>
> Second, Michal tried it.
> http://lkml.org/lkml/2010/9/6/41
>
> http://lwn.net/Articles/403643/
> https://patchwork.kernel.org/patch/157451/
This shows why ARM is in such a problem. People love to create new
frameworks and infrastructure where they find existing stuff lacking,
rather than looking at what other architectures do and adapt.
As I'm sure has already mentioned, the kernel already has support for
IOMMUs in the DMA path with several non-ARM architectures, and this
support is managed through the existing DMA API.
When you want a device behind an IOMMU to perform DMA, the driver
uses the DMA API in the standard way as if there was no IOMMU there.
As part of the DMA API, particularly the SG list part, the DMA API
is allowed to coalesce the SG entries together if it can arrange
the IOMMU mappings to achieve a continguous mapping for the device.
Remember, dma_map_sg() is allowed to return _fewer_ entries in the
scatterlist than was passed to it for this very purpose.
In order to transition ARM over to this, we need to modify the
scatterlist structure by enabling CONFIG_NEED_SG_DMA_LENGTH. We
then need to arrange for the DMA API to have the hooks _both_ into
the DMA cache coherency code (to ensure that the data hits memory)
and the IOMMU code (this part is missing from ARM.) The current
abstractions in the generic header files don't allow for this.
I don't believe there's any need for any major new framework - and I
don't think I'm alone in thinking that, especially as this point has
been raised more than once when this framework has been submitted.
I see no reason why ARM should be any different from other architectures
which have IOMMUs, and I don't see why ARM should have to invent a whole
new framework to handle IOMMUs. And I see no explanation why the
existing hooks are unsuitable - at least as the _initial_ starting point.
So, can you please explain why your IOMMU code can not be hidden behind
the DMA API. (Please omit any complaints about the mechanics of hooking
it in there, such things are solvable.)
On Tuesday 19 April 2011, Kyungmin Park wrote:
> On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <[email protected]> wrote:
> > On Tuesday 19 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <[email protected]> wrote:
> >> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <[email protected]> wrote:
> >> >> >
> >> >> > One missing piece is still a way for a platform to provide both
> >> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> >> > you have to export both interface for a generic solution.
> >> >>
> >> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> >> implementation into mm, but MM did't accept it.
> >> >
> >> > I'm confused. What do you mean with MM?
> >> linux/mm, Memory Management.
> >
> > I'm still confused. What were you suggesting to merge in there?
> > Do you have a link to a mailing list discussion?
>
> First, Zach Pfeffer <[email protected]> sent the patch
> https://patchwork.kernel.org/patch/108736/
>
> Second, Michal tried it.
> http://lkml.org/lkml/2010/9/6/41
>
> http://lwn.net/Articles/403643/
> https://patchwork.kernel.org/patch/157451/
Ah, I did not realize you were talking about VCMM. I believe the main
reason what that patch was not received well is that it tried to add
yet another abstraction for IOMMUs when we already have too many of them.
It is essentially doing the opposite of what I was referring to above:
If we had added VCMM, each platform that has IOMMUs would now have to
implement three interfaces: dma-mapping.h (struct dma_map_ops),
iommu.h (struct iommu_ops) and and vcm_driver.h (struct vcm_driver).
What I really meant was to unify the two we already have, so that
a platform only needs to implement e.g. iommu_ops and get the dma_mapping.h
interfaces for free.
Arnd
On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
> Russell,
>
> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
> <[email protected]> wrote:
> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> >> From: Ramesh Gupta <[email protected]>
> >>
> >> This patch is to flush the iommu page table entries from L1 and L2
> >> caches using dma_map_single. This also simplifies the implementation
> >> by removing the functions ?flush_iopgd_range/flush_iopte_range.
> >
> > No. ?This usage is just wrong. ?If you're going to use the DMA API then
> > unmap it, otherwise the DMA API debugging will go awol.
> >
>
> Thank you for the comments, this particular memory is always a write
> from the A9 for MMU programming and
> only read from the slave processor, that is the reason for not calling
> the unmap. I can re-look into the changes to call
> unmap in a proper way as this impacts the DMA API.
> Are there any other ways to perform only flush the memory from L1/L2 caches?
We _could_ invent a new API to deal with this, which is probably going
to be far better in the longer term for page table based iommus. That's
going to need some thought - eg, do we need to pass a struct device
argument for the iommu cache flushing so we know whether we need to flush
or not (eg, if we have cache coherent iommus)...
Hi Russel,
On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
<[email protected]> wrote:
> On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
>> Russell,
>>
>> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
>> <[email protected]> wrote:
>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> >> From: Ramesh Gupta <[email protected]>
>> >>
>> >> This patch is to flush the iommu page table entries from L1 and L2
>> >> caches using dma_map_single. This also simplifies the implementation
>> >> by removing the functions ?flush_iopgd_range/flush_iopte_range.
>> >
>> > No. ?This usage is just wrong. ?If you're going to use the DMA API then
>> > unmap it, otherwise the DMA API debugging will go awol.
>> >
>>
>> Thank you for the comments, this particular memory is always a write
>> from the A9 for MMU programming and
>> only read from the slave processor, that is the reason for not calling
>> the unmap. I can re-look into the changes to call
>> unmap in a proper way as this impacts the DMA API.
>> Are there any other ways to perform only flush the memory from L1/L2 caches?
>
> We _could_ invent a new API to deal with this, which is probably going
> to be far better in the longer term for page table based iommus. ?That's
> going to need some thought - eg, do we need to pass a struct device
> argument for the iommu cache flushing so we know whether we need to flush
> or not (eg, if we have cache coherent iommus)...
I agree with you, right now these functions are taking only start and
end arguments but
we can pass a struct deivice to these functions.
thank you and regards
Ramesh Gupta G
Hi Russell,
On Fri, Aug 12, 2011 at 9:35 PM, Gupta, Ramesh <[email protected]> wrote:
> On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
> <[email protected]> wrote:
>> On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
>>> Hi Russel,
>>
>> grr.
>
> Sorry, typo.
>
>>
>>> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <[email protected]> wrote:
>>> > Hi Russel,
>>> >
>>> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
>>> > <[email protected]> wrote:
>>> >> We _could_ invent a new API to deal with this, which is probably going
>>> >> to be far better in the longer term for page table based iommus. ?That's
>>> >> going to need some thought - eg, do we need to pass a struct device
>>> >> argument for the iommu cache flushing so we know whether we need to flush
>>> >> or not (eg, if we have cache coherent iommus)...
>>>
>>> my apologies for a late mail on this topic.
>>>
>>> do you think of any other requirements for this new API?
>>>
>>> Could we use the existing dmac_flush_range(), outer_flush_range()
>>> for this purpose instead of a new API?
>>>
>>> I see a comment in the arch/arm/include/asm/cacheflush.h
>>> for ?_not_ to use these APIs directly, but I am not really understand
>>> the reason for that.
>>
>> When I create APIs, I create them to solve a _purpose_ to code which wants
>> to do something. ?They're not created to provide some facility which can
>> be re-used for unrelated stuff.
>>
>> This has been proven many times to be the correct approach. ?Over time,
>> things change.
>>
>
> I agree.
>
>> Let's say for arguments sake that you decide to use the DMA API stuff
>> to achieve your goals. ?Then, lets say that ARM DMA becomes fully
>> cache coherent, but your IOMMU tables still need to be flushed from
>> the L1 caches.
>>
>> Suddenly, dmac_flush_range() starts doing absolutely nothing. ?Your
>> driver breaks. ?I get whinged at because a previously working driver
>> stops working. ?In effect, that usage _prevents_ me making the changes
>> necessary to keep the core architecture support moving forward as
>> things develop. ?Or, alternatively I just ignore your driver, make the
>> changes anyway and leave it to rot.
>>
>> So, APIs get created to provide a purpose. ?Like - handling the DMA
>> issues when mapping a buffer to DMA. ?Like handling the DMA issues
>> when unmapping a buffer from DMA. ?If you start using those _because_
>> they happen to clean or invalidate the cache for you, you're really
>> asking for your driver to be broken at some point in the future.
>>
>> What is far better to do is to ensure that we have the right APIs in
>> place for the purposes for which they are to be used. ?So, if we need
>> an API to flush out the IOMMU page table entries, then that's what
>> we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
>> Inside the processors implementation, yes, it may well be the same
>> thing, but that's a decision for the _processor_ support code to make,
>> not the IOMMU writer.
>
> I completely agree, thank you for explaining the need to use the correct API.
>
>
>> As to what shape a new API should be - as I said above, maybe it should
>> take a struct device argument, virtual base address and size. ?Or maybe
>> if we don't have any coherent IOMMUs then just ignore the device
>> argument for the time being, and just pass the virtual base address and
>> size.
>>
>> The next issue is whether it should require the virtual base address to
>> be in the kernel direct mapped region. ?If you're touching L2, then
>> that's a yes, because we need to use virt_to_phys on it to get at the
>> phys address for the L2 operations.
>>
>> So, I think: extend the cpu cache operations structure to have a method
>> for dealing with IOMMUs. ?Add an inline function to deal with calling
>> that, and the L2 ops if that's what's required.
my apologies for a late response. I have posted a patch for a new L1
cache maintenance api for handling iommu as you suggested. I will send
iommu patches to use this api.
Thank you and regards
Ramesh Gupta G