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

Groups > linux.kernel > #1382937 > unrolled thread

[RFC/PATCHv2 v2 2/4] dma-mapping: Add dma_remap() APIs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC/PATCHv2 v2 2/4] dma-mapping: Add dma_remap() APIs Stephen Boyd <stephen.boyd@linaro.org> - 2016-04-20 03:10 +0200
    Re: [RFC/PATCHv2 v2 2/4] dma-mapping: Add dma_remap() APIs Catalin Marinas <catalin.marinas@arm.com> - 2016-04-21 12:40 +0200
      Re: [RFC/PATCHv2 v2 2/4] dma-mapping: Add dma_remap() APIs Catalin Marinas <catalin.marinas@arm.com> - 2016-04-27 17:30 +0200
        Re: [RFC/PATCHv2 v2 2/4] dma-mapping: Add dma_remap() APIs Laura Abbott <labbott@redhat.com> - 2016-04-27 20:20 +0200

#1382937 — [RFC/PATCHv2 v2 2/4] dma-mapping: Add dma_remap() APIs

From: Laura Abbott <lauraa@codeaurora.org>

Some systems are memory constrained but they need to load very
large firmwares. The firmware subsystem allows drivers to request
this firmware be loaded from the filesystem, but this requires
that the entire firmware be loaded into kernel memory first
before it's provided to the driver. This can lead to a situation
where we map the firmware twice, once to load the firmware into
kernel memory and once to copy the firmware into the final
resting place.

This design creates needless memory pressure and delays loading
because we have to copy from kernel memory to somewhere else.
Let's add a couple DMA APIs that allow us to map DMA buffers into
the CPU's address space in arbitrary sizes. With this API, we can
allocate a DMA buffer with DMA_ATTR_NO_KERNEL_MAPPING and move a
small mapping window across our large DMA buffer to load the
firmware directly into buffer.

Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
[stephen.boyd@linaro.org: Add dma_attrs and offset to API, use
dma_common_contiguous_remap() instead of ioremap_page_range(),
support dma_remap() even when DMA_ATTR_NO_KERNEL_MAPPING isn't
specified, rewrite commit text]
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 arch/arm64/mm/dma-mapping.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/dma-mapping.h | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 9686e722a047..c5816cfc155f 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -345,6 +345,43 @@ static int __swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
 	return ret;
 }
 
