From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QtgR6-0000uc-Eb for qemu-devel@nongnu.org; Wed, 17 Aug 2011 09:45:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QtgR5-0007NP-5e for qemu-devel@nongnu.org; Wed, 17 Aug 2011 09:45:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53441) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QtgR4-0007NK-S0 for qemu-devel@nongnu.org; Wed, 17 Aug 2011 09:45:35 -0400 Message-ID: <4E4BC5FA.1020409@redhat.com> Date: Wed, 17 Aug 2011 06:45:30 -0700 From: Avi Kivity MIME-Version: 1.0 References: <1313513145-5348-1-git-send-email-rth@twiddle.net> <1313513145-5348-3-git-send-email-rth@twiddle.net> In-Reply-To: <1313513145-5348-3-git-send-email-rth@twiddle.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 02/14] isa: Add isa_register_old_portio_list(). List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org On 08/16/2011 09:45 AM, Richard Henderson wrote: > > +void isa_register_old_portio_list(ISADevice *dev, uint16_t start, > + const MemoryRegionPortio *pio_start, > + void *opaque, const char *name) _old_ implies it's deprecated, please drop. It's only old if it's in a user specified MemoryRegionOps. > +{ > + MemoryRegion *io_space = isabus->address_space_io; > + const MemoryRegionPortio *pio_iter; > + > + /* START is how we should treat DEV, regardless of the actual > + contents of the portio array. This is how the old code > + actually handled e.g. the FDC device. */ > + if (dev) { > + isa_init_ioport(dev, start); > + } > + > + for (; pio_start->size != 0; pio_start = pio_iter + 1) { > + unsigned int off_low = UINT_MAX, off_high = 0; > + MemoryRegionOps *ops; > + MemoryRegion *region; > + > + for (pio_iter = pio_start; pio_iter->size; ++pio_iter) { > + if (pio_iter->offset< off_low) { > + off_low = pio_iter->offset; > + } > + if (pio_iter->offset + pio_iter->len> off_high) { > + off_high = pio_iter->offset + pio_iter->len; > + } This is supposed to pick up MRPs that are for the same port address? If so that should be in the loop termination condition. > + } > + > + ops = g_new(MemoryRegionOps, 1); g_new0(), we rely on initialized memory here > + ops->old_portio = pio_start; > + > + region = g_new(MemoryRegion, 1); (but not here) > + memory_region_init_io(region, ops, opaque, name, off_high - off_low); > + memory_region_set_offset(region, start + off_low); I think the memory core ignores set_offset() for portio. > + memory_region_add_subregion(io_space, start + off_low, region); > + } > +} > +void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start); > + > +/** > + * isa_register_old_portio_list: Initialize a set of ISA io ports > + * > + * Several ISA devices have many dis-joint I/O ports. Worse, these I/O > + * ports can be interleaved with I/O ports from other devices. This > + * function makes it easy to create multiple MemoryRegions for a single > + * device and use the legacy portio routines. > + * > + * @dev: the ISADevice against which these are registered; may be NULL. > + * @start: the base I/O port against which the portio->offset is applied. > + * @old_portio: A concatenation of several #MemoryRegionOps old_portio > + * parameters. The entire list should be terminated by a double > + * PORTIO_END_OF_LIST(). double seems harsh. > + * @opaque: passed into the old_portio callbacks. > + * @name: passed into memory_region_init_io. > + */ > +void isa_register_old_portio_list(ISADevice *dev, uint16_t start, > + const MemoryRegionPortio *old_portio, > + void *opaque, const char *name); > + > extern target_phys_addr_t isa_mem_base; > > void isa_mmio_setup(MemoryRegion *mr, target_phys_addr_t size); -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.