Re: [PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Marek Szyprowski
Hi Thierry,

On 2019-02-05 23:29, Thierry Reding wrote:
> On Tue, Feb 05, 2019 at 07:02:18PM +0100, Christoph Hellwig wrote:
>> On Tue, Feb 05, 2019 at 06:56:11PM +0100, Thierry Reding wrote:
>>> Sure, here you go:
>>>
>>> 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/host1x/cdma.c#n106
>> Yes, I think we I can come up with a nicer helper for that.
> One thing I could also try is to remove direct IOMMU API usage at least
> from the host1x driver. I think this might work nowadays.
>
> For Tegra DRM we still need direct IOMMU API usage because we need to be
> able to map into an IOMMU domain without knowing the struct device *
> that we're mapping for (it could be needed by any of up to four display
> controllers). For host1x we always only have one struct device *, so the
> DMA mapping API should be good enough.

In case of Tegra DRM you may try to use Exynos DRM approach. Exynos DRM
also consists of more than one DMA capable device and uses one common
IOMMU address space for them. It is achieved by attaching all those
devices to the IOMMU domain of the first CRTC device that has been
registered. Then that device is used for DMA-mapping calls. It is not
very elegant, but works fine and allows to use standard DMA-mapping calls.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] dma-direct: do not allocate a single page from CMA area

2019-02-05 Thread Christoph Hellwig
On Tue, Feb 05, 2019 at 03:05:30PM -0800, Nicolin Chen wrote:
> > And my other concern is that this skips allocating from the per-device
> > pool, which drivers might rely on.
> 
> Actually Robin had the same concern at v1 and suggested that we could
> always use DMA_ATTR_FORCE_CONTIGUOUS to enforce into per-device pool.

That is both against the documented behavior of DMA_ATTR_FORCE_CONTIGUOUS
and doesn't help existing drivers that specify their CMA area in DT.

> > To be honest I'm not sure there is
> > much of a point in the per-device CMA pool vs the traditional per-device
> > coherent pool, but I'd rather change that behavior in a clearly documented
> > commit with intentions rather as a side effect from a random optimization.
> 
> Hmm..sorry, I don't really follow this suggestion. Is it possible for
> you to make it clear that what should I do for the change?

Something like this (plus proper comments):

diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index b2a87905846d..789d734f0f77 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -192,10 +192,19 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
phys_addr_t base,
 struct page *dma_alloc_from_contiguous(struct device *dev, size_t count,
   unsigned int align, bool no_warn)
 {
+   struct cma *cma;
+
if (align > CONFIG_CMA_ALIGNMENT)
align = CONFIG_CMA_ALIGNMENT;
 
-   return cma_alloc(dev_get_cma_area(dev), count, align, no_warn);
+   if (dev && dev->cma_area)
+   cma = dev->cma_area;
+   else if (count > PAGE_SIZE)
+   cma = dma_contiguous_default_area;
+   else
+   return NULL;
+
+   return cma_alloc(cma, count, align, no_warn);
 }
 
 /**
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Christoph Hellwig
On Tue, Feb 05, 2019 at 11:29:12PM +0100, Thierry Reding wrote:
> > >   
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/host1x/cdma.c#n106
> > 
> > Yes, I think we I can come up with a nicer helper for that.
> 
> One thing I could also try is to remove direct IOMMU API usage at least
> from the host1x driver. I think this might work nowadays.
> 
> For Tegra DRM we still need direct IOMMU API usage because we need to be
> able to map into an IOMMU domain without knowing the struct device *
> that we're mapping for (it could be needed by any of up to four display
> controllers). For host1x we always only have one struct device *, so the
> DMA mapping API should be good enough.

If you can convert it to plain DMA API usage, please do.

I did look into an IOMMU API memory allocator, and while we can do it
easily for coherent devices, we need some arch hooks for non-coherent
device support.  With a pending series from me we have those for arm64,
but 32-bit arm support will require a lot more work first.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: Allow io-pgtable to be used outside of drivers/iommu/

2019-02-05 Thread Christoph Hellwig
On Tue, Feb 05, 2019 at 05:12:29PM +, Robin Murphy wrote:
> On 05/02/2019 16:55, Christoph Hellwig wrote:
> > On Tue, Feb 05, 2019 at 10:37:31AM -0600, Rob Herring wrote:
> > > Move io-pgtable.h to include/linux/ and export alloc_io_pgtable_ops
> > > and free_io_pgtable_ops. This enables drivers outside drivers/iommu/ to
> > > use the ARM page table library. Specifically, some ARM Mali GPUs use the
> > > ARM page table formats.
> > 
> > Maybe rename it to arm-io-pgtable.h to make the usage a little more
> > clear?
> 
> It's not Arm-specific, though - the whole point of io-pgtable is to be an
> architecture-agnostic library of IOMMU pagetable code. It just happens that
> the only formats implemented so far are the Arm ones that already have more
> than one in-tree user each.

Oh, ok.  I always thought of it as an ARM thing, but then I missed the
intention behind it.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

2019-02-05 Thread Michael S. Tsirkin
On Fri, Feb 01, 2019 at 09:09:46AM +0100, Christoph Hellwig wrote:
> For some reason patch 5 didn't make it to my inbox, but assuming
> nothing has changed this whole series looks good to me now.

Could you send a formal series ack pls?

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

2019-02-05 Thread Michael S. Tsirkin
On Thu, Jan 31, 2019 at 05:33:58PM +0100, Joerg Roedel wrote:
> Hi,
> 
> here is the next version of this patch-set. Previous
> versions can be found here:
> 
>   V1: https://lore.kernel.org/lkml/20190110134433.15672-1-j...@8bytes.org/
> 
>   V2: https://lore.kernel.org/lkml/20190115132257.6426-1-j...@8bytes.org/
> 
>   V3: https://lore.kernel.org/lkml/20190123163049.24863-1-j...@8bytes.org/
> 
>   V4: https://lore.kernel.org/lkml/20190129084342.26030-1-j...@8bytes.org/
> 
>   V5: https://lore.kernel.org/lkml/20190130164007.26497-1-j...@8bytes.org/
> 
> The problem solved here is a limitation of the SWIOTLB implementation,
> which does not support allocations larger than 256kb.  When the
> virtio-blk driver tries to read/write a block larger than that, the
> allocation of the dma-handle fails and an IO error is reported.
> 
> Changes to v5 are:
> 
>   - Changed patch 3 to uninline dma_max_mapping_size()

And this lead to problems reported by kbuild :(

BTW when you repost, can I ask you to pls include
the version in all patches? Both --subject-prefix
and -v flags to git format-patch will do this for you.


> Please review.
> 
> Thanks,
> 
>   Joerg
> 
> Joerg Roedel (5):
>   swiotlb: Introduce swiotlb_max_mapping_size()
>   swiotlb: Add is_swiotlb_active() function
>   dma: Introduce dma_max_mapping_size()
>   virtio: Introduce virtio_max_dma_size()
>   virtio-blk: Consider virtio_max_dma_size() for maximum segment size
> 
>  Documentation/DMA-API.txt|  8 
>  drivers/block/virtio_blk.c   | 10 ++
>  drivers/virtio/virtio_ring.c | 11 +++
>  include/linux/dma-mapping.h  |  8 
>  include/linux/swiotlb.h  | 11 +++
>  include/linux/virtio.h   |  2 ++
>  kernel/dma/direct.c  | 11 +++
>  kernel/dma/mapping.c | 14 ++
>  kernel/dma/swiotlb.c | 14 ++
>  9 files changed, 85 insertions(+), 4 deletions(-)
> 
> -- 
> 2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/9] iommu/vt-d: Allow interrupts from the entire bus for aliased devices

2019-02-05 Thread Logan Gunthorpe



On 2019-02-05 12:19 p.m., Jacob Pan wrote:
> On Fri, 1 Feb 2019 10:27:29 -0700
> Logan Gunthorpe  wrote:
> 
>> On 2019-02-01 9:44 a.m., Joerg Roedel wrote:
>>> On Thu, Jan 31, 2019 at 11:56:48AM -0700, Logan Gunthorpe wrote:  
 @@ -394,6 +402,10 @@ static int set_msi_sid(struct irte *irte,
 struct pci_dev *dev) set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16,
 PCI_DEVID(PCI_BUS_NUM(data.alias),
   dev->bus->number));
> I guess devfn can be removed also. but that is separate cleanup.

Actually, no, I've dug into this and we *do* need the devfn here but
it's needlessly confusing. We should not be using PCI_DEVID() as we
aren't actually representing a DEVID in this case...

According to the Intel VT-D spec, when using SVT_VERIFY_BUS, the MSB of
the SID field represents the starting bus number and the LSB represents
the end bus number. The requester id's bus number must then be within
that range. The PCI_DEVID macro matches these semantics if you assume
the devfn is the end bus, but doesn't really make sense here and just
confuses the issue.

So the code was correct, I'll just try to clean it up to make it less
confusing.

Thanks,

Logan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/9] iommu/vt-d: Allow interrupts from the entire bus for aliased devices

2019-02-05 Thread Jacob Pan
On Fri, 1 Feb 2019 10:27:29 -0700
Logan Gunthorpe  wrote:

> On 2019-02-01 9:44 a.m., Joerg Roedel wrote:
> > On Thu, Jan 31, 2019 at 11:56:48AM -0700, Logan Gunthorpe wrote:  
> >> @@ -394,6 +402,10 @@ static int set_msi_sid(struct irte *irte,
> >> struct pci_dev *dev) set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16,
> >> PCI_DEVID(PCI_BUS_NUM(data.alias),
> >>   dev->bus->number));
I guess devfn can be removed also. but that is separate cleanup.
 
> >> +  else if (data.count >= 2 && data.busmatch_count ==
> >> data.count)
> >> +  set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16,
> >> +   PCI_DEVID(dev->bus->number,
> >> + dev->bus->number));  
> > 
> > The dev->bus->number argument for the devfn parameter of PCI_DEVID
> > is not needed, right?  
> 
> Oh, yes. I think you are right.
> 
> > Also, this requires at least a comment to document that special
> > case.  
> 
> I'll add a comment for v2.
> 
> Thanks for the review!
> 
> Logan
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Christoph Hellwig
On Tue, Feb 05, 2019 at 06:56:11PM +0100, Thierry Reding wrote:
> Sure, here you go:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/host1x/cdma.c#n106

Yes, I think we I can come up with a nicer helper for that.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Thierry Reding
On Tue, Feb 05, 2019 at 05:38:37PM +0100, Christoph Hellwig wrote:
> On Tue, Feb 05, 2019 at 05:20:57PM +0100, Thierry Reding wrote:
> > The problem is that if I use dma_alloc_coherent(), then the memory will
> > already be mapped via the SMMU at that point and then the driver, not
> > knowing that memory has already been mapped, will attempt to map an IOVA
> > which will cause an SMMU fault when the host1x tries to access the
> > memory.
> > 
> > I didn't find an equivalent to arm_iommu_detach_device() for non-ARM,
> > but then stumbled across this and thought it was rather convenient for
> > these cases. If there's a better way to deal with this situation, I'd be
> > happy to do so.
> 
> So you basically do a dma_direct_alloc + iommu_map?  Can you send me
> a pointer to your code?  Maybe we need to add a proper IOMMU-layer
> API for that.

Sure, here you go:


https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/host1x/cdma.c#n106

Thierry


signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu: Allow io-pgtable to be used outside of drivers/iommu/

2019-02-05 Thread Rob Herring
On Tue, Feb 5, 2019 at 10:55 AM Christoph Hellwig  wrote:
>
> On Tue, Feb 05, 2019 at 10:37:31AM -0600, Rob Herring wrote:
> > Move io-pgtable.h to include/linux/ and export alloc_io_pgtable_ops
> > and free_io_pgtable_ops. This enables drivers outside drivers/iommu/ to
> > use the ARM page table library. Specifically, some ARM Mali GPUs use the
> > ARM page table formats.
>
> Maybe rename it to arm-io-pgtable.h to make the usage a little more
> clear?

I should drop the first 'ARM' in the commit message.

Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: Allow io-pgtable to be used outside of drivers/iommu/

2019-02-05 Thread Robin Murphy

On 05/02/2019 16:55, Christoph Hellwig wrote:

On Tue, Feb 05, 2019 at 10:37:31AM -0600, Rob Herring wrote:

Move io-pgtable.h to include/linux/ and export alloc_io_pgtable_ops
and free_io_pgtable_ops. This enables drivers outside drivers/iommu/ to
use the ARM page table library. Specifically, some ARM Mali GPUs use the
ARM page table formats.


Maybe rename it to arm-io-pgtable.h to make the usage a little more
clear?


It's not Arm-specific, though - the whole point of io-pgtable is to be 
an architecture-agnostic library of IOMMU pagetable code. It just 
happens that the only formats implemented so far are the Arm ones that 
already have more than one in-tree user each.


Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: Allow io-pgtable to be used outside of drivers/iommu/

2019-02-05 Thread Christoph Hellwig
On Tue, Feb 05, 2019 at 10:37:31AM -0600, Rob Herring wrote:
> Move io-pgtable.h to include/linux/ and export alloc_io_pgtable_ops
> and free_io_pgtable_ops. This enables drivers outside drivers/iommu/ to
> use the ARM page table library. Specifically, some ARM Mali GPUs use the
> ARM page table formats.

Maybe rename it to arm-io-pgtable.h to make the usage a little more
clear? 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Christoph Hellwig
On Tue, Feb 05, 2019 at 05:20:57PM +0100, Thierry Reding wrote:
> The problem is that if I use dma_alloc_coherent(), then the memory will
> already be mapped via the SMMU at that point and then the driver, not
> knowing that memory has already been mapped, will attempt to map an IOVA
> which will cause an SMMU fault when the host1x tries to access the
> memory.
> 
> I didn't find an equivalent to arm_iommu_detach_device() for non-ARM,
> but then stumbled across this and thought it was rather convenient for
> these cases. If there's a better way to deal with this situation, I'd be
> happy to do so.

So you basically do a dma_direct_alloc + iommu_map?  Can you send me
a pointer to your code?  Maybe we need to add a proper IOMMU-layer
API for that.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu: Allow io-pgtable to be used outside of drivers/iommu/

2019-02-05 Thread Rob Herring
Move io-pgtable.h to include/linux/ and export alloc_io_pgtable_ops
and free_io_pgtable_ops. This enables drivers outside drivers/iommu/ to
use the ARM page table library. Specifically, some ARM Mali GPUs use the
ARM page table formats.

Cc: Will Deacon 
Cc: Robin Murphy 
Cc: Joerg Roedel 
Cc: Matthias Brugger 
Cc: Rob Clark 
Cc: linux-arm-ker...@lists.infradead.org
Cc: iommu@lists.linux-foundation.org
Cc: linux-media...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Signed-off-by: Rob Herring 
---
This will be needed for an in-progress Mali GPU DRM driver. It's using 
the page table lib, but is not a full IOMMU driver.

It's going to be a few cycles I'd guess before the DRM driver is 
anywhere close to merging. So I can carry this if preferred, but release 
early, release often.

Rob

 drivers/iommu/arm-smmu-v3.c   | 3 +--
 drivers/iommu/arm-smmu.c  | 2 +-
 drivers/iommu/io-pgtable-arm-v7s.c| 3 +--
 drivers/iommu/io-pgtable-arm.c| 3 +--
 drivers/iommu/io-pgtable.c| 5 +++--
 drivers/iommu/ipmmu-vmsa.c| 3 +--
 drivers/iommu/msm_iommu.c | 2 +-
 drivers/iommu/mtk_iommu.h | 3 +--
 drivers/iommu/qcom_iommu.c| 2 +-
 {drivers/iommu => include/linux}/io-pgtable.h | 0
 10 files changed, 11 insertions(+), 15 deletions(-)
 rename {drivers/iommu => include/linux}/io-pgtable.h (100%)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 0d284029dc73..d3880010c6cf 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -32,8 +33,6 @@
 
 #include 
 
-#include "io-pgtable.h"
-
 /* MMIO registers */
 #define ARM_SMMU_IDR0  0x0
 #define IDR0_ST_LVLGENMASK(28, 27)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index af18a7e7f917..045d93884164 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -56,7 +57,6 @@
 #include 
 #include 
 