+static void *arm64_dma_remap(struct device *dev, void *cpu_addr,
+			     dma_addr_t handle, size_t size,
+			     unsigned long offset, struct dma_attrs *attrs)
+{
+	struct page *page = phys_to_page(dma_to_phys(dev, handle) + offset);
+	bool coherent = is_device_dma_coherent(dev);
+	pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
+	void *ptr;
+
+	if (dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs)) {
+		offset &= ~PAGE_MASK;
+		size = PAGE_ALIGN(size + offset);
+
+		ptr = dma_common_contiguous_remap(page, size, VM_USERMAP, prot,
+						  NULL);
+	} else {
+		ptr = cpu_addr;
+	}
+	if (!ptr)
+		return NULL;
+
+	return ptr + offset;
+}
+
+static void arm64_dma_unremap(struct device *dev, void *cpu_addr,
+			      size_t size, unsigned long offset,
+			      struct dma_attrs *attrs)
+{
+	if (!dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs))
+		return;
+
+	offset &= ~PAGE_MASK;
+	cpu_addr -= offset;
+
+	vunmap(cpu_addr);
+}
+
 static struct dma_map_ops swiotlb_dma_ops = {
 	.alloc = __dma_alloc,
 	.free = __dma_free,
@@ -360,6 +397,8 @@ static struct dma_map_ops swiotlb_dma_ops = {
 	.sync_sg_for_device = __swiotlb_sync_sg_for_device,
 	.dma_supported = swiotlb_dma_supported,
 	.mapping_error = swiotlb_dma_mapping_error,
+	.remap = arm64_dma_remap,
+	.unremap = arm64_dma_unremap,
 };
 
 static int __init atomic_pool_init(void)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 9ea9aba28049..737c38a6151d 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -64,6 +64,12 @@ struct dma_map_ops {
 	int (*mapping_error)(struct device *dev, dma_addr_t dma_addr);
 	int (*dma_supported)(struct device *dev, u64 mask);
 	int (*set_dma_mask)(struct device *dev, u64 mask);
+	void *(*remap)(struct device *dev, void *cpu_addr, dma_addr_t handle,
+			size_t size, unsigned long offset,
+			struct dma_attrs *attrs);
+	void (*unremap)(struct device *dev, void *cpu_addr,
+			size_t size, unsigned long offset,
+			struct dma_attrs *attrs);
 #ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
 	u64 (*get_required_mask)(struct device *dev);
 #endif
@@ -467,6 +473,35 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
 }
 #endif
 
+static inline void *dma_remap(struct device *dev, void *cpu_addr,
+		dma_addr_t dma_handle, size_t size, unsigned long offset,
+		struct dma_attrs *attrs)
+{
+	const struct dma_map_ops *ops = get_dma_ops(dev);
+
+	if (!ops)
+		return NULL;
+	if (!ops->remap)
+		return NULL;
+
+	return ops->remap(dev, cpu_addr, dma_handle, size, offset, attrs);
+}
+
+
+static inline void dma_unremap(struct device *dev, void *remapped_addr,
+				size_t size, unsigned long offset,
+				struct dma_attrs *attrs)
+{
+	const struct dma_map_ops *ops = get_dma_ops(dev);
+
+	if (!ops)
+		return;
+	if (!ops->unremap)
+		return;
+
+	return ops->unremap(dev, remapped_addr, size, offset, attrs);
+}
+
 static inline u64 dma_get_mask(struct device *dev)
 {
 	if (dev && dev->dma_mask && *dev->dma_mask)
-- 
2.8.0.rc4

[toc] | [next] | [standalone]

#1384046

Hi Stephen,

On Tue, Apr 19, 2016 at 06:04:27PM -0700, Stephen Boyd wrote:
> From: Laura Abbott <lauraa@codeaurora.org>
> 
> Some systems are memory constrained but they need to load very
> large firmwares. The firmware subsystem allows drivers to request
> this firmware be loaded from the filesystem, but this requires
> that the entire firmware be loaded into kernel memory first
> before it's provided to the driver. This can lead to a situation
> where we map the firmware twice, once to load the firmware into
> kernel memory and once to copy the firmware into the final
> resting place.
> 
> This design creates needless memory pressure and delays loading
> because we have to copy from kernel memory to somewhere else.
> Let's add a couple DMA APIs that allow us to map DMA buffers into
> the CPU's address space in arbitrary sizes. With this API, we can
> allocate a DMA buffer with DMA_ATTR_NO_KERNEL_MAPPING and move a
> small mapping window across our large DMA buffer to load the
> firmware directly into buffer.

The first two patches in this series don't make sense to me. I don't
understand what the memory pressure is: physical or virtual? Because
they don't seem to address the former (the DMA buffer is allocated in
full) while the latter doesn't need any addressing at all on arm64, we
have plenty of VA space.

Why do you even need the coherent DMA API? Can you use the streaming API
(map_sg etc.) with a separately allocated buffer?

-- 
Catalin

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

#1389227

On Fri, Apr 22, 2016 at 05:35:16PM -0700, Stephen Boyd wrote:
> Quoting Catalin Marinas (2016-04-21 03:35:12)
> > On Tue, Apr 19, 2016 at 06:04:27PM -0700, Stephen Boyd wrote:
> > > From: Laura Abbott <lauraa@codeaurora.org>
> > > 
> > > Some systems are memory constrained but they need to load very
> > > large firmwares. The firmware subsystem allows drivers to request
> > > this firmware be loaded from the filesystem, but this requires
> > > that the entire firmware be loaded into kernel memory first
> > > before it's provided to the driver. This can lead to a situation
> > > where we map the firmware twice, once to load the firmware into
> > > kernel memory and once to copy the firmware into the final
> > > resting place.
> > > 
> > > This design creates needless memory pressure and delays loading
> > > because we have to copy from kernel memory to somewhere else.
> > > Let's add a couple DMA APIs that allow us to map DMA buffers into
> > > the CPU's address space in arbitrary sizes. With this API, we can
> > > allocate a DMA buffer with DMA_ATTR_NO_KERNEL_MAPPING and move a
> > > small mapping window across our large DMA buffer to load the
> > > firmware directly into buffer.
> > 
> > The first two patches in this series don't make sense to me. I don't
> > understand what the memory pressure is: physical or virtual? Because
> > they don't seem to address the former (the DMA buffer is allocated in
> > full) while the latter doesn't need any addressing at all on arm64, we
> > have plenty of VA space.
> > 
> > Why do you even need the coherent DMA API? Can you use the streaming API
> > (map_sg etc.) with a separately allocated buffer?
> 
> Hmm I guess I need to add in the patches that show how this is used on
> top of "no-map" DT reserved memory regions. There are some more patches
> that allow us to assigned reserved memory regions with the "no-map"
> attribute to devices and then allocate from those regions using the
> coherent DMA APIs. In the downstream kernel it's called a removed dma
> pool[1].
> 
> So the plan is to wire that all up so that the device can have a
> reserved chunk of memory for the firmware that doesn't exist in the
> kernel's linear memory mappings. Once we have allocated the region, we
> can map it into the kernel's view of memory for a short time so that we
> can load the firmware into it (dma_remap part). Once that's over, we
> want to destroy the mapping so that we 1) don't use any of the kernel's
> virtual memory space (dma_unremap part) to back the buffer and 2) so
> that the secure world can protect the memory from the non-secure world.

