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 11:54 PM, Michael S. Tsirkin wrote:
> On Mon, Apr 27, 2015 at 04:04:38PM +0300, Marcel Apfelbaum wrote:
>> 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.
>
> So instead, just go over all roots.
I am not sure it will work that way. We'll know more after I will work on hotplug.
>
>>>
>>>>
>>>>   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
>
> Do you have hotplug working?
> Maybe wait until it's there, then we can discuss.
I agree, I hope I'll have something next week.
For next version I'll keep this around.

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