Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges

2012-08-13 Thread Anthony Liguori 
Andreas Färber afaer...@suse.de writes:

 Uglify the parent field to enforce QOM-style access via casts.
 Don't just typedef PCIHostState, either use it directly or embed it.

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  hw/alpha_typhoon.c |2 +-
  hw/dec_pci.c   |2 +-
  hw/grackle_pci.c   |2 +-
  hw/gt64xxx.c   |   26 --
  hw/piix_pci.c  |6 --
  hw/ppc4xx_pci.c|8 +---
  hw/ppce500_pci.c   |2 +-
  hw/prep_pci.c  |8 +---
  hw/spapr_pci.c |   12 +++-
  hw/spapr_pci.h |2 +-
  hw/unin_pci.c  |   14 +++---
  11 files changed, 49 insertions(+), 35 deletions(-)

 diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
 index 7667412..b7cf4e2 100644
 --- a/hw/alpha_typhoon.c
 +++ b/hw/alpha_typhoon.c
 @@ -46,7 +46,7 @@ typedef struct TyphoonPchip {
  OBJECT_CHECK(TyphoonState, (obj), TYPE_TYPHOON_PCI_HOST_BRIDGE)
  
  typedef struct TyphoonState {
 -PCIHostState host;
 +PCIHostState parent_obj;
  
  TyphoonCchip cchip;
  TyphoonPchip pchip;
 diff --git a/hw/dec_pci.c b/hw/dec_pci.c
 index de16361..c30ade3 100644
 --- a/hw/dec_pci.c
 +++ b/hw/dec_pci.c
 @@ -43,7 +43,7 @@
  #define DEC_21154(obj) OBJECT_CHECK(DECState, (obj), TYPE_DEC_21154)
  
  typedef struct DECState {
 -PCIHostState host_state;
 +PCIHostState parent_obj;
  } DECState;
  
  static int dec_map_irq(PCIDevice *pci_dev, int irq_num)
 diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
 index 066f6e1..67da307 100644
 --- a/hw/grackle_pci.c
 +++ b/hw/grackle_pci.c
 @@ -41,7 +41,7 @@
  OBJECT_CHECK(GrackleState, (obj), TYPE_GRACKLE_PCI_HOST_BRIDGE)
  
  typedef struct GrackleState {
 -PCIHostState host_state;
 +PCIHostState parent_obj;
  
  MemoryRegion pci_mmio;
  MemoryRegion pci_hole;
 diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
 index 857758e..e95e664 100644
 --- a/hw/gt64xxx.c
 +++ b/hw/gt64xxx.c
 @@ -235,7 +235,7 @@
  OBJECT_CHECK(GT64120State, (obj), TYPE_GT64120_PCI_HOST_BRIDGE)
  
  typedef struct GT64120State {
 -PCIHostState pci;
 +PCIHostState parent_obj;
  
  uint32_t regs[GT_REGS];
  PCI_MAPPING_ENTRY(PCI0IO);
 @@ -315,6 +315,7 @@ static void gt64120_writel (void *opaque, 
 target_phys_addr_t addr,
  uint64_t val, unsigned size)
  {
  GT64120State *s = opaque;
 +PCIHostState *phb = PCI_HOST_BRIDGE(s);
  uint32_t saddr;
  
  if (!(s-regs[GT_CPU]  0x1000))
 @@ -535,13 +536,15 @@ static void gt64120_writel (void *opaque, 
 target_phys_addr_t addr,
  /* not implemented */
  break;
  case GT_PCI0_CFGADDR:
 -s-pci.config_reg = val  0x80fc;
 +phb-config_reg = val  0x80fc;
  break;
  case GT_PCI0_CFGDATA:
 -if (!(s-regs[GT_PCI0_CMD]  1)  (s-pci.config_reg  0x00fff800))
 +if (!(s-regs[GT_PCI0_CMD]  1)  (phb-config_reg  0x00fff800)) {
  val = bswap32(val);
 -if (s-pci.config_reg  (1u  31))
 -pci_data_write(s-pci.bus, s-pci.config_reg, val, 4);
 +}
 +if (phb-config_reg  (1u  31)) {
 +pci_data_write(phb-bus, phb-config_reg, val, 4);
 +}
  break;
  
  /* Interrupts */
 @@ -594,6 +597,7 @@ static uint64_t gt64120_readl (void *opaque,
 target_phys_addr_t addr, unsigned size)
  {
  GT64120State *s = opaque;
 +PCIHostState *phb = PCI_HOST_BRIDGE(s);
  uint32_t val;
  uint32_t saddr;
  
 @@ -775,15 +779,17 @@ static uint64_t gt64120_readl (void *opaque,
  
  /* PCI Internal */
  case GT_PCI0_CFGADDR:
 -val = s-pci.config_reg;
 +val = phb-config_reg;
  break;
  case GT_PCI0_CFGDATA:
 -if (!(s-pci.config_reg  (1  31)))
 +if (!(phb-config_reg  (1  31))) {
  val = 0x;
 -else
 -val = pci_data_read(s-pci.bus, s-pci.config_reg, 4);
 -if (!(s-regs[GT_PCI0_CMD]  1)  (s-pci.config_reg  0x00fff800))
 +} else {
 +val = pci_data_read(phb-bus, phb-config_reg, 4);
 +}
 +if (!(s-regs[GT_PCI0_CMD]  1)  (phb-config_reg  0x00fff800)) {
  val = bswap32(val);
 +}
  break;
  
  case GT_PCI0_CMD:
 diff --git a/hw/piix_pci.c b/hw/piix_pci.c
 index 04ceccf..537fc19 100644
 --- a/hw/piix_pci.c
 +++ b/hw/piix_pci.c
 @@ -36,7 +36,9 @@
   * http://download.intel.com/design/chipsets/datashts/29054901.pdf
   */
  
 -typedef PCIHostState I440FXState;
 +typedef struct I440FXState {
 +PCIHostState parent_obj;
 +} I440FXState;
  
  #define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
  #define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
 @@ -274,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
  dev = qdev_create(NULL, i440FX-pcihost);
  s = PCI_HOST_BRIDGE(dev);
  s-address_space = address_space_mem;
 -b = pci_bus_new(s-busdev.qdev, NULL,

Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges

2012-08-13 Thread Michael S. Tsirkin 
On Thu, Aug 02, 2012 at 03:47:06AM +0200, Andreas Färber wrote:
 Uglify the parent field to enforce QOM-style access via casts.
 Don't just typedef PCIHostState, either use it directly or embed it.
 
 Signed-off-by: Andreas Färber afaer...@suse.de


IMHO only one chunk from this patch should be applied (below).
Below it is split out but needs to be rebased on top of patches
1-13.

--

From: Andreas Färber andreas.faer...@web.de

piix: minor code simplification

There's no need to deal with qdev internals in piix - we get device
state from qdev_create so just use that.

Signed-off-by: Andreas Färber andreas.faer...@web.de
Signed-off-by: Michael S. Tsirkin m...@redhat.com

---

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index c497a01..18554a6 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -274,7 +274,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
 dev = qdev_create(NULL, i440FX-pcihost);
 s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
 s-address_space = address_space_mem;
-b = pci_bus_new(s-busdev.qdev, NULL, pci_address_space,
+b = pci_bus_new(dev, NULL, pci_address_space,
 address_space_io, 0);
 s-bus = b;
 object_property_add_child(qdev_get_machine(), i440fx, OBJECT(dev), NULL);

Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges

2012-08-13 Thread Andreas Färber 
Am 13.08.2012 15:14, schrieb Anthony Liguori:
 Andreas Färber afaer...@suse.de writes:
 
 diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
 index df70cd2..8937030 100644
 --- a/hw/spapr_pci.c
 +++ b/hw/spapr_pci.c
 @@ -36,16 +36,18 @@ static PCIDevice *find_dev(sPAPREnvironment *spapr,
 uint64_t buid, uint32_t config_addr)
  {
  int devfn = (config_addr  8)  0xFF;
 -sPAPRPHBState *phb;
 +sPAPRPHBState *sphb;
  
 -QLIST_FOREACH(phb, spapr-phbs, list) {
 +QLIST_FOREACH(sphb, spapr-phbs, list) {
 +PCIHostState *phb;
  BusChild *kid;
  
 -if (phb-buid != buid) {
 +if (sphb-buid != buid) {
  continue;
  }
  
 -QTAILQ_FOREACH(kid, phb-host_state.bus-qbus.children, sibling) {
 +phb = PCI_HOST_BRIDGE(sphb);
 +QTAILQ_FOREACH(kid, BUS(phb-bus)-children, sibling) {
  PCIDevice *dev = (PCIDevice *)kid-child;
  if (dev-devfn == devfn) {
  return dev;

 @@ -319,7 +321,7 @@ static int spapr_phb_init(SysBusDevice *s)
 pci_spapr_set_irq, pci_spapr_map_irq, phb,
 phb-memspace, phb-iospace,
 PCI_DEVFN(0, 0), PCI_NUM_PINS);
 -phb-host_state.bus = bus;
 +PCI_HOST_BRIDGE(phb)-bus = bus;
 
 I think you meant:
 
 PCI_HOST_BRIDGE(sphb)-bus
 
 But really you meant:
 
 phb-bus = bus;

The patch is misleading here, in the initfn phb historically is
sPAPRPHBState, not PCIHostState.

But you are right that this inline macro usage should be fixed - will
solve by squashing phb - sphb renaming into the spapr_pci patch.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges

2012-08-13 Thread Anthony Liguori 
Michael S. Tsirkin m...@redhat.com writes:

 On Thu, Aug 02, 2012 at 03:47:06AM +0200, Andreas Färber wrote:
 Uglify the parent field to enforce QOM-style access via casts.
 Don't just typedef PCIHostState, either use it directly or embed it.
 
 Signed-off-by: Andreas Färber afaer...@suse.de


 IMHO only one chunk from this patch should be applied (below).
 Below it is split out but needs to be rebased on top of patches
 1-13.

I understand what your objection is but it's unreasonable IMHO.  The
purpose of QOM is to bring consistency across large swaths of code in
QEMU that have historically done things there own way.

This means expressing concepts like inheritence and casting in the same
way across the board.  The common way (the QOM way) is to make the
parent type the first member of the struct (typically named parent or
parent_obj) and then to use cast macros to upcast and downcast.

This patch is 100% correct in that regard and I'm going to apply it once
Andreas makes the change I requested.

For my part, I'm long over due in writing up a device authoring style
guide that I promised a few weeks ago.  I'll write that up this
afternoon and send it out today.

We can debate the merits of this sort of thing in the style guide.

Regards,

Anthony Liguori


 --

 From: Andreas Färber andreas.faer...@web.de

 piix: minor code simplification

 There's no need to deal with qdev internals in piix - we get device
 state from qdev_create so just use that.

 Signed-off-by: Andreas Färber andreas.faer...@web.de
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

 ---

 diff --git a/hw/piix_pci.c b/hw/piix_pci.c
 index c497a01..18554a6 100644
 --- a/hw/piix_pci.c
 +++ b/hw/piix_pci.c
 @@ -274,7 +274,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
  dev = qdev_create(NULL, i440FX-pcihost);
  s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
  s-address_space = address_space_mem;
 -b = pci_bus_new(s-busdev.qdev, NULL, pci_address_space,
 +b = pci_bus_new(dev, NULL, pci_address_space,
  address_space_io, 0);
  s-bus = b;
  object_property_add_child(qdev_get_machine(), i440fx, OBJECT(dev), 
 NULL);

Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges

2012-08-13 Thread Mark Cave-Ayland 

On 13/08/12 15:16, Anthony Liguori wrote:


I understand what your objection is but it's unreasonable IMHO.  The
purpose of QOM is to bring consistency across large swaths of code in
QEMU that have historically done things there own way.

This means expressing concepts like inheritence and casting in the same
way across the board.  The common way (the QOM way) is to make the
parent type the first member of the struct (typically named parent or
parent_obj) and then to use cast macros to upcast and downcast.

This patch is 100% correct in that regard and I'm going to apply it once
Andreas makes the change I requested.

For my part, I'm long over due in writing up a device authoring style
guide that I promised a few weeks ago.  I'll write that up this
afternoon and send it out today.

We can debate the merits of this sort of thing in the style guide.


Yes please! I've spent quite a fair bit of time over the past couple of 
weeks in this area, and have found the lack of documentation related to 
trying to create a new hardware device fairly frustrating. The hardest 
part is not the coding, but figuring which of the following I should be 
using out of:


qom
qdev
qmp
hmp
sysbus
VMState

(I think that half of these are now obsolete interfaces, but a general 
document which explains how these things are related and which ones I 
should be using would be very welcome)



ATB,

Mark.

Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges

2012-08-02 Thread Michael S. Tsirkin 
On Thu, Aug 02, 2012 at 03:47:06AM +0200, Andreas Färber wrote:
 Uglify the parent field to enforce QOM-style access via casts.
 Don't just typedef PCIHostState, either use it directly or embed it.
 
 Signed-off-by: Andreas Färber afaer...@suse.de

NAK I'd prefer to drop this one for now.

 ---
  hw/alpha_typhoon.c |2 +-
  hw/dec_pci.c   |2 +-
  hw/grackle_pci.c   |2 +-
  hw/gt64xxx.c   |   26 --
  hw/piix_pci.c  |6 --
  hw/ppc4xx_pci.c|8 +---
  hw/ppce500_pci.c   |2 +-
  hw/prep_pci.c  |8 +---
  hw/spapr_pci.c |   12 +++-
  hw/spapr_pci.h |2 +-
  hw/unin_pci.c  |   14 +++---
  11 files changed, 49 insertions(+), 35 deletions(-)
 
 diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
 index 7667412..b7cf4e2 100644
 --- a/hw/alpha_typhoon.c
 +++ b/hw/alpha_typhoon.c
 @@ -46,7 +46,7 @@ typedef struct TyphoonPchip {
  OBJECT_CHECK(TyphoonState, (obj), TYPE_TYPHOON_PCI_HOST_BRIDGE)
  
  typedef struct TyphoonState {
 -PCIHostState host;
 +PCIHostState parent_obj;
  
  TyphoonCchip cchip;
  TyphoonPchip pchip;
 diff --git a/hw/dec_pci.c b/hw/dec_pci.c
 index de16361..c30ade3 100644
 --- a/hw/dec_pci.c
 +++ b/hw/dec_pci.c
 @@ -43,7 +43,7 @@
  #define DEC_21154(obj) OBJECT_CHECK(DECState, (obj), TYPE_DEC_21154)
  
  typedef struct DECState {
 -PCIHostState host_state;
 +PCIHostState parent_obj;
  } DECState;
  
  static int dec_map_irq(PCIDevice *pci_dev, int irq_num)
 diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
 index 066f6e1..67da307 100644
 --- a/hw/grackle_pci.c
 +++ b/hw/grackle_pci.c
 @@ -41,7 +41,7 @@
  OBJECT_CHECK(GrackleState, (obj), TYPE_GRACKLE_PCI_HOST_BRIDGE)
  
  typedef struct GrackleState {
 -PCIHostState host_state;
 +PCIHostState parent_obj;
  
  MemoryRegion pci_mmio;
  MemoryRegion pci_hole;
 diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
 index 857758e..e95e664 100644
 --- a/hw/gt64xxx.c
 +++ b/hw/gt64xxx.c
 @@ -235,7 +235,7 @@
  OBJECT_CHECK(GT64120State, (obj), TYPE_GT64120_PCI_HOST_BRIDGE)
  
  typedef struct GT64120State {
 -PCIHostState pci;
 +PCIHostState parent_obj;
  
  uint32_t regs[GT_REGS];
  PCI_MAPPING_ENTRY(PCI0IO);
 @@ -315,6 +315,7 @@ static void gt64120_writel (void *opaque, 
 target_phys_addr_t addr,
  uint64_t val, unsigned size)
  {
  GT64120State *s = opaque;
 +PCIHostState *phb = PCI_HOST_BRIDGE(s);
  uint32_t saddr;
  
  if (!(s-regs[GT_CPU]  0x1000))
 @@ -535,13 +536,15 @@ static void gt64120_writel (void *opaque, 
 target_phys_addr_t addr,
  /* not implemented */
  break;
  case GT_PCI0_CFGADDR:
 -s-pci.config_reg = val  0x80fc;
 +phb-config_reg = val  0x80fc;
  break;
  case GT_PCI0_CFGDATA:
 -if (!(s-regs[GT_PCI0_CMD]  1)  (s-pci.config_reg  0x00fff800))
 +if (!(s-regs[GT_PCI0_CMD]  1)  (phb-config_reg  0x00fff800)) {
  val = bswap32(val);
 -if (s-pci.config_reg  (1u  31))
 -pci_data_write(s-pci.bus, s-pci.config_reg, val, 4);
 +}
 +if (phb-config_reg  (1u  31)) {
 +pci_data_write(phb-bus, phb-config_reg, val, 4);
 +}
  break;
  
  /* Interrupts */
 @@ -594,6 +597,7 @@ static uint64_t gt64120_readl (void *opaque,
 target_phys_addr_t addr, unsigned size)
  {
  GT64120State *s = opaque;
 +PCIHostState *phb = PCI_HOST_BRIDGE(s);
  uint32_t val;
  uint32_t saddr;
  
 @@ -775,15 +779,17 @@ static uint64_t gt64120_readl (void *opaque,
  
  /* PCI Internal */
  case GT_PCI0_CFGADDR:
 -val = s-pci.config_reg;
 +val = phb-config_reg;
  break;
  case GT_PCI0_CFGDATA:
 -if (!(s-pci.config_reg  (1  31)))
 +if (!(phb-config_reg  (1  31))) {
  val = 0x;
 -else
 -val = pci_data_read(s-pci.bus, s-pci.config_reg, 4);
 -if (!(s-regs[GT_PCI0_CMD]  1)  (s-pci.config_reg  0x00fff800))
 +} else {
 +val = pci_data_read(phb-bus, phb-config_reg, 4);
 +}
 +if (!(s-regs[GT_PCI0_CMD]  1)  (phb-config_reg  0x00fff800)) {
  val = bswap32(val);
 +}
  break;
  
  case GT_PCI0_CMD:
 diff --git a/hw/piix_pci.c b/hw/piix_pci.c
 index 04ceccf..537fc19 100644
 --- a/hw/piix_pci.c
 +++ b/hw/piix_pci.c
 @@ -36,7 +36,9 @@
   * http://download.intel.com/design/chipsets/datashts/29054901.pdf
   */
  
 -typedef PCIHostState I440FXState;
 +typedef struct I440FXState {
 +PCIHostState parent_obj;
 +} I440FXState;
  
  #define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
  #define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
 @@ -274,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
  dev = qdev_create(NULL, i440FX-pcihost);
  s = PCI_HOST_BRIDGE(dev);
  s-address_space =

[Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges

2012-08-01 Thread Andreas Färber 
Uglify the parent field to enforce QOM-style access via casts.
Don't just typedef PCIHostState, either use it directly or embed it.

Signed-off-by: Andreas Färber afaer...@suse.de
---
 hw/alpha_typhoon.c |2 +-
 hw/dec_pci.c   |2 +-
 hw/grackle_pci.c   |2 +-
 hw/gt64xxx.c   |   26 --
 hw/piix_pci.c  |6 --
 hw/ppc4xx_pci.c|8 +---
 hw/ppce500_pci.c   |2 +-
 hw/prep_pci.c  |8 +---
 hw/spapr_pci.c |   12 +++-
 hw/spapr_pci.h |2 +-
 hw/unin_pci.c  |   14 +++---
 11 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
index 7667412..b7cf4e2 100644
--- a/hw/alpha_typhoon.c
+++ b/hw/alpha_typhoon.c
@@ -46,7 +46,7 @@ typedef struct TyphoonPchip {
 OBJECT_CHECK(TyphoonState, (obj), TYPE_TYPHOON_PCI_HOST_BRIDGE)
 
 typedef struct TyphoonState {
-PCIHostState host;
+PCIHostState parent_obj;
 
 TyphoonCchip cchip;
 TyphoonPchip pchip;
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index de16361..c30ade3 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -43,7 +43,7 @@
 #define DEC_21154(obj) OBJECT_CHECK(DECState, (obj), TYPE_DEC_21154)
 
 typedef struct DECState {
-PCIHostState host_state;
+PCIHostState parent_obj;
 } DECState;
 
 static int dec_map_irq(PCIDevice *pci_dev, int irq_num)
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index 066f6e1..67da307 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -41,7 +41,7 @@
 OBJECT_CHECK(GrackleState, (obj), TYPE_GRACKLE_PCI_HOST_BRIDGE)
 
 typedef struct GrackleState {
-PCIHostState host_state;
+PCIHostState parent_obj;
 
 MemoryRegion pci_mmio;
 MemoryRegion pci_hole;
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index 857758e..e95e664 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -235,7 +235,7 @@
 OBJECT_CHECK(GT64120State, (obj), TYPE_GT64120_PCI_HOST_BRIDGE)
 
 typedef struct GT64120State {
-PCIHostState pci;
+PCIHostState parent_obj;
 
 uint32_t regs[GT_REGS];
 PCI_MAPPING_ENTRY(PCI0IO);
@@ -315,6 +315,7 @@ static void gt64120_writel (void *opaque, 
target_phys_addr_t addr,
 uint64_t val, unsigned size)
 {
 GT64120State *s = opaque;
+PCIHostState *phb = PCI_HOST_BRIDGE(s);
 uint32_t saddr;
 
 if (!(s-regs[GT_CPU]  0x1000))
@@ -535,13 +536,15 @@ static void gt64120_writel (void *opaque, 
target_phys_addr_t addr,
 /* not implemented */
 break;
 case GT_PCI0_CFGADDR:
-s-pci.config_reg = val  0x80fc;
+phb-config_reg = val  0x80fc;
 break;
 case GT_PCI0_CFGDATA:
-if (!(s-regs[GT_PCI0_CMD]  1)  (s-pci.config_reg  0x00fff800))
+if (!(s-regs[GT_PCI0_CMD]  1)  (phb-config_reg  0x00fff800)) {
 val = bswap32(val);
-if (s-pci.config_reg  (1u  31))
-pci_data_write(s-pci.bus, s-pci.config_reg, val, 4);
+}
+if (phb-config_reg  (1u  31)) {
+pci_data_write(phb-bus, phb-config_reg, val, 4);
+}
 break;
 
 /* Interrupts */
@@ -594,6 +597,7 @@ static uint64_t gt64120_readl (void *opaque,
target_phys_addr_t addr, unsigned size)
 {
 GT64120State *s = opaque;
+PCIHostState *phb = PCI_HOST_BRIDGE(s);
 uint32_t val;
 uint32_t saddr;
 
@@ -775,15 +779,17 @@ static uint64_t gt64120_readl (void *opaque,
 
 /* PCI Internal */
 case GT_PCI0_CFGADDR:
-val = s-pci.config_reg;
+val = phb-config_reg;
 break;
 case GT_PCI0_CFGDATA:
-if (!(s-pci.config_reg  (1  31)))
+if (!(phb-config_reg  (1  31))) {
 val = 0x;
-else
-val = pci_data_read(s-pci.bus, s-pci.config_reg, 4);
-if (!(s-regs[GT_PCI0_CMD]  1)  (s-pci.config_reg  0x00fff800))
+} else {
+val = pci_data_read(phb-bus, phb-config_reg, 4);
+}
+if (!(s-regs[GT_PCI0_CMD]  1)  (phb-config_reg  0x00fff800)) {
 val = bswap32(val);
+}
 break;
 
 case GT_PCI0_CMD:
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 04ceccf..537fc19 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -36,7 +36,9 @@
  * http://download.intel.com/design/chipsets/datashts/29054901.pdf
  */
 
-typedef PCIHostState I440FXState;
+typedef struct I440FXState {
+PCIHostState parent_obj;
+} I440FXState;
 
 #define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
 #define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
@@ -274,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
 dev = qdev_create(NULL, i440FX-pcihost);
 s = PCI_HOST_BRIDGE(dev);
 s-address_space = address_space_mem;
-b = pci_bus_new(s-busdev.qdev, NULL, pci_address_space,
+b = pci_bus_new(dev, NULL, pci_address_space,
 address_space_io, 0);
 s-bus = b;
 object_property_add_child(qdev_get_machine(), i440fx, OBJECT(dev),