qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH 05/15] Coalesce userspace/kernel irqchip interru


From: Jan Kiszka
Subject: [Qemu-devel] Re: [PATCH 05/15] Coalesce userspace/kernel irqchip interrupt injection logic.
Date: Sun, 19 Apr 2009 17:06:29 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666

Jan Kiszka wrote:
> Gleb Natapov wrote:
>> On Sat, Apr 18, 2009 at 07:28:20PM +0300, Gleb Natapov wrote:
>>>> So this patch may either expose a bug in the svm emulation of qemu or
>>>> comes with a subtle regression that only triggers due to qemu's timing.
>>>> This needs to be understood. Gleb, any progress on reproducing it on
>>>> your side?
>>>>
>>> I reproduced it and I am debugging it. In my case the boot hangs on sti;hlt
>>> sequence. Instrumentation thus far shows that at this point interrupts no 
>>> longer
>>> injected because ppr value is too big. Need to see why, but tpr handling
>>> is not complete in qemu svm. May be this is the reason. Will know more
>>> tomorrow.
>>>
>> I've looked into this and my conclusion is that if you are not going to
>> develop SVM in qemu don't use it just yet.
> 
> We had a resource conflict regarding SVM capable AMD boxes and a tight
> schedule, so we decided to pick qemu as initial development platform.
> Turns out that this has was a bit too optimistic. :)
> 
>> QEMU doesn't handle exceptions
>> during event injection properly. Actually it does not handle it at all,
>> so if PF happens during interrupt injection interrupt is lost and, what
>> worse, is never acked. If interrupt was high prio it blocks all other
>> interrupts.
>>
>> The patch below adds exception handling during event injection. Valid
>> flag removed from EVENTINJ only after successful injection and EVENTINJ
>> is copied to EXITINTINFO on exit. Can you give it a try?
> 
> Ah, great, thanks. Will test.

I can confirm: patch below makes my kvm-in-qemu test case happy, too.
Maybe you want to post this with changelog and signed-off to qemu-devel.

Jan

>> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
>> index be09263..9264afd 100644
>> --- a/target-i386/op_helper.c
>> +++ b/target-i386/op_helper.c
>> @@ -1249,6 +1249,10 @@ void do_interrupt(int intno, int is_int, int 
>> error_code,
>>      } else {
>>          do_interrupt_real(intno, is_int, error_code, next_eip);
>>      }
>> +    if (env->hflags & HF_SVMI_MASK) {
>> +        uint32_t event_inj = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
>> control.event_inj));
>> +        stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 
>> event_inj & ~SVM_EVTINJ_VALID);
>> +    }
>>  }
>>  
>>  /* This should come from sysemu.h - if we could include it here... */
>> @@ -4994,7 +4998,6 @@ void helper_vmrun(int aflag, int next_eip_addend)
>>          uint8_t vector = event_inj & SVM_EVTINJ_VEC_MASK;
>>          uint16_t valid_err = event_inj & SVM_EVTINJ_VALID_ERR;
>>          uint32_t event_inj_err = ldl_phys(env->vm_vmcb + offsetof(struct 
>> vmcb, control.event_inj_err));
>> -        stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 
>> event_inj & ~SVM_EVTINJ_VALID);
>>  
>>          qemu_log_mask(CPU_LOG_TB_IN_ASM, "Injecting(%#hx): ", valid_err);
>>          /* FIXME: need to implement valid_err */
>> @@ -5331,6 +5334,8 @@ void helper_vmexit(uint32_t exit_code, uint64_t 
>> exit_info_1)
>>      cpu_x86_set_cpl(env, 0);
>>      stq_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_code), 
>> exit_code);
>>      stq_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_info_1), 
>> exit_info_1);
>> +    stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info), 
>> ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj)));
>> +    stl_phys(env->vm_vmcb + offsetof(struct vmcb, 
>> control.exit_int_info_err), ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
>> control.event_inj_err)));
>>  
>>      env->hflags2 &= ~HF2_GIF_MASK;
>>      /* FIXME: Resets the current ASID register to zero (host ASID). */
>> --
>>                      Gleb.


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]