Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1681321 > unrolled thread

[RFC PATCH 0/5] Fixes for loadable modules implementing DMA/IOMMU APIs

Started byTomasz Figa <tfiga@chromium.org>
First post2017-07-05 09:20 +0200
Last post2017-07-06 16:50 +0200
Articles 20 on this page of 29 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [RFC PATCH 0/5] Fixes for loadable modules implementing DMA/IOMMU APIs Tomasz Figa <tfiga@chromium.org> - 2017-07-05 09:20 +0200
    [RFC PATCH 5/5] iommu/dma: Add iommu_dma_cleanup() Tomasz Figa <tfiga@chromium.org> - 2017-07-05 09:20 +0200
    [RFC PATCH 2/5] base: dma-mapping: Provide a function to look up remapped pages Tomasz Figa <tfiga@chromium.org> - 2017-07-05 09:20 +0200
    [RFC PATCH 3/5] iommu: Export non-static functions to use in modules Tomasz Figa <tfiga@chromium.org> - 2017-07-05 09:20 +0200
    [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-05 09:20 +0200
      Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Christoph Hellwig <hch@lst.de> - 2017-07-05 17:20 +0200
        Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-05 17:30 +0200
          Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Christoph Hellwig <hch@lst.de> - 2017-07-05 19:30 +0200
            Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-06 03:50 +0200
              Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Arnd Bergmann <arnd@arndb.de> - 2017-07-06 10:30 +0200
                Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-06 10:40 +0200
                  Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-06 10:50 +0200
                    Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Arnd Bergmann <arnd@arndb.de> - 2017-07-06 14:30 +0200
                      Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-06 15:40 +0200
                        Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-06 15:50 +0200
                          Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-06 16:10 +0200
                            Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Arnd Bergmann <arnd@arndb.de> - 2017-07-06 16:30 +0200
                              Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa <tfiga@chromium.org> - 2017-07-06 16:40 +0200
                          Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Arnd Bergmann <arnd@arndb.de> - 2017-07-06 16:10 +0200
                        Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Arnd Bergmann <arnd@arndb.de> - 2017-07-06 16:00 +0200
    [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in modules Tomasz Figa <tfiga@chromium.org> - 2017-07-05 09:20 +0200
      Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in  modules Robin Murphy <robin.murphy@arm.com> - 2017-07-05 18:30 +0200
        Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in modules Tomasz Figa <tfiga@chromium.org> - 2017-07-06 04:30 +0200
          Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in  modules Robin Murphy <robin.murphy@arm.com> - 2017-07-06 13:20 +0200
            Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use         in modules Christoph Hellwig <hch@lst.de> - 2017-07-06 16:20 +0200
              Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in modules Tomasz Figa <tfiga@chromium.org> - 2017-07-06 16:20 +0200
                Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in modules Tomasz Figa <tfiga@chromium.org> - 2017-07-06 16:30 +0200
                  Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in modules Arnd Bergmann <arnd@arndb.de> - 2017-07-06 16:40 +0200
                    Re: [RFC PATCH 4/5] iommu/dma: Export non-static functions to use in modules Tomasz Figa <tfiga@chromium.org> - 2017-07-06 16:50 +0200

Page 1 of 2  [1] 2  Next page →


#1681321 — [RFC PATCH 0/5] Fixes for loadable modules implementing DMA/IOMMU APIs

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-05 09:20 +0200
Subject[RFC PATCH 0/5] Fixes for loadable modules implementing DMA/IOMMU APIs
Message-ID<tZMLn-3av-5@gated-at.bofh.it>
This series attempts to prepare the common DMA mapping helpers, IOMMU
subsystem and IOMMU DMA helpers to be used from within loadable modules.

It does not introduce any functional changes to the code itself. The
only things done are:
 - exporting related non-static functions,
 - adding a common DMA mapping helper that removes the need for the
   module to import find_vm_area() and rely on implementation detail
   of other DMA mapping helpers,
 - adding missing cleanup function in IOMMU DMA framework.

Obviously there is no mainline user that could benefit from the above,
as for now, but there is a work in progress on mainlining the Intel IPU3
camera subsystem driver, which requires its own DMA mapping code and
IOMMU driver. It can be found on ChromiumOS gerrit at

    https://chromium-review.googlesource.com/c/548626/4

or checked out directly with git with following commands

    git fetch https://chromium.googlesource.com/chromiumos/third_party/kernel refs/changes/26/548626/4
    git checkout FETCH_HEAD

The above is based on ChromeOS 4.4 kernel branch and has been used for
testing this series with code using it on real devices.

Tomasz Figa (5):
  base: dma-mapping: Export commonly used symbols
  base: dma-mapping: Provide a function to look up remapped pages
  iommu: Export non-static functions to use in modules
  iommu/dma: Export non-static functions to use in modules
  iommu/dma: Add iommu_dma_cleanup()

 drivers/base/dma-mapping.c  | 18 ++++++++++++++++++
 drivers/iommu/dma-iommu.c   | 19 +++++++++++++++++++
 drivers/iommu/iommu.c       | 12 ++++++++++++
 include/linux/dma-iommu.h   |  6 ++++++
 include/linux/dma-mapping.h |  2 ++
 5 files changed, 57 insertions(+)

-- 
2.13.2.725.g09c95d1e9-goog

[toc] | [next] | [standalone]


#1681324 — [RFC PATCH 5/5] iommu/dma: Add iommu_dma_cleanup()

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-05 09:20 +0200
Subject[RFC PATCH 5/5] iommu/dma: Add iommu_dma_cleanup()
Message-ID<tZMLn-3av-11@gated-at.bofh.it>
In reply to#1681321
In case of loadable modules using dma-iommu helpers, it makes sense to
drop the reference to the iova cache on module exit. Add a helper called
iommu_dma_cleanup() that undoes the effects of iommu_dma_init(), so that
modules can be unloaded cleanly.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/iommu/dma-iommu.c | 6 ++++++
 include/linux/dma-iommu.h | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 7cdeaf930106..51afa18a5de1 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -82,6 +82,12 @@ int iommu_dma_init(void)
 }
 EXPORT_SYMBOL(iommu_dma_init);
 
+void iommu_dma_cleanup(void)
+{
+	iova_cache_put();
+}
+EXPORT_SYMBOL(iommu_dma_cleanup);
+
 /**
  * iommu_get_dma_cookie - Acquire DMA-API resources for a domain
  * @domain: IOMMU domain to prepare for DMA-API usage
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 92f20832fd28..b12d9207a87a 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -24,7 +24,9 @@
 #include <linux/iommu.h>
 #include <linux/msi.h>
 
+/* Framework initialization - reference counted */
 int iommu_dma_init(void);
+void iommu_dma_cleanup(void);
 
 /* Domain management interface for IOMMU drivers */
 int iommu_get_dma_cookie(struct iommu_domain *domain);
@@ -85,6 +87,10 @@ static inline int iommu_dma_init(void)
 	return 0;
 }
 
