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:10:15 +0100 Message-ID: <521B6FD7.9060908@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> <521B6F41.6090204@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VDyQs-0008FW-QT for xen-devel@lists.xenproject.org; Mon, 26 Aug 2013 15:10:19 +0000 In-Reply-To: <521B6F41.6090204@citrix.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 16:07, Andrew Cooper wrote: > 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 Sorry - sent early. I would suggest putting all constraints beside their values, rather than mixing them in amongst the asm code itself, but do admit that this is only a matter of preference. > > 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 > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel