On Tue, 31 May 2016 20:48:33 +0300 Marcel Apfelbaum <mar...@redhat.com> wrote:
> Get the pci root-bus once and pass it around. > > Signed-off-by: Marcel Apfelbaum <mar...@redhat.com> > --- > hw/i386/acpi-build.c | 58 > +++++++++++++++++++++++----------------------------- > 1 file changed, 26 insertions(+), 32 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 9aef25d..c6f4afe 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1990,7 +1990,7 @@ build_dsdt(GArray *table_data, GArray *linker, > PCMachineState *pcms = PC_MACHINE(machine); > uint32_t nr_mem = machine->ram_slots; > int root_bus_limit = 0xFF; > - PCIBus *bus = NULL; > + PCIBus *root_bus = NULL; > int i; > > dsdt = init_aml_allocator(); > @@ -2033,6 +2033,9 @@ build_dsdt(GArray *table_data, GArray *linker, > build_q35_pci0_int(dsdt); > } > > + root_bus = PC_MACHINE(machine)->bus; > + assert(root_bus); nit: I'd put it in place of removed bus = PC_MACHINE(machine)->bus; in the next hunk > + > build_cpu_hotplug_aml(dsdt); > build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base, > pm->mem_hp_io_len); > @@ -2077,8 +2080,8 @@ build_dsdt(GArray *table_data, GArray *linker, > } > aml_append(dsdt, scope); > > - bus = PC_MACHINE(machine)->bus; > - if (bus) { > + { > + PCIBus *bus = root_bus; > QLIST_FOREACH(bus, &bus->child, sibling) { > uint8_t bus_num = pci_bus_num(bus); > uint8_t numa_node = pci_bus_numa_node(bus); > @@ -2318,38 +2321,29 @@ build_dsdt(GArray *table_data, GArray *linker, > pm->mem_hp_io_len); > > { > - Object *pci_host; > - PCIBus *bus = NULL; > + Aml *scope = aml_scope("PCI0"); > > - pci_host = acpi_get_i386_pci_host(); perhaps it's worth to replace acpi_get_i386_pci_host() usage in other places with PC_MACHINE(machine)->bus as well and remove acpi_get_i386_pci_host() > - if (pci_host) { > - bus = PCI_HOST_BRIDGE(pci_host)->bus; > - } > - > - if (bus) { > - Aml *scope = aml_scope("PCI0"); > - /* Scan all PCI buses. Generate tables to support hotplug. */ > - build_append_pci_bus_devices(scope, bus, > pm->pcihp_bridge_en); > - > - if (misc->tpm_version != TPM_VERSION_UNSPEC) { > - dev = aml_device("ISA.TPM"); > - aml_append(dev, aml_name_decl("_HID", > aml_eisaid("PNP0C31"))); > - aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > - crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE, > - TPM_TIS_ADDR_SIZE, AML_READ_WRITE)); > - /* > - FIXME: TPM_TIS_IRQ=5 conflicts with PNP0C0F irqs, > - Rewrite to take IRQ from TPM device model and > - fix default IRQ value there to use some unused IRQ > - */ > - /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > - } > + /* Scan all PCI buses. Generate tables to support hotplug. */ > + build_append_pci_bus_devices(scope, root_bus, > + pm->pcihp_bridge_en); no need for line break here, it all fits in 80 column limit > > - aml_append(sb_scope, scope); > + if (misc->tpm_version != TPM_VERSION_UNSPEC) { > + dev = aml_device("ISA.TPM"); > + aml_append(dev, aml_name_decl("_HID", > aml_eisaid("PNP0C31"))); > + aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > + crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE, > + TPM_TIS_ADDR_SIZE, AML_READ_WRITE)); > + /* > + FIXME: TPM_TIS_IRQ=5 conflicts with PNP0C0F irqs, > + Rewrite to take IRQ from TPM device model and > + fix default IRQ value there to use some unused IRQ > + */ > + /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > } > + aml_append(sb_scope, scope); > } > aml_append(dsdt, sb_scope); > } Otherwise looks good, Reviewed-by: Igor Mammedov <imamm...@redhat.com>