Does the firmware already know about such memory? If yes, I presume the
kernel would have to be told about it and won't try to map it in the
linear mapping.

At this point, wouldn't a combination of:

	dma_declare_coherent_memory()
	dma_alloc_from_coherent()
	dma_release_from_coherent()
	dma_release_declared_memory()

work? The removed_alloc() implementation in the link you posted doesn't
seem far from dma_alloc_from_coherent(). The releasing of the declared
memory above would unmap the memory, so there are no VA mappings left.

-- 
Catalin

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

#1389433

On 04/27/2016 08:25 AM, Catalin Marinas wrote:
> On Fri, Apr 22, 2016 at 05:35:16PM -0700, Stephen Boyd wrote:
>> Quoting Catalin Marinas (2016-04-21 03:35:12)
>>> On Tue, Apr 19, 2016 at 06:04:27PM -0700, Stephen Boyd wrote:
>>>> From: Laura Abbott <lauraa@codeaurora.org>
>>>>
>>>> Some systems are memory constrained but they need to load very
>>>> large firmwares. The firmware subsystem allows drivers to request
>>>> this firmware be loaded from the filesystem, but this requires
>>>> that the entire firmware be loaded into kernel memory first
>>>> before it's provided to the driver. This can lead to a situation
>>>> where we map the firmware twice, once to load the firmware into
>>>> kernel memory and once to copy the firmware into the final
>>>> resting place.
>>>>
>>>> This design creates needless memory pressure and delays loading
>>>> because we have to copy from kernel memory to somewhere else.
>>>> Let's add a couple DMA APIs that allow us to map DMA buffers into
>>>> the CPU's address space in arbitrary sizes. With this API, we can
>>>> allocate a DMA buffer with DMA_ATTR_NO_KERNEL_MAPPING and move a
>>>> small mapping window across our large DMA buffer to load the
>>>> firmware directly into buffer.
>>>
>>> The first two patches in this series don't make sense to me. I don't
>>> understand what the memory pressure is: physical or virtual? Because
>>> they don't seem to address the former (the DMA buffer is allocated in
>>> full) while the latter doesn't need any addressing at all on arm64, we
>>> have plenty of VA space.
>>>
>>> Why do you even need the coherent DMA API? Can you use the streaming API
>>> (map_sg etc.) with a separately allocated buffer?
>>
>> Hmm I guess I need to add in the patches that show how this is used on
>> top of "no-map" DT reserved memory regions. There are some more patches
>> that allow us to assigned reserved memory regions with the "no-map"
>> attribute to devices and then allocate from those regions using the
>> coherent DMA APIs. In the downstream kernel it's called a removed dma
>> pool[1].
>>
>> So the plan is to wire that all up so that the device can have a
>> reserved chunk of memory for the firmware that doesn't exist in the
>> kernel's linear memory mappings. Once we have allocated the region, we
>> can map it into the kernel's view of memory for a short time so that we
>> can load the firmware into it (dma_remap part). Once that's over, we
>> want to destroy the mapping so that we 1) don't use any of the kernel's
>> virtual memory space (dma_unremap part) to back the buffer and 2) so
>> that the secure world can protect the memory from the non-secure world.
>
> Does the firmware already know about such memory? If yes, I presume the
> kernel would have to be told about it and won't try to map it in the
> linear mapping.
>
> At this point, wouldn't a combination of:
>
> 	dma_declare_coherent_memory()
> 	dma_alloc_from_coherent()
> 	dma_release_from_coherent()
> 	dma_release_declared_memory()
>
> work? The removed_alloc() implementation in the link you posted doesn't
> seem far from dma_alloc_from_coherent(). The releasing of the declared
> memory above would unmap the memory, so there are no VA mappings left.
>

The removed alloc was specifically written as a fork of the coherent
pool. This was a choice for ease of out of tree maintenance. The better
choice here would be to fold those features back into dma-coherent.c
if needed.

Thanks,
Laura

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web