On Tue, 2023-01-10 at 15:43 +0000, Joao Martins wrote:
> On 10/01/2023 12:37, David Woodhouse wrote:
> > Some parts of it are relatively straightforward; others less so. In
> > particular, it looks really hard to provide a contiguous virtual mapping
> > of multiple potentially discontiguous pages granted by the guest. To
> > fix that we might actually need the guest memory blocks to be backed
> > by real files (perhaps deleted or shmem) in order that they can be mapped
> > again in at a different virtual address. 
> 
> I wonder if we really need to go to that extent.
> 
> As far as Qemu emulated-Xen is concerned, gref is mostly a different lookup
> mechanism a GPA i.e. an index on a table (that the VMM knows about) that
> references a GPA. Perhaps if it's simpler to teach the backends to deal with
> discontiguous grefs (if there's such a case today even).

We thought about making the back ends deal with discontiguous grefs by
returning an array[nr_refs] of pointers to each one.

The problem is that in some cases the ring entries don't even land on
page boundaries, leaving one entry crossing from one page to the next.
For example the 32-bit blkif response is 12 bytes, which means it lines
up at the end of the *first* page (64-byte ring header + 336 @12 byte
entries == 4096) but it won't line up at the end of the *second* page.

Which is still not insurmountable, but it's kind of awful.

> The only user of multi-gref mapping is the block xen driver ... and only for
> mapping the shared ring if I understood correctly. But even there you could
> probably twist it... considering that the multi-gref ring is contiguous is guest
> address space, thus the gref -> HVA conversion ius contiguous too (?). So that
> way you could still return a single HVA (provided that map-grant implementation
> validates the backing frame contiguosity).

There's no reason a guest couldn't vmalloc its multi-page ring and have
a virtually contiguous view for itself, while the actual GPAs are
discontiguous. Sure, that's probably not the common case, but I didn't
really want to have a multi-page mapping succeed *sometimes*.

> > So for now we'll limit the
> > back ends to mapping a single grant ref at a time.
>
> Which is not a practical limitation right now.

That sentence can be read both ways. I first read it as "that
limitation is impractical". But prefer to read it as "that doesn't
limit us in practice", and I *think* the latter is what you meant from
the rest of the paragraph...

>  One grant ref is actually fine
> for the everything else that is not the block shared-ring.  Xen devices in Qemu
> seem to be using the legacy backend interface, and thus (un)mapping one grant at
> a time, or otherwise copying grefs.

Right. We can absolutely live with that limitation for now; we'll teach
xen-block not to advertise a higher max-ring-page-order if ring_order
if the MAP_MULTIPLE feature isn't set.

And I absolutely don't give a crap about 9p or usb just yet. :)

> > https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xenfv-kvm-backends-1
> > 
> Cool stuff! A lot better than the RFC redirection layer
> 
> > David Woodhouse (14):
> >       hw/xen: Remove old version of Xen headers
> 
> This patch looks more appropriate to your earlier v6 (?)

I would have liked that but it depends on a bit on the header
untangling that happens in between. As we introduce the backend ops
structures, rename xen_common.h → xen_native.h and remove that from the
C files that don't need it, we subtly fix things such that this final
patch removing the old headers makes it *look* easy.