Re: [Xen-devel] [PATCH v5 3/4] VMX: use proper instruction mnemonics if assembler supports them

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.08.13 at 15:11, Tim Deegan <tim@xxxxxxx> wrote:
> At 13:30 +0100 on 29 Aug (1377783058), Jan Beulich wrote:
>> >>> On 29.08.13 at 13:47, Tim Deegan <tim@xxxxxxx> 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.

Jan


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