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

[Patrick Colp <pjcolp@xxxxxxxxx>]

Jan Beulich wrote:
> > > > Patrick Colp <pjcolp@xxxxxxxxx> 17.12.09 18:05 >>>
> > Jan Beulich wrote:
> > > > > > Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 17.12.09 17:38 >>>
> > > > 1). The  "*mfnp |= 0x80000000U;" and "*mfnp |= 0xf0000000U;" should
> > > >    use a #define. Maybe copy over the #defines from the xen tree ?
> > > Did you find any defines in the tools sources for that? The only place I
> > > found this condition being checked at all was in xc_map_foreign_pages(),
> > > where it used hard-coded values. Or are you referring to the
> > > XEN_DOMCTL_PFINFO_* values? I'd say they're being mis-used when
> > > applied to the mfn array used by mmap-batch (including apparent
> > > pre-existing uses).
> > I think many people agree that this idea of using 0xf0000000 is a very 
> > inelegant solution (myself included). Maybe now would be a good time to 
> > hash out what the proper way to deal with this is. I think this discussion 
> > should extend to the privcmd/libxc mmap interfaces generally as well.
> Yes, I fully agree. While I had cooked together a patch to make the
> tools auto-detect whether the underlying (64-bit) kernel uses a
> sufficiently wide mask as error indicator (and a Linux side patch to
> actually make the kernel behaviorally match the pseudo-documentation
> in privcmd.h ("array of mfns - top nibble set on err"), I meanwhile
> realized that this would break older tools running on a fixed kernel
> (a combination I suppose to be valid).
>
> The only way I can see to solve this is a new ioctl, and if we need a
> new ioctl anyway, we can as well design it properly. The question
> however is what the best way for an error indication is: Fully over-
> writing the passed in MFNs, an extra bit vector passed in (does user
> mode have a sufficiently common interface for dealing with bit fields,
> like the kernel's bitops.h?), or yet something else?
A new ioctl seems like the reasonable approach. And as you, say if we're 
going to have a new ioctl, then let's do it right. When calling 
xc_map_foreign_batch(), is there any requirement that the pfns you pass in 
are contiguous (ordering incrementally)? If not, then I think completely 
over-writing the MFNs is probably the wrong thing to do, as it requires 
callers to keep two copies of the array to find out which pages didn't map. 
I would be more in favour of returning a bit vector. As far as I know, 
there is currently no common userland bitops.h. However, in my paging tool 
I have one, so it is possible to have it. We could include it with libxc 
(or in some other common library).

Patrick

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