Re: [PATCH V6 3/3] docs: Add documentation for generic virtio devices

[Viresh Kumar <viresh.kumar@xxxxxxxxxx>]

On 04-12-22, 20:52, Oleksandr Tyshchenko wrote:
> So as I understand current series adds support for two virtio devices
> (i2c/gpio) that require specific device-tree sub node with specific
> compatible in it [1]. Those backends are standalone userspace applications
> (daemons) that do not require any additional configuration parameters from
> the toolstack other than just virtio-mmio irq and base (please correct me if
> I am wrong).

For now, yes. But we may want to link these devices with other devices
in DT, like GPIO line consumers. I am not pushing a half informed
solution for that right now and that can be taken up later.

> Well, below just some thoughts (which might be wrong) regarding the possible
> extensions for future use. Please note, I do not suggest the following to be
> implemented right now (I mean within the context of current series):
> 
> 1. For supporting usual virtio devices that don't require specific
> device-tree sub node with specific compatible in it [2] we would probably
> need to either make "compatible" (or type?) string optional or to reserve
> some value for it ("common" for the instance).

I agree. Maybe we can use "virtio,device" without a number for the
device in this case.

> 2. For supporting Qemu based virtio devices we would probably need to add
> "backendtype" string (with "standalone" value for daemons like yours and
> "qemu" value for Qemu backends).

Hmm, I realize now that my patch did define a new type for this,
libxl_virtio_backend, which defines STANDALONE already, but it isn't
used currently. Maybe I should remove it too.

And I am not sure sure how to use these values, STANDALONE or QEMU.
Should the DT nodes be created only for STANDALONE and never for QEMU
?

Maybe we can add these fields and a config param, once someone wants
to reuse this stuff for QEMU ?

> 3. For supporting additional configuration parameters for Qemu based virtio
> devices we could probably reuse "device_model_args" (although it is not
> clear to me what alternative to use for daemons).

I would leave it for the person who will make use of this eventually,
as then we will have more information on the same.

> > +=item B<compatible=STRING>
> 
> Shouldn't it be "type" instead (the parsing code is looking for type and the
> example below suggests the type)?

Yes.

> > +Specifies the compatible string for the specific Virtio device. The same 
> > will be
> > +written in the Device Tree compatible property of the Virtio device. For
> > +example, "type=virtio,device22" for the I2C device > +
> > +=item B<transport=STRING>
> > +
> > +Specifies the transport mechanism for the Virtio device, like "mmio" or 
> > "pci".
> > +
> > +=back
> > +
> >   =item B<tee="STRING">
> >   B<Arm only.> Set TEE type for the guest. TEE is a Trusted Execution
> 
> Also the commit description for #1/3 mentions that Virtio backend could run
> in any domain. So looks like the "backend" string is missing here. I would
> add the following:
> 
> =item B<backend=domain-id>
> 
> Specify the backend domain name or id, defaults to dom0.

I haven't used the backend in any other domain for now, just Dom0, but
the idea is definitely there to run backends in separate user domains.

> P.S. I am wondering do i2c/gpio virtio backends support Xen grant mappings
> for the virtio?

Not yet, we haven't made much progress in that area until now, but it
is very much part of what we intend to do.

> Have you tried to run the backends in non-hardware domain
> with CONFIG_XEN_VIRTIO=y in Linux?

Not yet.

-- 
viresh