Re: [PATCH v2 1/3] x86/viridian: Re-purpose the HVM parameter to be a feature mask

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 04/08/14 14:50, Jan Beulich wrote:
>>>> On 04.08.14 at 15:31, <andrew.cooper3@citrix.com> wrote:
>> On 04/08/14 14:12, Paul Durrant wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -5533,8 +5533,22 @@ long do_hvm_op(unsigned long op, 
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>                      rc = -EINVAL;
>>>                  break;
>>>              case HVM_PARAM_VIRIDIAN:
>>> -                if ( a.value > 1 )
>>> -                    rc = -EINVAL;
>>> +                /* This should only ever be set once by the tools and read by the guest. */
>>> +                rc = -EPERM;
>>> +                if ( curr_d == d )
>>> +                    break;
>>> +
>>> +                rc = -EPERM;
>>> +                if ( d->arch.hvm_domain.params[a.index] &&
>>> +                     a.value != d->arch.hvm_domain.params[a.index] )
>>> +                    break;
>> Setting it twice should be an error, even if it is set to the same value
>> again.
> I specifically asked for it to be done this way, such that redundant
> calls wouldn't needlessly fail. Remember that we're altering an
> existing interface, and hence should be careful about breaking
> existing callers.

The only valid users are domain builder parts of the toolstack, which
necessarily needs to be in sync with Xen.  All current in-tree callers
are ok.  While in general I would agree, we are already changing the
interface quite substantially.  A stricter interface is easier to
augment later if the need arises, and here I feel there is sufficient
change to warrant doing the interface properly rather than leaving this
quirk around forevermore.

In a more general sense, having worked on the migration code, I was
considering that it would be a *very* good thing to move all of this
logic into the toolstack, with Xen interacting with a pristine set of
up-to-latest-interface state.  This would reduce the amount of Xen code
doing input sanitisation/manipulation, and moves all of the backwards
compatibility cruft into a safer context to run.

One frequently requested feature of XenServer (which has a number of
large obstacles, but is sane in principle) is the ability to migrate
backwards.  The usecase is for this is being able to undo a half-upgrade
which has gone very wrong.  With all the compatibility code in the
toolstack rather than Xen, this would be feasible to implement.

~Andrew