diff mbox

[v3,3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.

Message ID b6635ae80ba8d946349f5befc9b5f208c01ef0c5.1418264106.git.hutao@cn.fujitsu.com
State New
Headers show

Commit Message

Hu Tao Dec. 11, 2014, 2:20 a.m. UTC
PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA are
defined in PCI specification, so move them to common place.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/pci-host/piix.c        |  8 ++++----
 hw/pci-host/prep.c        |  6 ++++--
 hw/pci-host/q35.c         |  8 ++++----
 include/hw/pci-host/q35.h |  3 ---
 include/hw/pci/pci_host.h |  5 +++++
 tests/libqos/pci-pc.c     | 25 +++++++++++++------------
 6 files changed, 30 insertions(+), 25 deletions(-)

Comments

Michael S. Tsirkin Jan. 21, 2015, 11:54 a.m. UTC | #1
On Thu, Dec 11, 2014 at 10:20:25AM +0800, Hu Tao wrote:
> PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA are
> defined in PCI specification, so move them to common place.

they are listed in the spec, but they are still PC specific.
Spec says:

	Two DWORD I/O locations are used to generate configuration transactions
---->	for PC-AT compatible systems.
	The first DWORD location (CF8h) references a read/write register
	that is named CONFIG_ADDRESS. The second DWORD address (CFCh) references a
	read/write register named CONFIG_DATA.


So this should at least have PC_ included in name, and probably go into
some pc - specific header.


> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  hw/pci-host/piix.c        |  8 ++++----
>  hw/pci-host/prep.c        |  6 ++++--
>  hw/pci-host/q35.c         |  8 ++++----
>  include/hw/pci-host/q35.h |  3 ---
>  include/hw/pci/pci_host.h |  5 +++++
>  tests/libqos/pci-pc.c     | 25 +++++++++++++------------
>  6 files changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 1530038..76f3757 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, PCI_HOST_BRIDGE_CONFIG_ADDR, &s->conf_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_CONFIG_ADDR, 4);
>  
> -    sysbus_add_io(sbd, 0xcfc, &s->data_mem);
> -    sysbus_init_ioports(sbd, 0xcfc, 4);
> +    sysbus_add_io(sbd, PCI_HOST_BRIDGE_CONFIG_DATA, &s->data_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_CONFIG_DATA, 4);
>  }
>  
>  static int i440fx_initfn(PCIDevice *dev)
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 1de3681..2ae21ad 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -228,11 +228,13 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>  
>      memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, s,
>                            "pci-conf-idx", 4);
> -    memory_region_add_subregion(&s->pci_io, 0xcf8, &h->conf_mem);
> +    memory_region_add_subregion(&s->pci_io, PCI_HOST_BRIDGE_CONFIG_ADDR,
> +                                &h->conf_mem);
>  
>      memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_le_ops, s,
>                            "pci-conf-data", 4);
> -    memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
> +    memory_region_add_subregion(&s->pci_io, PCI_HOST_BRIDGE_CONFIG_DATA,
> +                                &h->data_mem);
>  
>      memory_region_init_io(&h->mmcfg, OBJECT(s), &raven_pci_io_ops, s,
>                            "pciio", 0x00400000);
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b20bad8..666afea 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, PCI_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_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, PCI_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_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_host.h b/include/hw/pci/pci_host.h
> index b48791d..2bae45a 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -30,6 +30,11 @@
>  
>  #include "hw/sysbus.h"
>  
> +/* PCI configuration */
> +
> +#define PCI_HOST_BRIDGE_CONFIG_ADDR      0xcf8
> +#define PCI_HOST_BRIDGE_CONFIG_DATA      0xcfc
> +
>  #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge"
>  #define PCI_HOST_BRIDGE(obj) \
>      OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST_BRIDGE)
> diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
> index 6dba0db..e4c3c11 100644
> --- a/tests/libqos/pci-pc.c
> +++ b/tests/libqos/pci-pc.c
> @@ -14,6 +14,7 @@
>  #include "libqos/pci-pc.h"
>  
>  #include "hw/pci/pci_regs.h"
> +#include "hw/pci/pci_host.h"
>  
>  #include "qemu-common.h"
>  #include "qemu/host-utils.h"
> @@ -113,38 +114,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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    return inb(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    return inw(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    return inl(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    outb(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    outw(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    outl(PCI_HOST_BRIDGE_CONFIG_DATA, value);
>  }
>  
>  static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t *sizeptr)
> -- 
> 1.9.3
Michael S. Tsirkin Jan. 27, 2015, 11:18 a.m. UTC | #2
On Thu, Dec 11, 2014 at 10:20:25AM +0800, Hu Tao wrote:
> PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA are
> defined in PCI specification, so move them to common place.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>

If you read spec carefully you will see that it says
that these are in fact PC specific.
So the definitions don't belong in hw/pci/.

> ---
>  hw/pci-host/piix.c        |  8 ++++----
>  hw/pci-host/prep.c        |  6 ++++--
>  hw/pci-host/q35.c         |  8 ++++----
>  include/hw/pci-host/q35.h |  3 ---
>  include/hw/pci/pci_host.h |  5 +++++
>  tests/libqos/pci-pc.c     | 25 +++++++++++++------------
>  6 files changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 1530038..76f3757 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, PCI_HOST_BRIDGE_CONFIG_ADDR, &s->conf_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_CONFIG_ADDR, 4);
>  
> -    sysbus_add_io(sbd, 0xcfc, &s->data_mem);
> -    sysbus_init_ioports(sbd, 0xcfc, 4);
> +    sysbus_add_io(sbd, PCI_HOST_BRIDGE_CONFIG_DATA, &s->data_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_CONFIG_DATA, 4);
>  }
>  
>  static int i440fx_initfn(PCIDevice *dev)
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 1de3681..2ae21ad 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -228,11 +228,13 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>  
>      memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, s,
>                            "pci-conf-idx", 4);
> -    memory_region_add_subregion(&s->pci_io, 0xcf8, &h->conf_mem);
> +    memory_region_add_subregion(&s->pci_io, PCI_HOST_BRIDGE_CONFIG_ADDR,
> +                                &h->conf_mem);
>  
>      memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_le_ops, s,
>                            "pci-conf-data", 4);
> -    memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
> +    memory_region_add_subregion(&s->pci_io, PCI_HOST_BRIDGE_CONFIG_DATA,
> +                                &h->data_mem);
>  
>      memory_region_init_io(&h->mmcfg, OBJECT(s), &raven_pci_io_ops, s,
>                            "pciio", 0x00400000);
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b20bad8..666afea 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, PCI_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_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, PCI_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
> +    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_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_host.h b/include/hw/pci/pci_host.h
> index b48791d..2bae45a 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -30,6 +30,11 @@
>  
>  #include "hw/sysbus.h"
>  
> +/* PCI configuration */
> +
> +#define PCI_HOST_BRIDGE_CONFIG_ADDR      0xcf8
> +#define PCI_HOST_BRIDGE_CONFIG_DATA      0xcfc
> +
>  #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge"
>  #define PCI_HOST_BRIDGE(obj) \
>      OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST_BRIDGE)
> diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
> index 6dba0db..e4c3c11 100644
> --- a/tests/libqos/pci-pc.c
> +++ b/tests/libqos/pci-pc.c
> @@ -14,6 +14,7 @@
>  #include "libqos/pci-pc.h"
>  
>  #include "hw/pci/pci_regs.h"
> +#include "hw/pci/pci_host.h"
>  
>  #include "qemu-common.h"
>  #include "qemu/host-utils.h"
> @@ -113,38 +114,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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    return inb(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    return inw(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    return inl(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    outb(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    outw(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
> +    outl(PCI_HOST_BRIDGE_CONFIG_DATA, value);
>  }
>  
>  static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t *sizeptr)
> -- 
> 1.9.3
diff mbox

Patch

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 1530038..76f3757 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, PCI_HOST_BRIDGE_CONFIG_ADDR, &s->conf_mem);
+    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_CONFIG_ADDR, 4);
 
-    sysbus_add_io(sbd, 0xcfc, &s->data_mem);
-    sysbus_init_ioports(sbd, 0xcfc, 4);
+    sysbus_add_io(sbd, PCI_HOST_BRIDGE_CONFIG_DATA, &s->data_mem);
+    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_CONFIG_DATA, 4);
 }
 
 static int i440fx_initfn(PCIDevice *dev)
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 1de3681..2ae21ad 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -228,11 +228,13 @@  static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 
     memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, s,
                           "pci-conf-idx", 4);
-    memory_region_add_subregion(&s->pci_io, 0xcf8, &h->conf_mem);
+    memory_region_add_subregion(&s->pci_io, PCI_HOST_BRIDGE_CONFIG_ADDR,
+                                &h->conf_mem);
 
     memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_le_ops, s,
                           "pci-conf-data", 4);
-    memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
+    memory_region_add_subregion(&s->pci_io, PCI_HOST_BRIDGE_CONFIG_DATA,
+                                &h->data_mem);
 
     memory_region_init_io(&h->mmcfg, OBJECT(s), &raven_pci_io_ops, s,
                           "pciio", 0x00400000);
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b20bad8..666afea 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, PCI_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
+    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_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, PCI_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
+    sysbus_init_ioports(sbd, PCI_HOST_BRIDGE_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_host.h b/include/hw/pci/pci_host.h
index b48791d..2bae45a 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -30,6 +30,11 @@ 
 
 #include "hw/sysbus.h"
 
+/* PCI configuration */
+
+#define PCI_HOST_BRIDGE_CONFIG_ADDR      0xcf8
+#define PCI_HOST_BRIDGE_CONFIG_DATA      0xcfc
+
 #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge"
 #define PCI_HOST_BRIDGE(obj) \
     OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST_BRIDGE)
diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index 6dba0db..e4c3c11 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -14,6 +14,7 @@ 
 #include "libqos/pci-pc.h"
 
 #include "hw/pci/pci_regs.h"
+#include "hw/pci/pci_host.h"
 
 #include "qemu-common.h"
 #include "qemu/host-utils.h"
@@ -113,38 +114,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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
+    return inb(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
+    return inw(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
+    return inl(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
+    outb(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
+    outw(PCI_HOST_BRIDGE_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(PCI_HOST_BRIDGE_CONFIG_ADDR, (1U << 31) | (devfn << 8) | offset);
+    outl(PCI_HOST_BRIDGE_CONFIG_DATA, value);
 }
 
 static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t *sizeptr)