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 11:09:06 -0400 Message-ID: <52274D12.2090607@oracle.com> References: <521786A002000078000EE064@nat28.tlf.novell.com> <521787FA02000078000EE083@nat28.tlf.novell.com> <52274612.4040506@oracle.com> <522764E002000078000F06C7@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.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VHEgD-0001aq-Bu for xen-devel@lists.xenproject.org; Wed, 04 Sep 2013 15:07:37 +0000 In-Reply-To: <522764E002000078000F06C7@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 10:50 AM, Jan Beulich wrote: >>>> On 04.09.13 at 16:39, Boris Ostrovsky wrote: >> On 08/23/2013 10:04 AM, Jan Beulich wrote: >>> @@ -92,25 +97,26 @@ 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 >>> push %r10 >>> push %r11 >>> push %rbx >>> + mov %rax,%rbx >> Can you continue with using %rax as 'current' pointer below and not save it >> to %rbx? %rax appears to be a temp register so perhaps you can use, say, >> %rdi, for that purpose. > I'm sorry, but I don't understand what you're asking for. > > We actually _want_ "current" to be in a register thats callee-saved, > such that we don't need to reload it after function calls. One goal of > the patch in fact is to eliminate such redundant reloads. But as I'm > not sure what you want, this explanation may be entirely off. 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' > > And as a side note: At this point I'm not really expecting requests > for further changes, unless you spot a mistake in the patch. It was > around for review for long enough, and as said it had two reviews > already. And further optimization should go in a separate, > incremental patch. > Right, this was more of an observation, not a mistake in your patch. Feel free to ignore it (assuming it's even correct). -boris