From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v4 3/4] VMX: use proper instruction mnemonics if assembler supports them Date: Mon, 26 Aug 2013 16:07:45 +0100 Message-ID: <521B6F41.6090204@citrix.com> References: <521786A002000078000EE064@nat28.tlf.novell.com> <521787C602000078000EE07F@nat28.tlf.novell.com> <52193148.2090601@citrix.com> <521B36AA02000078000EE496@nat28.tlf.novell.com> <521B1F15.1030104@citrix.com> <521B479902000078000EE505@nat28.tlf.novell.com> <521B528C.8080107@citrix.com> <521B7C6402000078000EE6D4@nat28.tlf.novell.com> <521B63AF.7000008@citrix.com> <521B825D02000078000EE72D@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VDyOS-00084D-EM for xen-devel@lists.xenproject.org; Mon, 26 Aug 2013 15:07:48 +0000 In-Reply-To: <521B825D02000078000EE72D@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser , Eddie Dong , Jun Nakajima List-Id: xen-devel@lists.xenproject.org On 26/08/2013 15:29, Jan Beulich wrote: >>>> On 26.08.13 at 16:18, Andrew Cooper wrote: >> On 26/08/2013 15:03, Jan Beulich wrote: >>> + 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" >>> +#ifdef HAVE_GAS_VMX >>> + : "=qm" (okay), "=rm" (*value) >>> + : "r" (field) >>> +#else >>> + : "=qm" (okay), "=c" (*value) >>> + : "a" (field) >>> +#endif >> From what I can work out while googling, the q constraint is equivalent >> to the r constraint for 64bit code. >> >> For consistency sake, I would suggest "=rm" (okay) here > And I'd like to keep it the way it is for generality's sake (i.e. not > making the code more 32-bit unclean than we need to). Ok > >>> @@ -365,14 +398,22 @@ static inline void __invept(int type, u6 >>> !cpu_has_vmx_ept_invept_single_context ) >>> type = INVEPT_ALL_CONTEXT; >>> >>> - asm volatile ( INVEPT_OPCODE >>> - MODRM_EAX_08 >>> + asm volatile ( >>> +#ifdef HAVE_GAS_EPT >>> + "invept %0, %q1\n" >> Another stray q > No - operand 1 is of type "int", and while the high 32 bits get > ignored (i.e. we don't need to do any zero- or sign-extension), we > still need to specify the 64-bit register name here. Or wait - I > thought it would ignore the upper bits, but it's not documented to. > In which case this involves more than just dropping the q modifier. I was more referring to having a q in the instruction, yet an "r" in the parameter list. I would suggest INV{EPT,VPID} is strictly defined to take r64 as the "type" parameter in long mode. Invalid/unsupported values found in this register can be detected based on the state of EFLAGS afterwards. Therefore, I would suggest possibly changing "int type" to "unsigned long type" if we are going to the effort of getting this correct. It shouldn't make a difference currently, as all calls use appropriate INVEPT_*_CONTEXT defines. As for the flags, should we be including "cc" to the clobber list as each of the VM*/INV* instructions explicitly sets the flags. I would hope that the toolchain is pessimistic enough to not trust the state of the flags across some inline assembly, but I can't find any hard information one way or another. ~Andrew