Re: [PATCH v2 17/17] devicetree: Add bindings for ftrace KHO

From
Krzysztof Kozlowski <krzk@kernel.org>
Date
2023-12-24 08:59:06
Also in
kexec, linux-arm-kernel, linux-devicetree, linux-doc, linux-mm, lkml
Reply to
Alexander Graf, 2023-12-23
On 24/12/2023 00:20, Alexander Graf wrote:
quoted
quoted
new file mode 100644
index 000000000000..9960fefc292d
--- /dev/null
+++ b/Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/kho/ftrace/ftrace-array.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ftrace trace array
+
Missing description. Commit msg also does not tell me much. This must
stand on its own and must describe the hardware. Whatever you have in
cover letter, does not matter, especially that you did not Cc us on it.

Alrighty, I'll add descriptions and make the commit message stand on its 
own.

For quick reference: KHO is a new mechanism this patch set introduces 
which allows Linux to pass arbitrary memory and metadata between kernels 
on kexec. I'm reusing FDTs to implement the hand over protocol, as 
Linux-to-Linux boot communication holds very similar properties to 
firmware-to-Linux boot communication. So this binding is not about 
hardware; it's about preserving Linux subsystem state across kexec.
Devicetree is for non-discoverable systems and their hardware, not for
passing arbitrary data between kernels. For me this does not suit DT at
all, please use other ways.

For more details, please refer to the KHO documentation which is part of 
patch 7 of this patch set: 
https://lore.kernel.org/lkml/20231222195144.24532-2-graf@amazon.com/ (local)

quoted
quoted
+maintainers:
+  - Alexander Graf <graf@amazon.com>
+
+properties:
+  compatible:
+    enum:
+      - ftrace,array-v1
+
+  trace_flags:
Underscores are not allowed. Does not look like generic property.

Let me make it "trace-flags" to not have underscores. Could you please 
elaborate on what you mean by generic property?
Generic property, so one without vendor prefix, is shared and common to
a group of devices.

quoted
quoted
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Bitmap of all the trace flags that were enabled in the trace array at the
+      point of serialization.
+
+# Subnodes will be of type "ftrace,cpu-v1", one each per CPU
+additionalProperties: true
No, this must be false. And it goes after required:

Ok, making it false and adding pattern matches instead for subnodes.

quoted
quoted
+
+required:
+  - compatible
+  - trace_flags
+
+examples:
+  - |
+    ftrace {
+        compatible = "ftrace-v1";
+        events = <1 1 2 2 3 3>;
+
+        global_trace {
Again, no underscores.

Ok :)

quoted
quoted
+          compatible = "ftrace,array-v1";
+          trace_flags = < 0x3354601 >;
+
+          cpu0 {
+            compatible = "ftrace,cpu-v1";
+            cpu = < 0x00 >;
Drop redundant spaces.

I don't understand what you're referring to as redundant spaces? Double 
checking, I believe indentation is off for every line below "ftrace {". 
Is that what you're referring to? Fixing :)
Open DTS, some recent, arm64 like Qualcomm. Do you see spaces around <>?
Or open the coding style document... Please do not introduce different
coding style.
quoted
quoted
+            mem = < 0x101000000ULL 0x38ULL 0x101000100ULL 0x1000ULL 0x101000038ULL 0x38ULL 0x101002000ULL 0x1000ULL>;
? Do you see any of such syntax in DTS?

I was trying to make it easy to reason to readers about 64bit numbers - 
64bit numbers are not a problem for DTS reading. Above syntax is.
and then potentially extend dtc to consume that new syntax. KHO DTs are 
native/little endian, so dtc already has some difficulties interpreting 
it which I'll need to fix up with patches to it eventually :). I'll 
change it to something that looks more 32bit'y for now.

Best regards,
Krzysztof
Thread (39 messages) 39 messages, 5 authors, last reply 2024-01-17
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help