linux-kernel - Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250519175606.GA9693@ranerica-svr.sc.intel.com>
Date: Mon, 19 May 2025 10:56:06 -0700
From: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
To: Rob Herring <robh@...nel.org>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, x86@...nel.org,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	"K. Y. Srinivasan" <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
	Michael Kelley <mhklinux@...look.com>, devicetree@...r.kernel.org,
	Saurabh Sengar <ssengar@...ux.microsoft.com>,
	Chris Oo <cho@...rosoft.com>, linux-hyperv@...r.kernel.org,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
	Ricardo Neri <ricardo.neri@...el.com>
Subject: Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox
 for Intel processors

On Mon, May 19, 2025 at 10:29:37AM -0500, Rob Herring wrote:
> On Wed, May 14, 2025 at 08:53:38PM -0700, Ricardo Neri wrote:
> > On Wed, May 14, 2025 at 10:42:48AM -0500, Rob Herring wrote:
> > > On Tue, May 13, 2025 at 03:14:56PM -0700, Ricardo Neri wrote:
> > > > On Mon, May 12, 2025 at 10:32:24AM -0500, Rob Herring wrote:
> > > > > On Tue, May 06, 2025 at 08:23:39PM -0700, Ricardo Neri wrote:
> > > > > > On Tue, May 06, 2025 at 09:10:22AM +0200, Krzysztof Kozlowski wrote:
> > > > > > > On Mon, May 05, 2025 at 10:16:10PM GMT, Ricardo Neri wrote:
> > > > > > > > > If this is a device, then compatibles specific to devices. You do not
> > > > > > > > > get different rules than all other bindings... or this does not have to
> > > > > > > > > be binding at all. Why standard reserved-memory does not work for here?
> > > > > > > > > 
> > > > > > > > > Why do you need compatible in the first place?
> > > > > > > > 
> > > > > > > > Are you suggesting something like this?
> > > > > > > > 
> > > > > > > > reserved-memory {
> > > > > > > > 	# address-cells = <2>;
> > > > > > > > 	# size-cells = <1>;
> > > > > > > > 
> > > > > > > > 	wakeup_mailbox: wakeupmb@...000 {
> > > > > > > > 		reg = < 0x0 0xfff000 0x1000>
> > > > > > > > 	}
> > > > > > > > 
> > > > > > > > and then reference to the reserved memory using the wakeup_mailbox
> > > > > > > > phandle?
> > > > > > > 
> > > > > > > Yes just like every other, typical reserved memory block.
> > > > > > 
> > > > > > Thanks! I will take this approach and drop this patch.
> > > > > 
> > > > > If there is nothing else to this other than the reserved region, then 
> > > > > don't do this. Keep it like you had. There's no need for 2 nodes.
> > > > 
> > > > Thank you for your feedback!
> > > > 
> > > > I was planning to use one reserved-memory node and inside of it a child
> > > > node to with a `reg` property to specify the location and size of the
> > > > mailbox. I would reference to that subnode from the kernel code.
> > > > 
> > > > IIUC, the reserved-memory node is only the container and the actual memory
> > > > regions are expressed as child nodes.
> > > > 
> > > > I had it like that before, but with a `compatible` property that I did not
> > > > need.
> > > > 
> > > > Am I missing anything?
> > > 
> > > Without a compatible, how do you identify which reserved region is the 
> > > wakeup mailbox?
> > 
> > I thought using a phandle to the wakeup_mailbox. Then I realized that the
> > device nodes using the mailbox would be CPUs. They would need a `memory-
> > region` property. This does not look right to me.
> 
> That doesn't really make sense unless it's a memory region per CPU.

Agreed.

> 
> 
> > > Before you say node name, those are supposed to be 
> > > generic though we failed to enforce anything for /reserved-memory child 
> > > nodes.
> > 
> > I see. Thanks for preventing me from doing this.
> > 
> > Then the `compatible` property seems the way to go after all.
> > 
> > This what motivated this patch in the first place. On further analysis,
> > IIUC, defining bindings and schema is not needed, IMO, since the mailbox
> > is already defined in the ACPI spec. No need to redefine.
> 
> You lost me...
> 
> You don't need to redefine the layout of the memory region as that's 
> defined already somewhere,

Great!

> but you do need to define where it is for DT. 
> And for that, you need a compatible. Do you know where it is in this 
> case?

The compatible is not defined anywhere yet. Is a DT schema needed to
document it? If yes, I am usure what to put in the description. We tried
to not redefine the mailbox and refer to the ACPI spec. That was a NAK
from Krzysztof [1].

Thanks and BR,
Ricardo

[1]. https://lore.kernel.org/r/624e1985-7dd2-4abe-a918-78cb43556967@kernel.org