Xen-4.5 HVMOP ABI issues

Xen-4.5 HVMOP ABI issues

[Andrew Cooper]

Hi,

I have finally worked out why XenServer is having issues with its legacy
windows drivers and Xen-4.5.

XenServer needs to support hvm ops with an indicies of 0x102 and 0x103
to prevent our legacy windows drivers from BSODing on boot.  (These
problems will disappear when we can drop Windows XP/Server 2k3 support,
but that time is not now.)

The root cause of the breakage is c/s e8b87b57b "x86/HVM: fix preemption
handling in do_hvm_op() (try 2)", and in particular the HVMOP_op_mask,
which turns the above hypercalls into HVMOP_set_{pci_intx,isa_irq}_level
hypercalls.


>From my point of view, I can work around this with the knowledge that
HVMOP_set_{pci_intx,isa_irq}_level hypercalls are not eligible for
pre-emption.

However, it occurs to me that HVMOP_op_mask absolutely must live in the
public header files, as it is guest visible, and forms part of the ABI.

Consider a userspace task which falls into a continuation, is preempted
and paused by the guest kernel, the entire VM migrated, and the task
unpaused later.  If HVMOP_op_mask has changed in that time, the
continuation will become erroneous.

In particular, all hypercall continuation strategies we use *must* be
forward compatible when moving to newer versions of Xen, because Xen
cannot possibly guarantee that a continuation which starts will finish
on the same hypervisor.

~Andrew

Re: Xen-4.5 HVMOP ABI issues

[Jan Beulich]

>>> On 28.11.14 at 12:36, <andrew.cooper3@citrix.com> wrote:
> However, it occurs to me that HVMOP_op_mask absolutely must live in the
> public header files, as it is guest visible, and forms part of the ABI.
> 
> Consider a userspace task which falls into a continuation, is preempted
> and paused by the guest kernel, the entire VM migrated, and the task
> unpaused later.  If HVMOP_op_mask has changed in that time, the
> continuation will become erroneous.
> 
> In particular, all hypercall continuation strategies we use *must* be
> forward compatible when moving to newer versions of Xen, because Xen
> cannot possibly guarantee that a continuation which starts will finish
> on the same hypervisor.