-#include "io-pgtable.h"
 #include "arm-smmu-regs.h"
 
 #define ARM_MMU500_ACTLR_CPRE  (1 << 1)
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c 
b/drivers/iommu/io-pgtable-arm-v7s.c
index cec29bf45c9b..75a8273d1ae9 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -45,8 +46,6 @@
 
 #include 
 
-#include "io-pgtable.h"
-
 /* Struct accessors */
 #define io_pgtable_to_data(x)  \
container_of((x), struct arm_v7s_io_pgtable, iop)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 237cacd4a62b..d3700ec15cbd 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -22,6 +22,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -31,8 +32,6 @@
 
 #include 
 
-#include "io-pgtable.h"
-
 #define ARM_LPAE_MAX_ADDR_BITS 52
 #define ARM_LPAE_S2_MAX_CONCAT_PAGES   16
 #define ARM_LPAE_MAX_LEVELS4
diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
index 127558d83667..93f2880be6c6 100644
--- a/drivers/iommu/io-pgtable.c
+++ b/drivers/iommu/io-pgtable.c
@@ -19,11 +19,10 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 
-#include "io-pgtable.h"
-
 static const struct io_pgtable_init_fns *
 io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = {
 #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE
@@ -61,6 +60,7 @@ struct io_pgtable_ops *alloc_io_pgtable_ops(enum 
io_pgtable_fmt fmt,
 
return >ops;
 }
+EXPORT_SYMBOL_GPL(alloc_io_pgtable_ops);
 
 /*
  * It is the IOMMU driver's responsibility to ensure that the page table
@@ -77,3 +77,4 @@ void free_io_pgtable_ops(struct io_pgtable_ops *ops)
io_pgtable_tlb_flush_all(iop);
io_pgtable_init_table[iop->fmt]->free(iop);
 }
+EXPORT_SYMBOL_GPL(free_io_pgtable_ops);
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 7a4529c61c19..9a380c10655e 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -35,8 +36,6 @@
 #define arm_iommu_detach_device(...)   do {} while (0)
 #endif
 
-#include "io-pgtable.h"
-
 #define IPMMU_CTX_MAX 8
 
 struct ipmmu_features {
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index fc4270733f11..ef7d1f995d6b 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -37,7 +38,6 @@
 
 #include "msm_iommu_hw-8xxx.h"
 #include "msm_iommu.h"
-#include "io-pgtable.h"
 
 #define MRC(reg, processor, op1, crn, 

Re: [PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Thierry Reding
On Tue, Feb 05, 2019 at 05:10:36PM +0100, Christoph Hellwig wrote:
> On Tue, Feb 05, 2019 at 12:06:02PM +0100, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > Drivers that are built as modules may want to use these functions, so
> > make them available like the rest of the functions.
> > 
> > Signed-off-by: Thierry Reding 
> 
> How do they want to use these functions?  The proper way is to
> call dma_alloc_coherent / dma_alloc_free and let the DMA API
> handle the rest.

This is again for the host1x driver and Tegra DRM where we had a similar
situation a little while ago where we needed to explicitly detach from a
DMA/IOMMU mapping.

I ran into a similar situation on Tegra186 (which has an ARM SMMU). The
host1x driver needs to allocate memory for a push buffer that commands
are written to. This push buffer is mapped translated through the SMMU
using direct IOMMU API usage. We use the same domain to also map command
buffers that userspace can pass in.

The problem is that if I use dma_alloc_coherent(), then the memory will
already be mapped via the SMMU at that point and then the driver, not
knowing that memory has already been mapped, will attempt to map an IOVA
which will cause an SMMU fault when the host1x tries to access the
memory.

I didn't find an equivalent to arm_iommu_detach_device() for non-ARM,
but then stumbled across this and thought it was rather convenient for
these cases. If there's a better way to deal with this situation, I'd be
happy to do so.

Thierry


signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-02-05 Thread Michael S. Tsirkin
On Tue, Feb 05, 2019 at 08:24:07AM +0100, Christoph Hellwig wrote:
> On Mon, Feb 04, 2019 at 04:38:21PM -0500, Michael S. Tsirkin wrote:
> > It was designed to make, when set, as many guests as we can work
> > correctly, and it seems to be successful in doing exactly that.
> > 
> > Unfortunately there could be legacy guests that do work correctly but
> > become slow. Whether trying to somehow work around that
> > can paint us into a corner where things again don't
> > work for some people is a question worth discussing.
> 
> The other problem is that some qemu machines just throw passthrough
> devices and virtio devices on the same virtual PCI(e) bus, and have a
> common IOMMU setup for the whole bus / root port / domain.  I think
> this is completely bogus, but unfortunately it is out in the field.
> 
> Given that power is one of these examples I suspect that is what
> Thiago referes to.  But in this case the answer can't be that we
> pile on hack ontop of another, but instead introduce a new qemu
> machine that separates these clearly, and make that mandatory for
> the secure guest support.

That could we one approach, assuming one exists that guests
already support.

-- 
MST
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Christoph Hellwig
On Tue, Feb 05, 2019 at 12:06:02PM +0100, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Drivers that are built as modules may want to use these functions, so
> make them available like the rest of the functions.
> 
> Signed-off-by: Thierry Reding 

How do they want to use these functions?  The proper way is to
call dma_alloc_coherent / dma_alloc_free and let the DMA API
handle the rest.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 06/19] dma-iommu: fix and refactor iommu_dma_mmap

2019-02-05 Thread Robin Murphy

On 14/01/2019 09:41, Christoph Hellwig wrote:

The current iommu_dma_mmap code does not properly handle memory from the
page allocator that hasn't been remapped, which can happen in the rare
case of allocations for a coherent device that aren't allowed to block.

Fix this by replacing iommu_dma_mmap with a slightly tweaked copy of
dma_common_mmap with special handling for the remapped array of
pages allocated from __iommu_dma_alloc.


If there's an actual bugfix here, can we make that before all of the 
other code movement? If it's at all related to other reports of weird 
mmap behaviour it might warrant backporting, and either way I'm finding 
it needlessly tough to follow what's going on in this patch :(


Robin.


Signed-off-by: Christoph Hellwig 
---
  drivers/iommu/dma-iommu.c | 59 +++
  1 file changed, 23 insertions(+), 36 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index e0ffe22775ac..26f479d49103 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -592,23 +592,27 @@ static struct page **__iommu_dma_alloc(struct device 
*dev, size_t size,
  }
  
  /**

- * __iommu_dma_mmap - Map a buffer into provided user VMA
- * @pages: Array representing buffer from __iommu_dma_alloc()
+ * iommu_dma_mmap_remap - Map a remapped page array into provided user VMA
+ * @cpu_addr: virtual address of the memory to be remapped
   * @size: Size of buffer in bytes
   * @vma: VMA describing requested userspace mapping
   *
- * Maps the pages of the buffer in @pages into @vma. The caller is responsible
+ * Maps the pages pointed to by @cpu_addr into @vma. The caller is responsible
   * for verifying the correct size and protection of @vma beforehand.
   */
-static int __iommu_dma_mmap(struct page **pages, size_t size,
+static int iommu_dma_mmap_remap(void *cpu_addr, size_t size,
struct vm_area_struct *vma)
  {
+   struct vm_struct *area = find_vm_area(cpu_addr);
unsigned long uaddr = vma->vm_start;
unsigned int i, count = PAGE_ALIGN(size) >> PAGE_SHIFT;
int ret = -ENXIO;
  
+	if (WARN_ON(!area || !area->pages))

+   return -ENXIO;
+
for (i = vma->vm_pgoff; i < count && uaddr < vma->vm_end; i++) {
-   ret = vm_insert_page(vma, uaddr, pages[i]);
+   ret = vm_insert_page(vma, uaddr, area->pages[i]);
if (ret)
break;
uaddr += PAGE_SIZE;
@@ -1047,29 +1051,14 @@ static void iommu_dma_free(struct device *dev, size_t 
size, void *cpu_addr,
}
  }
  
-static int __iommu_dma_mmap_pfn(struct vm_area_struct *vma,

- unsigned long pfn, size_t size)
-{
-   int ret = -ENXIO;
-   unsigned long nr_vma_pages = vma_pages(vma);
-   unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
-   unsigned long off = vma->vm_pgoff;
-
-   if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
-   ret = remap_pfn_range(vma, vma->vm_start,
- pfn + off,
- vma->vm_end - vma->vm_start,
- vma->vm_page_prot);
-   }
-
-   return ret;
-}
-
  static int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
void *cpu_addr, dma_addr_t dma_addr, size_t size,
unsigned long attrs)
  {
-   struct vm_struct *area;
+   unsigned long user_count = vma_pages(vma);
+   unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
+   unsigned long off = vma->vm_pgoff;
+   unsigned long pfn;
int ret;
  
  	vma->vm_page_prot = arch_dma_mmap_pgprot(dev, vma->vm_page_prot, attrs);

@@ -1077,20 +1066,18 @@ static int iommu_dma_mmap(struct device *dev, struct 
vm_area_struct *vma,
if (dma_mmap_from_dev_coherent(dev, vma, cpu_addr, size, ))
return ret;
  
-	if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {

-   /*
-* DMA_ATTR_FORCE_CONTIGUOUS allocations are always remapped,
-* hence in the vmalloc space.
-*/
-   unsigned long pfn = vmalloc_to_pfn(cpu_addr);
-   return __iommu_dma_mmap_pfn(vma, pfn, size);
-   }
-
-   area = find_vm_area(cpu_addr);
-   if (WARN_ON(!area || !area->pages))
+   if (off >= count || user_count > count - off)
return -ENXIO;
  
-	return __iommu_dma_mmap(area->pages, size, vma);

+   if (is_vmalloc_addr(cpu_addr)) {
+   if (!(attrs & DMA_ATTR_FORCE_CONTIGUOUS))
+   return iommu_dma_mmap_remap(cpu_addr, size, vma);
+   pfn = vmalloc_to_pfn(cpu_addr);
+   } else
+   pfn = page_to_pfn(virt_to_page(cpu_addr));
+
+   return remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
+   user_count << PAGE_SHIFT, vma->vm_page_prot);
  }
  
  static int 

[PATCH] dma-direct: Export dma_direct_alloc() and dma_direct_free()

2019-02-05 Thread Thierry Reding
From: Thierry Reding 

Drivers that are built as modules may want to use these functions, so
make them available like the rest of the functions.

Signed-off-by: Thierry Reding 
---
 kernel/dma/direct.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 25bd19974223..f14b7ced4592 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -200,6 +200,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
return dma_direct_alloc_pages(dev, size, dma_handle, gfp, attrs);
 }
+EXPORT_SYMBOL(dma_direct_alloc);
 
 void dma_direct_free(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
@@ -209,6 +210,7 @@ void dma_direct_free(struct device *dev, size_t size,
else
dma_direct_free_pages(dev, size, cpu_addr, dma_addr, attrs);
 }
+EXPORT_SYMBOL(dma_direct_free);
 
 #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
 defined(CONFIG_SWIOTLB)
-- 
2.19.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu