[Qemu-devel] Re: [PATCH V6 12/32] pci_host.h: move functions in pci_host.h into .c file.

[Michael S. Tsirkin]

On Fri, Oct 30, 2009 at 09:21:06PM +0900, Isaku Yamahata wrote:
> split static functions in pci_host.h into pci_host.c and
> pci_host_template.h.
> Later a structures declared in pci_host.h, PCIHostState, will be used.
> However pci_host.h doesn't allow to include itself easily. This patches
> addresses it.
> 
> pci_host.h includes functions which are instantiated in .c by including
> pci_host.h with typedefing pci_addr_t.
> pci_addr_t is per pci host bridge and is typedef'ed to uint32_t for ioio
> or target_phys_addr_t for mmio in .c file.
> That prevents from including pci_host.h to use PCIHostState because of
> requiring type, pci_addr_t.
> 
> Its purpose to include is to instantiate io function for mmio or ioio
> depending on which pci host bridge requires ioio or mmio.
> To avoid including code, we always instantiate both version.
> 
> Signed-off-by: Isaku Yamahata <address@hidden>

I think this is good cleanup.  Maybe names can be shorter:
pci_host_data_register_io_memory -> pci_host_register_io_memory
pci_host_data_register_ioport -> pci_host_register_ioport

What do you think?

Acked-by: Michael S. Tsirkin <address@hidden>

