Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

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

On 2011-04-19 16:15, Zachary Amsden wrote:
>>
>> On 2011-04-19 08:46, Roedel, Joerg wrote:
>>   
>>> On Mon, Apr 18, 2011 at 08:02:35PM -0400, Zachary Amsden wrote:
>>>     
>>>>> On Sat, Apr 16, 2011 at 06:09:17PM +0200, Jan Kiszka wrote:
>>>>>
>>>>>         
>>>>>> On 2011-03-25 09:44, Joerg Roedel wrote:
>>>>>>
>>>>>>            
>>>>>
>>>>>         
>>>>>>> +        tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
>>>>>>> +                 tsc - vcpu->arch.last_guest_tsc;
>>>>>>>
>>>>>>>              
>>>>>
>>>>>         
>>>>>> This patch appears to cause troubles to Linux guests on TSC
>>>>>> clocksource
>>>>>> and APIC highres timer. The first boot after qemu start is always
>>>>>> fine,
>>>>>> but after a reboot the guest timer appears to fire incorrectly or
>>>>>> even
>>>>>> not at all.
>>>>>>
>>>>>> Was this patch tested with a guest reboot scenario as well? Does it
>>>>>> account for the TSC being reset to 0 on reboot?
>>>>>>
>>>>>>            
>>>>> Hmm, probably the last_guest_tsc is not updated correctly in this
>>>>> scenario. I will have a look tomorrow.
>>>>>
>>>>>     Joerg
>>>>>
>>>>>
>>>>>          
>>>> To avoid this problem, when the TSC is reset, the overshoot protection
>>>> where last_guest_tsc is used is specifically disabled:
>>>>
>>>>           /* Reset of TSC must disable overshoot protection below */
>>>>           vcpu->arch.hv_clock.tsc_timestamp = 0;
>>>>           vcpu->arch.last_tsc_write = data;
>>>>           vcpu->arch.last_tsc_nsec = ns;
>>>>
>>>> You can probably use the same test - last_guest_tsc is only valid if
>>>> tsc_timestamp above != 0.
>>>>        
>>> Yes, this should also work. But the other way we get rid of
>>> last_host_tsc which is also a good thing :)
>>>      
>> That reminds me: I think we still have the bug that KVM overeagerly
>> declares the host's TSC unstable after resume from S3 because the TSC
>> appears to go backward when KVM checks. Or should this have been fixed
>> now?
>>
>>    
> 
> That should have been fixed... let me check if the patch has made it
> upstream.

I have "Marking TSC unstable due to KVM discovered backwards TSC" in my
recent log after I forgot to kill some guest before suspending the host.
And that was with kvm-kmod/kvm.git head. So I tend to believe that it's
not yet completely fixed...

Jan

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