Re: [PATCH 2/5] Don't call svm_complete_interrupts for nested guests

From: Alexander Graf <agraf@suse.de>
To: Joerg Roedel <joro@8bytes.org>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH 2/5] Don't call svm_complete_interrupts for nested guests
Date: Wed, 23 Sep 2009 01:05:57 -0700	[thread overview]
Message-ID: <D305191D-FE31-4CD6-85F5-0CB13D856DA8@suse.de> (raw)
In-Reply-To: <20090923012640.GC4654@8bytes.org>

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