Hmm, I see the issue, but surfacing such implementation details
would not only be unfortunate, but eventually prevent us from
making future changes. Just recall the mem-op case where we had
to widen the continuation encoding mask at some point. Hence while
I can't immediately see a way to avoid the situation you describe
(and it doesn't even take a user space task in a preemptible kernel),
I think we should allow ourselves a little more time to find possible
solutions other than locking down these masks (really they don't
need to be exposed in the public headers, but we would need
them to not change if no other solution can be found).

One thing to note is that the _introduction_ of such a mask (as
has happened for hvm-op between 4.4 and 4.5) is not a problem
as long as the respective bits all being zero has no special
meaning. What I considered for mem-op a while ago though (and
then forgot again) was to refuse non-zero start_extent values
for any operations not making use of that mechanism. The same
would of course be applicable to gnttab-op and hvm-op.

What might additionally make this not that urgent an issue for 4.5
is that only XSM_DM_PRIV guarded operations are affected, and
I suppose a stubdom gets re-created on the target host (rather
than migrated) when its client migrates.

Jan

Re: Xen-4.5 HVMOP ABI issues

[Ian Campbell]

On Fri, 2014-11-28 at 13:07 +0000, Jan Beulich wrote:
> I suppose a stubdom gets re-created on the target host (rather
> than migrated) when its client migrates.

Correct.

Ian.

Re: Xen-4.5 HVMOP ABI issues

[Andrew Cooper]

On 28/11/14 13:07, Jan Beulich wrote:
>>>> On 28.11.14 at 12:36, <andrew.cooper3@citrix.com> wrote:
>> However, it occurs to me that HVMOP_op_mask absolutely must live in the
>> public header files, as it is guest visible, and forms part of the ABI.
>>
>> Consider a userspace task which falls into a continuation, is preempted
>> and paused by the guest kernel, the entire VM migrated, and the task
>> unpaused later.  If HVMOP_op_mask has changed in that time, the
>> continuation will become erroneous.
>>
>> In particular, all hypercall continuation strategies we use *must* be
>> forward compatible when moving to newer versions of Xen, because Xen
>> cannot possibly guarantee that a continuation which starts will finish
>> on the same hypervisor.
> Hmm, I see the issue, but surfacing such implementation details
> would not only be unfortunate, but eventually prevent us from
> making future changes. Just recall the mem-op case where we had
> to widen the continuation encoding mask at some point. Hence while
> I can't immediately see a way to avoid the situation you describe
> (and it doesn't even take a user space task in a preemptible kernel),
> I think we should allow ourselves a little more time to find possible
> solutions other than locking down these masks (really they don't
> need to be exposed in the public headers, but we would need
> them to not change if no other solution can be found).

It is certainly unfortunate, but I don't see that we have any choice. 
The implementation details of continuations have already slipped into
the ABI.

>
> One thing to note is that the _introduction_ of such a mask (as
> has happened for hvm-op between 4.4 and 4.5) is not a problem
> as long as the respective bits all being zero has no special
> meaning. What I considered for mem-op a while ago though (and
> then forgot again) was to refuse non-zero start_extent values
> for any operations not making use of that mechanism. The same
> would of course be applicable to gnttab-op and hvm-op.
>
> What might additionally make this not that urgent an issue for 4.5
> is that only XSM_DM_PRIV guarded operations are affected, and
> I suppose a stubdom gets re-created on the target host (rather
> than migrated) when its client migrates.

The problem is with continuations which reuse the upper bits of the
input register, not with this HVMOP_op_mask specifically; the
HVMOP_op_mask simply adds to an existing problem.  This is something
which needs considering urgently because, as you identify above, we have
already suffered an accidental ABI breakage with the mem-op widening.

32bit HVM guests reliably form a continuation on every single iteration
of a continuable hypercall (e.g. decrease reservation, which is the base
of my XSA-111 PoC), making it trivial to construct a migrateable HVM
domain which exposes the issue.

~Andrew

Re: Xen-4.5 HVMOP ABI issues

[Jan Beulich]

>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
> The problem is with continuations which reuse the upper bits of the
> input register, not with this HVMOP_op_mask specifically; the
> HVMOP_op_mask simply adds to an existing problem.  This is something
> which needs considering urgently because, as you identify above, we have
> already suffered an accidental ABI breakage with the mem-op widening.

Since we can't retroactively fix the mem-op widening, I still don't see
what's urgent here: As long as we don't change any of these masks,
nothing bad is going to happen. Of course one thing to consider with
this aspect in mind is whether to change the hvm-op or gnttab-op
masks again _before_ 4.5 goes out, based on whether we feel they're
wide enough for the (un)foreseeable future.

> 32bit HVM guests reliably form a continuation on every single iteration
> of a continuable hypercall (e.g. decrease reservation, which is the base
> of my XSA-111 PoC), making it trivial to construct a migrateable HVM
> domain which exposes the issue.

Hmm, I can't offhand see why that would be, but what you write
reads like you determined the reason? I ask because an unavoidable
use of continuations is certainly nice for making sure those code paths
get tested, but would be quite desirable to get eliminated at least for
non-debug builds.

Jan

Re: Xen-4.5 HVMOP ABI issues

[Andrew Cooper]

On 28/11/14 15:18, Jan Beulich wrote:
>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
>> The problem is with continuations which reuse the upper bits of the
>> input register, not with this HVMOP_op_mask specifically; the
>> HVMOP_op_mask simply adds to an existing problem.  This is something
>> which needs considering urgently because, as you identify above, we have
>> already suffered an accidental ABI breakage with the mem-op widening.
> Since we can't retroactively fix the mem-op widening, I still don't see
> what's urgent here: As long as we don't change any of these masks,
> nothing bad is going to happen. Of course one thing to consider with
> this aspect in mind is whether to change the hvm-op or gnttab-op
> masks again _before_ 4.5 goes out, based on whether we feel they're
> wide enough for the (un)foreseeable future.

By urgent, I mean exactly this, while we have the ability to tweak the
masks.

>
>> 32bit HVM guests reliably form a continuation on every single iteration
>> of a continuable hypercall (e.g. decrease reservation, which is the base
>> of my XSA-111 PoC), making it trivial to construct a migrateable HVM
>> domain which exposes the issue.
> Hmm, I can't offhand see why that would be, but what you write
> reads like you determined the reason?

I have never identified why, but it is reliable.  c/s 79de2d31f1 proves
that some preemption condition is true for 32bit HVM guests by the time
the hypercall handler is called.  It is unreliable with 64bit HVM
guests, suggesting that the compat translation layer may be the root
cause of the issue.

> I ask because an unavoidable
> use of continuations is certainly nice for making sure those code paths
> get tested, but would be quite desirable to get eliminated at least for
> non-debug builds.

While the preemption code is necessary to avoid spending ludicrous
quantities of time synchronously in Xen, it is currently having the
worst possible effect it could have on the system, by causing 32bit HVM
guests to undergo a vmentry/vmexit and double compat translation for
each iteration.

~Andrew

Re: Xen-4.5 HVMOP ABI issues

[Jan Beulich]

>>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
> On 28/11/14 15:18, Jan Beulich wrote:
>>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
>>> The problem is with continuations which reuse the upper bits of the
>>> input register, not with this HVMOP_op_mask specifically; the
>>> HVMOP_op_mask simply adds to an existing problem.  This is something
>>> which needs considering urgently because, as you identify above, we have
>>> already suffered an accidental ABI breakage with the mem-op widening.
>> Since we can't retroactively fix the mem-op widening, I still don't see
>> what's urgent here: As long as we don't change any of these masks,
>> nothing bad is going to happen. Of course one thing to consider with
>> this aspect in mind is whether to change the hvm-op or gnttab-op
>> masks again _before_ 4.5 goes out, based on whether we feel they're
>> wide enough for the (un)foreseeable future.
> 
> By urgent, I mean exactly this, while we have the ability to tweak the
> masks.

With no-one else voicing an opinion:

For hvmop, the mask currently is 8 bits and we've got 22 ops defined.

For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.

For the latter, we're fine even without further consideration. For the
former, the two operations actively using the continuation encoding
are tools-only ones. Since we're fine to alter the tools only interfaces,
and since it was intended for the tools-only HVM-ops to be split off
to a separate hypercall (e.g. hvmctl) anyway, the range restriction
would then no longer be a problem. Plus, in the worst case we could
always introduce yet another hypercall if we ran out of numbers.

Consequently what I'd like to propose (and I guess I'll craft a patch
as soon as I finished this mail) is that we add comments to these
masks (also the memop one) to make clear that they mustn't change.
Additionally for forward compatibility I'd also like to add checks for
the upper bits to be zero for any of the sub-ops that don't actually
use them to encode continuation information. Konrad, would you
consider doing so acceptable for 4.5?

Jan

Re: Xen-4.5 HVMOP ABI issues

[Andrew Cooper]

On 04/12/14 13:49, Jan Beulich wrote:
>>>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
>> On 28/11/14 15:18, Jan Beulich wrote:
>>>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
>>>> The problem is with continuations which reuse the upper bits of the
>>>> input register, not with this HVMOP_op_mask specifically; the
>>>> HVMOP_op_mask simply adds to an existing problem.  This is something
>>>> which needs considering urgently because, as you identify above, we have
>>>> already suffered an accidental ABI breakage with the mem-op widening.
>>> Since we can't retroactively fix the mem-op widening, I still don't see
>>> what's urgent here: As long as we don't change any of these masks,
>>> nothing bad is going to happen. Of course one thing to consider with
>>> this aspect in mind is whether to change the hvm-op or gnttab-op
>>> masks again _before_ 4.5 goes out, based on whether we feel they're
>>> wide enough for the (un)foreseeable future.
>> By urgent, I mean exactly this, while we have the ability to tweak the
>> masks.
> With no-one else voicing an opinion:
>
> For hvmop, the mask currently is 8 bits and we've got 22 ops defined.
>
> For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.
>
> For the latter, we're fine even without further consideration. For the
> former, the two operations actively using the continuation encoding
> are tools-only ones. Since we're fine to alter the tools only interfaces,
> and since it was intended for the tools-only HVM-ops to be split off
> to a separate hypercall (e.g. hvmctl) anyway, the range restriction
> would then no longer be a problem. Plus, in the worst case we could
> always introduce yet another hypercall if we ran out of numbers.

Are you suggesting that we make a new hvmctl now and remove the hvmop
mask before 4.5?  If we ship 4.5 with the hvmop mask, we cannot
subsequently remove it even if all continuable hypercalls move to a
separate hypercall.

>
> Consequently what I'd like to propose (and I guess I'll craft a patch
> as soon as I finished this mail) is that we add comments to these
> masks (also the memop one) to make clear that they mustn't change.
> Additionally for forward compatibility I'd also like to add checks for
> the upper bits to be zero for any of the sub-ops that don't actually
> use them to encode continuation information. Konrad, would you
> consider doing so acceptable for 4.5?

