Re: [PATCH] VMX: Return to userspace on invalid state emulation failure

[Avi Kivity <avi@redhat.com>]

On 08/26/2009 01:07 PM, Mohammed Gamal wrote:
> On Wed, Aug 26, 2009 at 12:02 PM, Avi Kivity<avi@redhat.com>  wrote:
>    
>> On 08/25/2009 01:37 AM, Mohammed Gamal wrote:
>>      
>>> Return to userspace instead of repeatedly trying to emulate
>>> instructions that have already failed
>>>
>>> Signed-off-by: Mohammed Gamal<m.gamal005@gmail.com>
>>> ---
>>>   arch/x86/kvm/vmx.c |    6 +++++-
>>>   1 files changed, 5 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 6b57eed..c559bb7 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -3337,6 +3337,8 @@ static void handle_invalid_guest_state(struct
>>> kvm_vcpu *vcpu)
>>>
>>>                 if (err != EMULATE_DONE) {
>>>                         kvm_report_emulation_failure(vcpu, "emulation
>>> failure");
>>> +                       vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>>> +                       vcpu->run->internal.suberror =
>>> KVM_INTERNAL_ERROR_EMULATION;
>>>                         break;
>>>                 }
>>>
>>> @@ -3607,7 +3609,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
>>>                 vmx->entry_time = ktime_get();
>>>
>>>         /* Handle invalid guest state instead of entering VMX */
>>> -       if (vmx->emulation_required&&    emulate_invalid_guest_state) {
>>> +       if (vmx->emulation_required&&    emulate_invalid_guest_state
>>> +&&    !(vcpu->run->exit_reason == KVM_EXIT_INTERNAL_ERROR&&
>>> +                 vcpu->run->internal.suberror ==
>>> KVM_INTERNAL_ERROR_EMULATION)) {
>>>                 handle_invalid_guest_state(vcpu);
>>>                 return;
>>>         }
>>>
>>>        
>> Still suffers from the same problem.  You don't always update
>> vcpu->run->exit_reason, so you can't test it.  Best to return a value from
>> handle_invalid_guest_state() (the standard return codes for exit handlers
>> are 1 for return-to-guest, 0 for return-to-host, and -errno to return with
>> an error).
>>
>>      
> I was thinking of the same idea since I was also concerned about
> vcpu->run->exit_reason not being updated. But how can we interpret the
> return values of handle_invalid_guest_state() inside vmx_vcpu_run()
> since it doesn't have a return value. Or would it be better to move
> handle_invalid_guest_state() to the standard vmx exit handlers?
>    

We can move the call to vmx_handle_exit().  We have a check for 
emulate_invalid_guest_state there anyway.  I don't think it should be a 
standard exit handler since there is no exit_reason for it.

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