[v5,for-2.3,26/28] acpi: restrict the aml emission to PXB host bridges

Initial implementation assumed that the aml used for
any extra root buses would be generic, however this
is not always true. Restrict aml emission only to i440fx and
PXB because is the only supported combination for now.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/acpi-build.c                | 56 ++++++++++++++++++++-----------------
 hw/pci-bridge/pci_expander_bridge.c |  1 -
 include/hw/pci/pci_host.h           |  2 ++
 3 files changed, 32 insertions(+), 27 deletions(-)

On 03/10/2015 05:41 PM, Michael S. Tsirkin wrote:
> On Tue, Mar 10, 2015 at 05:32:12PM +0200, Marcel Apfelbaum wrote:
>> Initial implementation assumed that the aml used for
>> any extra root buses would be generic, however this
>> is not always true. Restrict aml emission only to i440fx and
>> PXB because is the only supported combination for now.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>
> I was wondering about this too.
> Please split this up and squash into appropriate patches.
> I know it's more work but it's worth it.
Sure,

Thanks,
Marcel
>
>
>> ---
>>   hw/i386/acpi-build.c                | 56 ++++++++++++++++++++-----------------
>>   hw/pci-bridge/pci_expander_bridge.c |  1 -
>>   include/hw/pci/pci_host.h           |  2 ++
>>   3 files changed, 32 insertions(+), 27 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index ff18a07..2bc8a80 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -887,7 +887,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>>       /* Reserve space for header */
>>       acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>>
>> -    {
>> +    if (find_i440fx()) {
>>           PciInfoList *info_list, *info;
>>           Error *err = NULL;
>>
>> @@ -901,37 +901,41 @@ build_ssdt(GArray *table_data, GArray *linker,
>>               PciInfo *bus_info = info->value;
>>               PCIHostState *host;
>>
>> -            if (bus_info->bus == 0) {
>> -                continue;
>> -            }
>> +            HOST_BRIDGE_FOREACH(host) {
>> +                int numa_node;
>>
>> -            if (bus_info->bus < root_bus_limit) {
>> -                root_bus_limit = bus_info->bus - 1;
>> -            }
>> +                if (!(pci_bus_num(host->bus) == bus_info->bus)) {
>> +                    continue;
>> +                }
>>
>> -            scope = aml_scope("\\_SB");
>> -            dev = aml_device("PC%.02X", (uint8_t)bus_info->bus);
>> -            aml_append(dev, aml_name_decl("_UID",
>> -                aml_string("PC%.02X", (uint8_t)bus_info->bus)));
>> -            aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A03")));
>> -            aml_append(dev,
>> -                aml_name_decl("_BBN", aml_int((uint8_t)bus_info->bus)));
>> +                if (!object_dynamic_cast(OBJECT(host), TYPE_PXB_HOST)) {
>> +                    break;
>> +                }
>
> Or you can check the device/vendor id if that's easier.
>
>>
>> -            HOST_BRIDGE_FOREACH(host) {
>> -                if (pci_bus_num(host->bus) == bus_info->bus) {
>> -                    int numa_node = pci_bus_numa_node(host->bus);
>> -                    if (numa_node != NUMA_NODE_UNASSIGNED) {
>> -                        aml_append(dev,
>> +                if (bus_info->bus < root_bus_limit) {
>> +                    root_bus_limit = bus_info->bus - 1;
>> +                }
>> +
>> +                scope = aml_scope("\\_SB");
>> +                dev = aml_device("PC%.02X", (uint8_t)bus_info->bus);
>> +                aml_append(dev, aml_name_decl("_UID",
>> +                            aml_string("PC%.02X", (uint8_t)bus_info->bus)));
>> +                aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A03")));
>> +                aml_append(dev,
>> +                        aml_name_decl("_BBN", aml_int((uint8_t)bus_info->bus)));
>> +
>> +                numa_node = pci_bus_numa_node(host->bus);
>> +                if (numa_node != NUMA_NODE_UNASSIGNED) {
>> +                    aml_append(dev,
>>                               aml_name_decl("_PXM", aml_int(numa_node)));
>> -                    }
>>                   }
>> -            }
>>
>> -            aml_append(dev, build_prt());
>> -            crs = build_crs(pci, bus_info, &io_ranges, &mem_ranges);
>> -            aml_append(dev, aml_name_decl("_CRS", crs));
>> -            aml_append(scope, dev);
>> -            aml_append(ssdt, scope);
>> +                aml_append(dev, build_prt());
>> +                crs = build_crs(pci, bus_info, &io_ranges, &mem_ranges);
>> +                aml_append(dev, aml_name_decl("_CRS", crs));
>> +                aml_append(scope, dev);
>> +                aml_append(ssdt, scope);
>> +            }
>>           }
>>
>>           qapi_free_PciInfoList(info_list);
>> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
>> index 9329aab..8e6740e 100644
>> --- a/hw/pci-bridge/pci_expander_bridge.c
>> +++ b/hw/pci-bridge/pci_expander_bridge.c
>> @@ -41,7 +41,6 @@ typedef struct PXBDev {
>>       uint16_t numa_node;
>>   } PXBDev;
>>
>> -#define TYPE_PXB_HOST "pxb-host"
>>
>>   static int pxb_bus_num(PCIBus *bus)
>>   {
>> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
>> index ba5272f..9a35389 100644
>> --- a/include/hw/pci/pci_host.h
>> +++ b/include/hw/pci/pci_host.h
>> @@ -30,6 +30,8 @@
>>
>>   #include "hw/sysbus.h"
>>
>> +#define TYPE_PXB_HOST "pxb-host"
>> +
>
> That's a wrong place for it I think.
>
>>   /**
>>    * Marker interface for classes whose instances can
>>    * be main host bridges. It is intended to be used
>> --
>> 2.1.0