| Message ID | 1253278832-31803-3-git-send-email-agraf@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Alexander Graf wrote: > SVM has some cleanup code, that tries to reinject interrupts and exceptions > when the guest didn't manage to deal with them yet. It basically transfers > them to KVM internal state. > > Unfortunately, the internal state is reserved for the L1 guest state, so we > shouldn't try to go through that logic when running a nested guest. > > When doing something the host KVM can handle, let's just reinject the event > into the L2 guest, because we didn't touch its state anyways. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > arch/x86/kvm/svm.c | 18 ++++++++++++++---- > 1 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index f12a669..61efd13 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2349,7 +2349,10 @@ static int handle_exit(struct kvm_vcpu *vcpu) > trace_kvm_exit(exit_code, svm->vmcb->save.rip); > > if (is_nested(svm)) { > + struct vmcb_control_area *control = &svm->vmcb->control; > int vmexit; > + int type; > + int vec; > > nsvm_printk("nested handle_exit: 0x%x | 0x%lx | 0x%lx | 0x%lx\n", > exit_code, svm->vmcb->control.exit_info_1, > @@ -2362,9 +2365,18 @@ static int handle_exit(struct kvm_vcpu *vcpu) > > if (vmexit == NESTED_EXIT_DONE) > return 1; > - } > > - svm_complete_interrupts(svm); > + type = control->exit_int_info & SVM_EXITINTINFO_TYPE_MASK; > + vec = control->exit_int_info & SVM_EXITINTINFO_VEC_MASK; > + if ((type == SVM_EXITINTINFO_TYPE_INTR) || > + ((type == SVM_EXITINTINFO_TYPE_EXEPT) && !kvm_exception_is_soft(vec))) { > + control->event_inj = control->exit_int_info; > + control->event_inj_err = control->exit_int_info_err; > + } > + } else { > + /* Don't interpret exit_info for nested guests */ Doesn't this comment belong to the block above? > + svm_complete_interrupts(svm); > + } > > if (npt_enabled) { > int mmu_reload = 0; > @@ -2602,8 +2614,6 @@ static void svm_complete_interrupts(struct vcpu_svm *svm) > case SVM_EXITINTINFO_TYPE_EXEPT: > /* In case of software exception do not reinject an exception > vector, but re-execute and instruction instead */ > - if (is_nested(svm)) > - break; > if (kvm_exception_is_soft(vector)) > break; > if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) { Jan
On Fri, Sep 18, 2009 at 03:00:29PM +0200, Alexander Graf wrote: > SVM has some cleanup code, that tries to reinject interrupts and exceptions > when the guest didn't manage to deal with them yet. It basically transfers > them to KVM internal state. > > Unfortunately, the internal state is reserved for the L1 guest state, so we > shouldn't try to go through that logic when running a nested guest. > > When doing something the host KVM can handle, let's just reinject the event > into the L2 guest, because we didn't touch its state anyways. I don't really understandt what problem this patch addresses. There are situations where we have events to reinject into the l2 guest directly. But the generic reinjection code works fine for it. The only problematic thing with it is that it implicitly relies on exit_int_info not to be changed in the exit cycle (which would be worth a comment). Joerg > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > arch/x86/kvm/svm.c | 18 ++++++++++++++---- > 1 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index f12a669..61efd13 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2349,7 +2349,10 @@ static int handle_exit(struct kvm_vcpu *vcpu) > trace_kvm_exit(exit_code, svm->vmcb->save.rip); > > if (is_nested(svm)) { > + struct vmcb_control_area *control = &svm->vmcb->control; > int vmexit; > + int type; > + int vec; > > nsvm_printk("nested handle_exit: 0x%x | 0x%lx | 0x%lx | 0x%lx\n", > exit_code, svm->vmcb->control.exit_info_1, > @@ -2362,9 +2365,18 @@ static int handle_exit(struct kvm_vcpu *vcpu) > > if (vmexit == NESTED_EXIT_DONE) > return 1; > - } > > - svm_complete_interrupts(svm); > + type = control->exit_int_info & SVM_EXITINTINFO_TYPE_MASK; > + vec = control->exit_int_info & SVM_EXITINTINFO_VEC_MASK; > + if ((type == SVM_EXITINTINFO_TYPE_INTR) || > + ((type == SVM_EXITINTINFO_TYPE_EXEPT) && !kvm_exception_is_soft(vec))) { > + control->event_inj = control->exit_int_info; > + control->event_inj_err = control->exit_int_info_err; > + } > + } else { > + /* Don't interpret exit_info for nested guests */ > + svm_complete_interrupts(svm); > + } > > if (npt_enabled) { > int mmu_reload = 0; > @@ -2602,8 +2614,6 @@ static void svm_complete_interrupts(struct vcpu_svm *svm) > case SVM_EXITINTINFO_TYPE_EXEPT: > /* In case of software exception do not reinject an exception > vector, but re-execute and instruction instead */ > - if (is_nested(svm)) > - break; > if (kvm_exception_is_soft(vector)) > break; > if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) { > -- > 1.6.0.2 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 22.09.2009 um 18:26 schrieb Joerg Roedel <joro@8bytes.org>: > On Fri, Sep 18, 2009 at 03:00:29PM +0200, Alexander Graf wrote: >> SVM has some cleanup code, that tries to reinject interrupts and >> exceptions >> when the guest didn't manage to deal with them yet. It basically >> transfers >> them to KVM internal state. >> >> Unfortunately, the internal state is reserved for the L1 guest >> state, so we >> shouldn't try to go through that logic when running a nested guest. >> >> When doing something the host KVM can handle, let's just reinject >> the event >> into the L2 guest, because we didn't touch its state anyways. > > I don't really understandt what problem this patch addresses. There > are > situations where we have events to reinject into the l2 guest > directly. > But the generic reinjection code works fine for it. > The only problematic thing with it is that it implicitly relies on > exit_int_info not to be changed in the exit cycle (which would be > worth > a comment). It si > > Joerg > >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> arch/x86/kvm/svm.c | 18 ++++++++++++++---- >> 1 files changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index f12a669..61efd13 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -2349,7 +2349,10 @@ static int handle_exit(struct kvm_vcpu *vcpu) >> trace_kvm_exit(exit_code, svm->vmcb->save.rip); >> >> if (is_nested(svm)) { >> + struct vmcb_control_area *control = &svm->vmcb->control; >> int vmexit; >> + int type; >> + int vec; >> >> nsvm_printk("nested handle_exit: 0x%x | 0x%lx | 0x%lx | 0x%lx >> \n", >> exit_code, svm->vmcb->control.exit_info_1, >> @@ -2362,9 +2365,18 @@ static int handle_exit(struct kvm_vcpu *vcpu) >> >> if (vmexit == NESTED_EXIT_DONE) >> return 1; >> - } >> >> - svm_complete_interrupts(svm); >> + type = control->exit_int_info & SVM_EXITINTINFO_TYPE_MASK; >> + vec = control->exit_int_info & SVM_EXITINTINFO_VEC_MASK; >> + if ((type == SVM_EXITINTINFO_TYPE_INTR) || >> + ((type == SVM_EXITINTINFO_TYPE_EXEPT) && ! >> kvm_exception_is_soft(vec))) { >> + control->event_inj = control->exit_int_info; >> + control->event_inj_err = control->exit_int_info_err; >> + } >> + } else { >> + /* Don't interpret exit_info for nested guests */ >> + svm_complete_interrupts(svm); >> + } >> >> if (npt_enabled) { >> int mmu_reload = 0; >> @@ -2602,8 +2614,6 @@ static void svm_complete_interrupts(struct >> vcpu_svm *svm) >> case SVM_EXITINTINFO_TYPE_EXEPT: >> /* In case of software exception do not reinject an exception >> vector, but re-execute and instruction instead */ >> - if (is_nested(svm)) >> - break; >> if (kvm_exception_is_soft(vector)) >> break; >> if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) { >> -- >> 1.6.0.2 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 22.09.2009 um 18:26 schrieb Joerg Roedel <joro@8bytes.org>: > On Fri, Sep 18, 2009 at 03:00:29PM +0200, Alexander Graf wrote: >> SVM has some cleanup code, that tries to reinject interrupts and >> exceptions >> when the guest didn't manage to deal with them yet. It basically >> transfers >> them to KVM internal state. >> >> Unfortunately, the internal state is reserved for the L1 guest >> state, so we >> shouldn't try to go through that logic when running a nested guest. >> >> When doing something the host KVM can handle, let's just reinject >> the event >> into the L2 guest, because we didn't touch its state anyways. > > I don't really understandt what problem this patch addresses. There > are > situations where we have events to reinject into the l2 guest > directly. > But the generic reinjection code works fine for it. > The only problematic thing with it is that it implicitly relies on > exit_int_info not to be changed in the exit cycle (which would be > worth > a comment). It simply tries to be too clever. Reevaluating exceptions won't work for example. Alex > > Joerg > >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> arch/x86/kvm/svm.c | 18 ++++++++++++++---- >> 1 files changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index f12a669..61efd13 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -2349,7 +2349,10 @@ static int handle_exit(struct kvm_vcpu *vcpu) >> trace_kvm_exit(exit_code, svm->vmcb->save.rip); >> >> if (is_nested(svm)) { >> + struct vmcb_control_area *control = &svm->vmcb->control; >> int vmexit; >> + int type; >> + int vec; >> >> nsvm_printk("nested handle_exit: 0x%x | 0x%lx | 0x%lx | 0x%lx >> \n", >> exit_code, svm->vmcb->control.exit_info_1, >> @@ -2362,9 +2365,18 @@ static int handle_exit(struct kvm_vcpu *vcpu) >> >> if (vmexit == NESTED_EXIT_DONE) >> return 1; >> - } >> >> - svm_complete_interrupts(svm); >> + type = control->exit_int_info & SVM_EXITINTINFO_TYPE_MASK; >> + vec = control->exit_int_info & SVM_EXITINTINFO_VEC_MASK; >> + if ((type == SVM_EXITINTINFO_TYPE_INTR) || >> + ((type == SVM_EXITINTINFO_TYPE_EXEPT) && ! >> kvm_exception_is_soft(vec))) { >> + control->event_inj = control->exit_int_info; >> + control->event_inj_err = control->exit_int_info_err; >> + } >> + } else { >> + /* Don't interpret exit_info for nested guests */ >> + svm_complete_interrupts(svm); >> + } >> >> if (npt_enabled) { >> int mmu_reload = 0; >> @@ -2602,8 +2614,6 @@ static void svm_complete_interrupts(struct >> vcpu_svm *svm) >> case SVM_EXITINTINFO_TYPE_EXEPT: >> /* In case of software exception do not reinject an exception >> vector, but re-execute and instruction instead */ >> - if (is_nested(svm)) >> - break; >> if (kvm_exception_is_soft(vector)) >> break; >> if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) { >> -- >> 1.6.0.2 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 23, 2009 at 01:05:57AM -0700, Alexander Graf wrote: > > Am 22.09.2009 um 18:26 schrieb Joerg Roedel <joro@8bytes.org>: > > >On Fri, Sep 18, 2009 at 03:00:29PM +0200, Alexander Graf wrote: > >>SVM has some cleanup code, that tries to reinject interrupts and > >>exceptions > >>when the guest didn't manage to deal with them yet. It basically > >>transfers > >>them to KVM internal state. > >> > >>Unfortunately, the internal state is reserved for the L1 guest > >>state, so we > >>shouldn't try to go through that logic when running a nested guest. > >> > >>When doing something the host KVM can handle, let's just > >>reinject the event > >>into the L2 guest, because we didn't touch its state anyways. > > > >I don't really understandt what problem this patch addresses. > >There are > >situations where we have events to reinject into the l2 guest > >directly. > >But the generic reinjection code works fine for it. > >The only problematic thing with it is that it implicitly relies on > >exit_int_info not to be changed in the exit cycle (which would be > >worth > >a comment). > > It simply tries to be too clever. Reevaluating exceptions won't work > for example. > Can you elaborate? What do you mean by "too clever" and why reevaluating exceptions won't work? -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index f12a669..61efd13 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2349,7 +2349,10 @@ static int handle_exit(struct kvm_vcpu *vcpu) trace_kvm_exit(exit_code, svm->vmcb->save.rip); if (is_nested(svm)) { + struct vmcb_control_area *control = &svm->vmcb->control; int vmexit; + int type; + int vec; nsvm_printk("nested handle_exit: 0x%x | 0x%lx | 0x%lx | 0x%lx\n", exit_code, svm->vmcb->control.exit_info_1, @@ -2362,9 +2365,18 @@ static int handle_exit(struct kvm_vcpu *vcpu) if (vmexit == NESTED_EXIT_DONE) return 1; - } - svm_complete_interrupts(svm); + type = control->exit_int_info & SVM_EXITINTINFO_TYPE_MASK; + vec = control->exit_int_info & SVM_EXITINTINFO_VEC_MASK; + if ((type == SVM_EXITINTINFO_TYPE_INTR) || + ((type == SVM_EXITINTINFO_TYPE_EXEPT) && !kvm_exception_is_soft(vec))) { + control->event_inj = control->exit_int_info; + control->event_inj_err = control->exit_int_info_err; + } + } else { + /* Don't interpret exit_info for nested guests */ + svm_complete_interrupts(svm); + } if (npt_enabled) { int mmu_reload = 0; @@ -2602,8 +2614,6 @@ static void svm_complete_interrupts(struct vcpu_svm *svm) case SVM_EXITINTINFO_TYPE_EXEPT: /* In case of software exception do not reinject an exception vector, but re-execute and instruction instead */ - if (is_nested(svm)) - break; if (kvm_exception_is_soft(vector)) break; if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) {
SVM has some cleanup code, that tries to reinject interrupts and exceptions when the guest didn't manage to deal with them yet. It basically transfers them to KVM internal state. Unfortunately, the internal state is reserved for the L1 guest state, so we shouldn't try to go through that logic when running a nested guest. When doing something the host KVM can handle, let's just reinject the event into the L2 guest, because we didn't touch its state anyways. Signed-off-by: Alexander Graf <agraf@suse.de> --- arch/x86/kvm/svm.c | 18 ++++++++++++++---- 1 files changed, 14 insertions(+), 4 deletions(-)