I will have to re-work around this new check for XenServer
compatibility, but I suppose that is fine.  I am certainly in favour of
leaving an ABI comment with the op masks.

~Andrew

Re: Xen-4.5 HVMOP ABI issues

[Jan Beulich]

>>> On 04.12.14 at 15:28, <andrew.cooper3@citrix.com> wrote:
> On 04/12/14 13:49, Jan Beulich wrote:
>>>>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
>>> On 28/11/14 15:18, Jan Beulich wrote:
>>>>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
>>>>> The problem is with continuations which reuse the upper bits of the
>>>>> input register, not with this HVMOP_op_mask specifically; the
>>>>> HVMOP_op_mask simply adds to an existing problem.  This is something
>>>>> which needs considering urgently because, as you identify above, we have
>>>>> already suffered an accidental ABI breakage with the mem-op widening.
>>>> Since we can't retroactively fix the mem-op widening, I still don't see
>>>> what's urgent here: As long as we don't change any of these masks,
>>>> nothing bad is going to happen. Of course one thing to consider with
>>>> this aspect in mind is whether to change the hvm-op or gnttab-op
>>>> masks again _before_ 4.5 goes out, based on whether we feel they're
>>>> wide enough for the (un)foreseeable future.
>>> By urgent, I mean exactly this, while we have the ability to tweak the
>>> masks.
>> With no-one else voicing an opinion:
>>
>> For hvmop, the mask currently is 8 bits and we've got 22 ops defined.
>>
>> For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.
>>
>> For the latter, we're fine even without further consideration. For the
>> former, the two operations actively using the continuation encoding
>> are tools-only ones. Since we're fine to alter the tools only interfaces,
>> and since it was intended for the tools-only HVM-ops to be split off
>> to a separate hypercall (e.g. hvmctl) anyway, the range restriction
>> would then no longer be a problem. Plus, in the worst case we could
>> always introduce yet another hypercall if we ran out of numbers.
> 
> Are you suggesting that we make a new hvmctl now and remove the hvmop
> mask before 4.5?  If we ship 4.5 with the hvmop mask, we cannot
> subsequently remove it even if all continuable hypercalls move to a
> separate hypercall.

