The purpose of this patchset is for MediaTek secure video playback, and
also to enable other potential uses of this in the future. The 'restricted
dma-heap' will be used to allocate dma_buf objects that reference memory
in the secure world that is inaccessible/unmappable by the non-secure
(i.e. kernel/userspace) world. That memory will be used by the secure/
trusted world to store secure information (i.e. decrypted media content).
The dma_bufs allocated from the kernel will be passed to V4L2 for video
decoding (as input and output). They will also be used by the drm
system for rendering of the content.
This patchset adds two MediaTek restricted heaps and they will be used in
v4l2[1] and drm[2].
1) restricted_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video
Path). The buffer is reserved for the secure world after bootup and it
is used for vcodec's ES/working buffer;
2) restricted_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is
dynamically reserved for the secure world and will be got when we start
playing secure videos. Once the security video playing is complete, the
CMA will be released. This heap is used for the vcodec's frame buffer.
[1] https://lore.kernel.org/linux-mediatek/[email protected]/
[2] https://lore.kernel.org/linux-mediatek/[email protected]/
Change note:
v5: 1) Reconstruct TEE commands to allow the kernel to obtain the PA of the
TEE buffer to initialize a valid sg table.
2) Previously, PA was hidden from the kernel. Then the kernel checks if
this is restricted buffer by "if (sg_page(sg) == NULL)".
In this version, we will add a new explicit interface
(sg_dma_is_restricted) for users to determine whether this is a
restricted buffer.
3) some words improve, like using "rheap".
Rebase on v6.9-rc7.
v4: https://lore.kernel.org/linux-mediatek/[email protected]/
1) Rename the heap name from "secure" to "restricted". suggested from
Simon/Pekka. There are still several "secure" string in MTK file
since we use ARM platform in which we call this "secure world"/
"secure command".
v3: https://lore.kernel.org/linux-mediatek/[email protected]/
1) Separate the secure heap to a common file(secure_heap.c) and mtk
special file (secure_heap_mtk.c), and put all the tee related code
into our special file.
2) About dt-binding, Add "mediatek," prefix since this is Mediatek TEE
firmware definition.
3) Remove the normal CMA heap which is a draft for qcom.
Rebase on v6.7-rc1.
v2: https://lore.kernel.org/linux-mediatek/[email protected]/
1) Move John's patches into the vcodec patchset since they use the new
dma heap interface directly.
https://lore.kernel.org/linux-mediatek/[email protected]/
2) Reword the dt-binding description.
3) Rename the heap name from mtk_svp to secure_mtk_cm.
This means the current vcodec/DRM upstream code doesn't match this.
4) Add a normal CMA heap. currently it should be a draft version.
5) Regarding the UUID, I still use hard code, but put it in a private
data which allow the others could set their own UUID. What's more, UUID
is necessary for the session with TEE. If we don't have it, we can't
communicate with the TEE, including the get_uuid interface, which tries
to make uuid more generic, not working. If there is other way to make
UUID more general, please free to tell me.
v1: https://lore.kernel.org/linux-mediatek/[email protected]/
Base on v6.6-rc1.
Yong Wu (9):
dt-bindings: reserved-memory: Add mediatek,dynamic-restricted-region
scatterlist: Add a flag for the restricted memory
lib/scatterlist: Add sg_dup_table
dma-buf: heaps: Initialize a restricted heap
dma-buf: heaps: restricted_heap: Add private heap ops
dma-buf: heaps: restricted_heap: Add dma_ops
dma-buf: heaps: restricted_heap: Add MediaTek restricted heap and
heap_init
dma-buf: heaps: restricted_heap_mtk: Add TEE memory service call
dma_buf: heaps: restricted_heap_mtk: Add a new CMA heap
.../mediatek,dynamic-restricted-region.yaml | 43 ++
drivers/dma-buf/heaps/Kconfig | 16 +
drivers/dma-buf/heaps/Makefile | 4 +-
drivers/dma-buf/heaps/restricted_heap.c | 219 +++++++++
drivers/dma-buf/heaps/restricted_heap.h | 45 ++
drivers/dma-buf/heaps/restricted_heap_mtk.c | 423 ++++++++++++++++++
drivers/dma-buf/heaps/system_heap.c | 27 +-
include/linux/scatterlist.h | 36 ++
lib/scatterlist.c | 26 ++
9 files changed, 812 insertions(+), 27 deletions(-)
create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,dynamic-restricted-region.yaml
create mode 100644 drivers/dma-buf/heaps/restricted_heap.c
create mode 100644 drivers/dma-buf/heaps/restricted_heap.h
create mode 100644 drivers/dma-buf/heaps/restricted_heap_mtk.c
--
2.18.0
Add "struct restricted_heap_ops". For the restricted memory, totally there
are two steps:
a) alloc: Allocate the buffer in kernel;
b) restrict_buf: Restrict/Protect/Secure that buffer.
The "alloc" is mandatory while "restrict_buf" is optional since it may
be part of "alloc".
Signed-off-by: Yong Wu <[email protected]>
---
drivers/dma-buf/heaps/restricted_heap.c | 41 ++++++++++++++++++++++++-
drivers/dma-buf/heaps/restricted_heap.h | 12 ++++++++
2 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/restricted_heap.c b/drivers/dma-buf/heaps/restricted_heap.c
index c2ae19ba7d7e..8bb3c1876a69 100644
--- a/drivers/dma-buf/heaps/restricted_heap.c
+++ b/drivers/dma-buf/heaps/restricted_heap.c
@@ -12,10 +12,44 @@
#include "restricted_heap.h"
+static int
+restricted_heap_memory_allocate(struct restricted_heap *rheap, struct restricted_buffer *buf)
+{
+ const struct restricted_heap_ops *ops = rheap->ops;
+ int ret;
+
+ ret = ops->alloc(rheap, buf);
+ if (ret)
+ return ret;
+
+ if (ops->restrict_buf) {
+ ret = ops->restrict_buf(rheap, buf);
+ if (ret)
+ goto buf_free;
+ }
+ return 0;
+
+buf_free:
+ ops->free(rheap, buf);
+ return ret;
+}
+
+static void
+restricted_heap_memory_free(struct restricted_heap *rheap, struct restricted_buffer *buf)
+{
+ const struct restricted_heap_ops *ops = rheap->ops;
+
+ if (ops->unrestrict_buf)
+ ops->unrestrict_buf(rheap, buf);
+
+ ops->free(rheap, buf);
+}
+
static struct dma_buf *
restricted_heap_allocate(struct dma_heap *heap, unsigned long size,
unsigned long fd_flags, unsigned long heap_flags)
{
+ struct restricted_heap *rheap = dma_heap_get_drvdata(heap);
struct restricted_buffer *restricted_buf;
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
struct dma_buf *dmabuf;
@@ -28,6 +62,9 @@ restricted_heap_allocate(struct dma_heap *heap, unsigned long size,
restricted_buf->size = ALIGN(size, PAGE_SIZE);
restricted_buf->heap = heap;
+ ret = restricted_heap_memory_allocate(rheap, restricted_buf);
+ if (ret)
+ goto err_free_buf;
exp_info.exp_name = dma_heap_get_name(heap);
exp_info.size = restricted_buf->size;
exp_info.flags = fd_flags;
@@ -36,11 +73,13 @@ restricted_heap_allocate(struct dma_heap *heap, unsigned long size,
dmabuf = dma_buf_export(&exp_info);
if (IS_ERR(dmabuf)) {
ret = PTR_ERR(dmabuf);
- goto err_free_buf;
+ goto err_free_rstrd_mem;
}
return dmabuf;
+err_free_rstrd_mem:
+ restricted_heap_memory_free(rheap, restricted_buf);
err_free_buf:
kfree(restricted_buf);
return ERR_PTR(ret);
diff --git a/drivers/dma-buf/heaps/restricted_heap.h b/drivers/dma-buf/heaps/restricted_heap.h
index b448f77616ac..5783275d5714 100644
--- a/drivers/dma-buf/heaps/restricted_heap.h
+++ b/drivers/dma-buf/heaps/restricted_heap.h
@@ -15,6 +15,18 @@ struct restricted_buffer {
struct restricted_heap {
const char *name;
+
+ const struct restricted_heap_ops *ops;
+};
+
+struct restricted_heap_ops {
+ int (*heap_init)(struct restricted_heap *rheap);
+
+ int (*alloc)(struct restricted_heap *rheap, struct restricted_buffer *buf);
+ void (*free)(struct restricted_heap *rheap, struct restricted_buffer *buf);
+
+ int (*restrict_buf)(struct restricted_heap *rheap, struct restricted_buffer *buf);
+ void (*unrestrict_buf)(struct restricted_heap *rheap, struct restricted_buffer *buf);
};
int restricted_heap_add(struct restricted_heap *rheap);
--
2.25.1
Add TEE service call for MediaTek heap. We have a limited number of
hardware entries to protect memory, therefore we cannot protect memory
arbitrarily, and our secure memory management is actually inside OPTEE.
Totally there are 3 commands:
1) MTK_TZCMD_SECMEM_ZALLOC: The kernel tells the TEE what size I want and
the TEE will return a "secure handle"/"secure address". To make the name
more general, We call it "restricted_addr" here. The restricted_addr is a
reference to the secure buffer within TEE.
2) MTK_TZCMD_SECMEM_FREE: Free the buffer. Match with the ALLOC command
above.
3) MTK_TZCMD_SECMEM_RETRIEVE_SG: If the tee buffer is discrete, this
command can retrieve the detailed PA list from the TEE with which the
kernel will initialize the sg table. Of course, if the tee buffer is
contiguous, the PA will be obtained directly from MTK_TZCMD_SECMEM_ZALLOC.
Signed-off-by: Yong Wu <[email protected]>
---
drivers/dma-buf/heaps/restricted_heap.h | 3 +
drivers/dma-buf/heaps/restricted_heap_mtk.c | 193 ++++++++++++++++++++
2 files changed, 196 insertions(+)
diff --git a/drivers/dma-buf/heaps/restricted_heap.h b/drivers/dma-buf/heaps/restricted_heap.h
index 2a33a1c7a48b..8cb9211093c5 100644
--- a/drivers/dma-buf/heaps/restricted_heap.h
+++ b/drivers/dma-buf/heaps/restricted_heap.h
@@ -13,6 +13,9 @@ struct restricted_buffer {
size_t size;
struct sg_table sg_table;
+
+ /* A reference to a buffer in the trusted or secure world. */
+ u64 restricted_addr;
};
struct restricted_heap {
diff --git a/drivers/dma-buf/heaps/restricted_heap_mtk.c b/drivers/dma-buf/heaps/restricted_heap_mtk.c
index 52e805eb9858..e571eae719e0 100644
--- a/drivers/dma-buf/heaps/restricted_heap_mtk.c
+++ b/drivers/dma-buf/heaps/restricted_heap_mtk.c
@@ -27,6 +27,46 @@ enum mtk_secure_mem_type {
MTK_SECURE_MEMORY_TYPE_CM_TZ = 1,
};
+/* This structure also is synchronized with tee, thus not use the phys_addr_t */
+struct mtk_tee_scatterlist {
+ u64 pa;
+ u32 length;
+} __packed;
+
+enum mtk_secure_buffer_tee_cmd {
+ /*
+ * Allocate the zeroed secure memory from TEE.
+ *
+ * [in] value[0].a: The buffer size.
+ * value[0].b: alignment.
+ * [in] value[1].a: enum mtk_secure_mem_type.
+ * [inout]
+ * [out] value[2].a: entry number of memory block.
+ * If this is 1, it means the memory is continuous.
+ * value[2].b: buffer PA base.
+ * [out] value[3].a: The secure handle.
+ */
+ MTK_TZCMD_SECMEM_ZALLOC = 0x10000, /* MTK TEE Command ID Base */
+
+ /*
+ * Free secure memory.
+ *
+ * [in] value[0].a: The secure handle of this buffer, It's value[3].a of
+ * MTK_TZCMD_SECMEM_ZALLOC.
+ * [out] value[1].a: return value, 0 means successful, otherwise fail.
+ */
+ MTK_TZCMD_SECMEM_FREE = 0x10001,
+
+ /*
+ * Get secure memory sg-list.
+ *
+ * [in] value[0].a: The secure handle of this buffer, It's value[3].a of
+ * MTK_TZCMD_SECMEM_ZALLOC.
+ * [out] value[1].a: The array of sg items (struct mtk_tee_scatterlist).
+ */
+ MTK_TZCMD_SECMEM_RETRIEVE_SG = 0x10002,
+};
+
struct mtk_restricted_heap_data {
struct tee_context *tee_ctx;
u32 tee_session;
@@ -76,6 +116,155 @@ static int mtk_tee_session_init(struct mtk_restricted_heap_data *data)
return ret;
}
+static int mtk_tee_service_call(struct tee_context *tee_ctx, u32 session,
+ unsigned int command, struct tee_param *params)
+{
+ struct tee_ioctl_invoke_arg arg = {0};
+ int ret;
+
+ arg.num_params = TEE_PARAM_NUM;
+ arg.session = session;
+ arg.func = command;
+
+ ret = tee_client_invoke_func(tee_ctx, &arg, params);
+ if (ret < 0 || arg.ret) {
+ pr_err("%s: cmd 0x%x ret %d:%x.\n", __func__, command, ret, arg.ret);
+ ret = -EOPNOTSUPP;
+ }
+ return ret;
+}
+
+static int mtk_tee_secmem_free(struct restricted_heap *rheap, u64 restricted_addr)
+{
+ struct mtk_restricted_heap_data *data = rheap->priv_data;
+ struct tee_param params[TEE_PARAM_NUM] = {0};
+
+ params[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
+ params[0].u.value.a = restricted_addr;
+ params[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
+
+ mtk_tee_service_call(data->tee_ctx, data->tee_session,
+ MTK_TZCMD_SECMEM_FREE, params);
+ if (params[1].u.value.a) {
+ pr_err("%s, SECMEM_FREE buffer(0x%llx) fail(%lld) from TEE.\n",
+ rheap->name, restricted_addr, params[1].u.value.a);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int mtk_tee_restrict_memory(struct restricted_heap *rheap, struct restricted_buffer *buf)
+{
+ struct mtk_restricted_heap_data *data = rheap->priv_data;
+ struct tee_param params[TEE_PARAM_NUM] = {0};
+ struct mtk_tee_scatterlist *tee_sg_item;
+ struct mtk_tee_scatterlist *tee_sg_buf;
+ unsigned int sg_num, size, i;
+ struct tee_shm *sg_shm;
+ struct scatterlist *sg;
+ phys_addr_t pa_tee;
+ u64 r_addr;
+ int ret;
+
+ /* Alloc the secure buffer and get the sg-list number from TEE */
+ params[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
+ params[0].u.value.a = buf->size;
+ params[0].u.value.b = PAGE_SIZE;
+ params[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
+ params[1].u.value.a = data->mem_type;
+ params[2].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT;
+ params[3].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
+ ret = mtk_tee_service_call(data->tee_ctx, data->tee_session,
+ MTK_TZCMD_SECMEM_ZALLOC, params);
+ if (ret)
+ return -ENOMEM;
+
+ sg_num = params[2].u.value.a;
+ r_addr = params[3].u.value.a;
+
+ /* If there is only one entry, It means the buffer is continuous, Get the PA directly. */
+ if (sg_num == 1) {
+ pa_tee = params[2].u.value.b;
+ if (!pa_tee)
+ goto tee_secmem_free;
+ if (sg_alloc_table(&buf->sg_table, 1, GFP_KERNEL))
+ goto tee_secmem_free;
+ sg_set_page(buf->sg_table.sgl, phys_to_page(pa_tee), buf->size, 0);
+ buf->restricted_addr = r_addr;
+ return 0;
+ }
+
+ /*
+ * If the buffer inside TEE are discontinuous, Use sharemem to retrieve
+ * the detail sg list from TEE.
+ */
+ tee_sg_buf = kmalloc_array(sg_num, sizeof(*tee_sg_item), GFP_KERNEL);
+ if (!tee_sg_buf) {
+ ret = -ENOMEM;
+ goto tee_secmem_free;
+ }
+
+ size = sg_num * sizeof(*tee_sg_item);
+ sg_shm = tee_shm_register_kernel_buf(data->tee_ctx, tee_sg_buf, size);
+ if (!sg_shm)
+ goto free_tee_sg_buf;
+
+ memset(params, 0, sizeof(params));
+ params[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
+ params[0].u.value.a = r_addr;
+ params[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ params[1].u.memref.shm = sg_shm;
+ params[1].u.memref.size = size;
+ ret = mtk_tee_service_call(data->tee_ctx, data->tee_session,
+ MTK_TZCMD_SECMEM_RETRIEVE_SG, params);
+ if (ret)
+ goto put_shm;
+
+ if (sg_alloc_table(&buf->sg_table, sg_num, GFP_KERNEL))
+ goto put_shm;
+
+ for_each_sgtable_sg(&buf->sg_table, sg, i) {
+ tee_sg_item = tee_sg_buf + i;
+ if (!tee_sg_item->pa)
+ goto free_buf_sg;
+ sg_set_page(sg, phys_to_page(tee_sg_item->pa),
+ tee_sg_item->length, 0);
+ }
+
+ tee_shm_put(sg_shm);
+ kfree(tee_sg_buf);
+ buf->restricted_addr = r_addr;
+ return 0;
+
+free_buf_sg:
+ sg_free_table(&buf->sg_table);
+put_shm:
+ tee_shm_put(sg_shm);
+free_tee_sg_buf:
+ kfree(tee_sg_buf);
+tee_secmem_free:
+ mtk_tee_secmem_free(rheap, r_addr);
+ return ret;
+}
+
+static void mtk_tee_unrestrict_memory(struct restricted_heap *rheap, struct restricted_buffer *buf)
+{
+ sg_free_table(&buf->sg_table);
+ mtk_tee_secmem_free(rheap, buf->restricted_addr);
+}
+
+static int
+mtk_restricted_memory_allocate(struct restricted_heap *rheap, struct restricted_buffer *buf)
+{
+ /* The memory allocating is within the TEE. */
+ return 0;
+}
+
+static void
+mtk_restricted_memory_free(struct restricted_heap *rheap, struct restricted_buffer *buf)
+{
+}
+
static int mtk_restricted_heap_init(struct restricted_heap *rheap)
{
struct mtk_restricted_heap_data *data = rheap->priv_data;
@@ -87,6 +276,10 @@ static int mtk_restricted_heap_init(struct restricted_heap *rheap)
static const struct restricted_heap_ops mtk_restricted_heap_ops = {
.heap_init = mtk_restricted_heap_init,
+ .alloc = mtk_restricted_memory_allocate,
+ .free = mtk_restricted_memory_free,
+ .restrict_buf = mtk_tee_restrict_memory,
+ .unrestrict_buf = mtk_tee_unrestrict_memory,
};
static struct mtk_restricted_heap_data mtk_restricted_heap_data = {
--
2.25.1
Introduce a FLAG for the restricted memory which means the memory is
protected by TEE or hypervisor, then it's inaccessiable for kernel.
Currently we don't use sg_dma_unmark_restricted, thus this interface
has not been added.
Signed-off-by: Yong Wu <[email protected]>
---
include/linux/scatterlist.h | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 77df3d7b18a6..a6ad9018eca0 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
#define SG_DMA_BUS_ADDRESS (1 << 0)
#define SG_DMA_SWIOTLB (1 << 1)
+#define SG_DMA_RESTRICTED (2 << 1)
/**
* sg_dma_is_bus_address - Return whether a given segment was marked
@@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist *sg)
sg->dma_flags |= SG_DMA_SWIOTLB;
}
+/**
+ * sg_dma_mark_restricted - Mark the scatterlist for restricted buffer.
+ * @sg: SG entry
+ *
+ * Description:
+ * Marks a a scatterlist for the restricted buffer that may be inaccessiable
+ * in kernel if it is protected.
+ */
+static inline void sg_dma_mark_restricted(struct scatterlist *sg)
+{
+ sg->dma_flags |= SG_DMA_RESTRICTED;
+}
+
+/**
+ * sg_dma_is_restricted - Return whether the scatterlist was marked as restricted
+ * buffer.
+ * @sg: SG entry
+ *
+ * Description:
+ * Returns true if the scatterlist was marked as restricted buffer.
+ */
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+ return sg->dma_flags & SG_DMA_RESTRICTED;
+}
#else
static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
@@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist *sg)
{
}
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+ return false;
+}
+
+static inline void sg_dma_mark_restrited(struct scatterlist *sg)
+{
+}
#endif /* CONFIG_NEED_SG_DMA_FLAGS */
/**
--
2.25.1
Am 15.05.24 um 13:23 schrieb Yong Wu:
> Introduce a FLAG for the restricted memory which means the memory is
> protected by TEE or hypervisor, then it's inaccessiable for kernel.
>
> Currently we don't use sg_dma_unmark_restricted, thus this interface
> has not been added.
Why should that be part of the scatterlist? It doesn't seem to affect
any of it's functionality.
As far as I can see the scatterlist shouldn't be the transport of this
kind of information.
Regards,
Christian.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> include/linux/scatterlist.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 77df3d7b18a6..a6ad9018eca0 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
>
> #define SG_DMA_BUS_ADDRESS (1 << 0)
> #define SG_DMA_SWIOTLB (1 << 1)
> +#define SG_DMA_RESTRICTED (2 << 1)
>
> /**
> * sg_dma_is_bus_address - Return whether a given segment was marked
> @@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist *sg)
> sg->dma_flags |= SG_DMA_SWIOTLB;
> }
>
> +/**
> + * sg_dma_mark_restricted - Mark the scatterlist for restricted buffer.
> + * @sg: SG entry
> + *
> + * Description:
> + * Marks a a scatterlist for the restricted buffer that may be inaccessiable
> + * in kernel if it is protected.
> + */
> +static inline void sg_dma_mark_restricted(struct scatterlist *sg)
> +{
> + sg->dma_flags |= SG_DMA_RESTRICTED;
> +}
> +
> +/**
> + * sg_dma_is_restricted - Return whether the scatterlist was marked as restricted
> + * buffer.
> + * @sg: SG entry
> + *
> + * Description:
> + * Returns true if the scatterlist was marked as restricted buffer.
> + */
> +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> +{
> + return sg->dma_flags & SG_DMA_RESTRICTED;
> +}
> #else
>
> static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
> @@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist *sg)
> {
> }
>
> +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> +{
> + return false;
> +}
> +
> +static inline void sg_dma_mark_restrited(struct scatterlist *sg)
> +{
> +}
> #endif /* CONFIG_NEED_SG_DMA_FLAGS */
>
> /**
Il 15/05/24 13:23, Yong Wu ha scritto:
> Introduce a FLAG for the restricted memory which means the memory is
> protected by TEE or hypervisor, then it's inaccessiable for kernel.
>
> Currently we don't use sg_dma_unmark_restricted, thus this interface
> has not been added.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> include/linux/scatterlist.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 77df3d7b18a6..a6ad9018eca0 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
>
> #define SG_DMA_BUS_ADDRESS (1 << 0)
> #define SG_DMA_SWIOTLB (1 << 1)
> +#define SG_DMA_RESTRICTED (2 << 1)
I think you wanted to write (1 << 2) here :-)
Cheers,
Angelo
>
> /**
> * sg_dma_is_bus_address - Return whether a given segment was marked
> @@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist *sg)
> sg->dma_flags |= SG_DMA_SWIOTLB;
> }
>
> +/**
> + * sg_dma_mark_restricted - Mark the scatterlist for restricted buffer.
> + * @sg: SG entry
> + *
> + * Description:
> + * Marks a a scatterlist for the restricted buffer that may be inaccessiable
> + * in kernel if it is protected.
> + */
> +static inline void sg_dma_mark_restricted(struct scatterlist *sg)
> +{
> + sg->dma_flags |= SG_DMA_RESTRICTED;
> +}
> +
> +/**
> + * sg_dma_is_restricted - Return whether the scatterlist was marked as restricted
> + * buffer.
> + * @sg: SG entry
> + *
> + * Description:
> + * Returns true if the scatterlist was marked as restricted buffer.
> + */
> +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> +{
> + return sg->dma_flags & SG_DMA_RESTRICTED;
> +}
> #else
>
> static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
> @@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist *sg)
> {
> }
>
> +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> +{
> + return false;
> +}
> +
> +static inline void sg_dma_mark_restrited(struct scatterlist *sg)
> +{
> +}
> #endif /* CONFIG_NEED_SG_DMA_FLAGS */
>
> /**
On Thu, 2024-05-16 at 10:17 +0200, Christian König wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Am 15.05.24 um 13:23 schrieb Yong Wu:
> > Introduce a FLAG for the restricted memory which means the memory
> is
> > protected by TEE or hypervisor, then it's inaccessiable for kernel.
> >
> > Currently we don't use sg_dma_unmark_restricted, thus this
> interface
> > has not been added.
>
> Why should that be part of the scatterlist? It doesn't seem to
> affect
> any of it's functionality.
>
> As far as I can see the scatterlist shouldn't be the transport of
> this
> kind of information.
Thanks for the review. I will remove this.
In our user scenario, DRM will import these buffers and check if this
is a restricted buffer. If yes, it will use secure GCE takes over.
If this judgment is not suitable to be placed in scatterlist. I don't
know if it is ok to limit this inside dma-buf. Adding such an
interface:
static bool dma_buf_is_restricted(struct dma_buf *dmabuf)
{
return !strncmp(dmabuf->exp_name, "restricted", 10);
}
Thanks.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---
> > include/linux/scatterlist.h | 34
> ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/include/linux/scatterlist.h
> b/include/linux/scatterlist.h
> > index 77df3d7b18a6..a6ad9018eca0 100644
> > --- a/include/linux/scatterlist.h
> > +++ b/include/linux/scatterlist.h
> > @@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct
> scatterlist *sg)
> >
> > #define SG_DMA_BUS_ADDRESS(1 << 0)
> > #define SG_DMA_SWIOTLB(1 << 1)
> > +#define SG_DMA_RESTRICTED(2 << 1)
> >
> > /**
> > * sg_dma_is_bus_address - Return whether a given segment was
> marked
> > @@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct
> scatterlist *sg)
> > sg->dma_flags |= SG_DMA_SWIOTLB;
> > }
> >
> > +/**
> > + * sg_dma_mark_restricted - Mark the scatterlist for restricted
> buffer.
> > + * @sg:SG entry
> > + *
> > + * Description:
> > + * Marks a a scatterlist for the restricted buffer that may be
> inaccessiable
> > + * in kernel if it is protected.
> > + */
> > +static inline void sg_dma_mark_restricted(struct scatterlist *sg)
> > +{
> > +sg->dma_flags |= SG_DMA_RESTRICTED;
> > +}
> > +
> > +/**
> > + * sg_dma_is_restricted - Return whether the scatterlist was
> marked as restricted
> > + * buffer.
> > + * @sg:SG entry
> > + *
> > + * Description:
> > + * Returns true if the scatterlist was marked as restricted
> buffer.
> > + */
> > +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> > +{
> > +return sg->dma_flags & SG_DMA_RESTRICTED;
> > +}
> > #else
> >
> > static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
> > @@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct
> scatterlist *sg)
> > {
> > }
> >
> > +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> > +{
> > +return false;
> > +}
> > +
> > +static inline void sg_dma_mark_restrited(struct scatterlist *sg)
> > +{
> > +}
> > #endif/* CONFIG_NEED_SG_DMA_FLAGS */
> >
> > /**
>
>
On Thu, 2024-05-16 at 11:59 +0200, AngeloGioacchino Del Regno wrote:
> Il 15/05/24 13:23, Yong Wu ha scritto:
> > Introduce a FLAG for the restricted memory which means the memory
> > is
> > protected by TEE or hypervisor, then it's inaccessiable for kernel.
> >
> > Currently we don't use sg_dma_unmark_restricted, thus this
> > interface
> > has not been added.
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---
> > include/linux/scatterlist.h | 34
> > ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/include/linux/scatterlist.h
> > b/include/linux/scatterlist.h
> > index 77df3d7b18a6..a6ad9018eca0 100644
> > --- a/include/linux/scatterlist.h
> > +++ b/include/linux/scatterlist.h
> > @@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct
> > scatterlist *sg)
> >
> > #define SG_DMA_BUS_ADDRESS (1 << 0)
> > #define SG_DMA_SWIOTLB (1 << 1)
> > +#define SG_DMA_RESTRICTED (2 << 1)
>
> I think you wanted to write (1 << 2) here :-)
Apparently, you are right:)
Thanks.
>
> Cheers,
> Angelo
Am 20.05.24 um 09:58 schrieb Yong Wu (吴勇):
> On Thu, 2024-05-16 at 10:17 +0200, Christian König wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> Am 15.05.24 um 13:23 schrieb Yong Wu:
>>> Introduce a FLAG for the restricted memory which means the memory
>> is
>>> protected by TEE or hypervisor, then it's inaccessiable for kernel.
>>>
>>> Currently we don't use sg_dma_unmark_restricted, thus this
>> interface
>>> has not been added.
>> Why should that be part of the scatterlist? It doesn't seem to
>> affect
>> any of it's functionality.
>>
>> As far as I can see the scatterlist shouldn't be the transport of
>> this
>> kind of information.
> Thanks for the review. I will remove this.
>
> In our user scenario, DRM will import these buffers and check if this
> is a restricted buffer. If yes, it will use secure GCE takes over.
>
> If this judgment is not suitable to be placed in scatterlist. I don't
> know if it is ok to limit this inside dma-buf. Adding such an
> interface:
>
> static bool dma_buf_is_restricted(struct dma_buf *dmabuf)
> {
> return !strncmp(dmabuf->exp_name, "restricted", 10);
> }
No, usually stuff like that doesn't belong into DMA buf either.
Question here really is who controls the security status of the memory
backing the buffer?
In other words who tells the exporter that it should allocate and fill a
buffer with encrypted data?
If that is userspace then that is part of the format information and it
is also userspace who should tell the importer that it needs to work
with encrypted data.
The kernel is intentionally not involved in stuff like that.
Regards,
Christian.
>
> Thanks.
>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Yong Wu <[email protected]>
>>> ---
>>> include/linux/scatterlist.h | 34
>> ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>>
>>> diff --git a/include/linux/scatterlist.h
>> b/include/linux/scatterlist.h
>>> index 77df3d7b18a6..a6ad9018eca0 100644
>>> --- a/include/linux/scatterlist.h
>>> +++ b/include/linux/scatterlist.h
>>> @@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct
>> scatterlist *sg)
>>>
>>> #define SG_DMA_BUS_ADDRESS(1 << 0)
>>> #define SG_DMA_SWIOTLB(1 << 1)
>>> +#define SG_DMA_RESTRICTED(2 << 1)
>>>
>>> /**
>>> * sg_dma_is_bus_address - Return whether a given segment was
>> marked
>>> @@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct
>> scatterlist *sg)
>>> sg->dma_flags |= SG_DMA_SWIOTLB;
>>> }
>>>
>>> +/**
>>> + * sg_dma_mark_restricted - Mark the scatterlist for restricted
>> buffer.
>>> + * @sg:SG entry
>>> + *
>>> + * Description:
>>> + * Marks a a scatterlist for the restricted buffer that may be
>> inaccessiable
>>> + * in kernel if it is protected.
>>> + */
>>> +static inline void sg_dma_mark_restricted(struct scatterlist *sg)
>>> +{
>>> +sg->dma_flags |= SG_DMA_RESTRICTED;
>>> +}
>>> +
>>> +/**
>>> + * sg_dma_is_restricted - Return whether the scatterlist was
>> marked as restricted
>>> + * buffer.
>>> + * @sg:SG entry
>>> + *
>>> + * Description:
>>> + * Returns true if the scatterlist was marked as restricted
>> buffer.
>>> + */
>>> +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
>>> +{
>>> +return sg->dma_flags & SG_DMA_RESTRICTED;
>>> +}
>>> #else
>>>
>>> static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
>>> @@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct
>> scatterlist *sg)
>>> {
>>> }
>>>
>>> +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
>>> +{
>>> +return false;
>>> +}
>>> +
>>> +static inline void sg_dma_mark_restrited(struct scatterlist *sg)
>>> +{
>>> +}
>>> #endif/* CONFIG_NEED_SG_DMA_FLAGS */
>>>
>>> /**
>>