From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [patch 3/4] QEMU/KVM: non-virtualized ACPI PMTimer support Date: Sun, 25 May 2008 13:18:46 +0300 Message-ID: <48393D06.2060406@qumranet.com> References: <20080524234342.983197667@localhost.localdomain> <20080525000036.645310064@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Chris Wright , kvm-devel To: Marcelo Tosatti Return-path: Received: from bzq-179-150-194.static.bezeqint.net ([212.179.150.194]:40623 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752210AbYEYKSt (ORCPT ); Sun, 25 May 2008 06:18:49 -0400 In-Reply-To: <20080525000036.645310064@localhost.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > QEMU support for direct pmtimer reads. Hopefully its safe, since its a > read-only register ? > > With self-disable C2 + this I'm seeing less CPU usage when idle with > CONFIG_CPU_IDLE enabled. Quite noticeable on SMP guests. Windows XP is > comparable to standard (never seen it consume less than 10% either way, > usually 20-30%). > This is because the TPR optimization hack is disabled on smp. You need a machine with FlexPriority to get usable smp Windows. > On migration the destination host can either lack ACPI or have the timer > in a different IO port, so emulation is necessary. > > Or luckily the pmtimer is in the same address. Since the 24-bit counter > overflow period is only ~= 4.6 seconds, its probably worthwhile to wait > for synchronization before restarting the guest. Not implemented though. > > Also simplify and fix the overflow emulation, which was happening every > 2.3 seconds instead of the expected 4.6s. > > Index: kvm-userspace.tip/bios/rombios32.c > =================================================================== > --- kvm-userspace.tip.orig/bios/rombios32.c > +++ kvm-userspace.tip/bios/rombios32.c > @@ -391,7 +391,7 @@ uint8_t bios_uuid[16]; > unsigned long ebda_cur_addr; > #endif > int acpi_enabled; > -uint32_t pm_io_base, smb_io_base; > +uint32_t pm_io_base, pmtmr_base, smb_io_base; > int pm_sci_int; > unsigned long bios_table_cur_addr; > unsigned long bios_table_end_addr; > @@ -819,6 +819,12 @@ static void pci_bios_init_device(PCIDevi > pci_config_writeb(d, PCI_INTERRUPT_LINE, 9); > > pm_io_base = PM_IO_BASE; > + pmtmr_base = cmos_readb(0x60); > + pmtmr_base |= cmos_readb(0x61) << 8; > + pmtmr_base |= cmos_readb(0x62) << 16; > + pmtmr_base |= cmos_readb(0x63) << 24; > + if (!pmtmr_base) > + pmtmr_base = pm_io_base + 0x08; > You're splitting the ACPI ioport range into two. I think the correct fix here is to have qemu supply a PMBA hint to the BIOS. If the hint is present, the bios should locate pm_io_base there, and should also avoid placing other pio resources there. > +static void schedule_pmtmr_sci(PIIX4PMState *s) > +{ > + int64_t expire_time; > + uint32_t pmtmr, left; > + > + if (s->direct_access) > + qemu_kvm_get_pmtimer(&pmtmr); > + else > + pmtmr = get_pmtmr(s); > get_pmtmr() should have this logic. > + > + left = (1 << 24) - pmtmr; > The docs say that SCI is generated when bit 23 toggles, not on overflow. See TMROF_STS PIIX4 documentation. In any case, this should be in a separate patch. -- error compiling committee.c: too many arguments to function