Message-ID: <a3cca0ed-64ca-4921-bb4c-27c0e06b78c6@kernel.org>
Date: Sun, 9 Feb 2025 16:23:09 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Mike Rapoport <rppt@...nel.org>
Cc: linux-kernel@...r.kernel.org, Alexander Graf <graf@...zon.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Andy Lutomirski
 <luto@...nel.org>, Anthony Yznaga <anthony.yznaga@...cle.com>,
 Arnd Bergmann <arnd@...db.de>, Ashish Kalra <ashish.kalra@....com>,
 Benjamin Herrenschmidt <benh@...nel.crashing.org>,
 Borislav Petkov <bp@...en8.de>, Catalin Marinas <catalin.marinas@....com>,
 Dave Hansen <dave.hansen@...ux.intel.com>,
 David Woodhouse <dwmw2@...radead.org>, Eric Biederman
 <ebiederm@...ssion.com>, Ingo Molnar <mingo@...hat.com>,
 James Gowans <jgowans@...zon.com>, Jonathan Corbet <corbet@....net>,
 Mark Rutland <mark.rutland@....com>, Paolo Bonzini <pbonzini@...hat.com>,
 Pasha Tatashin <pasha.tatashin@...een.com>, "H. Peter Anvin"
 <hpa@...or.com>, Peter Zijlstra <peterz@...radead.org>,
 Pratyush Yadav <ptyadav@...zon.de>, Rob Herring <robh+dt@...nel.org>,
 Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>,
 Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>,
 Steven Rostedt <rostedt@...dmis.org>, Thomas Gleixner <tglx@...utronix.de>,
 Tom Lendacky <thomas.lendacky@....com>, Usama Arif
 <usama.arif@...edance.com>, Will Deacon <will@...nel.org>,
 devicetree@...r.kernel.org, kexec@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
 linux-mm@...ck.org, x86@...nel.org
Subject: Re: [PATCH v4 14/14] Documentation: KHO: Add memblock bindings

On 09/02/2025 16:10, Mike Rapoport wrote:
> On Sun, Feb 09, 2025 at 11:29:41AM +0100, Krzysztof Kozlowski wrote:
>> On 06/02/2025 14:27, Mike Rapoport wrote:
>>> From: "Mike Rapoport (Microsoft)" <rppt@...nel.org>
>>>
>>> We introduced KHO into Linux: A framework that allows Linux to pass
>>> metadata and memory across kexec from Linux to Linux. KHO reuses fdt
>>> as file format and shares a lot of the same properties of firmware-to-
>>> Linux boot formats: It needs a stable, documented ABI that allows for
>>> forward and backward compatibility as well as versioning.
>>
>> 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. For bindings, the preferred subjects are
>> explained here:
>> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>  
> These are not devicetree binding for communicating data from firmware to
> the kernel. These bindings are specific to KHO which is perfectly
> reflected by the subject.

No, it is not. None of the bindings use above subject prefix.

> 
> Just a brief reminder from v2 discussion:
> (https://lore.kernel.org/linux-mm/20231222193607.15474-1-graf@amazon.com/)
> 
> "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.

does not matter. You added file to ABI documentation so you must follow
that ABI documentation rules. One rule is proper subject prefix.

> 
> 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/"

I fail to see how this is related to the incorrect subject prefix as I
pointed.

> 
> and
> 
> "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."
>  
>>> As first user of KHO, we introduced memblock which can now preserve
>>> memory ranges reserved with reserve_mem command line options contents
>>> across kexec, so you can use the post-kexec kernel to read traces from
>>> the pre-kexec kernel.
>>>
>>> This patch adds memblock schemas similar to "device" device tree ones to
>>> a new kho bindings directory. This allows us to force contributors to
>>> document the data that moves across KHO kexecs and catch breaking change
>>> during review.
>>>
>>> Co-developed-by: Alexander Graf <graf@...zon.com>
>>> Signed-off-by: Alexander Graf <graf@...zon.com>
>>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
>>> ---
>>>  .../kho/bindings/memblock/reserve_mem.yaml    | 41 ++++++++++++++++++
>>>  .../bindings/memblock/reserve_mem_map.yaml    | 42 +++++++++++++++++++
>>>  2 files changed, 83 insertions(+)
>>>  create mode 100644 Documentation/kho/bindings/memblock/reserve_mem.yaml
>>>  create mode 100644 Documentation/kho/bindings/memblock/reserve_mem_map.yaml
>>>
>>> diff --git a/Documentation/kho/bindings/memblock/reserve_mem.yaml b/Documentation/kho/bindings/memblock/reserve_mem.yaml
>>> new file mode 100644
>>> index 000000000000..7b01791b10b3
>>> --- /dev/null
>>> +++ b/Documentation/kho/bindings/memblock/reserve_mem.yaml
>>> @@ -0,0 +1,41 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/memblock/reserve_mem.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Memblock reserved memory
>>> +
>>> +maintainers:
>>> +  - Mike Rapoport <rppt@...nel.org>
>>> +
>>> +description: |
>>> +  Memblock can serialize its current memory reservations created with
>>> +  reserve_mem command line option across kexec through KHO.
>>> +  The post-KHO kernel can then consume these reservations and they are
>>> +  guaranteed to have the same physical address.
>>> +
>>> +examples:
>>> +  - |
>>> +    reserve_mem {
>>
>> Again, do not introduce own coding style.
>>
>> I don't understand why do you need this in the first place. There is
>> already reserved-memory block.
> 
> Because these regions are not "... designed for the special usage by
> various device drivers"

So you use now very similar name, but different with few letters just to
note that you do not fit into existing formats. If this does not fit
existing usage, then use different name.

>  and should not be exclude by the operating system
> from normal usage. 

Then it does not look like a reserved memory
>  
>> Best regards,
>> Krzysztof
> 


Best regards,
Krzysztof

Powered by blists - more mailing lists