Re: [patch 3/4] QEMU/KVM: non-virtualized ACPI PMTimer support

[Avi Kivity <avi@qumranet.com>]

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