On Wed, Aug 17, 2011 at 1:45 PM, Avi Kivity <a...@redhat.com> wrote: > 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
Please avoid g_new/g_malloc until they are taught to use qemu_malloc or was it the other way around. >> + 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. > > >