Re: [PATCH v3 02/26] KVM: Split cpuid register access from computation

[Avi Kivity <avi@redhat.com>]

On 07/03/2012 08:34 PM, Marcelo Tosatti wrote:
> On Wed, Jun 27, 2012 at 06:24:50PM +0300, Avi Kivity wrote:
>> Introduce kvm_cpuid() to perform the leaf limit check and calculate
>> register values, and let kvm_emulate_cpuid() just handle reading and
>> writing the registers from/to the vcpu.  This allows us to reuse
>> kvm_cpuid() in a context where directly reading and writing registers
>> is not desired.
>> 
>> Signed-off-by: Avi Kivity <avi@redhat.com>
>> ---
>>  arch/x86/kvm/cpuid.c | 38 ++++++++++++++++++++++++--------------
>>  arch/x86/kvm/cpuid.h |  1 +
>>  2 files changed, 25 insertions(+), 14 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 7df1c6d..44476fb 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -639,33 +639,43 @@ static struct kvm_cpuid_entry2* check_cpuid_limit(struct kvm_vcpu *vcpu,
>>  	return kvm_find_cpuid_entry(vcpu, maxlevel->eax, index);
>>  }
>>  
>> -void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>> +void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
>>  {
>> -	u32 function, index;
>> +	u32 function = *eax, index = *ecx;
>>  	struct kvm_cpuid_entry2 *best;
>>  
>> -	function = kvm_register_read(vcpu, VCPU_REGS_RAX);
>> -	index = kvm_register_read(vcpu, VCPU_REGS_RCX);
>> -	kvm_register_write(vcpu, VCPU_REGS_RAX, 0);
>> -	kvm_register_write(vcpu, VCPU_REGS_RBX, 0);
>> -	kvm_register_write(vcpu, VCPU_REGS_RCX, 0);
>> -	kvm_register_write(vcpu, VCPU_REGS_RDX, 0);
>>  	best = kvm_find_cpuid_entry(vcpu, function, index);
>>  
>>  	if (!best)
>>  		best = check_cpuid_limit(vcpu, function, index);
>>  
>>  	if (best) {
>> -		kvm_register_write(vcpu, VCPU_REGS_RAX, best->eax);
>> -		kvm_register_write(vcpu, VCPU_REGS_RBX, best->ebx);
>> -		kvm_register_write(vcpu, VCPU_REGS_RCX, best->ecx);
>> -		kvm_register_write(vcpu, VCPU_REGS_RDX, best->edx);
>> -	}
>> -	kvm_x86_ops->skip_emulated_instruction(vcpu);
>> +		*eax = best->eax;
>> +		*ebx = best->ebx;
>> +		*ecx = best->ecx;
>> +		*edx = best->edx;
>> +	} else
>> +		*eax = *ebx = *ecx = *edx = 0;
>> +
>>  	trace_kvm_cpuid(function,
>>  			kvm_register_read(vcpu, VCPU_REGS_RAX),
>>  			kvm_register_read(vcpu, VCPU_REGS_RBX),
>>  			kvm_register_read(vcpu, VCPU_REGS_RCX),
>>  			kvm_register_read(vcpu, VCPU_REGS_RDX));
>>  }
>> +
>> +void kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>> +{
>> +	u32 function, eax, ebx, ecx, edx;
>> +
>> +	function = eax = kvm_register_read(vcpu, VCPU_REGS_RAX);
>> +	ecx = kvm_register_read(vcpu, VCPU_REGS_RCX);
>> +	kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx);
>> +	kvm_register_write(vcpu, VCPU_REGS_RAX, eax);
>> +	kvm_register_write(vcpu, VCPU_REGS_RBX, ebx);
>> +	kvm_register_write(vcpu, VCPU_REGS_RCX, ecx);
>> +	kvm_register_write(vcpu, VCPU_REGS_RDX, edx);
>> +	kvm_x86_ops->skip_emulated_instruction(vcpu);
>> +	trace_kvm_cpuid(function, eax, ebx, ecx, edx);
>> +}
> 
> Tracing is duplicated. Is that intented?

The first instance is bogus, since it's reading vcpu registers which
might not have been the input to the function.  I'll remove it.

-- 
error compiling committee.c: too many arguments to function