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

[Marcelo Tosatti <mtosatti@redhat.com>]

On Sun, May 25, 2008 at 01:18:46PM +0300, Avi Kivity wrote:
> > 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 
> placing other pio resources there.

What is PMBA?

>From my understand ACPI supports an address for each register block, and
the PMTimer resides in a separate block. So what is the problem with
having different ACPI blocks in different ports?

Note that the GPE0 registers are in a different port range than
PM1EVT/PM1CNT/PMTimer already.

> >+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.

Yeah, misread the docs, as usual.

> 
> In any case, this should be in a separate patch.
> 
> -- 
> error compiling committee.c: too many arguments to function