From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: Re: [kvm-devel] [PATCH 2/5] SCI fixes (v2) Date: Wed, 28 May 2008 06:37:50 -0600 Message-ID: <1211978270.14859.40.camel@bling> References: <1202137865-20232-1-git-send-email-aliguori@us.ibm.com> <1202137865-20232-3-git-send-email-aliguori@us.ibm.com> <1211857519.14708.30.camel@bling> <483C52D2.2080603@us.ibm.com> <483CFB6D.3070302@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , kvm-devel To: Avi Kivity Return-path: Received: from g5t0007.atlanta.hp.com ([15.192.0.44]:24000 "EHLO g5t0007.atlanta.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbYE1Mhz (ORCPT ); Wed, 28 May 2008 08:37:55 -0400 In-Reply-To: <483CFB6D.3070302@qumranet.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, 2008-05-28 at 09:27 +0300, Avi Kivity wrote: > This is > > commit ce35c9534137b71327466fa9abc243cbe2d7e8dc > > Author: Avi Kivity > > Date: Wed Jan 2 12:52:28 2008 +0200 > > > > kvm: qemu: fix power management timer overflow handling > > > > The PMSTS overflow bit needs to be set each time bit 23 of the pm > > timer > > is toggled. This means we need to adjust the overflow time every time > > we have an overflow. > > > > Taken from qemu patch by TeLeMan in > > > > http://www.mail-archive.com/qemu-devel@nongnu.org/msg14680.html > > And, like the explanation says, we have to advance the overflow time in > order to get an interrupt. Is there something horribly broken? Yeah, it kinda seems like there is. With this change, the timer expires and we go through this path: pm_tmr_timer() -> pm_update_sci() -> get_pmsts() -> qemu_set_irq() [but not for a TMFOF_EN] -> qemu_mod_timer() bump tmr_overlfow_time We bumped tmr_overflow_time in pm_update_sci after setting the timer to expire on the old value. Unless something goes horribly wrong with timers, we'll always get the timer event before overflow time and get_pmsts never adds in the TMROF_EN bit to the status flag. We therefore never toggle the SCI interrupt because of a timer overflow, and we never report a timer overflow status to the guest. The author of this patch is correct that the timer in the original code only goes off a couple times before we del_timer(). However, I think the way it's supposed to work is that we set the timer overflow status, toggle the SCI, then wait for the OSPM to come in through pm_ioport_writew() to clear the timer overflow status, at which point we call pm_update_sci() mod_timer and start it all over again. At least that's the way I see it working after removing this change. It doesn't make much sense to bump tmr_overflow_time so that we never hit it, unless I'm completely misunderstanding the code. Thanks, Alex -- Alex Williamson HP Open Source & Linux Org.