Re: [PATCH 8/9] dt-bindings: reserved-memory: MediaTek: Add reserved memory for SVP

On 12/09/2023 4:53 pm, Rob Herring wrote:

On Tue, Sep 12, 2023 at 11:13:50AM +0100, Robin Murphy wrote:

quoted

On 12/09/2023 9:28 am, Krzysztof Kozlowski wrote:

quoted

On 12/09/2023 08:16, Yong Wu (吴勇) wrote:

quoted

Hi Rob,

Thanks for your review.

On Mon, 2023-09-11 at 10:44 -0500, Rob Herring wrote:

quoted

   	
External email : Please do not click links or open attachments until
you have verified the sender or the content.
   On Mon, Sep 11, 2023 at 10:30:37AM +0800, Yong Wu wrote:

quoted

This adds the binding for describing a CMA memory for MediaTek

SVP(Secure

quoted

Video Path).

CMA is a Linux thing. How is this related to CMA?

quoted

quoted

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
   .../mediatek,secure_cma_chunkmem.yaml         | 42

+++++++++++++++++++

quoted

   1 file changed, 42 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/reserved-

memory/mediatek,secure_cma_chunkmem.yaml

quoted

diff --git a/Documentation/devicetree/bindings/reserved-

memory/mediatek,secure_cma_chunkmem.yaml
b/Documentation/devicetree/bindings/reserved-
memory/mediatek,secure_cma_chunkmem.yaml

quoted

new file mode 100644
index 000000000000..cc10e00d35c4

--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-

memory/mediatek,secure_cma_chunkmem.yaml

quoted

@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id:

http://devicetree.org/schemas/reserved-memory/mediatek,secure_cma_chunkmem.yaml#

quoted

+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Secure Video Path Reserved Memory

What makes this specific to Mediatek? Secure video path is fairly
common, right?

Here we just reserve a buffer and would like to create a dma-buf secure
heap for SVP, then the secure engines(Vcodec and DRM) could prepare
secure buffer through it.
But the heap driver is pure SW driver, it is not platform device and

All drivers are pure SW.

quoted

we don't have a corresponding HW unit for it. Thus I don't think I
could create a platform dtsi node and use "memory-region" pointer to
the region. I used RESERVEDMEM_OF_DECLARE currently(The code is in
[9/9]). Sorry if this is not right.

If this is not for any hardware and you already understand this (since
you cannot use other bindings) then you cannot have custom bindings for
it either.

quoted

Then in our usage case, is there some similar method to do this? or
any other suggestion?

Don't stuff software into DTS.

Aren't most reserved-memory bindings just software policy if you look at it
that way, though? IIUC this is a pool of memory that is visible and
available to the Non-Secure OS, but is fundamentally owned by the Secure
TEE, and pages that the TEE allocates from it will become physically
inaccessible to the OS. Thus the platform does impose constraints on how the
Non-Secure OS may use it, and per the rest of the reserved-memory bindings,
describing it as a "reusable" reservation seems entirely appropriate. If
anything that's *more* platform-related and so DT-relevant than typical
arbitrary reservations which just represent "save some memory to dedicate to
a particular driver" and don't actually bear any relationship to firmware or
hardware at all.

Yes, a memory range defined by hardware or firmware is within scope of
DT. (CMA at aribitrary address was questionable.)

My issue here is more that 'secure video memory' is not any way Mediatek
specific. AIUI, it's a requirement from certain content providers for
video playback to work. So why the Mediatek specific binding?

Based on the implementation, I'd ask the question the other way round - the way it works looks to be at least somewhat dependent on Mediatek's TEE, in ways where other vendors' equivalent implementations may be functionally incompatible, however nothing suggests it's actually specific to video (beyond that presumably being the primary use-case they had in mind).

Thanks,
Robin.