On Fri, Oct 30, 2009 at 09:21:07PM +0900, Isaku Yamahata wrote:
> consolidate pci_config address access into pci_host.c
>
> Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
> ---
> hw/apb_pci.c | 43 +---------------------
> hw/grackle_pci.c | 45 +---------------------
> hw/pci_host.c | 108
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/pci_host.h | 3 +
> hw/piix_pci.c | 15 +-------
> hw/ppce500_pci.c | 34 +----------------
> hw/prep_pci.c | 15 +-------
> hw/unin_pci.c | 81 ++--------------------------------------
> 8 files changed, 121 insertions(+), 223 deletions(-)
>
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index 560617a..3999879 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -54,46 +54,6 @@ typedef struct APBState {
> PCIHostState host_state;
> } APBState;
>
> -static void pci_apb_config_writel (void *opaque, target_phys_addr_t addr,
> - uint32_t val)
> -{
> - APBState *s = opaque;
> -
> -#ifdef TARGET_WORDS_BIGENDIAN
> - val = bswap32(val);
> -#endif
> - APB_DPRINTF("config_writel addr " TARGET_FMT_plx " val %x\n", addr,
> - val);
> - s->host_state.config_reg = val;
> -}
> -
> -static uint32_t pci_apb_config_readl (void *opaque,
> - target_phys_addr_t addr)
> -{
> - APBState *s = opaque;
> - uint32_t val;
> -
> - val = s->host_state.config_reg;
> -#ifdef TARGET_WORDS_BIGENDIAN
> - val = bswap32(val);
> -#endif
> - APB_DPRINTF("config_readl addr " TARGET_FMT_plx " val %x\n", addr,
> - val);
> - return val;
> -}
> -
> -static CPUWriteMemoryFunc * const pci_apb_config_write[] = {
> - &pci_apb_config_writel,
> - &pci_apb_config_writel,
> - &pci_apb_config_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_apb_config_read[] = {
> - &pci_apb_config_readl,
> - &pci_apb_config_readl,
> - &pci_apb_config_readl,
> -};
> -
> static void apb_config_writel (void *opaque, target_phys_addr_t addr,
> uint32_t val)
> {
> @@ -275,8 +235,7 @@ static int pci_pbm_init_device(SysBusDevice *dev)
> pci_apb_iowrite, s);
> sysbus_init_mmio(dev, 0x10000ULL, pci_ioport);
> /* mem_config */
> - pci_mem_config = cpu_register_io_memory(pci_apb_config_read,
> - pci_apb_config_write, s);
> + pci_mem_config = pci_host_config_register_io_memory(&s->host_state);
> sysbus_init_mmio(dev, 0x10ULL, pci_mem_config);
> /* mem_data */
> pci_mem_data = pci_host_data_register_io_memory(&s->host_state);
> diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
> index 8407cd2..58dcd11 100644
> --- a/hw/grackle_pci.c
> +++ b/hw/grackle_pci.c
> @@ -43,45 +43,6 @@ typedef struct GrackleState {
> PCIHostState host_state;
> } GrackleState;
>
> -static void pci_grackle_config_writel (void *opaque, target_phys_addr_t addr,
> - uint32_t val)
> -{
> - GrackleState *s = opaque;
> -
> - GRACKLE_DPRINTF("config_writel addr " TARGET_FMT_plx " val %x\n", addr,
> - val);
> -#ifdef TARGET_WORDS_BIGENDIAN
> - val = bswap32(val);
> -#endif
> - s->host_state.config_reg = val;
> -}
> -
> -static uint32_t pci_grackle_config_readl (void *opaque, target_phys_addr_t
> addr)
> -{
> - GrackleState *s = opaque;
> - uint32_t val;
> -
> - val = s->host_state.config_reg;
> -#ifdef TARGET_WORDS_BIGENDIAN
> - val = bswap32(val);
> -#endif
> - GRACKLE_DPRINTF("config_readl addr " TARGET_FMT_plx " val %x\n", addr,
> - val);
> - return val;
> -}
> -
> -static CPUWriteMemoryFunc * const pci_grackle_config_write[] = {
> - &pci_grackle_config_writel,
> - &pci_grackle_config_writel,
> - &pci_grackle_config_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_grackle_config_read[] = {
> - &pci_grackle_config_readl,
> - &pci_grackle_config_readl,
> - &pci_grackle_config_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)
> {
> @@ -147,8 +108,7 @@ static int pci_grackle_init_device(SysBusDevice *dev)
>
> s = FROM_SYSBUS(GrackleState, dev);
>
> - pci_mem_config = cpu_register_io_memory(pci_grackle_config_read,
> - pci_grackle_config_write, s);
> + pci_mem_config = pci_host_config_register_io_memory(&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);
> @@ -167,8 +127,7 @@ static int pci_dec_21154_init_device(SysBusDevice *dev)
>
> s = FROM_SYSBUS(GrackleState, dev);
>
> - pci_mem_config = cpu_register_io_memory(pci_grackle_config_read,
> - pci_grackle_config_write, s);
> + pci_mem_config = pci_host_config_register_io_memory(&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);
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 45da1e7..6009e37 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); }
> while (0)
> #define PCI_DPRINTF(fmt, ...)
> #endif
>
> +static void pci_host_config_writel(void *opaque, target_phys_addr_t addr,
> + uint32_t val)
> +{
> + PCIHostState *s = opaque;
> +
> +#ifdef TARGET_WORDS_BIGENDIAN
> + val = bswap32(val);
> +#endif
I know you just copied it, but isn't this just
val = le32_to_cpu(val);
?
> + PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
> + __func__, addr, val);
> + s->config_reg = val;
> +}
> +
> +static uint32_t pci_host_config_readl(void *opaque, target_phys_addr_t addr)
> +{
> + PCIHostState *s = opaque;
> + uint32_t val = s->config_reg;
> +
> +#ifdef TARGET_WORDS_BIGENDIAN
> + val = bswap32(val);
> +#endif
same here.
> + PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
> + __func__, addr, val);
> + return val;
> +}
> +
> +static CPUWriteMemoryFunc * const pci_host_config_write[] = {
> + &pci_host_config_writel,
> + &pci_host_config_writel,
> + &pci_host_config_writel,
> +};
> +
> +static CPUReadMemoryFunc * const pci_host_config_read[] = {
> + &pci_host_config_readl,
> + &pci_host_config_readl,
> + &pci_host_config_readl,
> +};
> +
> +int pci_host_config_register_io_memory(PCIHostState *s)
> +{
> + return cpu_register_io_memory(pci_host_config_read,
> + pci_host_config_write, s);
> +}
> +
> +static void pci_host_config_writel_noswap(void *opaque,
> + target_phys_addr_t addr,
> + uint32_t val)
> +{
> + PCIHostState *s = opaque;
> +
> + PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
> + __func__, addr, val);
> + s->config_reg = val;
> +}
> +
> +static uint32_t pci_host_config_readl_noswap(void *opaque,
> + target_phys_addr_t addr)
> +{
> + PCIHostState *s = opaque;
> + uint32_t val = s->config_reg;
> +
> + PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
> + __func__, addr, val);
> + return val;
> +}
> +
> +static CPUWriteMemoryFunc * const pci_host_config_write_noswap[] = {
> + &pci_host_config_writel_noswap,
> + &pci_host_config_writel_noswap,
> + &pci_host_config_writel_noswap,
> +};
> +
> +static CPUReadMemoryFunc * const pci_host_config_read_noswap[] = {
> + &pci_host_config_readl_noswap,
> + &pci_host_config_readl_noswap,
> + &pci_host_config_readl_noswap,
> +};
> +
> +int pci_host_config_register_io_memory_noswap(PCIHostState *s)
This name is clearly too long,
as a result when you use it you run over the 80 character
limit. Let's not fix it, let's make name shorter.
io_memory -> memory?
> +{
> + return cpu_register_io_memory(pci_host_config_read_noswap,
> + pci_host_config_write_noswap, s);
> +}
Do you happen to know whether no swap is really needed? I suspect some
of these places really only happen to work because everyone uses intel,
so they simply would not work on ppc host.
> +
> +static void pci_host_config_writel_ioport(void *opaque,
> + uint32_t addr, uint32_t val)
> +{
> + PCIHostState *s = opaque;
> +
> + PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr, val);
> + s->config_reg = val;
> +}
> +
> +static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t addr)
> +{
> + PCIHostState *s = opaque;
> + uint32_t val = s->config_reg;
> +
> + PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr, val);
> + return val;
> +}
> +
> +void pci_host_config_register_ioport(pio_addr_t ioport, PCIHostState *s)
> +{
> + register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport, s);
> + register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
> +}
> +
> #define PCI_ADDR_T target_phys_addr_t
> #define PCI_HOST_SUFFIX _mmio
>
> diff --git a/hw/pci_host.h b/hw/pci_host.h
> index 92a35f9..e5e877f 100644
> --- a/hw/pci_host.h
> +++ b/hw/pci_host.h
> @@ -37,9 +37,12 @@ typedef struct {
> } PCIHostState;
>
> /* for mmio */
> +int pci_host_config_register_io_memory(PCIHostState *s);
> +int pci_host_config_register_io_memory_noswap(PCIHostState *s);
> int pci_host_data_register_io_memory(PCIHostState *s);
>
> /* for ioio */
> +void pci_host_config_register_ioport(pio_addr_t ioport, PCIHostState *s);
> void pci_host_data_register_ioport(pio_addr_t ioport, PCIHostState *s);
>
> #endif /* PCI_HOST_H */
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 866348d..bf0005e 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -44,18 +44,6 @@ struct PCII440FXState {
> PIIX3State *piix3;
> };
>
> -static void i440fx_addr_writel(void* opaque, uint32_t addr, uint32_t val)
> -{
> - I440FXState *s = opaque;
> - s->config_reg = val;
> -}
> -
> -static uint32_t i440fx_addr_readl(void* opaque, uint32_t addr)
> -{
> - I440FXState *s = opaque;
> - return s->config_reg;
> -}
> -
> static void piix3_set_irq(void *opaque, int irq_num, int level);
>
> /* return the global irq number corresponding to a given device irq
> @@ -192,8 +180,7 @@ static int i440fx_pcihost_initfn(SysBusDevice *dev)
> {
> I440FXState *s = FROM_SYSBUS(I440FXState, dev);
>
> - register_ioport_write(0xcf8, 4, 4, i440fx_addr_writel, s);
> - register_ioport_read(0xcf8, 4, 4, i440fx_addr_readl, s);
> + pci_host_config_register_ioport(0xcf8, s);
>
> pci_host_data_register_ioport(0xcfc, s);
> return 0;
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 7c8cdad..223de3a 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -84,37 +84,6 @@ struct PPCE500PCIState {
>
> typedef struct PPCE500PCIState PPCE500PCIState;
>
> -static uint32_t pcie500_cfgaddr_readl(void *opaque, target_phys_addr_t addr)
> -{
> - PPCE500PCIState *pci = opaque;
> -
> - pci_debug("%s: (addr:" TARGET_FMT_plx ") -> value:%x\n", __func__, addr,
> - pci->pci_state.config_reg);
> - return pci->pci_state.config_reg;
> -}
> -
> -static CPUReadMemoryFunc * const pcie500_cfgaddr_read[] = {
> - &pcie500_cfgaddr_readl,
> - &pcie500_cfgaddr_readl,
> - &pcie500_cfgaddr_readl,
> -};
> -
> -static void pcie500_cfgaddr_writel(void *opaque, target_phys_addr_t addr,
> - uint32_t value)
> -{
> - PPCE500PCIState *controller = opaque;
> -
> - pci_debug("%s: value:%x -> (addr:" TARGET_FMT_plx ")\n", __func__, value,
> - addr);
> - controller->pci_state.config_reg = value & ~0x3;
> -}
> -
> -static CPUWriteMemoryFunc * const pcie500_cfgaddr_write[] = {
> - &pcie500_cfgaddr_writel,
> - &pcie500_cfgaddr_writel,
> - &pcie500_cfgaddr_writel,
> -};
> -
> static uint32_t pci_reg_read4(void *opaque, target_phys_addr_t addr)
> {
> PPCE500PCIState *pci = opaque;
> @@ -324,8 +293,7 @@ PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4],
> target_phys_addr_t registers)
> controller->pci_dev = d;
>
> /* CFGADDR */
> - index = cpu_register_io_memory(pcie500_cfgaddr_read,
> - pcie500_cfgaddr_write, controller);
> + index =
> pci_host_config_register_io_memory_noswap(&controller->pci_state);
> if (index < 0)
> goto free;
> cpu_register_physical_memory(registers + PCIE500_CFGADDR, 4, index);
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index 5a5b3da..a338f81 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -28,18 +28,6 @@
>
> typedef PCIHostState PREPPCIState;
>
> -static void pci_prep_addr_writel(void* opaque, uint32_t addr, uint32_t val)
> -{
> - PREPPCIState *s = opaque;
> - s->config_reg = val;
> -}
> -
> -static uint32_t pci_prep_addr_readl(void* opaque, uint32_t addr)
> -{
> - PREPPCIState *s = opaque;
> - return s->config_reg;
> -}
> -
> static inline uint32_t PPC_PCIIO_config(target_phys_addr_t addr)
> {
> int i;
> @@ -139,8 +127,7 @@ PCIBus *pci_prep_init(qemu_irq *pic)
> s->bus = pci_register_bus(NULL, "pci",
> prep_set_irq, prep_map_irq, pic, 0, 4);
>
> - register_ioport_write(0xcf8, 4, 4, pci_prep_addr_writel, s);
> - register_ioport_read(0xcf8, 4, 4, pci_prep_addr_readl, s);
> + pci_host_config_register_ioport(0xcf8, s);
>
> pci_host_data_register_ioport(0xcfc, s);
>
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index 6b8f98b..a9a62fd 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -41,74 +41,6 @@ typedef struct UNINState {
> PCIHostState host_state;
> } UNINState;
>
> -static void pci_unin_main_config_writel (void *opaque, target_phys_addr_t
> addr,
> - uint32_t val)
> -{
> - UNINState *s = opaque;
> -
> - UNIN_DPRINTF("config_writel addr " TARGET_FMT_plx " val %x\n", addr,
> val);
> -#ifdef TARGET_WORDS_BIGENDIAN
> - val = bswap32(val);
> -#endif
> -
> - s->host_state.config_reg = val;
> -}
> -
> -static uint32_t pci_unin_main_config_readl (void *opaque,
> - target_phys_addr_t addr)
> -{
> - UNINState *s = opaque;
> - uint32_t val;
> -
> - val = s->host_state.config_reg;
> -#ifdef TARGET_WORDS_BIGENDIAN
> - val = bswap32(val);
> -#endif
> - UNIN_DPRINTF("config_readl addr " TARGET_FMT_plx " val %x\n", addr, val);
> -
> - return val;
> -}
> -
> -static CPUWriteMemoryFunc * const pci_unin_main_config_write[] = {
> - &pci_unin_main_config_writel,
> - &pci_unin_main_config_writel,
> - &pci_unin_main_config_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_unin_main_config_read[] = {
> - &pci_unin_main_config_readl,
> - &pci_unin_main_config_readl,
> - &pci_unin_main_config_readl,
> -};
> -
> -static void pci_unin_config_writel (void *opaque, target_phys_addr_t addr,
> - uint32_t val)
> -{
> - UNINState *s = opaque;
> -
> - s->host_state.config_reg = val;
> -}
> -
> -static uint32_t pci_unin_config_readl (void *opaque,
> - target_phys_addr_t addr)
> -{
> - UNINState *s = opaque;
> -
> - return s->host_state.config_reg;
> -}
> -
> -static CPUWriteMemoryFunc * const pci_unin_config_write[] = {
> - &pci_unin_config_writel,
> - &pci_unin_config_writel,
> - &pci_unin_config_writel,
> -};
> -
> -static CPUReadMemoryFunc * const pci_unin_config_read[] = {
> - &pci_unin_config_readl,
> - &pci_unin_config_readl,
> - &pci_unin_config_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)
> {
> @@ -152,10 +84,8 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
> /* Uninorth main bus */
> s = FROM_SYSBUS(UNINState, dev);
>
> - pci_mem_config = cpu_register_io_memory(pci_unin_main_config_read,
> - pci_unin_main_config_write, s);
> + pci_mem_config = pci_host_config_register_io_memory(&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);
>
> @@ -174,8 +104,7 @@ static int pci_dec_21154_init_device(SysBusDevice *dev)
> s = FROM_SYSBUS(UNINState, dev);
>
> // XXX: s = &pci_bridge[2];
> - pci_mem_config = cpu_register_io_memory(pci_unin_config_read,
> - pci_unin_config_write, s);
> + pci_mem_config =
> pci_host_config_register_io_memory_noswap(&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);
> @@ -190,8 +119,7 @@ static int pci_unin_agp_init_device(SysBusDevice *dev)
> /* Uninorth AGP bus */
> s = FROM_SYSBUS(UNINState, dev);
>
> - pci_mem_config = cpu_register_io_memory(pci_unin_config_read,
> - pci_unin_config_write, s);
> + pci_mem_config =
> pci_host_config_register_io_memory_noswap(&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);
> @@ -206,8 +134,7 @@ static int pci_unin_internal_init_device(SysBusDevice
> *dev)
> /* Uninorth internal bus */
> s = FROM_SYSBUS(UNINState, dev);
>
> - pci_mem_config = cpu_register_io_memory(pci_unin_config_read,
> - pci_unin_config_write, s);
> + pci_mem_config =
> pci_host_config_register_io_memory_noswap(&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);
> --
> 1.6.0.2
