From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48952) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8rGO-0001tr-Rl for qemu-devel@nongnu.org; Tue, 04 Sep 2012 07:25:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8rGM-0002jj-Ux for qemu-devel@nongnu.org; Tue, 04 Sep 2012 07:25:48 -0400 Received: from smtp.citrix.com ([66.165.176.89]:60260) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8rGM-0002jW-PH for qemu-devel@nongnu.org; Tue, 04 Sep 2012 07:25:46 -0400 Message-ID: <5045E5AB.4020007@citrix.com> Date: Tue, 4 Sep 2012 12:27:39 +0100 From: Julien Grall MIME-Version: 1.0 References: <7f5a672e348b8f917dcc92c67e2ae5f7ce75ea60.1346665074.git.julien.grall@citrix.com> <5045ACB5.40106@web.de> In-Reply-To: <5045ACB5.40106@web.de> Content-Type: text/plain; charset="ISO-8859-15"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V7 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: "qemu-devel@nongnu.org" , Stefano Stabellini , "avi@redhat.com" , "afaerber@suse.de" , "kraxel@redhat.com" On 09/04/2012 08:24 AM, Jan Kiszka wrote: > On 2012-09-03 12:03, 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 | 151 +++++++++++++++++++++++++++++++++++++++++++------------ >> 1 files changed, 119 insertions(+), 32 deletions(-) >> >> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c >> index 1ad45ce..527dfc1 100644 >> --- a/hw/acpi_piix4.c >> +++ b/hw/acpi_piix4.c >> @@ -41,8 +41,7 @@ >> >> #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 >> >> @@ -55,7 +54,8 @@ struct pci_status { >> >> typedef struct PIIX4PMState { >> PCIDevice dev; >> - IORange ioport; >> + MemoryRegion pm_io; >> + uint32_t pm_io_base; >> 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, >> + }, >> }; >> >> static void apm_ctrl_changed(uint32_t val, void *arg) >> @@ -185,7 +196,8 @@ static void apm_ctrl_changed(uint32_t val, void *arg) >> } >> } >> >> -static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val) >> +static void acpi_dbg_writel(void *opaque, target_phys_addr_t addr, >> + uint64_t val, unsigned size) >> { >> PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val); >> } >> @@ -200,8 +212,18 @@ static void pm_io_space_update(PIIX4PMState *s) >> >> /* XXX: need to improve memory and ioport allocation */ >> PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base); >> - iorange_init(&s->ioport,&pm_iorange_ops, pm_io_base, 64); >> - ioport_register(&s->ioport); >> + >> + if (!s->pm_io_base) { >> + memory_region_add_subregion(pci_address_space_io(&s->dev), >> + pm_io_base,&s->pm_io); >> > This doesn't work reliably. Add the region in a disabled state (ie. the > default) during init, only toggle enable and update the base here. > What is the default base address ? The datasheet doesn't give default value. But the base address seems to always be 0xb000. Is there any drawbacks to used it ? -- Julien Grall