Why? We certainly don't guarantee compatibility for undefined
hypercalls to behave in a certain way.

Jan

Re: Xen-4.5 HVMOP ABI issues

[Andrew Cooper]

On 04/12/14 14:55, Jan Beulich wrote:
>>>> On 04.12.14 at 15:28, <andrew.cooper3@citrix.com> wrote:
>> On 04/12/14 13:49, Jan Beulich wrote:
>>>>>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
>>>> On 28/11/14 15:18, Jan Beulich wrote:
>>>>>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
>>>>>> The problem is with continuations which reuse the upper bits of the
>>>>>> input register, not with this HVMOP_op_mask specifically; the
>>>>>> HVMOP_op_mask simply adds to an existing problem.  This is something
>>>>>> which needs considering urgently because, as you identify above, we have
>>>>>> already suffered an accidental ABI breakage with the mem-op widening.
>>>>> Since we can't retroactively fix the mem-op widening, I still don't see
>>>>> what's urgent here: As long as we don't change any of these masks,
>>>>> nothing bad is going to happen. Of course one thing to consider with
>>>>> this aspect in mind is whether to change the hvm-op or gnttab-op
>>>>> masks again _before_ 4.5 goes out, based on whether we feel they're
>>>>> wide enough for the (un)foreseeable future.
>>>> By urgent, I mean exactly this, while we have the ability to tweak the
>>>> masks.
>>> With no-one else voicing an opinion:
>>>
>>> For hvmop, the mask currently is 8 bits and we've got 22 ops defined.
>>>
>>> For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.
>>>
>>> For the latter, we're fine even without further consideration. For the
>>> former, the two operations actively using the continuation encoding
>>> are tools-only ones. Since we're fine to alter the tools only interfaces,
>>> and since it was intended for the tools-only HVM-ops to be split off
>>> to a separate hypercall (e.g. hvmctl) anyway, the range restriction
>>> would then no longer be a problem. Plus, in the worst case we could
>>> always introduce yet another hypercall if we ran out of numbers.
>> Are you suggesting that we make a new hvmctl now and remove the hvmop
>> mask before 4.5?  If we ship 4.5 with the hvmop mask, we cannot
>> subsequently remove it even if all continuable hypercalls move to a
>> separate hypercall.
> Why? We certainly don't guarantee compatibility for undefined
> hypercalls to behave in a certain way.

