[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] 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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |