Re: [PATCH 02/11] dt-bindings: Add binding for gunyah hypervisor

From: Elliot Berman
Date: Wed Mar 02 2022 - 18:53:47 EST



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@xxxxxxxxxxx>
---
.../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@xxxxxxxxxxx>
+ - Elliot Berman <quic_eberman@xxxxxxxxxxx>
+
+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@abbf0da3c3c965cc {
+ 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@xxxxxxxxxxx>
+ - Elliot Berman <quic_eberman@xxxxxxxxxxx>
+
+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@0000000000000001 {
+ 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@xxxxxxxxxxx>
M: Murali Nalajala <quic_mnalajal@xxxxxxxxxxx>
L: linux-arm-msm@xxxxxxxxxxxxxxx
S: Maintained
+F: Documentation/devicetree/bindings/gunyah/
F: Documentation/virt/gunyah/
H8/300 ARCHITECTURE
--
2.25.1