Hi,
This patch set is based on top of Yong Wu's restricted heap patch set [1].
It's also a continuation on Olivier's Add dma-buf secure-heap patch set [2].
The Linaro restricted heap uses genalloc in the kernel to manage the heap
carvout. This is a difference from the Mediatek restricted heap which
relies on the secure world to manage the carveout.
I've tried to adress the comments on [2], but [1] introduces changes so I'm
afraid I've had to skip some comments.
This can be tested on QEMU with the following steps:
repo init -u https://github.com/jenswi-linaro/manifest.git -m qemu_v8.xml \
-b prototype/sdp-v1
repo sync -j8
cd build
make toolchains -j4
make all -j$(nproc)
make run-only
# login and at the prompt:
xtest --sdp-basic
https://optee.readthedocs.io/en/latest/building/prerequisites.html
list dependencies needed to build the above.
The tests are pretty basic, mostly checking that a Trusted Application in
the secure world can access and manipulate the memory.
Cheers,
Jens
[1] https://lore.kernel.org/dri-devel/20240515112308.10171-1-yong.wu@mediatek.c…
[2] https://lore.kernel.org/lkml/20220805135330.970-1-olivier.masse@nxp.com/
Changes since Olivier's post [2]:
* Based on Yong Wu's post [1] where much of dma-buf handling is done in
the generic restricted heap
* Simplifications and cleanup
* New commit message for "dma-buf: heaps: add Linaro restricted dmabuf heap
support"
* Replaced the word "secure" with "restricted" where applicable
Etienne Carriere (1):
tee: new ioctl to a register tee_shm from a dmabuf file descriptor
Jens Wiklander (2):
dma-buf: heaps: restricted_heap: add no_map attribute
dma-buf: heaps: add Linaro restricted dmabuf heap support
Olivier Masse (1):
dt-bindings: reserved-memory: add linaro,restricted-heap
.../linaro,restricted-heap.yaml | 56 ++++++
drivers/dma-buf/heaps/Kconfig | 10 ++
drivers/dma-buf/heaps/Makefile | 1 +
drivers/dma-buf/heaps/restricted_heap.c | 17 +-
drivers/dma-buf/heaps/restricted_heap.h | 2 +
.../dma-buf/heaps/restricted_heap_linaro.c | 165 ++++++++++++++++++
drivers/tee/tee_core.c | 38 ++++
drivers/tee/tee_shm.c | 104 ++++++++++-
include/linux/tee_drv.h | 11 ++
include/uapi/linux/tee.h | 29 +++
10 files changed, 426 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/reserved-memory/linaro,restricted-heap.yaml
create mode 100644 drivers/dma-buf/heaps/restricted_heap_linaro.c
--
2.34.1
Condsider the following call sequence:
/* Upper layer */
dma_fence_begin_signalling();
lock(tainted_shared_lock);
/* Driver callback */
dma_fence_begin_signalling();
...
The driver might here use a utility that is annotated as intended for the
dma-fence signalling critical path. Now if the upper layer isn't correctly
annotated yet for whatever reason, resulting in
/* Upper layer */
lock(tainted_shared_lock);
/* Driver callback */
dma_fence_begin_signalling();
We will receive a false lockdep locking order violation notification from
dma_fence_begin_signalling(). However entering a dma-fence signalling
critical section itself doesn't block and could not cause a deadlock.
So use a successful read_trylock() annotation instead for
dma_fence_begin_signalling(). That will make sure that the locking order
is correctly registered in the first case, and doesn't register any
locking order in the second case.
The alternative is of course to make sure that the "Upper layer" is always
correctly annotated. But experience shows that's not easily achievable
in all cases.
Signed-off-by: Thomas Hellström <thomas.hellstrom(a)linux.intel.com>
---
drivers/dma-buf/dma-fence.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index f177c56269bb..17f632768ef9 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -308,8 +308,8 @@ bool dma_fence_begin_signalling(void)
if (in_atomic())
return true;
- /* ... and non-recursive readlock */
- lock_acquire(&dma_fence_lockdep_map, 0, 0, 1, 1, NULL, _RET_IP_);
+ /* ... and non-recursive successful read_trylock */
+ lock_acquire(&dma_fence_lockdep_map, 0, 1, 1, 1, NULL, _RET_IP_);
return false;
}
@@ -340,7 +340,7 @@ void __dma_fence_might_wait(void)
lock_map_acquire(&dma_fence_lockdep_map);
lock_map_release(&dma_fence_lockdep_map);
if (tmp)
- lock_acquire(&dma_fence_lockdep_map, 0, 0, 1, 1, NULL, _THIS_IP_);
+ lock_acquire(&dma_fence_lockdep_map, 0, 1, 1, 1, NULL, _THIS_IP_);
}
#endif
--
2.39.2
Until VM_DONTEXPAND was added in commit 1c1914d6e8c6 ("dma-buf: heaps:
Don't track CMA dma-buf pages under RssFile") it was possible to obtain
a mapping larger than the buffer size via mremap and bypass the overflow
check in dma_buf_mmap_internal. When using such a mapping to attempt to
fault past the end of the buffer, the CMA heap fault handler also checks
the fault offset against the buffer size, but gets the boundary wrong by
1. Fix the boundary check so that we don't read off the end of the pages
array and insert an arbitrary page in the mapping.
Reported-by: Xingyu Jin <xingyuj(a)google.com>
Fixes: a5d2d29e24be ("dma-buf: heaps: Move heap-helper logic into the cma_heap implementation")
Cc: stable(a)vger.kernel.org # Applicable >= 5.10. Needs adjustments only for 5.10.
Signed-off-by: T.J. Mercier <tjmercier(a)google.com>
---
drivers/dma-buf/heaps/cma_heap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index c384004b918e..93be88b805fe 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -165,7 +165,7 @@ static vm_fault_t cma_heap_vm_fault(struct vm_fault *vmf)
struct vm_area_struct *vma = vmf->vma;
struct cma_heap_buffer *buffer = vma->vm_private_data;
- if (vmf->pgoff > buffer->pagecount)
+ if (vmf->pgoff >= buffer->pagecount)
return VM_FAULT_SIGBUS;
return vmf_insert_pfn(vma, vmf->address, page_to_pfn(buffer->pages[vmf->pgoff]));
--
2.46.0.469.g59c65b2a67-goog
Hi Hui-Ping,
hpchen0nvt(a)gmail.com wrote on Wed, 28 Aug 2024 10:47:17 +0800:
> Dear Miquèl,
>
> Thank you for your reply.
>
>
>
> On 2024/8/24 上午 12:26, Miquel Raynal wrote:
> > Hi,
> >
> > hpchen0nvt(a)gmail.com wrote on Wed, 21 Aug 2024 07:11:31 +0000:
> >
> >> Add dt-bindings for the Nuvoton MA35 SoC NAND Controller.
> >>
> >> Signed-off-by: Hui-Ping Chen <hpchen0nvt(a)gmail.com>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org>
> >> ---
> >> .../bindings/mtd/nuvoton,ma35d1-nand.yaml | 93 +++++++++++++++++++
> >> 1 file changed, 93 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml
> >> new file mode 100644
> >> index 000000000000..152784e73263
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml
> >> @@ -0,0 +1,93 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/mtd/nuvoton,ma35d1-nand.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Nuvoton MA35D1 NAND Flash Interface (NFI) Controller
> >> +
> >> +maintainers:
> >> + - Hui-Ping Chen <hpchen0nvt(a)gmail.com>
> >> +
> >> +allOf:
> >> + - $ref: nand-controller.yaml#
> >> +
> >> +properties:
> >> + compatible:
> >> + enum:
> >> + - nuvoton,ma35d1-nand
> > Can we please use the -nand-controller suffix. A NAND is a the common
> > name for a chip with storage inside. You are describing a host
> > controller that can be connected to in order to talk to a NAND.
>
>
> Okay, I will change it to nuvoton,ma35d1-nfi.
>
> Because in our platform, it is the NAND Flash Interface.
nfi is not an acronym that is understandable by everyone. Please use
-nand-controller. Don't be worried by the size of the string.
You can use the acronym as prefix for your NAND controller functions
though.
Thanks,
Miquèl
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/20240412090851.24999-1-yunfei.dong@m…
[2] https://lore.kernel.org/linux-mediatek/20240403102701.369-1-shawn.sung@medi…
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/20240112092014.23999-1-yong.wu@media…
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/20231212024607.3681-1-yong.wu@mediat…
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/20231111111559.8218-1-yong.wu@mediat…
1) Move John's patches into the vcodec patchset since they use the new
dma heap interface directly.
https://lore.kernel.org/linux-mediatek/20231106120423.23364-1-yunfei.dong@m…
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/20230911023038.30649-1-yong.wu@media…
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