From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tim Deegan <tim@xen.org>
Subject: Re: [PATCH v5 3/4] VMX: use proper instruction
	mnemonics if assembler supports them
Date: Thu, 29 Aug 2013 15:02:21 +0100
Message-ID: <20130829140221.GH75385@ocelot.phlegethon.org>
References: <521B1F15.1030104@citrix.com>
	<521B479902000078000EE505@nat28.tlf.novell.com>
	<521B528C.8080107@citrix.com>
	<521B7C6402000078000EE6D4@nat28.tlf.novell.com>
	<521B63AF.7000008@citrix.com>
	<521B90F802000078000EE7A3@nat28.tlf.novell.com>
	<20130829114742.GC75385@ocelot.phlegethon.org>
	<521F5B2202000078000EF6A0@nat28.tlf.novell.com>
	<20130829131147.GF75385@ocelot.phlegethon.org>
	<521F685102000078000EF752@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta4.messagelabs.com ([85.158.143.247])
	by lists.xen.org with esmtp (Exim 4.72) (envelope-from <tim@xen.org>)
	id 1VF2ns-0000BE-6o
	for xen-devel@lists.xenproject.org; Thu, 29 Aug 2013 14:02:28 +0000
Content-Disposition: inline
In-Reply-To: <521F685102000078000EF752@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Keir Fraser <keir@xen.org>, Eddie Dong <eddie.dong@intel.com>, Jun Nakajima <jun.nakajima@intel.com>, xen-devel <xen-devel@lists.xenproject.org>
List-Id: xen-devel@lists.xenproject.org

At 14:27 +0100 on 29 Aug (1377786433), Jan Beulich wrote:
> >>> On 29.08.13 at 15:11, Tim Deegan <tim@xen.org> wrote:
> > At 13:30 +0100 on 29 Aug (1377783058), Jan Beulich wrote:
> >> >>> On 29.08.13 at 13:47, Tim Deegan <tim@xen.org> wrote:
> >> > At 16:31 +0100 on 26 Aug (1377534696), Jan Beulich wrote:
> >> >> -static inline unsigned long __vmread_safe(unsigned long field, int *error)
> >> >> +static inline bool_t __vmread_safe(unsigned long field, unsigned long 
> > *value)
> >> >>  {
> >> >> -    unsigned long ecx;
> >> >> +    bool_t okay;
> >> >>  
> >> >> -    asm volatile ( VMREAD_OPCODE
> >> >> -                   MODRM_EAX_ECX
> >> >> -                   /* CF==1 or ZF==1 --> rc = -1 */
> >> >> -                   "setna %b0 ; neg %0"
> >> >> -                   : "=q" (*error), "=c" (ecx)
> >> >> -                   : "0" (0), "a" (field)
> >> >> +    asm volatile (
> >> >> +#ifdef HAVE_GAS_VMX
> >> >> +                   "vmread %2, %1\n\t"
> >> >> +#else
> >> >> +                   VMREAD_OPCODE MODRM_EAX_ECX
> >> >> +#endif
> >> >> +                   /* CF==1 or ZF==1 --> rc = 0 */
> >> >> +                   "setnbe %0"
> >> > 
> >> > This inversion of the (undocumented) return value could be a nasty
> >> > surprise for anyone backporting code that uses __vmread_safe().  Can you
> >> > please leave it as it was?
> >> 
> >> The prior return value was the value read
> > 
> > Sorry, I had somehow missed this.  That's enough to cause compile issues
> > so I guess it'll be obvious that the function has changed.  Would be
> > nice if the new verions had a comment to say when it returns 0 and when
> > 1.
> 
> If I had named the used variable e.g. "ret" or "rc", I could see the
> need for a comment. It being named "okay" I rather think a
> comment would redundant. But if you're strongly of different
> opinion, I can certainly add such a comment.

No, leave it if you prefer.  It occurs to me that the useful case would
be for someone going the other way (with a patch against xen-unstable
and porting to an older tree) which would require a comment in the
version being removed. :)

Tim.