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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bb863c8f-a92c-42d0-abc4-ff0b92f701c2@linux.microsoft.com>
Date: Wed, 12 Feb 2025 15:57:23 -0800
From: Roman Kisel <romank@...ux.microsoft.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: arnd@...db.de, bhelgaas@...gle.com, bp@...en8.de,
 catalin.marinas@....com, conor+dt@...nel.org, dave.hansen@...ux.intel.com,
 decui@...rosoft.com, haiyangz@...rosoft.com, hpa@...or.com,
 krzk+dt@...nel.org, kw@...ux.com, kys@...rosoft.com, lpieralisi@...nel.org,
 manivannan.sadhasivam@...aro.org, mingo@...hat.com, robh@...nel.org,
 ssengar@...ux.microsoft.com, tglx@...utronix.de, wei.liu@...nel.org,
 will@...nel.org, devicetree@...r.kernel.org, linux-arch@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-hyperv@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org, x86@...nel.org,
 benhill@...rosoft.com, bperkins@...rosoft.com, sunilmut@...rosoft.com
Subject: Re: [PATCH hyperv-next v4 4/6] dt-bindings: microsoft,vmbus: Add GIC
 and DMA coherence to the example



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@...ux.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@...000000: '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@...00000 {
>> +              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@...000000 {
>>                   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@...00000 {
-              compatible = "arm,gic-v3";
-              reg = <0x0 0xfe200000 0x0 0x10000>,   /* GIC Dist */
-                    <0x0 0xfe280000 0x0 0x200000>;  /* GICR */
-              interrupt-controller;
-              #interrupt-cells = <3>;
-            }
-
              vmbus@...000000 {
                  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

-- 
Thank you,
Roman


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