A task is in the middle of a hypercall continuation.  The VM is migrated
to a newer Xen which has lost the op mask and gained a new hypercall
which would alias.

At this point, the task has transparently swapped hypercall subops,
through no fault of its own.

As I have said before, once an op mask has found its way into a released
version of Xen, it is irrevocably part of the ABI, and cannot change in
the future.

~Andrew

Re: Xen-4.5 HVMOP ABI issues

[Tim Deegan]

At 14:28 +0000 on 04 Dec (1417699730), Andrew Cooper wrote:
> On 04/12/14 13:49, Jan Beulich wrote:
> >>>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
> >> On 28/11/14 15:18, Jan Beulich wrote:
> >>>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
> >>>> The problem is with continuations which reuse the upper bits of the
> >>>> input register, not with this HVMOP_op_mask specifically; the
> >>>> HVMOP_op_mask simply adds to an existing problem.  This is something
> >>>> which needs considering urgently because, as you identify above, we have
> >>>> already suffered an accidental ABI breakage with the mem-op widening.
> >>> Since we can't retroactively fix the mem-op widening, I still don't see
> >>> what's urgent here: As long as we don't change any of these masks,
> >>> nothing bad is going to happen. Of course one thing to consider with
> >>> this aspect in mind is whether to change the hvm-op or gnttab-op
> >>> masks again _before_ 4.5 goes out, based on whether we feel they're
> >>> wide enough for the (un)foreseeable future.
> >> By urgent, I mean exactly this, while we have the ability to tweak the
> >> masks.
> > With no-one else voicing an opinion:
> >
> > For hvmop, the mask currently is 8 bits and we've got 22 ops defined.
> >
> > For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.
> >
> > For the latter, we're fine even without further consideration. For the
> > former, the two operations actively using the continuation encoding
> > are tools-only ones. Since we're fine to alter the tools only interfaces,
> > and since it was intended for the tools-only HVM-ops to be split off
> > to a separate hypercall (e.g. hvmctl) anyway, the range restriction
> > would then no longer be a problem. Plus, in the worst case we could
> > always introduce yet another hypercall if we ran out of numbers.
> 
> Are you suggesting that we make a new hvmctl now and remove the hvmop
> mask before 4.5?  If we ship 4.5 with the hvmop mask, we cannot
> subsequently remove it even if all continuable hypercalls move to a
> separate hypercall.

I think we can if the only hypercalls that use continuations are
tools-only (and so not liable to work across migration anyway).

Tim.

Re: Xen-4.5 HVMOP ABI issues

[Jan Beulich]

>>> On 04.12.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
> On 04/12/14 14:55, Jan Beulich wrote:
>>>>> On 04.12.14 at 15:28, <andrew.cooper3@citrix.com> wrote:
>>> On 04/12/14 13:49, Jan Beulich wrote:
>>>>>>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
>>>>> On 28/11/14 15:18, Jan Beulich wrote:
>>>>>>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
>>>>>>> The problem is with continuations which reuse the upper bits of the
>>>>>>> input register, not with this HVMOP_op_mask specifically; the
>>>>>>> HVMOP_op_mask simply adds to an existing problem.  This is something
>>>>>>> which needs considering urgently because, as you identify above, we have
>>>>>>> already suffered an accidental ABI breakage with the mem-op widening.
>>>>>> Since we can't retroactively fix the mem-op widening, I still don't see
>>>>>> what's urgent here: As long as we don't change any of these masks,
>>>>>> nothing bad is going to happen. Of course one thing to consider with
>>>>>> this aspect in mind is whether to change the hvm-op or gnttab-op
>>>>>> masks again _before_ 4.5 goes out, based on whether we feel they're
>>>>>> wide enough for the (un)foreseeable future.
>>>>> By urgent, I mean exactly this, while we have the ability to tweak the
>>>>> masks.
>>>> With no-one else voicing an opinion:
>>>>
>>>> For hvmop, the mask currently is 8 bits and we've got 22 ops defined.
>>>>
>>>> For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.
>>>>
>>>> For the latter, we're fine even without further consideration. For the
>>>> former, the two operations actively using the continuation encoding
>>>> are tools-only ones. Since we're fine to alter the tools only interfaces,
>>>> and since it was intended for the tools-only HVM-ops to be split off
>>>> to a separate hypercall (e.g. hvmctl) anyway, the range restriction
>>>> would then no longer be a problem. Plus, in the worst case we could
>>>> always introduce yet another hypercall if we ran out of numbers.
>>> Are you suggesting that we make a new hvmctl now and remove the hvmop
>>> mask before 4.5?  If we ship 4.5 with the hvmop mask, we cannot
>>> subsequently remove it even if all continuable hypercalls move to a
>>> separate hypercall.
>> Why? We certainly don't guarantee compatibility for undefined
>> hypercalls to behave in a certain way.
> 
> A task is in the middle of a hypercall continuation.  The VM is migrated
> to a newer Xen which has lost the op mask and gained a new hypercall
> which would alias.

Impossible: A continuation could be in progress only when we
actually use the high bits (or else you have nowhere to encode
it). Operations not using continuations consequently aren't
susceptible to the mask disappearing.

Jan

Re: Xen-4.5 HVMOP ABI issues

[Andrew Cooper]

On 04/12/14 16:11, Jan Beulich wrote:
>>>> On 04.12.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
>> On 04/12/14 14:55, Jan Beulich wrote:
>>>>>> On 04.12.14 at 15:28, <andrew.cooper3@citrix.com> wrote:
>>>> On 04/12/14 13:49, Jan Beulich wrote:
>>>>>>>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
>>>>>> On 28/11/14 15:18, Jan Beulich wrote:
>>>>>>>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
>>>>>>>> The problem is with continuations which reuse the upper bits of the
>>>>>>>> input register, not with this HVMOP_op_mask specifically; the
>>>>>>>> HVMOP_op_mask simply adds to an existing problem.  This is something
>>>>>>>> which needs considering urgently because, as you identify above, we have
>>>>>>>> already suffered an accidental ABI breakage with the mem-op widening.
>>>>>>> Since we can't retroactively fix the mem-op widening, I still don't see
>>>>>>> what's urgent here: As long as we don't change any of these masks,
>>>>>>> nothing bad is going to happen. Of course one thing to consider with
>>>>>>> this aspect in mind is whether to change the hvm-op or gnttab-op
>>>>>>> masks again _before_ 4.5 goes out, based on whether we feel they're
>>>>>>> wide enough for the (un)foreseeable future.
>>>>>> By urgent, I mean exactly this, while we have the ability to tweak the
>>>>>> masks.
>>>>> With no-one else voicing an opinion:
>>>>>
>>>>> For hvmop, the mask currently is 8 bits and we've got 22 ops defined.
>>>>>
>>>>> For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.
>>>>>
>>>>> For the latter, we're fine even without further consideration. For the
>>>>> former, the two operations actively using the continuation encoding
>>>>> are tools-only ones. Since we're fine to alter the tools only interfaces,
>>>>> and since it was intended for the tools-only HVM-ops to be split off
>>>>> to a separate hypercall (e.g. hvmctl) anyway, the range restriction
>>>>> would then no longer be a problem. Plus, in the worst case we could
>>>>> always introduce yet another hypercall if we ran out of numbers.
>>>> Are you suggesting that we make a new hvmctl now and remove the hvmop
>>>> mask before 4.5?  If we ship 4.5 with the hvmop mask, we cannot
>>>> subsequently remove it even if all continuable hypercalls move to a
>>>> separate hypercall.
>>> Why? We certainly don't guarantee compatibility for undefined
>>> hypercalls to behave in a certain way.
>> A task is in the middle of a hypercall continuation.  The VM is migrated
>> to a newer Xen which has lost the op mask and gained a new hypercall
>> which would alias.
> Impossible: A continuation could be in progress only when we
> actually use the high bits (or else you have nowhere to encode
> it). Operations not using continuations consequently aren't
> susceptible to the mask disappearing.

Ah yes - if nothing guest usable is currently continuable, then this is ok.

~Andrew

Re: Xen-4.5 HVMOP ABI issues

[Konrad Rzeszutek Wilk]

On Thu, Dec 04, 2014 at 01:49:47PM +0000, Jan Beulich wrote:
> >>> On 28.11.14 at 16:46, <andrew.cooper3@citrix.com> wrote:
> > On 28/11/14 15:18, Jan Beulich wrote:
> >>>>> On 28.11.14 at 14:55, <andrew.cooper3@citrix.com> wrote:
> >>> The problem is with continuations which reuse the upper bits of the
> >>> input register, not with this HVMOP_op_mask specifically; the
> >>> HVMOP_op_mask simply adds to an existing problem.  This is something
> >>> which needs considering urgently because, as you identify above, we have
> >>> already suffered an accidental ABI breakage with the mem-op widening.
> >> Since we can't retroactively fix the mem-op widening, I still don't see
> >> what's urgent here: As long as we don't change any of these masks,
> >> nothing bad is going to happen. Of course one thing to consider with
> >> this aspect in mind is whether to change the hvm-op or gnttab-op
> >> masks again _before_ 4.5 goes out, based on whether we feel they're
> >> wide enough for the (un)foreseeable future.
> > 
> > By urgent, I mean exactly this, while we have the ability to tweak the
> > masks.
> 
> With no-one else voicing an opinion:
> 
> For hvmop, the mask currently is 8 bits and we've got 22 ops defined.
> 
> For gnttabop, the mask currently is 12 bits and we've got 12 ops defined.
> 
> For the latter, we're fine even without further consideration. For the
> former, the two operations actively using the continuation encoding
> are tools-only ones. Since we're fine to alter the tools only interfaces,
> and since it was intended for the tools-only HVM-ops to be split off
> to a separate hypercall (e.g. hvmctl) anyway, the range restriction
> would then no longer be a problem. Plus, in the worst case we could
> always introduce yet another hypercall if we ran out of numbers.
> 
> Consequently what I'd like to propose (and I guess I'll craft a patch
> as soon as I finished this mail) is that we add comments to these
> masks (also the memop one) to make clear that they mustn't change.
> Additionally for forward compatibility I'd also like to add checks for
> the upper bits to be zero for any of the sub-ops that don't actually
> use them to encode continuation information. Konrad, would you
> consider doing so acceptable for 4.5?

Yes.

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel