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 03:48 PM, Michael S. Tsirkin wrote:
> On Mon, Apr 27, 2015 at 03:30:01PM +0300, Marcel Apfelbaum wrote:
>> 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.
>
> It's related since you will want to describe all host bridges in acpi.
When I'll submit the pxb hotplug support it will be more clear,
Basically, I'll go over the pxbs leaving the default host bridge alone.

>
>>
>>   How about an interface to
>>> iterate over objects by type?
>>> You can then blacklist pxbs for now.
>> Less elegant in my opinion,
>
> Right but it will be needed in the future anyway.
I think we should leave this for when we'll need it, this series
is already long enough without adding a new QOM iteration type.

Thanks,
Marcel

>
>>>
>>>> ---
>>>>   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