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
