[Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup

[Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup

[Eduardo Habkost]

I've noticed that pci_bus_new*() and pci_register_bus() require
'parent' to be a PCI_HOST_BRIDGE object, but this is not clear
from the function signatures.

This series implements two changes in the PCI code:

1) Replace DeviceState with PCIHostState on functions that
   already require a PCI_HOST_BRIDGE argument. Makes the
   functions harder to misuse.
2) Move PCIHostState::bus initialization inside pci_bus_new*(),
   to avoid code duplication and make sure the field will be
   always initialized consistently.

Eduardo Habkost (7):
  pci: Change pci_host_bus_register() parameter to PCIHostState
  pci: Change pci_bus_init() 'parent' parameter to PCIHostState
  pci: Change pci_bus_new*() parameter to PCIHostState
  pci: Change pci_register_bus() 'parent' parameter to PCIHostState
  pci: Set phb->bus inside pci_register_bus()
  pci: Set phb->bus inside pci_bus_new()
  pci: Set phb->bus inside pci_bus_new_inplace()

 include/hw/pci/pci.h                | 17 ++++++++--------
 hw/alpha/typhoon.c                  | 10 +++++-----
 hw/mips/gt64xxx_pci.c               |  9 +++------
 hw/pci-bridge/pci_expander_bridge.c | 15 +++++++-------
 hw/pci-host/apb.c                   |  7 ++-----
 hw/pci-host/bonito.c                |  7 +++----
 hw/pci-host/gpex.c                  |  5 ++---
 hw/pci-host/grackle.c               |  9 ++-------
 hw/pci-host/piix.c                  |  3 +--
 hw/pci-host/ppce500.c               |  8 ++++----
 hw/pci-host/prep.c                  |  4 +---
 hw/pci-host/q35.c                   |  6 +++---
 hw/pci-host/uninorth.c              | 18 ++++++-----------
 hw/pci-host/versatile.c             |  3 +--
 hw/pci-host/xilinx-pcie.c           |  6 +++---
 hw/pci/pci.c                        | 40 ++++++++++++++++++-------------------
 hw/ppc/ppc4xx_pci.c                 |  8 ++++----
 hw/ppc/spapr_pci.c                  | 10 +++++-----
 hw/s390x/s390-pci-bus.c             | 10 +++++-----
 hw/sh4/sh_pci.c                     |  9 +++------
 20 files changed, 89 insertions(+), 115 deletions(-)

-- 
2.11.0.259.g40922b1

[Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState

[Eduardo Habkost]

The function requires a PCI_HOST_BRIDGE object, so change the parameter
type to reflect that.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pci/pci.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 259483b1c0..25118fb91d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -312,11 +312,9 @@ static void pcibus_reset(BusState *qbus)
     }
 }
 
-static void pci_host_bus_register(DeviceState *host)
+static void pci_host_bus_register(PCIHostState *phb)
 {
-    PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
-
-    QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
+    QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
 }
 
 PCIBus *pci_find_primary_bus(void)
@@ -377,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
     /* host bridge */
     QLIST_INIT(&bus->child);
 
-    pci_host_bus_register(parent);
+    pci_host_bus_register(PCI_HOST_BRIDGE(host));
 }
 
 bool pci_bus_is_express(PCIBus *bus)
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

On Mon, Apr 17, 2017 at 06:59:10PM -0300, Eduardo Habkost wrote:
> The function requires a PCI_HOST_BRIDGE object, so change the parameter
> type to reflect that.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/pci/pci.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 259483b1c0..25118fb91d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -312,11 +312,9 @@ static void pcibus_reset(BusState *qbus)
>      }
>  }
>  
> -static void pci_host_bus_register(DeviceState *host)
> +static void pci_host_bus_register(PCIHostState *phb)
>  {
> -    PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> -
> -    QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> +    QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
>  }
>  
>  PCIBus *pci_find_primary_bus(void)
> @@ -377,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>      /* host bridge */
>      QLIST_INIT(&bus->child);
>  
> -    pci_host_bus_register(parent);
> +    pci_host_bus_register(PCI_HOST_BRIDGE(host));
>  }
>  
>  bool pci_bus_is_express(PCIBus *bus)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState

[Marcel Apfelbaum]

On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> The function requires a PCI_HOST_BRIDGE object, so change the parameter
> type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/pci/pci.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 259483b1c0..25118fb91d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -312,11 +312,9 @@ static void pcibus_reset(BusState *qbus)
>      }
>  }
>
> -static void pci_host_bus_register(DeviceState *host)
> +static void pci_host_bus_register(PCIHostState *phb)
>  {
> -    PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> -
> -    QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> +    QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
>  }
>
>  PCIBus *pci_find_primary_bus(void)
> @@ -377,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>      /* host bridge */
>      QLIST_INIT(&bus->child);
>
> -    pci_host_bus_register(parent);
> +    pci_host_bus_register(PCI_HOST_BRIDGE(host));
>  }
>
>  bool pci_bus_is_express(PCIBus *bus)
>


Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

[Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState

[Eduardo Habkost]

pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
so change the parameter type to reflect that.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pci/pci.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 25118fb91d..d9535c0bdc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
     return rootbus->qbus.name;
 }
 
-static void pci_bus_init(PCIBus *bus, DeviceState *parent,
+static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
                          MemoryRegion *address_space_mem,
                          MemoryRegion *address_space_io,
                          uint8_t devfn_min)
@@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
     /* host bridge */
     QLIST_INIT(&bus->child);
 
-    pci_host_bus_register(PCI_HOST_BRIDGE(host));
+    pci_host_bus_register(phb);
 }
 
 bool pci_bus_is_express(PCIBus *bus)
@@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
                          MemoryRegion *address_space_io,
                          uint8_t devfn_min, const char *typename)
 {
+    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
     qbus_create_inplace(bus, bus_size, typename, parent, name);
-    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
+    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
 }
 
 PCIBus *pci_bus_new(DeviceState *parent, const char *name,
@@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
                     MemoryRegion *address_space_io,
                     uint8_t devfn_min, const char *typename)
 {
+    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
     PCIBus *bus;
 
     bus = PCI_BUS(qbus_create(typename, parent, name));
-    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
+    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
     return bus;
 }
 
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2922 bytes --]

On Mon, Apr 17, 2017 at 06:59:11PM -0300, Eduardo Habkost wrote:
> pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
> so change the parameter type to reflect that.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

I had to do some looking to convince myself this wouldn't break P2P
bridges.

I wonder if we should rename pci_bus_init() / pci_bus_new() to make it
clearer that they're about creating a whole new PCI domain, and not
for a new bus within an existing domain.

> ---
>  hw/pci/pci.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 25118fb91d..d9535c0bdc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
>      return rootbus->qbus.name;
>  }
>  
> -static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> +static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
>                           MemoryRegion *address_space_mem,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min)
> @@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>      /* host bridge */
>      QLIST_INIT(&bus->child);
>  
> -    pci_host_bus_register(PCI_HOST_BRIDGE(host));
> +    pci_host_bus_register(phb);
>  }
>  
>  bool pci_bus_is_express(PCIBus *bus)
> @@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, const char *typename)
>  {
> +    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>      qbus_create_inplace(bus, bus_size, typename, parent, name);
> -    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> +    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>  }
>  
>  PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> @@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename)
>  {
> +    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>      PCIBus *bus;
>  
>      bus = PCI_BUS(qbus_create(typename, parent, name));
> -    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> +    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>      return bus;
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState

[Eduardo Habkost]

On Tue, Apr 18, 2017 at 01:51:02PM +1000, David Gibson wrote:
> On Mon, Apr 17, 2017 at 06:59:11PM -0300, Eduardo Habkost wrote:
> > pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
> > so change the parameter type to reflect that.
> > 
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> I had to do some looking to convince myself this wouldn't break P2P
> bridges.
> 
> I wonder if we should rename pci_bus_init() / pci_bus_new() to make it
> clearer that they're about creating a whole new PCI domain, and not
> for a new bus within an existing domain.

Good point. The current naming is confusing. I only knew the
change was safe because pci_bus_new() would crash if 'parent'
wasn't a PCI_HOST_BRIDGE. But I took some time to find the other
places where PCI buses could be created, and I'm not sure yet if
I found all of them.

Is there any other code that creates TYPE_PCI_BUS objects, except
for the ones touched by this series and pci_bridge_initfn()?

> 
> > ---
> >  hw/pci/pci.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 25118fb91d..d9535c0bdc 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
> >      return rootbus->qbus.name;
> >  }
> >  
> > -static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> > +static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
> >                           MemoryRegion *address_space_mem,
> >                           MemoryRegion *address_space_io,
> >                           uint8_t devfn_min)
> > @@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> >      /* host bridge */
> >      QLIST_INIT(&bus->child);
> >  
> > -    pci_host_bus_register(PCI_HOST_BRIDGE(host));
> > +    pci_host_bus_register(phb);
> >  }
> >  
> >  bool pci_bus_is_express(PCIBus *bus)
> > @@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> >                           MemoryRegion *address_space_io,
> >                           uint8_t devfn_min, const char *typename)
> >  {
> > +    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> >      qbus_create_inplace(bus, bus_size, typename, parent, name);
> > -    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> > +    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> >  }
> >  
> >  PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> > @@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> >                      MemoryRegion *address_space_io,
> >                      uint8_t devfn_min, const char *typename)
> >  {
> > +    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> >      PCIBus *bus;
> >  
> >      bus = PCI_BUS(qbus_create(typename, parent, name));
> > -    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> > +    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> >      return bus;
> >  }
> >  
> 
> -- 
> David Gibson			| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> 				| _way_ _around_!
> http://www.ozlabs.org/~dgibson



-- 
Eduardo

Re: [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState

[Marcel Apfelbaum]

On 04/18/2017 02:48 PM, Eduardo Habkost wrote:
> On Tue, Apr 18, 2017 at 01:51:02PM +1000, David Gibson wrote:
>> On Mon, Apr 17, 2017 at 06:59:11PM -0300, Eduardo Habkost wrote:
>>> pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
>>> so change the parameter type to reflect that.
>>>
>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>> Cc: Marcel Apfelbaum <marcel@redhat.com>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>
>> I had to do some looking to convince myself this wouldn't break P2P
>> bridges.
>>
>> I wonder if we should rename pci_bus_init() / pci_bus_new() to make it
>> clearer that they're about creating a whole new PCI domain, and not
>> for a new bus within an existing domain.
>
> Good point. The current naming is confusing. I only knew the
> change was safe because pci_bus_new() would crash if 'parent'
> wasn't a PCI_HOST_BRIDGE. But I took some time to find the other
> places where PCI buses could be created, and I'm not sure yet if
> I found all of them.
>
> Is there any other code that creates TYPE_PCI_BUS objects, except
> for the ones touched by this series and pci_bridge_initfn()?
>

pxb and pxb-pcie devices have a built-in PCI/PCIe bus,
but I saw them handled by another patch.

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>


Thanks,
Marcel

>>
>>> ---
>>>  hw/pci/pci.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>>> index 25118fb91d..d9535c0bdc 100644
>>> --- a/hw/pci/pci.c
>>> +++ b/hw/pci/pci.c
>>> @@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
>>>      return rootbus->qbus.name;
>>>  }
>>>
>>> -static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>>> +static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
>>>                           MemoryRegion *address_space_mem,
>>>                           MemoryRegion *address_space_io,
>>>                           uint8_t devfn_min)
>>> @@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>>>      /* host bridge */
>>>      QLIST_INIT(&bus->child);
>>>
>>> -    pci_host_bus_register(PCI_HOST_BRIDGE(host));
>>> +    pci_host_bus_register(phb);
>>>  }
>>>
>>>  bool pci_bus_is_express(PCIBus *bus)
>>> @@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
>>>                           MemoryRegion *address_space_io,
>>>                           uint8_t devfn_min, const char *typename)
>>>  {
>>> +    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>>>      qbus_create_inplace(bus, bus_size, typename, parent, name);
>>> -    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
>>> +    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>>>  }
>>>
>>>  PCIBus *pci_bus_new(DeviceState *parent, const char *name,
>>> @@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
>>>                      MemoryRegion *address_space_io,
>>>                      uint8_t devfn_min, const char *typename)
>>>  {
>>> +    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>>>      PCIBus *bus;
>>>
>>>      bus = PCI_BUS(qbus_create(typename, parent, name));
>>> -    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
>>> +    pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>>>      return bus;
>>>  }
>>>
>>
>> --
>> David Gibson			| I'll have my music baroque, and my code
>> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
>> 				| _way_ _around_!
>> http://www.ozlabs.org/~dgibson
>
>
>

[Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState

[Eduardo Habkost]

The pci_bus_new*() functions already require the 'parent' argument to be
a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-ppc@nongnu.org
Cc: qemu-arm@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/pci/pci.h                |  5 +++--
 hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
 hw/pci-host/piix.c                  |  2 +-
 hw/pci-host/prep.c                  |  2 +-
 hw/pci-host/q35.c                   |  2 +-
 hw/pci-host/versatile.c             |  2 +-
 hw/pci/pci.c                        | 13 ++++++-------
 7 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index a37a2d5cb6..2242aa25eb 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -393,12 +393,13 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
 
 bool pci_bus_is_express(PCIBus *bus);
 bool pci_bus_is_root(PCIBus *bus);
-void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
+void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
+                         PCIHostState *phb,
                          const char *name,
                          MemoryRegion *address_space_mem,
                          MemoryRegion *address_space_io,
                          uint8_t devfn_min, const char *typename);
-PCIBus *pci_bus_new(DeviceState *parent, const char *name,
+PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
                     MemoryRegion *address_space_mem,
                     MemoryRegion *address_space_io,
                     uint8_t devfn_min, const char *typename);
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 6ac187fa32..39d29d2230 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
 static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
 {
     PXBDev *pxb = convert_to_pxb(dev);
-    DeviceState *ds, *bds = NULL;
+    DeviceState *bds = NULL;
+    PCIHostState *phb;
     PCIBus *bus;
     const char *dev_name = NULL;
     Error *local_err = NULL;
@@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
         dev_name = dev->qdev.id;
     }
 
