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

Krzysztof Kozlowski
2025-05-04T16:51:20Z

On Sat, May 03, 2025 at 12:15:08PM GMT, Ricardo Neri wrote:
> Add DeviceTree bindings for the wakeup mailbox used on Intel processors.
>

Start using b4, so your cover letter will have proper lore links to
previous versions.

> x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert
> followed by Start-Up IPI messages. The wakeup mailbox can be used when this
> mechanism unavailable.
>
> The wakeup mailbox offers more control to the operating system to boot
> secondary CPUs than a spin-table. It allows the reuse of same wakeup vector
> for all CPUs while maintaining control over which CPUs to boot and when.
> While it is possible to achieve the same level of control using a spin-
> table, it would require to specify a separate cpu-release-addr for each
> secondary CPU.
>
> Originally-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>

What does this tag mean? Why you cannot use standard tags - SoB and
co-developed?

> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v2:
> - Implemented the mailbox as a reserved-memory node. Add to it a
> `compatible` property. (Krzysztof)
> - Explained the relationship between the mailbox and the `enable-mehod`
> property of the CPU nodes.
> - Expanded the documentation of the binding.
>
> Changes since v1:
> - Added more details to the description of the binding.
> - Added requirement a new requirement for cpu@N nodes to add an
> `enable-method`.
> ---
> .../reserved-memory/intel,wakeup-mailbox.yaml | 87 +++++++++++++++++++
> 1 file changed, 87 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
>
> diff --git a/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> new file mode 100644
> index 000000000000..d97755b4673d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/intel,wakeup-mailbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wakeup Mailbox for Intel processors
> +
> +description: |
> + The Wakeup Mailbox provides a mechanism for the operating system to wake up
> + secondary CPUs on Intel processors. It is an alternative to the INIT-!INIT-
> + SIPI sequence used on most x86 systems.
> +
> + Firmware must define the enable-method property in the CPU nodes as
> + "intel,wakeup-mailbox" to use the mailbox.
> +
> + Firmware implements the wakeup mailbox as a 4KB-aligned memory region of size
> + of 4KB. It is memory that the firmware reserves so that each secondary CPU can
> + have the operating system send a single message to them. The firmware is
> + responsible for putting the secondary CPUs in a state to check the mailbox.
> +
> + The structure of the mailbox is as follows:
> +
> + Field Byte Byte Description
> + Length Offset
> + ------------------------------------------------------------------------------
> + Command 2 0 Command to wake up the secondary CPU:
> + 0: Noop
> + 1: Wakeup: Jump to the wakeup_vector
> + 2-0xFFFF: Reserved:
> + Reserved 2 2 Must be 0.
> + APIC_ID 4 4 APIC ID of the secondary CPU to wake up.
> + Wakeup_Vector 8 8 The wakeup address for the secondary CPU.
> + ReservedForOs 2032 16 Reserved for OS use.
> + ReservedForFW 2048 2048 Reserved for firmware use.
> + ------------------------------------------------------------------------------
> +
> + To wake up a secondary CPU, the operating system 1) prepares the wakeup
> + routine; 2) populates the address of the wakeup routine address into the
> + Wakeup_Vector field; 3) populates the APIC_ID field with the APIC ID of the
> + secondary CPU; 4) writes Wakeup in the Command field. Upon receiving the
> + Wakeup command, the secondary CPU acknowledges the command by writing Noop in
> + the Command field and jumps to the Wakeup_Vector. The operating system can
> + send the next command only after the Command field is changed to Noop.
> +
> + The secondary CPU will no longer check the mailbox after waking up. The
> + secondary CPU must ignore the command if its APIC_ID written in the mailbox
> + does not match its own.
> +
> + When entering the Wakeup_Vector, interrupts must be disabled and 64-bit
> + addressing mode must be enabled. Paging mode must be enabled. The virtual
> + address of the Wakeup_Vector page must be equal to its physical address.
> + Segment selectors are not used.
> +
> +maintainers:
> + - Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> +
> +allOf:
> + - $ref: reserved-memory.yaml
> +
> +properties:
> + compatible:
> + const: intel,wakeup-mailbox

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?

> +
> + alignment:
> + description: The mailbox must be 4KB-aligned.

Then drop it because it is implied by compatible.

> + const: 0x1000
> +
> +required:
> + - compatible
> + - alignment
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <1>;
> +
> + wakeup-mailbox@12340000 {

Please use real addresses from real DT.

> + compatible = "intel,wakeup-mailbox";
> + alignment = <0x1000>;
> + reg = <0x0 0x12340000 0x1000>;

reg is always the second property. See DTS coding style.

Best regards,
Krzysztof