On Wed, Mar 19, 2025 at 08:52:37AM +0100, Krzysztof Kozlowski wrote: > 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. > Thank you again for your feedback. I will send a third version of the patchset. This will give the full context. I will incorporate your feedback in such version. Thanks and BR, Ricardo
