[PATCH] SVM: do not generate "external interrupt exit" if other exit is pending

[PATCH] SVM: do not generate "external interrupt exit" if other exit is pending

[Gleb Natapov]

Nested SVM checks for external interrupt after injecting nested exception.
In case there is external interrupt pending the code generates "external
interrupt exit" and overwrites previous exit info. If previously injected
exception already generated exit it will be lost.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 43f5558..1a1f86b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1707,6 +1707,9 @@ static inline bool nested_svm_intr(struct vcpu_svm *svm)
 	if (!(svm->vcpu.arch.hflags & HF_HIF_MASK))
 		return false;
 
+	if (svm->nested.exit_required)
+		return false;
+
 	svm->vmcb->control.exit_code   = SVM_EXIT_INTR;
 	svm->vmcb->control.exit_info_1 = 0;
 	svm->vmcb->control.exit_info_2 = 0;
--
			Gleb.

Re: [PATCH] SVM: do not generate "external interrupt exit" if other exit is pending

[Joerg Roedel]

On Sun, Sep 19, 2010 at 06:41:27PM +0200, Gleb Natapov wrote:
> Nested SVM checks for external interrupt after injecting nested exception.
> In case there is external interrupt pending the code generates "external
> interrupt exit" and overwrites previous exit info. If previously injected
> exception already generated exit it will be lost.

Right. Have you seen specific mismehavior due to this problem? I am just
curious how you found this :-)
And another question, can you put the reason for this "if(...) return"
into an comment before the statement? The whole svm-emulation thing is
complicated enough so that we should document it well in the code :-)

	Joerg

> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 43f5558..1a1f86b 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1707,6 +1707,9 @@ static inline bool nested_svm_intr(struct vcpu_svm *svm)
>  	if (!(svm->vcpu.arch.hflags & HF_HIF_MASK))
>  		return false;
>  
> +	if (svm->nested.exit_required)
> +		return false;
> +
>  	svm->vmcb->control.exit_code   = SVM_EXIT_INTR;
>  	svm->vmcb->control.exit_info_1 = 0;
>  	svm->vmcb->control.exit_info_2 = 0;
> --
> 			Gleb.

Re: [PATCH] SVM: do not generate "external interrupt exit" if other exit is pending

[Gleb Natapov]

On Sun, Sep 19, 2010 at 07:29:41PM +0200, Joerg Roedel wrote:
> On Sun, Sep 19, 2010 at 06:41:27PM +0200, Gleb Natapov wrote:
> > Nested SVM checks for external interrupt after injecting nested exception.
> > In case there is external interrupt pending the code generates "external
> > interrupt exit" and overwrites previous exit info. If previously injected
> > exception already generated exit it will be lost.
> 
> Right. Have you seen specific mismehavior due to this problem? I am just
> curious how you found this :-)
Yes. Trying to make async page fault work with nested svm :) It was hard
to fix with reproducible test case. I am not dreaming to fix it just by
code review.

> And another question, can you put the reason for this "if(...) return"
> into an comment before the statement? The whole svm-emulation thing is
> complicated enough so that we should document it well in the code :-)
> 
Complicated is understatement. Will add a comment.

> 	Joerg
> 
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index 43f5558..1a1f86b 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -1707,6 +1707,9 @@ static inline bool nested_svm_intr(struct vcpu_svm *svm)
> >  	if (!(svm->vcpu.arch.hflags & HF_HIF_MASK))
> >  		return false;
> >  
> > +	if (svm->nested.exit_required)
> > +		return false;
> > +
> >  	svm->vmcb->control.exit_code   = SVM_EXIT_INTR;
> >  	svm->vmcb->control.exit_info_1 = 0;
> >  	svm->vmcb->control.exit_info_2 = 0;
> > --
> > 			Gleb.

--
			Gleb.

Re: [PATCH] SVM: do not generate "external interrupt exit" if other exit is pending

[Joerg Roedel]

On Sun, Sep 19, 2010 at 07:50:27PM +0200, Gleb Natapov wrote:
> On Sun, Sep 19, 2010 at 07:29:41PM +0200, Joerg Roedel wrote:
> > On Sun, Sep 19, 2010 at 06:41:27PM +0200, Gleb Natapov wrote:
> > > Nested SVM checks for external interrupt after injecting nested exception.
> > > In case there is external interrupt pending the code generates "external
> > > interrupt exit" and overwrites previous exit info. If previously injected
> > > exception already generated exit it will be lost.
> > 
> > Right. Have you seen specific mismehavior due to this problem? I am just
> > curious how you found this :-)
> Yes. Trying to make async page fault work with nested svm :) It was hard
> to fix with reproducible test case. I am not dreaming to fix it just by
> code review.

Interesting. Hope this was the only problem and you have it working now?

> > And another question, can you put the reason for this "if(...) return"
> > into an comment before the statement? The whole svm-emulation thing is
> > complicated enough so that we should document it well in the code :-)
> > 
> Complicated is understatement. Will add a comment.

Thanks, the most complicated thing is to get all the interrupt-
exception- and event-reinjection thing right. You just fixed another bug
there, thanks again :)

	Joerg

Re: [PATCH] SVM: do not generate "external interrupt exit" if other exit is pending

[Gleb Natapov]

On Sun, Sep 19, 2010 at 08:19:33PM +0200, Joerg Roedel wrote:
> On Sun, Sep 19, 2010 at 07:50:27PM +0200, Gleb Natapov wrote:
> > On Sun, Sep 19, 2010 at 07:29:41PM +0200, Joerg Roedel wrote:
> > > On Sun, Sep 19, 2010 at 06:41:27PM +0200, Gleb Natapov wrote:
> > > > Nested SVM checks for external interrupt after injecting nested exception.
> > > > In case there is external interrupt pending the code generates "external
> > > > interrupt exit" and overwrites previous exit info. If previously injected
> > > > exception already generated exit it will be lost.
> > > 
> > > Right. Have you seen specific mismehavior due to this problem? I am just
> > > curious how you found this :-)
> > Yes. Trying to make async page fault work with nested svm :) It was hard
> > to fix with reproducible test case. I am not dreaming to fix it just by
> > code review.
> 
> Interesting. Hope this was the only problem and you have it working now?
> 
Yes. Now my test case works.

--
			Gleb.