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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 04.08.14 at 16:50, <andrew.cooper3@citrix.com> wrote:
> 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.

Let me put it that way: I'd prefer this to be done as asked for earlier,
but I also wouldn't veto it being done the way you want it.

Jan