| Message ID | 20250212014321.1108840-5-romank@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | arm64: hyperv: Support Virtual Trust Level Boot | expand |
On Tue, Feb 11, 2025 at 05:43:19PM -0800, Roman Kisel wrote: > The existing example lacks the GIC interrupt controller property > making it not possible to boot on ARM64, and it lacks the DMA GIC controller is not relevant to this binding. > coherence property making the kernel do more work on maintaining > CPU caches on ARM64 although the VMBus trancations are cache-coherent. > > Add the GIC node, specify DMA coherence, and define interrupt-parent > and interrupts properties in the example to provide a complete reference > for platforms utilizing GIC-based interrupts, and add the DMA coherence > property to not do extra work on the architectures where DMA defaults to > non cache-coherent. > > Signed-off-by: Roman Kisel <romank@linux.microsoft.com> > --- > .../devicetree/bindings/bus/microsoft,vmbus.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) Last time I said: not tested by automation. Now: I see automation build failures, although I do not see anything incorrect in the code, so that's a bit surprising. Please confirm that binding was tested on latest dtschema. > > diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml > index a8d40c766dcd..5ec69226ab85 100644 > --- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml > +++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml > @@ -44,11 +44,22 @@ examples: > #size-cells = <1>; > ranges; > > + gic: intc@fe200000 { > + compatible = "arm,gic-v3"; > + reg = <0x0 0xfe200000 0x0 0x10000>, /* GIC Dist */ > + <0x0 0xfe280000 0x0 0x200000>; /* GICR */ > + interrupt-controller; > + #interrupt-cells = <3>; > + } I fail to see how this is relevant here. This is example only of vmbus. Look how other bindings are done. Drop the example. > + > vmbus@ff0000000 { > compatible = "microsoft,vmbus"; > #address-cells = <2>; > #size-cells = <1>; > ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>; > + dma-coherent; > + interrupt-parent = <&gic>; > + interrupts = <1 2 1>; Use proper defines for known constants. Best regards, Krzysztof
On 2/11/2025 10:42 PM, Krzysztof Kozlowski wrote: > On Tue, Feb 11, 2025 at 05:43:19PM -0800, Roman Kisel wrote: >> The existing example lacks the GIC interrupt controller property >> making it not possible to boot on ARM64, and it lacks the DMA > > GIC controller is not relevant to this binding. > Will remove, thank you for pointing that out! >> coherence property making the kernel do more work on maintaining >> CPU caches on ARM64 although the VMBus trancations are cache-coherent. >> >> Add the GIC node, specify DMA coherence, and define interrupt-parent >> and interrupts properties in the example to provide a complete reference >> for platforms utilizing GIC-based interrupts, and add the DMA coherence >> property to not do extra work on the architectures where DMA defaults to >> non cache-coherent. >> >> Signed-off-by: Roman Kisel <romank@linux.microsoft.com> >> --- >> .../devicetree/bindings/bus/microsoft,vmbus.yaml | 11 +++++++++++ >> 1 file changed, 11 insertions(+) > > Last time I said: not tested by automation. > Now: I see automation build failures, although I do not see anything > incorrect in the code, so that's a bit surprising. Please confirm that > binding was tested on latest dtschema. They weren't for which I am sorry. Read through https://www.kernel.org/doc/html/v6.14-rc2/devicetree/bindings/writing-schema.html and was able to see and fix the break by bringing the YAML to [1]. Getting now this /Documentation/devicetree/bindings/bus/microsoft,vmbus.example.dtb: vmbus@ff0000000: 'dma-coherent', 'interrupts' do not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/bus/microsoft,vmbus.yaml# so maybe I need to add some more to the "requires" section. Will follow other examples as you suggested. > >> >> diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml >> index a8d40c766dcd..5ec69226ab85 100644 >> --- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml >> +++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml >> @@ -44,11 +44,22 @@ examples: >> #size-cells = <1>; >> ranges; >> >> + gic: intc@fe200000 { >> + compatible = "arm,gic-v3"; >> + reg = <0x0 0xfe200000 0x0 0x10000>, /* GIC Dist */ >> + <0x0 0xfe280000 0x0 0x200000>; /* GICR */ >> + interrupt-controller; >> + #interrupt-cells = <3>; >> + } > > I fail to see how this is relevant here. This is example only of vmbus. > Look how other bindings are done. Drop the example. The bus refers to the interrupt controller, and I didn't have it, so added it :) Now I in other examples that is not required, and the tooling generates fake intc's. Appreciate your advice very much! > > >> + >> vmbus@ff0000000 { >> compatible = "microsoft,vmbus"; >> #address-cells = <2>; >> #size-cells = <1>; >> ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>; >> + dma-coherent; >> + interrupt-parent = <&gic>; >> + interrupts = <1 2 1>; > > Use proper defines for known constants. Will do as in [1], thank you! > [1] --- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml +++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml @@ -28,6 +28,7 @@ properties: required: - compatible - ranges + - interrupts - '#address-cells' - '#size-cells' @@ -35,6 +36,8 @@ additionalProperties: false examples: - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> soc { #address-cells = <2>; #size-cells = <1>; @@ -44,14 +47,6 @@ examples: #size-cells = <1>; ranges; - gic: intc@fe200000 { - compatible = "arm,gic-v3"; - reg = <0x0 0xfe200000 0x0 0x10000>, /* GIC Dist */ - <0x0 0xfe280000 0x0 0x200000>; /* GICR */ - interrupt-controller; - #interrupt-cells = <3>; - } - vmbus@ff0000000 { compatible = "microsoft,vmbus"; #address-cells = <2>; @@ -59,7 +54,7 @@ examples: ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>; dma-coherent; interrupt-parent = <&gic>; - interrupts = <1 2 1>; + interrupts = <GIC_PPI 2 IRQ_TYPE_EDGE_RISING>; }; }; }; > Best regards, > Krzysztof
On 2/12/2025 3:57 PM, Roman Kisel wrote: > [...] Thank you for your guidance!! The below passes tests and addresses the feedback you have provided. If no further comments from you, I'll send the file in this form in the next version of the patch series (also fixing the commit title and description). # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 --- $id: http://devicetree.org/schemas/bus/microsoft,vmbus.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# title: Microsoft Hyper-V VMBus maintainers: - Saurabh Sengar <ssengar@linux.microsoft.com> description: VMBus is a software bus that implement the protocols for communication between the root or host OS and guest OSs (virtual machines). properties: compatible: const: microsoft,vmbus ranges: true '#address-cells': const: 2 '#size-cells': const: 1 required: - compatible - ranges - interrupts - '#address-cells' - '#size-cells' additionalProperties: true examples: - | #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/interrupt-controller/arm-gic.h> soc { #address-cells = <2>; #size-cells = <1>; bus { compatible = "simple-bus"; #address-cells = <2>; #size-cells = <1>; ranges; vmbus@ff0000000 { compatible = "microsoft,vmbus"; #address-cells = <2>; #size-cells = <1>; ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>; dma-coherent; interrupt-parent = <&gic>; interrupts = <GIC_PPI 2 IRQ_TYPE_EDGE_RISING>; }; }; }; >> Best regards, >> Krzysztof >
diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml index a8d40c766dcd..5ec69226ab85 100644 --- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml +++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml @@ -44,11 +44,22 @@ examples: #size-cells = <1>; ranges; + gic: intc@fe200000 { + compatible = "arm,gic-v3"; + reg = <0x0 0xfe200000 0x0 0x10000>, /* GIC Dist */ + <0x0 0xfe280000 0x0 0x200000>; /* GICR */ + interrupt-controller; + #interrupt-cells = <3>; + } + vmbus@ff0000000 { compatible = "microsoft,vmbus"; #address-cells = <2>; #size-cells = <1>; ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>; + dma-coherent; + interrupt-parent = <&gic>; + interrupts = <1 2 1>; }; }; };
The existing example lacks the GIC interrupt controller property making it not possible to boot on ARM64, and it lacks the DMA coherence property making the kernel do more work on maintaining CPU caches on ARM64 although the VMBus trancations are cache-coherent. Add the GIC node, specify DMA coherence, and define interrupt-parent and interrupts properties in the example to provide a complete reference for platforms utilizing GIC-based interrupts, and add the DMA coherence property to not do extra work on the architectures where DMA defaults to non cache-coherent. Signed-off-by: Roman Kisel <romank@linux.microsoft.com> --- .../devicetree/bindings/bus/microsoft,vmbus.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+)