From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59969) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8w2f-00065x-7T for qemu-devel@nongnu.org; Tue, 04 Sep 2012 12:31:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8w2a-00057y-Mg for qemu-devel@nongnu.org; Tue, 04 Sep 2012 12:31:57 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:27975) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8w2a-00057O-Ht for qemu-devel@nongnu.org; Tue, 04 Sep 2012 12:31:52 -0400 Message-ID: <50462D68.1020508@citrix.com> Date: Tue, 4 Sep 2012 17:33:44 +0100 From: Julien Grall MIME-Version: 1.0 References: <13a02c99e22d6180bc4a5d58d7b38b249773fa73.1346741532.git.julien.grall@citrix.com> <50461AF8.3090304@siemens.com> In-Reply-To: <50461AF8.3090304@siemens.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V8 4/8] hw/acpi_piix4.c: replace register_ioport* List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Stefano Stabellini , "kraxel@redhat.com" , "avi@redhat.com" , "afaerber@suse.de" , "qemu-devel@nongnu.org" On 09/04/2012 04:15 PM, Jan Kiszka wrote: > On 2012-09-04 09:28, Julien Grall wrote: > >> This patch replaces all register_ioport* with the new memory API. It permits >> to use the new Memory stuff like listener. >> >> Signed-off-by: Julien Grall >> --- >> hw/acpi_piix4.c | 145 +++++++++++++++++++++++++++++++++++++++++++------------ >> 1 files changed, 113 insertions(+), 32 deletions(-) >> >> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c >> index 0b4ad24..cd70610 100644 >> --- a/hw/acpi_piix4.c >> +++ b/hw/acpi_piix4.c >> @@ -41,10 +41,10 @@ >> >> #define GPE_BASE 0xafe0 >> #define GPE_LEN 4 >> -#define PCI_UP_BASE 0xae00 >> -#define PCI_DOWN_BASE 0xae04 >> +#define PCI_BASE 0xae00 >> #define PCI_EJ_BASE 0xae08 >> #define PCI_RMV_BASE 0xae0c >> +#define PM_BASE 0x00 >> >> #define PIIX4_PCI_HOTPLUG_STATUS 2 >> >> @@ -55,7 +55,7 @@ struct pci_status { >> >> typedef struct PIIX4PMState { >> PCIDevice dev; >> - IORange ioport; >> + MemoryRegion pm_io; >> ACPIREGS ar; >> >> APMState apm; >> @@ -64,6 +64,11 @@ typedef struct PIIX4PMState { >> uint32_t smb_io_base; >> >> MemoryRegion smb_io; >> + MemoryRegion acpi_io; >> + MemoryRegion acpi_hot_io; >> + PortioList pci_hot_port_list; >> + MemoryRegion pciej_hot_io; >> + MemoryRegion pcirmv_hot_io; >> >> qemu_irq irq; >> qemu_irq smi_irq; >> @@ -110,10 +115,10 @@ static void pm_tmr_timer(ACPIREGS *ar) >> pm_update_sci(s); >> } >> >> -static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width, >> - uint64_t val) >> +static void pm_ioport_write(void *opaque, target_phys_addr_t addr, >> + uint64_t val, unsigned width) >> { >> - PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport); >> + PIIX4PMState *s = opaque; >> >> if (width != 2) { >> PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n", >> @@ -139,11 +144,11 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width, >> (unsigned int)val); >> } >> >> -static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width, >> - uint64_t *data) >> +static uint64_t pm_ioport_read(void *opaque, target_phys_addr_t addr, >> + unsigned width) >> { >> - PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport); >> - uint32_t val; >> + PIIX4PMState *s = opaque; >> + uint64_t val; >> >> switch(addr) { >> case 0x00: >> @@ -163,12 +168,18 @@ static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width, >> break; >> } >> PIIX4_DPRINTF("PM readw port=0x%04x val=0x%04x\n", (unsigned int)addr, val); >> - *data = val; >> + >> + return val; >> } >> >> -static const IORangeOps pm_iorange_ops = { >> +static const MemoryRegionOps pm_io_ops = { >> .read = pm_ioport_read, >> .write = pm_ioport_write, >> + .endianness = DEVICE_LITTLE_ENDIAN, >> + .impl = { >> + .min_access_size = 2, >> + .max_access_size = 2, >> > Where do these constraints come from? > I don't pay enough attention about the size. > OK, this one breaks my Win7 guest. Following my suspect above and the > endless loop over > > kvm_pio: pio_read at 0xb008 size 4 count 1 > > I played with max_access_size = 4 for pm_io_ops, and Windows boots > again. Looking at the details, the PIO range was apparently not properly > specified so far. It implements 2-bytes accesses for the offsets 0x00, > 0x02, 0x04 and 4-bytes access for 0x08. But the specification was that > accesses of all sizes are provided. > > Given this experience, we will have to review at least the hacky ACPI > stuff very carefully. > Could we change max_access_size to 4 and check on each PIO if the size is correct ? ie 2-bytes access for 0x00, 0x02, 0x04 and 4-bytes access for 0x08. -- Julien Grall