Re: [Qemu-devel] [RFC v2 2/6] pci: Move pci_bus_init() logic to pci_bus_new_inplace()

From: David Gibson <david@gibson.dropbear.id.au>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org, aik@ozlabs.ru,
	David Gibson <dgibson@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Laszlo Ersek <lersek@redhat.com>,
	Marcel Apfelbaum <marcel@redhat.com>
Subject: Re: [Qemu-devel] [RFC v2 2/6] pci: Move pci_bus_init() logic to pci_bus_new_inplace()
Date: Wed, 19 Apr 2017 10:23:14 +1000	[thread overview]
Message-ID: <20170419002314.GB23273@umbus.fritz.box> (raw)
In-Reply-To: <20170418221724.5707-3-ehabkost@redhat.com>

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

On Tue, Apr 18, 2017 at 07:17:20PM -0300, Eduardo Habkost wrote:
> Instead of having 3 separate functions, just make pci_bus_new() a
> wrapper that allocates the object and calls pci_bus_new_inplace().
> 
> 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 | 39 +++++++++++++++++----------------------
>  1 file changed, 17 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 328f36cd21..0d28ee4e3f 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -357,24 +357,6 @@ const char *pci_root_bus_path(PCIDevice *dev)
>      return rootbus->qbus.name;
>  }
>  
> -static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> -                         MemoryRegion *address_space_mem,
> -                         MemoryRegion *address_space_io,
> -                         uint8_t devfn_min)
> -{
> -    PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> -
> -    assert(PCI_FUNC(devfn_min) == 0);
> -    bus->devfn_min = devfn_min;
> -    bus->address_space_mem = address_space_mem;
> -    bus->address_space_io = address_space_io;
> -
> -    /* host bridge */
> -    QLIST_INIT(&bus->child);
> -
> -    QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
> -}
> -
>  bool pci_bus_is_express(PCIBus *bus)
>  {
>      return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> @@ -391,8 +373,20 @@ 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);
> +
> +    assert(PCI_FUNC(devfn_min) == 0);
> +    bus->devfn_min = devfn_min;
> +    bus->address_space_mem = address_space_mem;
> +    bus->address_space_io = address_space_io;
> +
> +    /* host bridge */
> +    QLIST_INIT(&bus->child);
> +
> +    QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
> +
>  }
>  
>  PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> @@ -400,10 +394,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
>                      MemoryRegion *address_space_io,
>                      uint8_t devfn_min, const char *typename)
>  {
> -    PCIBus *bus;
> +    size_t bus_size = object_type_get_instance_size(typename);
> +    PCIBus *bus = g_malloc(bus_size);
>  
> -    bus = PCI_BUS(qbus_create(typename, parent, name));
> -    pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> +    pci_bus_new_inplace(bus, bus_size, parent, name, address_space_mem,
> +                        address_space_io, devfn_min, typename);
>      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 --]