* [Qemu-devel] [PATCH v2 1/5] pci: reorganize QEMU_PCI_CAP_*
2014-11-05 9:02 [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Hu Tao
@ 2014-11-05 9:02 ` Hu Tao
2014-11-05 17:12 ` Marcel Apfelbaum
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 2/5] pci: introduce pci_host_config_enabled() Hu Tao
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Hu Tao @ 2014-11-05 9:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum
This makes code more readable.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.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] 12+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/5] pci: reorganize QEMU_PCI_CAP_*
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 1/5] pci: reorganize QEMU_PCI_CAP_* Hu Tao
@ 2014-11-05 17:12 ` Marcel Apfelbaum
0 siblings, 0 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2014-11-05 17:12 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Michael S. Tsirkin
On Wed, 2014-11-05 at 17:02 +0800, Hu Tao wrote:
> This makes code more readable.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.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"
Definitely more readable.
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 2/5] pci: introduce pci_host_config_enabled()
2014-11-05 9:02 [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Hu Tao
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 1/5] pci: reorganize QEMU_PCI_CAP_* Hu Tao
@ 2014-11-05 9:02 ` Hu Tao
2014-11-05 17:12 ` Marcel Apfelbaum
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 3/5] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA Hu Tao
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Hu Tao @ 2014-11-05 9:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum
This makes code more readable.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.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] 12+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/5] pci: introduce pci_host_config_enabled()
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 2/5] pci: introduce pci_host_config_enabled() Hu Tao
@ 2014-11-05 17:12 ` Marcel Apfelbaum
0 siblings, 0 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2014-11-05 17:12 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Michael S. Tsirkin
On Wed, 2014-11-05 at 17:02 +0800, Hu Tao wrote:
> This makes code more readable.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.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;
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 3/5] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.
2014-11-05 9:02 [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Hu Tao
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 1/5] pci: reorganize QEMU_PCI_CAP_* Hu Tao
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 2/5] pci: introduce pci_host_config_enabled() Hu Tao
@ 2014-11-05 9:02 ` Hu Tao
2014-11-05 17:12 ` Marcel Apfelbaum
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 4/5] pci: remove the limit parameter of pci_host_config_read_common Hu Tao
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Hu Tao @ 2014-11-05 9:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum
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>
---
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] 12+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/5] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 3/5] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA Hu Tao
@ 2014-11-05 17:12 ` Marcel Apfelbaum
0 siblings, 0 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2014-11-05 17:12 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Michael S. Tsirkin
On Wed, 2014-11-05 at 17:02 +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>
> ---
> 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)
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 4/5] pci: remove the limit parameter of pci_host_config_read_common
2014-11-05 9:02 [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Hu Tao
` (2 preceding siblings ...)
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 3/5] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA Hu Tao
@ 2014-11-05 9:02 ` Hu Tao
2014-11-05 17:33 ` Marcel Apfelbaum
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 5/5] pci: remove the limit parameter of pci_host_config_write_common Hu Tao
2014-11-05 17:48 ` [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Michael S. Tsirkin
5 siblings, 1 reply; 12+ messages in thread
From: Hu Tao @ 2014-11-05 9:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum
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 ad0da7f..7f38117 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] 12+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/5] pci: remove the limit parameter of pci_host_config_read_common
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 4/5] pci: remove the limit parameter of pci_host_config_read_common Hu Tao
@ 2014-11-05 17:33 ` Marcel Apfelbaum
0 siblings, 0 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2014-11-05 17:33 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Hu Tao, qemu-devel
On Wed, 2014-11-05 at 17:02 +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>
> ---
> 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 ad0da7f..7f38117 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);
I am not sure if it worth the effort, but the patch seems to be correct.
Michael what do you think?
Thanks,
Marcel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 5/5] pci: remove the limit parameter of pci_host_config_write_common
2014-11-05 9:02 [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Hu Tao
` (3 preceding siblings ...)
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 4/5] pci: remove the limit parameter of pci_host_config_read_common Hu Tao
@ 2014-11-05 9:02 ` Hu Tao
2014-11-05 17:48 ` [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Michael S. Tsirkin
5 siblings, 0 replies; 12+ messages in thread
From: Hu Tao @ 2014-11-05 9:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum
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 7f38117..f306d42 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] 12+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches
2014-11-05 9:02 [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Hu Tao
` (4 preceding siblings ...)
2014-11-05 9:02 ` [Qemu-devel] [PATCH v2 5/5] pci: remove the limit parameter of pci_host_config_write_common Hu Tao
@ 2014-11-05 17:48 ` Michael S. Tsirkin
2014-11-06 1:09 ` Hu Tao
5 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2014-11-05 17:48 UTC (permalink / raw)
To: Hu Tao; +Cc: qemu-devel, Marcel Apfelbaum
On Wed, Nov 05, 2014 at 05:02:41PM +0800, Hu Tao wrote:
> Hi,
>
> This is v2 of PCI clenaup series. See each patch for the detail.
Thanks for the patches!
Pls note this is all not 2.2 material.
Pls resubmit after 2.2 is out.
> changes:
> 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 (5):
> 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
>
> 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_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 | 39 ++++++++++++++++++++-------------------
> include/hw/pci/pci_host.h | 14 ++++++++++++--
> tests/libqos/pci-pc.c | 25 +++++++++++++------------
> 11 files changed, 87 insertions(+), 77 deletions(-)
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches
2014-11-05 17:48 ` [Qemu-devel] [PATCH v2 0/5] Some PCI related cleanup patches Michael S. Tsirkin
@ 2014-11-06 1:09 ` Hu Tao
0 siblings, 0 replies; 12+ messages in thread
From: Hu Tao @ 2014-11-06 1:09 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum
On Wed, Nov 05, 2014 at 07:48:10PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 05, 2014 at 05:02:41PM +0800, Hu Tao wrote:
> > Hi,
> >
> > This is v2 of PCI clenaup series. See each patch for the detail.
>
> Thanks for the patches!
>
> Pls note this is all not 2.2 material.
> Pls resubmit after 2.2 is out.
Sure.
Regards,
Hu
^ permalink raw reply [flat|nested] 12+ messages in thread