On Tue, 2014-11-04 at 17:12 +0800, Hu Tao wrote:
> PC_PCI_CONFIG_ADDR and PC_PCI_CONFIG_DATA are defined in PCI
> specification, so move them to common place.
>
> Signed-off-by: Hu Tao <hu...@cn.fujitsu.com>
> ---
> hw/pci-host/piix.c | 8 ++++----
> hw/pci-host/q35.c | 8 ++++----
> include/hw/pci-host/q35.h | 3 ---
> include/hw/pci/pci.h | 5 +++++
> tests/libqos/pci-pc.c | 24 ++++++++++++------------
> 5 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 1530038..eb92bde 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -288,11 +288,11 @@ static void i440fx_pcihost_realize(DeviceState *dev,
> Error **errp)
> PCIHostState *s = PCI_HOST_BRIDGE(dev);
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>
> - sysbus_add_io(sbd, 0xcf8, &s->conf_mem);
> - sysbus_init_ioports(sbd, 0xcf8, 4);
> + sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &s->conf_mem);
> + sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4);
>
> - sysbus_add_io(sbd, 0xcfc, &s->data_mem);
> - sysbus_init_ioports(sbd, 0xcfc, 4);
> + sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &s->data_mem);
> + sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4);
> }
>
> static int i440fx_initfn(PCIDevice *dev)
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b20bad8..9e66835 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -41,11 +41,11 @@ static void q35_host_realize(DeviceState *dev, Error
> **errp)
> Q35PCIHost *s = Q35_HOST_DEVICE(dev);
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>
> - sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
> - sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
> + sysbus_add_io(sbd, PC_PCI_CONFIG_ADDR, &pci->conf_mem);
> + sysbus_init_ioports(sbd, PC_PCI_CONFIG_ADDR, 4);
>
> - sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
> - sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
> + sysbus_add_io(sbd, PC_PCI_CONFIG_DATA, &pci->data_mem);
> + sysbus_init_ioports(sbd, PC_PCI_CONFIG_DATA, 4);
>
> pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
> s->mch.pci_address_space, s->mch.address_space_io,
> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
> index 025d6e6..3a026b0 100644
> --- a/include/hw/pci-host/q35.h
> +++ b/include/hw/pci-host/q35.h
> @@ -82,9 +82,6 @@ typedef struct Q35PCIHost {
> /* PCI configuration */
> #define MCH_HOST_BRIDGE "MCH"
>
> -#define MCH_HOST_BRIDGE_CONFIG_ADDR 0xcf8
> -#define MCH_HOST_BRIDGE_CONFIG_DATA 0xcfc
> -
> /* D0:F0 configuration space */
> #define MCH_HOST_BRIDGE_REVISION_DEFAULT 0x0
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 3d42d7f..e42589a 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -13,6 +13,11 @@
>
> #include "hw/pci/pcie.h"
>
> +/* PCI configuration */
> +
> +#define PC_PCI_CONFIG_ADDR 0xcf8
> +#define PC_PCI_CONFIG_DATA 0xcfc
I would move the macros also to hw/pci/pci_host.h,
and only personal opinion, change them to
PCI_HOST_BRIDGE_...
Thanks,
Marcel
> +
> #define PC_PCI_CONFIG_ENABLED(addr) (addr & (1U << 31))
>
> /* PCI bus */
> diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
> index 6dba0db..2762608 100644
> --- a/tests/libqos/pci-pc.c
> +++ b/tests/libqos/pci-pc.c
> @@ -113,38 +113,38 @@ static void qpci_pc_io_writel(QPCIBus *bus, void *addr,
> uint32_t value)
>
> static uint8_t qpci_pc_config_readb(QPCIBus *bus, int devfn, uint8_t offset)
> {
> - outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
> - return inb(0xcfc);
> + outl(PC_PCI_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> + return inb(PC_PCI_CONFIG_DATA);
> }
>
> static uint16_t qpci_pc_config_readw(QPCIBus *bus, int devfn, uint8_t offset)
> {
> - outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
> - return inw(0xcfc);
> + outl(PC_PCI_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> + return inw(PC_PCI_CONFIG_DATA);
> }
>
> static uint32_t qpci_pc_config_readl(QPCIBus *bus, int devfn, uint8_t offset)
> {
> - outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
> - return inl(0xcfc);
> + outl(PC_PCI_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> + return inl(PC_PCI_CONFIG_DATA);
> }
>
> static void qpci_pc_config_writeb(QPCIBus *bus, int devfn, uint8_t offset,
> uint8_t value)
> {
> - outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
> - outb(0xcfc, value);
> + outl(PC_PCI_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> + outb(PC_PCI_CONFIG_DATA, value);
> }
>
> static void qpci_pc_config_writew(QPCIBus *bus, int devfn, uint8_t offset,
> uint16_t value)
> {
> - outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
> - outw(0xcfc, value);
> + outl(PC_PCI_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> + outw(PC_PCI_CONFIG_DATA, value);
> }
>
> static void qpci_pc_config_writel(QPCIBus *bus, int devfn, uint8_t offset,
> uint32_t value)
> {
> - outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
> - outl(0xcfc, value);
> + outl(PC_PCI_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> + outl(PC_PCI_CONFIG_DATA, value);
> }
>
> static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno,
> uint64_t *sizeptr)