-    ds = qdev_create(NULL, TYPE_PXB_HOST);
+    phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
     if (pcie) {
-        bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
+        bus = pci_bus_new(phb, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
     } else {
-        bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
+        bus = pci_bus_new(phb, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
         bds = qdev_create(BUS(bus), "pci-bridge");
         bds->id = dev_name;
         qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr);
@@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
     bus->address_space_io = dev->bus->address_space_io;
     bus->map_irq = pxb_map_irq_fn;
 
-    PCI_HOST_BRIDGE(ds)->bus = bus;
+    phb->bus = bus;
 
     pxb_register_bus(dev, bus, &local_err);
     if (local_err) {
@@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
         goto err_register_bus;
     }
 
-    qdev_init_nofail(ds);
+    qdev_init_nofail(DEVICE(phb));
     if (bds) {
         qdev_init_nofail(bds);
     }
@@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
 err_register_bus:
     object_unref(OBJECT(bds));
     object_unparent(OBJECT(bus));
-    object_unref(OBJECT(ds));
+    object_unref(OBJECT(phb));
 }
 
 static void pxb_dev_realize(PCIDevice *dev, Error **errp)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9218aa952..91fec05b38 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -340,7 +340,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
 
     dev = qdev_create(NULL, host_type);
     s = PCI_HOST_BRIDGE(dev);
-    b = pci_bus_new(dev, NULL, pci_address_space,
+    b = pci_bus_new(s, NULL, pci_address_space,
                     address_space_io, 0, TYPE_PCI_BUS);
     s->bus = b;
     object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 260a119a9e..2e2cd267f4 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -269,7 +269,7 @@ static void raven_pcihost_initfn(Object *obj)
     memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
                                         &s->pci_io_non_contiguous, 1);
     memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
-    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
+    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
                         &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
 
     /* Bus master address space */
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 344f77b10c..860b47a1ba 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -49,7 +49,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
-    pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
+    pci->bus = pci_bus_new(pci, "pcie.0",
                            s->mch.pci_address_space, s->mch.address_space_io,
                            0, TYPE_PCIE_BUS);
     PC_MACHINE(qdev_get_machine())->bus = pci->bus;
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 467cbb9cb8..24ef87610b 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
     memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
     memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
 
-    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), "pci",
+    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
                         &s->pci_mem_space, &s->pci_io_space,
                         PCI_DEVFN(11, 0), TYPE_PCI_BUS);
     h->bus = &s->pci_bus;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d9535c0bdc..f4488b46fc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -388,26 +388,25 @@ bool pci_bus_is_root(PCIBus *bus)
     return PCI_BUS_GET_CLASS(bus)->is_root(bus);
 }
 
-void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
+void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
+                         PCIHostState *phb,
                          const char *name,
                          MemoryRegion *address_space_mem,
                          MemoryRegion *address_space_io,
                          uint8_t devfn_min, const char *typename)
 {
-    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
-    qbus_create_inplace(bus, bus_size, typename, parent, name);
+    qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
     pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
 }
 
-PCIBus *pci_bus_new(DeviceState *parent, const char *name,
+PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
                     MemoryRegion *address_space_mem,
                     MemoryRegion *address_space_io,
                     uint8_t devfn_min, const char *typename)
 {
-    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
     PCIBus *bus;
 
-    bus = PCI_BUS(qbus_create(typename, parent, name));
+    bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
     pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
     return bus;
 }
@@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
 {
     PCIBus *bus;
 
-    bus = pci_bus_new(parent, name, address_space_mem,
+    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
                       address_space_io, devfn_min, typename);
     pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
     return bus;
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 9306 bytes --]

On Mon, Apr 17, 2017 at 06:59:12PM -0300, Eduardo Habkost wrote:
> The pci_bus_new*() functions already require the 'parent' argument to be
> a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/pci/pci.h                |  5 +++--
>  hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
>  hw/pci-host/piix.c                  |  2 +-
>  hw/pci-host/prep.c                  |  2 +-
>  hw/pci-host/q35.c                   |  2 +-
>  hw/pci-host/versatile.c             |  2 +-
>  hw/pci/pci.c                        | 13 ++++++-------
>  7 files changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index a37a2d5cb6..2242aa25eb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -393,12 +393,13 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
>  
>  bool pci_bus_is_express(PCIBus *bus);
>  bool pci_bus_is_root(PCIBus *bus);
> -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> +                         PCIHostState *phb,
>                           const char *name,
>                           MemoryRegion *address_space_mem,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, const char *typename);
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename);
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 6ac187fa32..39d29d2230 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
>  static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>  {
>      PXBDev *pxb = convert_to_pxb(dev);
> -    DeviceState *ds, *bds = NULL;
> +    DeviceState *bds = NULL;
> +    PCIHostState *phb;
>      PCIBus *bus;
>      const char *dev_name = NULL;
>      Error *local_err = NULL;
> @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>          dev_name = dev->qdev.id;
>      }
>  
> -    ds = qdev_create(NULL, TYPE_PXB_HOST);
> +    phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
>      if (pcie) {
> -        bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> +        bus = pci_bus_new(phb, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
>      } else {
> -        bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
> +        bus = pci_bus_new(phb, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
>          bds = qdev_create(BUS(bus), "pci-bridge");
>          bds->id = dev_name;
>          qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr);
> @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>      bus->address_space_io = dev->bus->address_space_io;
>      bus->map_irq = pxb_map_irq_fn;
>  
> -    PCI_HOST_BRIDGE(ds)->bus = bus;
> +    phb->bus = bus;
>  
>      pxb_register_bus(dev, bus, &local_err);
>      if (local_err) {
> @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>          goto err_register_bus;
>      }
>  
> -    qdev_init_nofail(ds);
> +    qdev_init_nofail(DEVICE(phb));
>      if (bds) {
>          qdev_init_nofail(bds);
>      }
> @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>  err_register_bus:
>      object_unref(OBJECT(bds));
>      object_unparent(OBJECT(bus));
> -    object_unref(OBJECT(ds));
> +    object_unref(OBJECT(phb));
>  }
>  
>  static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9218aa952..91fec05b38 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -340,7 +340,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>  
>      dev = qdev_create(NULL, host_type);
>      s = PCI_HOST_BRIDGE(dev);
> -    b = pci_bus_new(dev, NULL, pci_address_space,
> +    b = pci_bus_new(s, NULL, pci_address_space,
>                      address_space_io, 0, TYPE_PCI_BUS);
>      s->bus = b;
>      object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 260a119a9e..2e2cd267f4 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -269,7 +269,7 @@ static void raven_pcihost_initfn(Object *obj)
>      memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
>                                          &s->pci_io_non_contiguous, 1);
>      memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
> -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
>                          &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
>  
>      /* Bus master address space */
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 344f77b10c..860b47a1ba 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,7 +49,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>      sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
>      sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
>  
> -    pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
> +    pci->bus = pci_bus_new(pci, "pcie.0",
>                             s->mch.pci_address_space, s->mch.address_space_io,
>                             0, TYPE_PCIE_BUS);
>      PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 467cbb9cb8..24ef87610b 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
>      memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
>      memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
>  
> -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), "pci",
> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
>                          &s->pci_mem_space, &s->pci_io_space,
>                          PCI_DEVFN(11, 0), TYPE_PCI_BUS);
>      h->bus = &s->pci_bus;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d9535c0bdc..f4488b46fc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -388,26 +388,25 @@ bool pci_bus_is_root(PCIBus *bus)
>      return PCI_BUS_GET_CLASS(bus)->is_root(bus);
>  }
>  
> -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> +                         PCIHostState *phb,
>                           const char *name,
>                           MemoryRegion *address_space_mem,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, const char *typename)
>  {
> -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> -    qbus_create_inplace(bus, bus_size, typename, parent, name);
> +    qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>  }
>  
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename)
>  {
> -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>      PCIBus *bus;
>  
> -    bus = PCI_BUS(qbus_create(typename, parent, name));
> +    bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>      return bus;
>  }
> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  {
>      PCIBus *bus;
>  
> -    bus = pci_bus_new(parent, name, address_space_mem,
> +    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
>      return bus;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState

[Marcel Apfelbaum]

On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> The pci_bus_new*() functions already require the 'parent' argument to be
> a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/hw/pci/pci.h                |  5 +++--
>  hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
>  hw/pci-host/piix.c                  |  2 +-
>  hw/pci-host/prep.c                  |  2 +-
>  hw/pci-host/q35.c                   |  2 +-
>  hw/pci-host/versatile.c             |  2 +-
>  hw/pci/pci.c                        | 13 ++++++-------
>  7 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index a37a2d5cb6..2242aa25eb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -393,12 +393,13 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
>
>  bool pci_bus_is_express(PCIBus *bus);
>  bool pci_bus_is_root(PCIBus *bus);
> -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> +                         PCIHostState *phb,
>                           const char *name,
>                           MemoryRegion *address_space_mem,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, const char *typename);
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename);
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 6ac187fa32..39d29d2230 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
>  static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>  {
>      PXBDev *pxb = convert_to_pxb(dev);
> -    DeviceState *ds, *bds = NULL;
> +    DeviceState *bds = NULL;
> +    PCIHostState *phb;
>      PCIBus *bus;
>      const char *dev_name = NULL;
>      Error *local_err = NULL;
> @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>          dev_name = dev->qdev.id;
>      }
>
> -    ds = qdev_create(NULL, TYPE_PXB_HOST);
> +    phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
>      if (pcie) {
> -        bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> +        bus = pci_bus_new(phb, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
>      } else {
> -        bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
> +        bus = pci_bus_new(phb, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
>          bds = qdev_create(BUS(bus), "pci-bridge");
>          bds->id = dev_name;
>          qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr);
> @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>      bus->address_space_io = dev->bus->address_space_io;
>      bus->map_irq = pxb_map_irq_fn;
>
> -    PCI_HOST_BRIDGE(ds)->bus = bus;
> +    phb->bus = bus;
>
>      pxb_register_bus(dev, bus, &local_err);
>      if (local_err) {
> @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>          goto err_register_bus;
>      }
>
> -    qdev_init_nofail(ds);
> +    qdev_init_nofail(DEVICE(phb));
>      if (bds) {
>          qdev_init_nofail(bds);
>      }
> @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>  err_register_bus:
>      object_unref(OBJECT(bds));
>      object_unparent(OBJECT(bus));
> -    object_unref(OBJECT(ds));
> +    object_unref(OBJECT(phb));
>  }
>
>  static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9218aa952..91fec05b38 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -340,7 +340,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>
>      dev = qdev_create(NULL, host_type);
>      s = PCI_HOST_BRIDGE(dev);
> -    b = pci_bus_new(dev, NULL, pci_address_space,
> +    b = pci_bus_new(s, NULL, pci_address_space,
>                      address_space_io, 0, TYPE_PCI_BUS);
>      s->bus = b;
>      object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 260a119a9e..2e2cd267f4 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -269,7 +269,7 @@ static void raven_pcihost_initfn(Object *obj)
>      memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
>                                          &s->pci_io_non_contiguous, 1);
>      memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
> -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
>                          &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
>
>      /* Bus master address space */
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 344f77b10c..860b47a1ba 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,7 +49,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>      sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
>      sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
>
> -    pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
> +    pci->bus = pci_bus_new(pci, "pcie.0",
>                             s->mch.pci_address_space, s->mch.address_space_io,
>                             0, TYPE_PCIE_BUS);
>      PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 467cbb9cb8..24ef87610b 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
>      memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
>      memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
>
> -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), "pci",
> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
>                          &s->pci_mem_space, &s->pci_io_space,
>                          PCI_DEVFN(11, 0), TYPE_PCI_BUS);
>      h->bus = &s->pci_bus;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d9535c0bdc..f4488b46fc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -388,26 +388,25 @@ bool pci_bus_is_root(PCIBus *bus)
>      return PCI_BUS_GET_CLASS(bus)->is_root(bus);
>  }
>
> -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> +                         PCIHostState *phb,
>                           const char *name,
>                           MemoryRegion *address_space_mem,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, const char *typename)
>  {
> -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> -    qbus_create_inplace(bus, bus_size, typename, parent, name);
> +    qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>  }
>
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename)
>  {
> -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>      PCIBus *bus;
>
> -    bus = PCI_BUS(qbus_create(typename, parent, name));
> +    bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>      return bus;
>  }
> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  {
>      PCIBus *bus;
>
> -    bus = pci_bus_new(parent, name, address_space_mem,
> +    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,

If you request the parent to be PCI_HOST_BRIDGE, why don't you change also the signature of
pci_register_bus ?

Thanks,
Marcel

>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
>      return bus;
>

Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState

[Eduardo Habkost]

On Tue, Apr 18, 2017 at 04:27:23PM +0300, Marcel Apfelbaum wrote:
[...]
> > @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> >  {
> >      PCIBus *bus;
> > 
> > -    bus = pci_bus_new(parent, name, address_space_mem,
> > +    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> 
> If you request the parent to be PCI_HOST_BRIDGE, why don't you change also the signature of
> pci_register_bus ?

I do it on patch 4/7.

-- 
Eduardo

Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState

[Marcel Apfelbaum]

On 04/18/2017 04:30 PM, Eduardo Habkost wrote:
> On Tue, Apr 18, 2017 at 04:27:23PM +0300, Marcel Apfelbaum wrote:
> [...]
>>> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>>>  {
>>>      PCIBus *bus;
>>>
>>> -    bus = pci_bus_new(parent, name, address_space_mem,
>>> +    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
>>
>> If you request the parent to be PCI_HOST_BRIDGE, why don't you change also the signature of
>> pci_register_bus ?
>
> I do it on patch 4/7.
>

Yes, I saw it :). at least it means I am following the patches...

Thanks,
Marcel

Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState

[Michael S. Tsirkin]

On Mon, Apr 17, 2017 at 06:59:12PM -0300, Eduardo Habkost wrote:
> The pci_bus_new*() functions already require the 'parent' argument to be
> a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

This function doesn't create a generic pci bus, it's only good
for creating root buses.

As we are changing all callers anyway, let's rename this to
pci_host_bus_new or pci_root_bus_new?

> ---
>  include/hw/pci/pci.h                |  5 +++--
>  hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
>  hw/pci-host/piix.c                  |  2 +-
>  hw/pci-host/prep.c                  |  2 +-
>  hw/pci-host/q35.c                   |  2 +-
>  hw/pci-host/versatile.c             |  2 +-
>  hw/pci/pci.c                        | 13 ++++++-------
>  7 files changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index a37a2d5cb6..2242aa25eb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -393,12 +393,13 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
>  
>  bool pci_bus_is_express(PCIBus *bus);
>  bool pci_bus_is_root(PCIBus *bus);
> -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> +                         PCIHostState *phb,
>                           const char *name,
>                           MemoryRegion *address_space_mem,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, const char *typename);
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename);
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 6ac187fa32..39d29d2230 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
>  static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>  {
>      PXBDev *pxb = convert_to_pxb(dev);
> -    DeviceState *ds, *bds = NULL;
> +    DeviceState *bds = NULL;
> +    PCIHostState *phb;
>      PCIBus *bus;
>      const char *dev_name = NULL;
>      Error *local_err = NULL;
> @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>          dev_name = dev->qdev.id;
>      }
>  
> -    ds = qdev_create(NULL, TYPE_PXB_HOST);
> +    phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
>      if (pcie) {
> -        bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> +        bus = pci_bus_new(phb, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
>      } else {
> -        bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
> +        bus = pci_bus_new(phb, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
>          bds = qdev_create(BUS(bus), "pci-bridge");
>          bds->id = dev_name;
>          qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr);
> @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>      bus->address_space_io = dev->bus->address_space_io;
>      bus->map_irq = pxb_map_irq_fn;
>  
> -    PCI_HOST_BRIDGE(ds)->bus = bus;
> +    phb->bus = bus;
>  
>      pxb_register_bus(dev, bus, &local_err);
>      if (local_err) {
> @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>          goto err_register_bus;
>      }
>  
> -    qdev_init_nofail(ds);
> +    qdev_init_nofail(DEVICE(phb));
>      if (bds) {
>          qdev_init_nofail(bds);
>      }
> @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>  err_register_bus:
>      object_unref(OBJECT(bds));
>      object_unparent(OBJECT(bus));
> -    object_unref(OBJECT(ds));
> +    object_unref(OBJECT(phb));
>  }
>  
>  static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9218aa952..91fec05b38 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -340,7 +340,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>  
>      dev = qdev_create(NULL, host_type);
>      s = PCI_HOST_BRIDGE(dev);
> -    b = pci_bus_new(dev, NULL, pci_address_space,
> +    b = pci_bus_new(s, NULL, pci_address_space,
>                      address_space_io, 0, TYPE_PCI_BUS);
>      s->bus = b;
>      object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 260a119a9e..2e2cd267f4 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -269,7 +269,7 @@ static void raven_pcihost_initfn(Object *obj)
>      memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
>                                          &s->pci_io_non_contiguous, 1);
>      memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
> -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
>                          &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
>  
>      /* Bus master address space */
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 344f77b10c..860b47a1ba 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,7 +49,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>      sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
>      sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
>  
> -    pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
> +    pci->bus = pci_bus_new(pci, "pcie.0",
>                             s->mch.pci_address_space, s->mch.address_space_io,
>                             0, TYPE_PCIE_BUS);
>      PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 467cbb9cb8..24ef87610b 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
>      memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
>      memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
>  
> -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), "pci",
> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
>                          &s->pci_mem_space, &s->pci_io_space,
>                          PCI_DEVFN(11, 0), TYPE_PCI_BUS);
>      h->bus = &s->pci_bus;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d9535c0bdc..f4488b46fc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -388,26 +388,25 @@ bool pci_bus_is_root(PCIBus *bus)
>      return PCI_BUS_GET_CLASS(bus)->is_root(bus);
>  }
>  
> -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> +                         PCIHostState *phb,
>                           const char *name,
>                           MemoryRegion *address_space_mem,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, const char *typename)
>  {
> -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> -    qbus_create_inplace(bus, bus_size, typename, parent, name);
> +    qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>  }
>  
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename)
>  {
> -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>      PCIBus *bus;
>  
> -    bus = PCI_BUS(qbus_create(typename, parent, name));
> +    bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>      return bus;
>  }
> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  {
>      PCIBus *bus;
>  
> -    bus = pci_bus_new(parent, name, address_space_mem,
> +    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
>      return bus;
> -- 
> 2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState

[Eduardo Habkost]

On Wed, Apr 26, 2017 at 07:16:26PM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 17, 2017 at 06:59:12PM -0300, Eduardo Habkost wrote:
> > The pci_bus_new*() functions already require the 'parent' argument to be
> > a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
> > 
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel@redhat.com>
> > Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: qemu-ppc@nongnu.org
> > Cc: qemu-arm@nongnu.org
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> This function doesn't create a generic pci bus, it's only good
> for creating root buses.
> 
> As we are changing all callers anyway, let's rename this to
> pci_host_bus_new or pci_root_bus_new?

Something similar was already done on v2[1]. But based on
additional feedback on v2, I plan to make pci_bus_new() more
generic, moving the PCIHostState-specific initialization to
pci_host_bus_init*() wrappers.

[1] The new function names were:
* pci_host_bus_init() (replacing pci_bus_new())
* pci_host_bus_init_inplace() (replacing pci_bus_new_inplace())
* pci_host_bus_init_irqs() (replacing pci_register_bus())


> 
> > ---
> >  include/hw/pci/pci.h                |  5 +++--
> >  hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
> >  hw/pci-host/piix.c                  |  2 +-
> >  hw/pci-host/prep.c                  |  2 +-
> >  hw/pci-host/q35.c                   |  2 +-
> >  hw/pci-host/versatile.c             |  2 +-
> >  hw/pci/pci.c                        | 13 ++++++-------
> >  7 files changed, 21 insertions(+), 20 deletions(-)
> > 
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index a37a2d5cb6..2242aa25eb 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -393,12 +393,13 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
> >  
> >  bool pci_bus_is_express(PCIBus *bus);
> >  bool pci_bus_is_root(PCIBus *bus);
> > -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> > +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> > +                         PCIHostState *phb,
> >                           const char *name,
> >                           MemoryRegion *address_space_mem,
> >                           MemoryRegion *address_space_io,
> >                           uint8_t devfn_min, const char *typename);
> > -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> > +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> >                      MemoryRegion *address_space_mem,
> >                      MemoryRegion *address_space_io,
> >                      uint8_t devfn_min, const char *typename);
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > index 6ac187fa32..39d29d2230 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
> >  static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> >  {
> >      PXBDev *pxb = convert_to_pxb(dev);
> > -    DeviceState *ds, *bds = NULL;
> > +    DeviceState *bds = NULL;
> > +    PCIHostState *phb;
> >      PCIBus *bus;
> >      const char *dev_name = NULL;
> >      Error *local_err = NULL;
> > @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> >          dev_name = dev->qdev.id;
> >      }
> >  
> > -    ds = qdev_create(NULL, TYPE_PXB_HOST);
> > +    phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
> >      if (pcie) {
> > -        bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> > +        bus = pci_bus_new(phb, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> >      } else {
> > -        bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
> > +        bus = pci_bus_new(phb, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
> >          bds = qdev_create(BUS(bus), "pci-bridge");
> >          bds->id = dev_name;
> >          qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr);
> > @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> >      bus->address_space_io = dev->bus->address_space_io;
> >      bus->map_irq = pxb_map_irq_fn;
> >  
> > -    PCI_HOST_BRIDGE(ds)->bus = bus;
> > +    phb->bus = bus;
> >  
> >      pxb_register_bus(dev, bus, &local_err);
> >      if (local_err) {
> > @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> >          goto err_register_bus;
> >      }
> >  
> > -    qdev_init_nofail(ds);
> > +    qdev_init_nofail(DEVICE(phb));
> >      if (bds) {
> >          qdev_init_nofail(bds);
> >      }
> > @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> >  err_register_bus:
> >      object_unref(OBJECT(bds));
> >      object_unparent(OBJECT(bus));
> > -    object_unref(OBJECT(ds));
> > +    object_unref(OBJECT(phb));
> >  }
> >  
> >  static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index f9218aa952..91fec05b38 100644
> > --- a/hw/pci-host/piix.c
> > +++ b/hw/pci-host/piix.c
> > @@ -340,7 +340,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >  
> >      dev = qdev_create(NULL, host_type);
> >      s = PCI_HOST_BRIDGE(dev);
> > -    b = pci_bus_new(dev, NULL, pci_address_space,
> > +    b = pci_bus_new(s, NULL, pci_address_space,
> >                      address_space_io, 0, TYPE_PCI_BUS);
> >      s->bus = b;
> >      object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
> > diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> > index 260a119a9e..2e2cd267f4 100644
> > --- a/hw/pci-host/prep.c
> > +++ b/hw/pci-host/prep.c
> > @@ -269,7 +269,7 @@ static void raven_pcihost_initfn(Object *obj)
> >      memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
> >                                          &s->pci_io_non_contiguous, 1);
> >      memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
> > -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
> > +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
> >                          &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
> >  
> >      /* Bus master address space */
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index 344f77b10c..860b47a1ba 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -49,7 +49,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
> >      sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
> >      sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
> >  
> > -    pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
> > +    pci->bus = pci_bus_new(pci, "pcie.0",
> >                             s->mch.pci_address_space, s->mch.address_space_io,
> >                             0, TYPE_PCIE_BUS);
> >      PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> > diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> > index 467cbb9cb8..24ef87610b 100644
> > --- a/hw/pci-host/versatile.c
> > +++ b/hw/pci-host/versatile.c
> > @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
> >      memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
> >      memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
> >  
> > -    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), "pci",
> > +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
> >                          &s->pci_mem_space, &s->pci_io_space,
> >                          PCI_DEVFN(11, 0), TYPE_PCI_BUS);
> >      h->bus = &s->pci_bus;
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index d9535c0bdc..f4488b46fc 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -388,26 +388,25 @@ bool pci_bus_is_root(PCIBus *bus)
> >      return PCI_BUS_GET_CLASS(bus)->is_root(bus);
> >  }
> >  
> > -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> > +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> > +                         PCIHostState *phb,
> >                           const char *name,
> >                           MemoryRegion *address_space_mem,
> >                           MemoryRegion *address_space_io,
> >                           uint8_t devfn_min, const char *typename)
> >  {
> > -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> > -    qbus_create_inplace(bus, bus_size, typename, parent, name);
> > +    qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
> >      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> >  }
> >  
> > -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> > +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> >                      MemoryRegion *address_space_mem,
> >                      MemoryRegion *address_space_io,
> >                      uint8_t devfn_min, const char *typename)
> >  {
> > -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> >      PCIBus *bus;
> >  
> > -    bus = PCI_BUS(qbus_create(typename, parent, name));
> > +    bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
> >      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> >      return bus;
> >  }
> > @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> >  {
> >      PCIBus *bus;
> >  
> > -    bus = pci_bus_new(parent, name, address_space_mem,
> > +    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> >                        address_space_io, devfn_min, typename);
> >      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> >      return bus;
> > -- 
> > 2.11.0.259.g40922b1

-- 
Eduardo

[Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState

[Eduardo Habkost]

pci_register_bus() already requires the 'parent' argument to be a
PCI_HOST_BRIDGE object. Change the parameter type to reflect that.

Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/pci/pci.h      | 2 +-
 hw/alpha/typhoon.c        | 2 +-
 hw/mips/gt64xxx_pci.c     | 2 +-
 hw/pci-host/apb.c         | 2 +-
 hw/pci-host/bonito.c      | 2 +-
 hw/pci-host/gpex.c        | 2 +-
 hw/pci-host/grackle.c     | 2 +-
 hw/pci-host/ppce500.c     | 2 +-
 hw/pci-host/uninorth.c    | 4 ++--
 hw/pci-host/xilinx-pcie.c | 2 +-
 hw/pci/pci.c              | 4 ++--
 hw/ppc/ppc4xx_pci.c       | 2 +-
 hw/ppc/spapr_pci.c        | 2 +-
 hw/s390x/s390-pci-bus.c   | 2 +-
 hw/sh4/sh_pci.c           | 2 +-
 15 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 2242aa25eb..56387ccb0c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
 int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
-PCIBus *pci_register_bus(DeviceState *parent, const char *name,
+PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
                          pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          void *irq_opaque,
                          MemoryRegion *address_space_mem,
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index f50f5cf186..ac0633a55e 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -883,7 +883,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
     memory_region_add_subregion(addr_space, 0x801fc000000ULL,
                                 &s->pchip.reg_io);
 
-    b = pci_register_bus(dev, "pci",
+    b = pci_register_bus(phb, "pci",
                          typhoon_set_irq, sys_map_irq, s,
                          &s->pchip.reg_mem, &s->pchip.reg_io,
                          0, 64, TYPE_PCI_BUS);
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 4811843ab6..bd131bcdc6 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1171,7 +1171,7 @@ PCIBus *gt64120_register(qemu_irq *pic)
     phb = PCI_HOST_BRIDGE(dev);
     memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
     address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
-    phb->bus = pci_register_bus(dev, "pci",
+    phb->bus = pci_register_bus(phb, "pci",
                                 gt64120_pci_set_irq, gt64120_pci_map_irq,
                                 pic,
                                 &d->pci0_mem,
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 653e711121..1156a54224 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -671,7 +671,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
     dev = qdev_create(NULL, TYPE_APB);
     d = APB_DEVICE(dev);
     phb = PCI_HOST_BRIDGE(dev);
-    phb->bus = pci_register_bus(DEVICE(phb), "pci",
+    phb->bus = pci_register_bus(phb, "pci",
                                 pci_apb_set_irq, pci_pbm_map_irq, d,
                                 &d->pci_mmio,
                                 get_system_io(),
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 1999ece590..27842edc04 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
 
-    phb->bus = pci_register_bus(DEVICE(dev), "pci",
+    phb->bus = pci_register_bus(phb, "pci",
                                 pci_bonito_set_irq, pci_bonito_map_irq, dev,
                                 get_system_memory(), get_system_io(),
                                 0x28, 32, TYPE_PCI_BUS);
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 66055ee5cc..042d127271 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -62,7 +62,7 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
         sysbus_init_irq(sbd, &s->irq[i]);
     }
 
-    pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq,
+    pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
                                 pci_swizzle_map_irq_fn, s, &s->io_mmio,
                                 &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
 
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 2c8acdaaca..a56c063be9 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -82,7 +82,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
     memory_region_add_subregion(address_space_mem, 0x80000000ULL,
                                 &d->pci_hole);
 
-    phb->bus = pci_register_bus(dev, NULL,
+    phb->bus = pci_register_bus(phb, NULL,
                                 pci_grackle_set_irq,
                                 pci_grackle_map_irq,
                                 pic,
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index e502bc0505..4a1e99f426 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -465,7 +465,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
     /* PIO lives at the bottom of our bus space */
     memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
 
-    b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
+    b = pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
                          mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
                          PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
     h->bus = b;
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index df342ac3cb..b1b89183fc 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -233,7 +233,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
     memory_region_add_subregion(address_space_mem, 0x80000000ULL,
                                 &d->pci_hole);
 
-    h->bus = pci_register_bus(dev, NULL,
+    h->bus = pci_register_bus(h, NULL,
                               pci_unin_set_irq, pci_unin_map_irq,
                               pic,
                               &d->pci_mmio,
@@ -299,7 +299,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
     memory_region_add_subregion(address_space_mem, 0x80000000ULL,
                                 &d->pci_hole);
 
-    h->bus = pci_register_bus(dev, NULL,
+    h->bus = pci_register_bus(h, NULL,
                               pci_unin_set_irq, pci_unin_map_irq,
                               pic,
                               &d->pci_mmio,
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 8b71e2d950..c951684148 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -129,7 +129,7 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(sbd, &pex->mmio);
     sysbus_init_mmio(sbd, &s->mmio);
 
-    pci->bus = pci_register_bus(dev, s->name, xilinx_pcie_set_irq,
+    pci->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
                                 pci_swizzle_map_irq_fn, s, &s->mmio,
                                 &s->io, 0, 4, TYPE_PCIE_BUS);
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f4488b46fc..f60d0497ef 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -421,7 +421,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
     bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
 }
 
-PCIBus *pci_register_bus(DeviceState *parent, const char *name,
+PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
                          pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          void *irq_opaque,
                          MemoryRegion *address_space_mem,
@@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
 {
     PCIBus *bus;
 
-    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
+    bus = pci_bus_new(phb, name, address_space_mem,
                       address_space_io, devfn_min, typename);
     pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
     return bus;
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index dc19682970..f755c6faae 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -314,7 +314,7 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
         sysbus_init_irq(dev, &s->irq[i]);
     }
 
-    b = pci_register_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq,
+    b = pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
                          ppc4xx_pci_map_irq, s->irq, get_system_memory(),
                          get_system_io(), 0, 4, TYPE_PCI_BUS);
     h->bus = b;
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 98c52e411f..0f293f9e75 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1697,7 +1697,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
                                 &sphb->iowindow);
 
-    bus = pci_register_bus(dev, NULL,
+    bus = pci_register_bus(phb, NULL,
                            pci_spapr_set_irq, pci_spapr_map_irq, sphb,
                            &sphb->memspace, &sphb->iospace,
                            PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 69b0291e8a..99aae965bb 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
 
     DPRINTF("host_init\n");
 
-    b = pci_register_bus(DEVICE(dev), NULL,
+    b = pci_register_bus(phb, NULL,
                          s390_pci_set_irq, s390_pci_map_irq, NULL,
                          get_system_memory(), get_system_io(), 0, 64,
                          TYPE_PCI_BUS);
diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index 1747628f3d..bca849c10f 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -131,7 +131,7 @@ static int sh_pci_device_init(SysBusDevice *dev)
     for (i = 0; i < 4; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
-    phb->bus = pci_register_bus(DEVICE(dev), "pci",
+    phb->bus = pci_register_bus(phb, "pci",
                                 sh_pci_set_irq, sh_pci_map_irq,
                                 s->irq,
                                 get_system_memory(),
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 11909 bytes --]

On Mon, Apr 17, 2017 at 06:59:13PM -0300, Eduardo Habkost wrote:
> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/pci/pci.h      | 2 +-
>  hw/alpha/typhoon.c        | 2 +-
>  hw/mips/gt64xxx_pci.c     | 2 +-
>  hw/pci-host/apb.c         | 2 +-
>  hw/pci-host/bonito.c      | 2 +-
>  hw/pci-host/gpex.c        | 2 +-
>  hw/pci-host/grackle.c     | 2 +-
>  hw/pci-host/ppce500.c     | 2 +-
>  hw/pci-host/uninorth.c    | 4 ++--
>  hw/pci-host/xilinx-pcie.c | 2 +-
>  hw/pci/pci.c              | 4 ++--
>  hw/ppc/ppc4xx_pci.c       | 2 +-
>  hw/ppc/spapr_pci.c        | 2 +-
>  hw/s390x/s390-pci-bus.c   | 2 +-
>  hw/sh4/sh_pci.c           | 2 +-
>  15 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 2242aa25eb..56387ccb0c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
>  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> -PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
>                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           void *irq_opaque,
>                           MemoryRegion *address_space_mem,
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index f50f5cf186..ac0633a55e 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,7 +883,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      memory_region_add_subregion(addr_space, 0x801fc000000ULL,
>                                  &s->pchip.reg_io);
>  
> -    b = pci_register_bus(dev, "pci",
> +    b = pci_register_bus(phb, "pci",
>                           typhoon_set_irq, sys_map_irq, s,
>                           &s->pchip.reg_mem, &s->pchip.reg_io,
>                           0, 64, TYPE_PCI_BUS);
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 4811843ab6..bd131bcdc6 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,7 +1171,7 @@ PCIBus *gt64120_register(qemu_irq *pic)
>      phb = PCI_HOST_BRIDGE(dev);
>      memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
>      address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
> -    phb->bus = pci_register_bus(dev, "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  gt64120_pci_set_irq, gt64120_pci_map_irq,
>                                  pic,
>                                  &d->pci0_mem,
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 653e711121..1156a54224 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,7 +671,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      dev = qdev_create(NULL, TYPE_APB);
>      d = APB_DEVICE(dev);
>      phb = PCI_HOST_BRIDGE(dev);
> -    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  pci_apb_set_irq, pci_pbm_map_irq, d,
>                                  &d->pci_mmio,
>                                  get_system_io(),
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 1999ece590..27842edc04 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>  
> -    phb->bus = pci_register_bus(DEVICE(dev), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  pci_bonito_set_irq, pci_bonito_map_irq, dev,
>                                  get_system_memory(), get_system_io(),
>                                  0x28, 32, TYPE_PCI_BUS);
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 66055ee5cc..042d127271 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,7 +62,7 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
>          sysbus_init_irq(sbd, &s->irq[i]);
>      }
>  
> -    pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq,
> +    pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
>                                  pci_swizzle_map_irq_fn, s, &s->io_mmio,
>                                  &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>  
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 2c8acdaaca..a56c063be9 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,7 +82,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    phb->bus = pci_register_bus(dev, NULL,
> +    phb->bus = pci_register_bus(phb, NULL,
>                                  pci_grackle_set_irq,
>                                  pci_grackle_map_irq,
>                                  pic,
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index e502bc0505..4a1e99f426 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,7 +465,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>      /* PIO lives at the bottom of our bus space */
>      memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
>  
> -    b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> +    b = pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
>                           mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
>                           PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
>      h->bus = b;
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index df342ac3cb..b1b89183fc 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,7 +233,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    h->bus = pci_register_bus(dev, NULL,
> +    h->bus = pci_register_bus(h, NULL,
>                                pci_unin_set_irq, pci_unin_map_irq,
>                                pic,
>                                &d->pci_mmio,
> @@ -299,7 +299,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    h->bus = pci_register_bus(dev, NULL,
> +    h->bus = pci_register_bus(h, NULL,
>                                pci_unin_set_irq, pci_unin_map_irq,
>                                pic,
>                                &d->pci_mmio,
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 8b71e2d950..c951684148 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,7 +129,7 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
>      sysbus_init_mmio(sbd, &pex->mmio);
>      sysbus_init_mmio(sbd, &s->mmio);
>  
> -    pci->bus = pci_register_bus(dev, s->name, xilinx_pcie_set_irq,
> +    pci->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
>                                  pci_swizzle_map_irq_fn, s, &s->mmio,
>                                  &s->io, 0, 4, TYPE_PCIE_BUS);
>  
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f4488b46fc..f60d0497ef 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,7 +421,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
>  }
>  
> -PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
>                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           void *irq_opaque,
>                           MemoryRegion *address_space_mem,
> @@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  {
>      PCIBus *bus;
>  
> -    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> +    bus = pci_bus_new(phb, name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
>      return bus;
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index dc19682970..f755c6faae 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,7 +314,7 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
>  
> -    b = pci_register_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq,
> +    b = pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
>                           ppc4xx_pci_map_irq, s->irq, get_system_memory(),
>                           get_system_io(), 0, 4, TYPE_PCI_BUS);
>      h->bus = b;
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 98c52e411f..0f293f9e75 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,7 +1697,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
>  
> -    bus = pci_register_bus(dev, NULL,
> +    bus = pci_register_bus(phb, NULL,
>                             pci_spapr_set_irq, pci_spapr_map_irq, sphb,
>                             &sphb->memspace, &sphb->iospace,
>                             PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..99aae965bb 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>  
>      DPRINTF("host_init\n");
>  
> -    b = pci_register_bus(DEVICE(dev), NULL,
> +    b = pci_register_bus(phb, NULL,
>                           s390_pci_set_irq, s390_pci_map_irq, NULL,
>                           get_system_memory(), get_system_io(), 0, 64,
>                           TYPE_PCI_BUS);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index 1747628f3d..bca849c10f 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,7 +131,7 @@ static int sh_pci_device_init(SysBusDevice *dev)
>      for (i = 0; i < 4; i++) {
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
> -    phb->bus = pci_register_bus(DEVICE(dev), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  sh_pci_set_irq, sh_pci_map_irq,
>                                  s->irq,
>                                  get_system_memory(),

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState

[Cornelia Huck]

On Mon, 17 Apr 2017 18:59:13 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/hw/pci/pci.h      | 2 +-
>  hw/alpha/typhoon.c        | 2 +-
>  hw/mips/gt64xxx_pci.c     | 2 +-
>  hw/pci-host/apb.c         | 2 +-
>  hw/pci-host/bonito.c      | 2 +-
>  hw/pci-host/gpex.c        | 2 +-
>  hw/pci-host/grackle.c     | 2 +-
>  hw/pci-host/ppce500.c     | 2 +-
>  hw/pci-host/uninorth.c    | 4 ++--
>  hw/pci-host/xilinx-pcie.c | 2 +-
>  hw/pci/pci.c              | 4 ++--
>  hw/ppc/ppc4xx_pci.c       | 2 +-
>  hw/ppc/spapr_pci.c        | 2 +-
>  hw/s390x/s390-pci-bus.c   | 2 +-
>  hw/sh4/sh_pci.c           | 2 +-
>  15 files changed, 17 insertions(+), 17 deletions(-)

s390 parts:

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState

[Marcel Apfelbaum]

On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/hw/pci/pci.h      | 2 +-
>  hw/alpha/typhoon.c        | 2 +-
>  hw/mips/gt64xxx_pci.c     | 2 +-
>  hw/pci-host/apb.c         | 2 +-
>  hw/pci-host/bonito.c      | 2 +-
>  hw/pci-host/gpex.c        | 2 +-
>  hw/pci-host/grackle.c     | 2 +-
>  hw/pci-host/ppce500.c     | 2 +-
>  hw/pci-host/uninorth.c    | 4 ++--
>  hw/pci-host/xilinx-pcie.c | 2 +-
>  hw/pci/pci.c              | 4 ++--
>  hw/ppc/ppc4xx_pci.c       | 2 +-
>  hw/ppc/spapr_pci.c        | 2 +-
>  hw/s390x/s390-pci-bus.c   | 2 +-
>  hw/sh4/sh_pci.c           | 2 +-
>  15 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 2242aa25eb..56387ccb0c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
>  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> -PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
>                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           void *irq_opaque,
>                           MemoryRegion *address_space_mem,
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index f50f5cf186..ac0633a55e 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,7 +883,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      memory_region_add_subregion(addr_space, 0x801fc000000ULL,
>                                  &s->pchip.reg_io);
>
> -    b = pci_register_bus(dev, "pci",
> +    b = pci_register_bus(phb, "pci",
>                           typhoon_set_irq, sys_map_irq, s,
>                           &s->pchip.reg_mem, &s->pchip.reg_io,
>                           0, 64, TYPE_PCI_BUS);
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 4811843ab6..bd131bcdc6 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,7 +1171,7 @@ PCIBus *gt64120_register(qemu_irq *pic)
>      phb = PCI_HOST_BRIDGE(dev);
>      memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
>      address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
> -    phb->bus = pci_register_bus(dev, "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  gt64120_pci_set_irq, gt64120_pci_map_irq,
>                                  pic,
>                                  &d->pci0_mem,
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 653e711121..1156a54224 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,7 +671,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      dev = qdev_create(NULL, TYPE_APB);
>      d = APB_DEVICE(dev);
>      phb = PCI_HOST_BRIDGE(dev);
> -    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  pci_apb_set_irq, pci_pbm_map_irq, d,
>                                  &d->pci_mmio,
>                                  get_system_io(),
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 1999ece590..27842edc04 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>
> -    phb->bus = pci_register_bus(DEVICE(dev), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  pci_bonito_set_irq, pci_bonito_map_irq, dev,
>                                  get_system_memory(), get_system_io(),
>                                  0x28, 32, TYPE_PCI_BUS);
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 66055ee5cc..042d127271 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,7 +62,7 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
>          sysbus_init_irq(sbd, &s->irq[i]);
>      }
>
> -    pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq,
> +    pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
>                                  pci_swizzle_map_irq_fn, s, &s->io_mmio,
>                                  &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 2c8acdaaca..a56c063be9 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,7 +82,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>
> -    phb->bus = pci_register_bus(dev, NULL,
> +    phb->bus = pci_register_bus(phb, NULL,
>                                  pci_grackle_set_irq,
>                                  pci_grackle_map_irq,
>                                  pic,
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index e502bc0505..4a1e99f426 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,7 +465,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>      /* PIO lives at the bottom of our bus space */
>      memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
>
> -    b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> +    b = pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
>                           mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
>                           PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
>      h->bus = b;
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index df342ac3cb..b1b89183fc 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,7 +233,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>
> -    h->bus = pci_register_bus(dev, NULL,
> +    h->bus = pci_register_bus(h, NULL,
>                                pci_unin_set_irq, pci_unin_map_irq,
>                                pic,
>                                &d->pci_mmio,
> @@ -299,7 +299,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>
> -    h->bus = pci_register_bus(dev, NULL,
> +    h->bus = pci_register_bus(h, NULL,
>                                pci_unin_set_irq, pci_unin_map_irq,
>                                pic,
>                                &d->pci_mmio,
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 8b71e2d950..c951684148 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,7 +129,7 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
>      sysbus_init_mmio(sbd, &pex->mmio);
>      sysbus_init_mmio(sbd, &s->mmio);
>
> -    pci->bus = pci_register_bus(dev, s->name, xilinx_pcie_set_irq,
> +    pci->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
>                                  pci_swizzle_map_irq_fn, s, &s->mmio,
>                                  &s->io, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f4488b46fc..f60d0497ef 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,7 +421,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
>  }
>
> -PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
>                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           void *irq_opaque,
>                           MemoryRegion *address_space_mem,
> @@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  {
>      PCIBus *bus;
>
> -    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> +    bus = pci_bus_new(phb, name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
>      return bus;
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index dc19682970..f755c6faae 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,7 +314,7 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
>
> -    b = pci_register_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq,
> +    b = pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
>                           ppc4xx_pci_map_irq, s->irq, get_system_memory(),
>                           get_system_io(), 0, 4, TYPE_PCI_BUS);
>      h->bus = b;
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 98c52e411f..0f293f9e75 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,7 +1697,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
>
> -    bus = pci_register_bus(dev, NULL,
> +    bus = pci_register_bus(phb, NULL,
>                             pci_spapr_set_irq, pci_spapr_map_irq, sphb,
>                             &sphb->memspace, &sphb->iospace,
>                             PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..99aae965bb 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>
>      DPRINTF("host_init\n");
>
> -    b = pci_register_bus(DEVICE(dev), NULL,
> +    b = pci_register_bus(phb, NULL,
>                           s390_pci_set_irq, s390_pci_map_irq, NULL,
>                           get_system_memory(), get_system_io(), 0, 64,
>                           TYPE_PCI_BUS);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index 1747628f3d..bca849c10f 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,7 +131,7 @@ static int sh_pci_device_init(SysBusDevice *dev)
>      for (i = 0; i < 4; i++) {
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
> -    phb->bus = pci_register_bus(DEVICE(dev), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  sh_pci_set_irq, sh_pci_map_irq,
>                                  s->irq,
>                                  get_system_memory(),
>


Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>


Thanks,
Marcel

Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState

[Michael S. Tsirkin]

On Mon, Apr 17, 2017 at 06:59:13PM -0300, Eduardo Habkost wrote:
> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---

Same here. Let's rename so it implies what it does?

>  include/hw/pci/pci.h      | 2 +-
>  hw/alpha/typhoon.c        | 2 +-
>  hw/mips/gt64xxx_pci.c     | 2 +-
>  hw/pci-host/apb.c         | 2 +-
>  hw/pci-host/bonito.c      | 2 +-
>  hw/pci-host/gpex.c        | 2 +-
>  hw/pci-host/grackle.c     | 2 +-
>  hw/pci-host/ppce500.c     | 2 +-
>  hw/pci-host/uninorth.c    | 4 ++--
>  hw/pci-host/xilinx-pcie.c | 2 +-
>  hw/pci/pci.c              | 4 ++--
>  hw/ppc/ppc4xx_pci.c       | 2 +-
>  hw/ppc/spapr_pci.c        | 2 +-
>  hw/s390x/s390-pci-bus.c   | 2 +-
>  hw/sh4/sh_pci.c           | 2 +-
>  15 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 2242aa25eb..56387ccb0c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
>  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> -PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
>                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           void *irq_opaque,
>                           MemoryRegion *address_space_mem,
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index f50f5cf186..ac0633a55e 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,7 +883,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      memory_region_add_subregion(addr_space, 0x801fc000000ULL,
>                                  &s->pchip.reg_io);
>  
> -    b = pci_register_bus(dev, "pci",
> +    b = pci_register_bus(phb, "pci",
>                           typhoon_set_irq, sys_map_irq, s,
>                           &s->pchip.reg_mem, &s->pchip.reg_io,
>                           0, 64, TYPE_PCI_BUS);
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 4811843ab6..bd131bcdc6 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,7 +1171,7 @@ PCIBus *gt64120_register(qemu_irq *pic)
>      phb = PCI_HOST_BRIDGE(dev);
>      memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
>      address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
> -    phb->bus = pci_register_bus(dev, "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  gt64120_pci_set_irq, gt64120_pci_map_irq,
>                                  pic,
>                                  &d->pci0_mem,
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 653e711121..1156a54224 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,7 +671,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      dev = qdev_create(NULL, TYPE_APB);
>      d = APB_DEVICE(dev);
>      phb = PCI_HOST_BRIDGE(dev);
> -    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  pci_apb_set_irq, pci_pbm_map_irq, d,
>                                  &d->pci_mmio,
>                                  get_system_io(),
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 1999ece590..27842edc04 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>  
> -    phb->bus = pci_register_bus(DEVICE(dev), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  pci_bonito_set_irq, pci_bonito_map_irq, dev,
>                                  get_system_memory(), get_system_io(),
>                                  0x28, 32, TYPE_PCI_BUS);
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 66055ee5cc..042d127271 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,7 +62,7 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
>          sysbus_init_irq(sbd, &s->irq[i]);
>      }
>  
> -    pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq,
> +    pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
>                                  pci_swizzle_map_irq_fn, s, &s->io_mmio,
>                                  &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>  
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 2c8acdaaca..a56c063be9 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,7 +82,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    phb->bus = pci_register_bus(dev, NULL,
> +    phb->bus = pci_register_bus(phb, NULL,
>                                  pci_grackle_set_irq,
>                                  pci_grackle_map_irq,
>                                  pic,
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index e502bc0505..4a1e99f426 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,7 +465,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>      /* PIO lives at the bottom of our bus space */
>      memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
>  
> -    b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> +    b = pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
>                           mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
>                           PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
>      h->bus = b;
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index df342ac3cb..b1b89183fc 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,7 +233,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    h->bus = pci_register_bus(dev, NULL,
> +    h->bus = pci_register_bus(h, NULL,
>                                pci_unin_set_irq, pci_unin_map_irq,
>                                pic,
>                                &d->pci_mmio,
> @@ -299,7 +299,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    h->bus = pci_register_bus(dev, NULL,
> +    h->bus = pci_register_bus(h, NULL,
>                                pci_unin_set_irq, pci_unin_map_irq,
>                                pic,
>                                &d->pci_mmio,
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 8b71e2d950..c951684148 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,7 +129,7 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
>      sysbus_init_mmio(sbd, &pex->mmio);
>      sysbus_init_mmio(sbd, &s->mmio);
>  
> -    pci->bus = pci_register_bus(dev, s->name, xilinx_pcie_set_irq,
> +    pci->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
>                                  pci_swizzle_map_irq_fn, s, &s->mmio,
>                                  &s->io, 0, 4, TYPE_PCIE_BUS);
>  
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f4488b46fc..f60d0497ef 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,7 +421,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
>  }
>  
> -PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
>                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           void *irq_opaque,
>                           MemoryRegion *address_space_mem,
> @@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  {
>      PCIBus *bus;
>  
> -    bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> +    bus = pci_bus_new(phb, name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
>      return bus;
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index dc19682970..f755c6faae 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,7 +314,7 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
>  
> -    b = pci_register_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq,
> +    b = pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
>                           ppc4xx_pci_map_irq, s->irq, get_system_memory(),
>                           get_system_io(), 0, 4, TYPE_PCI_BUS);
>      h->bus = b;
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 98c52e411f..0f293f9e75 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,7 +1697,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
>  
> -    bus = pci_register_bus(dev, NULL,
> +    bus = pci_register_bus(phb, NULL,
>                             pci_spapr_set_irq, pci_spapr_map_irq, sphb,
>                             &sphb->memspace, &sphb->iospace,
>                             PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..99aae965bb 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>  
>      DPRINTF("host_init\n");
>  
> -    b = pci_register_bus(DEVICE(dev), NULL,
> +    b = pci_register_bus(phb, NULL,
>                           s390_pci_set_irq, s390_pci_map_irq, NULL,
>                           get_system_memory(), get_system_io(), 0, 64,
>                           TYPE_PCI_BUS);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index 1747628f3d..bca849c10f 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,7 +131,7 @@ static int sh_pci_device_init(SysBusDevice *dev)
>      for (i = 0; i < 4; i++) {
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
> -    phb->bus = pci_register_bus(DEVICE(dev), "pci",
> +    phb->bus = pci_register_bus(phb, "pci",
>                                  sh_pci_set_irq, sh_pci_map_irq,
>                                  s->irq,
>                                  get_system_memory(),
> -- 
> 2.11.0.259.g40922b1

[Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()

[Eduardo Habkost]

Every single caller of of pci_register_bus() saves the return value in
phb->bus. Do that inside pci_register_bus() to avoid code duplication
and make it harder to break.

Most (but not all) conversions done using the following Coccinelle script:

  @@
  identifier b;
  expression phb;
  @@
  -b = pci_register_bus(phb, ARGS);
  +phb->bus = pci_register_bus(phb, ARGS);
   ...
  -phb->bus = b;

  @@
  expression phb;
  expression list ARGS;
  @@
  -phb->bus = pci_register_bus(phb, ARGS);
  +pci_register_bus(phb, ARGS);

Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/pci/pci.h      | 12 ++++++------
 hw/alpha/typhoon.c        | 10 +++++-----
 hw/mips/gt64xxx_pci.c     |  9 +++------
 hw/pci-host/apb.c         |  7 ++-----
 hw/pci-host/bonito.c      |  7 +++----
 hw/pci-host/gpex.c        |  5 ++---
 hw/pci-host/grackle.c     |  9 ++-------
 hw/pci-host/ppce500.c     |  8 ++++----
 hw/pci-host/uninorth.c    | 18 ++++++------------
 hw/pci-host/xilinx-pcie.c |  6 +++---
 hw/pci/pci.c              | 14 +++++++-------
 hw/ppc/ppc4xx_pci.c       |  8 ++++----
 hw/ppc/spapr_pci.c        | 10 +++++-----
 hw/s390x/s390-pci-bus.c   | 10 +++++-----
 hw/sh4/sh_pci.c           |  9 +++------
 15 files changed, 60 insertions(+), 82 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 56387ccb0c..3b1e2c408a 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -408,12 +408,12 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
 int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
-PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
-                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-                         void *irq_opaque,
-                         MemoryRegion *address_space_mem,
-                         MemoryRegion *address_space_io,
-                         uint8_t devfn_min, int nirq, const char *typename);
+void pci_register_bus(PCIHostState *phb, const char *name,
+                      pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+                      void *irq_opaque,
+                      MemoryRegion *address_space_mem,
+                      MemoryRegion *address_space_io,
+                      uint8_t devfn_min, int nirq, const char *typename);
 void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
 PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
 bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index ac0633a55e..5926686d79 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -883,11 +883,11 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
     memory_region_add_subregion(addr_space, 0x801fc000000ULL,
                                 &s->pchip.reg_io);
 
-    b = pci_register_bus(phb, "pci",
-                         typhoon_set_irq, sys_map_irq, s,
-                         &s->pchip.reg_mem, &s->pchip.reg_io,
-                         0, 64, TYPE_PCI_BUS);
-    phb->bus = b;
+    pci_register_bus(phb, "pci",
+                     typhoon_set_irq, sys_map_irq, s,
+                     &s->pchip.reg_mem, &s->pchip.reg_io,
+                     0, 64, TYPE_PCI_BUS);
+    b = phb->bus;
     qdev_init_nofail(dev);
 
     /* Host memory as seen from the PCI side, via the IOMMU.  */
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index bd131bcdc6..69963453f0 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1171,12 +1171,9 @@ PCIBus *gt64120_register(qemu_irq *pic)
     phb = PCI_HOST_BRIDGE(dev);
     memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
     address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
-    phb->bus = pci_register_bus(phb, "pci",
-                                gt64120_pci_set_irq, gt64120_pci_map_irq,
-                                pic,
-                                &d->pci0_mem,
-                                get_system_io(),
-                                PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
+    pci_register_bus(phb, "pci", gt64120_pci_set_irq, gt64120_pci_map_irq,
+                     pic, &d->pci0_mem, get_system_io(), PCI_DEVFN(18, 0), 4,
+                     TYPE_PCI_BUS);
     qdev_init_nofail(dev);
     memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d, "isd-mem", 0x1000);
 
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 1156a54224..ea86260b04 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -671,11 +671,8 @@ PCIBus *pci_apb_init(hwaddr special_base,
     dev = qdev_create(NULL, TYPE_APB);
     d = APB_DEVICE(dev);
     phb = PCI_HOST_BRIDGE(dev);
-    phb->bus = pci_register_bus(phb, "pci",
-                                pci_apb_set_irq, pci_pbm_map_irq, d,
-                                &d->pci_mmio,
-                                get_system_io(),
-                                0, 32, TYPE_PCI_BUS);
+    pci_register_bus(phb, "pci", pci_apb_set_irq, pci_pbm_map_irq, d,
+                     &d->pci_mmio, get_system_io(), 0, 32, TYPE_PCI_BUS);
     qdev_init_nofail(dev);
     s = SYS_BUS_DEVICE(dev);
     /* apb_config */
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 27842edc04..bbe595605d 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -714,10 +714,9 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
 
-    phb->bus = pci_register_bus(phb, "pci",
-                                pci_bonito_set_irq, pci_bonito_map_irq, dev,
-                                get_system_memory(), get_system_io(),
-                                0x28, 32, TYPE_PCI_BUS);
+    pci_register_bus(phb, "pci", pci_bonito_set_irq, pci_bonito_map_irq, dev,
+                     get_system_memory(), get_system_io(), 0x28, 32,
+                     TYPE_PCI_BUS);
 
     return 0;
 }
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 042d127271..66e2f3d29c 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -62,9 +62,8 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
         sysbus_init_irq(sbd, &s->irq[i]);
     }
 
-    pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
-                                pci_swizzle_map_irq_fn, s, &s->io_mmio,
-                                &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
+    pci_register_bus(pci, "pcie.0", gpex_set_irq, pci_swizzle_map_irq_fn, s,
+                     &s->io_mmio, &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
 
     qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->gpex_root));
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index a56c063be9..691af0a29e 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -82,13 +82,8 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
     memory_region_add_subregion(address_space_mem, 0x80000000ULL,
                                 &d->pci_hole);
 
-    phb->bus = pci_register_bus(phb, NULL,
-                                pci_grackle_set_irq,
-                                pci_grackle_map_irq,
-                                pic,
-                                &d->pci_mmio,
-                                address_space_io,
-                                0, 4, TYPE_PCI_BUS);
+    pci_register_bus(phb, NULL, pci_grackle_set_irq, pci_grackle_map_irq, pic,
+                     &d->pci_mmio, address_space_io, 0, 4, TYPE_PCI_BUS);
 
     pci_create_simple(phb->bus, 0, "grackle");
     qdev_init_nofail(dev);
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index 4a1e99f426..43a06961d0 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -465,10 +465,10 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
     /* PIO lives at the bottom of our bus space */
     memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
 
-    b = pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
-                         mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
-                         PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
-    h->bus = b;
+    pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
+                     mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
+                     PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
+    b = h->bus;
 
     /* Set up PCI view of memory */
     memory_region_init(&s->bm, OBJECT(s), "bm-e500", UINT64_MAX);
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index b1b89183fc..4d709e5468 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -233,12 +233,9 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
     memory_region_add_subregion(address_space_mem, 0x80000000ULL,
                                 &d->pci_hole);
 
-    h->bus = pci_register_bus(h, NULL,
-                              pci_unin_set_irq, pci_unin_map_irq,
-                              pic,
-                              &d->pci_mmio,
-                              address_space_io,
-                              PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
+    pci_register_bus(h, NULL, pci_unin_set_irq, pci_unin_map_irq, pic,
+                     &d->pci_mmio, address_space_io, PCI_DEVFN(11, 0), 4,
+                     TYPE_PCI_BUS);
 
 #if 0
     pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north");
@@ -299,12 +296,9 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
     memory_region_add_subregion(address_space_mem, 0x80000000ULL,
                                 &d->pci_hole);
 
-    h->bus = pci_register_bus(h, NULL,
-                              pci_unin_set_irq, pci_unin_map_irq,
-                              pic,
-                              &d->pci_mmio,
-                              address_space_io,
-                              PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
+    pci_register_bus(h, NULL, pci_unin_set_irq, pci_unin_map_irq, pic,
+                     &d->pci_mmio, address_space_io, PCI_DEVFN(11, 0), 4,
+                     TYPE_PCI_BUS);
 
     sysbus_mmio_map(s, 0, 0xf0800000);
     sysbus_mmio_map(s, 1, 0xf0c00000);
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index c951684148..6d53677f57 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -129,9 +129,9 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(sbd, &pex->mmio);
     sysbus_init_mmio(sbd, &s->mmio);
 
-    pci->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
-                                pci_swizzle_map_irq_fn, s, &s->mmio,
-                                &s->io, 0, 4, TYPE_PCIE_BUS);
+    pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
+                     pci_swizzle_map_irq_fn, s, &s->mmio, &s->io, 0, 4,
+                     TYPE_PCIE_BUS);
 
     qdev_set_parent_bus(DEVICE(&s->root), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->root));
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f60d0497ef..d3adf806e5 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -421,19 +421,19 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
     bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
 }
 
-PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
-                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-                         void *irq_opaque,
-                         MemoryRegion *address_space_mem,
-                         MemoryRegion *address_space_io,
-                         uint8_t devfn_min, int nirq, const char *typename)
+void pci_register_bus(PCIHostState *phb, const char *name,
+                      pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+                      void *irq_opaque,
+                      MemoryRegion *address_space_mem,
+                      MemoryRegion *address_space_io,
+                      uint8_t devfn_min, int nirq, const char *typename)
 {
     PCIBus *bus;
 
     bus = pci_bus_new(phb, name, address_space_mem,
                       address_space_io, devfn_min, typename);
     pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
-    return bus;
+    phb->bus = bus;
 }
 
 int pci_bus_num(PCIBus *s)
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index f755c6faae..ab158b3ef1 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -314,10 +314,10 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
         sysbus_init_irq(dev, &s->irq[i]);
     }
 
-    b = pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
-                         ppc4xx_pci_map_irq, s->irq, get_system_memory(),
-                         get_system_io(), 0, 4, TYPE_PCI_BUS);
-    h->bus = b;
+    pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
+                     ppc4xx_pci_map_irq, s->irq, get_system_memory(),
+                     get_system_io(), 0, 4, TYPE_PCI_BUS);
+    b = h->bus;
 
     pci_create_simple(b, 0, "ppc4xx-host-bridge");
 
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 0f293f9e75..5749f14d9f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1697,11 +1697,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
                                 &sphb->iowindow);
 
-    bus = pci_register_bus(phb, NULL,
-                           pci_spapr_set_irq, pci_spapr_map_irq, sphb,
-                           &sphb->memspace, &sphb->iospace,
-                           PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
-    phb->bus = bus;
+    pci_register_bus(phb, NULL,
+                     pci_spapr_set_irq, pci_spapr_map_irq, sphb,
+                     &sphb->memspace, &sphb->iospace,
+                     PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
+    bus = phb->bus;
     qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL);
 
     /*
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 99aae965bb..466067a380 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -560,15 +560,15 @@ static int s390_pcihost_init(SysBusDevice *dev)
 
     DPRINTF("host_init\n");
 
-    b = pci_register_bus(phb, NULL,
-                         s390_pci_set_irq, s390_pci_map_irq, NULL,
-                         get_system_memory(), get_system_io(), 0, 64,
-                         TYPE_PCI_BUS);
+    pci_register_bus(phb, NULL,
+                     s390_pci_set_irq, s390_pci_map_irq, NULL,
+                     get_system_memory(), get_system_io(), 0, 64,
+                     TYPE_PCI_BUS);
+    b = phb->bus;
     pci_setup_iommu(b, s390_pci_dma_iommu, s);
 
     bus = BUS(b);
     qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
-    phb->bus = b;
 
     s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
     qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index bca849c10f..bb43bad77d 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -131,12 +131,9 @@ static int sh_pci_device_init(SysBusDevice *dev)
     for (i = 0; i < 4; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
-    phb->bus = pci_register_bus(phb, "pci",
-                                sh_pci_set_irq, sh_pci_map_irq,
-                                s->irq,
-                                get_system_memory(),
-                                get_system_io(),
-                                PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+    pci_register_bus(phb, "pci", sh_pci_set_irq, sh_pci_map_irq, s->irq,
+                     get_system_memory(), get_system_io(), PCI_DEVFN(0, 0), 4,
+                     TYPE_PCI_BUS);
     memory_region_init_io(&s->memconfig_p4, OBJECT(s), &sh_pci_reg_ops, s,
                           "sh_pci", 0x224);
     memory_region_init_alias(&s->memconfig_a7, OBJECT(s), "sh_pci.2",
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 17890 bytes --]

On Mon, Apr 17, 2017 at 06:59:14PM -0300, Eduardo Habkost wrote:
> Every single caller of of pci_register_bus() saves the return value in
> phb->bus. Do that inside pci_register_bus() to avoid code duplication
> and make it harder to break.
> 
> Most (but not all) conversions done using the following Coccinelle script:
> 
>   @@
>   identifier b;
>   expression phb;
>   @@
>   -b = pci_register_bus(phb, ARGS);
>   +phb->bus = pci_register_bus(phb, ARGS);
>    ...
>   -phb->bus = b;
> 
>   @@
>   expression phb;
>   expression list ARGS;
>   @@
>   -phb->bus = pci_register_bus(phb, ARGS);
>   +pci_register_bus(phb, ARGS);
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>


Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/pci/pci.h      | 12 ++++++------
>  hw/alpha/typhoon.c        | 10 +++++-----
>  hw/mips/gt64xxx_pci.c     |  9 +++------
>  hw/pci-host/apb.c         |  7 ++-----
>  hw/pci-host/bonito.c      |  7 +++----
>  hw/pci-host/gpex.c        |  5 ++---
>  hw/pci-host/grackle.c     |  9 ++-------
>  hw/pci-host/ppce500.c     |  8 ++++----
>  hw/pci-host/uninorth.c    | 18 ++++++------------
>  hw/pci-host/xilinx-pcie.c |  6 +++---
>  hw/pci/pci.c              | 14 +++++++-------
>  hw/ppc/ppc4xx_pci.c       |  8 ++++----
>  hw/ppc/spapr_pci.c        | 10 +++++-----
>  hw/s390x/s390-pci-bus.c   | 10 +++++-----
>  hw/sh4/sh_pci.c           |  9 +++------
>  15 files changed, 60 insertions(+), 82 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 56387ccb0c..3b1e2c408a 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,12 +408,12 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
>  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> -PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> -                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> -                         void *irq_opaque,
> -                         MemoryRegion *address_space_mem,
> -                         MemoryRegion *address_space_io,
> -                         uint8_t devfn_min, int nirq, const char *typename);
> +void pci_register_bus(PCIHostState *phb, const char *name,
> +                      pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> +                      void *irq_opaque,
> +                      MemoryRegion *address_space_mem,
> +                      MemoryRegion *address_space_io,
> +                      uint8_t devfn_min, int nirq, const char *typename);
>  void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
>  PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
>  bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index ac0633a55e..5926686d79 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,11 +883,11 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      memory_region_add_subregion(addr_space, 0x801fc000000ULL,
>                                  &s->pchip.reg_io);
>  
> -    b = pci_register_bus(phb, "pci",
> -                         typhoon_set_irq, sys_map_irq, s,
> -                         &s->pchip.reg_mem, &s->pchip.reg_io,
> -                         0, 64, TYPE_PCI_BUS);
> -    phb->bus = b;
> +    pci_register_bus(phb, "pci",
> +                     typhoon_set_irq, sys_map_irq, s,
> +                     &s->pchip.reg_mem, &s->pchip.reg_io,
> +                     0, 64, TYPE_PCI_BUS);
> +    b = phb->bus;
>      qdev_init_nofail(dev);
>  
>      /* Host memory as seen from the PCI side, via the IOMMU.  */
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index bd131bcdc6..69963453f0 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,12 +1171,9 @@ PCIBus *gt64120_register(qemu_irq *pic)
>      phb = PCI_HOST_BRIDGE(dev);
>      memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
>      address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                gt64120_pci_set_irq, gt64120_pci_map_irq,
> -                                pic,
> -                                &d->pci0_mem,
> -                                get_system_io(),
> -                                PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", gt64120_pci_set_irq, gt64120_pci_map_irq,
> +                     pic, &d->pci0_mem, get_system_io(), PCI_DEVFN(18, 0), 4,
> +                     TYPE_PCI_BUS);
>      qdev_init_nofail(dev);
>      memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d, "isd-mem", 0x1000);
>  
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 1156a54224..ea86260b04 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,11 +671,8 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      dev = qdev_create(NULL, TYPE_APB);
>      d = APB_DEVICE(dev);
>      phb = PCI_HOST_BRIDGE(dev);
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                pci_apb_set_irq, pci_pbm_map_irq, d,
> -                                &d->pci_mmio,
> -                                get_system_io(),
> -                                0, 32, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", pci_apb_set_irq, pci_pbm_map_irq, d,
> +                     &d->pci_mmio, get_system_io(), 0, 32, TYPE_PCI_BUS);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      /* apb_config */
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 27842edc04..bbe595605d 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,10 +714,9 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>  
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                pci_bonito_set_irq, pci_bonito_map_irq, dev,
> -                                get_system_memory(), get_system_io(),
> -                                0x28, 32, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", pci_bonito_set_irq, pci_bonito_map_irq, dev,
> +                     get_system_memory(), get_system_io(), 0x28, 32,
> +                     TYPE_PCI_BUS);
>  
>      return 0;
>  }
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 042d127271..66e2f3d29c 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,9 +62,8 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
>          sysbus_init_irq(sbd, &s->irq[i]);
>      }
>  
> -    pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
> -                                pci_swizzle_map_irq_fn, s, &s->io_mmio,
> -                                &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
> +    pci_register_bus(pci, "pcie.0", gpex_set_irq, pci_swizzle_map_irq_fn, s,
> +                     &s->io_mmio, &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>  
>      qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
>      qdev_init_nofail(DEVICE(&s->gpex_root));
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index a56c063be9..691af0a29e 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,13 +82,8 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    phb->bus = pci_register_bus(phb, NULL,
> -                                pci_grackle_set_irq,
> -                                pci_grackle_map_irq,
> -                                pic,
> -                                &d->pci_mmio,
> -                                address_space_io,
> -                                0, 4, TYPE_PCI_BUS);
> +    pci_register_bus(phb, NULL, pci_grackle_set_irq, pci_grackle_map_irq, pic,
> +                     &d->pci_mmio, address_space_io, 0, 4, TYPE_PCI_BUS);
>  
>      pci_create_simple(phb->bus, 0, "grackle");
>      qdev_init_nofail(dev);
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index 4a1e99f426..43a06961d0 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,10 +465,10 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>      /* PIO lives at the bottom of our bus space */
>      memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
>  
> -    b = pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
> -                         mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
> -                         PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
> -    h->bus = b;
> +    pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
> +                     mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
> +                     PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
> +    b = h->bus;
>  
>      /* Set up PCI view of memory */
>      memory_region_init(&s->bm, OBJECT(s), "bm-e500", UINT64_MAX);
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index b1b89183fc..4d709e5468 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,12 +233,9 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    h->bus = pci_register_bus(h, NULL,
> -                              pci_unin_set_irq, pci_unin_map_irq,
> -                              pic,
> -                              &d->pci_mmio,
> -                              address_space_io,
> -                              PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(h, NULL, pci_unin_set_irq, pci_unin_map_irq, pic,
> +                     &d->pci_mmio, address_space_io, PCI_DEVFN(11, 0), 4,
> +                     TYPE_PCI_BUS);
>  
>  #if 0
>      pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north");
> @@ -299,12 +296,9 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>  
> -    h->bus = pci_register_bus(h, NULL,
> -                              pci_unin_set_irq, pci_unin_map_irq,
> -                              pic,
> -                              &d->pci_mmio,
> -                              address_space_io,
> -                              PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(h, NULL, pci_unin_set_irq, pci_unin_map_irq, pic,
> +                     &d->pci_mmio, address_space_io, PCI_DEVFN(11, 0), 4,
> +                     TYPE_PCI_BUS);
>  
>      sysbus_mmio_map(s, 0, 0xf0800000);
>      sysbus_mmio_map(s, 1, 0xf0c00000);
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index c951684148..6d53677f57 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,9 +129,9 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
>      sysbus_init_mmio(sbd, &pex->mmio);
>      sysbus_init_mmio(sbd, &s->mmio);
>  
> -    pci->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
> -                                pci_swizzle_map_irq_fn, s, &s->mmio,
> -                                &s->io, 0, 4, TYPE_PCIE_BUS);
> +    pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
> +                     pci_swizzle_map_irq_fn, s, &s->mmio, &s->io, 0, 4,
> +                     TYPE_PCIE_BUS);
>  
>      qdev_set_parent_bus(DEVICE(&s->root), BUS(pci->bus));
>      qdev_init_nofail(DEVICE(&s->root));
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f60d0497ef..d3adf806e5 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,19 +421,19 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
>  }
>  
> -PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> -                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> -                         void *irq_opaque,
> -                         MemoryRegion *address_space_mem,
> -                         MemoryRegion *address_space_io,
> -                         uint8_t devfn_min, int nirq, const char *typename)
> +void pci_register_bus(PCIHostState *phb, const char *name,
> +                      pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> +                      void *irq_opaque,
> +                      MemoryRegion *address_space_mem,
> +                      MemoryRegion *address_space_io,
> +                      uint8_t devfn_min, int nirq, const char *typename)
>  {
>      PCIBus *bus;
>  
>      bus = pci_bus_new(phb, name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> -    return bus;
> +    phb->bus = bus;
>  }
>  
>  int pci_bus_num(PCIBus *s)
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index f755c6faae..ab158b3ef1 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,10 +314,10 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
>  
> -    b = pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
> -                         ppc4xx_pci_map_irq, s->irq, get_system_memory(),
> -                         get_system_io(), 0, 4, TYPE_PCI_BUS);
> -    h->bus = b;
> +    pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
> +                     ppc4xx_pci_map_irq, s->irq, get_system_memory(),
> +                     get_system_io(), 0, 4, TYPE_PCI_BUS);
> +    b = h->bus;
>  
>      pci_create_simple(b, 0, "ppc4xx-host-bridge");
>  
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 0f293f9e75..5749f14d9f 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,11 +1697,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
>  
> -    bus = pci_register_bus(phb, NULL,
> -                           pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> -                           &sphb->memspace, &sphb->iospace,
> -                           PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> -    phb->bus = bus;
> +    pci_register_bus(phb, NULL,
> +                     pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> +                     &sphb->memspace, &sphb->iospace,
> +                     PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> +    bus = phb->bus;
>      qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL);
>  
>      /*
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 99aae965bb..466067a380 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,15 +560,15 @@ static int s390_pcihost_init(SysBusDevice *dev)
>  
>      DPRINTF("host_init\n");
>  
> -    b = pci_register_bus(phb, NULL,
> -                         s390_pci_set_irq, s390_pci_map_irq, NULL,
> -                         get_system_memory(), get_system_io(), 0, 64,
> -                         TYPE_PCI_BUS);
> +    pci_register_bus(phb, NULL,
> +                     s390_pci_set_irq, s390_pci_map_irq, NULL,
> +                     get_system_memory(), get_system_io(), 0, 64,
> +                     TYPE_PCI_BUS);
> +    b = phb->bus;
>      pci_setup_iommu(b, s390_pci_dma_iommu, s);
>  
>      bus = BUS(b);
>      qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
> -    phb->bus = b;
>  
>      s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
>      qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index bca849c10f..bb43bad77d 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,12 +131,9 @@ static int sh_pci_device_init(SysBusDevice *dev)
>      for (i = 0; i < 4; i++) {
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                sh_pci_set_irq, sh_pci_map_irq,
> -                                s->irq,
> -                                get_system_memory(),
> -                                get_system_io(),
> -                                PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", sh_pci_set_irq, sh_pci_map_irq, s->irq,
> +                     get_system_memory(), get_system_io(), PCI_DEVFN(0, 0), 4,
> +                     TYPE_PCI_BUS);
>      memory_region_init_io(&s->memconfig_p4, OBJECT(s), &sh_pci_reg_ops, s,
>                            "sh_pci", 0x224);
>      memory_region_init_alias(&s->memconfig_a7, OBJECT(s), "sh_pci.2",

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()

[Cornelia Huck]

On Mon, 17 Apr 2017 18:59:14 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Every single caller of of pci_register_bus() saves the return value in
> phb->bus. Do that inside pci_register_bus() to avoid code duplication
> and make it harder to break.
> 
> Most (but not all) conversions done using the following Coccinelle script:
> 
>   @@
>   identifier b;
>   expression phb;
>   @@
>   -b = pci_register_bus(phb, ARGS);
>   +phb->bus = pci_register_bus(phb, ARGS);
>    ...
>   -phb->bus = b;
> 
>   @@
>   expression phb;
>   expression list ARGS;
>   @@
>   -phb->bus = pci_register_bus(phb, ARGS);
>   +pci_register_bus(phb, ARGS);
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/hw/pci/pci.h      | 12 ++++++------
>  hw/alpha/typhoon.c        | 10 +++++-----
>  hw/mips/gt64xxx_pci.c     |  9 +++------
>  hw/pci-host/apb.c         |  7 ++-----
>  hw/pci-host/bonito.c      |  7 +++----
>  hw/pci-host/gpex.c        |  5 ++---
>  hw/pci-host/grackle.c     |  9 ++-------
>  hw/pci-host/ppce500.c     |  8 ++++----
>  hw/pci-host/uninorth.c    | 18 ++++++------------
>  hw/pci-host/xilinx-pcie.c |  6 +++---
>  hw/pci/pci.c              | 14 +++++++-------
>  hw/ppc/ppc4xx_pci.c       |  8 ++++----
>  hw/ppc/spapr_pci.c        | 10 +++++-----
>  hw/s390x/s390-pci-bus.c   | 10 +++++-----
>  hw/sh4/sh_pci.c           |  9 +++------
>  15 files changed, 60 insertions(+), 82 deletions(-)

s390 parts:

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()

[Marcel Apfelbaum]

On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> Every single caller of of pci_register_bus() saves the return value in
> phb->bus. Do that inside pci_register_bus() to avoid code duplication
> and make it harder to break.
>

I personally find that more difficult to follow, maybe the function
name is misleading.
Maybe we can find a better function name or explain the side effect
in a comment?

Thanks,
Marcel

> Most (but not all) conversions done using the following Coccinelle script:
>
>   @@
>   identifier b;
>   expression phb;
>   @@
>   -b = pci_register_bus(phb, ARGS);
>   +phb->bus = pci_register_bus(phb, ARGS);
>    ...
>   -phb->bus = b;
>
>   @@
>   expression phb;
>   expression list ARGS;
>   @@
>   -phb->bus = pci_register_bus(phb, ARGS);
>   +pci_register_bus(phb, ARGS);
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/hw/pci/pci.h      | 12 ++++++------
>  hw/alpha/typhoon.c        | 10 +++++-----
>  hw/mips/gt64xxx_pci.c     |  9 +++------
>  hw/pci-host/apb.c         |  7 ++-----
>  hw/pci-host/bonito.c      |  7 +++----
>  hw/pci-host/gpex.c        |  5 ++---
>  hw/pci-host/grackle.c     |  9 ++-------
>  hw/pci-host/ppce500.c     |  8 ++++----
>  hw/pci-host/uninorth.c    | 18 ++++++------------
>  hw/pci-host/xilinx-pcie.c |  6 +++---
>  hw/pci/pci.c              | 14 +++++++-------
>  hw/ppc/ppc4xx_pci.c       |  8 ++++----
>  hw/ppc/spapr_pci.c        | 10 +++++-----
>  hw/s390x/s390-pci-bus.c   | 10 +++++-----
>  hw/sh4/sh_pci.c           |  9 +++------
>  15 files changed, 60 insertions(+), 82 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 56387ccb0c..3b1e2c408a 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,12 +408,12 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
>  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> -PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> -                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> -                         void *irq_opaque,
> -                         MemoryRegion *address_space_mem,
> -                         MemoryRegion *address_space_io,
> -                         uint8_t devfn_min, int nirq, const char *typename);
> +void pci_register_bus(PCIHostState *phb, const char *name,
> +                      pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> +                      void *irq_opaque,
> +                      MemoryRegion *address_space_mem,
> +                      MemoryRegion *address_space_io,
> +                      uint8_t devfn_min, int nirq, const char *typename);
>  void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
>  PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
>  bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index ac0633a55e..5926686d79 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,11 +883,11 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      memory_region_add_subregion(addr_space, 0x801fc000000ULL,
>                                  &s->pchip.reg_io);
>
> -    b = pci_register_bus(phb, "pci",
> -                         typhoon_set_irq, sys_map_irq, s,
> -                         &s->pchip.reg_mem, &s->pchip.reg_io,
> -                         0, 64, TYPE_PCI_BUS);
> -    phb->bus = b;
> +    pci_register_bus(phb, "pci",
> +                     typhoon_set_irq, sys_map_irq, s,
> +                     &s->pchip.reg_mem, &s->pchip.reg_io,
> +                     0, 64, TYPE_PCI_BUS);
> +    b = phb->bus;
>      qdev_init_nofail(dev);
>
>      /* Host memory as seen from the PCI side, via the IOMMU.  */
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index bd131bcdc6..69963453f0 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,12 +1171,9 @@ PCIBus *gt64120_register(qemu_irq *pic)
>      phb = PCI_HOST_BRIDGE(dev);
>      memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
>      address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                gt64120_pci_set_irq, gt64120_pci_map_irq,
> -                                pic,
> -                                &d->pci0_mem,
> -                                get_system_io(),
> -                                PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", gt64120_pci_set_irq, gt64120_pci_map_irq,
> +                     pic, &d->pci0_mem, get_system_io(), PCI_DEVFN(18, 0), 4,
> +                     TYPE_PCI_BUS);
>      qdev_init_nofail(dev);
>      memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d, "isd-mem", 0x1000);
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 1156a54224..ea86260b04 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,11 +671,8 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      dev = qdev_create(NULL, TYPE_APB);
>      d = APB_DEVICE(dev);
>      phb = PCI_HOST_BRIDGE(dev);
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                pci_apb_set_irq, pci_pbm_map_irq, d,
> -                                &d->pci_mmio,
> -                                get_system_io(),
> -                                0, 32, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", pci_apb_set_irq, pci_pbm_map_irq, d,
> +                     &d->pci_mmio, get_system_io(), 0, 32, TYPE_PCI_BUS);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      /* apb_config */
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 27842edc04..bbe595605d 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,10 +714,9 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                pci_bonito_set_irq, pci_bonito_map_irq, dev,
> -                                get_system_memory(), get_system_io(),
> -                                0x28, 32, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", pci_bonito_set_irq, pci_bonito_map_irq, dev,
> +                     get_system_memory(), get_system_io(), 0x28, 32,
> +                     TYPE_PCI_BUS);
>
>      return 0;
>  }
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 042d127271..66e2f3d29c 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,9 +62,8 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
>          sysbus_init_irq(sbd, &s->irq[i]);
>      }
>
> -    pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
> -                                pci_swizzle_map_irq_fn, s, &s->io_mmio,
> -                                &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
> +    pci_register_bus(pci, "pcie.0", gpex_set_irq, pci_swizzle_map_irq_fn, s,
> +                     &s->io_mmio, &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>
>      qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
>      qdev_init_nofail(DEVICE(&s->gpex_root));
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index a56c063be9..691af0a29e 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,13 +82,8 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>
> -    phb->bus = pci_register_bus(phb, NULL,
> -                                pci_grackle_set_irq,
> -                                pci_grackle_map_irq,
> -                                pic,
> -                                &d->pci_mmio,
> -                                address_space_io,
> -                                0, 4, TYPE_PCI_BUS);
> +    pci_register_bus(phb, NULL, pci_grackle_set_irq, pci_grackle_map_irq, pic,
> +                     &d->pci_mmio, address_space_io, 0, 4, TYPE_PCI_BUS);
>
>      pci_create_simple(phb->bus, 0, "grackle");
>      qdev_init_nofail(dev);
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index 4a1e99f426..43a06961d0 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,10 +465,10 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>      /* PIO lives at the bottom of our bus space */
>      memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
>
> -    b = pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
> -                         mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
> -                         PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
> -    h->bus = b;
> +    pci_register_bus(h, NULL, mpc85xx_pci_set_irq,
> +                     mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
> +                     PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
> +    b = h->bus;
>
>      /* Set up PCI view of memory */
>      memory_region_init(&s->bm, OBJECT(s), "bm-e500", UINT64_MAX);
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index b1b89183fc..4d709e5468 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,12 +233,9 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>
> -    h->bus = pci_register_bus(h, NULL,
> -                              pci_unin_set_irq, pci_unin_map_irq,
> -                              pic,
> -                              &d->pci_mmio,
> -                              address_space_io,
> -                              PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(h, NULL, pci_unin_set_irq, pci_unin_map_irq, pic,
> +                     &d->pci_mmio, address_space_io, PCI_DEVFN(11, 0), 4,
> +                     TYPE_PCI_BUS);
>
>  #if 0
>      pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north");
> @@ -299,12 +296,9 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
>      memory_region_add_subregion(address_space_mem, 0x80000000ULL,
>                                  &d->pci_hole);
>
> -    h->bus = pci_register_bus(h, NULL,
> -                              pci_unin_set_irq, pci_unin_map_irq,
> -                              pic,
> -                              &d->pci_mmio,
> -                              address_space_io,
> -                              PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(h, NULL, pci_unin_set_irq, pci_unin_map_irq, pic,
> +                     &d->pci_mmio, address_space_io, PCI_DEVFN(11, 0), 4,
> +                     TYPE_PCI_BUS);
>
>      sysbus_mmio_map(s, 0, 0xf0800000);
>      sysbus_mmio_map(s, 1, 0xf0c00000);
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index c951684148..6d53677f57 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,9 +129,9 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
>      sysbus_init_mmio(sbd, &pex->mmio);
>      sysbus_init_mmio(sbd, &s->mmio);
>
> -    pci->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
> -                                pci_swizzle_map_irq_fn, s, &s->mmio,
> -                                &s->io, 0, 4, TYPE_PCIE_BUS);
> +    pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
> +                     pci_swizzle_map_irq_fn, s, &s->mmio, &s->io, 0, 4,
> +                     TYPE_PCIE_BUS);
>
>      qdev_set_parent_bus(DEVICE(&s->root), BUS(pci->bus));
>      qdev_init_nofail(DEVICE(&s->root));
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f60d0497ef..d3adf806e5 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,19 +421,19 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
>  }
>
> -PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> -                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> -                         void *irq_opaque,
> -                         MemoryRegion *address_space_mem,
> -                         MemoryRegion *address_space_io,
> -                         uint8_t devfn_min, int nirq, const char *typename)
> +void pci_register_bus(PCIHostState *phb, const char *name,
> +                      pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> +                      void *irq_opaque,
> +                      MemoryRegion *address_space_mem,
> +                      MemoryRegion *address_space_io,
> +                      uint8_t devfn_min, int nirq, const char *typename)
>  {
>      PCIBus *bus;
>
>      bus = pci_bus_new(phb, name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> -    return bus;
> +    phb->bus = bus;
>  }
>
>  int pci_bus_num(PCIBus *s)
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index f755c6faae..ab158b3ef1 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,10 +314,10 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
>
> -    b = pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
> -                         ppc4xx_pci_map_irq, s->irq, get_system_memory(),
> -                         get_system_io(), 0, 4, TYPE_PCI_BUS);
> -    h->bus = b;
> +    pci_register_bus(h, NULL, ppc4xx_pci_set_irq,
> +                     ppc4xx_pci_map_irq, s->irq, get_system_memory(),
> +                     get_system_io(), 0, 4, TYPE_PCI_BUS);
> +    b = h->bus;
>
>      pci_create_simple(b, 0, "ppc4xx-host-bridge");
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 0f293f9e75..5749f14d9f 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,11 +1697,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
>
> -    bus = pci_register_bus(phb, NULL,
> -                           pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> -                           &sphb->memspace, &sphb->iospace,
> -                           PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> -    phb->bus = bus;
> +    pci_register_bus(phb, NULL,
> +                     pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> +                     &sphb->memspace, &sphb->iospace,
> +                     PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> +    bus = phb->bus;
>      qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL);
>
>      /*
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 99aae965bb..466067a380 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,15 +560,15 @@ static int s390_pcihost_init(SysBusDevice *dev)
>
>      DPRINTF("host_init\n");
>
> -    b = pci_register_bus(phb, NULL,
> -                         s390_pci_set_irq, s390_pci_map_irq, NULL,
> -                         get_system_memory(), get_system_io(), 0, 64,
> -                         TYPE_PCI_BUS);
> +    pci_register_bus(phb, NULL,
> +                     s390_pci_set_irq, s390_pci_map_irq, NULL,
> +                     get_system_memory(), get_system_io(), 0, 64,
> +                     TYPE_PCI_BUS);
> +    b = phb->bus;
>      pci_setup_iommu(b, s390_pci_dma_iommu, s);
>
>      bus = BUS(b);
>      qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
> -    phb->bus = b;
>
>      s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
>      qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index bca849c10f..bb43bad77d 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,12 +131,9 @@ static int sh_pci_device_init(SysBusDevice *dev)
>      for (i = 0; i < 4; i++) {
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
> -    phb->bus = pci_register_bus(phb, "pci",
> -                                sh_pci_set_irq, sh_pci_map_irq,
> -                                s->irq,
> -                                get_system_memory(),
> -                                get_system_io(),
> -                                PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
> +    pci_register_bus(phb, "pci", sh_pci_set_irq, sh_pci_map_irq, s->irq,
> +                     get_system_memory(), get_system_io(), PCI_DEVFN(0, 0), 4,
> +                     TYPE_PCI_BUS);
>      memory_region_init_io(&s->memconfig_p4, OBJECT(s), &sh_pci_reg_ops, s,
>                            "sh_pci", 0x224);
>      memory_region_init_alias(&s->memconfig_a7, OBJECT(s), "sh_pci.2",
>

Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()

[Eduardo Habkost]

On Tue, Apr 18, 2017 at 04:43:48PM +0300, Marcel Apfelbaum wrote:
> On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> > Every single caller of of pci_register_bus() saves the return value in
> > phb->bus. Do that inside pci_register_bus() to avoid code duplication
> > and make it harder to break.
> > 
> 
> I personally find that more difficult to follow, maybe the function
> name is misleading.
> Maybe we can find a better function name or explain the side effect
> in a comment?

I agree we need better function names (for both
pci_register_bus() and pci_bus_new*()). But I am not experienced
enough with the PCI code to find a good name. Any suggestions?

-- 
Eduardo

[Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new()

[Eduardo Habkost]

Every single caller of pci_bus_new() saves the return value inside
phb->bus. Do that inside pci_bus_new() to avoid code duplication and
make it harder to break.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pci-bridge/pci_expander_bridge.c | 2 --
 hw/pci-host/piix.c                  | 1 -
 hw/pci-host/q35.c                   | 6 +++---
 hw/pci/pci.c                        | 2 +-
 4 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 39d29d2230..8344ca1cc8 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -245,8 +245,6 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
     bus->address_space_io = dev->bus->address_space_io;
     bus->map_irq = pxb_map_irq_fn;
 
-    phb->bus = bus;
-
     pxb_register_bus(dev, bus, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 91fec05b38..818e4979d8 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -342,7 +342,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
     s = PCI_HOST_BRIDGE(dev);
     b = pci_bus_new(s, NULL, pci_address_space,
                     address_space_io, 0, TYPE_PCI_BUS);
-    s->bus = b;
     object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 860b47a1ba..5b41412075 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -49,9 +49,9 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
-    pci->bus = pci_bus_new(pci, "pcie.0",
-                           s->mch.pci_address_space, s->mch.address_space_io,
-                           0, TYPE_PCIE_BUS);
+    pci_bus_new(pci, "pcie.0",
+                s->mch.pci_address_space, s->mch.address_space_io,
+                0, TYPE_PCIE_BUS);
     PC_MACHINE(qdev_get_machine())->bus = pci->bus;
     qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->mch));
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d3adf806e5..486aeb7514 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -408,6 +408,7 @@ PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
 
     bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
     pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
+    phb->bus = bus;
     return bus;
 }
 
@@ -433,7 +434,6 @@ void pci_register_bus(PCIHostState *phb, const char *name,
     bus = pci_bus_new(phb, name, address_space_mem,
                       address_space_io, devfn_min, typename);
     pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
-    phb->bus = bus;
 }
 
 int pci_bus_num(PCIBus *s)
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3593 bytes --]

On Mon, Apr 17, 2017 at 06:59:15PM -0300, Eduardo Habkost wrote:
> Every single caller of pci_bus_new() saves the return value inside
> phb->bus. Do that inside pci_bus_new() to avoid code duplication and
> make it harder to break.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/pci-bridge/pci_expander_bridge.c | 2 --
>  hw/pci-host/piix.c                  | 1 -
>  hw/pci-host/q35.c                   | 6 +++---
>  hw/pci/pci.c                        | 2 +-
>  4 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 39d29d2230..8344ca1cc8 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -245,8 +245,6 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>      bus->address_space_io = dev->bus->address_space_io;
>      bus->map_irq = pxb_map_irq_fn;
>  
> -    phb->bus = bus;
> -
>      pxb_register_bus(dev, bus, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 91fec05b38..818e4979d8 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -342,7 +342,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>      s = PCI_HOST_BRIDGE(dev);
>      b = pci_bus_new(s, NULL, pci_address_space,
>                      address_space_io, 0, TYPE_PCI_BUS);
> -    s->bus = b;
>      object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
>      qdev_init_nofail(dev);
>  
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 860b47a1ba..5b41412075 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,9 +49,9 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>      sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
>      sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
>  
> -    pci->bus = pci_bus_new(pci, "pcie.0",
> -                           s->mch.pci_address_space, s->mch.address_space_io,
> -                           0, TYPE_PCIE_BUS);
> +    pci_bus_new(pci, "pcie.0",
> +                s->mch.pci_address_space, s->mch.address_space_io,
> +                0, TYPE_PCIE_BUS);
>      PC_MACHINE(qdev_get_machine())->bus = pci->bus;
>      qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
>      qdev_init_nofail(DEVICE(&s->mch));
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d3adf806e5..486aeb7514 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -408,6 +408,7 @@ PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>  
>      bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> +    phb->bus = bus;
>      return bus;
>  }
>  
> @@ -433,7 +434,6 @@ void pci_register_bus(PCIHostState *phb, const char *name,
>      bus = pci_bus_new(phb, name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> -    phb->bus = bus;
>  }
>  
>  int pci_bus_num(PCIBus *s)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace()

[Eduardo Habkost]

Every single caller of pci_bus_new_inplace() sets phb->bus to point to
'bus'. Do that inside pci_bus_new_inplace() to avoid code duplication
and make it harder to break.

Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: qemu-ppc@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pci-host/prep.c      | 2 --
 hw/pci-host/versatile.c | 1 -
 hw/pci/pci.c            | 1 +
 3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 2e2cd267f4..6efa5bc5ef 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -284,8 +284,6 @@ static void raven_pcihost_initfn(Object *obj)
     address_space_init(&s->bm_as, &s->bm, "raven-bm");
     pci_setup_iommu(&s->pci_bus, raven_pcihost_set_iommu, s);
 
-    h->bus = &s->pci_bus;
-
     object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
     pci_dev = DEVICE(&s->pci_dev);
     qdev_set_parent_bus(pci_dev, BUS(&s->pci_bus));
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 24ef87610b..630f1ac1c5 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -389,7 +389,6 @@ static void pci_vpb_init(Object *obj)
     pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
                         &s->pci_mem_space, &s->pci_io_space,
                         PCI_DEVFN(11, 0), TYPE_PCI_BUS);
-    h->bus = &s->pci_bus;
 
     object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_VERSATILE_PCI_HOST);
     qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 486aeb7514..ef226f8b41 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -397,6 +397,7 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
 {
     qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
     pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
+    phb->bus = bus;
 }
 
 PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]

On Mon, Apr 17, 2017 at 06:59:16PM -0300, Eduardo Habkost wrote:
> Every single caller of pci_bus_new_inplace() sets phb->bus to point to
> 'bus'. Do that inside pci_bus_new_inplace() to avoid code duplication
> and make it harder to break.
> 
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/pci-host/prep.c      | 2 --
>  hw/pci-host/versatile.c | 1 -
>  hw/pci/pci.c            | 1 +
>  3 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 2e2cd267f4..6efa5bc5ef 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -284,8 +284,6 @@ static void raven_pcihost_initfn(Object *obj)
>      address_space_init(&s->bm_as, &s->bm, "raven-bm");
>      pci_setup_iommu(&s->pci_bus, raven_pcihost_set_iommu, s);
>  
> -    h->bus = &s->pci_bus;
> -
>      object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
>      pci_dev = DEVICE(&s->pci_dev);
>      qdev_set_parent_bus(pci_dev, BUS(&s->pci_bus));
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 24ef87610b..630f1ac1c5 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -389,7 +389,6 @@ static void pci_vpb_init(Object *obj)
>      pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
>                          &s->pci_mem_space, &s->pci_io_space,
>                          PCI_DEVFN(11, 0), TYPE_PCI_BUS);
> -    h->bus = &s->pci_bus;
>  
>      object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_VERSATILE_PCI_HOST);
>      qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 486aeb7514..ef226f8b41 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -397,6 +397,7 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
>  {
>      qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> +    phb->bus = bus;
>  }
>  
>  PCIBus *pci_bus_new(PCIHostState *phb, const char *name,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]