[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)



>>> On 21.09.16 at 13:21, <ian.jackson@xxxxxxxxxxxxx> wrote:
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> re qemu depriv)"):
>> On 13.09.16 at 18:07, <david.vrabel@xxxxxxxxxx> wrote:
>> > This is an direct result of the requirement that the privcmd driver does
>> > not know the types of the sub-ops themselves.  We can't have this
>> > requirement and type safety.  Which do we want?
>> 
>> Both, which the proposal utilizing side band information on VA
>> ranges allows for. (And in any event this to me clearly is an
>> aspect that would need to be mentioned in the disadvantages
>> section of the document.)
> 
> The side band information approach has the problem that it is easy to
> accidentally write hypervisor code which misses the
> additionally-required range check.
> 
> It might be possible to provide both strict type safety of the kind
> you're concerned about here, _and_ protection against missing access
> checks, but it's not very straightforward.
> 
> One approach to do that would be to augment the guest handle array
> proposal with a typesafe macro system for accessing the guest handle
> slots.
> 
> But I think George has a good argument as to why this is not needed:
> 
>   If most of the time the hypercalls are made by calling libxc functions,
>   and the libxc functions have types as arguments, then the end caller has
>   the same type safety.  We'll have to be careful inside the individual
>   libxc functions, but that should be fairly straightforward to do.  So
>   the places where we need to take extra care should be very localized.
> 
> We do not expect DMOP callers to make the hypercalls directly.  (They
> can't sensibly do so because the DMOP ABI is not stable - they need
> the assistance of the stability layer which we intend to provide in
> libxc.)
> 
> So the actual hypercall call sites are all in-tree, in libxc.  If the
> format of the struct used for one of these guest handle slots changes,
> the same struct definition from the Xen headers is used both in the
> hypervisor and in libxc, and any incompatibility will be detected.

Wait, no. The guest handle slots in Jenny's proposal are basically
typeless:

typedef struct dm_op_buffer {
     XEN_GUEST_HANDLE(void) h;
     size_t len;
} dm_op_buffer_t;

Each sub-op implies a certain type for each handle slot it actually
uses.

And then I disagree with the general view taken here: The issue
I see is not with caller of the libxc interfaces (it was always clear
that it would be possible to have type safety at that layer), but
between libxc as the consumer and Xen as the producer of the
interface.

> It's true that changes to the semantics of these slots (for example, a
> change of a slot from "array of struct X" to "one struct Y") will not
> be detected by the compiler.

Well, the "one" vs "array of" part can't normally be taken care of
by compiler type checking anyway, except that for the "one" case
we wouldn't normally use a handle in the first place.

> But *all* ABI changes to the DMOP interface need to be accompanied by
> changes to libxc to add compatibility code.  I think it will be fairly
> straightforward to check, during review, that each DMOP change comes
> with a corresponding libxc change.

Well, yes, we can only hope for that. The main risk is that someone
doesn't update the other side at all, when a change to one side gets
done which compiles fine at both ends, and perhaps doesn't even
alter the public header.

> But if you do not agree, then how about hiding the slots with a macro
> setup ?  Something like:
>[...]

Well, that's exactly the kind of workaround I'd like to avoid.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.