From: Laura Abbott <labbott@redhat.com>
To: "Andrew F. Davis" <afd@ti.com>,
"Sumit Semwal" <sumit.semwal@linaro.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Arve Hjønnevåg" <arve@android.com>
Cc: devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 14/14] staging: android: ion: Add UNMAPPED heap type and helper
Date: Mon, 14 Jan 2019 18:32:01 -0800 [thread overview]
Message-ID: <bd3c15f2-2635-588e-80b4-18e8e44d27b3@redhat.com> (raw)
In-Reply-To: <20190111180523.27862-15-afd@ti.com>
On 1/11/19 10:05 AM, Andrew F. Davis wrote:
> The "unmapped" heap is very similar to the carveout heap except
> the backing memory is presumed to be unmappable by the host, in
> my specific case due to firewalls. This memory can still be
> allocated from and used by devices that do have access to the
> backing memory.
>
> Based originally on the secure/unmapped heap from Linaro for
> the OP-TEE SDP implementation, this was re-written to match
> the carveout heap helper code.
>
> Suggested-by: Etienne Carriere <etienne.carriere@linaro.org>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
> drivers/staging/android/ion/Kconfig | 10 ++
> drivers/staging/android/ion/Makefile | 1 +
> drivers/staging/android/ion/ion.h | 16 +++
> .../staging/android/ion/ion_unmapped_heap.c | 123 ++++++++++++++++++
> drivers/staging/android/uapi/ion.h | 3 +
> 5 files changed, 153 insertions(+)
> create mode 100644 drivers/staging/android/ion/ion_unmapped_heap.c
>
> diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
> index 0fdda6f62953..a117b8b91b14 100644
> --- a/drivers/staging/android/ion/Kconfig
> +++ b/drivers/staging/android/ion/Kconfig
> @@ -42,3 +42,13 @@ config ION_CMA_HEAP
> Choose this option to enable CMA heaps with Ion. This heap is backed
> by the Contiguous Memory Allocator (CMA). If your system has these
> regions, you should say Y here.
> +
> +config ION_UNMAPPED_HEAP
> + bool "ION unmapped heap support"
> + depends on ION
> + help
> + Choose this option to enable UNMAPPED heaps with Ion. This heap is
> + backed in specific memory pools, carveout from the Linux memory.
> + Unlike carveout heaps these are assumed to be not mappable by
> + kernel or user-space.
> + Unless you know your system has these regions, you should say N here.
> diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
> index 17f3a7569e3d..c71a1f3de581 100644
> --- a/drivers/staging/android/ion/Makefile
> +++ b/drivers/staging/android/ion/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o
> obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
> obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
> obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
> +obj-$(CONFIG_ION_UNMAPPED_HEAP) += ion_unmapped_heap.o
> diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
> index 97b2876b165a..ce74332018ba 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -341,4 +341,20 @@ static inline struct ion_heap *ion_chunk_heap_create(phys_addr_t base, size_t si
> }
> #endif
>
> +#ifdef CONFIG_ION_UNMAPPED_HEAP
> +/**
> + * ion_unmapped_heap_create
> + * @base: base address of carveout memory
> + * @size: size of carveout memory region
> + *
> + * Creates an unmapped ion_heap using the passed in data
> + */
> +struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t size);
> +#else
> +struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t size)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +#endif
> +
> #endif /* _ION_H */
> diff --git a/drivers/staging/android/ion/ion_unmapped_heap.c b/drivers/staging/android/ion/ion_unmapped_heap.c
> new file mode 100644
> index 000000000000..7602b659c2ec
> --- /dev/null
> +++ b/drivers/staging/android/ion/ion_unmapped_heap.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ION Memory Allocator unmapped heap helper
> + *
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd@ti.com>
> + *
> + * ION "unmapped" heaps are physical memory heaps not by default mapped into
> + * a virtual address space. The buffer owner can explicitly request kernel
> + * space mappings but the underlying memory may still not be accessible for
> + * various reasons, such as firewalls.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/genalloc.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +
> +#include "ion.h"
> +
> +#define ION_UNMAPPED_ALLOCATE_FAIL -1
> +
> +struct ion_unmapped_heap {
> + struct ion_heap heap;
> + struct gen_pool *pool;
> +};
> +
> +static phys_addr_t ion_unmapped_allocate(struct ion_heap *heap,
> + unsigned long size)
> +{
> + struct ion_unmapped_heap *unmapped_heap =
> + container_of(heap, struct ion_unmapped_heap, heap);
> + unsigned long offset;
> +
> + offset = gen_pool_alloc(unmapped_heap->pool, size);
> + if (!offset)
> + return ION_UNMAPPED_ALLOCATE_FAIL;
> +
> + return offset;
> +}
> +
> +static void ion_unmapped_free(struct ion_heap *heap, phys_addr_t addr,
> + unsigned long size)
> +{
> + struct ion_unmapped_heap *unmapped_heap =
> + container_of(heap, struct ion_unmapped_heap, heap);
> +
> + gen_pool_free(unmapped_heap->pool, addr, size);
> +}
> +
> +static int ion_unmapped_heap_allocate(struct ion_heap *heap,
> + struct ion_buffer *buffer,
> + unsigned long size,
> + unsigned long flags)
> +{
> + struct sg_table *table;
> + phys_addr_t paddr;
> + int ret;
> +
> + table = kmalloc(sizeof(*table), GFP_KERNEL);
> + if (!table)
> + return -ENOMEM;
> + ret = sg_alloc_table(table, 1, GFP_KERNEL);
> + if (ret)
> + goto err_free;
> +
> + paddr = ion_unmapped_allocate(heap, size);
> + if (paddr == ION_UNMAPPED_ALLOCATE_FAIL) {
> + ret = -ENOMEM;
> + goto err_free_table;
> + }
> +
> + sg_set_page(table->sgl, pfn_to_page(PFN_DOWN(paddr)), size, 0);
> + buffer->sg_table = table;
> +
If this memory is actually unmapped this is not going to work because
the struct page will not be valid.
> + return 0;
> +
> +err_free_table:
> + sg_free_table(table);
> +err_free:
> + kfree(table);
> + return ret;
> +}
> +
> +static void ion_unmapped_heap_free(struct ion_buffer *buffer)
> +{
> + struct ion_heap *heap = buffer->heap;
> + struct sg_table *table = buffer->sg_table;
> + struct page *page = sg_page(table->sgl);
> + phys_addr_t paddr = PFN_PHYS(page_to_pfn(page));
> +
> + ion_unmapped_free(heap, paddr, buffer->size);
> + sg_free_table(buffer->sg_table);
> + kfree(buffer->sg_table);
> +}
> +
> +static struct ion_heap_ops unmapped_heap_ops = {
> + .allocate = ion_unmapped_heap_allocate,
> + .free = ion_unmapped_heap_free,
> + /* no .map_user, user mapping of unmapped heaps not allowed */
> + .map_kernel = ion_heap_map_kernel,
> + .unmap_kernel = ion_heap_unmap_kernel,
> +};
> +
> +struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t size)
> +{
> + struct ion_unmapped_heap *unmapped_heap;
> +
> + unmapped_heap = kzalloc(sizeof(*unmapped_heap), GFP_KERNEL);
> + if (!unmapped_heap)
> + return ERR_PTR(-ENOMEM);
> +
> + unmapped_heap->pool = gen_pool_create(PAGE_SHIFT, -1);
> + if (!unmapped_heap->pool) {
> + kfree(unmapped_heap);
> + return ERR_PTR(-ENOMEM);
> + }
> + gen_pool_add(unmapped_heap->pool, base, size, -1);
> + unmapped_heap->heap.ops = &unmapped_heap_ops;
> + unmapped_heap->heap.type = ION_HEAP_TYPE_UNMAPPED;
> +
> + return &unmapped_heap->heap;
> +}
> diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
> index 5d7009884c13..d5f98bc5f340 100644
> --- a/drivers/staging/android/uapi/ion.h
> +++ b/drivers/staging/android/uapi/ion.h
> @@ -19,6 +19,8 @@
> * carveout heap, allocations are physically
> * contiguous
> * @ION_HEAP_TYPE_DMA: memory allocated via DMA API
> + * @ION_HEAP_TYPE_UNMAPPED: memory not intended to be mapped into the
> + * linux address space unless for debug cases
> * @ION_NUM_HEAPS: helper for iterating over heaps, a bit mask
> * is used to identify the heaps, so only 32
> * total heap types are supported
> @@ -29,6 +31,7 @@ enum ion_heap_type {
> ION_HEAP_TYPE_CARVEOUT,
> ION_HEAP_TYPE_CHUNK,
> ION_HEAP_TYPE_DMA,
> + ION_HEAP_TYPE_UNMAPPED,
> ION_HEAP_TYPE_CUSTOM, /*
> * must be last so device specific heaps always
> * are at the end of this enum
>
Overall this seems way too similar to the carveout heap
to justify adding another heap type. It also still missing
the part of where exactly you call ion_unmapped_heap_create.
Figuring that out is one of the blocking items for moving
Ion out of staging.
Thanks,
Laura
next prev parent reply other threads:[~2019-01-15 2:34 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-11 18:05 [PATCH 00/14] Misc ION cleanups and adding unmapped heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 01/14] staging: android: ion: Add proper header information Andrew F. Davis
2019-01-11 18:05 ` [PATCH 02/14] staging: android: ion: Remove empty ion_ioctl_dir() function Andrew F. Davis
2019-01-11 18:05 ` [PATCH 03/14] staging: android: ion: Merge ion-ioctl.c into ion.c Andrew F. Davis
2019-01-11 18:05 ` [PATCH 04/14] staging: android: ion: Remove leftover comment Andrew F. Davis
2019-01-11 18:05 ` [PATCH 05/14] staging: android: ion: Remove struct ion_platform_heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 06/14] staging: android: ion: Fixup some white-space issues Andrew F. Davis
2019-01-11 18:05 ` [PATCH 07/14] staging: android: ion: Sync comment docs with struct ion_buffer Andrew F. Davis
2019-01-11 18:05 ` [PATCH 08/14] staging: android: ion: Remove base from ion_carveout_heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 09/14] staging: android: ion: Remove base from ion_chunk_heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 10/14] staging: android: ion: Remove unused headers Andrew F. Davis
2019-01-11 18:05 ` [PATCH 11/14] staging: android: ion: Allow heap name to be null Andrew F. Davis
2019-01-16 15:28 ` Brian Starkey
2019-01-16 17:12 ` Andrew F. Davis
2019-01-18 19:53 ` Laura Abbott
2019-01-21 14:30 ` Andrew F. Davis
2019-01-11 18:05 ` [PATCH 12/14] staging: android: ion: Declare helpers for carveout and chunk heaps Andrew F. Davis
2019-01-18 9:59 ` Greg Kroah-Hartman
2019-01-18 16:09 ` Andrew F. Davis
2019-01-18 19:54 ` Laura Abbott
2019-01-11 18:05 ` [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap Andrew F. Davis
2019-01-14 17:13 ` Liam Mark
2019-01-15 15:44 ` Andrew F. Davis
2019-01-15 17:45 ` Liam Mark
2019-01-15 18:38 ` Andrew F. Davis
2019-01-15 18:40 ` Andrew F. Davis
2019-01-16 15:19 ` Brian Starkey
2019-01-16 17:05 ` Andrew F. Davis
2019-01-16 22:54 ` Liam Mark
2019-01-17 16:25 ` Andrew F. Davis
2019-01-18 1:11 ` Liam Mark
2019-01-18 17:16 ` Andrew F. Davis
2019-01-21 11:22 ` Brian Starkey
2019-01-21 21:21 ` Andrew F. Davis
2019-01-22 17:33 ` Sumit Semwal
2019-01-23 16:51 ` Andrew F. Davis
2019-01-23 17:11 ` Brian Starkey
2019-01-24 16:04 ` Andrew F. Davis
2019-01-24 16:44 ` Brian Starkey
2019-02-19 21:37 ` Laura Abbott
[not found] ` <CAO_48GHZPkE8Or_dB6CVMi5Jv5eufE_Z36MT7ztJwTqTzkTpKA@mail.gmail.com>
2019-01-23 17:09 ` Andrew F. Davis
2019-01-22 22:56 ` Liam Mark
2019-01-21 20:11 ` Liam Mark
2019-01-15 19:05 ` Laura Abbott
2019-01-16 16:17 ` Andrew F. Davis
2019-01-16 22:48 ` Liam Mark
2019-01-17 16:13 ` Andrew F. Davis
2019-01-18 1:04 ` Liam Mark
2019-01-18 16:50 ` Andrew F. Davis
2019-01-18 21:43 ` Liam Mark
2019-01-21 15:15 ` Andrew F. Davis
2019-01-21 20:04 ` Liam Mark
2019-01-18 20:31 ` Laura Abbott
2019-01-18 20:43 ` Andrew F. Davis
2019-01-18 20:46 ` Laura Abbott
2019-01-19 10:11 ` Christoph Hellwig
2019-01-17 9:02 ` Christoph Hellwig
2019-01-11 18:05 ` [PATCH 14/14] staging: android: ion: Add UNMAPPED heap type and helper Andrew F. Davis
2019-01-15 2:32 ` Laura Abbott [this message]
2019-01-15 15:58 ` Andrew F. Davis
2019-01-15 18:43 ` Laura Abbott
2019-01-15 19:11 ` Laura Abbott
2019-01-16 16:18 ` Andrew F. Davis
2019-01-15 2:39 ` [PATCH 00/14] Misc ION cleanups and adding unmapped heap Laura Abbott
2019-01-15 17:47 ` Andrew F. Davis
2019-01-15 18:58 ` Laura Abbott
2019-01-16 16:05 ` Andrew F. Davis
2019-01-18 20:19 ` Laura Abbott
2019-01-21 14:58 ` Andrew F. Davis
2019-01-18 9:56 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bd3c15f2-2635-588e-80b4-18e8e44d27b3@redhat.com \
--to=labbott@redhat.com \
--cc=afd@ti.com \
--cc=arve@android.com \
--cc=devel@driverdev.osuosl.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sumit.semwal@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).