Hey Rob,
Thanks a lot for taking the time to review!
On 02.01.24 16:20, Rob Herring wrote:
On Fri, Dec 22, 2023 at 07:51:44PM +0000, Alexander Graf wrote:
With ftrace in KHO, we are creating an ABI between old kernel and new
kernel about the state that they transfer. To ensure that we document
that state and catch any breaking change, let's add its schema to the
common devicetree bindings. This way, we can quickly reason about the
state that gets passed.
Why so much data in DT rather than putting all this information into
memory in your own data structure and DT just has a single property
pointing to that? That's what is done with every other blob of data
passed by kexec.
This is our own data structure for KHO that just happens to again
contain a DT structure. The reason is simple: I want a unified,
versioned, introspectable data format that is cross platform so you
don't need to touch every architecture specific boot passing logic every
time you want to add a tiny piece of data.
Signed-off-by: Alexander Graf <graf@xxxxxxxxxx>
---
.../bindings/kho/ftrace/ftrace-array.yaml | 46 +++++++++++++++
.../bindings/kho/ftrace/ftrace-cpu.yaml | 56 +++++++++++++++++++
.../bindings/kho/ftrace/ftrace.yaml | 48 ++++++++++++++++
3 files changed, 150 insertions(+)
create mode 100644 Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml
create mode 100644 Documentation/devicetree/bindings/kho/ftrace/ftrace-cpu.yaml
create mode 100644 Documentation/devicetree/bindings/kho/ftrace/ftrace.yaml
diff --git a/Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml b/Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml
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
+
+maintainers:
+ - Alexander Graf <graf@xxxxxxxxxx>
+
+properties:
+ compatible:
+ enum:
+ - ftrace,array-v1
+
+ trace_flags:
+ $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
This can be expressed as a schema.
Could you please give me a few more hints here? I'm not sure I understand how :)
+additionalProperties: true
+
+required:
+ - compatible
+ - trace_flags
+
+examples:
+ - |
+ ftrace {
+ compatible = "ftrace-v1";
+ events = <1 1 2 2 3 3>;
+
+ global_trace {
+ compatible = "ftrace,array-v1";
+ trace_flags = < 0x3354601 >;
+
+ cpu0 {
+ compatible = "ftrace,cpu-v1";
+ cpu = < 0x00 >;
+ mem = < 0x101000000ULL 0x38ULL 0x101000100ULL 0x1000ULL 0x101000038ULL 0x38ULL 0x101002000ULL 0x1000ULL>;
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/kho/ftrace/ftrace-cpu.yaml b/Documentation/devicetree/bindings/kho/ftrace/ftrace-cpu.yaml
new file mode 100644
index 000000000000..58c715e93f37
--- /dev/null
+++ b/Documentation/devicetree/bindings/kho/ftrace/ftrace-cpu.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/kho/ftrace/ftrace-cpu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ftrace per-CPU ring buffer contents
+
+maintainers:
+ - Alexander Graf <graf@xxxxxxxxxx>
+
+properties:
+ compatible:
+ enum:
+ - ftrace,cpu-v1
+
+ cpu:
+ $ref: /schemas/types.yaml#/definitions/uint32
'cpu' is already a defined property of type 'phandle'. While we can have
multiple types for a given property name, best practice is to avoid
that. The normal way to refer to a CPU would be a phandle to the CPU
node, but I can see that might not make sense here.
"CPU numbers" on arm64 are 64-bit values as well as they are the
CPU's MPIDR value.
Here we're looking at the Linux internal CPU numbering which I believe
does not have to use the MIDR value?
+ description:
+ CPU number of the CPU that this ring buffer belonged to when it was
+ serialized.
+
+ mem:
Too vague. Make the property name indicate what's in the memory.
"mem" is a generic property for every node in the KHO DT that contains a
<u64 phys_start, u64 size> array that describes memory to pass over. I
use it in generic code so that we don't need to do memory reservations
individually per node. That means I can't change the name here.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ Array of { u64 phys_addr, u64 len } elements that describe a list of ring
+ buffer pages. Each page consists of two elements. The first element
+ describes the location of the struct buffer_page that contains metadata
+ for a given ring buffer page, such as the ring's head indicator. The
+ second element points to the ring buffer data page which contains the raw
+ trace data.
+
+additionalProperties: false
+
+required:
+ - compatible
+ - cpu
+ - mem
+
+examples:
+ - |
+ ftrace {
+ compatible = "ftrace-v1";
+ events = <1 1 2 2 3 3>;
+
+ global_trace {
+ compatible = "ftrace,array-v1";
+ trace_flags = < 0x3354601 >;
+
+ cpu0 {
+ compatible = "ftrace,cpu-v1";
+ cpu = < 0x00 >;
+ mem = < 0x101000000ULL 0x38ULL 0x101000100ULL 0x1000ULL 0x101000038ULL 0x38ULL 0x101002000ULL 0x1000ULL>;
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/kho/ftrace/ftrace.yaml b/Documentation/devicetree/bindings/kho/ftrace/ftrace.yaml
new file mode 100644
index 000000000000..b87a64843af3
--- /dev/null
+++ b/Documentation/devicetree/bindings/kho/ftrace/ftrace.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/kho/ftrace/ftrace.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ftrace core data
+
+maintainers:
+ - Alexander Graf <graf@xxxxxxxxxx>
+
+properties:
+ compatible:
+ enum:
+ - ftrace-v1
+
+ events:
Again, too vague.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ Array of { u32 crc, u32 type } elements. Each element contains a unique
+ identifier for an event, followed by the identifier that this event had
+ in the previous kernel's trace buffers.
+
+# Other child nodes will be of type "ftrace,array-v1". Each of which describe
+# a trace buffer
+additionalProperties: true
+
+required:
+ - compatible
+ - events
+
+examples:
+ - |
+ ftrace {
This should go under /chosen. Show that here. Start the example with
It can't go under /chosen because x86 doesn't have /chosen :). This is
not a device DT, it's a KHO DT.
'/{' to do that and not add the usual boilerplate we add when extracting
the examples.
What exact difference does /{ and ftrace { make?
Also, we don't need 3 examples. Just do 1 complete example here.
Great idea :)
+ compatible = "ftrace-v1";
+ events = <1 1 2 2 3 3>;
+
+ global_trace {
+ compatible = "ftrace,array-v1";
+ trace_flags = < 0x3354601 >;
+
+ cpu0 {
+ compatible = "ftrace,cpu-v1";
+ cpu = < 0x00 >;
+ mem = < 0x101000000ULL 0x38ULL 0x101000100ULL 0x1000ULL 0x101000038ULL 0x38ULL 0x101002000ULL 0x1000ULL>;
+ };
+ };
+ };
--
2.40.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
