2023-10-19 04:47:19

by Vijayanand Jitta

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



On 9/11/2023 8:00 AM, Yong Wu wrote:
> This adds the binding for describing a CMA memory for MediaTek SVP(Secure
> Video Path).
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> .../mediatek,secure_cma_chunkmem.yaml | 42 +++++++++++++++++++
> 1 file changed, 42 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,secure_cma_chunkmem.yaml
>
> diff --git a/Documentation/devicetree/bindings/reserved-memory/mediatek,secure_cma_chunkmem.yaml b/Documentation/devicetree/bindings/reserved-memory/mediatek,secure_cma_chunkmem.yaml
> new file mode 100644
> index 000000000000..cc10e00d35c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/mediatek,secure_cma_chunkmem.yaml
> @@ -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#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Secure Video Path Reserved Memory
> +
> +description:
> + This binding describes the reserved memory for secure video path.
> +
> +maintainers:
> + - Yong Wu <[email protected]>
> +
> +allOf:
> + - $ref: reserved-memory.yaml
> +
> +properties:
> + compatible:
> + const: mediatek,secure_cma_chunkmem
> +
> +required:
> + - compatible
> + - reg
> + - reusable
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + reserved-memory@80000000 {
> + compatible = "mediatek,secure_cma_chunkmem";
> + reusable;
> + reg = <0x80000000 0x18000000>;
> + };
> + };

Instead of having a vendor specific binding for cma area, How about retrieving
https://lore.kernel.org/lkml/[email protected]/ ?
dma_heap_add_cma can just associate cma region and create a heap. So, we can reuse cma heap
code for allocation instead of replicating that code here.

Thanks,
Vijay



2023-10-20 09:51:48

by Yong Wu (吴勇)

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

On Thu, 2023-10-19 at 10:16 +0530, Vijayanand Jitta wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On 9/11/2023 8:00 AM, Yong Wu wrote:
> > This adds the binding for describing a CMA memory for MediaTek
> SVP(Secure
> > Video Path).
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---
> > .../mediatek,secure_cma_chunkmem.yaml | 42
> +++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/reserved-
> memory/mediatek,secure_cma_chunkmem.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/reserved-
> memory/mediatek,secure_cma_chunkmem.yaml
> b/Documentation/devicetree/bindings/reserved-
> memory/mediatek,secure_cma_chunkmem.yaml
> > new file mode 100644
> > index 000000000000..cc10e00d35c4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reserved-
> memory/mediatek,secure_cma_chunkmem.yaml
> > @@ -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#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek Secure Video Path Reserved Memory
> > +
> > +description:
> > + This binding describes the reserved memory for secure video
> path.
> > +
> > +maintainers:
> > + - Yong Wu <[email protected]>
> > +
> > +allOf:
> > + - $ref: reserved-memory.yaml
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,secure_cma_chunkmem
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reusable
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > +
> > + reserved-memory {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
> > +
> > + reserved-memory@80000000 {
> > + compatible = "mediatek,secure_cma_chunkmem";
> > + reusable;
> > + reg = <0x80000000 0x18000000>;
> > + };
> > + };
>
> Instead of having a vendor specific binding for cma area, How about
> retrieving
>
https://lore.kernel.org/lkml/[email protected]/
> ?
> dma_heap_add_cma can just associate cma region and create a heap. So,
> we can reuse cma heap
> code for allocation instead of replicating that code here.
>

Thanks for the reference. I guess we can't use it. There are two
reasons:

a) The secure heap driver is a pure software driver and we have no
device for it, therefore we cannot call dma_heap_add_cma.

b) The CMA area here is dynamic for SVP. Normally this CMA can be used
in the kernel. In the SVP case we use cma_alloc to get it and pass the
entire CMA physical start address and size into TEE to protect the CMA
region. The original CMA heap cannot help with the TEE part.

Thanks.

> Thanks,
> Vijay
>
>
>

2023-11-01 05:52:02

by Jaskaran Singh

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

On 10/20/2023 3:20 PM, Yong Wu (吴勇) wrote:
> On Thu, 2023-10-19 at 10:16 +0530, Vijayanand Jitta wrote:
>>
>> Instead of having a vendor specific binding for cma area, How about
>> retrieving
>>
> https://lore.kernel.org/lkml/[email protected]/
>> ?
>> dma_heap_add_cma can just associate cma region and create a heap. So,
>> we can reuse cma heap
>> code for allocation instead of replicating that code here.
>>
>
> Thanks for the reference. I guess we can't use it. There are two
> reasons:
>
> a) The secure heap driver is a pure software driver and we have no
> device for it, therefore we cannot call dma_heap_add_cma.
>

