From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc Date: Tue, 19 Apr 2011 16:24:03 +0200 Message-ID: <4DAD9B03.4060709@siemens.com> References: <1301042691-22929-1-git-send-email-joerg.roedel@amd.com> <1301042691-22929-4-git-send-email-joerg.roedel@amd.com> <4DA9BF2D.7010804@web.de> <20110417120656.GT18463@8bytes.org> <4DACD11B.4080200@redhat.com> <20110419064601.GM2192@amd.com> <4DAD308A.3090503@web.de> <4DAD9912.6050101@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: "Roedel, Joerg" , Joerg Roedel , Avi Kivity , Marcelo Tosatti , "kvm@vger.kernel.org" To: Zachary Amsden Return-path: Received: from goliath.siemens.de ([192.35.17.28]:15564 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751567Ab1DSOYK (ORCPT ); Tue, 19 Apr 2011 10:24:10 -0400 In-Reply-To: <4DAD9912.6050101@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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