WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] hypercall_xlat_continuation()

>>> Ian Campbell <Ian.Campbell@xxxxxxxxxx> 24.05.09 00:36 >>>
>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.

It indeed is a sanity check, to avoid silently doing the wrong thing.

>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.

... the primary purpose of which is to deal with NULL arguments.

>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 intention here is to avoid someone trying to put translated arguments
in guest visible space (i.e. va < 4G).

>...
>
>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)

No, this is not just for sanity checking. For an example, look at
compat_memory_op()'s use of it: It has no other way of knowing how much
of the current batch do_memory_op() actually processed, since the return
value is either an error code or __HYPERVISOR_memory_op.

>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...

Indeed, I didn't try to make it artificially complicated, but the main goal
was to keep the users of it simple (which I still believe they are). I'd be
all for simplifying it *without* imposing more complexity on the callers.
Otoh I don't think it's *that* complicated...

Jan


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