Re: [Qemu-devel] [PATCH V6 for-2.3 12/26] hw/pci: introduce TYPE_PCI_MAIN_HOST_BRIDGE interface

[Marcel Apfelbaum]

On 04/27/2015 02:24 PM, Michael S. Tsirkin wrote:
> On Thu, Mar 19, 2015 at 08:52:47PM +0200, Marcel Apfelbaum wrote:
> > From: Marcel Apfelbaum <address@hidden>
> >
> > This is a marker interface used to differentiate the
> > "default" host bridge on a system with multiple host bridges.
> > This differentiation is required only for pc machines for now
> > by the ACPI subsystem.
> >
> > Signed-off-by: Marcel Apfelbaum <address@hidden>
>
> Fixing hotplug for pxb almost for sure means we'll need
> to drop this later.
Not really, I am not sure how it is related. I'll go over the pxb pci devices
for this hotplug. I have no intention to drop this later.
This interface denotes the main/default/bus 0 host bridge.


 How about an interface to
> iterate over objects by type?
> You can then blacklist pxbs for now.
Less elegant in my opinion,
> > ---
> >   hw/i386/acpi-build.c      | 9 ++++++---
> >   hw/pci-host/piix.c        | 5 +++++
> >   hw/pci-host/q35.c         | 4 ++++
> >   hw/pci/pci_host.c         | 6 ++++++
> >   include/hw/pci/pci_host.h | 7 +++++++
> >   5 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index d0a5c85..86e474a 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -249,7 +249,8 @@ static void acpi_get_pci_info(PcPciInfo *info)
> >       Object *pci_host;
> >       bool ambiguous;
> >
> > -    pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, &ambiguous);
> > +    pci_host = object_resolve_path_type("", TYPE_PCI_MAIN_HOST_BRIDGE,
> > +                                        &ambiguous);
> >       g_assert(!ambiguous);
> >       g_assert(pci_host);
> >
> > @@ -993,7 +994,8 @@ build_ssdt(GArray *table_data, GArray *linker,
> >               PCIBus *bus = NULL;
> >               bool ambiguous;
> >
> > -            pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, 
> > &ambiguous);
> > +            pci_host = object_resolve_path_type("", TYPE_PCI_MAIN_HOST_BRIDGE,
> > +                                                &ambiguous);
> >               if (!ambiguous && pci_host) {
> >                   bus = PCI_HOST_BRIDGE(pci_host)->bus;
> >               }
> > @@ -1338,7 +1340,8 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
> >       QObject *o;
> >       bool ambiguous;
> >
> > -    pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, &ambiguous);
> > +    pci_host = object_resolve_path_type("", TYPE_PCI_MAIN_HOST_BRIDGE,
> > +                                        &ambiguous);
> >       g_assert(!ambiguous);
> >       g_assert(pci_host);
> >
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index 292b6e9..0033ab4 100644
> > --- a/hw/pci-host/piix.c
> > +++ b/hw/pci-host/piix.c
> > @@ -766,6 +766,11 @@ static const TypeInfo i440fx_pcihost_info = {
> >       .instance_size = sizeof(I440FXState),
> >       .instance_init = i440fx_pcihost_initfn,
> >       .class_init    = i440fx_pcihost_class_init,
> > +    .interfaces    = (InterfaceInfo[]) {
> > +        { TYPE_PCI_MAIN_HOST_BRIDGE },
> > +        { }
> > +    }
> > +
> >   };
> >
> >   static void i440fx_register_types(void)
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index 5dd559e..e35b3a2 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -193,6 +193,10 @@ static const TypeInfo q35_host_info = {
> >       .instance_size = sizeof(Q35PCIHost),
> >       .instance_init = q35_host_initfn,
> >       .class_init = q35_host_class_init,
> > +    .interfaces = (InterfaceInfo[]) {
> > +        { TYPE_PCI_MAIN_HOST_BRIDGE },
> > +        { }
> > +    }
> >   };
> >
> >   /****************************************************************************
> > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> > index 3e26f92..87180c8 100644
> > --- a/hw/pci/pci_host.c
> > +++ b/hw/pci/pci_host.c
> > @@ -175,6 +175,11 @@ const MemoryRegionOps pci_host_data_be_ops = {
> >       .endianness = DEVICE_BIG_ENDIAN,
> >   };
> >
> > +static const TypeInfo pci_main_host_interface_info = {
> > +    .name          = TYPE_PCI_MAIN_HOST_BRIDGE,
> > +    .parent        = TYPE_INTERFACE,
> > +};
> > +
> >   static const TypeInfo pci_host_type_info = {
> >       .name = TYPE_PCI_HOST_BRIDGE,
> >       .parent = TYPE_SYS_BUS_DEVICE,
> > @@ -185,6 +190,7 @@ static const TypeInfo pci_host_type_info = {
> >
> >   static void pci_host_register_types(void)
> >   {
> > +    type_register_static(&pci_main_host_interface_info);
> >       type_register_static(&pci_host_type_info);
> >   }
> >
> > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> > index ba31595..3c72e26 100644
> > --- a/include/hw/pci/pci_host.h
> > +++ b/include/hw/pci/pci_host.h
> > @@ -30,6 +30,13 @@
> >
> >   #include "hw/sysbus.h"
> >
> > +/**
> > + * Marker interface for classes whose instances can
> > + * be main host bridges. It is intended to be used
> > + * when the QOM tree includes multiple host bridges.
> > + */
> > +#define TYPE_PCI_MAIN_HOST_BRIDGE "pci-main-host-bridge"
> > +
> >   #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge"
> >   #define PCI_HOST_BRIDGE(obj) \
> >       OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST_BRIDGE)
> > --
> > 2.1.0