| 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
| ||
|
Message-ID: <ce19852f-8a69-3c98-cf0f-f78e37616fa6@quicinc.com>
Date: Wed, 2 Mar 2022 15:53:41 -0800
From: Elliot Berman <quic_eberman@...cinc.com>
To: Rob Herring <robh@...nel.org>
CC: Bjorn Andersson <bjorn.andersson@...aro.org>,
<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
Trilok Soni <quic_tsoni@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
"Andy Gross" <agross@...nel.org>, <linux-arm-msm@...r.kernel.org>,
Murali Nalajala <quic_mnalajal@...cinc.com>
Subject: Re: [PATCH 02/11] dt-bindings: Add binding for gunyah hypervisor
On 2/25/2022 12:01 PM, Rob Herring wrote:
> On Wed, Feb 23, 2022 at 03:37:20PM -0800, Elliot Berman wrote:
>> When Linux is booted as a guest under the Gunyah hypervisor, Gunyah
>> applies a devicetree overlay describing the virtual platform
>> configuration of the guest VM, such as the message queue capability IDs
>> for communicating with the Resource Manager. Add the DT bindings that
>> Gunyah adheres for the hypervisor node and message queues.
>>
>> Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
>> ---
>> .../bindings/gunyah/message-queue.yml | 100 ++++++++++++++
>> .../bindings/gunyah/qcom,hypervisor.yml | 122 ++++++++++++++++++
>
> How did testing these files work? It didn't because .yml files are
> ignored. Get 'make dt_binding_check' actually working and resubmit.
>
I'll double check this, thanks!
> No, you don't get your own directory.
>
Do you have a suggestion for an alternate directory? I'm not sure if
misc/ makes sense here?
> In general, DT is for undiscoverable hardware that we are stuck with
> because it was not made discoverable. As this is not h/w and you control
> each side of the interface, make it discoverable and don't use DT for
> your discovery mechanism.
>
Gunyah follows a micro-kernel architecture. The EL2 hypervisor doesn't
keep the necessary context to share initial message queue information in
order to communicate with the resource manager VM, and it's considered
undiscoverable in this regard.
This feedback is good for the other properties, though. I'll improve the
discoverability for other properties and drop the bindings for them.
In summary, I can remove all of the properties and subnodes from
/hypervisor node, except for /hypervisor/resource-manager-rpc. For that
node, I only need to keep a few fields (reg and interrupts) because that
information isn't discoverable.
> Incomplete review follows...
>
>> MAINTAINERS | 1 +
>> 3 files changed, 223 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/gunyah/message-queue.yml
>> create mode 100644 Documentation/devicetree/bindings/gunyah/qcom,hypervisor.yml
>>
>> diff --git a/Documentation/devicetree/bindings/gunyah/message-queue.yml b/Documentation/devicetree/bindings/gunyah/message-queue.yml
>> new file mode 100644
>> index 000000000000..1a96d3de2a19
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gunyah/message-queue.yml
>> @@ -0,0 +1,100 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/gunyah/qcom,hypervisor.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Gunyah message queue
>> +
>> +maintainers:
>> + - Murali Nalajala <quic_mnalajal@...cinc.com>
>> + - Elliot Berman <quic_eberman@...cinc.com>
>> +
>> +properties:
>> + compatible:
>> + items:
>> + - const: qcom,gunyah-message-queue
>> + - const: qcom,gunyah-capability
>
> I'm not following how capability is a fallback to message-queue.
>
This node shares common properties with other Gunyah capabilities. Linux
drivers don't need to make use of this compatible, but Gunyah needs to
make it present for other operating systems. In that case, should I drop
"gunyah-capability" from the bindings?
>> + peer:
>> + description: VMID of the VM on the other end of message queue
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + allOf:
>
> Check your indentation.
>
>> + - if:
>> + anyOf:
>> + - properties:
>> + qcom,is-sender: true
>> + - properties:
>> + qcom,is-full-duplex: true
>> + then:
>> + properties:
>> + qcom,tx-message-size:
>> + description: Maximum size in bytes of a message which can be sent by this queue
>> + $ref: /schemas/types.yaml#/definitions/int32
>> + qcom,tx-queue-depth:
>> + description: Depth of transmit queue for messages sent by this queue
>> + $ref: /schemas/types.yaml#/definitions/int32
>> + - if:
>> + anyOf:
>> + - properties:
>> + qcom,is-receiver: true
>> + - properties:
>> + qcom,is-full-duplex: true
>> + then:
>> + properties:
>> + qcom,rx-message-size:
>> + description: Maximum size in bytes of a message which can be received by this queue
>> + $ref: /schemas/types.yaml#/definitions/int32
>> + qcom,rx-queue-depth:
>> + description: Depth of transmit queue for messages received by this queue
>> + $ref: /schemas/types.yaml#/definitions/int32
>> + - if:
>> + anyOf:
>> + - properties:
>> + qcom,is-receiver: true
>> + - properties:
>> + qcom,is-sender: true
>> + then:
>> + properties:
>> + reg:
>> + description: Hypervisor capability ID of the message queue
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minItems: 1
>> + maxItems: 1
>> + interrupts:
>> + minItems: 1
>> + maxItems: 1
>> + - if:
>> + properties:
>> + qcom,is-full-duplex: true
>> + then:
>> + properties:
>> + reg:
>> + description:
>> + Hypervisor capability IDs of the message queue
>> + The first is tx side, the second is rx side
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minItems: 2
>> + maxItems: 2
>> + interrupts:
>> + description: The first is tx interrupt, second is rx interrupt
>> + minItems: 2
>> + maxItems: 2
>> + required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>> +
>> +examples:
>> + - |
>> + display-msgq-pair@...f0da3c3c965cc {
>> + compatible = "qcom,gunyah-message-queue", "qcom,gunyah-capability";
>> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>, /* TX full IRQ */
>> + <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>; /* RX empty IRQ */
>> + reg = <0x00000000 0x00000000>, <0x00000000 0x00000001>; /* TX, RX cap ids */
>> + qcom,is-full-duplex;
>> + qcom,tx-queue-depth = <8>;
>> + qcom,tx-message-size = <0xf0>;
>> + qcom,rx-queue-depth = <8>;
>> + qcom,rx-message-size = <0xf0>;
>> + };
>> \ No newline at end of file
>
> Fix this.
>
>> diff --git a/Documentation/devicetree/bindings/gunyah/qcom,hypervisor.yml b/Documentation/devicetree/bindings/gunyah/qcom,hypervisor.yml
>> new file mode 100644
>> index 000000000000..f637d51c52f0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gunyah/qcom,hypervisor.yml
>> @@ -0,0 +1,122 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/gunyah/qcom,hypervisor.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Hypervisor node to define virtual devices and other services provided by a Gunyah hypervisor
>> + to this virtual machine.
>> +
>> +maintainers:
>> + - Murali Nalajala <quic_mnalajal@...cinc.com>
>> + - Elliot Berman <quic_eberman@...cinc.com>
>> +
>> +description: |+
>> + On systems which support devicetree, Gunyah generates and overlays a deviceetree overlay which
>> + describes the basic configuration of the hypervisor. Virtual machines use this information for
>> + initial discovery that they are running as a Gunyah guest VM.
>> + See also: https://github.com/quic/gunyah-resource-manager/blob/develop/src/vm_creation/dto_construct.c
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - items:
>> + - const: qcom,gunyah-hypervisor-1.0
>> + - const: qcom,gunyah-hypervisor
>> +
>> + "#address-cells":
>> + description: Number of cells needed to represent 64-bit capability IDs.
>> + const: 2
>> + "#size-cells":
>> + description: must be 0, because capability IDs are not memory address
>> + ranges and do not have a size.
>> + const: 0
>> +
>> + qcom,gunyah-vm:
>> + type: object
>> + description:
>> + The VM Identification is a virtual node that conveys to the VM information
>> + about this virtual machine in the context of the hypervisor-based system
>> + properties:
>> + compatible:
>> + oneOf:
>> + - items:
>> + - const: qcom,gunyah-vm-id-1.0
>> + - const: qcom,gunyah-vm-id
>> + qcom,vendor:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description: Vendor of the Virtual Machine, e.g. Qualcomm
>
> Doesn't the compatible say this already?
>
We'll drop "qcom," vendor prefix.
>> + qcom,vmid:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: contains the VMID of this VM as a 32-bit value
>> + qcom,owner-vmid:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Contains the hypervisor VMID of the VM's owner. The owner
>> + is the VM that allocated and created the VM. VMs directly
>> + managed by the resource manager, such as the primary VM do
>> + not have an owner.
>> + required:
>> + - compatible
>> + - qcom,vmid
>> + - qcom,owner-vmid
>> +
>> +patternProperties:
>> + "^qcom,resource-manager-rpc(@.*)?":
>
> We don't use vendor prefixes in node names. QCom really liked to though.
>
We'll drop "qcom," vendor prefix.
>> + type: object
>> + description:
>> + Resource Manager node which is required to communicate to Resource
>> + Manager VM using Gunyah Message Queues.
>> + allOf: "message-queue.yml#"
>
> Not valid json-schema...
>
Will double check this in next patch.
>> +
>> + properties:
>> + compatible:
>> + oneOf:
>> + items:
>> + - const: qcom,resource-manager-1-0
>> + - const: qcom,resource-manager
>> + qcom,console-dev:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description: if set, the resource-manger will accept console logs from the VM
>> + qcom,free-irq-start:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Set on ARM systems which use a GIC. First VIRQ number which is free
>> + for virtual interrupt use.
>> + required:
>> + - qcom,is-full-duplex
>> +
>> +
>> +required:
>> +- compatible
>> +- "#address-cells"
>> +- "#size-cells"
>> +
>> +examples:
>> + - |
>> + hypervisor {
>> + #address-cells = <2>;
>> + #size-cells = <0>;
>> + compatible = "qcom,gunyah-hypervisor-1.0", "qcom,gunyah-hypervisor", "simple-bus";
>> + name = "hypervisor";
>> +
>> + qcom,gunyah-vm {
>> + compatible = "qcom,gunyah-vm-id-1.0", "qcom,gunyah-vm-id";
>> + qcom,vendor = "Qualcomm Technologies, Inc.";
>> + qcom,vmid = <45>;
>> + qcom,owner-vmid = <3>;
>> + };
>> +
>> + qcom,resource-manager-rpc@...0000000000001 {
>> + compatible = "qcom,resource-manager-1-0", "qcom,resource-manager",
>> + "qcom,gunyah-message-queue", "qcom,gunyah-capability";
>> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>, /* TX full IRQ */
>> + <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>; /* RX empty IRQ */
>> + reg = <0x00000000 0x00000000>, <0x00000000 0x00000001>;
>> + /* TX, RX cap ids */
>> + qcom,is-full-duplex;
>> + qcom,free-irq-start = <0>;
>> + qcom,tx-queue-depth = <8>;
>> + qcom,tx-message-size = <0xf0>;
>> + qcom,rx-queue-depth = <8>;
>> + qcom,rx-message-size = <0xf0>;
>> + };
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index bed175adc4c3..6a918f653eac 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8400,6 +8400,7 @@ M: Elliot Berman <quic_eberman@...cinc.com>
>> M: Murali Nalajala <quic_mnalajal@...cinc.com>
>> L: linux-arm-msm@...r.kernel.org
>> S: Maintained
>> +F: Documentation/devicetree/bindings/gunyah/
>> F: Documentation/virt/gunyah/
>>
>> H8/300 ARCHITECTURE
>> --
>> 2.25.1
>>
>>
Powered by blists - more mailing lists