Re: [QEMU][PATCHv2 0/8] Xen: support grant mappings.

[Juergen Gross]

On 26.10.23 22:56, Stefano Stabellini wrote:
> On Thu, 26 Oct 2023, David Woodhouse wrote:
> > On Thu, 2023-10-26 at 13:36 -0700, Stefano Stabellini wrote:
> > > > This seems like a lot of code to replace that simpler option... is
> > > > there a massive performance win from doing it this way? Would we want
> > > > to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
> > > > make sense to introduce the simple version and *then* the optimisation,
> > > > with some clear benchmarking to show the win?
> > >
> > > This is not done for performance but for safety (as in safety
> > > certifications, ISO 26262, etc.). This is to enable unprivileged virtio
> > > backends running in a DomU. By unprivileged I mean a virtio backend that
> > > is unable to map arbitrary memory (the xenforeignmemory interface is
> > > prohibited).
> > >
> > > The goal is to run Xen on safety-critical systems such as cars,
> > > industrial robots and more. In this configuration there is no
> > > traditional Dom0 in the system at all. If you  would like to know more:
> > > https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8
> >
> > Yeah, I understand why we're using grant mappings instead of just
> > directly having access via foreignmem mappings. That wasn't what I was
> > confused about.
> >
> > What I haven't worked out is why we're implementing this through an
> > automatically-populated MemoryRegion in QEMU, rather than just using
> > grant mapping ops like we always have.
> >
> > It seems like a lot of complexity just to avoid calling
> > qemu_xen_gnttab_map_refs() from the virtio backend.
>
> I think there are two questions here. One question is "Why do we need
> all the new grant mapping code added to xen-mapcache.c in patch #7?
> Can't we use qemu_xen_gnttab_map_refs() instead?"

The main motivation was to _avoid_ having to change all the backends.

My implementation enables _all_ qemu based virtio backends to use grant
mappings. And if a new backend is added to qemu, there will be no change
required to make it work with grants.

> And the second question is where to call the grant mapping from (whether
> the new code or the old qemu_xen_gnttab_map_refs code it doesn't
> matter). It could be even simpler than calling it from the virtio
> backends: we could just call it from the existing qemu_ram_ptr_length()
> hook. See this discussion:
> https://marc.info/?l=qemu-devel&m=169828434927778

I wanted to be explicit where and when the mapping and unmapping are
happening. Adding the mapping to qemu_ram_ptr_length() would be probably
possible, but it would be quite hard to verify that no double mappings
are happening. And I had problems with that approach when qemu was setting
up the ring page access.

Adding a map() and an unmap() hook seemed to be the cleanest solution, even
if it needs more code churn. The qemu_ram_ptr_length() approach seemed to be
more like a hack than a solution.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature