* [Qemu-devel] [PATCH v3 1/6] pci: reorganize QEMU_PCI_CAP_*
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
@ 2014-12-11 2:20 ` Hu Tao
2015-01-21 11:48 ` Michael S. Tsirkin
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled() Hu Tao
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Marcel Apfelbaum, Michael S. Tsirkin
This makes code more readable.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
include/hw/pci/pci.h | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index c352c7b..b18759a 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -142,25 +142,26 @@ enum {
/* Bits in cap_present field. */
enum {
- QEMU_PCI_CAP_MSI = 0x1,
- QEMU_PCI_CAP_MSIX = 0x2,
- QEMU_PCI_CAP_EXPRESS = 0x4,
-
- /* multifunction capable device */
-#define QEMU_PCI_CAP_MULTIFUNCTION_BITNR 3
- QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR),
-
- /* command register SERR bit enabled */
-#define QEMU_PCI_CAP_SERR_BITNR 4
- QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
- /* Standard hot plug controller. */
-#define QEMU_PCI_SHPC_BITNR 5
- QEMU_PCI_CAP_SHPC = (1 << QEMU_PCI_SHPC_BITNR),
-#define QEMU_PCI_SLOTID_BITNR 6
- QEMU_PCI_CAP_SLOTID = (1 << QEMU_PCI_SLOTID_BITNR),
- /* PCI Express capability - Power Controller Present */
-#define QEMU_PCIE_SLTCAP_PCP_BITNR 7
- QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
+ QEMU_PCI_CAP_MSI_BITNR = 0,
+ QEMU_PCI_CAP_MSIX_BITNR,
+ QEMU_PCI_CAP_EXPRESS_BITNR,
+ QEMU_PCI_CAP_MULTIFUNCTION_BITNR, /* multifunction capable device */
+ QEMU_PCI_CAP_SERR_BITNR, /* command register SERR bit enabled */
+ QEMU_PCI_SHPC_BITNR, /* Standard hot plug controller */
+ QEMU_PCI_SLOTID_BITNR,
+ QEMU_PCIE_SLTCAP_PCP_BITNR, /* PCI Express capability - Power Controller
+ Present */
+};
+
+enum {
+ QEMU_PCI_CAP_MSI = (1 << QEMU_PCI_CAP_MSI_BITNR),
+ QEMU_PCI_CAP_MSIX = (1 << QEMU_PCI_CAP_MSIX_BITNR),
+ QEMU_PCI_CAP_EXPRESS = (1 << QEMU_PCI_CAP_EXPRESS_BITNR),
+ QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR),
+ QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
+ QEMU_PCI_CAP_SHPC = (1 << QEMU_PCI_SHPC_BITNR),
+ QEMU_PCI_CAP_SLOTID = (1 << QEMU_PCI_SLOTID_BITNR),
+ QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
};
#define TYPE_PCI_DEVICE "pci-device"
--
1.9.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/6] pci: reorganize QEMU_PCI_CAP_*
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 1/6] pci: reorganize QEMU_PCI_CAP_* Hu Tao
@ 2015-01-21 11:48 ` Michael S. Tsirkin
0 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-21 11:48 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Thu, Dec 11, 2014 at 10:20:23AM +0800, Hu Tao wrote:
> This makes code more readable.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> include/hw/pci/pci.h | 39 ++++++++++++++++++++-------------------
> 1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index c352c7b..b18759a 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -142,25 +142,26 @@ enum {
>
> /* Bits in cap_present field. */
> enum {
> - QEMU_PCI_CAP_MSI = 0x1,
> - QEMU_PCI_CAP_MSIX = 0x2,
> - QEMU_PCI_CAP_EXPRESS = 0x4,
> -
> - /* multifunction capable device */
> -#define QEMU_PCI_CAP_MULTIFUNCTION_BITNR 3
> - QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR),
> -
> - /* command register SERR bit enabled */
> -#define QEMU_PCI_CAP_SERR_BITNR 4
> - QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
> - /* Standard hot plug controller. */
> -#define QEMU_PCI_SHPC_BITNR 5
> - QEMU_PCI_CAP_SHPC = (1 << QEMU_PCI_SHPC_BITNR),
> -#define QEMU_PCI_SLOTID_BITNR 6
> - QEMU_PCI_CAP_SLOTID = (1 << QEMU_PCI_SLOTID_BITNR),
> - /* PCI Express capability - Power Controller Present */
> -#define QEMU_PCIE_SLTCAP_PCP_BITNR 7
> - QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
> + QEMU_PCI_CAP_MSI_BITNR = 0,
> + QEMU_PCI_CAP_MSIX_BITNR,
> + QEMU_PCI_CAP_EXPRESS_BITNR,
> + QEMU_PCI_CAP_MULTIFUNCTION_BITNR, /* multifunction capable device */
> + QEMU_PCI_CAP_SERR_BITNR, /* command register SERR bit enabled */
> + QEMU_PCI_SHPC_BITNR, /* Standard hot plug controller */
> + QEMU_PCI_SLOTID_BITNR,
> + QEMU_PCIE_SLTCAP_PCP_BITNR, /* PCIE Slot - Power Controller
>+ Present */
Pls shorten so it fits on one line: /* PCIE Slot - Power Controller Present */
> +};
> +
> +enum {
> + QEMU_PCI_CAP_MSI = (1 << QEMU_PCI_CAP_MSI_BITNR),
> + QEMU_PCI_CAP_MSIX = (1 << QEMU_PCI_CAP_MSIX_BITNR),
> + QEMU_PCI_CAP_EXPRESS = (1 << QEMU_PCI_CAP_EXPRESS_BITNR),
> + QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR),
> + QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
> + QEMU_PCI_CAP_SHPC = (1 << QEMU_PCI_SHPC_BITNR),
> + QEMU_PCI_CAP_SLOTID = (1 << QEMU_PCI_SLOTID_BITNR),
> + QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
> };
>
> #define TYPE_PCI_DEVICE "pci-device"
> --
> 1.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled()
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 1/6] pci: reorganize QEMU_PCI_CAP_* Hu Tao
@ 2014-12-11 2:20 ` Hu Tao
2015-01-21 11:50 ` Michael S. Tsirkin
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA Hu Tao
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Marcel Apfelbaum, Michael S. Tsirkin
This makes code more readable.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
hw/mips/gt64xxx_pci.c | 4 ++--
hw/pci/pci_host.c | 5 +++--
include/hw/pci/pci_host.h | 5 +++++
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 1f2fe5f..f118c9c 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -564,7 +564,7 @@ static void gt64120_writel (void *opaque, hwaddr addr,
if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) {
val = bswap32(val);
}
- if (phb->config_reg & (1u << 31)) {
+ if (pci_host_config_enabled(phb)) {
pci_data_write(phb->bus, phb->config_reg, val, 4);
}
break;
@@ -804,7 +804,7 @@ static uint64_t gt64120_readl (void *opaque,
val = phb->config_reg;
break;
case GT_PCI0_CFGDATA:
- if (!(phb->config_reg & (1 << 31))) {
+ if (!pci_host_config_enabled(phb)) {
val = 0xffffffff;
} else {
val = pci_data_read(phb->bus, phb->config_reg, 4);
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 3e26f92..9bc47d8 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -133,8 +133,9 @@ static void pci_host_data_write(void *opaque, hwaddr addr,
PCIHostState *s = opaque;
PCI_DPRINTF("write addr " TARGET_FMT_plx " len %d val %x\n",
addr, len, (unsigned)val);
- if (s->config_reg & (1u << 31))
+ if (pci_host_config_enabled(s)) {
pci_data_write(s->bus, s->config_reg | (addr & 3), val, len);
+ }
}
static uint64_t pci_host_data_read(void *opaque,
@@ -142,7 +143,7 @@ static uint64_t pci_host_data_read(void *opaque,
{
PCIHostState *s = opaque;
uint32_t val;
- if (!(s->config_reg & (1U << 31))) {
+ if (!pci_host_config_enabled(s)) {
return 0xffffffff;
}
val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index ba31595..b48791d 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -65,6 +65,11 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
+static inline bool pci_host_config_enabled(struct PCIHostState *pci_host)
+{
+ return pci_host->config_reg & (1U << 31);
+}
+
extern const MemoryRegionOps pci_host_conf_le_ops;
extern const MemoryRegionOps pci_host_conf_be_ops;
extern const MemoryRegionOps pci_host_data_le_ops;
--
1.9.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled()
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled() Hu Tao
@ 2015-01-21 11:50 ` Michael S. Tsirkin
0 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-21 11:50 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Thu, Dec 11, 2014 at 10:20:24AM +0800, Hu Tao wrote:
> This makes code more readable.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> hw/mips/gt64xxx_pci.c | 4 ++--
> hw/pci/pci_host.c | 5 +++--
> include/hw/pci/pci_host.h | 5 +++++
> 3 files changed, 10 insertions(+), 4 deletions(-)
We have a ton of other places hard-coding 1<<31,
why special-case these?
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 1f2fe5f..f118c9c 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -564,7 +564,7 @@ static void gt64120_writel (void *opaque, hwaddr addr,
> if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) {
> val = bswap32(val);
> }
> - if (phb->config_reg & (1u << 31)) {
> + if (pci_host_config_enabled(phb)) {
> pci_data_write(phb->bus, phb->config_reg, val, 4);
> }
> break;
> @@ -804,7 +804,7 @@ static uint64_t gt64120_readl (void *opaque,
> val = phb->config_reg;
> break;
> case GT_PCI0_CFGDATA:
> - if (!(phb->config_reg & (1 << 31))) {
> + if (!pci_host_config_enabled(phb)) {
> val = 0xffffffff;
> } else {
> val = pci_data_read(phb->bus, phb->config_reg, 4);
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 3e26f92..9bc47d8 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -133,8 +133,9 @@ static void pci_host_data_write(void *opaque, hwaddr addr,
> PCIHostState *s = opaque;
> PCI_DPRINTF("write addr " TARGET_FMT_plx " len %d val %x\n",
> addr, len, (unsigned)val);
> - if (s->config_reg & (1u << 31))
> + if (pci_host_config_enabled(s)) {
> pci_data_write(s->bus, s->config_reg | (addr & 3), val, len);
> + }
> }
>
> static uint64_t pci_host_data_read(void *opaque,
> @@ -142,7 +143,7 @@ static uint64_t pci_host_data_read(void *opaque,
> {
> PCIHostState *s = opaque;
> uint32_t val;
> - if (!(s->config_reg & (1U << 31))) {
> + if (!pci_host_config_enabled(s)) {
> return 0xffffffff;
> }
> val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> index ba31595..b48791d 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -65,6 +65,11 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
> void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
>
> +static inline bool pci_host_config_enabled(struct PCIHostState *pci_host)
> +{
> + return pci_host->config_reg & (1U << 31);
> +}
> +
Better:
#define PCI_HOST_CONFIG_ENABLE (1U << 31)
then everyone can just do s->config_reg & PCI_HOST_CONFIG_ENABLE
better as it'll work for code like this:
phb->config_reg = (pciaddr) | (1u << 31);
> extern const MemoryRegionOps pci_host_conf_le_ops;
> extern const MemoryRegionOps pci_host_conf_be_ops;
> extern const MemoryRegionOps pci_host_data_le_ops;
> --
> 1.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v3 3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 1/6] pci: reorganize QEMU_PCI_CAP_* Hu Tao
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled() Hu Tao
@ 2014-12-11 2:20 ` Hu Tao
2015-01-21 11:54 ` Michael S. Tsirkin
2015-01-27 11:18 ` Michael S. Tsirkin
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 4/6] pci: remove the limit parameter of pci_host_config_read_common Hu Tao
` (4 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Marcel Apfelbaum, Michael S. Tsirkin
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(-)
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA Hu Tao
@ 2015-01-21 11:54 ` Michael S. Tsirkin
2015-01-27 11:18 ` Michael S. Tsirkin
1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-21 11:54 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA Hu Tao
2015-01-21 11:54 ` Michael S. Tsirkin
@ 2015-01-27 11:18 ` Michael S. Tsirkin
1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-27 11:18 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v3 4/6] pci: remove the limit parameter of pci_host_config_read_common
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
` (2 preceding siblings ...)
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 3/6] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA Hu Tao
@ 2014-12-11 2:20 ` Hu Tao
2015-01-21 11:57 ` Michael S. Tsirkin
2015-01-27 11:20 ` Michael S. Tsirkin
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 5/6] pci: remove the limit parameter of pci_host_config_write_common Hu Tao
` (3 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Marcel Apfelbaum, Michael S. Tsirkin
Since the limit parameter is always set to the size of pci device's
configuration space, and we can determine the size from the type of pci
device.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
hw/pci/pci_host.c | 15 +++++++++++----
hw/pci/pcie_host.c | 9 +--------
hw/ppc/spapr_pci.c | 3 +--
include/hw/pci/pci_host.h | 2 +-
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 9bc47d8..2b11551 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -58,12 +58,20 @@ void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
}
uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
- uint32_t limit, uint32_t len)
+ uint32_t len)
{
+ uint32_t limit = pci_config_size(pci_dev);
uint32_t ret;
assert(len <= 4);
- ret = pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr));
+
+ if (limit <= addr) {
+ /* conventional pci device can be behind pcie-to-pci bridge.
+ 256 <= addr < 4K has no effects. */
+ ret = ~0x0;
+ } else {
+ ret = pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr));
+ }
trace_pci_cfg_read(pci_dev->name, PCI_SLOT(pci_dev->devfn),
PCI_FUNC(pci_dev->devfn), addr, ret);
@@ -95,8 +103,7 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
return ~0x0;
}
- val = pci_host_config_read_common(pci_dev, config_addr,
- PCI_CONFIG_SPACE_SIZE, len);
+ val = pci_host_config_read_common(pci_dev, config_addr, len);
PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
__func__, pci_dev->name, config_addr, val, len);
diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
index 3db038f..cf8587b 100644
--- a/hw/pci/pcie_host.c
+++ b/hw/pci/pcie_host.c
@@ -62,19 +62,12 @@ static uint64_t pcie_mmcfg_data_read(void *opaque,
PCIBus *s = e->pci.bus;
PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
uint32_t addr;
- uint32_t limit;
if (!pci_dev) {
return ~0x0;
}
addr = PCIE_MMCFG_CONFOFFSET(mmcfg_addr);
- limit = pci_config_size(pci_dev);
- if (limit <= addr) {
- /* conventional pci device can be behind pcie-to-pci bridge.
- 256 <= addr < 4K has no effects. */
- return ~0x0;
- }
- return pci_host_config_read_common(pci_dev, addr, limit, len);
+ return pci_host_config_read_common(pci_dev, addr, len);
}
static const MemoryRegionOps pcie_mmcfg_ops = {
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 21b95b3..59c6608 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -105,8 +105,7 @@ static void finish_read_pci_config(sPAPREnvironment *spapr, uint64_t buid,
return;
}
- val = pci_host_config_read_common(pci_dev, addr,
- pci_config_size(pci_dev), size);
+ val = pci_host_config_read_common(pci_dev, addr, size);
rtas_st(rets, 0, RTAS_OUT_SUCCESS);
rtas_st(rets, 1, val);
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index 2bae45a..72a1b8b 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -65,7 +65,7 @@ typedef struct PCIHostBridgeClass {
void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
uint32_t limit, uint32_t val, uint32_t len);
uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
- uint32_t limit, uint32_t len);
+ uint32_t len);
void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
--
1.9.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/6] pci: remove the limit parameter of pci_host_config_read_common
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 4/6] pci: remove the limit parameter of pci_host_config_read_common Hu Tao
@ 2015-01-21 11:57 ` Michael S. Tsirkin
2015-01-27 11:20 ` Michael S. Tsirkin
1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-21 11:57 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Thu, Dec 11, 2014 at 10:20:26AM +0800, Hu Tao wrote:
> Since the limit parameter is always set to the size of pci device's
> configuration space, and we can determine the size from the type of pci
> device.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Not true e.g. for pci_data_read, is it?
Need a bit more comments to explain why it's
correct there.
> ---
> hw/pci/pci_host.c | 15 +++++++++++----
> hw/pci/pcie_host.c | 9 +--------
> hw/ppc/spapr_pci.c | 3 +--
> include/hw/pci/pci_host.h | 2 +-
> 4 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 9bc47d8..2b11551 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -58,12 +58,20 @@ void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
> }
>
> uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
> - uint32_t limit, uint32_t len)
> + uint32_t len)
> {
> + uint32_t limit = pci_config_size(pci_dev);
> uint32_t ret;
>
> assert(len <= 4);
> - ret = pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr));
> +
> + if (limit <= addr) {
> + /* conventional pci device can be behind pcie-to-pci bridge.
> + 256 <= addr < 4K has no effects. */
> + ret = ~0x0;
> + } else {
> + ret = pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr));
> + }
> trace_pci_cfg_read(pci_dev->name, PCI_SLOT(pci_dev->devfn),
> PCI_FUNC(pci_dev->devfn), addr, ret);
>
> @@ -95,8 +103,7 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
> return ~0x0;
> }
>
> - val = pci_host_config_read_common(pci_dev, config_addr,
> - PCI_CONFIG_SPACE_SIZE, len);
> + val = pci_host_config_read_common(pci_dev, config_addr, len);
> PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
> __func__, pci_dev->name, config_addr, val, len);
>
> diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
> index 3db038f..cf8587b 100644
> --- a/hw/pci/pcie_host.c
> +++ b/hw/pci/pcie_host.c
> @@ -62,19 +62,12 @@ static uint64_t pcie_mmcfg_data_read(void *opaque,
> PCIBus *s = e->pci.bus;
> PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
> uint32_t addr;
> - uint32_t limit;
>
> if (!pci_dev) {
> return ~0x0;
> }
> addr = PCIE_MMCFG_CONFOFFSET(mmcfg_addr);
> - limit = pci_config_size(pci_dev);
> - if (limit <= addr) {
> - /* conventional pci device can be behind pcie-to-pci bridge.
> - 256 <= addr < 4K has no effects. */
> - return ~0x0;
> - }
> - return pci_host_config_read_common(pci_dev, addr, limit, len);
> + return pci_host_config_read_common(pci_dev, addr, len);
> }
>
> static const MemoryRegionOps pcie_mmcfg_ops = {
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 21b95b3..59c6608 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -105,8 +105,7 @@ static void finish_read_pci_config(sPAPREnvironment *spapr, uint64_t buid,
> return;
> }
>
> - val = pci_host_config_read_common(pci_dev, addr,
> - pci_config_size(pci_dev), size);
> + val = pci_host_config_read_common(pci_dev, addr, size);
>
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> rtas_st(rets, 1, val);
> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> index 2bae45a..72a1b8b 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -65,7 +65,7 @@ typedef struct PCIHostBridgeClass {
> void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
> uint32_t limit, uint32_t val, uint32_t len);
> uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
> - uint32_t limit, uint32_t len);
> + uint32_t len);
>
> void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
> --
> 1.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/6] pci: remove the limit parameter of pci_host_config_read_common
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 4/6] pci: remove the limit parameter of pci_host_config_read_common Hu Tao
2015-01-21 11:57 ` Michael S. Tsirkin
@ 2015-01-27 11:20 ` Michael S. Tsirkin
1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-27 11:20 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Thu, Dec 11, 2014 at 10:20:26AM +0800, Hu Tao wrote:
> Since the limit parameter is always set to the size of pci device's
> configuration space,
That's not true in case of pci_data_read, is it?
While the patch might be correct, a better analysis
why that's so seems called for.
> and we can determine the size from the type of pci
> device.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> hw/pci/pci_host.c | 15 +++++++++++----
> hw/pci/pcie_host.c | 9 +--------
> hw/ppc/spapr_pci.c | 3 +--
> include/hw/pci/pci_host.h | 2 +-
> 4 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 9bc47d8..2b11551 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -58,12 +58,20 @@ void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
> }
>
> uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
> - uint32_t limit, uint32_t len)
> + uint32_t len)
> {
> + uint32_t limit = pci_config_size(pci_dev);
> uint32_t ret;
>
> assert(len <= 4);
> - ret = pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr));
> +
> + if (limit <= addr) {
> + /* conventional pci device can be behind pcie-to-pci bridge.
> + 256 <= addr < 4K has no effects. */
> + ret = ~0x0;
> + } else {
> + ret = pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr));
> + }
> trace_pci_cfg_read(pci_dev->name, PCI_SLOT(pci_dev->devfn),
> PCI_FUNC(pci_dev->devfn), addr, ret);
>
> @@ -95,8 +103,7 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
> return ~0x0;
> }
>
> - val = pci_host_config_read_common(pci_dev, config_addr,
> - PCI_CONFIG_SPACE_SIZE, len);
> + val = pci_host_config_read_common(pci_dev, config_addr, len);
> PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
> __func__, pci_dev->name, config_addr, val, len);
>
> diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
> index 3db038f..cf8587b 100644
> --- a/hw/pci/pcie_host.c
> +++ b/hw/pci/pcie_host.c
> @@ -62,19 +62,12 @@ static uint64_t pcie_mmcfg_data_read(void *opaque,
> PCIBus *s = e->pci.bus;
> PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
> uint32_t addr;
> - uint32_t limit;
>
> if (!pci_dev) {
> return ~0x0;
> }
> addr = PCIE_MMCFG_CONFOFFSET(mmcfg_addr);
> - limit = pci_config_size(pci_dev);
> - if (limit <= addr) {
> - /* conventional pci device can be behind pcie-to-pci bridge.
> - 256 <= addr < 4K has no effects. */
> - return ~0x0;
> - }
> - return pci_host_config_read_common(pci_dev, addr, limit, len);
> + return pci_host_config_read_common(pci_dev, addr, len);
> }
>
> static const MemoryRegionOps pcie_mmcfg_ops = {
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 21b95b3..59c6608 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -105,8 +105,7 @@ static void finish_read_pci_config(sPAPREnvironment *spapr, uint64_t buid,
> return;
> }
>
> - val = pci_host_config_read_common(pci_dev, addr,
> - pci_config_size(pci_dev), size);
> + val = pci_host_config_read_common(pci_dev, addr, size);
>
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> rtas_st(rets, 1, val);
> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> index 2bae45a..72a1b8b 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -65,7 +65,7 @@ typedef struct PCIHostBridgeClass {
> void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
> uint32_t limit, uint32_t val, uint32_t len);
> uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
> - uint32_t limit, uint32_t len);
> + uint32_t len);
>
> void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
> --
> 1.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v3 5/6] pci: remove the limit parameter of pci_host_config_write_common
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
` (3 preceding siblings ...)
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 4/6] pci: remove the limit parameter of pci_host_config_read_common Hu Tao
@ 2014-12-11 2:20 ` Hu Tao
2015-01-21 11:57 ` Michael S. Tsirkin
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO Hu Tao
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Marcel Apfelbaum, Michael S. Tsirkin
Since the limit parameter is always set to the size of pci device's
configuration space, and we can determine the size from the type of pci
device.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
hw/pci/pci_host.c | 13 ++++++++++---
hw/pci/pcie_host.c | 9 +--------
hw/ppc/spapr_pci.c | 3 +--
include/hw/pci/pci_host.h | 2 +-
4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 2b11551..4a59b0e 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -49,8 +49,16 @@ static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr)
}
void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
- uint32_t limit, uint32_t val, uint32_t len)
+ uint32_t val, uint32_t len)
{
+ uint32_t limit = pci_config_size(pci_dev);
+
+ if (limit <= addr) {
+ /* conventional pci device can be behind pcie-to-pci bridge.
+ 256 <= addr < 4K has no effects. */
+ return;
+ }
+
assert(len <= 4);
trace_pci_cfg_write(pci_dev->name, PCI_SLOT(pci_dev->devfn),
PCI_FUNC(pci_dev->devfn), addr, val);
@@ -89,8 +97,7 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len)
PCI_DPRINTF("%s: %s: addr=%02" PRIx32 " val=%08" PRIx32 " len=%d\n",
__func__, pci_dev->name, config_addr, val, len);
- pci_host_config_write_common(pci_dev, config_addr, PCI_CONFIG_SPACE_SIZE,
- val, len);
+ pci_host_config_write_common(pci_dev, config_addr, val, len);
}
uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
index cf8587b..e3a2a80 100644
--- a/hw/pci/pcie_host.c
+++ b/hw/pci/pcie_host.c
@@ -39,19 +39,12 @@ static void pcie_mmcfg_data_write(void *opaque, hwaddr mmcfg_addr,
PCIBus *s = e->pci.bus;
PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
uint32_t addr;
- uint32_t limit;
if (!pci_dev) {
return;
}
addr = PCIE_MMCFG_CONFOFFSET(mmcfg_addr);
- limit = pci_config_size(pci_dev);
- if (limit <= addr) {
- /* conventional pci device can be behind pcie-to-pci bridge.
- 256 <= addr < 4K has no effects. */
- return;
- }
- pci_host_config_write_common(pci_dev, addr, limit, val, len);
+ pci_host_config_write_common(pci_dev, addr, val, len);
}
static uint64_t pcie_mmcfg_data_read(void *opaque,
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 59c6608..8c566dd 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -171,8 +171,7 @@ static void finish_write_pci_config(sPAPREnvironment *spapr, uint64_t buid,
return;
}
- pci_host_config_write_common(pci_dev, addr, pci_config_size(pci_dev),
- val, size);
+ pci_host_config_write_common(pci_dev, addr, val, size);
rtas_st(rets, 0, RTAS_OUT_SUCCESS);
}
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index 72a1b8b..67e007f 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -63,7 +63,7 @@ typedef struct PCIHostBridgeClass {
/* common internal helpers for PCI/PCIe hosts, cut off overflows */
void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
- uint32_t limit, uint32_t val, uint32_t len);
+ uint32_t val, uint32_t len);
uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
uint32_t len);
--
1.9.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 5/6] pci: remove the limit parameter of pci_host_config_write_common
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 5/6] pci: remove the limit parameter of pci_host_config_write_common Hu Tao
@ 2015-01-21 11:57 ` Michael S. Tsirkin
0 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-21 11:57 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Thu, Dec 11, 2014 at 10:20:27AM +0800, Hu Tao wrote:
> Since the limit parameter is always set to the size of pci device's
> configuration space, and we can determine the size from the type of pci
> device.
Same comment.
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> hw/pci/pci_host.c | 13 ++++++++++---
> hw/pci/pcie_host.c | 9 +--------
> hw/ppc/spapr_pci.c | 3 +--
> include/hw/pci/pci_host.h | 2 +-
> 4 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 2b11551..4a59b0e 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -49,8 +49,16 @@ static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr)
> }
>
> void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
> - uint32_t limit, uint32_t val, uint32_t len)
> + uint32_t val, uint32_t len)
> {
> + uint32_t limit = pci_config_size(pci_dev);
> +
> + if (limit <= addr) {
> + /* conventional pci device can be behind pcie-to-pci bridge.
> + 256 <= addr < 4K has no effects. */
> + return;
> + }
> +
> assert(len <= 4);
> trace_pci_cfg_write(pci_dev->name, PCI_SLOT(pci_dev->devfn),
> PCI_FUNC(pci_dev->devfn), addr, val);
> @@ -89,8 +97,7 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len)
>
> PCI_DPRINTF("%s: %s: addr=%02" PRIx32 " val=%08" PRIx32 " len=%d\n",
> __func__, pci_dev->name, config_addr, val, len);
> - pci_host_config_write_common(pci_dev, config_addr, PCI_CONFIG_SPACE_SIZE,
> - val, len);
> + pci_host_config_write_common(pci_dev, config_addr, val, len);
> }
>
> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
> diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
> index cf8587b..e3a2a80 100644
> --- a/hw/pci/pcie_host.c
> +++ b/hw/pci/pcie_host.c
> @@ -39,19 +39,12 @@ static void pcie_mmcfg_data_write(void *opaque, hwaddr mmcfg_addr,
> PCIBus *s = e->pci.bus;
> PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
> uint32_t addr;
> - uint32_t limit;
>
> if (!pci_dev) {
> return;
> }
> addr = PCIE_MMCFG_CONFOFFSET(mmcfg_addr);
> - limit = pci_config_size(pci_dev);
> - if (limit <= addr) {
> - /* conventional pci device can be behind pcie-to-pci bridge.
> - 256 <= addr < 4K has no effects. */
> - return;
> - }
> - pci_host_config_write_common(pci_dev, addr, limit, val, len);
> + pci_host_config_write_common(pci_dev, addr, val, len);
> }
>
> static uint64_t pcie_mmcfg_data_read(void *opaque,
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 59c6608..8c566dd 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -171,8 +171,7 @@ static void finish_write_pci_config(sPAPREnvironment *spapr, uint64_t buid,
> return;
> }
>
> - pci_host_config_write_common(pci_dev, addr, pci_config_size(pci_dev),
> - val, size);
> + pci_host_config_write_common(pci_dev, addr, val, size);
>
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> }
> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> index 72a1b8b..67e007f 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -63,7 +63,7 @@ typedef struct PCIHostBridgeClass {
>
> /* common internal helpers for PCI/PCIe hosts, cut off overflows */
> void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
> - uint32_t limit, uint32_t val, uint32_t len);
> + uint32_t val, uint32_t len);
> uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
> uint32_t len);
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
` (4 preceding siblings ...)
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 5/6] pci: remove the limit parameter of pci_host_config_write_common Hu Tao
@ 2014-12-11 2:20 ` Hu Tao
2014-12-11 2:46 ` Hu Tao
2015-01-21 12:00 ` Michael S. Tsirkin
2014-12-11 2:50 ` [Qemu-devel] [PATCH v3 RESEND " Hu Tao
2015-01-21 6:41 ` [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
7 siblings, 2 replies; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Marcel Apfelbaum, Michael S. Tsirkin
Introduce PCI_DEVFN_AUTO rather than using -1 in code.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
hw/core/qdev-properties.c | 1 +
hw/pci/pci.c | 5 ++---
include/hw/pci/pci.h | 2 ++
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2e47f70..df4ad14 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -6,6 +6,7 @@
#include "net/hub.h"
#include "qapi/visitor.h"
#include "sysemu/char.h"
+#include "hw/pci/pci.h"
void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
Error **errp)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 371699c..73c7dec 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -50,7 +50,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
static void pcibus_reset(BusState *qbus);
static Property pci_props[] = {
- DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
+ DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, PCI_DEVFN_AUTO),
DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
@@ -801,7 +801,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
address_space_destroy(&pci_dev->bus_master_as);
}
-/* -1 for devfn means auto assign */
static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
const char *name, int devfn)
{
@@ -810,7 +809,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
PCIConfigWriteFunc *config_write = pc->config_write;
AddressSpace *dma_as;
- if (devfn < 0) {
+ if (devfn == PCI_DEVFN_AUTO) {
for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
devfn += PCI_FUNC_MAX) {
if (!bus->devices[devfn])
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index b18759a..9206b12 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -164,6 +164,8 @@ enum {
QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
};
+#define PCI_DEVFN_AUTO -1
+
#define TYPE_PCI_DEVICE "pci-device"
#define PCI_DEVICE(obj) \
OBJECT_CHECK(PCIDevice, (obj), TYPE_PCI_DEVICE)
--
1.9.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO Hu Tao
@ 2014-12-11 2:46 ` Hu Tao
2015-01-21 12:00 ` Michael S. Tsirkin
1 sibling, 0 replies; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum
On Thu, Dec 11, 2014 at 10:20:28AM +0800, Hu Tao wrote:
> Introduce PCI_DEVFN_AUTO rather than using -1 in code.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> hw/core/qdev-properties.c | 1 +
> hw/pci/pci.c | 5 ++---
> include/hw/pci/pci.h | 2 ++
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 2e47f70..df4ad14 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -6,6 +6,7 @@
> #include "net/hub.h"
> #include "qapi/visitor.h"
> #include "sysemu/char.h"
> +#include "hw/pci/pci.h"
Oops, this one shouldn't be here, will resend.
Regards,
Hu
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO Hu Tao
2014-12-11 2:46 ` Hu Tao
@ 2015-01-21 12:00 ` Michael S. Tsirkin
2015-01-27 6:42 ` Hu Tao
1 sibling, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-21 12:00 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Thu, Dec 11, 2014 at 10:20:28AM +0800, Hu Tao wrote:
> Introduce PCI_DEVFN_AUTO rather than using -1 in code.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> hw/core/qdev-properties.c | 1 +
> hw/pci/pci.c | 5 ++---
> include/hw/pci/pci.h | 2 ++
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 2e47f70..df4ad14 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -6,6 +6,7 @@
> #include "net/hub.h"
> #include "qapi/visitor.h"
> #include "sysemu/char.h"
> +#include "hw/pci/pci.h"
>
> void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
> Error **errp)
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 371699c..73c7dec 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -50,7 +50,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
> static void pcibus_reset(BusState *qbus);
>
> static Property pci_props[] = {
> - DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> + DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, PCI_DEVFN_AUTO),
> DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
> DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
> @@ -801,7 +801,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
> address_space_destroy(&pci_dev->bus_master_as);
> }
>
> -/* -1 for devfn means auto assign */
> static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> const char *name, int devfn)
> {
> @@ -810,7 +809,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> PCIConfigWriteFunc *config_write = pc->config_write;
> AddressSpace *dma_as;
>
> - if (devfn < 0) {
> + if (devfn == PCI_DEVFN_AUTO) {
How do we know we found all places that do this?
How do we know parameter was already validate for
other values < 0?
Can you pls mention in the commit log why you think
it's safe?
> for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> devfn += PCI_FUNC_MAX) {
> if (!bus->devices[devfn])
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index b18759a..9206b12 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -164,6 +164,8 @@ enum {
> QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
> };
>
> +#define PCI_DEVFN_AUTO -1
> +
> #define TYPE_PCI_DEVICE "pci-device"
> #define PCI_DEVICE(obj) \
> OBJECT_CHECK(PCIDevice, (obj), TYPE_PCI_DEVICE)
> --
> 1.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO
2015-01-21 12:00 ` Michael S. Tsirkin
@ 2015-01-27 6:42 ` Hu Tao
0 siblings, 0 replies; 21+ messages in thread
From: Hu Tao @ 2015-01-27 6:42 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum
On Wed, Jan 21, 2015 at 02:00:02PM +0200, Michael S. Tsirkin wrote:
> On Thu, Dec 11, 2014 at 10:20:28AM +0800, Hu Tao wrote:
> > Introduce PCI_DEVFN_AUTO rather than using -1 in code.
> >
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> > hw/core/qdev-properties.c | 1 +
> > hw/pci/pci.c | 5 ++---
> > include/hw/pci/pci.h | 2 ++
> > 3 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 2e47f70..df4ad14 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -6,6 +6,7 @@
> > #include "net/hub.h"
> > #include "qapi/visitor.h"
> > #include "sysemu/char.h"
> > +#include "hw/pci/pci.h"
> >
> > void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
> > Error **errp)
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 371699c..73c7dec 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -50,7 +50,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
> > static void pcibus_reset(BusState *qbus);
> >
> > static Property pci_props[] = {
> > - DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> > + DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, PCI_DEVFN_AUTO),
> > DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> > DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
> > DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
> > @@ -801,7 +801,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
> > address_space_destroy(&pci_dev->bus_master_as);
> > }
> >
> > -/* -1 for devfn means auto assign */
> > static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> > const char *name, int devfn)
> > {
> > @@ -810,7 +809,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> > PCIConfigWriteFunc *config_write = pc->config_write;
> > AddressSpace *dma_as;
> >
> > - if (devfn < 0) {
> > + if (devfn == PCI_DEVFN_AUTO) {
>
> How do we know we found all places that do this?
Good question! Actually I found other places creating devfn=-1 pci
devices, by calling pci_create_simple_multifunction() and
pci_create_simple(). But I don't have a solid way to discover all these
places. So we should keep the original code here.
Please drop this patch.
Thanks,
Hu
> How do we know parameter was already validate for
> other values < 0?
> Can you pls mention in the commit log why you think
> it's safe?
>
>
> > for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> > devfn += PCI_FUNC_MAX) {
> > if (!bus->devices[devfn])
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index b18759a..9206b12 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -164,6 +164,8 @@ enum {
> > QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
> > };
> >
> > +#define PCI_DEVFN_AUTO -1
> > +
> > #define TYPE_PCI_DEVICE "pci-device"
> > #define PCI_DEVICE(obj) \
> > OBJECT_CHECK(PCIDevice, (obj), TYPE_PCI_DEVICE)
> > --
> > 1.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v3 RESEND 6/6] pci: introduce PCI_DEVFN_AUTO
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
` (5 preceding siblings ...)
2014-12-11 2:20 ` [Qemu-devel] [PATCH v3 6/6] pci: introduce PCI_DEVFN_AUTO Hu Tao
@ 2014-12-11 2:50 ` Hu Tao
2015-01-21 6:41 ` [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
7 siblings, 0 replies; 21+ messages in thread
From: Hu Tao @ 2014-12-11 2:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Marcel Apfelbaum, Michael S. Tsirkin
Introduce PCI_DEVFN_AUTO rather than using -1 in code.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
hw/pci/pci.c | 5 ++---
include/hw/pci/pci.h | 2 ++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 371699c..73c7dec 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -50,7 +50,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
static void pcibus_reset(BusState *qbus);
static Property pci_props[] = {
- DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
+ DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, PCI_DEVFN_AUTO),
DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
@@ -801,7 +801,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
address_space_destroy(&pci_dev->bus_master_as);
}
-/* -1 for devfn means auto assign */
static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
const char *name, int devfn)
{
@@ -810,7 +809,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
PCIConfigWriteFunc *config_write = pc->config_write;
AddressSpace *dma_as;
- if (devfn < 0) {
+ if (devfn == PCI_DEVFN_AUTO) {
for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
devfn += PCI_FUNC_MAX) {
if (!bus->devices[devfn])
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index b18759a..9206b12 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -164,6 +164,8 @@ enum {
QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR),
};
+#define PCI_DEVFN_AUTO -1
+
#define TYPE_PCI_DEVICE "pci-device"
#define PCI_DEVICE(obj) \
OBJECT_CHECK(PCIDevice, (obj), TYPE_PCI_DEVICE)
--
1.9.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches
2014-12-11 2:20 [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
` (6 preceding siblings ...)
2014-12-11 2:50 ` [Qemu-devel] [PATCH v3 RESEND " Hu Tao
@ 2015-01-21 6:41 ` Hu Tao
2015-01-21 12:00 ` Michael S. Tsirkin
7 siblings, 1 reply; 21+ messages in thread
From: Hu Tao @ 2015-01-21 6:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum
ping...
On Thu, Dec 11, 2014 at 10:20:22AM +0800, Hu Tao wrote:
> Hi,
>
> This is v3 of PCI clenaup series. See each patch for the detail.
>
> Regards,
> Hu
>
> changes:
>
> v3:
> - rebase on top of 7fb8da2b886, all 5 patches applied cleanly.
> - new patch: pci: introduce PCI_DEVFN_AUTO
>
> v2:
> - remove patch 3 from v1 which is incorrect.
> - rename defined macros as per Marcel's suggestion
> - place macros in pci_host.h as per Marcel's suggestion
> - new patch 'pci: reorganize QEMU_PCI_CAP_*'
>
> Hu Tao (6):
> pci: reorganize QEMU_PCI_CAP_*
> pci: introduce pci_host_config_enabled()
> pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and
> PCI_HOST_BRIDGE_CONFIG_DATA.
> pci: remove the limit parameter of pci_host_config_read_common
> pci: remove the limit parameter of pci_host_config_write_common
> pci: introduce PCI_DEVFN_AUTO
>
> hw/core/qdev-properties.c | 1 +
> hw/mips/gt64xxx_pci.c | 4 ++--
> hw/pci-host/piix.c | 8 ++++----
> hw/pci-host/prep.c | 6 ++++--
> hw/pci-host/q35.c | 8 ++++----
> hw/pci/pci.c | 5 ++---
> hw/pci/pci_host.c | 33 ++++++++++++++++++++++++---------
> hw/pci/pcie_host.c | 18 ++----------------
> hw/ppc/spapr_pci.c | 6 ++----
> include/hw/pci-host/q35.h | 3 ---
> include/hw/pci/pci.h | 41 ++++++++++++++++++++++-------------------
> include/hw/pci/pci_host.h | 14 ++++++++++++--
> tests/libqos/pci-pc.c | 25 +++++++++++++------------
> 13 files changed, 92 insertions(+), 80 deletions(-)
>
> --
> 1.9.3
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches
2015-01-21 6:41 ` [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches Hu Tao
@ 2015-01-21 12:00 ` Michael S. Tsirkin
2015-01-27 6:44 ` Hu Tao
0 siblings, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2015-01-21 12:00 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Wed, Jan 21, 2015 at 02:41:33PM +0800, Hu Tao wrote:
> ping...
At some point you said "will resend".
> On Thu, Dec 11, 2014 at 10:20:22AM +0800, Hu Tao wrote:
> > Hi,
> >
> > This is v3 of PCI clenaup series. See each patch for the detail.
> >
> > Regards,
> > Hu
> >
> > changes:
> >
> > v3:
> > - rebase on top of 7fb8da2b886, all 5 patches applied cleanly.
> > - new patch: pci: introduce PCI_DEVFN_AUTO
> >
> > v2:
> > - remove patch 3 from v1 which is incorrect.
> > - rename defined macros as per Marcel's suggestion
> > - place macros in pci_host.h as per Marcel's suggestion
> > - new patch 'pci: reorganize QEMU_PCI_CAP_*'
> >
> > Hu Tao (6):
> > pci: reorganize QEMU_PCI_CAP_*
> > pci: introduce pci_host_config_enabled()
> > pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and
> > PCI_HOST_BRIDGE_CONFIG_DATA.
> > pci: remove the limit parameter of pci_host_config_read_common
> > pci: remove the limit parameter of pci_host_config_write_common
> > pci: introduce PCI_DEVFN_AUTO
> >
> > hw/core/qdev-properties.c | 1 +
> > hw/mips/gt64xxx_pci.c | 4 ++--
> > hw/pci-host/piix.c | 8 ++++----
> > hw/pci-host/prep.c | 6 ++++--
> > hw/pci-host/q35.c | 8 ++++----
> > hw/pci/pci.c | 5 ++---
> > hw/pci/pci_host.c | 33 ++++++++++++++++++++++++---------
> > hw/pci/pcie_host.c | 18 ++----------------
> > hw/ppc/spapr_pci.c | 6 ++----
> > include/hw/pci-host/q35.h | 3 ---
> > include/hw/pci/pci.h | 41 ++++++++++++++++++++++-------------------
> > include/hw/pci/pci_host.h | 14 ++++++++++++--
> > tests/libqos/pci-pc.c | 25 +++++++++++++------------
> > 13 files changed, 92 insertions(+), 80 deletions(-)
> >
> > --
> > 1.9.3
> >
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/6] Some PCI related cleanup patches
2015-01-21 12:00 ` Michael S. Tsirkin
@ 2015-01-27 6:44 ` Hu Tao
0 siblings, 0 replies; 21+ messages in thread
From: Hu Tao @ 2015-01-27 6:44 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum
On Wed, Jan 21, 2015 at 02:00:58PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 21, 2015 at 02:41:33PM +0800, Hu Tao wrote:
> > ping...
>
> At some point you said "will resend".
I meant resend patch 6.
Patches 1-5 can still apply cleanly.
Regards,
Hu
>
> > On Thu, Dec 11, 2014 at 10:20:22AM +0800, Hu Tao wrote:
> > > Hi,
> > >
> > > This is v3 of PCI clenaup series. See each patch for the detail.
> > >
> > > Regards,
> > > Hu
> > >
> > > changes:
> > >
> > > v3:
> > > - rebase on top of 7fb8da2b886, all 5 patches applied cleanly.
> > > - new patch: pci: introduce PCI_DEVFN_AUTO
> > >
> > > v2:
> > > - remove patch 3 from v1 which is incorrect.
> > > - rename defined macros as per Marcel's suggestion
> > > - place macros in pci_host.h as per Marcel's suggestion
> > > - new patch 'pci: reorganize QEMU_PCI_CAP_*'
> > >
> > > Hu Tao (6):
> > > pci: reorganize QEMU_PCI_CAP_*
> > > pci: introduce pci_host_config_enabled()
> > > pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and
> > > PCI_HOST_BRIDGE_CONFIG_DATA.
> > > pci: remove the limit parameter of pci_host_config_read_common
> > > pci: remove the limit parameter of pci_host_config_write_common
> > > pci: introduce PCI_DEVFN_AUTO
> > >
> > > hw/core/qdev-properties.c | 1 +
> > > hw/mips/gt64xxx_pci.c | 4 ++--
> > > hw/pci-host/piix.c | 8 ++++----
> > > hw/pci-host/prep.c | 6 ++++--
> > > hw/pci-host/q35.c | 8 ++++----
> > > hw/pci/pci.c | 5 ++---
> > > hw/pci/pci_host.c | 33 ++++++++++++++++++++++++---------
> > > hw/pci/pcie_host.c | 18 ++----------------
> > > hw/ppc/spapr_pci.c | 6 ++----
> > > include/hw/pci-host/q35.h | 3 ---
> > > include/hw/pci/pci.h | 41 ++++++++++++++++++++++-------------------
> > > include/hw/pci/pci_host.h | 14 ++++++++++++--
> > > tests/libqos/pci-pc.c | 25 +++++++++++++------------
> > > 13 files changed, 92 insertions(+), 80 deletions(-)
> > >
> > > --
> > > 1.9.3
> > >
> > >
^ permalink raw reply [flat|nested] 21+ messages in thread