Some PCI related cleanup patches
v2 cover Some PCI related cleanup patches
Hi,
This is v2 of PCI clenaup series. See each patch for the detail.
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(-)
Signature · 1 line
1.9.3
PATCH v2 1/5 pci: reorganize QEMU_PCI_CAP_*
This makes code more readable.
--- include/hw/pci/pci.h | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-)
include/hw/pci/pci.h +20 -19
@@ -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"
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 1/5] pci: reorganize QEMU_PCI_CAP_*
Quoted 47 lines · Hu Tao
This makes code more readable.
Signed-off-by Hu Tao <[email protected]>--- include/hw/pci/pci.h | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-)include/hw/pci/pci.h +13
-19@@ -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 ),
include/hw/pci/pci.h enum { · line 167
Quoted 10 lines
include/hw/pci/pci.h +7
-0+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.
PATCH v2 2/5 pci: introduce pci_host_config_enabled()
In reply to Hu TaoThis makes code more readable.
--- 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(-)
hw/mips/gt64xxx_pci.c +2 -2
@@ -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 );
hw/pci/pci_host.c +3 -2
@@ -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);
include/hw/pci/pci_host.h +5 -0
@@ -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;
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 2/5] pci: introduce pci_host_config_enabled()
Quoted 61 lines · Hu Tao
This makes code more readable.
Signed-off-by Hu Tao <[email protected]>--- 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(-)hw/mips/gt64xxx_pci.c +2
-2@@ -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 );hw/pci/pci_host.c +3
-2@@ -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);include/hw/pci/pci_host.h +0
-0@@ -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);
include/hw/pci/pci_host.h uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr, · line 75
Quoted 10 lines
include/hw/pci/pci_host.h +5
-0uint32_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;
PATCH v2 3/5 pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.
In reply to Hu TaoPCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA are defined in PCI specification, so move them to common place.
--- 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(-)
hw/pci-host/piix.c +4 -4
@@ -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)
hw/pci-host/prep.c +4 -2
@@ -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 );
hw/pci-host/q35.c +4 -4
@@ -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,
include/hw/pci-host/q35.h +0 -3
@@ -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
include/hw/pci/pci_host.h +5 -0
@@ -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)
tests/libqos/pci-pc.c +13 -12
@@ -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)
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 3/5] pci: define PCI_HOST_BRIDGE_CONFIG_ADDR and PCI_HOST_BRIDGE_CONFIG_DATA.
Quoted 156 lines · Hu Tao
PCI_HOST_BRIDGE_CONFIG_ADDRandPCI_HOST_BRIDGE_CONFIG_DATAare defined in PCI specification, so move them to common place.Signed-off-by Hu Tao <[email protected]>--- 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(-)hw/pci-host/piix.c +4
-4@@ -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)hw/pci-host/prep.c +4
-2@@ -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 );hw/pci-host/q35.c +4
-4@@ -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,include/hw/pci-host/q35.h +0
-3@@ -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 0x0include/hw/pci/pci_host.h +5
-0@@ -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)tests/libqos/pci-pc.c +11
-10@@ -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); }
tests/libqos/pci-pc.c static void qpci_pc_io_writel(QPCIBus *bus, void *addr, uint32_t value) · line 151
Quoted 10 lines
tests/libqos/pci-pc.c +2
-2static 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)
PATCH v2 4/5 pci: remove the limit parameter of pci_host_config_read_common
In reply to Hu TaoSince 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.
--- 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(-)
hw/pci/pci_host.c +11 -4
@@ -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);
hw/pci/pcie_host.c +1 -8
@@ -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 = {
hw/ppc/spapr_pci.c +1 -2
@@ -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);
include/hw/pci/pci_host.h +1 -1
@@ -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);
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 4/5] pci: remove the limit parameter of pci_host_config_read_common
Quoted 93 lines · Hu Tao
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 <[email protected]>--- 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(-)hw/pci/pci_host.c +11
-4@@ -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);hw/pci/pcie_host.c +1
-8@@ -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 = {hw/ppc/spapr_pci.c +1
-2@@ -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);include/hw/pci/pci_host.h +0
-0
include/hw/pci/pci_host.h typedef struct PCIHostBridgeClass { · line 71
Quoted 8 lines
include/hw/pci/pci_host.h +1
-1@@ -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
PATCH v2 5/5 pci: remove the limit parameter of pci_host_config_write_common
In reply to Hu TaoSince 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.
--- 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(-)
hw/pci/pci_host.c +10 -3
@@ -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)
hw/pci/pcie_host.c +1 -8
@@ -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,
hw/ppc/spapr_pci.c +1 -2
@@ -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 ); }
include/hw/pci/pci_host.h +1 -1
@@ -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);
Signature · 1 line
1.9.3
Quoted 3 lines · Hu Tao
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.
Quoted 30 lines
changes:
v2:
- remove patch 3 from v1 which is incorrect.
- rename defined macros as per Marcel's suggestion
- place macros inpci_host.has per Marcel's suggestion
- new patch 'pci: reorganize QEMU_PCI_CAP_*'Hu Tao (5):
pci: reorganize QEMU_PCI_CAP_*
pci: introducepci_host_config_enabled()
pci: definePCI_HOST_BRIDGE_CONFIG_ADDRand
PCI_HOST_BRIDGE_CONFIG_DATA.
pci: remove the limit parameter ofpci_host_config_read_common
pci: remove the limit parameter ofpci_host_config_write_commonhw/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(-)Signature · 1 line
1.9.3
Quoted 9 lines · Michael S. Tsirkin
Quoted 3 lines · Hu Tao
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