Re: [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root()

[Eduardo Habkost <ehabkost@redhat.com>]

On Tue, Oct 03, 2017 at 08:14:22PM +1100, David Gibson wrote:
> pci_bus_is_root() currently relies on a method in the PCIBusClass.
> But it's always known if a PCI bus is a root bus when we create it, so
> using a dynamic method is overkill.
> 
> This replaces it with an IS_ROOT bit in a new flags field, which is set on
> root buses and otherwise clear.  As a bonus this removes the special
> is_root logic from pci_expander_bridge, since it already creates its bus
> as a root bus.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> # Conflicts:
> #	include/hw/pci/pci_bus.h

Should this be part of the commit message?

> ---
[...]
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 77d92a3dc4..cbb3386207 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
>  #define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
>  #define TYPE_PCIE_BUS "PCIE"
>  
> +enum PCIBusFlags {
> +    /* This bus is the root of a PCI domain */
> +    PCI_BUS_IS_ROOT                                         = 0x0001,
> +};
> +
>  typedef struct PCIBusClass {
>      /*< private >*/
>      BusClass parent_class;
> @@ -416,6 +421,7 @@ typedef struct PCIBusClass {
>  
>  struct PCIBus {
>      BusState qbus;
> +    enum PCIBusFlags flags;

Why not a simple boolean field?  If we want to keep the struct
size smaller when adding more flags, we can use bit-fields.

>      PCIIOMMUFunc iommu_fn;
>      void *iommu_opaque;
>      uint8_t devfn_min;
> @@ -440,8 +446,12 @@ struct PCIBus {
>      Notifier machine_done;
>  };
>  
> +static inline bool pci_bus_is_root(PCIBus *bus)
> +{
> +    return !!(bus->flags & PCI_BUS_IS_ROOT);
> +}
> +
>  bool pci_bus_is_express(PCIBus *bus);
> -bool pci_bus_is_root(PCIBus *bus);
>  void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
>                                const char *name,
>                                MemoryRegion *address_space_mem,
> -- 
> 2.13.6
> 

-- 
Eduardo