On Mon, Sep 16, 2024 at 10:56:38AM +0200, Krzysztof Kozlowski wrote: > On 10/09/2024 08:13, Yunhong Jiang wrote: > > On Tue, Aug 27, 2024 at 01:45:49PM -0700, Yunhong Jiang wrote: > >> On Sun, Aug 25, 2024 at 09:10:01AM +0200, Krzysztof Kozlowski wrote: > >>> On Fri, Aug 23, 2024 at 04:23:20PM -0700, Yunhong Jiang wrote: > >>>> Add the binding to use mailbox wakeup mechanism to bringup APs. > >>>> > >>>> Signed-off-by: Yunhong Jiang <yunhong.jiang@xxxxxxxxxxxxxxx> > >>>> --- > >>>> .../devicetree/bindings/x86/wakeup.yaml | 64 +++++++++++++++++++ > >>>> 1 file changed, 64 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml > >>>> new file mode 100644 > >>>> index 000000000000..cb84e2756bca > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/x86/wakeup.yaml > >>>> @@ -0,0 +1,64 @@ > >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>> +# Copyright (C) 2024 Intel Corporation > >>>> +%YAML 1.2 > >>>> +--- > >>>> +$id: http://devicetree.org/schemas/x86/wakeup.yaml# > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>> + > >>>> +title: x86 mailbox wakeup > >>>> +maintainers: > >>>> + - Yunhong Jiang <yunhong.jiang@xxxxxxxxxxxxxxx> > >>>> + > >>>> +description: | > >>>> + The x86 mailbox wakeup mechanism defines a mechanism to let the bootstrap > >>>> + processor (BSP) to wake up application processors (APs) through a wakeup > >>>> + mailbox. > >>>> + > >>>> + The "wakeup-mailbox-addr" property specifies the wakeup mailbox address. The > >>>> + wakeup mailbox is a 4K-aligned 4K-size memory block allocated in the reserved > >>>> + memory. > >>>> + > >>>> + The wakeup mailbox structure is defined as follows. > >>>> + > >>>> + uint16_t command; > >>>> + uint16_t reserved; > >>>> + uint32_t apic_id; > >>>> + uint64_t wakeup_vector; > >>>> + uint8_t reservedForOs[2032]; > >>>> + > >>>> + The memory after reservedForOs field is reserved and OS should not touch it. > >>>> + > >>>> + To wakes up a AP, the BSP prepares the wakeup routine, fills the wakeup > >>>> + routine's address into the wakeup_vector field, fill the apic_id field with > >>>> + the target AP's APIC_ID, and write 1 to the command field. After receiving the > >>>> + wakeup command, the target AP will jump to the wakeup routine. > >>>> + > >>>> + For each AP, the mailbox can be used only once for the wakeup command. After > >>>> + the AP jumps to the wakeup routine, the mailbox will no longer be checked by > >>>> + this AP. > >>>> + > >>>> + The wakeup mailbox structure and the wakeup process is the same as > >>>> + the Multiprocessor Wakeup Mailbox Structure defined in ACPI spec version 6.5, > >>>> + section 5.2.12.19 [1]. > >>>> + > >>>> + References: > >>>> + > >>>> + [1] https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html > >>>> + > >>>> +select: false > >>> > >>> This schema is still a no-op because of this false. > >>> > >>> What is the point of defining one property if it is not placed anywhere? > >>> Every device node can have it? Seems wrong... > >>> > >>> You need to come with proper schema. Lack of an example is another thing > >>> - this cannot be even validated by the tools. > >>> > >>> Best regards, > >>> Krzysztof > > > > Hi, Krzysztof, I'm working to address your comments and have some questions. > > Hope to get help/guide from your side. > > > > For the select, the writing-schema.rst describes it as "A json-schema used to > > match nodes for applying the schema" but I'm a bit confused. In my case, should > > it be "cpus" node? Is there any code/tools that uses this property, so that I > > can have a better understanding? > > Usually we expect matching by compatible, but it does not seem suitable > here because it is not related to any specific device, right? That is > the problem with all this DT-reuse-for-virtual-stuff work. It just does > not follow usual expectations and guidelines - you do not describe a device. Thank you for the reply. I'm a bit confused on your "do not describe a device". I think VM is also a device, it's just a virtual device, but I don't see much difference of the virtual and physical device from DT point of view, possibly I missed some point. > > You can still match by nodes. See all top-level bindings. After checking the code at https://github.com/devicetree-org/dt-schema/blob/main/dtschema/validator.py, seems the 'select' is translated to 'if'/'then'. Do you have any example of "top-level bindings"? I tried to check binding for enable-methods like arm/cpu-enable-method/nuvoton,npcm750-smp or cpu/idle-states.yaml, but they are either not schema file, or quite different. I have been struggling on this device binding document for a while. I reconsidered what this binding is for. This binding means, if the cpus node has "enable-method" as "acpi-wakeup-mailbox", then the device should have property "wakeup-mailbox-addr" with uint64 type. In that case, I'm considering to set the "select" to be true so that it will apply to any potential device, and add if/then keyword to check the enable-method. But seems it does not work and I'm still trying to figure out the reason (I'm new to the json/json schema and is still learning). I received followed error: cpus: '#address-cells', '#size-cells', 'cpu@0', 'enable-method' do not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/x86/wakeup.yaml# cpu@0: 'device_type', 'reg' do not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/x86/wakeup.yaml# With the followed yaml file (I delete some description). $ cat Documentation/devicetree/bindings/x86/wakeup.yaml # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright (C) 2024 Intel Corporation %YAML 1.2 --- $id: http://devicetree.org/schemas/x86/wakeup.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# title: x86 mailbox wakeup maintainers: - Yunhong Jiang <yunhong.jiang@xxxxxxxxxxxxxxx> description: | ...... Removed to save space. properties: wakeup-mailbox-addr: $ref: /schemas/types.yaml#/definitions/uint32 description: | ...... Removed to save space. select: true if: properties: enable-method: contains: const: acpi-wakeup-mailbox required: - enable-method then: required: - wakeup-mailbox-addr additionalProperties: false examples: - | cpus { #address-cells = <1>; #size-cells = <0>; enable-method = "acpi-wakeup-mailbox"; wakeup-mailbox-addr = <0x1c000500>; cpu@0 { device_type = "cpu"; reg = <0x1>; }; }; ... > > > > > For your "validated by the tools", can you please share the tools you used to > > validate the schema? I used "make dt_binding_check" per the > > submitting-patches.rst but I think your comments is about another tool. > > See writing-schema document. Yes, I figured out in the end that the validate tools means the dt-schema tools. Thank you --jyh > > > Best regards, > Krzysztof > >
