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

[Marcel Apfelbaum <marcel@redhat.com>]

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 <marcel.a@redhat.com>
>>
>> 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 <marcel@redhat.com>
>
> 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