| Message ID | 20231111111559.8218-7-yong.wu@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | dma-buf: heaps: Add secure heap | expand |
On Sat, 11 Nov 2023 19:15:57 +0800, Yong Wu wrote: > Add a binding for describing the secure CMA reserved memory range. The > memory range also will be defined in the TEE firmware. It means the TEE > will be configured with the same address/size that is being set in this > DT node. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > .../reserved-memory/secure_cma_region.yaml | 44 +++++++++++++++++++ > 1 file changed, 44 insertions(+) > create mode 100644 Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml:12:1: [error] syntax error: could not find expected ':' (syntax) dtschema/dtc warnings/errors: make[2]: *** Deleting file 'Documentation/devicetree/bindings/reserved-memory/secure_cma_region.example.dts' Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml:12:1: could not find expected ':' make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/reserved-memory/secure_cma_region.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml:12:1: could not find expected ':' /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml: ignoring, error parsing file make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1427: dt_binding_check] Error 2 make: *** [Makefile:234: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231111111559.8218-7-yong.wu@mediatek.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 13/11/2023 6:37 am, Yong Wu (吴勇) wrote: [...] >>> +properties: >>> + compatible: >>> + const: secure_cma_region >> >> Still wrong compatible. Look at other bindings - there is nowhere >> underscore. Look at other reserved memory bindings especially. >> >> Also, CMA is a Linux thingy, so either not suitable for bindings at >> all, >> or you need Linux specific compatible. I don't quite get why do you >> evennot >> put CMA there - adding Linux specific stuff will get obvious >> pushback... > > Thanks. I will change to: secure-region. Is this ok? No, the previous discussion went off in entirely the wrong direction. To reiterate, the point of the binding is not to describe the expected usage of the thing nor the general concept of the thing, but to describe the actual thing itself. There are any number of different ways software may interact with a "secure region", so that is meaningless as a compatible. It needs to describe *this* secure memory interface offered by *this* TEE, so that software knows that to use it requires making those particular SiP calls with that particular UUID etc. Thanks, Robin.
May I suggest the following for the device tree binding? (I'm not very
familiar w/ device trees, so apologies for any oversights, but trying
to process the feedback here and help move Mediatek along). This
should align with my other suggestions for having an MTK specific
portion to their secure heap implementation; which also means there
should be an MTK specific device tree binding.
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
$id: http://devicetree.org/schemas/reserved-memory/mediatek,dynamic-secure-region.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
title: Mediatek Dynamic Reserved Region
description:
A memory region that can dynamically transition as a whole between
secure and non-secure states. This memory will be protected by OP-TEE
when allocations are active and unprotected otherwise.
maintainers:
- Yong Wu <yong.wu@mediatek.com>
allOf:
- $ref: reserved-memory.yaml
properties:
compatible:
const: mediatek,dynamic-secure-region
required:
- compatible
- reg
- reusable
unevaluatedProperties: false
examples:
- |
reserved-memory {
#address-cells = <1>;
#size-cells = <1>;
ranges;
reserved-memory@80000000 {
compatible = "mediatek,dynamic-secure-region";
reusable;
reg = <0x80000000 0x18000000>;
};
};
On Tue, Nov 14, 2023 at 5:18 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 13/11/2023 6:37 am, Yong Wu (吴勇) wrote:
> [...]
> >>> +properties:
> >>> + compatible:
> >>> + const: secure_cma_region
> >>
> >> Still wrong compatible. Look at other bindings - there is nowhere
> >> underscore. Look at other reserved memory bindings especially.
> >>
> >> Also, CMA is a Linux thingy, so either not suitable for bindings at
> >> all,
> >> or you need Linux specific compatible. I don't quite get why do you
> >> evennot
> >> put CMA there - adding Linux specific stuff will get obvious
> >> pushback...
> >
> > Thanks. I will change to: secure-region. Is this ok?
>
> No, the previous discussion went off in entirely the wrong direction. To
> reiterate, the point of the binding is not to describe the expected
> usage of the thing nor the general concept of the thing, but to describe
> the actual thing itself. There are any number of different ways software
> may interact with a "secure region", so that is meaningless as a
> compatible. It needs to describe *this* secure memory interface offered
> by *this* TEE, so that software knows that to use it requires making
> those particular SiP calls with that particular UUID etc.
>
> Thanks,
> Robin.
diff --git a/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml b/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml new file mode 100644 index 000000000000..8ab559595fbe --- /dev/null +++ b/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reserved-memory/secure_cma_region.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Secure Reserved CMA Region + +description: + This binding describes a CMA region that can dynamically transition +between secure and non-secure states that a TEE can allocate memory +from. + +maintainers: + - Yong Wu <yong.wu@mediatek.com> + +allOf: + - $ref: reserved-memory.yaml + +properties: + compatible: + const: secure_cma_region + +required: + - compatible + - reg + - reusable + +unevaluatedProperties: false + +examples: + - | + + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + reserved-memory@80000000 { + compatible = "secure_cma_region"; + reusable; + reg = <0x80000000 0x18000000>; + }; + };
Add a binding for describing the secure CMA reserved memory range. The memory range also will be defined in the TEE firmware. It means the TEE will be configured with the same address/size that is being set in this DT node. Signed-off-by: Yong Wu <yong.wu@mediatek.com> --- .../reserved-memory/secure_cma_region.yaml | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml