Re: [PATCH V1 4/6] dt-bindings: Add xen,dev-domid property description for xen-grant DMA ops

[Rob Herring]

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@xxxxxxxx>
> > > 
> > > 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@xxxxxxxx>
> > > ---
> > > 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@xxxxxxxx>
> > > +
> > > +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