+static inline void iommu_dma_cleanup(void)
+{
+}
+
 static inline int iommu_get_dma_cookie(struct iommu_domain *domain)
 {
 	return -ENODEV;
-- 
2.13.2.725.g09c95d1e9-goog

[toc] | [prev] | [next] | [standalone]


#1681327 — [RFC PATCH 2/5] base: dma-mapping: Provide a function to look up remapped pages

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-05 09:20 +0200
Subject[RFC PATCH 2/5] base: dma-mapping: Provide a function to look up remapped pages
Message-ID<tZMLo-3av-21@gated-at.bofh.it>
In reply to#1681321
DMA API implementations, which use the dma_common_*() helpers, typically
use them in pair with other helpers, such as iommu_dma_*(). For example,
a typical .free() callback needs to retrieve the pages remapped earlier
by dma_common_remap() and call iommu_dma_unmap() on them. Currently it
is done by calling find_vm_area() manually, however it relies on
implementation details of dma_common_remap() and is also difficult to
expose to loadable modules, due to find_vm_area() being quite a low
level function without its symbol exported.

Improve this by providing a function to look-up the pages previously
remapped. It hides implementation details, can do more sanity checks
than find_vm_area() and can be exported for use in loadable modules.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/base/dma-mapping.c  | 15 +++++++++++++++
 include/linux/dma-mapping.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 1fda8df3d849..9add50dd7a08 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -335,6 +335,21 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
 	vunmap(cpu_addr);
 }
 EXPORT_SYMBOL(dma_common_free_remap);
+
+struct page **dma_common_get_mapped_pages(void *cpu_addr,
+					  unsigned long vm_flags)
+{
+	struct vm_struct *area = find_vm_area(cpu_addr);
+
+	if (!area || (area->flags & vm_flags) != vm_flags) {
+		WARN(1, "trying to get pages for invalid coherent area: %p\n",
+		     cpu_addr);
+		return NULL;
+	}
+
+	return area->pages;
+}
+EXPORT_SYMBOL(dma_common_get_mapped_pages);
 #endif
 
 /*
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 843ab866e0f4..bd20435dac16 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -422,6 +422,8 @@ void *dma_common_pages_remap(struct page **pages, size_t size,
 			unsigned long vm_flags, pgprot_t prot,
 			const void *caller);
 void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags);
+struct page **dma_common_get_mapped_pages(void *cpu_addr,
+					  unsigned long vm_flags);
 
 /**
  * dma_mmap_attrs - map a coherent DMA allocation into user space
-- 
2.13.2.725.g09c95d1e9-goog

[toc] | [prev] | [next] | [standalone]


#1681330 — [RFC PATCH 3/5] iommu: Export non-static functions to use in modules

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-05 09:20 +0200
Subject[RFC PATCH 3/5] iommu: Export non-static functions to use in modules
Message-ID<tZMLp-3av-33@gated-at.bofh.it>
In reply to#1681321
There are some non-static functions potentially useful in IOMMU drivers
that do not have their symbols exported. Export them too, so that
loadable modules can benefit from them. Use EXPORT_SYMBOL_GPL() for
consistency with other exports in the file.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/iommu/iommu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3f6ea160afed..41dd6b435ae3 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -96,6 +96,7 @@ int iommu_device_register(struct iommu_device *iommu)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(iommu_device_register);
 
 void iommu_device_unregister(struct iommu_device *iommu)
 {
@@ -103,6 +104,7 @@ void iommu_device_unregister(struct iommu_device *iommu)
 	list_del(&iommu->list);
 	spin_unlock(&iommu_device_lock);
 }
+EXPORT_SYMBOL_GPL(iommu_device_unregister);
 
 static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
 						 unsigned type);
@@ -744,6 +746,7 @@ struct iommu_group *iommu_group_ref_get(struct iommu_group *group)
 	kobject_get(group->devices_kobj);
 	return group;
 }
+EXPORT_SYMBOL_GPL(iommu_group_ref_get);
 
 /**
  * iommu_group_put - Decrement group reference
@@ -917,6 +920,7 @@ struct iommu_group *generic_device_group(struct device *dev)
 {
 	return iommu_group_alloc();
 }
+EXPORT_SYMBOL_GPL(generic_device_group);
 
 /*
  * Use standard PCI bus topology, isolation features, and DMA alias quirks
@@ -984,6 +988,7 @@ struct iommu_group *pci_device_group(struct device *dev)
 	/* No shared group found, allocate new */
 	return iommu_group_alloc();
 }
