Patchwork [v4,14/18] pci: Tidy up PCI host bridges

login
register
mail settings
Submitter Andreas Färber
Date July 26, 2012, 12:28 p.m.
Message ID <1343305716-5178-15-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/173435/
State New
Headers show

Comments

Andreas Färber - July 26, 2012, 12:28 p.m.
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 <afaerber@suse.de>
---
 hw/alpha_typhoon.c |    4 ++--
 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, 50 insertions(+), 36 deletions(-)
Michael S. Tsirkin - July 29, 2012, 12:22 p.m.
On Thu, Jul 26, 2012 at 02:28:32PM +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 <afaerber@suse.de>

I already said I don't see the point of this change.  Replacing
compile-time checks with runtime ones seems like a bad idea, and "host"
seems like a better name for a PCIHostState field than parent_obj - you
can guess what type it is by name as opposed to looking it up in the
structure.

You said there are follow-up patches that need this, so
let's drop it for now re-introduce when it's actually helpful.

> ---
>  hw/alpha_typhoon.c |    4 ++--
>  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, 50 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
> index 58025a3..955d628 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;
> @@ -770,7 +770,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      b = pci_register_bus(dev, "pci",
>                           typhoon_set_irq, sys_map_irq, s,
>                           &s->pchip.reg_mem, addr_space_io, 0, 64);
> -    s->host.bus = b;
> +    PCI_HOST_BRIDGE(s)->bus = b;
>  
>      /* Pchip0 PCI special/interrupt acknowledge, 0x801.F800.0000, 64MB.  */
>      memory_region_init_io(&s->pchip.reg_iack, &alpha_pci_iack_ops, b,
> 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] & 0x00001000))
> @@ -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 & 0x80fffffc;
> +        phb->config_reg = val & 0x80fffffc;
>          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 = 0xffffffff;
> -        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 0523d81..9522a27 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] */
> @@ -273,7 +275,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), NULL);
> diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
> index 5583321..a14fd42 100644
> --- a/hw/ppc4xx_pci.c
> +++ b/hw/ppc4xx_pci.c
> @@ -52,7 +52,7 @@ struct PCITargetMap {
>  #define PPC4xx_PCI_NR_PTMS 2
>  
>  struct PPC4xxPCIState {
> -    PCIHostState pci_state;
> +    PCIHostState parent_obj;
>  
>      struct PCIMasterMap pmm[PPC4xx_PCI_NR_PMMS];
>      struct PCITargetMap ptm[PPC4xx_PCI_NR_PTMS];
> @@ -96,16 +96,18 @@ static uint64_t pci4xx_cfgaddr_read(void *opaque, target_phys_addr_t addr,
>                                      unsigned size)
>  {
>      PPC4xxPCIState *ppc4xx_pci = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(ppc4xx_pci);
>  
> -    return ppc4xx_pci->pci_state.config_reg;
> +    return phb->config_reg;
>  }
>  
>  static void pci4xx_cfgaddr_write(void *opaque, target_phys_addr_t addr,
>                                    uint64_t value, unsigned size)
>  {
>      PPC4xxPCIState *ppc4xx_pci = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(ppc4xx_pci);
>  
> -    ppc4xx_pci->pci_state.config_reg = value & ~0x3;
> +    phb->config_reg = value & ~0x3;
>  }
>  
>  static const MemoryRegionOps pci4xx_cfgaddr_ops = {
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 3333967..92b1dc0 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -78,7 +78,7 @@ struct pci_inbound {
>      OBJECT_CHECK(PPCE500PCIState, (obj), TYPE_PPC_E500_PCI_HOST_BRIDGE)
>  
>  struct PPCE500PCIState {
> -    PCIHostState pci_state;
> +    PCIHostState parent_obj;
>  
>      struct pci_outbound pob[PPCE500_PCI_NR_POBS];
>      struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index 35cb9b2..cc44e61 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -34,7 +34,7 @@
>      OBJECT_CHECK(PREPPCIState, (obj), TYPE_RAVEN_PCI_HOST_BRIDGE)
>  
>  typedef struct PRePPCIState {
> -    PCIHostState host_state;
> +    PCIHostState parent_obj;
>  
>      MemoryRegion intack;
>      qemu_irq irq[4];
> @@ -60,14 +60,16 @@ static void ppc_pci_io_write(void *opaque, target_phys_addr_t addr,
>                               uint64_t val, unsigned int size)
>  {
>      PREPPCIState *s = opaque;
> -    pci_data_write(s->host_state.bus, PPC_PCIIO_config(addr), val, size);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(s);
> +    pci_data_write(phb->bus, PPC_PCIIO_config(addr), val, size);
>  }
>  
>  static uint64_t ppc_pci_io_read(void *opaque, target_phys_addr_t addr,
>                                  unsigned int size)
>  {
>      PREPPCIState *s = opaque;
> -    return pci_data_read(s->host_state.bus, PPC_PCIIO_config(addr), size);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(s);
> +    return pci_data_read(phb->bus, PPC_PCIIO_config(addr), size);
>  }
>  
>  static const MemoryRegionOps PPC_PCIIO_ops = {
> 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;
>  
>      liobn = SPAPR_PCI_BASE_LIOBN | (pci_find_domain(bus) << 16);
>      phb->dma = spapr_tce_new_dma_context(liobn, 0x40000000);
> diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
> index 06e2742..6840814 100644
> --- a/hw/spapr_pci.h
> +++ b/hw/spapr_pci.h
> @@ -33,7 +33,7 @@
>      OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
>  
>  typedef struct sPAPRPHBState {
> -    PCIHostState host_state;
> +    PCIHostState parent_obj;
>  
>      uint64_t buid;
>      char *busname;
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index 0db7c1f..d3aaa5a 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -53,7 +53,7 @@ static const int unin_irq_line[] = { 0x1b, 0x1c, 0x1d, 0x1e };
>      OBJECT_CHECK(UNINState, (obj), TYPE_U3_AGP_HOST_BRIDGE)
>  
>  typedef struct UNINState {
> -    PCIHostState host_state;
> +    PCIHostState parent_obj;
>  
>      MemoryRegion pci_mmio;
>      MemoryRegion pci_hole;
> @@ -114,22 +114,22 @@ static uint32_t unin_get_config_reg(uint32_t reg, uint32_t addr)
>  static void unin_data_write(void *opaque, target_phys_addr_t addr,
>                              uint64_t val, unsigned len)
>  {
> -    UNINState *s = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(opaque);
>      UNIN_DPRINTF("write addr %" TARGET_FMT_plx " len %d val %"PRIx64"\n",
>                   addr, len, val);
> -    pci_data_write(s->host_state.bus,
> -                   unin_get_config_reg(s->host_state.config_reg, addr),
> +    pci_data_write(phb->bus,
> +                   unin_get_config_reg(phb->config_reg, addr),
>                     val, len);
>  }
>  
>  static uint64_t unin_data_read(void *opaque, target_phys_addr_t addr,
>                                 unsigned len)
>  {
> -    UNINState *s = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(opaque);
>      uint32_t val;
>  
> -    val = pci_data_read(s->host_state.bus,
> -                        unin_get_config_reg(s->host_state.config_reg, addr),
> +    val = pci_data_read(phb->bus,
> +                        unin_get_config_reg(phb->config_reg, addr),
>                          len);
>      UNIN_DPRINTF("read addr %" TARGET_FMT_plx " len %d val %x\n",
>                   addr, len, val);
> -- 
> 1.7.7
Andreas Färber - July 29, 2012, 11:08 p.m.
Am 29.07.2012 14:22, schrieb Michael S. Tsirkin:
> On Thu, Jul 26, 2012 at 02:28:32PM +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 <afaerber@suse.de>
> 
> I already said I don't see the point of this change. [...]

You did, and still Anthony instructed me otherwise. Please discuss that
with him, thanks.

Andreas
Anthony Liguori - July 30, 2012, 4:07 p.m.
Andreas Färber <afaerber@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 <afaerber@suse.de>
> ---
>  hw/alpha_typhoon.c |    4 ++--
>  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, 50 insertions(+), 36 deletions(-)
>
> diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
> index 58025a3..955d628 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;
> @@ -770,7 +770,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      b = pci_register_bus(dev, "pci",
>                           typhoon_set_irq, sys_map_irq, s,
>                           &s->pchip.reg_mem, addr_space_io, 0, 64);
> -    s->host.bus = b;
> +    PCI_HOST_BRIDGE(s)->bus = b;

Using cast macros is a good thing but doing it like this is bad.  You
should use a temporary variable.

Regards,

Anthony Liguori

>  
>      /* Pchip0 PCI special/interrupt acknowledge, 0x801.F800.0000, 64MB.  */
>      memory_region_init_io(&s->pchip.reg_iack, &alpha_pci_iack_ops, b,
> 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] & 0x00001000))
> @@ -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 & 0x80fffffc;
> +        phb->config_reg = val & 0x80fffffc;
>          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 = 0xffffffff;
> -        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 0523d81..9522a27 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] */
> @@ -273,7 +275,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), NULL);
> diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
> index 5583321..a14fd42 100644
> --- a/hw/ppc4xx_pci.c
> +++ b/hw/ppc4xx_pci.c
> @@ -52,7 +52,7 @@ struct PCITargetMap {
>  #define PPC4xx_PCI_NR_PTMS 2
>  
>  struct PPC4xxPCIState {
> -    PCIHostState pci_state;
> +    PCIHostState parent_obj;
>  
>      struct PCIMasterMap pmm[PPC4xx_PCI_NR_PMMS];
>      struct PCITargetMap ptm[PPC4xx_PCI_NR_PTMS];
> @@ -96,16 +96,18 @@ static uint64_t pci4xx_cfgaddr_read(void *opaque, target_phys_addr_t addr,
>                                      unsigned size)
>  {
>      PPC4xxPCIState *ppc4xx_pci = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(ppc4xx_pci);
>  
> -    return ppc4xx_pci->pci_state.config_reg;
> +    return phb->config_reg;
>  }
>  
>  static void pci4xx_cfgaddr_write(void *opaque, target_phys_addr_t addr,
>                                    uint64_t value, unsigned size)
>  {
>      PPC4xxPCIState *ppc4xx_pci = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(ppc4xx_pci);
>  
> -    ppc4xx_pci->pci_state.config_reg = value & ~0x3;
> +    phb->config_reg = value & ~0x3;
>  }
>  
>  static const MemoryRegionOps pci4xx_cfgaddr_ops = {
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 3333967..92b1dc0 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -78,7 +78,7 @@ struct pci_inbound {
>      OBJECT_CHECK(PPCE500PCIState, (obj), TYPE_PPC_E500_PCI_HOST_BRIDGE)
>  
>  struct PPCE500PCIState {
> -    PCIHostState pci_state;
> +    PCIHostState parent_obj;
>  
>      struct pci_outbound pob[PPCE500_PCI_NR_POBS];
>      struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index 35cb9b2..cc44e61 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -34,7 +34,7 @@
>      OBJECT_CHECK(PREPPCIState, (obj), TYPE_RAVEN_PCI_HOST_BRIDGE)
>  
>  typedef struct PRePPCIState {
> -    PCIHostState host_state;
> +    PCIHostState parent_obj;
>  
>      MemoryRegion intack;
>      qemu_irq irq[4];
> @@ -60,14 +60,16 @@ static void ppc_pci_io_write(void *opaque, target_phys_addr_t addr,
>                               uint64_t val, unsigned int size)
>  {
>      PREPPCIState *s = opaque;
> -    pci_data_write(s->host_state.bus, PPC_PCIIO_config(addr), val, size);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(s);
> +    pci_data_write(phb->bus, PPC_PCIIO_config(addr), val, size);
>  }
>  
>  static uint64_t ppc_pci_io_read(void *opaque, target_phys_addr_t addr,
>                                  unsigned int size)
>  {
>      PREPPCIState *s = opaque;
> -    return pci_data_read(s->host_state.bus, PPC_PCIIO_config(addr), size);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(s);
> +    return pci_data_read(phb->bus, PPC_PCIIO_config(addr), size);
>  }
>  
>  static const MemoryRegionOps PPC_PCIIO_ops = {
> 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;
>  
>      liobn = SPAPR_PCI_BASE_LIOBN | (pci_find_domain(bus) << 16);
>      phb->dma = spapr_tce_new_dma_context(liobn, 0x40000000);
> diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
> index 06e2742..6840814 100644
> --- a/hw/spapr_pci.h
> +++ b/hw/spapr_pci.h
> @@ -33,7 +33,7 @@
>      OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
>  
>  typedef struct sPAPRPHBState {
> -    PCIHostState host_state;
> +    PCIHostState parent_obj;
>  
>      uint64_t buid;
>      char *busname;
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index 0db7c1f..d3aaa5a 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -53,7 +53,7 @@ static const int unin_irq_line[] = { 0x1b, 0x1c, 0x1d, 0x1e };
>      OBJECT_CHECK(UNINState, (obj), TYPE_U3_AGP_HOST_BRIDGE)
>  
>  typedef struct UNINState {
> -    PCIHostState host_state;
> +    PCIHostState parent_obj;
>  
>      MemoryRegion pci_mmio;
>      MemoryRegion pci_hole;
> @@ -114,22 +114,22 @@ static uint32_t unin_get_config_reg(uint32_t reg, uint32_t addr)
>  static void unin_data_write(void *opaque, target_phys_addr_t addr,
>                              uint64_t val, unsigned len)
>  {
> -    UNINState *s = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(opaque);
>      UNIN_DPRINTF("write addr %" TARGET_FMT_plx " len %d val %"PRIx64"\n",
>                   addr, len, val);
> -    pci_data_write(s->host_state.bus,
> -                   unin_get_config_reg(s->host_state.config_reg, addr),
> +    pci_data_write(phb->bus,
> +                   unin_get_config_reg(phb->config_reg, addr),
>                     val, len);
>  }
>  
>  static uint64_t unin_data_read(void *opaque, target_phys_addr_t addr,
>                                 unsigned len)
>  {
> -    UNINState *s = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(opaque);
>      uint32_t val;
>  
> -    val = pci_data_read(s->host_state.bus,
> -                        unin_get_config_reg(s->host_state.config_reg, addr),
> +    val = pci_data_read(phb->bus,
> +                        unin_get_config_reg(phb->config_reg, addr),
>                          len);
>      UNIN_DPRINTF("read addr %" TARGET_FMT_plx " len %d val %x\n",
>                   addr, len, val);
> -- 
> 1.7.7
Andreas Färber - July 30, 2012, 4:13 p.m.
Am 30.07.2012 18:07, schrieb Anthony Liguori:
> Andreas Färber <afaerber@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 <afaerber@suse.de>
>> ---
>>  hw/alpha_typhoon.c |    4 ++--
>>  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, 50 insertions(+), 36 deletions(-)
>>
>> diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
>> index 58025a3..955d628 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;
>> @@ -770,7 +770,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>>      b = pci_register_bus(dev, "pci",
>>                           typhoon_set_irq, sys_map_irq, s,
>>                           &s->pchip.reg_mem, addr_space_io, 0, 64);
>> -    s->host.bus = b;
>> +    PCI_HOST_BRIDGE(s)->bus = b;
> 
> Using cast macros is a good thing but doing it like this is bad.  You
> should use a temporary variable.

Sorry, seems I overlooked that in the big to-cast-or-not-to-cast thread.
That was how I wanted to change it, yes.

Andreas

Patch

diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
index 58025a3..955d628 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;
@@ -770,7 +770,7 @@  PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
     b = pci_register_bus(dev, "pci",
                          typhoon_set_irq, sys_map_irq, s,
                          &s->pchip.reg_mem, addr_space_io, 0, 64);
-    s->host.bus = b;
+    PCI_HOST_BRIDGE(s)->bus = b;
 
     /* Pchip0 PCI special/interrupt acknowledge, 0x801.F800.0000, 64MB.  */
     memory_region_init_io(&s->pchip.reg_iack, &alpha_pci_iack_ops, b,
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] & 0x00001000))
@@ -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 & 0x80fffffc;
+        phb->config_reg = val & 0x80fffffc;
         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 = 0xffffffff;
-        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 0523d81..9522a27 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] */
@@ -273,7 +275,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), NULL);
diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
index 5583321..a14fd42 100644
--- a/hw/ppc4xx_pci.c
+++ b/hw/ppc4xx_pci.c
@@ -52,7 +52,7 @@  struct PCITargetMap {
 #define PPC4xx_PCI_NR_PTMS 2
 
 struct PPC4xxPCIState {
-    PCIHostState pci_state;
+    PCIHostState parent_obj;
 
     struct PCIMasterMap pmm[PPC4xx_PCI_NR_PMMS];
     struct PCITargetMap ptm[PPC4xx_PCI_NR_PTMS];
@@ -96,16 +96,18 @@  static uint64_t pci4xx_cfgaddr_read(void *opaque, target_phys_addr_t addr,
                                     unsigned size)
 {
     PPC4xxPCIState *ppc4xx_pci = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(ppc4xx_pci);
 
-    return ppc4xx_pci->pci_state.config_reg;
+    return phb->config_reg;
 }
 
 static void pci4xx_cfgaddr_write(void *opaque, target_phys_addr_t addr,
                                   uint64_t value, unsigned size)
 {
     PPC4xxPCIState *ppc4xx_pci = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(ppc4xx_pci);
 
-    ppc4xx_pci->pci_state.config_reg = value & ~0x3;
+    phb->config_reg = value & ~0x3;
 }
 
 static const MemoryRegionOps pci4xx_cfgaddr_ops = {
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 3333967..92b1dc0 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -78,7 +78,7 @@  struct pci_inbound {
     OBJECT_CHECK(PPCE500PCIState, (obj), TYPE_PPC_E500_PCI_HOST_BRIDGE)
 
 struct PPCE500PCIState {
-    PCIHostState pci_state;
+    PCIHostState parent_obj;
 
     struct pci_outbound pob[PPCE500_PCI_NR_POBS];
     struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
diff --git a/hw/prep_pci.c b/hw/prep_pci.c
index 35cb9b2..cc44e61 100644
--- a/hw/prep_pci.c
+++ b/hw/prep_pci.c
@@ -34,7 +34,7 @@ 
     OBJECT_CHECK(PREPPCIState, (obj), TYPE_RAVEN_PCI_HOST_BRIDGE)
 
 typedef struct PRePPCIState {
-    PCIHostState host_state;
+    PCIHostState parent_obj;
 
     MemoryRegion intack;
     qemu_irq irq[4];
@@ -60,14 +60,16 @@  static void ppc_pci_io_write(void *opaque, target_phys_addr_t addr,
                              uint64_t val, unsigned int size)
 {
     PREPPCIState *s = opaque;
-    pci_data_write(s->host_state.bus, PPC_PCIIO_config(addr), val, size);
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
+    pci_data_write(phb->bus, PPC_PCIIO_config(addr), val, size);
 }
 
 static uint64_t ppc_pci_io_read(void *opaque, target_phys_addr_t addr,
                                 unsigned int size)
 {
     PREPPCIState *s = opaque;
-    return pci_data_read(s->host_state.bus, PPC_PCIIO_config(addr), size);
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
+    return pci_data_read(phb->bus, PPC_PCIIO_config(addr), size);
 }
 
 static const MemoryRegionOps PPC_PCIIO_ops = {
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;
 
     liobn = SPAPR_PCI_BASE_LIOBN | (pci_find_domain(bus) << 16);
     phb->dma = spapr_tce_new_dma_context(liobn, 0x40000000);
diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
index 06e2742..6840814 100644
--- a/hw/spapr_pci.h
+++ b/hw/spapr_pci.h
@@ -33,7 +33,7 @@ 
     OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
 
 typedef struct sPAPRPHBState {
-    PCIHostState host_state;
+    PCIHostState parent_obj;
 
     uint64_t buid;
     char *busname;
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index 0db7c1f..d3aaa5a 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -53,7 +53,7 @@  static const int unin_irq_line[] = { 0x1b, 0x1c, 0x1d, 0x1e };
     OBJECT_CHECK(UNINState, (obj), TYPE_U3_AGP_HOST_BRIDGE)
 
 typedef struct UNINState {
-    PCIHostState host_state;
+    PCIHostState parent_obj;
 
     MemoryRegion pci_mmio;
     MemoryRegion pci_hole;
@@ -114,22 +114,22 @@  static uint32_t unin_get_config_reg(uint32_t reg, uint32_t addr)
 static void unin_data_write(void *opaque, target_phys_addr_t addr,
                             uint64_t val, unsigned len)
 {
-    UNINState *s = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(opaque);
     UNIN_DPRINTF("write addr %" TARGET_FMT_plx " len %d val %"PRIx64"\n",
                  addr, len, val);
-    pci_data_write(s->host_state.bus,
-                   unin_get_config_reg(s->host_state.config_reg, addr),
+    pci_data_write(phb->bus,
+                   unin_get_config_reg(phb->config_reg, addr),
                    val, len);
 }
 
 static uint64_t unin_data_read(void *opaque, target_phys_addr_t addr,
                                unsigned len)
 {
-    UNINState *s = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(opaque);
     uint32_t val;
 
-    val = pci_data_read(s->host_state.bus,
-                        unin_get_config_reg(s->host_state.config_reg, addr),
+    val = pci_data_read(phb->bus,
+                        unin_get_config_reg(phb->config_reg, addr),
                         len);
     UNIN_DPRINTF("read addr %" TARGET_FMT_plx " len %d val %x\n",
                  addr, len, val);