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

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On 05.12.22 11:11, Viresh Kumar wrote:


Hello Viresh

> 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.
I got it, ok.

> > 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.

Fine with me.


> > 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
> ?
If we expose virtio-mmio device to the guest via device-tree on Arm, 
then I think the DT nodes should be always created here, no matter where 
the corresponding virtio backend is located itself (either STANDALONE or 
QEMU).
> Maybe we can add these fields and a config param, once someone wants
> to reuse this stuff for QEMU ?

I don't know what to suggest here, sorry.

On the one hand, it is an extra work for you trying to add functionality 
you don't need at the moment. On the other hand if we add "backendtype" 
config param right now with default to STANDALONE it might simplify work 
for someone who ends up adding other type (in particular, the QEMU). 
Let's see what the maintainers will say.


> > 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.
Sure, these are just thoughts for now.

> > > +=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.

ok, good. My point is the following: if backend domain is configurable 
then it should be documented here.
> > 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.

Thanks for the information.

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

>