+EXPORT_SYMBOL_GPL(pci_device_group);
 
 /**
  * iommu_group_get_for_dev - Find or create the IOMMU group for a device
@@ -1044,11 +1049,13 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
 
 	return group;
 }
+EXPORT_SYMBOL_GPL(iommu_group_get_for_dev);
 
 struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
 {
 	return group->default_domain;
 }
+EXPORT_SYMBOL_GPL(iommu_group_default_domain);
 
 static int add_iommu_group(struct device *dev, void *data)
 {
@@ -1795,6 +1802,7 @@ void iommu_get_resv_regions(struct device *dev, struct list_head *list)
 	if (ops && ops->get_resv_regions)
 		ops->get_resv_regions(dev, list);
 }
+EXPORT_SYMBOL_GPL(iommu_get_resv_regions);
 
 void iommu_put_resv_regions(struct device *dev, struct list_head *list)
 {
@@ -1803,6 +1811,7 @@ void iommu_put_resv_regions(struct device *dev, struct list_head *list)
 	if (ops && ops->put_resv_regions)
 		ops->put_resv_regions(dev, list);
 }
+EXPORT_SYMBOL_GPL(iommu_put_resv_regions);
 
 struct iommu_resv_region *iommu_alloc_resv_region(phys_addr_t start,
 						  size_t length, int prot,
@@ -1821,6 +1830,7 @@ struct iommu_resv_region *iommu_alloc_resv_region(phys_addr_t start,
 	region->type = type;
 	return region;
 }
+EXPORT_SYMBOL_GPL(iommu_alloc_resv_region);
 
 /* Request that a device is direct mapped by the IOMMU */
 int iommu_request_dm_for_dev(struct device *dev)
@@ -1874,6 +1884,7 @@ int iommu_request_dm_for_dev(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(iommu_request_dm_for_dev);
 
 const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 {
@@ -1889,6 +1900,7 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	spin_unlock(&iommu_device_lock);
 	return ops;
 }
+EXPORT_SYMBOL_GPL(iommu_ops_from_fwnode);
 
 int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
 		      const struct iommu_ops *ops)
-- 
2.13.2.725.g09c95d1e9-goog

[toc] | [prev] | [next] | [standalone]


