On Sat, 2023-11-11 at 13:48 +0100, Krzysztof Kozlowski wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content. On 11/11/2023 12:15, 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
What was the outcome of previous discussion? I don't see any references to the conclusion and your changelog "Reword the dt-binding description" is way too generic.
You must explain what happened here.
I don't think there is a final conclusion yet in v1. Jeff helped explain that this region also is defined in TEE firmware. I put this a bit in the commit message above.
Sorry for confusing.
.../reserved-memory/secure_cma_region.yaml | 44
+++++++++++++++++++
1 file changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-
memory/secure_cma_region.yaml
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
Will change to: Secure Region. Is it ok?
+description:
- This binding describes a CMA region that can dynamically
transition
Describe the hardware or firmware, not the binding. Drop first four words and rephrase it.
Memory region for TEE usage, which is also defined in the TEE firmware. When an activity (e.g. secure video playback) requiring usage of this starts, this region will be protected by MPU (Memory Protect Unit) in the TEE firmware. After the activity is completed, the region will be unprotected by the TEE and usable by the non-secure side (i.e. kernel and userspace).
Does this description make sense for you?
+between secure and non-secure states that a TEE can allocate
memory
+from.
It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint.
Do not send untested code.
Sorry. I will update them and test this before sending.
+maintainers:
- Yong Wu yong.wu@mediatek.com
+allOf:
- $ref: reserved-memory.yaml
+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?
+required:
- compatible
- reg
- reusable
+unevaluatedProperties: false
+examples:
- |
Stray blank line.
Thanks for reviewing so careful. Will fix this and below.
- reserved-memory {
#address-cells = <1>;#size-cells = <1>;ranges;reserved-memory@80000000 {compatible = "secure_cma_region";reusable;reg = <0x80000000 0x18000000>;reg is second property. Open DTS and check how it is there.
};- };
Best regards, Krzysztof