From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4vBO-0005ol-8O for qemu-devel@nongnu.org; Fri, 24 Aug 2012 10:48:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4vBM-0004D9-QP for qemu-devel@nongnu.org; Fri, 24 Aug 2012 10:48:22 -0400 Received: from smtp.citrix.com ([66.165.176.89]:38091) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4vBM-0004D1-LC for qemu-devel@nongnu.org; Fri, 24 Aug 2012 10:48:20 -0400 Message-ID: <50379467.5020907@citrix.com> Date: Fri, 24 Aug 2012 15:49:11 +0100 From: Julien Grall MIME-Version: 1.0 References: <26cea9d1d7137ffbf0d133e0e553b6b79c9d1e6d.1345549695.git.julien.grall@citrix.com> <50378530.9040209@siemens.com> In-Reply-To: <50378530.9040209@siemens.com> Content-Type: text/plain; charset="ISO-8859-15"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V5 3/8] hw/cirrus_vga.c: replace register_ioport* List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Stefano Stabellini , "qemu-devel@nongnu.org" , "avi@redhat.com" On 08/24/2012 02:44 PM, Jan Kiszka wrote: > On 2012-08-22 14:27, Julien Grall wrote: > >> This patch replaces all register_ioport* with portio_*. It permits to >> use the new Memory stuff like listener. >> >> Signed-off-by: Julien Grall >> --- >> hw/cirrus_vga.c | 42 ++++++++++++++++++++++++------------------ >> 1 files changed, 24 insertions(+), 18 deletions(-) >> >> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c >> index e8dcc6b..adfc855 100644 >> --- a/hw/cirrus_vga.c >> +++ b/hw/cirrus_vga.c >> @@ -200,6 +200,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s, >> typedef struct CirrusVGAState { >> VGACommonState vga; >> >> + MemoryRegion cirrus_vga_io; >> MemoryRegion cirrus_linear_io; >> MemoryRegion cirrus_linear_bitblt_io; >> MemoryRegion cirrus_mmio_io; >> @@ -2528,12 +2529,15 @@ static uint32_t cirrus_vga_ioport_read(void *opaque, uint32_t addr) >> return val; >> } >> >> -static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val) >> +static void cirrus_vga_ioport_write(void *opaque, target_phys_addr_t addr, >> + uint64_t val, unsigned size) >> { >> CirrusVGAState *c = opaque; >> VGACommonState *s =&c->vga; >> int index; >> >> + addr += 0x3b0; >> + >> /* check port range access depending on color/monochrome mode */ >> if (vga_ioport_invalid(s, addr)) { >> return; >> @@ -2657,7 +2661,7 @@ static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr, >> if (addr>= 0x100) { >> cirrus_mmio_blt_write(s, addr - 0x100, val); >> } else { >> - cirrus_vga_ioport_write(s, addr + 0x3c0, val); >> + cirrus_vga_ioport_write(s, addr + 0x10, val, size); >> } >> } >> >> @@ -2783,8 +2787,18 @@ static const MemoryRegionOps cirrus_linear_io_ops = { >> }, >> }; >> >> +static const MemoryRegionOps cirrus_vga_io_ops = { >> + .write = cirrus_vga_ioport_write, >> + .endianness = DEVICE_LITTLE_ENDIAN, >> + .impl = { >> + .min_access_size = 1, >> + .max_access_size = 1, >> + }, >> +}; >> + >> static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci, >> - MemoryRegion *system_memory) >> + MemoryRegion *system_memory, >> + MemoryRegion *system_io) >> { >> int i; >> static int inited; >> @@ -2816,19 +2830,10 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci, >> s->bustype = CIRRUS_BUSTYPE_ISA; >> } >> >> - register_ioport_write(0x3c0, 16, 1, cirrus_vga_ioport_write, s); >> - >> - register_ioport_write(0x3b4, 2, 1, cirrus_vga_ioport_write, s); >> - register_ioport_write(0x3d4, 2, 1, cirrus_vga_ioport_write, s); >> - register_ioport_write(0x3ba, 1, 1, cirrus_vga_ioport_write, s); >> - register_ioport_write(0x3da, 1, 1, cirrus_vga_ioport_write, s); >> - >> - register_ioport_read(0x3c0, 16, 1, cirrus_vga_ioport_read, s); >> - >> - register_ioport_read(0x3b4, 2, 1, cirrus_vga_ioport_read, s); >> - register_ioport_read(0x3d4, 2, 1, cirrus_vga_ioport_read, s); >> - register_ioport_read(0x3ba, 1, 1, cirrus_vga_ioport_read, s); >> - register_ioport_read(0x3da, 1, 1, cirrus_vga_ioport_read, s); >> + /* Register ioport 0x3b0 - 0x3df */ >> + memory_region_init_io(&s->cirrus_vga_io,&cirrus_vga_io_ops, s, >> + "cirrus-io", 0x30); >> + memory_region_add_subregion(system_io, 0x3b0,&s->cirrus_vga_io); >> > Can't imagine that this reflects the original ranges and constraints > correctly. Or were they all wrong? > > I made a version (V4) with the same mapping, but Anthony has proposed to register 0x3b0 - 0x3df (https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03329.html) I don't see a problem, and it works on my computer. By the way, I will resend this patch because I forget read access in MemoryRegionOps. Sorry. > Jan > >