#1681332 — [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-05 09:20 +0200
Subject[RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<tZMLp-3av-35@gated-at.bofh.it>
In reply to#1681321
There is nothing wrong in having a loadable module implementing DMA API,
for example to be used for sub-devices registered by the module.
However, most of the functions from dma-mapping do not have their
symbols exported, making it impossible to use them from loadable modules.

Export the remaining non-static functions in the file, so that loadable
modules can benefit from them. Use EXPORT_SYMBOL() for consistency with
other exports in the file.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/base/dma-mapping.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 5096755d185e..1fda8df3d849 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -287,6 +287,7 @@ void *dma_common_pages_remap(struct page **pages, size_t size,
 
 	return area->addr;
 }
+EXPORT_SYMBOL(dma_common_pages_remap);
 
 /*
  * remaps an allocated contiguous region into another vm_area.
@@ -316,6 +317,7 @@ void *dma_common_contiguous_remap(struct page *page, size_t size,
 		return NULL;
 	return area->addr;
 }
+EXPORT_SYMBOL(dma_common_contiguous_remap);
 
 /*
  * unmaps a range previously mapped by dma_common_*_remap
@@ -332,6 +334,7 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
 	unmap_kernel_range((unsigned long)cpu_addr, PAGE_ALIGN(size));
 	vunmap(cpu_addr);
 }
+EXPORT_SYMBOL(dma_common_free_remap);
 #endif
 
 /*
-- 
2.13.2.725.g09c95d1e9-goog

[toc] | [prev] | [next] | [standalone]


#1681577 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromChristoph Hellwig <hch@lst.de>
Date2017-07-05 17:20 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<tZUfT-7Zu-9@gated-at.bofh.it>
In reply to#1681332
Please use EXPORT_SYMBOL_GPL for any of these exports, as they are
internal linux implementration details by any definition of it.

On Wed, Jul 05, 2017 at 04:12:11PM +0900, Tomasz Figa wrote:
> There is nothing wrong in having a loadable module implementing DMA API,
> for example to be used for sub-devices registered by the module.
> However, most of the functions from dma-mapping do not have their
> symbols exported, making it impossible to use them from loadable modules.

I'd like to see the patches for this use case as well.  We don't
generally export symbols without seeing the users.

[toc] | [prev] | [next] | [standalone]


#1681593 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-05 17:30 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<tZUpA-83i-29@gated-at.bofh.it>
In reply to#1681577
Hi Christoph,

Thanks for comments!

On Thu, Jul 6, 2017 at 12:17 AM, Christoph Hellwig <hch@lst.de> wrote:
> Please use EXPORT_SYMBOL_GPL for any of these exports, as they are
> internal linux implementration details by any definition of it.

Right. I typically lean towards EXPORT_SYMBOL_GPL(), but was misled by
existing EXPORT_SYMBOL()s in the file. Will fix.

>
> On Wed, Jul 05, 2017 at 04:12:11PM +0900, Tomasz Figa wrote:
>> There is nothing wrong in having a loadable module implementing DMA API,
>> for example to be used for sub-devices registered by the module.
>> However, most of the functions from dma-mapping do not have their
>> symbols exported, making it impossible to use them from loadable modules.
>
> I'd like to see the patches for this use case as well.  We don't
> generally export symbols without seeing the users.

Generally the user is a work in progress that should be posted in a
very near future. You can find a reference to our downstream tree at
chromium.org in the cover letter. Obviously I don't mind including
patches from this series in the driver series later and that's one of
the reasons for this series being RFC.

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]


#1681675 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromChristoph Hellwig <hch@lst.de>
Date2017-07-05 19:30 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<tZWhJ-MR-35@gated-at.bofh.it>
In reply to#1681593
On Thu, Jul 06, 2017 at 12:22:35AM +0900, Tomasz Figa wrote:
> Generally the user is a work in progress that should be posted in a
> very near future. You can find a reference to our downstream tree at
> chromium.org in the cover letter. Obviously I don't mind including
> patches from this series in the driver series later and that's one of
> the reasons for this series being RFC.

Please post and explain them here.  In general I think moving dma
ops and iommu implementations into modules is a bad idea, but I
don't want to reject the idea before seeing the code.  Or maybe
by looking at the user we can come up with an even better idea
to solve the original issue you're trying to solve, so please also
explain your rationale.

[toc] | [prev] | [next] | [standalone]


#1682003 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-06 03:50 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u045A-5Wp-27@gated-at.bofh.it>
In reply to#1681675
On Thu, Jul 6, 2017 at 2:20 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Jul 06, 2017 at 12:22:35AM +0900, Tomasz Figa wrote:
>> Generally the user is a work in progress that should be posted in a
>> very near future. You can find a reference to our downstream tree at
>> chromium.org in the cover letter. Obviously I don't mind including
>> patches from this series in the driver series later and that's one of
>> the reasons for this series being RFC.
>
> Please post and explain them here.

That's the intention. I will talk with people to try to get it posted earlier.

You can also refer to the old series, which we are reworking to
address review comments:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg113505.html

The old code essentially reimplements all the DMA mapping operations
on its own, without using the generic helpers (and not even following
the API properly, see my review comments to the original patch 2 and
3), which IMHO is really ugly (but avoids exporting symbols of the
helpers, which I assumed to be a no-problem).

> In general I think moving dma
> ops and iommu implementations into modules is a bad idea

Could you elaborate on this? I'd be interested in seeing the reasoning
behind this.

> but I
> don't want to reject the idea before seeing the code.  Or maybe
> by looking at the user we can come up with an even better idea
> to solve the original issue you're trying to solve, so please also
> explain your rationale.

Basically we have an x86 platform with a camera subsystem that is a
PCI device, has its own MMU and needs cache maintenance. Moreover, the
V4L2 subsystem, which is the right place for camera drivers, heavily
relies on DMA mapping as a way to abstract memory allocation, mapping
and cache maintenance. So it feels natural to me to hide the hardware
details (additional cache maintenance, mapping into the built-in
IOMMU) in the DMA mapping ops for this camera subsystem and simply
make V4L2 just work without knowing those details.

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]


#1682176 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromArnd Bergmann <arnd@arndb.de>
Date2017-07-06 10:30 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0akI-1Ps-39@gated-at.bofh.it>
In reply to#1682003
On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 2:20 AM, Christoph Hellwig <hch@lst.de> wrote:
>> On Thu, Jul 06, 2017 at 12:22:35AM +0900, Tomasz Figa wrote:

>> In general I think moving dma
>> ops and iommu implementations into modules is a bad idea
>
> Could you elaborate on this? I'd be interested in seeing the reasoning
> behind this.
>
>> but I
>> don't want to reject the idea before seeing the code.  Or maybe
>> by looking at the user we can come up with an even better idea
>> to solve the original issue you're trying to solve, so please also
>> explain your rationale.

I had pretty much the same thoughts here.

> Basically we have an x86 platform with a camera subsystem that is a
> PCI device, has its own MMU and needs cache maintenance. Moreover, the
> V4L2 subsystem, which is the right place for camera drivers, heavily
> relies on DMA mapping as a way to abstract memory allocation, mapping
> and cache maintenance. So it feels natural to me to hide the hardware
> details (additional cache maintenance, mapping into the built-in
> IOMMU) in the DMA mapping ops for this camera subsystem and simply
> make V4L2 just work without knowing those details.

I can understand your reasoning here, but I'm also not convinced
that this is the best approach. There may be a middle ground somewhere
though.

Generally speaking I don't want to have to deal with the horrors of
deciding whether an IOMMU is going to be there eventually or not
at probe() time. At some point, we had decided that IOMMUs need to
be initialized (almost) as early as irqchips and clocksources so we can
rely on them to be there at device discovery time. That got pushed
back already, and now we may have to deal with -EPROBE_DEFER
when an IOMMU has not been fully initialized at device probe time,
but at least we can reliably see if one is there or not. Making IOMMUs
modular will add further uncertainty here. Obviously we cannot attach
an IOMMU to a device once we have started using DMA mapping
calls on it.

For your particular use case, I would instead leave the knowledge
about the IOMMU in the driver itself, like we do for the IOMMUs
that are integrated in desktop GPUs, and have the code use the
DMA mapping API with the system-provided dma_map_ops to
get dma_addr_t tokens which you then program into the device
IOMMU.

An open question however would be whether to use the IOMMU
API without the DMA mapping API here, or whether to completely
leave the knowledge of the IOMMU inside of the driver itself.
I don't have a strong opinion on that part, and I guess this mostly
depends on what the hardware looks like.

       Arnd

[toc] | [prev] | [next] | [standalone]


#1682189 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-06 10:40 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0aun-1Sl-33@gated-at.bofh.it>
In reply to#1682176
On Thu, Jul 6, 2017 at 5:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 2:20 AM, Christoph Hellwig <hch@lst.de> wrote:
>>> On Thu, Jul 06, 2017 at 12:22:35AM +0900, Tomasz Figa wrote:
>
>>> In general I think moving dma
>>> ops and iommu implementations into modules is a bad idea
>>
>> Could you elaborate on this? I'd be interested in seeing the reasoning
>> behind this.
>>
>>> but I
>>> don't want to reject the idea before seeing the code.  Or maybe
>>> by looking at the user we can come up with an even better idea
>>> to solve the original issue you're trying to solve, so please also
>>> explain your rationale.
>
> I had pretty much the same thoughts here.
>
>> Basically we have an x86 platform with a camera subsystem that is a
>> PCI device, has its own MMU and needs cache maintenance. Moreover, the
>> V4L2 subsystem, which is the right place for camera drivers, heavily
>> relies on DMA mapping as a way to abstract memory allocation, mapping
>> and cache maintenance. So it feels natural to me to hide the hardware
>> details (additional cache maintenance, mapping into the built-in
>> IOMMU) in the DMA mapping ops for this camera subsystem and simply
>> make V4L2 just work without knowing those details.
>
> I can understand your reasoning here, but I'm also not convinced
> that this is the best approach. There may be a middle ground somewhere
> though.
>
> Generally speaking I don't want to have to deal with the horrors of
> deciding whether an IOMMU is going to be there eventually or not
> at probe() time. At some point, we had decided that IOMMUs need to
> be initialized (almost) as early as irqchips and clocksources so we can
> rely on them to be there at device discovery time. That got pushed
> back already, and now we may have to deal with -EPROBE_DEFER
> when an IOMMU has not been fully initialized at device probe time,
> but at least we can reliably see if one is there or not. Making IOMMUs
> modular will add further uncertainty here. Obviously we cannot attach
> an IOMMU to a device once we have started using DMA mapping
> calls on it.

The hardware can only work with IOMMU and so the main module is highly
tied with the IOMMU module and it initialized it directly. There is no
separate struct driver or device associated with the IOMMU, as it's a
part of the one and only one PCI device (as visible from the system
PCI bus point of view) and technically handled by one pci_driver.

>
> For your particular use case, I would instead leave the knowledge
> about the IOMMU in the driver itself, like we do for the IOMMUs
> that are integrated in desktop GPUs, and have the code use the
> DMA mapping API with the system-provided dma_map_ops to
> get dma_addr_t tokens which you then program into the device
> IOMMU.
>
> An open question however would be whether to use the IOMMU
> API without the DMA mapping API here, or whether to completely
> leave the knowledge of the IOMMU inside of the driver itself.
> I don't have a strong opinion on that part, and I guess this mostly
> depends on what the hardware looks like.

+ linux-media and some media folks

I'd say that this is something that has been consistently tried to be
avoided by V4L2 and that's why it's so tightly integrated with DMA
mapping. IMHO re-implementing the code that's already there in
videobuf2 again in the driver, only because, for no good reason
mentioned as for now, having a loadable module providing DMA ops was
disliked.

Similarly with IOMMU API. It provides a lot of help in managing the
mappings and re-implementing this would be IMHO backwards.

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]


#1682198 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-06 10:50 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0aE2-1W0-15@gated-at.bofh.it>
In reply to#1682189
On Thu, Jul 6, 2017 at 5:34 PM, Tomasz Figa <tfiga@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 5:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga@chromium.org> wrote:
>>> On Thu, Jul 6, 2017 at 2:20 AM, Christoph Hellwig <hch@lst.de> wrote:
>>>> On Thu, Jul 06, 2017 at 12:22:35AM +0900, Tomasz Figa wrote:
>>
>>>> In general I think moving dma
>>>> ops and iommu implementations into modules is a bad idea
>>>
>>> Could you elaborate on this? I'd be interested in seeing the reasoning
>>> behind this.
>>>
>>>> but I
>>>> don't want to reject the idea before seeing the code.  Or maybe
>>>> by looking at the user we can come up with an even better idea
>>>> to solve the original issue you're trying to solve, so please also
>>>> explain your rationale.
>>
>> I had pretty much the same thoughts here.
>>
>>> Basically we have an x86 platform with a camera subsystem that is a
>>> PCI device, has its own MMU and needs cache maintenance. Moreover, the
>>> V4L2 subsystem, which is the right place for camera drivers, heavily
>>> relies on DMA mapping as a way to abstract memory allocation, mapping
>>> and cache maintenance. So it feels natural to me to hide the hardware
>>> details (additional cache maintenance, mapping into the built-in
>>> IOMMU) in the DMA mapping ops for this camera subsystem and simply
>>> make V4L2 just work without knowing those details.
>>
>> I can understand your reasoning here, but I'm also not convinced
>> that this is the best approach. There may be a middle ground somewhere
>> though.
>>
>> Generally speaking I don't want to have to deal with the horrors of
>> deciding whether an IOMMU is going to be there eventually or not
>> at probe() time. At some point, we had decided that IOMMUs need to
>> be initialized (almost) as early as irqchips and clocksources so we can
>> rely on them to be there at device discovery time. That got pushed
>> back already, and now we may have to deal with -EPROBE_DEFER
>> when an IOMMU has not been fully initialized at device probe time,
>> but at least we can reliably see if one is there or not. Making IOMMUs
>> modular will add further uncertainty here. Obviously we cannot attach
>> an IOMMU to a device once we have started using DMA mapping
>> calls on it.
>
> The hardware can only work with IOMMU and so the main module is highly
> tied with the IOMMU module and it initialized it directly. There is no
> separate struct driver or device associated with the IOMMU, as it's a
> part of the one and only one PCI device (as visible from the system
> PCI bus point of view) and technically handled by one pci_driver.
>
>>
>> For your particular use case, I would instead leave the knowledge
>> about the IOMMU in the driver itself, like we do for the IOMMUs
>> that are integrated in desktop GPUs, and have the code use the
>> DMA mapping API with the system-provided dma_map_ops to
>> get dma_addr_t tokens which you then program into the device
>> IOMMU.
>>
>> An open question however would be whether to use the IOMMU
>> API without the DMA mapping API here, or whether to completely
>> leave the knowledge of the IOMMU inside of the driver itself.
>> I don't have a strong opinion on that part, and I guess this mostly
>> depends on what the hardware looks like.
>
> + linux-media and some media folks
>
> I'd say that this is something that has been consistently tried to be
> avoided by V4L2 and that's why it's so tightly integrated with DMA
> mapping. IMHO re-implementing the code that's already there in
> videobuf2 again in the driver, only because, for no good reason
> mentioned as for now, having a loadable module providing DMA ops was
> disliked.

Sorry, I intended to mean:

IMHO re-implementing the code that's already there in videobuf2 again
in the driver, only because, for no good reason mentioned as for now,
having a loadable module providing DMA ops was disliked, would make no
sense.

>
> Similarly with IOMMU API. It provides a lot of help in managing the
> mappings and re-implementing this would be IMHO backwards.
>
> Best regards,
> Tomasz

[toc] | [prev] | [next] | [standalone]


#1682394 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromArnd Bergmann <arnd@arndb.de>
Date2017-07-06 14:30 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0e4X-4gf-43@gated-at.bofh.it>
In reply to#1682198
On Thu, Jul 6, 2017 at 10:36 AM, Tomasz Figa <tfiga@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 5:34 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 5:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga@chromium.org> wrote:

>>
>> I'd say that this is something that has been consistently tried to be
>> avoided by V4L2 and that's why it's so tightly integrated with DMA
>> mapping. IMHO re-implementing the code that's already there in
>> videobuf2 again in the driver, only because, for no good reason
>> mentioned as for now, having a loadable module providing DMA ops was
>> disliked.
>
> Sorry, I intended to mean:
>
> IMHO re-implementing the code that's already there in videobuf2 again
> in the driver, only because, for no good reason mentioned as for now,
> having a loadable module providing DMA ops was disliked, would make no
> sense.

Why would we need to duplicate that code? I would expect that the videobuf2
core can simply call the regular dma_mapping interfaces, and you handle the
IOPTE generation at the point when the buffer is handed off from the core
code to the device driver. Am I missing something?

       Arnd

[toc] | [prev] | [next] | [standalone]


#1682441 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-06 15:40 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0faG-4Uc-19@gated-at.bofh.it>
In reply to#1682394
On Thu, Jul 6, 2017 at 9:23 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Jul 6, 2017 at 10:36 AM, Tomasz Figa <tfiga@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 5:34 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>>> On Thu, Jul 6, 2017 at 5:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga@chromium.org> wrote:
>
>>>
>>> I'd say that this is something that has been consistently tried to be
>>> avoided by V4L2 and that's why it's so tightly integrated with DMA
>>> mapping. IMHO re-implementing the code that's already there in
>>> videobuf2 again in the driver, only because, for no good reason
>>> mentioned as for now, having a loadable module providing DMA ops was
>>> disliked.
>>
>> Sorry, I intended to mean:
>>
>> IMHO re-implementing the code that's already there in videobuf2 again
>> in the driver, only because, for no good reason mentioned as for now,
>> having a loadable module providing DMA ops was disliked, would make no
>> sense.
>
> Why would we need to duplicate that code? I would expect that the videobuf2
> core can simply call the regular dma_mapping interfaces, and you handle the
> IOPTE generation at the point when the buffer is handed off from the core
> code to the device driver. Am I missing something?

Well, for example, the iommu-dma helpers already implement all the
IOVA management, SG iterations, IOMMU API calls, sanity checks and so
on. There is a significant amount of common code.

On the other hand, if it's strictly about base/dma-mapping, we might
not need it indeed. The driver could call iommu-dma helpers directly,
without the need to provide its own DMA ops. One caveat, though, we
are not able to obtain coherent (i.e. uncached) memory with this
approach, which might have some performance effects and complicates
the code, that would now need to flush caches even for some small
internal buffers.

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]


#1682443 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-06 15:50 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0fkl-4Ys-3@gated-at.bofh.it>
In reply to#1682441
On Thu, Jul 6, 2017 at 10:31 PM, Tomasz Figa <tfiga@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 9:23 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thu, Jul 6, 2017 at 10:36 AM, Tomasz Figa <tfiga@chromium.org> wrote:
>>> On Thu, Jul 6, 2017 at 5:34 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>>>> On Thu, Jul 6, 2017 at 5:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>>> On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga@chromium.org> wrote:
>>
>>>>
>>>> I'd say that this is something that has been consistently tried to be
>>>> avoided by V4L2 and that's why it's so tightly integrated with DMA
>>>> mapping. IMHO re-implementing the code that's already there in
>>>> videobuf2 again in the driver, only because, for no good reason
>>>> mentioned as for now, having a loadable module providing DMA ops was
>>>> disliked.
>>>
>>> Sorry, I intended to mean:
>>>
>>> IMHO re-implementing the code that's already there in videobuf2 again
>>> in the driver, only because, for no good reason mentioned as for now,
>>> having a loadable module providing DMA ops was disliked, would make no
>>> sense.
>>
>> Why would we need to duplicate that code? I would expect that the videobuf2
>> core can simply call the regular dma_mapping interfaces, and you handle the
>> IOPTE generation at the point when the buffer is handed off from the core
>> code to the device driver. Am I missing something?
>
> Well, for example, the iommu-dma helpers already implement all the
> IOVA management, SG iterations, IOMMU API calls, sanity checks and so
> on. There is a significant amount of common code.
>
> On the other hand, if it's strictly about base/dma-mapping, we might
> not need it indeed. The driver could call iommu-dma helpers directly,
> without the need to provide its own DMA ops. One caveat, though, we
> are not able to obtain coherent (i.e. uncached) memory with this
> approach, which might have some performance effects and complicates
> the code, that would now need to flush caches even for some small
> internal buffers.

I think I should add a bit of explanation here:
 1) the device is non-coherent with CPU caches, even on x86,
 2) it looks like x86 does not have non-coherent DMA ops, (but it
might be something that could be fixed)
 3) one technically could still use __get_vm_area() and map_vm_area(),
which _are_ exported, to create an uncached mapping. I'll leave it to
you to judge if it would be better than using the already available
generic helpers.

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]


#1682456 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-06 16:10 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0fDH-5sz-7@gated-at.bofh.it>
In reply to#1682443
On Thu, Jul 6, 2017 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Jul 6, 2017 at 3:49 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 10:31 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>
>>> On the other hand, if it's strictly about base/dma-mapping, we might
>>> not need it indeed. The driver could call iommu-dma helpers directly,
>>> without the need to provide its own DMA ops. One caveat, though, we
>>> are not able to obtain coherent (i.e. uncached) memory with this
>>> approach, which might have some performance effects and complicates
>>> the code, that would now need to flush caches even for some small
>>> internal buffers.
>>
>> I think I should add a bit of explanation here:
>>  1) the device is non-coherent with CPU caches, even on x86,
>>  2) it looks like x86 does not have non-coherent DMA ops, (but it
>> might be something that could be fixed)
>
> I don't understand what this means here. The PCI on x86 is always
> cache-coherent, so why is the device not?
>
> Do you mean that the device has its own caches that may need
> flushing to make the device cache coherent with the CPU cache,
> rather than flushing the CPU caches?

Sakari might be able to explain this with more technical details, but
generally the device is not a standard PCI device one might find on
existing x86 systems.

It is some kind of embedded subsystem that behaves mostly like a PCI
device, with certain exceptions, one being the lack of coherency with
CPU caches, at least for certain parts of the subsystem. The reference
vendor code disables the coherency completely, for reasons not known
to me, but AFAICT this is the preferred operating mode, possibly due
to performance effects (this is a memory-heavy image processing
subsystem).

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]


#1682472 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromArnd Bergmann <arnd@arndb.de>
Date2017-07-06 16:30 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0fX5-5RS-21@gated-at.bofh.it>
In reply to#1682456
On Thu, Jul 6, 2017 at 4:06 PM, Tomasz Figa <tfiga@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thu, Jul 6, 2017 at 3:49 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>>> On Thu, Jul 6, 2017 at 10:31 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>>
>>>> On the other hand, if it's strictly about base/dma-mapping, we might
>>>> not need it indeed. The driver could call iommu-dma helpers directly,
>>>> without the need to provide its own DMA ops. One caveat, though, we
>>>> are not able to obtain coherent (i.e. uncached) memory with this
>>>> approach, which might have some performance effects and complicates
>>>> the code, that would now need to flush caches even for some small
>>>> internal buffers.
>>>
>>> I think I should add a bit of explanation here:
>>>  1) the device is non-coherent with CPU caches, even on x86,
>>>  2) it looks like x86 does not have non-coherent DMA ops, (but it
>>> might be something that could be fixed)
>>
>> I don't understand what this means here. The PCI on x86 is always
>> cache-coherent, so why is the device not?
>>
>> Do you mean that the device has its own caches that may need
>> flushing to make the device cache coherent with the CPU cache,
>> rather than flushing the CPU caches?
>
> Sakari might be able to explain this with more technical details, but
> generally the device is not a standard PCI device one might find on
> existing x86 systems.
>
> It is some kind of embedded subsystem that behaves mostly like a PCI
> device, with certain exceptions, one being the lack of coherency with
> CPU caches, at least for certain parts of the subsystem. The reference
> vendor code disables the coherency completely, for reasons not known
> to me, but AFAICT this is the preferred operating mode, possibly due
> to performance effects (this is a memory-heavy image processing

Ok, got it. I think something similar happens on integrated GPUs for
a certain CPU family. The DRM code has its own ways of dealing with
this kind of device. If you find that the hardware to be closely
related (either the implementation, or the location on the internal
buses) to the GPU on this machine, I'd recommend having a look
in drivers/gpu/drm to see how it's handled there, and if that code could
be shared.

        Arnd

[toc] | [prev] | [next] | [standalone]


#1682481 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromTomasz Figa <tfiga@chromium.org>
Date2017-07-06 16:40 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0g6L-60R-29@gated-at.bofh.it>
In reply to#1682472
On Thu, Jul 6, 2017 at 11:27 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Jul 6, 2017 at 4:06 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Thu, Jul 6, 2017 at 3:49 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>>>> On Thu, Jul 6, 2017 at 10:31 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>>>
>>>>> On the other hand, if it's strictly about base/dma-mapping, we might
>>>>> not need it indeed. The driver could call iommu-dma helpers directly,
>>>>> without the need to provide its own DMA ops. One caveat, though, we
>>>>> are not able to obtain coherent (i.e. uncached) memory with this
>>>>> approach, which might have some performance effects and complicates
>>>>> the code, that would now need to flush caches even for some small
>>>>> internal buffers.
>>>>
>>>> I think I should add a bit of explanation here:
>>>>  1) the device is non-coherent with CPU caches, even on x86,
>>>>  2) it looks like x86 does not have non-coherent DMA ops, (but it
>>>> might be something that could be fixed)
>>>
>>> I don't understand what this means here. The PCI on x86 is always
>>> cache-coherent, so why is the device not?
>>>
>>> Do you mean that the device has its own caches that may need
>>> flushing to make the device cache coherent with the CPU cache,
>>> rather than flushing the CPU caches?
>>
>> Sakari might be able to explain this with more technical details, but
>> generally the device is not a standard PCI device one might find on
>> existing x86 systems.
>>
>> It is some kind of embedded subsystem that behaves mostly like a PCI
>> device, with certain exceptions, one being the lack of coherency with
>> CPU caches, at least for certain parts of the subsystem. The reference
>> vendor code disables the coherency completely, for reasons not known
>> to me, but AFAICT this is the preferred operating mode, possibly due
>> to performance effects (this is a memory-heavy image processing
>
> Ok, got it. I think something similar happens on integrated GPUs for
> a certain CPU family. The DRM code has its own ways of dealing with
> this kind of device. If you find that the hardware to be closely
> related (either the implementation, or the location on the internal
> buses) to the GPU on this machine, I'd recommend having a look
> in drivers/gpu/drm to see how it's handled there, and if that code could
> be shared.

I think it's not closely related, but might be a very similar case.

Still, DRM is very liberal in terms of not using common code for doing
things, while V4L2 tries to makes things generic as much as possible.
There is already the vb2_dma_contig backend, which allocates coherent
memory (in case of V4L2-allocated buffers), manages caches (in case of
userptr or DMA-buf buffers) and so on for you. If we can't have the
DMA ops do the right thing, the code there is essentially useless and
you are left with vb2_dma_sg that uses a page allocator and gives the
driver sg tables (it actually can also do cache management for you,
but since dma_sync_sg_*() is essentially a no-op on x86, the driver
would have to do it on its own).

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]


#1682460 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromArnd Bergmann <arnd@arndb.de>
Date2017-07-06 16:10 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0fDH-5sz-9@gated-at.bofh.it>
In reply to#1682443
On Thu, Jul 6, 2017 at 3:49 PM, Tomasz Figa <tfiga@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 10:31 PM, Tomasz Figa <tfiga@chromium.org> wrote:

>> On the other hand, if it's strictly about base/dma-mapping, we might
>> not need it indeed. The driver could call iommu-dma helpers directly,
>> without the need to provide its own DMA ops. One caveat, though, we
>> are not able to obtain coherent (i.e. uncached) memory with this
>> approach, which might have some performance effects and complicates
>> the code, that would now need to flush caches even for some small
>> internal buffers.
>
> I think I should add a bit of explanation here:
>  1) the device is non-coherent with CPU caches, even on x86,
>  2) it looks like x86 does not have non-coherent DMA ops, (but it
> might be something that could be fixed)

I don't understand what this means here. The PCI on x86 is always
cache-coherent, so why is the device not?

Do you mean that the device has its own caches that may need
flushing to make the device cache coherent with the CPU cache,
rather than flushing the CPU caches?

          Arnd

[toc] | [prev] | [next] | [standalone]


#1682452 — Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols

FromArnd Bergmann <arnd@arndb.de>
Date2017-07-06 16:00 +0200
SubjectRe: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols
Message-ID<u0fu2-551-11@gated-at.bofh.it>
In reply to#1682441
On Thu, Jul 6, 2017 at 3:31 PM, Tomasz Figa <tfiga@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 9:23 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thu, Jul 6, 2017 at 10:36 AM, Tomasz Figa <tfiga@chromium.org> wrote:
>>> On Thu, Jul 6, 2017 at 5:34 PM, Tomasz Figa <tfiga@chromium.org> wrote:
>>>
>>> Sorry, I intended to mean:
>>>
>>> IMHO re-implementing the code that's already there in videobuf2 again
>>> in the driver, only because, for no good reason mentioned as for now,
>>> having a loadable module providing DMA ops was disliked, would make no
>>> sense.
>>
>> Why would we need to duplicate that code? I would expect that the videobuf2
>> core can simply call the regular dma_mapping interfaces, and you handle the
>> IOPTE generation at the point when the buffer is handed off from the core
>> code to the device driver. Am I missing something?
>
> Well, for example, the iommu-dma helpers already implement all the
> IOVA management, SG iterations, IOMMU API calls, sanity checks and so
> on. There is a significant amount of common code.
>
> On the other hand, if it's strictly about base/dma-mapping, we might
> not need it indeed. The driver could call iommu-dma helpers directly,
> without the need to provide its own DMA ops.

Yes, that's what I meant: if using the IOMMU interface helps, I don't
see anything wrong with that, but using the iommu based
dma_map_ops seems like it may introduce more problems than
it solves.

        Arnd

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web