All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: StefanoStabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Jennifer Herbert <Jennifer.Herbert@citrix.com>,
	TimDeegan <tim@xen.org>, David Vrabel <david.vrabel@citrix.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	dgdegra@tycho.nsa.gov
Subject: Re: Device model operation hypercall (DMOP, re qemu depriv)
Date: Wed, 21 Sep 2016 05:55:34 -0600	[thread overview]
Message-ID: <57E291560200007800111079@prv-mh.provo.novell.com> (raw)
In-Reply-To: <22498.27970.77634.32169@mariner.uk.xensource.com>

>>> On 21.09.16 at 13:21, <ian.jackson@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> re qemu depriv)"):
>> On 13.09.16 at 18:07, <david.vrabel@citrix.com> 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@lists.xen.org
https://lists.xen.org/xen-devel

  parent reply	other threads:[~2016-09-21 11:55 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-28 17:01 XenProject/XenServer QEMU working group, Friday 8th July, 2016, 15:00 Jennifer Herbert
2016-08-01 11:32 ` Device model operation hypercall (DMOP, re qemu depriv) Ian Jackson
2016-08-01 12:41   ` Jan Beulich
2016-08-02 11:38     ` Wei Liu
2016-08-02 11:58       ` Jan Beulich
2016-08-02 13:02         ` David Vrabel
2016-08-02 13:29           ` Jan Beulich
2016-08-03 10:29       ` Ian Jackson
2016-08-03 12:03         ` Jan Beulich
2016-08-03 13:37           ` Ian Jackson
2016-08-03 14:16             ` Jan Beulich
2016-08-03 14:21               ` George Dunlap
2016-08-03 16:10                 ` Ian Jackson
2016-08-03 16:18                   ` Jan Beulich
2016-08-04 11:21                     ` Ian Jackson
2016-08-04 13:24                       ` Jan Beulich
2016-08-05 16:28                         ` Ian Jackson
2016-08-08 11:18                           ` Jan Beulich
2016-08-08 13:46                             ` Ian Jackson
2016-08-08 14:07                               ` Jan Beulich
2016-08-26 11:38                                 ` Ian Jackson
2016-08-26 12:58                                   ` Jan Beulich
2016-08-26 14:35                                     ` Ian Jackson
2016-08-26 15:13                                       ` Jan Beulich
2016-08-30 11:02                                         ` Ian Jackson
2016-08-30 21:47                                           ` Stefano Stabellini
2016-09-02 14:08                                           ` Wei Liu
2016-08-09 10:29                               ` Jan Beulich
2016-08-09 10:48                                 ` Ian Jackson
2016-08-09 11:30                                   ` Jan Beulich
2016-08-12  9:44                                     ` George Dunlap
2016-08-12 11:50                                       ` Jan Beulich
2016-08-15  9:39                                         ` George Dunlap
2016-08-15 10:19                                           ` Jan Beulich
2016-08-15 10:47                                             ` George Dunlap
2016-08-15 11:20                                               ` Jan Beulich
2016-08-15 12:07                                                 ` Ian Jackson
2016-08-15 14:20                                                   ` Jan Beulich
2016-08-15 14:57                                                 ` George Dunlap
2016-08-15 15:22                                                   ` Jan Beulich
2016-08-15 14:50                                 ` David Vrabel
2016-08-15 15:24                                   ` Jan Beulich
2016-08-26 11:29                                     ` Ian Jackson
2016-08-26 12:58                                       ` Jan Beulich
2016-08-02 11:37   ` Wei Liu
2016-08-02 11:42     ` George Dunlap
2016-08-02 12:34       ` Wei Liu
2016-09-09 15:16   ` Jennifer Herbert
2016-09-09 15:34     ` David Vrabel
2016-09-12 13:47     ` George Dunlap
2016-09-12 14:32     ` Jan Beulich
2016-09-13 10:37       ` George Dunlap
2016-09-13 11:53         ` Jan Beulich
2016-09-13 16:07       ` David Vrabel
2016-09-14  9:51         ` Jan Beulich
2016-09-21 11:21           ` Ian Jackson
2016-09-21 11:28             ` George Dunlap
2016-09-21 11:58               ` Jan Beulich
2016-09-21 11:55             ` Jan Beulich [this message]
2016-09-21 12:23               ` Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages] Ian Jackson
2016-09-21 12:48                 ` Jan Beulich
2016-09-21 13:24                   ` Ian Jackson
2016-09-21 13:56                     ` Jan Beulich
2016-09-21 15:06                       ` Ian Jackson
2016-09-21 17:09                       ` George Dunlap
2016-09-22  8:47                         ` Jan Beulich
2016-09-09 16:18 ` XenProject/XenServer QEMU working group minutes, 30th August 2016 Jennifer Herbert
2016-09-12  7:16   ` Juergen Gross
2016-10-14 18:01   ` QEMU XenServer/XenProject Working group meeting 29th September 2016 Jennifer Herbert
2016-10-18 19:54     ` Stefano Stabellini
2016-10-20 17:37       ` Lars Kurth
2016-10-20 18:53         ` Stefano Stabellini
2017-02-28 18:18     ` QEMU XenServer/XenProject Working group meeting 10th February 2017 Jennifer Herbert
2017-06-05 13:48       ` QEMU XenServer/XenProject Working group meeting 10th May 2017 Jennifer Herbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57E291560200007800111079@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=Jennifer.Herbert@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=david.vrabel@citrix.com \
    --cc=dgdegra@tycho.nsa.gov \
    --cc=ian.jackson@eu.citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.