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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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



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