Re: [PATCH/RFC] Let gcc to choose which registers to save

[Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>]

Avi Kivity a écrit :
> Laurent Vivier wrote:
>> As x86_64 ABI defines some registers saved by the calling function, it 
>> is not
>> needed to save all registers in the called function when switching to 
>> VCPU.
>> (see http://www.x86-64.org/documentation/abi.pdf, chapter 3.2.1)
>>
>> The best way to do that is to inform GCC which registers we use and let
>> it to save only needed registers.
>>
>>   
> 
> Strange, yesterday I started to do the same thing but dropped it after I 
> got discouraged by reload errors from gcc.

In french, we say "Les beaux esprits se rencontrent" (Voltaire) ;-)
("Great minds think alike")

>> diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
>> index 2c6b64a..d6c91ac 100644
>> --- a/drivers/kvm/vmx.c
>> +++ b/drivers/kvm/vmx.c
>> @@ -2243,16 +2243,12 @@ static void vmx_vcpu_run(struct kvm_vcpu 
>> *vcpu, struct kvm_run *kvm_run)
>>      asm(
>>          /* Store host registers */
>>  #ifdef CONFIG_X86_64
>> -        "push %%rax; push %%rbx; push %%rdx;"
>> -        "push %%rsi; push %%rdi; push %%rbp;"
>> -        "push %%r8;  push %%r9;  push %%r10; push %%r11;"
>> -        "push %%r12; push %%r13; push %%r14; push %%r15;"
>> +        "push %%rdx; push %%rbp;"
>>          "push %%rcx \n\t"
>> -        ASM_VMX_VMWRITE_RSP_RDX "\n\t"
>>  #else
>>          "pusha; push %%ecx \n\t"
>> -        ASM_VMX_VMWRITE_RSP_RDX "\n\t"
>>  #endif
>> +        ASM_VMX_VMWRITE_RSP_RDX "\n\t"
>>          /* Check if vmlaunch of vmresume is needed */
>>          "cmp $0, %1 \n\t"
>>          /* Load guest registers.  Don't clobber flags. */
>> @@ -2311,12 +2307,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu, 
>> struct kvm_run *kvm_run)
>>          "mov %%r15, %c[r15](%3) \n\t"
>>          "mov %%cr2, %%rax   \n\t"
>>          "mov %%rax, %c[cr2](%3) \n\t"
>> -        "mov (%%rsp), %3 \n\t"
>>  
>> -        "pop  %%rcx; pop  %%r15; pop  %%r14; pop  %%r13; pop  %%r12;"
>> -        "pop  %%r11; pop  %%r10; pop  %%r9;  pop  %%r8;"
>> -        "pop  %%rbp; pop  %%rdi; pop  %%rsi;"
>> -        "pop  %%rdx; pop  %%rbx; pop  %%rax \n\t"
>> +        "pop  %%rcx; pop  %%rbp; pop  %%rdx \n\t"
>>  #else
>>          "xchg %3, (%%esp) \n\t"
>>          "mov %%eax, %c[rax](%3) \n\t"
>> @@ -2354,7 +2346,12 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu, 
>> struct kvm_run *kvm_run)
>>          [r15]"i"(offsetof(struct kvm_vcpu, regs[VCPU_REGS_R15])),
>>  #endif
>>          [cr2]"i"(offsetof(struct kvm_vcpu, cr2))
>> -          : "cc", "memory");
>> +          : "cc", "memory",
>> +#ifdef CONFIG_X86_64
>> +        "rbx", "rdi", "rsi",
>> +        "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"
>> +#endif
>> +          );
>>  
> 
> The comma after "memory" worries me.  Can you compile-test on i386?

You're right, I thought I've corrected this. I rework this and test on i386.

> Other than that the patch is very welcome -- the excessive register 
> saving is very annoying to me.

I think we can do the same thing with svm.c, but I can't test it.

Regards,
Laurent
-- 
---------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  -----------------
"Given enough eyeballs, all bugs are shallow" E. S. Raymond


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/