Re: [Xen-devel] [PATCH] Paging and memory sharing for HVM guests

[Grzegorz Milos <gm281@xxxxxxxxx>]

> An overview description of the design would be nice, to have a basic
> understanding before looking at the individual patches. In particular,
> from a first brief look, I'm having the impression that only HVM guests'
> pages can be subject to paging.

Indeed. Paging and memory sharing is designed for HVM guests only. We
are compiling proper documentation for it, but we'll try to write a
short(er) overview too.

> On the Linux patches:
>
> Introducing another bogus failure indicator for the mmap_batch
> privcmd operations seems rather undesirable - we'll already need to
> find a backwards-compatible solution to the current (broken) or-ing
> in of 0xf0000000 (broken because MFNs can now be more than
> 28 bits wide).

Surly you mean > 30 bits wide?
Anyway, I'll let Patrick comment on that, since he is the author of
this bit of the code.

> Using msleep() with hard-coded values (in at least one case even
> contradicting the accompanying comment) seems more like a hack
> than a permanent solution. Can't there be some signaling done, or
> can't there alternatively be a polling hypercall?

We intent do use proper signalling for EAGAIN failed grant maps at
least. For example, you'll notice that I've separated the block
backend patch into two, with the
'mem_sharing_blkback_periodic_retry.patch' implementing a temporary
retry loop. Once we've got memory event interface ready, we'll replace
the loop with a mem event 'kick'. In some cases though, a simple retry
loop seem fine to me (e.g. grant maps of ring pages).

WRT to a catch-all retry loop for 'direct' foreign mappings, the idea
is that we'll provide a separate non-blocking call (which may fail
with EAGAIN) for the callers who care about performance. For the time
being a single retry loop was the quickest way to get the code out for
people to test/comment on it.

> Removing support for IOCTL_PRIVCMD_MMAP from the pv-ops
> implementation seems pretty unrelated, so should probably be a
> separate patch.

Forwarding this Q to Patrick again.

> Also, most of the patches seem to use blanks instead of tabs for
> indentation, and occasionally other non-standard formatting.

Mea culpa. I tend to have 'tab expansion' set in my editor. I'll fix
whitespace in my linux kernel patches.

Thanks
Gregor

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel