Re: [PATCH 2/5] kvm/svm: enhance MOV CR intercept handler

[Andre Przywara <andre.przywara@amd.com>]

Avi Kivity wrote:
> On 12/07/2010 12:59 PM, Andre Przywara wrote:
>> Newer SVM implementations provide the GPR number in the VMCB, so
>> that the emulation path is no longer necesarry to handle CR
>> register access intercepts. Implement the handling in svm.c and
>> use it when the info is provided.
>>
>> Signed-off-by: Andre Przywara<andre.przywara@amd.com>
>> ---
>>   arch/x86/include/asm/svm.h |    2 +
>>   arch/x86/kvm/svm.c         |   74 +++++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 68 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index 11dbca7..589fc25 100644
>> --- a/arch/x86/include/asm/svm.h
>> +++ b/arch/x86/include/asm/svm.h
>> ...
>> @@ -2864,14 +2922,14 @@ static int pause_interception(struct vcpu_svm *svm)
>>   }
>>
>>   static int (*svm_exit_handlers[])(struct vcpu_svm *svm) = {
>> -	[SVM_EXIT_READ_CR0]			= emulate_on_interception,
>> -	[SVM_EXIT_READ_CR3]			= emulate_on_interception,
>> -	[SVM_EXIT_READ_CR4]			= emulate_on_interception,
>> -	[SVM_EXIT_READ_CR8]			= emulate_on_interception,
>> +	[SVM_EXIT_READ_CR0]			= cr_interception,
>> +	[SVM_EXIT_READ_CR3]			= cr_interception,
>> +	[SVM_EXIT_READ_CR4]			= cr_interception,
>> +	[SVM_EXIT_READ_CR8]			= cr_interception,
>>   	[SVM_EXIT_CR0_SEL_WRITE]		= emulate_on_interception,
>>   	[SVM_EXIT_WRITE_CR0]			= cr0_write_interception,
>> -	[SVM_EXIT_WRITE_CR3]			= emulate_on_interception,
>> -	[SVM_EXIT_WRITE_CR4]			= emulate_on_interception,
>> +	[SVM_EXIT_WRITE_CR3]			= cr_interception,
>> +	[SVM_EXIT_WRITE_CR4]			= cr_interception,
>>   	[SVM_EXIT_WRITE_CR8]			= cr8_write_interception,
> 
> We could move cr[08]_write_interception into cr_interception, but that 
> takes a bit more thought.  Best done later.
Yes, I thought about that, too. But since we still have to deal with the 
emulation code path, it would not make the code easier. But if we 
overwrite the svm_exit_handlers[] on detecting the SVM feature, this 
could make more sense. I will check this out.

Thanks for the review, I will address your comments.

Regards,
Andre.