Refactor portio dispatching

PATCH In review Jan Kiszka 2 participants 15 messages Jun 21, 2013

Share & tools

Jan Kiszka

author

2013-06-21 16:42 UTC #0 lore raw

v2 cover Refactor portio dispatching

First, this series converts the remaining users of register_ioport* to portio lists. Then it replaces the current portio dispatcher with the existing one for MMIO and removes several lines of code. This also allows to build BQL-free portio on top once we enhance the memory layer accordingly.

Changes in v2:
- rebased over master (which already contains some patches from v1)
- refactored PIO dispatching according to Paolo's suggestions

Jan

CC malc <[email protected]>

Jan Kiszka (13):
adlib: replace register_ioport*
applesmc: replace register_ioport*
wdt_ib700: replace register_ioport*
i82374: replace register_ioport*
prep: replace register_ioport*
vt82c686: replace register_ioport*
Privatize register_ioport_read/write
isa: implement isa_is_ioport_assigned via memory_region_find
vmware-vga: Accept unaligned I/O accesses
xen: Mark fixed platform I/O as unaligned
ioport: Switch dispatching to memory core layer
ioport: Remove unused old dispatching services
ioport: Move IOPortRead/WriteFunc typedefs to memory.h

 exec.c                         |   27 ---
 hw/acpi/piix4.c                |    9 +-
 hw/audio/adlib.c               |   20 ++-
 hw/display/vmware_vga.c        |    4 +
 hw/dma/i82374.c                |   18 ++-
 hw/isa/lpc_ich9.c              |    9 +-
 hw/isa/vt82c686.c              |   40 +++--
 hw/misc/applesmc.c             |   50 ++++--
 hw/ppc/prep.c                  |   23 ++-
 hw/watchdog/wdt_ib700.c        |   12 +-
 hw/xen/xen_platform.c          |    4 +
 include/exec/ioport.h          |   19 +--
 include/exec/iorange.h         |   31 ----
 include/exec/memory-internal.h |    2 -
 include/exec/memory.h          |   18 +--
 ioport.c                       |  380 +++++++++++-----------------------------
 memory.c                       |   88 ---------
 17 files changed, 236 insertions(+), 518 deletions(-)
 delete mode 100644 include/exec/iorange.h

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #1 lore raw

PATCH v2 1/13 adlib: replace register_ioport*

Convert over to memory regions to obsolete register_ioport*.

CC malc <[email protected]>

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/audio/adlib.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

hw/audio/adlib.c +12 -8

index 6a7d377..8b9b81e 100644
@@ -283,9 +283,17 @@ static void Adlib_fini (AdlibState *s)
     AUD_remove_card (&s->card);
 }
 
+static MemoryRegionPortio adlib_portio_list[] = {
+    { 0x388, 4, 1, .read = adlib_read, .write = adlib_write, },
+    { 0, 4, 1, .read = adlib_read, .write = adlib_write, },
+    { 0, 2, 1, .read = adlib_read, .write = adlib_write, },
+    PORTIO_END_OF_LIST(),
+};
+
 static void adlib_realizefn (DeviceState *dev, Error **errp)
 {
     AdlibState *s = ADLIB(dev);
+    PortioList *port_list = g_new(PortioList, 1);
     struct audsettings as;
 
     if (glob_adlib) {

@@ -339,14 +347,10 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
     s->samples = AUD_get_buffer_size_out (s->voice) >> SHIFT;
     s->mixbuf = g_malloc0 (s->samples << SHIFT);
 
-    register_ioport_read (0x388, 4, 1, adlib_read, s);
-    register_ioport_write (0x388, 4, 1, adlib_write, s);
-
-    register_ioport_read (s->port, 4, 1, adlib_read, s);
-    register_ioport_write (s->port, 4, 1, adlib_write, s);
-
-    register_ioport_read (s->port + 8, 2, 1, adlib_read, s);
-    register_ioport_write (s->port + 8, 2, 1, adlib_write, s);
+    adlib_portio_list[1].offset = s->port;
+    adlib_portio_list[2].offset = s->port + 8;
+    portio_list_init (port_list, adlib_portio_list, s, "adlib");
+    portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0);
 }
 
 static Property adlib_properties[] = {

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:41 UTC #2 lore raw

PATCH v2 2/13 applesmc: replace register_ioport*

In reply to Jan Kiszka

Convert over to memory regions to obsolete register_ioport*.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/misc/applesmc.c |   50 ++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 38 insertions(+), 12 deletions(-)

hw/misc/applesmc.c +38 ~~-12~~

index 46f4fbd..83468dc 100644
@@ -73,6 +73,8 @@ typedef struct AppleSMCState AppleSMCState;
 struct AppleSMCState {
     ISADevice parent_obj;
 
+    MemoryRegion io_data;
+    MemoryRegion io_cmd;
     uint32_t iobase;
     uint8_t cmd;
     uint8_t status;

@@ -86,7 +88,8 @@ struct AppleSMCState {
     QLIST_HEAD(, AppleSMCData) data_def;
 };
 
-static void applesmc_io_cmd_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
+                                  unsigned size)
 {
     AppleSMCState *s = opaque;

@@ -115,7 +118,8 @@ static void applesmc_fill_data(AppleSMCState *s)
     }
 }
 
