[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 4/4] SVM: streamline entry.S code



On 09/04/2013 10:50 AM, Jan Beulich wrote:
On 04.09.13 at 16:39, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 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

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.