kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] VMX: Return to userspace on invalid state emulation failure
@ 2009-08-24  4:07 Mohammed Gamal
  0 siblings, 0 replies; 6+ messages in thread
From: Mohammed Gamal @ 2009-08-24  4:07 UTC (permalink / raw)
  To: avi; +Cc: kvm, Mohammed Gamal

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 |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1ee811c..6030671 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3341,6 +3341,8 @@ static void handle_invalid_guest_state(struct kvm_vcpu *vcpu,
 
 		if (err != EMULATE_DONE) {
 			kvm_report_emulation_failure(vcpu, "emulation failure");
+			kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+			kvm_run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
 			break;
 		}
 
@@ -3612,7 +3614,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		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
+		&& kvm_run->internal.suberror != KVM_INTERNAL_ERROR_EMULATION) {
 		handle_invalid_guest_state(vcpu, kvm_run);
 		return;
 	}
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] VMX: Return to userspace on invalid state emulation failure
@ 2009-08-24 11:51 Mohammed Gamal
  0 siblings, 0 replies; 6+ messages in thread
From: Mohammed Gamal @ 2009-08-24 11:51 UTC (permalink / raw)
  To: avi; +Cc: kvm, Mohammed Gamal

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 1ee811c..423e44f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3341,6 +3341,8 @@ static void handle_invalid_guest_state(struct kvm_vcpu *vcpu,
 
 		if (err != EMULATE_DONE) {
 			kvm_report_emulation_failure(vcpu, "emulation failure");
+			kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+			kvm_run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
 			break;
 		}
 
@@ -3612,7 +3614,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		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
+		&& !(kvm_run->exit_reason == KVM_EXIT_INTERNAL_ERROR && 
+		  kvm_run->internal.suberror == KVM_INTERNAL_ERROR_EMULATION)) {
 		handle_invalid_guest_state(vcpu, kvm_run);
 		return;
 	}
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] VMX: Return to userspace on invalid state emulation failure
@ 2009-08-24 22:37 Mohammed Gamal
  2009-08-26  9:02 ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Mohammed Gamal @ 2009-08-24 22:37 UTC (permalink / raw)
  To: avi; +Cc: kvm, Mohammed Gamal

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;
 	}
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: Return to userspace on invalid state emulation failure
  2009-08-24 22:37 [PATCH] VMX: Return to userspace on invalid state emulation failure Mohammed Gamal
@ 2009-08-26  9:02 ` Avi Kivity
  2009-08-26 10:07   ` Mohammed Gamal
  0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-08-26  9:02 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: kvm

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).


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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: Return to userspace on invalid state emulation failure
  2009-08-26  9:02 ` Avi Kivity
@ 2009-08-26 10:07   ` Mohammed Gamal
  2009-08-26 10:11     ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Mohammed Gamal @ 2009-08-26 10:07 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VMX: Return to userspace on invalid state emulation failure
  2009-08-26 10:07   ` Mohammed Gamal
@ 2009-08-26 10:11     ` Avi Kivity
  0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2009-08-26 10:11 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: kvm

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-08-26 10:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-24 22:37 [PATCH] VMX: Return to userspace on invalid state emulation failure Mohammed Gamal
2009-08-26  9:02 ` Avi Kivity
2009-08-26 10:07   ` Mohammed Gamal
2009-08-26 10:11     ` Avi Kivity
  -- strict thread matches above, loose matches on Subject: below --
2009-08-24 11:51 Mohammed Gamal
2009-08-24  4:07 Mohammed Gamal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).