-static void applesmc_io_data_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
+                                   unsigned size)
 {
     AppleSMCState *s = opaque;

@@ -138,7 +142,8 @@ static void applesmc_io_data_writeb(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-static uint32_t applesmc_io_data_readb(void *opaque, uint32_t addr1)
+static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr1,
+                                      unsigned size)
 {
     AppleSMCState *s = opaque;
     uint8_t retval = 0;

@@ -162,7 +167,7 @@ static uint32_t applesmc_io_data_readb(void *opaque, uint32_t addr1)
     return retval;
 }
 
-static uint32_t applesmc_io_cmd_readb(void *opaque, uint32_t addr1)
+static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr1, unsigned size)
 {
     AppleSMCState *s = opaque;

@@ -201,18 +206,39 @@ static void qdev_applesmc_isa_reset(DeviceState *dev)
     applesmc_add_key(s, "MSSD", 1, "\0x3");
 }
 
+static const MemoryRegionOps applesmc_data_io_ops = {
+    .write = applesmc_io_data_write,
+    .read = applesmc_io_data_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static const MemoryRegionOps applesmc_cmd_io_ops = {
+    .write = applesmc_io_cmd_write,
+    .read = applesmc_io_cmd_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 static void applesmc_isa_realize(DeviceState *dev, Error **errp)
 {
     AppleSMCState *s = APPLE_SMC(dev);
 
-    register_ioport_read(s->iobase + APPLESMC_DATA_PORT, 4, 1,
-                         applesmc_io_data_readb, s);
-    register_ioport_read(s->iobase + APPLESMC_CMD_PORT, 4, 1,
-                         applesmc_io_cmd_readb, s);
-    register_ioport_write(s->iobase + APPLESMC_DATA_PORT, 4, 1,
-                          applesmc_io_data_writeb, s);
-    register_ioport_write(s->iobase + APPLESMC_CMD_PORT, 4, 1,
-                          applesmc_io_cmd_writeb, s);
+    memory_region_init_io(&s->io_data, &applesmc_data_io_ops, s,
+                          "applesmc-data", 4);
+    isa_register_ioport(&s->parent_obj, &s->io_data,
+                        s->iobase + APPLESMC_DATA_PORT);
+
+    memory_region_init_io(&s->io_cmd, &applesmc_cmd_io_ops, s,
+                          "applesmc-cmd", 4);
+    isa_register_ioport(&s->parent_obj, &s->io_cmd,
+                        s->iobase + APPLESMC_CMD_PORT);
 
     if (!s->osk || (strlen(s->osk) != 64)) {
         fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #3 lore raw

PATCH v2 3/13 wdt_ib700: replace register_ioport*

In reply to Jan Kiszka

Convert over to memory regions to obsolete register_ioport*.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/watchdog/wdt_ib700.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

hw/watchdog/wdt_ib700.c +10 -2

index d85c894..597a923 100644
@@ -97,15 +97,23 @@ static const VMStateDescription vmstate_ib700 = {
     }
 };
 
+static const MemoryRegionPortio wdt_portio_list[] = {
+    { 0x441, 2, 1, .write = ib700_write_disable_reg, },
+    { 0x443, 2, 1, .write = ib700_write_enable_reg, },
+    PORTIO_END_OF_LIST(),
+};
+
 static void wdt_ib700_realize(DeviceState *dev, Error **errp)
 {
     IB700State *s = IB700(dev);
+    PortioList *port_list = g_new(PortioList, 1);
 
     ib700_debug("watchdog init\n");
 
     s->timer = qemu_new_timer_ns(vm_clock, ib700_timer_expired, s);
-    register_ioport_write(0x441, 2, 1, ib700_write_disable_reg, s);
-    register_ioport_write(0x443, 2, 1, ib700_write_enable_reg, s);
+
+    portio_list_init(port_list, wdt_portio_list, s, "ib700");
+    portio_list_add(port_list, isa_address_space_io(&s->parent_obj), 0);
 }
 
 static void wdt_ib700_reset(DeviceState *dev)

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #4 lore raw

PATCH v2 4/13 i82374: replace register_ioport*

In reply to Jan Kiszka

Convert over to memory regions to obsolete register_ioport*.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/dma/i82374.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

hw/dma/i82374.c +13 -5

index 6192780..ecda5cb 100644
@@ -124,16 +124,24 @@ static const VMStateDescription vmstate_isa_i82374 = {
     },
 };
 
+static const MemoryRegionPortio i82374_portio_list[] = {
+    { 0x0A, 1, 1, .read = i82374_read_isr, },
+    { 0x10, 8, 1, .write = i82374_write_command, },
+    { 0x18, 8, 1, .read = i82374_read_status, },
+    { 0x20, 0x20, 1,
+      .write = i82374_write_descriptor, .read = i82374_read_descriptor, },
+    PORTIO_END_OF_LIST(),
+};
+
 static void i82374_isa_realize(DeviceState *dev, Error **errp)
 {
     ISAi82374State *isa = I82374(dev);
     I82374State *s = &isa->state;
+    PortioList *port_list = g_new(PortioList, 1);
 
-    register_ioport_read(isa->iobase + 0x0A, 1, 1, i82374_read_isr, s);
-    register_ioport_write(isa->iobase + 0x10, 8, 1, i82374_write_command, s);
-    register_ioport_read(isa->iobase + 0x18, 8, 1, i82374_read_status, s);
-    register_ioport_write(isa->iobase + 0x20, 0x20, 1, i82374_write_descriptor, s);
-    register_ioport_read(isa->iobase + 0x20, 0x20, 1, i82374_read_descriptor, s);
+    portio_list_init(port_list, i82374_portio_list, s, "i82374");
+    portio_list_add(port_list, isa_address_space_io(&isa->parent_obj),
+                    isa->iobase);
 
     i82374_realize(s, errp);

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:41 UTC #5 lore raw

PATCH v2 5/13 prep: replace register_ioport*

In reply to Jan Kiszka

Convert over to memory regions to obsolete register_ioport*.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/ppc/prep.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

hw/ppc/prep.c +15 -8

index 4fdc164..e7689ad 100644
@@ -434,6 +434,16 @@ static void ppc_prep_reset(void *opaque)
     cpu->env.nip = 0xfffffffc;
 }
 
+static const MemoryRegionPortio prep_portio_list[] = {
+    /* System control ports */
+    { 0x0092, 1, 1, .read = PREP_io_800_readb, .write = PREP_io_800_writeb, },
+    { 0x0800, 0x52, 1,
+      .read = PREP_io_800_readb, .write = PREP_io_800_writeb, },
+    /* Special port to get debug messages from Open-Firmware */
+    { 0x0F00, 4, 1, .write = PPC_debug_write, },
+    PORTIO_END_OF_LIST(),
+};
+
 /* PowerPC PREP hardware initialisation */
 static void ppc_prep_init(QEMUMachineInitArgs *args)
 {

@@ -450,6 +460,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     nvram_t nvram;
     M48t59State *m48t59;
     MemoryRegion *PPC_io_memory = g_new(MemoryRegion, 1);
+    PortioList *port_list = g_new(PortioList, 1);
 #if 0
     MemoryRegion *xcsr = g_new(MemoryRegion, 1);
 #endif

@@ -641,11 +652,10 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     isa_create_simple(isa_bus, "i8042");
 
     sysctrl->reset_irq = first_cpu->irq_inputs[PPC6xx_INPUT_HRESET];
-    /* System control ports */
-    register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
-    register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);
-    register_ioport_read(0x0800, 0x52, 1, &PREP_io_800_readb, sysctrl);
-    register_ioport_write(0x0800, 0x52, 1, &PREP_io_800_writeb, sysctrl);
+
+    portio_list_init(port_list, prep_portio_list, sysctrl, "prep");
+    portio_list_add(port_list, get_system_io(), 0x0);
+
     /* PowerPC control and status register group */
 #if 0
     memory_region_init_io(xcsr, &PPC_XCSR_ops, NULL, "ppc-xcsr", 0x1000);

@@ -672,9 +682,6 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
                          /* XXX: need an option to load a NVRAM image */
                          0,
                          graphic_width, graphic_height, graphic_depth);
-
-    /* Special port to get debug messages from Open-Firmware */
-    register_ioport_write(0x0F00, 4, 1, &PPC_debug_write, NULL);
 }
 
 static QEMUMachine prep_machine = {

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:41 UTC #6 lore raw

PATCH v2 6/13 vt82c686: replace register_ioport*

In reply to Jan Kiszka

Convert over to memory regions to obsolete register_ioport*.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/isa/vt82c686.c |   40 ++++++++++++++++++++++++++--------------
 1 files changed, 26 insertions(+), 14 deletions(-)

hw/isa/vt82c686.c +26 ~~-14~~

index 391d90d..e5cd4cd 100644
@@ -43,10 +43,12 @@ typedef struct SuperIOConfig
 
 typedef struct VT82C686BState {
     PCIDevice dev;
+    MemoryRegion superio;
     SuperIOConfig superio_conf;
 } VT82C686BState;
 
-static void superio_ioport_writeb(void *opaque, uint32_t addr, uint32_t data)
+static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
+                                  unsigned size)
 {
     int can_write;
     SuperIOConfig *superio_conf = opaque;

@@ -93,7 +95,7 @@ static void superio_ioport_writeb(void *opaque, uint32_t addr, uint32_t data)
     }
 }
 
-static uint32_t superio_ioport_readb(void *opaque, uint32_t addr)
+static uint64_t superio_ioport_readb(void *opaque, hwaddr addr, unsigned size)
 {
     SuperIOConfig *superio_conf = opaque;

@@ -101,6 +103,16 @@ static uint32_t superio_ioport_readb(void *opaque, uint32_t addr)
     return (superio_conf->config[superio_conf->index]);
 }
 
+static const MemoryRegionOps superio_ops = {
+    .read = superio_ioport_readb,
+    .write = superio_ioport_writeb,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 static void vt82c686b_reset(void * opaque)
 {
     PCIDevice *d = opaque;

@@ -140,17 +152,7 @@ static void vt82c686b_write_config(PCIDevice * d, uint32_t address,
 
     pci_default_write_config(d, address, val, len);
     if (address == 0x85) {  /* enable or disable super IO configure */
-        if (val & 0x2) {
-            /* floppy also uses 0x3f0 and 0x3f1.
-             * But we do not emulate flopy,so just set it here. */
-            isa_unassign_ioport(0x3f0, 2);
-            register_ioport_read(0x3f0, 2, 1, superio_ioport_readb,
-                                 &vt686->superio_conf);
-            register_ioport_write(0x3f0, 2, 1, superio_ioport_writeb,
-                                  &vt686->superio_conf);
-        } else {
-            isa_unassign_ioport(0x3f0, 2);
-        }
+        memory_region_set_enabled(&vt686->superio, val & 0x2);
     }
 }

@@ -423,11 +425,13 @@ static const VMStateDescription vmstate_via = {
 /* init the PCI-to-ISA bridge */
 static int vt82c686b_initfn(PCIDevice *d)
 {
+    VT82C686BState *vt82c = DO_UPCAST(VT82C686BState, dev, d);
     uint8_t *pci_conf;
+    ISABus *isa_bus;
     uint8_t *wmask;
     int i;
 
-    isa_bus_new(&d->qdev, pci_address_space_io(d));
+    isa_bus = isa_bus_new(&d->qdev, pci_address_space_io(d));
 
     pci_conf = d->config;
     pci_config_set_prog_interface(pci_conf, 0x0);

@@ -439,6 +443,14 @@ static int vt82c686b_initfn(PCIDevice *d)
        }
     }
 
+    memory_region_init_io(&vt82c->superio, &superio_ops, &vt82c->superio_conf,
+                          "superio", 2);
+    memory_region_set_enabled(&vt82c->superio, false);
+    /* The floppy also uses 0x3f0 and 0x3f1.
+     * But we do not emulate a floppy, so just set it here. */
+    memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
+                                &vt82c->superio);
+
     qemu_register_reset(vt82c686b_reset, d);
 
     return 0;

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #7 lore raw

PATCH v2 7/13 Privatize register_ioport_read/write

In reply to Jan Kiszka

No more users outside of ioport.c.

Signed-off-by Jan Kiszka <[email protected]>

---
 include/exec/ioport.h |    4 ----
 ioport.c              |    8 ++++----
 2 files changed, 4 insertions(+), 8 deletions(-)

include/exec/ioport.h +0 -4

index fc28350..4953892 100644
@@ -39,10 +39,6 @@ typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
 typedef void (IOPortDestructor)(void *opaque);
 
 void ioport_register(IORange *iorange);
-int register_ioport_read(pio_addr_t start, int length, int size,
-                         IOPortReadFunc *func, void *opaque);
-int register_ioport_write(pio_addr_t start, int length, int size,
-                          IOPortWriteFunc *func, void *opaque);
 void isa_unassign_ioport(pio_addr_t start, int length);
 bool isa_is_ioport_assigned(pio_addr_t start);

ioport.c +4 -4

index a0ac2a0..d5b7fbd 100644
@@ -139,8 +139,8 @@ static int ioport_bsize(int size, int *bsize)
 }
 
 /* size is the word size in byte */
-int register_ioport_read(pio_addr_t start, int length, int size,
-                         IOPortReadFunc *func, void *opaque)
+static int register_ioport_read(pio_addr_t start, int length, int size,
+                                IOPortReadFunc *func, void *opaque)
 {
     int i, bsize;

@@ -159,8 +159,8 @@ int register_ioport_read(pio_addr_t start, int length, int size,
 }
 
 /* size is the word size in byte */
-int register_ioport_write(pio_addr_t start, int length, int size,
-                          IOPortWriteFunc *func, void *opaque)
+static int register_ioport_write(pio_addr_t start, int length, int size,
+                                 IOPortWriteFunc *func, void *opaque)
 {
     int i, bsize;

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #8 lore raw

PATCH v2 8/13 isa: implement isa_is_ioport_assigned via memory_region_find

In reply to Jan Kiszka

Open-code isa_is_ioport_assigned via a memory region lookup. As all IO ports are now directly or indirectly registered via the memory API, this becomes possible and will finally allow us to drop the ioport tables.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/acpi/piix4.c       |    9 +++++----
 hw/isa/lpc_ich9.c     |    9 +++++----
 include/exec/ioport.h |    1 -
 ioport.c              |    7 -------
 4 files changed, 10 insertions(+), 16 deletions(-)

hw/acpi/piix4.c +5 -4

index 756df3b..ff559c0 100644
@@ -383,14 +383,15 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque)
 static void piix4_pm_machine_ready(Notifier *n, void *opaque)
 {
     PIIX4PMState *s = container_of(n, PIIX4PMState, machine_ready);
+    MemoryRegion *io_as = pci_address_space_io(&s->dev);
     uint8_t *pci_conf;
 
     pci_conf = s->dev.config;
-    pci_conf[0x5f] = (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10;
+    pci_conf[0x5f] = 0x10 |
+        (memory_region_find(io_as, 0x378, 1).mr ? 0x80 : 0);
     pci_conf[0x63] = 0x60;
-    pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) |
-	(isa_is_ioport_assigned(0x2f8) ? 0x90 : 0);
-
+    pci_conf[0x67] = (memory_region_find(io_as, 0x3f8, 1).mr ? 0x08 : 0) |
+        (memory_region_find(io_as, 0x2f8, 1).mr ? 0x90 : 0);
 }
 
 static int piix4_pm_initfn(PCIDevice *dev)

hw/isa/lpc_ich9.c +5 -4

index 667e882..461ab7c 100644
@@ -477,22 +477,23 @@ static const MemoryRegionOps rbca_mmio_ops = {
 static void ich9_lpc_machine_ready(Notifier *n, void *opaque)
 {
     ICH9LPCState *s = container_of(n, ICH9LPCState, machine_ready);
+    MemoryRegion *io_as = pci_address_space_io(&s->d);
     uint8_t *pci_conf;
 
     pci_conf = s->d.config;
-    if (isa_is_ioport_assigned(0x3f8)) {
+    if (memory_region_find(io_as, 0x3f8, 1).mr) {
         /* com1 */
         pci_conf[0x82] |= 0x01;
     }
-    if (isa_is_ioport_assigned(0x2f8)) {
+    if (memory_region_find(io_as, 0x2f8, 1).mr) {
         /* com2 */
         pci_conf[0x82] |= 0x02;
     }
-    if (isa_is_ioport_assigned(0x378)) {
+    if (memory_region_find(io_as, 0x378, 1).mr) {
         /* lpt */
         pci_conf[0x82] |= 0x04;
     }
-    if (isa_is_ioport_assigned(0x3f0)) {
+    if (memory_region_find(io_as, 0x3f0, 1).mr) {
         /* floppy */
         pci_conf[0x82] |= 0x08;
     }

include/exec/ioport.h +0 -1

index 4953892..eb99ffe 100644
@@ -40,7 +40,6 @@ typedef void (IOPortDestructor)(void *opaque);
 
 void ioport_register(IORange *iorange);
 void isa_unassign_ioport(pio_addr_t start, int length);
-bool isa_is_ioport_assigned(pio_addr_t start);
 
 void cpu_outb(pio_addr_t addr, uint8_t val);
 void cpu_outw(pio_addr_t addr, uint16_t val);

ioport.c +0 -7

index d5b7fbd..56470c5 100644
@@ -273,13 +273,6 @@ void isa_unassign_ioport(pio_addr_t start, int length)
     }
 }
 
-bool isa_is_ioport_assigned(pio_addr_t start)
-{
-    return (ioport_read_table[0][start] || ioport_write_table[0][start] ||
-	    ioport_read_table[1][start] || ioport_write_table[1][start] ||
-	    ioport_read_table[2][start] || ioport_write_table[2][start]);
-}
-
 /***********************************************************/
 
 void cpu_outb(pio_addr_t addr, uint8_t val)

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:41 UTC #9 lore raw

PATCH v2 9/13 vmware-vga: Accept unaligned I/O accesses

In reply to Jan Kiszka

Before switching to the memory core dispatcher, we need to make sure that this pv-device will continue to receive unaligned portio accesses.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/display/vmware_vga.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

hw/display/vmware_vga.c +4 -0

index fd3569d..ec41681 100644
@@ -1241,6 +1241,10 @@ static const MemoryRegionOps vmsvga_io_ops = {
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
+        .unaligned = true,
+    },
+    .impl = {
+        .unaligned = true,
     },
 };

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:41 UTC #10 lore raw

PATCH v2 10/13 xen: Mark fixed platform I/O as unaligned

In reply to Jan Kiszka

Before switching to the memory core dispatcher, we need to make sure that this pv-device will continue to receive unaligned portio accesses.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/xen/xen_platform.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

hw/xen/xen_platform.c +4 -0

index b6c6793..f8f5dd5 100644
@@ -262,9 +262,13 @@ static void platform_fixed_ioport_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps platform_fixed_io_ops = {
     .read = platform_fixed_ioport_read,
     .write = platform_fixed_ioport_write,
+    .valid = {
+        .unaligned = true,
+    },
     .impl = {
         .min_access_size = 1,
         .max_access_size = 4,
+        .unaligned = true,
     },
     .endianness = DEVICE_LITTLE_ENDIAN,
 };

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #11 lore raw

PATCH v2 11/13 ioport: Switch dispatching to memory core layer

In reply to Jan Kiszka

The current ioport dispatcher is a complex beast, mostly due to the need to deal with old portio interface users. But we can overcome it without converting all portio users by embedding the required base address of a MemoryRegionPortio access into that data structure. That removes the need to have the additional MemoryRegionIORange structure in the loop on every access.

To handle old portio memory ops, we simply install dispatching handlers for portio memory regions when registering them with the memory core. This removes the need for the old_portio field.

We can drop the additional aliasing of ioport regions and also the special address space listener. cpu_in and cpu_out now simply call address_space_read/write. And we can concentrate portio handling in a single source file.

Signed-off-by Jan Kiszka <[email protected]>

---
 exec.c                         |   27 --------
 include/exec/ioport.h          |    1 -
 include/exec/memory-internal.h |    2 -
 include/exec/memory.h          |    5 +-
 ioport.c                       |  137 +++++++++++++++++++++++++++++-----------
 memory.c                       |   88 -------------------------
 6 files changed, 101 insertions(+), 159 deletions(-)

exec.c +0 ~~-27~~

index 0b0118b..87c7ef3 100644
@@ -1760,26 +1760,6 @@ static void core_log_global_stop(MemoryListener *listener)
     cpu_physical_memory_set_dirty_tracking(0);
 }
 
-static void io_region_add(MemoryListener *listener,
-                          MemoryRegionSection *section)
-{
-    MemoryRegionIORange *mrio = g_new(MemoryRegionIORange, 1);
-
-    mrio->mr = section->mr;
-    mrio->offset = section->offset_within_region;
-    iorange_init(&mrio->iorange, &memory_region_iorange_ops,
-                 section->offset_within_address_space,
-                 int128_get64(section->size));
-    ioport_register(&mrio->iorange);
-}
-
-static void io_region_del(MemoryListener *listener,
-                          MemoryRegionSection *section)
-{
-    isa_unassign_ioport(section->offset_within_address_space,
-                        int128_get64(section->size));
-}
-
 static MemoryListener core_memory_listener = {
     .begin = core_begin,
     .log_global_start = core_log_global_start,

@@ -1787,12 +1767,6 @@ static MemoryListener core_memory_listener = {
     .priority = 1,
 };
 
-static MemoryListener io_memory_listener = {
-    .region_add = io_region_add,
-    .region_del = io_region_del,
-    .priority = 0,
-};
-
 static MemoryListener tcg_memory_listener = {
     .commit = tcg_commit,
 };

@@ -1834,7 +1808,6 @@ static void memory_map_init(void)
     address_space_init(&address_space_io, system_io, "I/O");
 
     memory_listener_register(&core_memory_listener, &address_space_memory);
-    memory_listener_register(&io_memory_listener, &address_space_io);
     memory_listener_register(&tcg_memory_listener, &address_space_memory);
 }

include/exec/ioport.h +0 -1

index eb99ffe..b476857 100644
@@ -56,7 +56,6 @@ typedef struct PortioList {
     struct MemoryRegion *address_space;
     unsigned nr;
     struct MemoryRegion **regions;
-    struct MemoryRegion **aliases;
     void *opaque;
     const char *name;
 } PortioList;

include/exec/memory-internal.h +0 -2

index 26689fe..d0e0633 100644
@@ -119,8 +119,6 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
 void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
                                      int dirty_flags);
 
-extern const IORangeOps memory_region_iorange_ops;
-
 #endif
 
 #endif

include/exec/memory.h +1 -4

index 3598c4f..7c1e6da 100644
@@ -124,10 +124,6 @@ struct MemoryRegionOps {
          bool unaligned;
     } impl;
 
-    /* If .read and .write are not present, old_portio may be used for
-     * backwards compatibility with old portio registration
-     */
-    const MemoryRegionPortio *old_portio;
     /* If .read and .write are not present, old_mmio may be used for
      * backwards compatibility with old mmio registration
      */

@@ -183,6 +179,7 @@ struct MemoryRegionPortio {
     unsigned size;
     IOPortReadFunc *read;
     IOPortWriteFunc *write;
+    uint32_t base; /* private field */
 };
 
 #define PORTIO_END_OF_LIST() { }

ioport.c +100 ~~-37~~

227 lines not on this page · raw message

memory.c +0 ~~-88~~

94 lines not on this page · raw message

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #12 lore raw

PATCH v2 12/13 ioport: Remove unused old dispatching services

In reply to Jan Kiszka

Remove unused ioport_register and isa_unassign_ioport along with everything that only those services used.

Signed-off-by Jan Kiszka <[email protected]>

---
 include/exec/ioport.h  |    5 -
 include/exec/iorange.h |   31 ------
 include/exec/memory.h  |    9 --
 ioport.c               |  238 ------------------------------------------------
 4 files changed, 0 insertions(+), 283 deletions(-)
 delete mode 100644 include/exec/iorange.h

include/exec/ioport.h +0 -5

index b476857..ba3ebb8 100644
@@ -25,7 +25,6 @@
 #define IOPORT_H
 
 #include "qemu-common.h"
-#include "exec/iorange.h"
 
 typedef uint32_t pio_addr_t;
 #define FMT_pioaddr     PRIx32

@@ -36,10 +35,6 @@ typedef uint32_t pio_addr_t;
 /* These should really be in isa.h, but are here to make pc.h happy.  */
 typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
 typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
-typedef void (IOPortDestructor)(void *opaque);
-
-void ioport_register(IORange *iorange);
-void isa_unassign_ioport(pio_addr_t start, int length);
 
 void cpu_outb(pio_addr_t addr, uint8_t val);
 void cpu_outw(pio_addr_t addr, uint16_t val);

include/exec/iorange.h deleted +0 ~~-31~~

deleted file mode 100644
index cd980a8..0000000
@@ -1,31 +0,0 @@
-#ifndef IORANGE_H
-#define IORANGE_H
-
-#include <stdint.h>
-
-typedef struct IORange IORange;
-typedef struct IORangeOps IORangeOps;
-
-struct IORangeOps {
-    void (*read)(IORange *iorange, uint64_t offset, unsigned width,
-                 uint64_t *data);
-    void (*write)(IORange *iorange, uint64_t offset, unsigned width,
-                  uint64_t data);
-    void (*destructor)(IORange *iorange);
-};
-
-struct IORange {
-    const IORangeOps *ops;
-    uint64_t base;
-    uint64_t len;
-};
-
-static inline void iorange_init(IORange *iorange, const IORangeOps *ops,
-                                uint64_t base, uint64_t len)
-{
-    iorange->ops = ops;
-    iorange->base = base;
-    iorange->len = len;
-}
-
-#endif

include/exec/memory.h +0 -9

index 7c1e6da..8c1373e 100644
@@ -22,7 +22,6 @@
 #include "exec/cpu-common.h"
 #include "exec/hwaddr.h"
 #include "qemu/queue.h"
-#include "exec/iorange.h"
 #include "exec/ioport.h"
 #include "qemu/int128.h"
 #include "qemu/notify.h"

@@ -46,14 +45,6 @@ struct MemoryRegionMmio {
     CPUWriteMemoryFunc *write[3];
 };
 
-/* Internal use; thunks between old-style IORange and MemoryRegions. */
-typedef struct MemoryRegionIORange MemoryRegionIORange;
-struct MemoryRegionIORange {
-    IORange iorange;
-    MemoryRegion *mr;
-    hwaddr offset;
-};
-
 typedef struct IOMMUTLBEntry IOMMUTLBEntry;
 
 /* See address_space_translate: bit 0 is read, bit 1 is write.  */

ioport.c +0 ~~-238~~

252 lines not on this page · raw message

Signature · 1 line

1.7.3.4

Jan Kiszka

author

2013-06-21 16:42 UTC #13 lore raw

PATCH v2 13/13 ioport: Move IOPortRead/WriteFunc typedefs to memory.h

In reply to Jan Kiszka

Move the function types required for MemoryRegionPortio to memory.h. This allows to let ioport.h depend on memory.h, which is more consistent instead than the other way around.

Signed-off-by Jan Kiszka <[email protected]>

---
 include/exec/ioport.h |    8 +-------
 include/exec/memory.h |    4 +++-
 2 files changed, 4 insertions(+), 8 deletions(-)

include/exec/ioport.h +1 -7

index ba3ebb8..c7da6d4 100644
@@ -25,6 +25,7 @@
 #define IOPORT_H
 
 #include "qemu-common.h"
+#include "exec/memory.h"
 
 typedef uint32_t pio_addr_t;
 #define FMT_pioaddr     PRIx32

@@ -32,10 +33,6 @@ typedef uint32_t pio_addr_t;
 #define MAX_IOPORTS     (64 * 1024)
 #define IOPORTS_MASK    (MAX_IOPORTS - 1)
 
-/* These should really be in isa.h, but are here to make pc.h happy.  */
-typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
-typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
-
 void cpu_outb(pio_addr_t addr, uint8_t val);
 void cpu_outw(pio_addr_t addr, uint16_t val);
 void cpu_outl(pio_addr_t addr, uint32_t val);

@@ -43,9 +40,6 @@ uint8_t cpu_inb(pio_addr_t addr);
 uint16_t cpu_inw(pio_addr_t addr);
 uint32_t cpu_inl(pio_addr_t addr);
 
-struct MemoryRegion;
-struct MemoryRegionPortio;
-
 typedef struct PortioList {
     const struct MemoryRegionPortio *ports;
     struct MemoryRegion *address_space;

include/exec/memory.h +3 -1

index 8c1373e..e73c7d0 100644
@@ -22,7 +22,6 @@
 #include "exec/cpu-common.h"
 #include "exec/hwaddr.h"
 #include "qemu/queue.h"
-#include "exec/ioport.h"
 #include "qemu/int128.h"
 #include "qemu/notify.h"

@@ -164,6 +163,9 @@ struct MemoryRegion {
     NotifierList iommu_notify;
 };
 
+typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
+typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
+
 struct MemoryRegionPortio {
     uint32_t offset;
     uint32_t len;

Signature · 1 line

1.7.3.4

Paolo Bonzini

2013-06-21 20:09 UTC #14 lore raw

In reply to Jan Kiszka

Il 21/06/2013 18:41, Jan Kiszka ha scritto:

Quoted 9 lines

First, this series converts the remaining users of register_ioport* to portio lists. Then it replaces the current portio dispatcher with the existing one for MMIO and removes several lines of code. This also allows to build BQL-free portio on top once we enhance the memory layer accordingly.
Changes in v2:
- rebased over master (which already contains some patches from v1)
- refactored PIO dispatching according to Paolo's suggestions

Patches 1-12 are fine.

For patch 13, perhaps you could instead move MemoryRegionPortio to ioport.h and remove all mention of ports from memory.h?

Paolo