[PATCH v2 17/17] devicetree: Add bindings for ftrace KHO
Alexander Graf
graf at amazon.com
Sat Dec 23 15:20:17 PST 2023
Hi Krzysztof!
Thanks a lot for the fast review!
On 23.12.23 15:30, Krzysztof Kozlowski wrote:
> On 22/12/2023 20:51, 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.
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC (and consider --no-git-fallback argument). It might
> happen, that command when run on an older kernel, gives you outdated
> entries. Therefore please be sure you base your patches on recent Linux
> kernel.
Ah, this is about directly CC'ing maintainers? I was slightly picky on
CCs since the CC list is already a bit long for this patch set, so I
limited the CC list to mailing lists and people that I know were
directly interested. Happy to CC you next time.
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
>
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
Happy to fix up for v3 :)
>
>> Signed-off-by: Alexander Graf <graf at amazon.com>
>> ---
>> .../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
>> +
> 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.
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/
>
>> +maintainers:
>> + - Alexander Graf <graf at 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?
>
>
>> + $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.
>
>
>> +
>> +required:
>> + - compatible
>> + - trace_flags
>> +
>> +examples:
>> + - |
>> + ftrace {
>> + compatible = "ftrace-v1";
>> + events = <1 1 2 2 3 3>;
>> +
>> + global_trace {
> Again, no underscores.
Ok :)
>
>> + 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 :)
>
>> + 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 -
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.
Alex
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
More information about the kexec
mailing list