Re: [PATCH v2 2/9] dt-bindings: x86: Add a binding for x86 wakeup mailbox

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/03/2025 06:51, Ricardo Neri wrote:
> On Tue, Mar 11, 2025 at 11:01:28AM +0100, Krzysztof Kozlowski wrote:
>> On 03/03/2025 23:21, Ricardo Neri wrote:
>>> On Fri, Sep 20, 2024 at 01:15:41PM +0200, Krzysztof Kozlowski wrote:
>>>
>>> [...]
>>>  
>>>> enable-method is part of CPUs, so you probably should match the CPUs...
>>>> I am not sure, I don't have the big picture here.
>>>>
>>>> Maybe if companies want to push more of bindings for purely virtual
>>>> systems, then they should first get involved more, instead of relying on
>>>> us. Provide reviews for your virtual stuff, provide guidance. There is
>>>> resistance in accepting bindings for such cases for a reason - I don't
>>>> even know what exactly is this and judging/reviewing based on my
>>>> practices will no be accurate.
>>>
>>> Hi Krzysztof,
>>>
>>> I am taking over this work from Yunhong.
>>>
>>> First of all, I apologize for the late reply. I will make sure
>>> communications are timely in the future.
>>>
>>> Our goal is to describe in the device tree a mechanism or artifact to boot
>>> secondary CPUs.
>>>
>>> In our setup, the firmware puts secondary CPUs to monitor a memory location
>>> (i.e., the wakeup mailbox) while spinning. From the boot CPU, the OS writes
>>> in the mailbox the wakeup vector and the ID of the secondary CPU it wants
>>> to boot. When a secondary CPU sees its own ID it will jump to the wakeup
>>> vector.
>>>
>>> This is similar to the spin-table described in the Device Tree
>>> specification. The key difference is that with the spin-table CPUs spin
>>> until a non-zero value is written in `cpu-release-addr`. The wakeup mailbox
>>> uses CPU IDs.
>>>
>>> You raised the issue of the lack of a `compatible` property, and the fact
>>> that we are not describing an actual device.
>>>
>>> I took your suggestion of matching by node and I came up with the binding
>>> below. I see these advantages in this approach:
>>>
>>>   * I define a new node with a `compatible` property.
>>>   * There is precedent: the psci node. In the `cpus` node, each cpu@n has
>>
> 
> Thanks for your feedback!
> 
>> psci is a standard. If you are documenting here a standard, clearly
>> express it and provide reference to the specification.
> 
> It is not really a standard, but this mailbox behaves indentically to the
> wakeup mailbox described in the ACPI spec [1]. I am happy reference the
> spec in the documentation of the binding... or describe in full the
> mechanism of mailbox without referring to ACPI. You had indicated you don't
> care about what ACPI does [2].

Behaving like ACPI and implementing a spec are two different things. The
question is whether you need to implement it like that and I believe
answer is: no.

> 
> In a nutshell, the wakeup mailbox is similar to the spin table used in ARM
> boards: it is reserved memory region that secondary CPUs monitor while
> spinning.
> 
>>
>>
>>>     an `enable-method` property that specify `psci`.
>>>   * The mailbox is a device as it is located in a reserved memory region.
>>>     This true regardless of the device tree describing bare-metal or
>>>     virtualized machines.
>>>
>>> Thanks in advance for your feedback!
>>>
>>> Best,
>>> Ricardo
>>>
>>> (only the relevant sections of the binding are shown for brevity)
>>>
>>> properties:
>>>   $nodename:
>>>     const: wakeup-mailbox
>>>
>>>   compatible:
>>>     const: x86,wakeup-mailbox
>>
>> You need vendor prefix for this particular device. If I pointed out lack
>> of device and specific compatible, then adding random compatible does
>> not solve it. I understand it solves for you, but not from the bindings
>> point of view.
> 
> I see. Platform firmware will implement the mailbox. It would not be any
> specific hardware from Intel. Perhaps `intel,wakeup-mailbox`?
> 
>>
>>>
>>>   mailbox-addr:
>>>     $ref: /schemas/types.yaml#/definitions/uint64
>>
>> So is this some sort of reserved memory?
> 
> Yes, the mailbox is located in reserved memory.


Then why reserved memory bindings are not working?

Anyway this was half a year ago. None of the emails are in my inbox.
None of the context is in my head.

It's the second or third email this month someone responds to my email
from 2024.

Frankly, that's a neat trick. I won't remember anything, but it would be
impolite to say just "no" without arguments. So now you will resend the
same code leading to the same discussions we had half a year ago. Or
ignoring that discussions.

I don't understand why this should be reviewers problem, so no, that's
just unfair.


Best regards,
Krzysztof