Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 3 May 2022 19:02:08 -0500
From:   Rob Herring <robh@kernel.org>
To:     Oleksandr <olekstysh@gmail.com>
Cc:     xen-devel@lists.xenproject.org,
        virtualization@lists.linux-foundation.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Jason Wang <jasowang@redhat.com>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        Julien Grall <julien@xen.org>, Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH V1 4/6] dt-bindings: Add xen,dev-domid property
 description for xen-grant DMA ops
Message-ID: <YnHCgBsQ90cJ58+0@robh.at.kernel.org>
References: <1650646263-22047-1-git-send-email-olekstysh@gmail.com>
 <1650646263-22047-5-git-send-email-olekstysh@gmail.com>
 <YnBUUclJqkvKsV2o@robh.at.kernel.org>
 <accbc6be-82c1-dfd2-586f-816141415d7c@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <accbc6be-82c1-dfd2-586f-816141415d7c@gmail.com>
Precedence: bulk

On Tue, May 03, 2022 at 08:09:32PM +0300, Oleksandr wrote:
> 
> On 03.05.22 00:59, Rob Herring wrote:
> 
> Hello Rob
> 
> 
> > On Fri, Apr 22, 2022 at 07:51:01PM +0300, Oleksandr Tyshchenko wrote:
> > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > > 
> > > Introduce Xen specific binding for the virtualized device (e.g. virtio)
> > > to be used by Xen grant DMA-mapping layer in the subsequent commit.
> > > 
> > > This binding indicates that Xen grant mappings scheme needs to be
> > > enabled for the device which DT node contains that property and specifies
> > > the ID of Xen domain where the corresponding backend resides. The ID
> > > (domid) is used as an argument to the grant mapping APIs.
> > > 
> > > This is needed for the option to restrict memory access using Xen grant
> > > mappings to work which primary goal is to enable using virtio devices
> > > in Xen guests.
> > > 
> > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > > ---
> > > Changes RFC -> V1:
> > >     - update commit subject/description and text in description
> > >     - move to devicetree/bindings/arm/
> > > ---
> > >   .../devicetree/bindings/arm/xen,dev-domid.yaml     | 37 ++++++++++++++++++++++
> > >   1 file changed, 37 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml b/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
> > > new file mode 100644
> > > index 00000000..ef0f747
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
> > > @@ -0,0 +1,37 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/arm/xen,dev-domid.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Xen specific binding for the virtualized device (e.g. virtio)
> > > +
> > > +maintainers:
> > > +  - Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > > +
> > > +select: true
> > Do we really need to support this property everywhere?
> 
> From my understanding - yes.
> 
> As, I think, any device node describing virtulized device in the guest
> device tree can have this property.? Initially (in the RFC series) the
> "solution to restrict memory access using Xen grant mappings" was
> virtio-specific.
> 
> Although the support of virtio is a primary target of this series, we
> decided to generalize this work and expand it to any device [1]. So the Xen
> grant mappings scheme (this property to be used for) can be theoretically
> used for any device emulated by the Xen backend.
> 
> 
> > > +
> > > +description:
> > > +  This binding indicates that Xen grant mappings scheme needs to be enabled
> > > +  for that device and specifies the ID of Xen domain where the corresponding
> > > +  device (backend) resides. This is needed for the option to restrict memory
> > > +  access using Xen grant mappings to work.
> > > +
> > > +properties:
> > > +  xen,dev-domid:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      The domid (domain ID) of the domain where the device (backend) is running.
> > > +
> > > +additionalProperties: true
> > > +
> > > +examples:
> > > +  - |
> > > +    virtio_block@3000 {
> > virtio@3000
> 
> ok, will change
> 
> 
> > 
> > > +            compatible = "virtio,mmio";
> > > +            reg = <0x3000 0x100>;
> > > +            interrupts = <41>;
> > > +
> > > +            /* The device is located in Xen domain with ID 1 */
> > > +            xen,dev-domid = <1>;
> > This fails validation:
> > 
> > Documentation/devicetree/bindings/arm/xen,dev-domid.example.dtb: virtio_block@3000: xen,dev-domid: [[1]] is not of type 'object'
> >          From schema: /home/rob/proj/git/linux-dt/Documentation/devicetree/bindings/virtio/mmio.yaml
> 
> Thank you for pointing this out, my fault, I haven't "properly" checked this
> before. I think, we need to remove "compatible = "virtio,mmio"; here

Uhh, no. That just means the example is incomplete. You need to add this 
property or reference this schema from virtio/mmio.yaml.


> diff --git a/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
> b/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
> index 2daa8aa..d2f2140 100644
> --- a/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
> +++ b/Documentation/devicetree/bindings/arm/xen,dev-domid.yaml
> @@ -28,7 +28,7 @@ additionalProperties: true
> ?examples:
> ?? - |
> ???? virtio_block@3000 {
> -??????????? compatible = "virtio,mmio";
> +??????????? /* ... */
> ???????????? reg = <0x3000 0x100>;
> ???????????? interrupts = <41>;
> 
> 
> 
> > 
> > The property has to be added to the virtio/mmio.yaml schema. If it is
> > not needed elsewhere, then *just* add the property there.
> 
> As I described above, the property is not virtio specific and can be used
> for any virtualized device for which Xen grant mappings scheme needs to be
> enabled (xen-grant DMA-mapping layer).

But that's a finite list of devices, right? In any case, you have to 
list the property anywhere it can be used.

Rob