Am 04.01.2013 22:29, schrieb Hervé Poussineau:
> Signed-off-by: Hervé Poussineau <hpous...@reactos.org>
> ---
> hw/isa-bus.c | 127
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> hw/isa.h | 2 +-
> 2 files changed, 125 insertions(+), 4 deletions(-)
>
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 86b0bbd..bcf7cd4 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -104,19 +104,140 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion
> *io, uint16_t start)
> isa_init_ioport(dev, start);
> }
>
> +typedef struct PortioState {
> + const char *name; /* debug purposes */
> + uint16_t start;
> + uint16_t offset;
> + const MemoryRegionPortio *pio_start;
> + void *opaque;
> +} PortioState;
> +
> +static const MemoryRegionPortio *portio_find(const MemoryRegionPortio *mrp,
> + uint64_t offset,
> + unsigned int width, bool write,
> + bool smaller)
> +{
> + for (; mrp->size; ++mrp) {
> + if (offset >= mrp->offset && offset < mrp->offset + mrp->len
> + && (width == mrp->size || (smaller && width < mrp->size))
> + && (write ? (bool)mrp->write : (bool)mrp->read)) {
> + return mrp;
> + }
> + }
> + return NULL;
> +}
> +
> +static uint64_t portio_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> + const PortioState *s = opaque;
> + const MemoryRegionPortio *mrp;
> +
> + addr += s->offset;
> + mrp = portio_find(s->pio_start, addr, size, false, false);
> + if (mrp) {
> + return mrp->read(s->opaque, s->start + addr);
> + } else if (size == 2) {
> + uint64_t data;
> + mrp = portio_find(s->pio_start, addr, 1, false, false);
> + assert(mrp);
> + data = mrp->read(s->opaque, s->start + addr) |
> + (mrp->read(s->opaque, s->start + addr + 1) << 8);
> + return data;
> + }
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid read from 0x%x size=%d",
> + s->name, s->start + (int)addr, size);
I note that patch 10/10 shows that memory.c does it similarly, assuming
Little Endian for size == 2 and ignoring assembled size == 4.
> + return -1U;
> +}
> +
> +static void portio_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned int size)
> +{
> + const PortioState *s = opaque;
> + const MemoryRegionPortio *mrp;
> +
> + addr += s->offset;
> + mrp = portio_find(s->pio_start, addr, size, true, false);
> + if (mrp) {
> + mrp->write(s->opaque, s->start + addr, data);
> + return;
> + } else if (size == 2) {
> + mrp = portio_find(s->pio_start, addr, 1, true, false);
> + assert(mrp);
> + mrp->write(s->opaque, s->start + addr, data & 0xff);
> + mrp->write(s->opaque, s->start + addr + 1, data >> 8);
> + return;
> + }
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid write to 0x%x size=%d",
> + s->name, s->start + (int)addr, size);
Ditto.
> +}
> +
> +static bool portio_accepts(void *opaque, hwaddr addr, unsigned int size,
> + bool is_write)
> +{
> + const PortioState *s = opaque;
> + const MemoryRegionPortio *mrp;
> +
> + addr += s->offset;
> + mrp = portio_find(s->pio_start, addr, size, is_write, true);
> + return (mrp != NULL);
> +}
> +
> +const MemoryRegionOps portio_ops = {
static const?
> + .read = portio_read,
> + .write = portio_write,
> + .valid.accepts = portio_accepts,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void isa_register_portio_list_1(ISADevice *dev, uint16_t start,
> + uint16_t offset, uint16_t end,
> + const MemoryRegionPortio *pio_start,
> + void *opaque, const char *name)
> +{
> + MemoryRegion *mr = g_new(MemoryRegion, 1);
> + PortioState *s = g_new(PortioState, 1);
> +
> + s->name = name;
> + s->start = start;
> + s->offset = offset;
> + s->pio_start = pio_start;
> + s->opaque = opaque;
> + memory_region_init_io(mr, &portio_ops, s, name, end - offset);
> + memory_region_add_subregion(isa_address_space_io(dev),
> + start + offset, mr);
> +}
> +
> void isa_register_portio_list(ISADevice *dev, uint16_t start,
> const MemoryRegionPortio *pio_start,
> void *opaque, const char *name)
> {
> - PortioList *piolist = g_new(PortioList, 1);
> + const MemoryRegionPortio *pio, *first;
> + uint16_t end;
>
> /* 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. */
> isa_init_ioport(dev, start);
>
> - portio_list_init(piolist, pio_start, opaque, name);
> - portio_list_add(piolist, isabus->address_space_io, start);
> + assert(pio_start->size);
> +
> + first = pio_start;
> + end = 0;
> + for (pio = pio_start; pio->size; pio++) {
> + assert(pio->offset >= first->offset);
> + if (pio->offset > first->offset + first->len) {
> + isa_register_portio_list_1(dev, start, first->offset, end,
> + pio_start, opaque, name);
> + first = pio;
> + end = 0;
> + }
> + if (pio->offset + pio->len > end) {
> + end = pio->offset + pio->len;
> + }
> + }
> +
> + isa_register_portio_list_1(dev, start, first->offset, end,
> + pio_start, opaque, name);
> }
>
> static int isa_qdev_init(DeviceState *qdev)
> diff --git a/hw/isa.h b/hw/isa.h
> index 62e89d3..c3b01ea 100644
> --- a/hw/isa.h
> +++ b/hw/isa.h
> @@ -73,7 +73,7 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io,
> uint16_t start);
> * @dev: the ISADevice against which these are registered; may be NULL.
> * @start: the base I/O port against which the portio->offset is applied.
> * @portio: the ports, sorted by offset.
> - * @opaque: passed into the old_portio callbacks.
> + * @opaque: passed into the portio callbacks.
> * @name: passed into memory_region_init_io.
> */
> void isa_register_portio_list(ISADevice *dev, uint16_t start,
Otherwise looks okay to me, but I'd appreciate another pair of eyes.
AFAIU this moves the current implementation from ioport.c (09/10) and
memory.c (10/10) into isa-bus.c, to remove MemoryRegionOps::old_portio
in 10/10. But ISA keeps using MemoryRegionPortio unlike memory.c itself.
I wonder if it were not a cleaner course of action to remove
isa_register_portio_list() completely by converting callers to use
isa_register_ioport() with a MemoryRegion owned by the caller?
Remaining MemoryRegionPortio lists after this series below.
Regards,
Andreas
dma.c:
/* IOport from page_base */
static const MemoryRegionPortio page_portio_list[] = {
{ 0x01, 3, 1, .write = write_page, .read = read_page, },
{ 0x07, 1, 1, .write = write_page, .read = read_page, },
PORTIO_END_OF_LIST(),
};
/* IOport from pageh_base */
static const MemoryRegionPortio pageh_portio_list[] = {
{ 0x01, 3, 1, .write = write_pageh, .read = read_pageh, },
{ 0x07, 3, 1, .write = write_pageh, .read = read_pageh, },
PORTIO_END_OF_LIST(),
};
fdc.c:
static const MemoryRegionPortio fdc_portio_list[] = {
{ 1, 5, 1, .read = fdctrl_read, .write = fdctrl_write },
{ 7, 1, 1, .read = fdctrl_read, .write = fdctrl_write },
PORTIO_END_OF_LIST(),
};
gus.c:
static const MemoryRegionPortio gus_portio_list1[] = {
{0x000, 1, 1, .write = gus_writeb },
{0x000, 1, 2, .write = gus_writew },
{0x006, 10, 1, .read = gus_readb, .write = gus_writeb },
{0x006, 10, 2, .read = gus_readw, .write = gus_writew },
{0x100, 8, 1, .read = gus_readb, .write = gus_writeb },
{0x100, 8, 2, .read = gus_readw, .write = gus_writew },
PORTIO_END_OF_LIST (),
};
static const MemoryRegionPortio gus_portio_list2[] = {
{0, 1, 1, .read = gus_readb },
{0, 1, 2, .read = gus_readw },
PORTIO_END_OF_LIST (),
};
ide/core.c:
static const MemoryRegionPortio ide_portio_list[] = {
{ 0, 8, 1, .read = ide_ioport_read, .write = ide_ioport_write },
{ 0, 2, 2, .read = ide_data_readw, .write = ide_data_writew },
{ 0, 4, 4, .read = ide_data_readl, .write = ide_data_writel },
PORTIO_END_OF_LIST(),
};
static const MemoryRegionPortio ide_portio2_list[] = {
{ 0, 1, 1, .read = ide_status_read, .write = ide_cmd_write },
PORTIO_END_OF_LIST(),
};
parallel.c:
static const MemoryRegionPortio isa_parallel_portio_hw_list[] = {
{ 0, 8, 1,
.read = parallel_ioport_read_hw,
.write = parallel_ioport_write_hw },
{ 4, 1, 2,
.read = parallel_ioport_eppdata_read_hw2,
.write = parallel_ioport_eppdata_write_hw2 },
{ 4, 1, 4,
.read = parallel_ioport_eppdata_read_hw4,
.write = parallel_ioport_eppdata_write_hw4 },
{ 0x400, 8, 1,
.read = parallel_ioport_ecp_read,
.write = parallel_ioport_ecp_write },
PORTIO_END_OF_LIST(),
};
static const MemoryRegionPortio isa_parallel_portio_sw_list[] = {
{ 0, 8, 1,
.read = parallel_ioport_read_sw,
.write = parallel_ioport_write_sw },
PORTIO_END_OF_LIST(),
};
qxl.c:
static const MemoryRegionPortio qxl_vga_portio_list[] = {
{ 0x04, 2, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3b4 */
{ 0x0a, 1, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3ba */
{ 0x10, 16, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3c0 */
{ 0x24, 2, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3d4 */
{ 0x2a, 1, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3da */
PORTIO_END_OF_LIST(),
};
sb16.c:
static const MemoryRegionPortio sb16_ioport_list[] = {
{ 4, 1, 1, .write = mixer_write_indexb },
{ 4, 1, 2, .write = mixer_write_indexw },
{ 5, 1, 1, .read = mixer_read, .write = mixer_write_datab },
{ 6, 1, 1, .read = dsp_read, .write = dsp_write },
{ 10, 1, 1, .read = dsp_read },
{ 12, 1, 1, .write = dsp_write },
{ 12, 4, 1, .read = dsp_read },
PORTIO_END_OF_LIST (),
};
vga.c:
static const MemoryRegionPortio vga_portio_list[] = {
{ 0x04, 2, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3b4 */
{ 0x0a, 1, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3ba */
{ 0x10, 16, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3c0 */
{ 0x24, 2, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3d4 */
{ 0x2a, 1, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3da */
PORTIO_END_OF_LIST(),
};
static const MemoryRegionPortio vbe_portio_list[] = {
{ 0, 1, 2, .read = vbe_ioport_read_index, .write =
vbe_ioport_write_index },
# ifdef TARGET_I386
{ 1, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
# endif
{ 2, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
PORTIO_END_OF_LIST(),
};
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
