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

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv) [and 1 more messages]"):
> On 21.09.16 at 14:23, <ian.jackson@xxxxxxxxxxxxx> wrote:
> > But changes in the contents of the specific struct /will/ be spotted.
> 
> As long as it is a structure, yes. What about someone changing
> uint64_t to xen_pfn_t?

You mean if someone changes one of the buffers from "array of
uint64_t" to "array of xen_pfn_t", so that the ABI changes on some but
not all architectures ?

We could declare a rule that a dmop buffer may contain only a struct
or type name specific to that dmop (or an array of such), and must
always be accessed through a copy to or from such a type.

The rule would mean that if we had a dmop which wanted to call, using
one of the buffers, a function like this:

     int some_function(uint64_t *pfns, size_t n_pfns);

You'd have to write the dmop definition like this:

     typedef uint64_t xen_dmop_foo_entry;

     xen_dmop_foo_entry *pfns;
     size_t n_pfns;

     ...allocate table somehow...
     ret = copy_dm_buffer_from_guest(pfns,
                                     sizeof(pfns[0])*n_pfns,
                                     buffers, nr_buffers, 1);
     ...
     ret = some_function(pfns, n_pfns);

And similar code, again using xen_dmop_foo_entry. on the calling side.
Then if the buffer entry type is changed to xen_pfn_t you change it
to:

     typedef xen_pfn_t xen_dmop_foo_entry;

Now, unless the rest of the code is changed too, the compiler will
warn that a xen_pfn_t* cannot be converted to a uint64_t* in the call
to some_function.  (And likewise at the hypercall site in libxc.)

Would this be enough of an improvement, for you, to be worth the
additional type name clutter etc. ?


> > But I don't think it is really fair to criticise *the discussion
> > document* (which is what Jennifer's email was) on the grounds that it
> > ommitted to discuss a potential downside which its authors felt was
> > minor[1].
> 
> Hmm, then I'm sorry if this came over the wrong way. I didn't
> mean to criticize the document as a whole, or how it was written.

Thanks.

> >  The purpose of a discussion document is to put forward a
> > proposal and/or summarise previous discussion.  It is not required to
> > be either neutral or, indeed, comprehensive - and even so, I found
> > this one quite comprehensive.
> 
> Nevertheless I think such a document should be as honest as
> possible wrt downsides of the (by the author(s)) preferred of
> potentially multiple approaches.

I agree with that, although the word "honest" is rather tendentious.

>  If some aspect is deemed minor,
> I don't think it should be omitted (as then readers won't know
> whether the aspect was considered at all), but mentioned and
> stated to be believed to be minor.

I think there is a balance to be struck between mentioning every
possible consideration, however minor and remote, and making the
document concise and readable.

It will inevitably sometimes occur (as it did here) that some issue
that one person thought not worth mentioning, seems quite important to
another person.

Ian.

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