Re: [PATCH] libxl: arm: Allow grant mappings for backends running on Dom0

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, May 05, 2023 at 03:08:35PM +0530, Viresh Kumar wrote:
> Hi Anthony,
> 
> On 02-05-23, 15:44, Anthony PERARD wrote:
> > > diff --git a/tools/libs/light/libxl_virtio.c 
> > > b/tools/libs/light/libxl_virtio.c
> > > index faada49e184e..e1f15344ef97 100644
> > > --- a/tools/libs/light/libxl_virtio.c
> > > +++ b/tools/libs/light/libxl_virtio.c
> > > @@ -48,11 +48,13 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, 
> > > uint32_t domid,
> > >      flexarray_append_pair(back, "base", GCSPRINTF("%#"PRIx64, 
> > > virtio->base));
> > >      flexarray_append_pair(back, "type", GCSPRINTF("%s", virtio->type));
> > >      flexarray_append_pair(back, "transport", GCSPRINTF("%s", transport));
> > > +    flexarray_append_pair(back, "forced_grant", GCSPRINTF("%u", 
> > > virtio->forced_grant));
> > >  
> > >      flexarray_append_pair(front, "irq", GCSPRINTF("%u", virtio->irq));
> > >      flexarray_append_pair(front, "base", GCSPRINTF("%#"PRIx64, 
> > > virtio->base));
> > >      flexarray_append_pair(front, "type", GCSPRINTF("%s", virtio->type));
> > >      flexarray_append_pair(front, "transport", GCSPRINTF("%s", 
> > > transport));
> > > +    flexarray_append_pair(front, "forced_grant", GCSPRINTF("%u", 
> > > virtio->forced_grant));
> > 
> > This "forced_grant" feels weird to me in the protocol, I feel like this
> > use of grant or not could be handled by the backend. For example in
> > "blkif" protocol, there's plenty of "feature-*" which allows both
> > front-end and back-end to advertise which feature they can or want to
> > use.
> > But maybe the fact that the device tree needs to be modified to be able
> > to accommodate grant mapping means that libxl needs to ask the backend to
> > use grant or not, and the frontend needs to now if it needs to use
> > grant.
> 
> I am not sure if I fully understand what you are suggesting here.

I guess the way virtio devices are implemented in libxl suggest to me
that the are just Xen PV devices. So I guess some documentation in the
tree would be useful, maybe some comments in libxl_virtio.c.

> The eventual fronend drivers (like drivers/i2c/busses/i2c-virtio.c)
> aren't Xen aware and the respective virtio protocol doesn't talk about
> how memory is mapped for the guest. The guest kernel allows both
> memory mapping models and the decision is made based on the presence
> or absence of the iommu node in the DT.

So, virtio's frontend don't know about xenstore? In this case, there's
no need to have all those nodes in xenstore under the frontend path.

I guess the nodes for the backends are at least somewhat useful for
libxl to reload the configuration of the virtio device. But even that
isn't probably useful if we can't hot-plug or hot-unplug virtio devices.

Are the xenstore node for the backend actually been used by a virtio
backend?

Cheers,

-- 
Anthony PERARD