Re: [PATCH 4/4] KVM: Rework of guest debug state writing

[Jan Kiszka <jan.kiszka@siemens.com>]

Marcelo Tosatti wrote:
> On Thu, Feb 04, 2010 at 04:41:44PM +0100, Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>> Marcelo Tosatti wrote:
>>>> On Thu, Feb 04, 2010 at 01:33:50AM +0100, Jan Kiszka wrote:
>>>>> Marcelo Tosatti wrote:
>>>>>> On Wed, Feb 03, 2010 at 10:29:45PM +0100, Jan Kiszka wrote:
>>>>>>> So far we synchronized any dirty VCPU state back into the kernel before
>>>>>>> updating the guest debug state. This was a tribute to a deficit in x86
>>>>>>> kernels before 2.6.33. But as this is an arch-dependent issue, it is
>>>>>>> better handle in the x86 part of KVM and remove the writeback point for
>>>>>>> generic code.
>>>>>> Jan,
>>>>>>
>>>>>> This patch breaks migration.
>>>>> Can you elaborate what you did? I can't reproduce, and I do not see any
>>>>> conceptual issue (given that guest debugging conflicts with migration
>>>>> anyway).
>>>> kvm-autotest fails (migration only, install is ok, both Linux and Win
>>>> guests). Not sure why, perhaps the unconditional KVM_SET_GUEST_DEBUG
>>>> corrupts state somehow? 
>>>>
>>>> Tested with io thread enabled.
>>> That's this default-off thing, so... OK, confirmed, investigating.
>>>
>> Heisenbug: It first also popped up (in form of a frozen migration
>> target) after removing this patch, but now it's totally unreproducible,
>> whatever patch I apply or revert from my series. Base is current master.
>>
>> I tend to think there is a hidden issue of iothread vs. migration,
>> unrelated to this patch.
> 
> Probably many :)
> 
> Do you have c5f32c99c6855d466737daf1cd262e7e92062f87 (from qemu-kvm.git
> uq/master) in?

Yes. And that might have been the reason why some early tests failed
when it was no yet applied here.

> 
> With kvm-autotest the failure is not sporadic (and the above commit
> applied): with KVM_SET_GUEST_DEBUG in arch_put_regs all migration 
> tests fail, without, all of them succeed. 
> 
> So env->kvm_guest_debug has been zeroed by cpu_x86_init, which means
> the writeback via KVM_SET_GUEST_DEBUG does almost nothing. It does
> get_rflags and set_rflags in the kernel.

Hmm, it also copies debug regs around... BTW, where do we save/restore
dr0..7 between kernel and user space?

But that should not be a problem, both shadow as well as effective regs
should be properly initialized, specifically for a newly created VCPU.

> 
> Test box is off, but the synchronous writeback via qemu_system_reset
> in main, after machine and vcpu thread initialization, might be
> problematic. But it would be nice to understand this.
> 
> Unrelated to this problem, won't put_vcpu_events, which is executed 
> after KVM_SET_GUEST_DEBUG, overwrite any queued debug exceptions?

Good point, SET_GUEST_DEBUG should be last in the writeback for that reason.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux