>>> On 01.07.16 at 18:42, wrote:
> On 01/07/16 17:18, Jan Beulich wrote:
> On 24.06.16 at 12:28, wrote:
>>> ... as a means to replace all HVMOP_* which a domain can't issue on
>>> itself (i.e. intended for use by only the control domain or device
>>> model).
>>>
>>> Signed-off-by: Jan Beulich
>>> Reviewed-by: Wei Liu
>> On the x86 side I'm just lacking feedback for this patch.
>
> I have just spent the afternoon being bitten extremely hard by our
> current unstable domctl abi, and in particular, the change of
> DOMCTL_API_VERSION when nothing relevant has changed, and am leaning
> towards David's views.
>
> With the current definition, we have 32 bits of cmd space, proper
> continuation logic via the opaque field, and 120 bytes of per-cmd space
> in the union, which plenty.
>
> How about making a proactive start to our ABI stabilisation effort,
> dropping the interface_version entirely and declaring this stable? We
> would of course want to triple check the suitability of the existing
> ops, but that can easily be rolled into this series (if any action is
> needed).
I have to admit that I'm a little frustrated by this request: The series
has been out for quite some time, and was supposedly ready to go
in if all acks had been given. Yet with what you say above you
effectively would withdraw the ones you gave on the later patches
in the series, even if you don't say so explicitly. The fact that you've
got bitten by the domctl being modeled in similar ways (yet without
actually saying how you got bitten, or what's wrong with that model)
shouldn't really have much of an influence here. Even more so if, as
I would guess, that issue of yours was with the domctl wrapping
logic in your privcmd driver in Dom0 (which is already conceptually
problematic, as the kernel isn't intended to know of or make use of
domctl-s, and hence you having made it know set you up for such
problems).
Nor do I see myself do the auditing of the involved operations right
now (and basically as a prereq for this series to go in), the more that
I've learned from commits aa956976a9 ("domctl: perform initial
post-XSA-77 auditing") and then 5590bd17c4 ("XSA-77: widen scope
again") that it's very easy to overlook some aspect(s), no matter
how much time and effort one invests. I really think that for any
such future efforts we first need to put down a complete check list
of things that need to be ensured prior to making _any_ interface
stable and usable by other than fully trusted entities.
As to the option of marking this interface stable without doing the
security audit - I don't think I would see the point.
> Another area (which is related to the issue which bit me) is the
> stability of operations like DOMCTL_pausedomain, which is unlikely to
> ever change.
>
> If we do chose to stabilise, we should design the new calls around how
> they would be used. We could do with a stable interface for "general
> emulator routines", which applies equally to things like pause/unpause
> and ioreq_server*, as opposed to most of the new hvmctl ops, which are
> specific to qemu being an LPC bus emulator.
>
> One thing I hadn't considered until now is that this takes an existing
> stable ABI and replaces it with an unstable ABI, which is a step
> backwards in terms of usability. There are certainly other advantages
> to moving the ops out of hvmop, but the instability is a detracting factor.
This is not true imo: The existing interface wasn't stable (demonstrated
by it being framed by __XEN_ / __XEN_TOOLS__ conditionals), yet it
_also_ wasn't versioned, so its instablility wasn't represented properly.
So as said to David already, I continue to think this series is an
improvement, albeit other than in the direction that both of you would
like things to move. And while I'm with you for those intentions, I don't
think I should be required to make two steps at once here.
Plus you don't comment at all on my counter proposal to stabilize the
libxc wrappers around these hypercalls instead of the hypercalls
themselves.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
