From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH 4/4] SVM: streamline entry.S code Date: Wed, 04 Sep 2013 12:42:18 -0400 Message-ID: <522762EA.20804@oracle.com> References: <521786A002000078000EE064@nat28.tlf.novell.com> <521787FA02000078000EE083@nat28.tlf.novell.com> <52274612.4040506@oracle.com> <522764E002000078000F06C7@nat28.tlf.novell.com> <52274D12.2090607@oracle.com> <52276BEB02000078000F0721@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VHG8R-0006cB-Da for xen-devel@lists.xenproject.org; Wed, 04 Sep 2013 16:40:51 +0000 In-Reply-To: <52276BEB02000078000F0721@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 , Jacob Shin , suravee.suthikulpanit@amd.com List-Id: xen-devel@lists.xenproject.org On 09/04/2013 11:20 AM, Jan Beulich wrote: >>>> On 04.09.13 at 17:09, Boris Ostrovsky wrote: >> I meant something like this: >> >> diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S >> index 1969629..b362637 100644 >> --- a/xen/arch/x86/hvm/svm/entry.S >> +++ b/xen/arch/x86/hvm/svm/entry.S >> @@ -92,10 +92,12 @@ UNLIKELY_END(svm_trace) >> >> VMRUN >> >> + GET_CURRENT(%rax) >> push %rdi >> push %rsi >> push %rdx >> push %rcx >> + mov VCPU_svm_vmcb(%rax),%rcx >> push %rax >> push %r8 >> push %r9 >> @@ -108,17 +110,15 @@ UNLIKELY_END(svm_trace) >> push %r14 >> push %r15 >> >> - GET_CURRENT(%rbx) >> - movb $0,VCPU_svm_vmcb_in_sync(%rbx) >> - mov VCPU_svm_vmcb(%rbx),%rcx >> - mov VMCB_rax(%rcx),%rax >> - mov %rax,UREGS_rax(%rsp) >> - mov VMCB_rip(%rcx),%rax >> - mov %rax,UREGS_rip(%rsp) >> - mov VMCB_rsp(%rcx),%rax >> - mov %rax,UREGS_rsp(%rsp) >> - mov VMCB_rflags(%rcx),%rax >> - mov %rax,UREGS_eflags(%rsp) >> + movb $0,VCPU_svm_vmcb_in_sync(%rax) >> + mov VMCB_rax(%rcx),%rdi >> + mov %rdi,UREGS_rax(%rsp) >> + mov VMCB_rip(%rcx),%rdi >> + mov %rdi,UREGS_rip(%rsp) >> + mov VMCB_rsp(%rcx),%rdi >> + mov %rdi,UREGS_rsp(%rsp) >> + mov VMCB_rflags(%rcx),%rdi >> + mov %rdi,UREGS_eflags(%rsp) >> >> #ifndef NDEBUG >> mov $0xbeef,%ax >> ostr@workbase> >> >> >> >> %rax is clobbered anyway by ' mov VMCB_rax(%rcx),%rax' > But the "current" pointer also isn't needed anymore after the clearing > of VCPU_svm_vmcb_in_sync and before doing the first function call > (it's needed after the function call, but for that it's can't be in %rax). > > So yes, that clearing of VCPU_svm_vmcb_in_sync could certainly be > done using %rax instead of %rbx, but no other code change would > seem necessary/desirable. If you agree, I'd be willing to do this one > line adjustment. Oh, yes, that's right, no reason to change other lines. My only reason for suggesting this change though was to eliminate 'mov %rax,%rbx' instruction (in your patch, not in the code above). If you want to keep it in though then I don't think the line adjustment that you are talking about is needed. -boris