> ---
>  Makefile.target                        |    2 +-
>  hw/apb_pci.c                           |   19 +------
>  hw/grackle_pci.c                       |   24 +-------
>  hw/gt64xxx.c                           |   11 +----
>  hw/pci_host.c                          |   75 ++++++++++++++++++++++++++
>  hw/pci_host.h                          |   90 
> +++-----------------------------
>  hw/{pci_host.h => pci_host_template.h} |   36 ++++---------
>  hw/piix_pci.c                          |   11 +---
>  hw/ppc4xx_pci.c                        |   18 +------
>  hw/ppce500_pci.c                       |   17 +------
>  hw/prep_pci.c                          |    9 +---
>  hw/unin_pci.c                          |   40 ++------------
>  12 files changed, 111 insertions(+), 241 deletions(-)
>  create mode 100644 hw/pci_host.c
>  copy hw/{pci_host.h => pci_host_template.h} (80%)
> 
> diff --git a/Makefile.target b/Makefile.target
> index fefd7ac..db0d0ab 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -154,7 +154,7 @@ endif #CONFIG_BSD_USER
>  # System emulator target
>  ifdef CONFIG_SOFTMMU
>  
> -obj-y = vl.o async.o monitor.o pci.o machine.o gdbstub.o
> +obj-y = vl.o async.o monitor.o pci.o pci_host.o machine.o gdbstub.o
>  # virtio has to be here due to weird dependency between PCI and virtio-net.
>  # need to fix this properly
>  obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o 
> virtio-pci.o
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index 1ea3e0d..560617a 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -28,6 +28,7 @@
>  
>  #include "sysbus.h"
>  #include "pci.h"
> +#include "pci_host.h"
>  
>  /* debug APB */
>  //#define DEBUG_APB
> @@ -48,9 +49,6 @@ do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
>   * http://www.sun.com/processors/manuals/805-1251.pdf
>   */
>  
> -typedef target_phys_addr_t pci_addr_t;
> -#include "pci_host.h"
> -
>  typedef struct APBState {
>      SysBusDevice busdev;
>      PCIHostState host_state;
> @@ -145,18 +143,6 @@ static CPUReadMemoryFunc * const apb_config_read[] = {
>      &apb_config_readl,
>  };
>  
> -static CPUWriteMemoryFunc * const pci_apb_write[] = {
> -    &pci_host_data_writeb,
> -    &pci_host_data_writew,
> -    &pci_host_data_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_apb_read[] = {
> -    &pci_host_data_readb,
> -    &pci_host_data_readw,
> -    &pci_host_data_readl,
> -};
> -
>  static void pci_apb_iowriteb (void *opaque, target_phys_addr_t addr,
>                                    uint32_t val)
>  {
> @@ -293,8 +279,7 @@ static int pci_pbm_init_device(SysBusDevice *dev)
>                                              pci_apb_config_write, s);
>      sysbus_init_mmio(dev, 0x10ULL, pci_mem_config);
>      /* mem_data */
> -    pci_mem_data = cpu_register_io_memory(pci_apb_read,
> -                                          pci_apb_write, &s->host_state);
> +    pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
>      sysbus_init_mmio(dev, 0x10000000ULL, pci_mem_data);
>      return 0;
>  }
> diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
> index b49cf1e..8407cd2 100644
> --- a/hw/grackle_pci.c
> +++ b/hw/grackle_pci.c
> @@ -26,6 +26,7 @@
>  #include "sysbus.h"
>  #include "ppc_mac.h"
>  #include "pci.h"
> +#include "pci_host.h"
>  
>  /* debug Grackle */
>  //#define DEBUG_GRACKLE
> @@ -37,9 +38,6 @@
>  #define GRACKLE_DPRINTF(fmt, ...)
>  #endif
>  
> -typedef target_phys_addr_t pci_addr_t;
> -#include "pci_host.h"
> -
>  typedef struct GrackleState {
>      SysBusDevice busdev;
>      PCIHostState host_state;
> @@ -84,18 +82,6 @@ static CPUReadMemoryFunc * const pci_grackle_config_read[] 
> = {
>      &pci_grackle_config_readl,
>  };
>  
> -static CPUWriteMemoryFunc * const pci_grackle_write[] = {
> -    &pci_host_data_writeb,
> -    &pci_host_data_writew,
> -    &pci_host_data_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_grackle_read[] = {
> -    &pci_host_data_readb,
> -    &pci_host_data_readw,
> -    &pci_host_data_readl,
> -};
> -
>  /* Don't know if this matches real hardware, but it agrees with OHW.  */
>  static int pci_grackle_map_irq(PCIDevice *pci_dev, int irq_num)
>  {
> @@ -163,9 +149,7 @@ static int pci_grackle_init_device(SysBusDevice *dev)
>  
>      pci_mem_config = cpu_register_io_memory(pci_grackle_config_read,
>                                              pci_grackle_config_write, s);
> -    pci_mem_data = cpu_register_io_memory(pci_grackle_read,
> -                                          pci_grackle_write,
> -                                          &s->host_state);
> +    pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_config);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_data);
>  
> @@ -185,9 +169,7 @@ static int pci_dec_21154_init_device(SysBusDevice *dev)
>  
>      pci_mem_config = cpu_register_io_memory(pci_grackle_config_read,
>                                              pci_grackle_config_write, s);
> -    pci_mem_data = cpu_register_io_memory(pci_grackle_read,
> -                                          pci_grackle_write,
> -                                          &s->host_state);
> +    pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_config);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_data);
>      return 0;
> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> index 8f9ae4a..fb7f5bd 100644
> --- a/hw/gt64xxx.c
> +++ b/hw/gt64xxx.c
> @@ -25,10 +25,8 @@
>  #include "hw.h"
>  #include "mips.h"
>  #include "pci.h"
> -#include "pc.h"
> -
> -typedef target_phys_addr_t pci_addr_t;
>  #include "pci_host.h"
> +#include "pc.h"
>  
>  //#define DEBUG
>  
> @@ -1119,13 +1117,6 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
>      GT64120State *s;
>      PCIDevice *d;
>  
> -    (void)&pci_host_data_writeb; /* avoid warning */
> -    (void)&pci_host_data_writew; /* avoid warning */
> -    (void)&pci_host_data_writel; /* avoid warning */
> -    (void)&pci_host_data_readb; /* avoid warning */
> -    (void)&pci_host_data_readw; /* avoid warning */
> -    (void)&pci_host_data_readl; /* avoid warning */
> -
>      s = qemu_mallocz(sizeof(GT64120State));
>      s->pci = qemu_mallocz(sizeof(GT64120PCIState));
>  
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> new file mode 100644
> index 0000000..45da1e7
> --- /dev/null
> +++ b/hw/pci_host.c
> @@ -0,0 +1,75 @@
> +/*
> + * pci_host.c
> + *
> + * Copyright (c) 2009 Isaku Yamahata <yamahata at valinux co jp>
> + *                    VA Linux Systems Japan K.K.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include "pci.h"
> +#include "pci_host.h"
> +
> +/* debug PCI */
> +//#define DEBUG_PCI
> +
> +#ifdef DEBUG_PCI
> +#define PCI_DPRINTF(fmt, ...) \
> +do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define PCI_DPRINTF(fmt, ...)
> +#endif
> +
> +#define PCI_ADDR_T      target_phys_addr_t
> +#define PCI_HOST_SUFFIX _mmio
> +
> +#include "pci_host_template.h"
> +
> +static CPUWriteMemoryFunc * const pci_host_data_write_mmio[] = {
> +    pci_host_data_writeb_mmio,
> +    pci_host_data_writew_mmio,
> +    pci_host_data_writel_mmio,
> +};
> +
> +static CPUReadMemoryFunc * const pci_host_data_read_mmio[] = {
> +    pci_host_data_readb_mmio,
> +    pci_host_data_readw_mmio,
> +    pci_host_data_readl_mmio,
> +};
> +
> +int pci_host_data_register_io_memory(PCIHostState *s)
> +{
> +    return cpu_register_io_memory(pci_host_data_read_mmio,
> +                                  pci_host_data_write_mmio,
> +                                  s);
> +}
> +
> +#undef PCI_ADDR_T
> +#undef PCI_HOST_SUFFIX
> +
> +#define PCI_ADDR_T      uint32_t
> +#define PCI_HOST_SUFFIX _ioport
> +
> +#include "pci_host_template.h"
> +
> +void pci_host_data_register_ioport(pio_addr_t ioport, PCIHostState *s)
> +{
> +    register_ioport_write(ioport, 4, 1, pci_host_data_writeb_ioport, s);
> +    register_ioport_write(ioport, 4, 2, pci_host_data_writew_ioport, s);
> +    register_ioport_write(ioport, 4, 4, pci_host_data_writel_ioport, s);
> +    register_ioport_read(ioport, 4, 1, pci_host_data_readb_ioport, s);
> +    register_ioport_read(ioport, 4, 2, pci_host_data_readw_ioport, s);
> +    register_ioport_read(ioport, 4, 4, pci_host_data_readl_ioport, s);
> +}
> diff --git a/hw/pci_host.h b/hw/pci_host.h
> index 48862b5..92a35f9 100644
> --- a/hw/pci_host.h
> +++ b/hw/pci_host.h
> @@ -25,97 +25,21 @@
>  /* Worker routines for a PCI host controller that uses an {address,data}
>     register pair to access PCI configuration space.  */
>  
> -/* debug PCI */
> -//#define DEBUG_PCI
> +#ifndef PCI_HOST_H
> +#define PCI_HOST_H
>  
>  #include "sysbus.h"
>  
> -#ifdef DEBUG_PCI
> -#define PCI_DPRINTF(fmt, ...) \
> -do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define PCI_DPRINTF(fmt, ...)
> -#endif
> -
>  typedef struct {
>      SysBusDevice busdev;
>      uint32_t config_reg;
>      PCIBus *bus;
>  } PCIHostState;
>  
> -static void pci_host_data_writeb(void* opaque, pci_addr_t addr, uint32_t val)
> -{
> -    PCIHostState *s = opaque;
> -
> -    PCI_DPRINTF("writeb addr " TARGET_FMT_plx " val %x\n",
> -                (target_phys_addr_t)addr, val);
> -    if (s->config_reg & (1u << 31))
> -        pci_data_write(s->bus, s->config_reg | (addr & 3), val, 1);
> -}
> -
> -static void pci_host_data_writew(void* opaque, pci_addr_t addr, uint32_t val)
> -{
> -    PCIHostState *s = opaque;
> -#ifdef TARGET_WORDS_BIGENDIAN
> -    val = bswap16(val);
> -#endif
> -    PCI_DPRINTF("writew addr " TARGET_FMT_plx " val %x\n",
> -                (target_phys_addr_t)addr, val);
> -    if (s->config_reg & (1u << 31))
> -        pci_data_write(s->bus, s->config_reg | (addr & 3), val, 2);
> -}
> -
> -static void pci_host_data_writel(void* opaque, pci_addr_t addr, uint32_t val)
> -{
> -    PCIHostState *s = opaque;
> -#ifdef TARGET_WORDS_BIGENDIAN
> -    val = bswap32(val);
> -#endif
> -    PCI_DPRINTF("writel addr " TARGET_FMT_plx " val %x\n",
> -                (target_phys_addr_t)addr, val);
> -    if (s->config_reg & (1u << 31))
> -        pci_data_write(s->bus, s->config_reg, val, 4);
> -}
> -
> -static uint32_t pci_host_data_readb(void* opaque, pci_addr_t addr)
> -{
> -    PCIHostState *s = opaque;
> -    uint32_t val;
> -
> -    if (!(s->config_reg & (1 << 31)))
> -        return 0xff;
> -    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 1);
> -    PCI_DPRINTF("readb addr " TARGET_FMT_plx " val %x\n",
> -                (target_phys_addr_t)addr, val);
> -    return val;
> -}
> +/* for mmio */
> +int pci_host_data_register_io_memory(PCIHostState *s);
>  
> -static uint32_t pci_host_data_readw(void* opaque, pci_addr_t addr)
> -{
> -    PCIHostState *s = opaque;
> -    uint32_t val;
> -    if (!(s->config_reg & (1 << 31)))
> -        return 0xffff;
> -    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 2);
> -    PCI_DPRINTF("readw addr " TARGET_FMT_plx " val %x\n",
> -                (target_phys_addr_t)addr, val);
> -#ifdef TARGET_WORDS_BIGENDIAN
> -    val = bswap16(val);
> -#endif
> -    return val;
> -}
> +/* for ioio */
> +void pci_host_data_register_ioport(pio_addr_t ioport, PCIHostState *s);
>  
> -static uint32_t pci_host_data_readl(void* opaque, pci_addr_t addr)
> -{
> -    PCIHostState *s = opaque;
> -    uint32_t val;
> -    if (!(s->config_reg & (1 << 31)))
> -        return 0xffffffff;
> -    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 4);
> -    PCI_DPRINTF("readl addr " TARGET_FMT_plx " val %x\n",
> -                (target_phys_addr_t)addr, val);
> -#ifdef TARGET_WORDS_BIGENDIAN
> -    val = bswap32(val);
> -#endif
> -    return val;
> -}
> +#endif /* PCI_HOST_H */
> diff --git a/hw/pci_host.h b/hw/pci_host_template.h
> similarity index 80%
> copy from hw/pci_host.h
> copy to hw/pci_host_template.h
> index 48862b5..11e6c88 100644
> --- a/hw/pci_host.h
> +++ b/hw/pci_host_template.h
> @@ -25,25 +25,8 @@
>  /* Worker routines for a PCI host controller that uses an {address,data}
>     register pair to access PCI configuration space.  */
>  
> -/* debug PCI */
> -//#define DEBUG_PCI
> -
> -#include "sysbus.h"
> -
> -#ifdef DEBUG_PCI
> -#define PCI_DPRINTF(fmt, ...) \
> -do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define PCI_DPRINTF(fmt, ...)
> -#endif
> -
> -typedef struct {
> -    SysBusDevice busdev;
> -    uint32_t config_reg;
> -    PCIBus *bus;
> -} PCIHostState;
> -
> -static void pci_host_data_writeb(void* opaque, pci_addr_t addr, uint32_t val)
> +static void glue(pci_host_data_writeb, PCI_HOST_SUFFIX)(
> +    void* opaque, PCI_ADDR_T addr, uint32_t val)
>  {
>      PCIHostState *s = opaque;
>  
> @@ -53,7 +36,8 @@ static void pci_host_data_writeb(void* opaque, pci_addr_t 
> addr, uint32_t val)
>          pci_data_write(s->bus, s->config_reg | (addr & 3), val, 1);
>  }
>  
> -static void pci_host_data_writew(void* opaque, pci_addr_t addr, uint32_t val)
> +static void glue(pci_host_data_writew, PCI_HOST_SUFFIX)(
> +    void* opaque, PCI_ADDR_T addr, uint32_t val)
>  {
>      PCIHostState *s = opaque;
>  #ifdef TARGET_WORDS_BIGENDIAN
> @@ -65,7 +49,8 @@ static void pci_host_data_writew(void* opaque, pci_addr_t 
> addr, uint32_t val)
>          pci_data_write(s->bus, s->config_reg | (addr & 3), val, 2);
>  }
>  
> -static void pci_host_data_writel(void* opaque, pci_addr_t addr, uint32_t val)
> +static void glue(pci_host_data_writel, PCI_HOST_SUFFIX)(
> +    void* opaque, PCI_ADDR_T addr, uint32_t val)
>  {
>      PCIHostState *s = opaque;
>  #ifdef TARGET_WORDS_BIGENDIAN
> @@ -77,7 +62,8 @@ static void pci_host_data_writel(void* opaque, pci_addr_t 
> addr, uint32_t val)
>          pci_data_write(s->bus, s->config_reg, val, 4);
>  }
>  
> -static uint32_t pci_host_data_readb(void* opaque, pci_addr_t addr)
> +static uint32_t glue(pci_host_data_readb, PCI_HOST_SUFFIX)(
> +    void* opaque, PCI_ADDR_T addr)
>  {
>      PCIHostState *s = opaque;
>      uint32_t val;
> @@ -90,7 +76,8 @@ static uint32_t pci_host_data_readb(void* opaque, 
> pci_addr_t addr)
>      return val;
>  }
>  
> -static uint32_t pci_host_data_readw(void* opaque, pci_addr_t addr)
> +static uint32_t glue(pci_host_data_readw, PCI_HOST_SUFFIX)(
> +    void* opaque, PCI_ADDR_T addr)
>  {
>      PCIHostState *s = opaque;
>      uint32_t val;
> @@ -105,7 +92,8 @@ static uint32_t pci_host_data_readw(void* opaque, 
> pci_addr_t addr)
>      return val;
>  }
>  
> -static uint32_t pci_host_data_readl(void* opaque, pci_addr_t addr)
> +static uint32_t glue(pci_host_data_readl, PCI_HOST_SUFFIX)(
> +    void* opaque, PCI_ADDR_T addr)
>  {
>      PCIHostState *s = opaque;
>      uint32_t val;
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index ed036fe..866348d 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -25,12 +25,10 @@
>  #include "hw.h"
>  #include "pc.h"
>  #include "pci.h"
> +#include "pci_host.h"
>  #include "isa.h"
>  #include "sysbus.h"
>  
> -typedef uint32_t pci_addr_t;
> -#include "pci_host.h"
> -
>  typedef PCIHostState I440FXState;
>  
>  typedef struct PIIX3State {
> @@ -197,12 +195,7 @@ static int i440fx_pcihost_initfn(SysBusDevice *dev)
>      register_ioport_write(0xcf8, 4, 4, i440fx_addr_writel, s);
>      register_ioport_read(0xcf8, 4, 4, i440fx_addr_readl, s);
>  
> -    register_ioport_write(0xcfc, 4, 1, pci_host_data_writeb, s);
> -    register_ioport_write(0xcfc, 4, 2, pci_host_data_writew, s);
> -    register_ioport_write(0xcfc, 4, 4, pci_host_data_writel, s);
> -    register_ioport_read(0xcfc, 4, 1, pci_host_data_readb, s);
> -    register_ioport_read(0xcfc, 4, 2, pci_host_data_readw, s);
> -    register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
> +    pci_host_data_register_ioport(0xcfc, s);
>      return 0;
>  }
>  
> diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
> index 655fe86..3aa7489 100644
> --- a/hw/ppc4xx_pci.c
> +++ b/hw/ppc4xx_pci.c
> @@ -22,8 +22,6 @@
>  #include "hw.h"
>  #include "ppc.h"
>  #include "ppc4xx.h"
> -
> -typedef target_phys_addr_t pci_addr_t;
>  #include "pci.h"
>  #include "pci_host.h"
>  #include "bswap.h"
> @@ -117,18 +115,6 @@ static CPUWriteMemoryFunc * const pci4xx_cfgaddr_write[] 
> = {
>      &pci4xx_cfgaddr_writel,
>  };
>  
> -static CPUReadMemoryFunc * const pci4xx_cfgdata_read[] = {
> -    &pci_host_data_readb,
> -    &pci_host_data_readw,
> -    &pci_host_data_readl,
> -};
> -
> -static CPUWriteMemoryFunc * const pci4xx_cfgdata_write[] = {
> -    &pci_host_data_writeb,
> -    &pci_host_data_writew,
> -    &pci_host_data_writel,
> -};
> -
>  static void ppc4xx_pci_reg_write4(void *opaque, target_phys_addr_t offset,
>                                    uint32_t value)
>  {
> @@ -392,9 +378,7 @@ PCIBus *ppc4xx_pci_init(CPUState *env, qemu_irq 
> pci_irqs[4],
>      cpu_register_physical_memory(config_space + PCIC0_CFGADDR, 4, index);
>  
>      /* CFGDATA */
> -    index = cpu_register_io_memory(pci4xx_cfgdata_read,
> -                                   pci4xx_cfgdata_write,
> -                                   &controller->pci_state);
> +    index = pci_host_data_register_io_memory(&controller->pci_state);
>      if (index < 0)
>          goto free;
>      cpu_register_physical_memory(config_space + PCIC0_CFGDATA, 4, index);
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 64fccfd..7c8cdad 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -17,7 +17,6 @@
>  #include "hw.h"
>  #include "ppc.h"
>  #include "ppce500.h"
> -typedef target_phys_addr_t pci_addr_t;
>  #include "pci.h"
>  #include "pci_host.h"
>  #include "bswap.h"
> @@ -116,18 +115,6 @@ static CPUWriteMemoryFunc * const 
> pcie500_cfgaddr_write[] = {
>      &pcie500_cfgaddr_writel,
>  };
>  
> -static CPUReadMemoryFunc * const pcie500_cfgdata_read[] = {
> -    &pci_host_data_readb,
> -    &pci_host_data_readw,
> -    &pci_host_data_readl,
> -};
> -
> -static CPUWriteMemoryFunc * const pcie500_cfgdata_write[] = {
> -    &pci_host_data_writeb,
> -    &pci_host_data_writew,
> -    &pci_host_data_writel,
> -};
> -
>  static uint32_t pci_reg_read4(void *opaque, target_phys_addr_t addr)
>  {
>      PPCE500PCIState *pci = opaque;
> @@ -344,9 +331,7 @@ PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], 
> target_phys_addr_t registers)
>      cpu_register_physical_memory(registers + PCIE500_CFGADDR, 4, index);
>  
>      /* CFGDATA */
> -    index = cpu_register_io_memory(pcie500_cfgdata_read,
> -                                   pcie500_cfgdata_write,
> -                                   &controller->pci_state);
> +    index = pci_host_data_register_io_memory(&controller->pci_state);
>      if (index < 0)
>          goto free;
>      cpu_register_physical_memory(registers + PCIE500_CFGDATA, 4, index);
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index 2d8a0fa..5a5b3da 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -24,8 +24,6 @@
>  
>  #include "hw.h"
>  #include "pci.h"
> -
> -typedef uint32_t pci_addr_t;
>  #include "pci_host.h"
>  
>  typedef PCIHostState PREPPCIState;
> @@ -144,12 +142,7 @@ PCIBus *pci_prep_init(qemu_irq *pic)
>      register_ioport_write(0xcf8, 4, 4, pci_prep_addr_writel, s);
>      register_ioport_read(0xcf8, 4, 4, pci_prep_addr_readl, s);
>  
> -    register_ioport_write(0xcfc, 4, 1, pci_host_data_writeb, s);
> -    register_ioport_write(0xcfc, 4, 2, pci_host_data_writew, s);
> -    register_ioport_write(0xcfc, 4, 4, pci_host_data_writel, s);
> -    register_ioport_read(0xcfc, 4, 1, pci_host_data_readb, s);
> -    register_ioport_read(0xcfc, 4, 2, pci_host_data_readw, s);
> -    register_ioport_read(0xcfc, 4, 4, pci_host_data_readl, s);
> +    pci_host_data_register_ioport(0xcfc, s);
>  
>      PPC_io_memory = cpu_register_io_memory(PPC_PCIIO_read,
>                                             PPC_PCIIO_write, s);
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index 4abb5c8..6b8f98b 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -24,6 +24,7 @@
>  #include "hw.h"
>  #include "ppc_mac.h"
>  #include "pci.h"
> +#include "pci_host.h"
>  
>  /* debug UniNorth */
>  //#define DEBUG_UNIN
> @@ -35,9 +36,6 @@
>  #define UNIN_DPRINTF(fmt, ...)
>  #endif
>  
> -typedef target_phys_addr_t pci_addr_t;
> -#include "pci_host.h"
> -
>  typedef struct UNINState {
>      SysBusDevice busdev;
>      PCIHostState host_state;
> @@ -83,18 +81,6 @@ static CPUReadMemoryFunc * const 
> pci_unin_main_config_read[] = {
>      &pci_unin_main_config_readl,
>  };
>  
> -static CPUWriteMemoryFunc * const pci_unin_main_write[] = {
> -    &pci_host_data_writeb,
> -    &pci_host_data_writew,
> -    &pci_host_data_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_unin_main_read[] = {
> -    &pci_host_data_readb,
> -    &pci_host_data_readw,
> -    &pci_host_data_readl,
> -};
> -
>  static void pci_unin_config_writel (void *opaque, target_phys_addr_t addr,
>                                      uint32_t val)
>  {
> @@ -123,18 +109,6 @@ static CPUReadMemoryFunc * const pci_unin_config_read[] 
> = {
>      &pci_unin_config_readl,
>  };
>  
> -static CPUWriteMemoryFunc * const pci_unin_write[] = {
> -    &pci_host_data_writeb,
> -    &pci_host_data_writew,
> -    &pci_host_data_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_unin_read[] = {
> -    &pci_host_data_readb,
> -    &pci_host_data_readw,
> -    &pci_host_data_readl,
> -};
> -
>  /* Don't know if this matches real hardware, but it agrees with OHW.  */
>  static int pci_unin_map_irq(PCIDevice *pci_dev, int irq_num)
>  {
> @@ -180,8 +154,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
>  
>      pci_mem_config = cpu_register_io_memory(pci_unin_main_config_read,
>                                              pci_unin_main_config_write, s);
> -    pci_mem_data = cpu_register_io_memory(pci_unin_main_read,
> -                                          pci_unin_main_write, 
> &s->host_state);
> +    pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
>  
>      sysbus_init_mmio(dev, 0x1000, pci_mem_config);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_data);
> @@ -203,8 +176,7 @@ static int pci_dec_21154_init_device(SysBusDevice *dev)
>      // XXX: s = &pci_bridge[2];
>      pci_mem_config = cpu_register_io_memory(pci_unin_config_read,
>                                              pci_unin_config_write, s);
> -    pci_mem_data = cpu_register_io_memory(pci_unin_main_read,
> -                                          pci_unin_main_write, 
> &s->host_state);
> +    pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_config);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_data);
>      return 0;
> @@ -220,8 +192,7 @@ static int pci_unin_agp_init_device(SysBusDevice *dev)
>  
>      pci_mem_config = cpu_register_io_memory(pci_unin_config_read,
>                                              pci_unin_config_write, s);
> -    pci_mem_data = cpu_register_io_memory(pci_unin_main_read,
> -                                          pci_unin_main_write, 
> &s->host_state);
> +    pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_config);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_data);
>      return 0;
> @@ -237,8 +208,7 @@ static int pci_unin_internal_init_device(SysBusDevice 
> *dev)
>  
>      pci_mem_config = cpu_register_io_memory(pci_unin_config_read,
>                                              pci_unin_config_write, s);
> -    pci_mem_data = cpu_register_io_memory(pci_unin_read,
> -                                          pci_unin_write, s);
> +    pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_config);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_data);
>      return 0;
> -- 
> 1.6.0.2