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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

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