Hi Yong,

We're considering using struct cma as the function argument to
dma_heap_add_cma() rather than struct device. Would this help
resolve the problem of usage with dma_heap_add_cma()?

> b) The CMA area here is dynamic for SVP. Normally this CMA can be used
> in the kernel. In the SVP case we use cma_alloc to get it and pass the
> entire CMA physical start address and size into TEE to protect the CMA
> region. The original CMA heap cannot help with the TEE part.
>

Referring the conversation at
https://lore.kernel.org/lkml/[email protected]/;

since we're considering abstracting secure mem ops, would it make sense
to use the default CMA heap ops (cma_heap_ops), allocate buffers from it
and secure each allocated buffer?

Thanks,
Jaskaran.

> Thanks.
>
>> Thanks,
>> Vijay
>>
>>
>>

2023-11-06 05:56:48

by Yong Wu (吴勇)

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

On Wed, 2023-11-01 at 11:20 +0530, Jaskaran Singh wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On 10/20/2023 3:20 PM, Yong Wu (吴勇) wrote:
> > On Thu, 2023-10-19 at 10:16 +0530, Vijayanand Jitta wrote:
> >>
> >> Instead of having a vendor specific binding for cma area, How
> about
> >> retrieving
> >>
> >
> https://lore.kernel.org/lkml/[email protected]/
> >> ?
> >> dma_heap_add_cma can just associate cma region and create a heap.
> So,
> >> we can reuse cma heap
> >> code for allocation instead of replicating that code here.
> >>
> >
> > Thanks for the reference. I guess we can't use it. There are two
> > reasons:
> >
> > a) The secure heap driver is a pure software driver and we have no
> > device for it, therefore we cannot call dma_heap_add_cma.
> >
>
> Hi Yong,
>
> We're considering using struct cma as the function argument to
> dma_heap_add_cma() rather than struct device. Would this help
> resolve the problem of usage with dma_heap_add_cma()?

Yes. If we use "struct cma", I guess it works.

>
> > b) The CMA area here is dynamic for SVP. Normally this CMA can be
> used
> > in the kernel. In the SVP case we use cma_alloc to get it and pass
> the
> > entire CMA physical start address and size into TEE to protect the
> CMA
> > region. The original CMA heap cannot help with the TEE part.
> >
>
> Referring the conversation at
>
https://lore.kernel.org/lkml/[email protected]/
> ;
>
> since we're considering abstracting secure mem ops, would it make
> sense
> to use the default CMA heap ops (cma_heap_ops), allocate buffers from
> it
> and secure each allocated buffer?

Then it looks you also need tee operation like tee_client_open_session
and tee_client_invoke_func, right?

It seems we also need change a bit for "cma_heap_allocate" to allow cma
support operations from secure heap.

I will send a v2 to move the discussion forward. The v2 is based on our
case, It won't include the cma part.

>
> Thanks,
> Jaskaran.
>
> > Thanks.
> >
> >> Thanks,
> >> Vijay
> >>
> >>
> >>
>

2023-11-20 08:22:16

by Jaskaran Singh

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



On 11/6/2023 11:26 AM, Yong Wu (吴勇) wrote:
> On Wed, 2023-11-01 at 11:20 +0530, Jaskaran Singh wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> On 10/20/2023 3:20 PM, Yong Wu (吴勇) wrote:
>>> On Thu, 2023-10-19 at 10:16 +0530, Vijayanand Jitta wrote:
>>>>
>>>> Instead of having a vendor specific binding for cma area, How
>> about
>>>> retrieving
>>>>
>>>
>> https://lore.kernel.org/lkml/[email protected]/
>>>> ?
>>>> dma_heap_add_cma can just associate cma region and create a heap.
>> So,
>>>> we can reuse cma heap
>>>> code for allocation instead of replicating that code here.
>>>>
>>>
>>> Thanks for the reference. I guess we can't use it. There are two
>>> reasons:
>>>
>>> a) The secure heap driver is a pure software driver and we have no
>>> device for it, therefore we cannot call dma_heap_add_cma.
>>>
>>
>> Hi Yong,
>>
>> We're considering using struct cma as the function argument to
>> dma_heap_add_cma() rather than struct device. Would this help
>> resolve the problem of usage with dma_heap_add_cma()?
>
> Yes. If we use "struct cma", I guess it works.
>

Great; I've posted a v2[1] for the API incorporating this change.

Thanks,
Jaskaran.

[1]
https://lore.kernel.org/lkml/[email protected]/