Re: [Xen-devel] hypercall_xlat_continuation()

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2009-05-21 at 22:57 -0400, Mukesh Rathor wrote:
> Can someone please explain the madness in the else part of this function? The 
> caller magically passes 2 for mask? Is this already documented anywhere by 
> chance for mortals like me :).

I was forced to understand this at one point, let's see how much I can
remember ;-)

The mask argument to hypercall_xlat_continuation indicates which of the
6 potential continuation arguments (corresponding to the up to 6
arguments to a hypercall) need to be translated from a native value to a
compat value. The least significant bit == the first argument, the
second least is the second argument etc. For each bit that is set the
varargs should contain a pair of additional arguments, the native value
and the replacement compat value. The native value is compared to the
value in the continuation before replacing it with the compat value. I
would have thought the native value must always match by design so this
might just be a sanity check.

For example do_memory_op takes arguments (cmd, nat.hnd) and therefore we
pass mask==0x2 followed by varargs (nat.hnd, compat). So if the second
continuation argument matches nat.hnd it will be replaced with compat.

Similarly do_mmuext_op takes (nat_ops, otherstuff, etc...) and we pass a
mask==0x01 with varargs (nat_ops, cmp_uops so if the first continuation
argument matches nat_ops we replace it with cmp_uops.

In both cases if the native and compat things are the same we ignore the
bit set in the mask.

I don't recall what the "BUG_ON(nval == (unsigned int)nval)" is all
about. I guess the assumption is that if an argument requires
translation it must be too large to fit in a compat sized variable. This
seems to be true for the existing cases (which are both
XEN_GUEST_HANDLEs), I don't see why it would be true in general. Maybe
the assumption is that only XEN_GUEST_HANDLES ever need translation?

The "BUG_ON(*reg != (unsigned int)*reg)" is there because if we didn't
request translation for a given argument it had better be the same in
both native and compat form.

The two halves of the outermost if-else are just to handle the different
location of the continuation arguments in the multicall vs regular
hypercall cases.

The first argument to hypercall_xlat_continuation (unsigned int *id) is
a pointer to an index which, if non-NULL, is replaced with the value of
the argument at that index in the continuation, I think it's just used
for sanity checking, I'm not all that convinced it is necessary (maybe
it was useful when initially debugging this stuff)

It all seems rather complicated and fragile to me, but it does seem to
work so I'm not inclined to go poking at it...

Ian.



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