From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v2 1/3] x86/viridian: Re-purpose the HVM parameter to be a feature mask Date: Mon, 04 Aug 2014 16:13:24 +0100 Message-ID: <53DFBF34020000780002920A@mail.emea.novell.com> References: <1407157961-7239-1-git-send-email-paul.durrant@citrix.com> <1407157961-7239-2-git-send-email-paul.durrant@citrix.com> <53DF8B21.2040907@citrix.com> <53DFABAA02000078000290C4@mail.emea.novell.com> <53DF9D9F.40209@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53DF9D9F.40209@citrix.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , Paul Durrant Cc: Keir Fraser , xen-devel@lists.xen.org, Ian Jackson , Ian Campbell , StefanoStabellini List-Id: xen-devel@lists.xenproject.org >>> On 04.08.14 at 16:50, wrote: > On 04/08/14 14:50, Jan Beulich wrote: >>>>> On 04.08.14 at 15:31, 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