| Message ID | 1464716918-29689-7-git-send-email-marcel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 31 May 2016 20:48:37 +0300 Marcel Apfelbaum <marcel@redhat.com> wrote: subj doesn't match patch, it does opposite i.e. makes sure that pcie isn't counted in when building acpi hotplug aml > Emit acpi code for pci hotplug on all PC machines: > - if legacy pci hotpug is enabled (pcihp_bridge_en) > - if there is at least one legacy pci bridge in current system > - for PCI buses only (skip PCIe buses) > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > --- > hw/i386/acpi-build.c | 49 > ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 > insertions(+), 17 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 8ae3e53..cc51f9e 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -250,6 +250,11 @@ static void acpi_get_pci_info(PcPciInfo *info) > NULL); > } > > +static void acpi_test_pci_bus(PCIBus *bus, void *opaque) > +{ > + *(bool *)opaque |= !pci_bus_is_express(bus); > +} > + > #define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is > APM_CNT_IOPORT */ > static void acpi_align_size(GArray *blob, unsigned align) > @@ -422,7 +427,7 @@ static void *acpi_set_bsel(PCIBus *bus, void > *opaque) unsigned *bsel_alloc = opaque; > unsigned *bus_bsel; > > - if (qbus_is_hotpluggable(BUS(bus))) { > + if (qbus_is_hotpluggable(BUS(bus)) && !pci_bus_is_express(bus)) { > bus_bsel = g_malloc(sizeof *bus_bsel); > > *bus_bsel = (*bsel_alloc)++; > @@ -433,15 +438,12 @@ static void *acpi_set_bsel(PCIBus *bus, void > *opaque) return bsel_alloc; > } > > -static void acpi_set_pci_info(void) > +static void acpi_set_pci_info(PCIBus *bus) > { > - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ > unsigned bsel_alloc = 0; > > - if (bus) { > - /* Scan all PCI buses. Set property to enable acpi based > hotplug. */ > - pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, > &bsel_alloc); > - } > + /* Scan all PCI buses. Set property to enable acpi based > hotplug. */ > + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, > &bsel_alloc); } > > static void build_append_pcihp_notify_entry(Aml *method, int slot) > @@ -503,7 +505,7 @@ static void build_append_pci_bus_devices(Aml > *parent_scope, PCIBus *bus, > * In this case they aren't themselves hot-pluggable. > * Hotplugged bridges *are* hot-pluggable. > */ > - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && > + bridge_in_acpi = pc->is_bridge && !pc->is_express && > pcihp_bridge_en && !DEVICE(pdev)->hotplugged; > > hotplug_enabled_dev = bsel && dc->hotpluggable > && !bridge_in_acpi; @@ -1892,7 +1894,7 @@ static void > build_piix4_isa_bridge(Aml *table) aml_append(table, scope); > } > > -static void build_piix4_pci_hotplug(Aml *table) > +static void build_acpi_pci_hotplug(Aml *table) > { > Aml *scope; > Aml *field; > @@ -1995,6 +1997,7 @@ build_dsdt(GArray *table_data, GArray *linker, > uint32_t nr_mem = machine->ram_slots; > int root_bus_limit = 0xFF; > PCIBus *root_bus = NULL; > + bool has_pci_bus = false; > int i; > > dsdt = init_aml_allocator(); > @@ -2016,7 +2019,6 @@ build_dsdt(GArray *table_data, GArray *linker, > build_piix4_pm(dsdt); > build_piix4_isa_bridge(dsdt); > build_isa_devices_aml(dsdt); > - build_piix4_pci_hotplug(dsdt); > build_piix4_pci0_int(dsdt); > } else { > sb_scope = aml_scope("_SB"); > @@ -2040,6 +2042,11 @@ build_dsdt(GArray *table_data, GArray *linker, > root_bus = PC_MACHINE(machine)->bus; > assert(root_bus); > > + pci_for_each_bus(root_bus, acpi_test_pci_bus, &has_pci_bus); > + if (pm->pcihp_bridge_en && has_pci_bus) { ^^^^^ wouldn't that forbid acpi hotplug on PCI0 is it's not set? > + build_acpi_pci_hotplug(dsdt); > + } > + > build_cpu_hotplug_aml(dsdt); > build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base, > pm->mem_hp_io_len); > @@ -2050,7 +2057,7 @@ build_dsdt(GArray *table_data, GArray *linker, > > aml_append(scope, aml_method("_L00", 0, AML_NOTSERIALIZED)); > > - if (misc->is_piix4) { > + if (pm->pcihp_bridge_en && has_pci_bus) { same question as above > method = aml_method("_E01", 0, AML_NOTSERIALIZED); > aml_append(method, > aml_acquire(aml_name("\\_SB.PCI0.BLCK"), 0xFFFF)); > @@ -2188,7 +2195,7 @@ build_dsdt(GArray *table_data, GArray *linker, > g_ptr_array_free(mem_ranges, true); > > /* reserve PCIHP resources */ > - if (pm->pcihp_io_len) { > + if (pm->pcihp_bridge_en && has_pci_bus && pm->pcihp_io_len) { same question as above > dev = aml_device("PHPR"); > aml_append(dev, aml_name_decl("_HID", > aml_string("PNP0A06"))); aml_append(dev, > @@ -2326,10 +2333,14 @@ build_dsdt(GArray *table_data, GArray *linker, > > { > Aml *scope = aml_scope("PCI0"); > + bool scope_used = false; Could you explain why ^^^ is needed? > > - /* Scan all PCI buses. Generate tables to support > hotplug. */ > - build_append_pci_bus_devices(scope, root_bus, > - pm->pcihp_bridge_en); > + if (pm->pcihp_bridge_en && has_pci_bus) { > + /* Scan all PCI buses. Generate tables to support > hotplug. */ > + build_append_pci_bus_devices(scope, root_bus, > + pm->pcihp_bridge_en); > + scope_used = true; > + } > > if (misc->tpm_version != TPM_VERSION_UNSPEC) { > dev = aml_device("ISA.TPM"); > @@ -2346,8 +2357,12 @@ build_dsdt(GArray *table_data, GArray *linker, > /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > aml_append(dev, aml_name_decl("_CRS", crs)); > aml_append(scope, dev); > + scope_used = true; > + } > + > + if (scope_used) { > + aml_append(sb_scope, scope); > } > - aml_append(sb_scope, scope); > } > aml_append(dsdt, sb_scope); > } > @@ -2870,7 +2885,7 @@ void acpi_setup(void) > > build_state = g_malloc0(sizeof *build_state); > > - acpi_set_pci_info(); > + acpi_set_pci_info(pcms->bus); > > acpi_build_tables_init(&tables); > acpi_build(&tables, MACHINE(pcms));
On 06/20/2016 05:11 PM, Igor Mammedov wrote: > On Tue, 31 May 2016 20:48:37 +0300 > Marcel Apfelbaum <marcel@redhat.com> wrote: > > subj doesn't match patch, it does opposite i.e. makes sure that > pcie isn't counted in when building acpi hotplug aml > Maybe I wasn't clear enough. Until now q35 had no acpi hotplug code. This patch adds acpi hotplug code for all Q35 PCI buses, but no for pcie buses That means for pci-pci bridge and dmi-pci bridge. Maybe I should rename it to hw/acpi: extend acpi pci hotplug support for Q35 pci buses. >> Emit acpi code for pci hotplug on all PC machines: >> - if legacy pci hotpug is enabled (pcihp_bridge_en) >> - if there is at least one legacy pci bridge in current system >> - for PCI buses only (skip PCIe buses) >> >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> >> --- >> hw/i386/acpi-build.c | 49 >> ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 >> insertions(+), 17 deletions(-) >> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 8ae3e53..cc51f9e 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -250,6 +250,11 @@ static void acpi_get_pci_info(PcPciInfo *info) >> NULL); >> } >> >> +static void acpi_test_pci_bus(PCIBus *bus, void *opaque) >> +{ >> + *(bool *)opaque |= !pci_bus_is_express(bus); >> +} >> + >> #define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is >> APM_CNT_IOPORT */ >> static void acpi_align_size(GArray *blob, unsigned align) >> @@ -422,7 +427,7 @@ static void *acpi_set_bsel(PCIBus *bus, void >> *opaque) unsigned *bsel_alloc = opaque; >> unsigned *bus_bsel; >> >> - if (qbus_is_hotpluggable(BUS(bus))) { >> + if (qbus_is_hotpluggable(BUS(bus)) && !pci_bus_is_express(bus)) { >> bus_bsel = g_malloc(sizeof *bus_bsel); >> >> *bus_bsel = (*bsel_alloc)++; >> @@ -433,15 +438,12 @@ static void *acpi_set_bsel(PCIBus *bus, void >> *opaque) return bsel_alloc; >> } >> >> -static void acpi_set_pci_info(void) >> +static void acpi_set_pci_info(PCIBus *bus) >> { >> - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ >> unsigned bsel_alloc = 0; >> >> - if (bus) { >> - /* Scan all PCI buses. Set property to enable acpi based >> hotplug. */ >> - pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, >> &bsel_alloc); >> - } >> + /* Scan all PCI buses. Set property to enable acpi based >> hotplug. */ >> + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, >> &bsel_alloc); } >> >> static void build_append_pcihp_notify_entry(Aml *method, int slot) >> @@ -503,7 +505,7 @@ static void build_append_pci_bus_devices(Aml >> *parent_scope, PCIBus *bus, >> * In this case they aren't themselves hot-pluggable. >> * Hotplugged bridges *are* hot-pluggable. >> */ >> - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && >> + bridge_in_acpi = pc->is_bridge && !pc->is_express && >> pcihp_bridge_en && !DEVICE(pdev)->hotplugged; >> >> hotplug_enabled_dev = bsel && dc->hotpluggable >> && !bridge_in_acpi; @@ -1892,7 +1894,7 @@ static void >> build_piix4_isa_bridge(Aml *table) aml_append(table, scope); >> } >> >> -static void build_piix4_pci_hotplug(Aml *table) >> +static void build_acpi_pci_hotplug(Aml *table) >> { >> Aml *scope; >> Aml *field; >> @@ -1995,6 +1997,7 @@ build_dsdt(GArray *table_data, GArray *linker, >> uint32_t nr_mem = machine->ram_slots; >> int root_bus_limit = 0xFF; >> PCIBus *root_bus = NULL; >> + bool has_pci_bus = false; >> int i; >> >> dsdt = init_aml_allocator(); >> @@ -2016,7 +2019,6 @@ build_dsdt(GArray *table_data, GArray *linker, >> build_piix4_pm(dsdt); >> build_piix4_isa_bridge(dsdt); >> build_isa_devices_aml(dsdt); >> - build_piix4_pci_hotplug(dsdt); >> build_piix4_pci0_int(dsdt); >> } else { >> sb_scope = aml_scope("_SB"); >> @@ -2040,6 +2042,11 @@ build_dsdt(GArray *table_data, GArray *linker, >> root_bus = PC_MACHINE(machine)->bus; >> assert(root_bus); >> >> + pci_for_each_bus(root_bus, acpi_test_pci_bus, &has_pci_bus); >> + if (pm->pcihp_bridge_en && has_pci_bus) { > ^^^^^ wouldn't that forbid acpi hotplug on PCI0 is it's not > set? Yes, and this is by design. 'pcihp_bridge_en' flag matches 'acpi hotplug'. I followed the initialization phase, am I wrong? Can you have acpi hotplug if the bit is off? > >> + build_acpi_pci_hotplug(dsdt); >> + } >> + >> build_cpu_hotplug_aml(dsdt); >> build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base, >> pm->mem_hp_io_len); >> @@ -2050,7 +2057,7 @@ build_dsdt(GArray *table_data, GArray *linker, >> >> aml_append(scope, aml_method("_L00", 0, AML_NOTSERIALIZED)); >> >> - if (misc->is_piix4) { >> + if (pm->pcihp_bridge_en && has_pci_bus) { > same question as above > >> method = aml_method("_E01", 0, AML_NOTSERIALIZED); >> aml_append(method, >> aml_acquire(aml_name("\\_SB.PCI0.BLCK"), 0xFFFF)); >> @@ -2188,7 +2195,7 @@ build_dsdt(GArray *table_data, GArray *linker, >> g_ptr_array_free(mem_ranges, true); >> >> /* reserve PCIHP resources */ >> - if (pm->pcihp_io_len) { >> + if (pm->pcihp_bridge_en && has_pci_bus && pm->pcihp_io_len) { > same question as above > >> dev = aml_device("PHPR"); >> aml_append(dev, aml_name_decl("_HID", >> aml_string("PNP0A06"))); aml_append(dev, >> @@ -2326,10 +2333,14 @@ build_dsdt(GArray *table_data, GArray *linker, >> >> { >> Aml *scope = aml_scope("PCI0"); >> + bool scope_used = false; > Could you explain why ^^^ is needed? > Sure. If none ACPI code will be generated by the below 'if' statements we will have an empty PCI0 scope. I wanted to get rid of it. Thanks, Marcel >> >> - /* Scan all PCI buses. Generate tables to support >> hotplug. */ >> - build_append_pci_bus_devices(scope, root_bus, >> - pm->pcihp_bridge_en); >> + if (pm->pcihp_bridge_en && has_pci_bus) { >> + /* Scan all PCI buses. Generate tables to support >> hotplug. */ >> + build_append_pci_bus_devices(scope, root_bus, >> + pm->pcihp_bridge_en); >> + scope_used = true; >> + } >> >> if (misc->tpm_version != TPM_VERSION_UNSPEC) { >> dev = aml_device("ISA.TPM"); >> @@ -2346,8 +2357,12 @@ build_dsdt(GArray *table_data, GArray *linker, >> /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ >> aml_append(dev, aml_name_decl("_CRS", crs)); >> aml_append(scope, dev); >> + scope_used = true; >> + } >> + >> + if (scope_used) { >> + aml_append(sb_scope, scope); >> } >> - aml_append(sb_scope, scope); >> } >> aml_append(dsdt, sb_scope); >> } >> @@ -2870,7 +2885,7 @@ void acpi_setup(void) >> >> build_state = g_malloc0(sizeof *build_state); >> >> - acpi_set_pci_info(); >> + acpi_set_pci_info(pcms->bus); >> >> acpi_build_tables_init(&tables); >> acpi_build(&tables, MACHINE(pcms)); >
On Tue, 21 Jun 2016 12:05:12 +0300 Marcel Apfelbaum <marcel@redhat.com> wrote: > On 06/20/2016 05:11 PM, Igor Mammedov wrote: > > On Tue, 31 May 2016 20:48:37 +0300 > > Marcel Apfelbaum <marcel@redhat.com> wrote: > > > > subj doesn't match patch, it does opposite i.e. makes sure that > > pcie isn't counted in when building acpi hotplug aml > > > > Maybe I wasn't clear enough. Until now q35 had no acpi hotplug code. > This patch adds acpi hotplug code for all Q35 PCI buses, but no for > pcie buses That means for pci-pci bridge and dmi-pci bridge. > > > Maybe I should rename it to hw/acpi: extend acpi pci hotplug support > for Q35 pci buses. this looks better to me > > >> Emit acpi code for pci hotplug on all PC machines: > >> - if legacy pci hotpug is enabled (pcihp_bridge_en) > >> - if there is at least one legacy pci bridge in current system > >> - for PCI buses only (skip PCIe buses) > >> > >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > >> --- > >> hw/i386/acpi-build.c | 49 > >> ++++++++++++++++++++++++++++++++----------------- 1 file changed, > >> 32 insertions(+), 17 deletions(-) > >> > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >> index 8ae3e53..cc51f9e 100644 > >> --- a/hw/i386/acpi-build.c > >> +++ b/hw/i386/acpi-build.c > >> @@ -250,6 +250,11 @@ static void acpi_get_pci_info(PcPciInfo *info) > >> NULL); > >> } > >> > >> +static void acpi_test_pci_bus(PCIBus *bus, void *opaque) > >> +{ > >> + *(bool *)opaque |= !pci_bus_is_express(bus); > >> +} > >> + > >> #define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is > >> APM_CNT_IOPORT */ > >> static void acpi_align_size(GArray *blob, unsigned align) > >> @@ -422,7 +427,7 @@ static void *acpi_set_bsel(PCIBus *bus, void > >> *opaque) unsigned *bsel_alloc = opaque; > >> unsigned *bus_bsel; > >> > >> - if (qbus_is_hotpluggable(BUS(bus))) { > >> + if (qbus_is_hotpluggable(BUS(bus)) > >> && !pci_bus_is_express(bus)) { bus_bsel = g_malloc(sizeof > >> *bus_bsel); > >> > >> *bus_bsel = (*bsel_alloc)++; > >> @@ -433,15 +438,12 @@ static void *acpi_set_bsel(PCIBus *bus, void > >> *opaque) return bsel_alloc; > >> } > >> > >> -static void acpi_set_pci_info(void) > >> +static void acpi_set_pci_info(PCIBus *bus) > >> { > >> - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ > >> unsigned bsel_alloc = 0; > >> > >> - if (bus) { > >> - /* Scan all PCI buses. Set property to enable acpi based > >> hotplug. */ > >> - pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, > >> &bsel_alloc); > >> - } > >> + /* Scan all PCI buses. Set property to enable acpi based > >> hotplug. */ > >> + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, > >> &bsel_alloc); } > >> > >> static void build_append_pcihp_notify_entry(Aml *method, int > >> slot) @@ -503,7 +505,7 @@ static void > >> build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, > >> * In this case they aren't themselves hot-pluggable. > >> * Hotplugged bridges *are* hot-pluggable. > >> */ > >> - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && > >> + bridge_in_acpi = pc->is_bridge && !pc->is_express && > >> pcihp_bridge_en && !DEVICE(pdev)->hotplugged; > >> > >> hotplug_enabled_dev = bsel && dc->hotpluggable > >> && !bridge_in_acpi; @@ -1892,7 +1894,7 @@ static void > >> build_piix4_isa_bridge(Aml *table) aml_append(table, scope); > >> } > >> > >> -static void build_piix4_pci_hotplug(Aml *table) > >> +static void build_acpi_pci_hotplug(Aml *table) > >> { > >> Aml *scope; > >> Aml *field; > >> @@ -1995,6 +1997,7 @@ build_dsdt(GArray *table_data, GArray > >> *linker, uint32_t nr_mem = machine->ram_slots; > >> int root_bus_limit = 0xFF; > >> PCIBus *root_bus = NULL; > >> + bool has_pci_bus = false; > >> int i; > >> > >> dsdt = init_aml_allocator(); > >> @@ -2016,7 +2019,6 @@ build_dsdt(GArray *table_data, GArray > >> *linker, build_piix4_pm(dsdt); > >> build_piix4_isa_bridge(dsdt); > >> build_isa_devices_aml(dsdt); > >> - build_piix4_pci_hotplug(dsdt); > >> build_piix4_pci0_int(dsdt); > >> } else { > >> sb_scope = aml_scope("_SB"); > >> @@ -2040,6 +2042,11 @@ build_dsdt(GArray *table_data, GArray > >> *linker, root_bus = PC_MACHINE(machine)->bus; > >> assert(root_bus); > >> > >> + pci_for_each_bus(root_bus, acpi_test_pci_bus, &has_pci_bus); > >> + if (pm->pcihp_bridge_en && has_pci_bus) { > > ^^^^^ wouldn't that forbid acpi hotplug on PCI0 is it's > > not set? > > Yes, and this is by design. 'pcihp_bridge_en' flag matches 'acpi > hotplug'. I followed the initialization phase, am I wrong? Can you > have acpi hotplug if the bit is off? my understanding was that pcihp_bridge_en governs only hotplug AML for bridges, while with it disabled hotplug AML would be generated only for PCI0. > > > >> + build_acpi_pci_hotplug(dsdt); > >> + } > >> + > >> build_cpu_hotplug_aml(dsdt); > >> build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base, > >> pm->mem_hp_io_len); > >> @@ -2050,7 +2057,7 @@ build_dsdt(GArray *table_data, GArray > >> *linker, > >> > >> aml_append(scope, aml_method("_L00", 0, > >> AML_NOTSERIALIZED)); > >> > >> - if (misc->is_piix4) { > >> + if (pm->pcihp_bridge_en && has_pci_bus) { > > same question as above > > > >> method = aml_method("_E01", 0, AML_NOTSERIALIZED); > >> aml_append(method, > >> aml_acquire(aml_name("\\_SB.PCI0.BLCK"), > >> 0xFFFF)); @@ -2188,7 +2195,7 @@ build_dsdt(GArray *table_data, > >> GArray *linker, g_ptr_array_free(mem_ranges, true); > >> > >> /* reserve PCIHP resources */ > >> - if (pm->pcihp_io_len) { > >> + if (pm->pcihp_bridge_en && has_pci_bus && pm->pcihp_io_len) { > > same question as above > > > >> dev = aml_device("PHPR"); > >> aml_append(dev, aml_name_decl("_HID", > >> aml_string("PNP0A06"))); aml_append(dev, > >> @@ -2326,10 +2333,14 @@ build_dsdt(GArray *table_data, GArray > >> *linker, > >> > >> { > >> Aml *scope = aml_scope("PCI0"); > >> + bool scope_used = false; > > Could you explain why ^^^ is needed? > > > > Sure. If none ACPI code will be generated by the below 'if' statements > we will have an empty PCI0 scope. I wanted to get rid of it. considering that hotplug AML for PCI0 is always generated, that seems useless. > > > Thanks, > Marcel > > >> > >> - /* Scan all PCI buses. Generate tables to support > >> hotplug. */ > >> - build_append_pci_bus_devices(scope, root_bus, > >> - pm->pcihp_bridge_en); > >> + if (pm->pcihp_bridge_en && has_pci_bus) { > >> + /* Scan all PCI buses. Generate tables to support > >> hotplug. */ > >> + build_append_pci_bus_devices(scope, root_bus, > >> + pm->pcihp_bridge_en); > >> + scope_used = true; > >> + } > >> > >> if (misc->tpm_version != TPM_VERSION_UNSPEC) { > >> dev = aml_device("ISA.TPM"); > >> @@ -2346,8 +2357,12 @@ build_dsdt(GArray *table_data, GArray > >> *linker, /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > >> aml_append(dev, aml_name_decl("_CRS", crs)); > >> aml_append(scope, dev); > >> + scope_used = true; > >> + } > >> + > >> + if (scope_used) { > >> + aml_append(sb_scope, scope); > >> } > >> - aml_append(sb_scope, scope); > >> } > >> aml_append(dsdt, sb_scope); > >> } > >> @@ -2870,7 +2885,7 @@ void acpi_setup(void) > >> > >> build_state = g_malloc0(sizeof *build_state); > >> > >> - acpi_set_pci_info(); > >> + acpi_set_pci_info(pcms->bus); > >> > >> acpi_build_tables_init(&tables); > >> acpi_build(&tables, MACHINE(pcms)); > > >
On 06/21/2016 02:28 PM, Igor Mammedov wrote: > On Tue, 21 Jun 2016 12:05:12 +0300 > Marcel Apfelbaum <marcel@redhat.com> wrote: > >> On 06/20/2016 05:11 PM, Igor Mammedov wrote: >>> On Tue, 31 May 2016 20:48:37 +0300 >>> Marcel Apfelbaum <marcel@redhat.com> wrote: >>> >>> subj doesn't match patch, it does opposite i.e. makes sure that >>> pcie isn't counted in when building acpi hotplug aml >>> >> >> Maybe I wasn't clear enough. Until now q35 had no acpi hotplug code. >> This patch adds acpi hotplug code for all Q35 PCI buses, but no for >> pcie buses That means for pci-pci bridge and dmi-pci bridge. >> >> >> Maybe I should rename it to hw/acpi: extend acpi pci hotplug support >> for Q35 pci buses. > this looks better to me > >> >>>> Emit acpi code for pci hotplug on all PC machines: >>>> - if legacy pci hotpug is enabled (pcihp_bridge_en) >>>> - if there is at least one legacy pci bridge in current system >>>> - for PCI buses only (skip PCIe buses) >>>> >>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> >>>> --- >>>> hw/i386/acpi-build.c | 49 >>>> ++++++++++++++++++++++++++++++++----------------- 1 file changed, >>>> 32 insertions(+), 17 deletions(-) >>>> >>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >>>> index 8ae3e53..cc51f9e 100644 >>>> --- a/hw/i386/acpi-build.c >>>> +++ b/hw/i386/acpi-build.c >>>> @@ -250,6 +250,11 @@ static void acpi_get_pci_info(PcPciInfo *info) >>>> NULL); >>>> } >>>> >>>> +static void acpi_test_pci_bus(PCIBus *bus, void *opaque) >>>> +{ >>>> + *(bool *)opaque |= !pci_bus_is_express(bus); >>>> +} >>>> + >>>> #define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is >>>> APM_CNT_IOPORT */ >>>> static void acpi_align_size(GArray *blob, unsigned align) >>>> @@ -422,7 +427,7 @@ static void *acpi_set_bsel(PCIBus *bus, void >>>> *opaque) unsigned *bsel_alloc = opaque; >>>> unsigned *bus_bsel; >>>> >>>> - if (qbus_is_hotpluggable(BUS(bus))) { >>>> + if (qbus_is_hotpluggable(BUS(bus)) >>>> && !pci_bus_is_express(bus)) { bus_bsel = g_malloc(sizeof >>>> *bus_bsel); >>>> >>>> *bus_bsel = (*bsel_alloc)++; >>>> @@ -433,15 +438,12 @@ static void *acpi_set_bsel(PCIBus *bus, void >>>> *opaque) return bsel_alloc; >>>> } >>>> >>>> -static void acpi_set_pci_info(void) >>>> +static void acpi_set_pci_info(PCIBus *bus) >>>> { >>>> - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ >>>> unsigned bsel_alloc = 0; >>>> >>>> - if (bus) { >>>> - /* Scan all PCI buses. Set property to enable acpi based >>>> hotplug. */ >>>> - pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, >>>> &bsel_alloc); >>>> - } >>>> + /* Scan all PCI buses. Set property to enable acpi based >>>> hotplug. */ >>>> + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, >>>> &bsel_alloc); } >>>> >>>> static void build_append_pcihp_notify_entry(Aml *method, int >>>> slot) @@ -503,7 +505,7 @@ static void >>>> build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, >>>> * In this case they aren't themselves hot-pluggable. >>>> * Hotplugged bridges *are* hot-pluggable. >>>> */ >>>> - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && >>>> + bridge_in_acpi = pc->is_bridge && !pc->is_express && >>>> pcihp_bridge_en && !DEVICE(pdev)->hotplugged; >>>> >>>> hotplug_enabled_dev = bsel && dc->hotpluggable >>>> && !bridge_in_acpi; @@ -1892,7 +1894,7 @@ static void >>>> build_piix4_isa_bridge(Aml *table) aml_append(table, scope); >>>> } >>>> >>>> -static void build_piix4_pci_hotplug(Aml *table) >>>> +static void build_acpi_pci_hotplug(Aml *table) >>>> { >>>> Aml *scope; >>>> Aml *field; >>>> @@ -1995,6 +1997,7 @@ build_dsdt(GArray *table_data, GArray >>>> *linker, uint32_t nr_mem = machine->ram_slots; >>>> int root_bus_limit = 0xFF; >>>> PCIBus *root_bus = NULL; >>>> + bool has_pci_bus = false; >>>> int i; >>>> >>>> dsdt = init_aml_allocator(); >>>> @@ -2016,7 +2019,6 @@ build_dsdt(GArray *table_data, GArray >>>> *linker, build_piix4_pm(dsdt); >>>> build_piix4_isa_bridge(dsdt); >>>> build_isa_devices_aml(dsdt); >>>> - build_piix4_pci_hotplug(dsdt); >>>> build_piix4_pci0_int(dsdt); >>>> } else { >>>> sb_scope = aml_scope("_SB"); >>>> @@ -2040,6 +2042,11 @@ build_dsdt(GArray *table_data, GArray >>>> *linker, root_bus = PC_MACHINE(machine)->bus; >>>> assert(root_bus); >>>> >>>> + pci_for_each_bus(root_bus, acpi_test_pci_bus, &has_pci_bus); >>>> + if (pm->pcihp_bridge_en && has_pci_bus) { >>> ^^^^^ wouldn't that forbid acpi hotplug on PCI0 is it's >>> not set? >> >> Yes, and this is by design. 'pcihp_bridge_en' flag matches 'acpi >> hotplug'. I followed the initialization phase, am I wrong? Can you >> have acpi hotplug if the bit is off? > my understanding was that pcihp_bridge_en governs only hotplug > AML for bridges, while with it disabled hotplug AML would be > generated only for PCI0. > >>> >>>> + build_acpi_pci_hotplug(dsdt); >>>> + } >>>> + >>>> build_cpu_hotplug_aml(dsdt); >>>> build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base, >>>> pm->mem_hp_io_len); >>>> @@ -2050,7 +2057,7 @@ build_dsdt(GArray *table_data, GArray >>>> *linker, >>>> >>>> aml_append(scope, aml_method("_L00", 0, >>>> AML_NOTSERIALIZED)); >>>> >>>> - if (misc->is_piix4) { >>>> + if (pm->pcihp_bridge_en && has_pci_bus) { >>> same question as above >>> >>>> method = aml_method("_E01", 0, AML_NOTSERIALIZED); >>>> aml_append(method, >>>> aml_acquire(aml_name("\\_SB.PCI0.BLCK"), >>>> 0xFFFF)); @@ -2188,7 +2195,7 @@ build_dsdt(GArray *table_data, >>>> GArray *linker, g_ptr_array_free(mem_ranges, true); >>>> >>>> /* reserve PCIHP resources */ >>>> - if (pm->pcihp_io_len) { >>>> + if (pm->pcihp_bridge_en && has_pci_bus && pm->pcihp_io_len) { >>> same question as above >>> >>>> dev = aml_device("PHPR"); >>>> aml_append(dev, aml_name_decl("_HID", >>>> aml_string("PNP0A06"))); aml_append(dev, >>>> @@ -2326,10 +2333,14 @@ build_dsdt(GArray *table_data, GArray >>>> *linker, >>>> >>>> { >>>> Aml *scope = aml_scope("PCI0"); >>>> + bool scope_used = false; >>> Could you explain why ^^^ is needed? >>> >> >> Sure. If none ACPI code will be generated by the below 'if' statements >> we will have an empty PCI0 scope. I wanted to get rid of it. > considering that hotplug AML for PCI0 is always generated, that seems > useless. > Not in Q35. pcie.0 is not hot-pluggable. Run Q35 without a dmi-pci bridge or pci-bridge and you'll have an empty scope. Thanks, Marcel >> >> >> Thanks, >> Marcel >> >>>> >>>> - /* Scan all PCI buses. Generate tables to support >>>> hotplug. */ >>>> - build_append_pci_bus_devices(scope, root_bus, >>>> - pm->pcihp_bridge_en); >>>> + if (pm->pcihp_bridge_en && has_pci_bus) { >>>> + /* Scan all PCI buses. Generate tables to support >>>> hotplug. */ >>>> + build_append_pci_bus_devices(scope, root_bus, >>>> + pm->pcihp_bridge_en); >>>> + scope_used = true; >>>> + } >>>> >>>> if (misc->tpm_version != TPM_VERSION_UNSPEC) { >>>> dev = aml_device("ISA.TPM"); >>>> @@ -2346,8 +2357,12 @@ build_dsdt(GArray *table_data, GArray >>>> *linker, /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ >>>> aml_append(dev, aml_name_decl("_CRS", crs)); >>>> aml_append(scope, dev); >>>> + scope_used = true; >>>> + } >>>> + >>>> + if (scope_used) { >>>> + aml_append(sb_scope, scope); >>>> } >>>> - aml_append(sb_scope, scope); >>>> } >>>> aml_append(dsdt, sb_scope); >>>> } >>>> @@ -2870,7 +2885,7 @@ void acpi_setup(void) >>>> >>>> build_state = g_malloc0(sizeof *build_state); >>>> >>>> - acpi_set_pci_info(); >>>> + acpi_set_pci_info(pcms->bus); >>>> >>>> acpi_build_tables_init(&tables); >>>> acpi_build(&tables, MACHINE(pcms)); >>> >> >
On Tue, 21 Jun 2016 14:47:24 +0300 Marcel Apfelbaum <marcel@redhat.com> wrote: > On 06/21/2016 02:28 PM, Igor Mammedov wrote: > > On Tue, 21 Jun 2016 12:05:12 +0300 > > Marcel Apfelbaum <marcel@redhat.com> wrote: > > > >> On 06/20/2016 05:11 PM, Igor Mammedov wrote: > >>> On Tue, 31 May 2016 20:48:37 +0300 > >>> Marcel Apfelbaum <marcel@redhat.com> wrote: > >>> > >>> subj doesn't match patch, it does opposite i.e. makes sure that > >>> pcie isn't counted in when building acpi hotplug aml > >>> > >> > >> Maybe I wasn't clear enough. Until now q35 had no acpi hotplug > >> code. This patch adds acpi hotplug code for all Q35 PCI buses, but > >> no for pcie buses That means for pci-pci bridge and dmi-pci bridge. > >> > >> > >> Maybe I should rename it to hw/acpi: extend acpi pci hotplug > >> support for Q35 pci buses. > > this looks better to me > > > >> > >>>> Emit acpi code for pci hotplug on all PC machines: > >>>> - if legacy pci hotpug is enabled (pcihp_bridge_en) > >>>> - if there is at least one legacy pci bridge in current system > >>>> - for PCI buses only (skip PCIe buses) > >>>> > >>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > >>>> --- > >>>> hw/i386/acpi-build.c | 49 > >>>> ++++++++++++++++++++++++++++++++----------------- 1 file changed, > >>>> 32 insertions(+), 17 deletions(-) > >>>> > >>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >>>> index 8ae3e53..cc51f9e 100644 > >>>> --- a/hw/i386/acpi-build.c > >>>> +++ b/hw/i386/acpi-build.c > >>>> @@ -250,6 +250,11 @@ static void acpi_get_pci_info(PcPciInfo > >>>> *info) NULL); > >>>> } > >>>> > >>>> +static void acpi_test_pci_bus(PCIBus *bus, void *opaque) > >>>> +{ > >>>> + *(bool *)opaque |= !pci_bus_is_express(bus); > >>>> +} > >>>> + > >>>> #define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is > >>>> APM_CNT_IOPORT */ > >>>> static void acpi_align_size(GArray *blob, unsigned align) > >>>> @@ -422,7 +427,7 @@ static void *acpi_set_bsel(PCIBus *bus, void > >>>> *opaque) unsigned *bsel_alloc = opaque; > >>>> unsigned *bus_bsel; > >>>> > >>>> - if (qbus_is_hotpluggable(BUS(bus))) { > >>>> + if (qbus_is_hotpluggable(BUS(bus)) > >>>> && !pci_bus_is_express(bus)) { bus_bsel = g_malloc(sizeof > >>>> *bus_bsel); > >>>> > >>>> *bus_bsel = (*bsel_alloc)++; > >>>> @@ -433,15 +438,12 @@ static void *acpi_set_bsel(PCIBus *bus, > >>>> void *opaque) return bsel_alloc; > >>>> } > >>>> > >>>> -static void acpi_set_pci_info(void) > >>>> +static void acpi_set_pci_info(PCIBus *bus) > >>>> { > >>>> - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ > >>>> unsigned bsel_alloc = 0; > >>>> > >>>> - if (bus) { > >>>> - /* Scan all PCI buses. Set property to enable acpi based > >>>> hotplug. */ > >>>> - pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, > >>>> &bsel_alloc); > >>>> - } > >>>> + /* Scan all PCI buses. Set property to enable acpi based > >>>> hotplug. */ > >>>> + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, > >>>> &bsel_alloc); } > >>>> > >>>> static void build_append_pcihp_notify_entry(Aml *method, int > >>>> slot) @@ -503,7 +505,7 @@ static void > >>>> build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, > >>>> * In this case they aren't themselves hot-pluggable. > >>>> * Hotplugged bridges *are* hot-pluggable. > >>>> */ > >>>> - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && > >>>> + bridge_in_acpi = pc->is_bridge && !pc->is_express && > >>>> pcihp_bridge_en && !DEVICE(pdev)->hotplugged; > >>>> > >>>> hotplug_enabled_dev = bsel && dc->hotpluggable > >>>> && !bridge_in_acpi; @@ -1892,7 +1894,7 @@ static void > >>>> build_piix4_isa_bridge(Aml *table) aml_append(table, scope); > >>>> } > >>>> > >>>> -static void build_piix4_pci_hotplug(Aml *table) > >>>> +static void build_acpi_pci_hotplug(Aml *table) > >>>> { > >>>> Aml *scope; > >>>> Aml *field; > >>>> @@ -1995,6 +1997,7 @@ build_dsdt(GArray *table_data, GArray > >>>> *linker, uint32_t nr_mem = machine->ram_slots; > >>>> int root_bus_limit = 0xFF; > >>>> PCIBus *root_bus = NULL; > >>>> + bool has_pci_bus = false; > >>>> int i; > >>>> > >>>> dsdt = init_aml_allocator(); > >>>> @@ -2016,7 +2019,6 @@ build_dsdt(GArray *table_data, GArray > >>>> *linker, build_piix4_pm(dsdt); > >>>> build_piix4_isa_bridge(dsdt); > >>>> build_isa_devices_aml(dsdt); > >>>> - build_piix4_pci_hotplug(dsdt); > >>>> build_piix4_pci0_int(dsdt); > >>>> } else { > >>>> sb_scope = aml_scope("_SB"); > >>>> @@ -2040,6 +2042,11 @@ build_dsdt(GArray *table_data, GArray > >>>> *linker, root_bus = PC_MACHINE(machine)->bus; > >>>> assert(root_bus); > >>>> > >>>> + pci_for_each_bus(root_bus, acpi_test_pci_bus, &has_pci_bus); > >>>> + if (pm->pcihp_bridge_en && has_pci_bus) { > >>> ^^^^^ wouldn't that forbid acpi hotplug on PCI0 is > >>> it's not set? > >> > >> Yes, and this is by design. 'pcihp_bridge_en' flag matches 'acpi > >> hotplug'. I followed the initialization phase, am I wrong? Can you > >> have acpi hotplug if the bit is off? > > my understanding was that pcihp_bridge_en governs only hotplug > > AML for bridges, while with it disabled hotplug AML would be > > generated only for PCI0. > > > >>> > >>>> + build_acpi_pci_hotplug(dsdt); > >>>> + } > >>>> + > >>>> build_cpu_hotplug_aml(dsdt); > >>>> build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base, > >>>> pm->mem_hp_io_len); > >>>> @@ -2050,7 +2057,7 @@ build_dsdt(GArray *table_data, GArray > >>>> *linker, > >>>> > >>>> aml_append(scope, aml_method("_L00", 0, > >>>> AML_NOTSERIALIZED)); > >>>> > >>>> - if (misc->is_piix4) { > >>>> + if (pm->pcihp_bridge_en && has_pci_bus) { > >>> same question as above > >>> > >>>> method = aml_method("_E01", 0, AML_NOTSERIALIZED); > >>>> aml_append(method, > >>>> aml_acquire(aml_name("\\_SB.PCI0.BLCK"), > >>>> 0xFFFF)); @@ -2188,7 +2195,7 @@ build_dsdt(GArray *table_data, > >>>> GArray *linker, g_ptr_array_free(mem_ranges, true); > >>>> > >>>> /* reserve PCIHP resources */ > >>>> - if (pm->pcihp_io_len) { > >>>> + if (pm->pcihp_bridge_en && has_pci_bus && pm->pcihp_io_len) > >>>> { > >>> same question as above > >>> > >>>> dev = aml_device("PHPR"); > >>>> aml_append(dev, aml_name_decl("_HID", > >>>> aml_string("PNP0A06"))); aml_append(dev, > >>>> @@ -2326,10 +2333,14 @@ build_dsdt(GArray *table_data, GArray > >>>> *linker, > >>>> > >>>> { > >>>> Aml *scope = aml_scope("PCI0"); > >>>> + bool scope_used = false; > >>> Could you explain why ^^^ is needed? > >>> > >> > >> Sure. If none ACPI code will be generated by the below 'if' > >> statements we will have an empty PCI0 scope. I wanted to get rid > >> of it. > > considering that hotplug AML for PCI0 is always generated, that > > seems useless. > > > > Not in Q35. pcie.0 is not hot-pluggable. > Run Q35 without a dmi-pci bridge or pci-bridge and you'll have an > empty scope. ok, just make sure patch affects only Q35 and doesn't regress piix4 (where PCI0 always has hotplug AML) I'd write it a little bit different though: if (pm->pcihp_bridge_en && has_pci_bus) { Aml *scope = aml_scope("PCI0"); build_append_pci_bus_devices(scope, root_bus,... aml_append(sb_scope, scope); } if (misc->tpm_version != TPM_VERSION_UNSPEC) { dev = aml_device("PCI0.ISA.TPM"); ... aml_append(sb_scope, dev); } which looks somewhat cleaner than tracking if scope was used. > > > Thanks, > Marcel > > >> > >> > >> Thanks, > >> Marcel > >> > >>>> > >>>> - /* Scan all PCI buses. Generate tables to support > >>>> hotplug. */ > >>>> - build_append_pci_bus_devices(scope, root_bus, > >>>> - pm->pcihp_bridge_en); > >>>> + if (pm->pcihp_bridge_en && has_pci_bus) { > >>>> + /* Scan all PCI buses. Generate tables to > >>>> support hotplug. */ > >>>> + build_append_pci_bus_devices(scope, root_bus, > >>>> + > >>>> pm->pcihp_bridge_en); > >>>> + scope_used = true; > >>>> + } > >>>> > >>>> if (misc->tpm_version != TPM_VERSION_UNSPEC) { > >>>> dev = aml_device("ISA.TPM"); > >>>> @@ -2346,8 +2357,12 @@ build_dsdt(GArray *table_data, GArray > >>>> *linker, /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ > >>>> aml_append(dev, aml_name_decl("_CRS", crs)); > >>>> aml_append(scope, dev); > >>>> + scope_used = true; > >>>> + } > >>>> + > >>>> + if (scope_used) { > >>>> + aml_append(sb_scope, scope); > >>>> } > >>>> - aml_append(sb_scope, scope); > >>>> } > >>>> aml_append(dsdt, sb_scope); > >>>> } > >>>> @@ -2870,7 +2885,7 @@ void acpi_setup(void) > >>>> > >>>> build_state = g_malloc0(sizeof *build_state); > >>>> > >>>> - acpi_set_pci_info(); > >>>> + acpi_set_pci_info(pcms->bus); > >>>> > >>>> acpi_build_tables_init(&tables); > >>>> acpi_build(&tables, MACHINE(pcms)); > >>> > >> > > >
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 8ae3e53..cc51f9e 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -250,6 +250,11 @@ static void acpi_get_pci_info(PcPciInfo *info) NULL); } +static void acpi_test_pci_bus(PCIBus *bus, void *opaque) +{ + *(bool *)opaque |= !pci_bus_is_express(bus); +} + #define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */ static void acpi_align_size(GArray *blob, unsigned align) @@ -422,7 +427,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) unsigned *bsel_alloc = opaque; unsigned *bus_bsel; - if (qbus_is_hotpluggable(BUS(bus))) { + if (qbus_is_hotpluggable(BUS(bus)) && !pci_bus_is_express(bus)) { bus_bsel = g_malloc(sizeof *bus_bsel); *bus_bsel = (*bsel_alloc)++; @@ -433,15 +438,12 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) return bsel_alloc; } -static void acpi_set_pci_info(void) +static void acpi_set_pci_info(PCIBus *bus) { - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ unsigned bsel_alloc = 0; - if (bus) { - /* Scan all PCI buses. Set property to enable acpi based hotplug. */ - pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); - } + /* Scan all PCI buses. Set property to enable acpi based hotplug. */ + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); } static void build_append_pcihp_notify_entry(Aml *method, int slot) @@ -503,7 +505,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, * In this case they aren't themselves hot-pluggable. * Hotplugged bridges *are* hot-pluggable. */ - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && + bridge_in_acpi = pc->is_bridge && !pc->is_express && pcihp_bridge_en && !DEVICE(pdev)->hotplugged; hotplug_enabled_dev = bsel && dc->hotpluggable && !bridge_in_acpi; @@ -1892,7 +1894,7 @@ static void build_piix4_isa_bridge(Aml *table) aml_append(table, scope); } -static void build_piix4_pci_hotplug(Aml *table) +static void build_acpi_pci_hotplug(Aml *table) { Aml *scope; Aml *field; @@ -1995,6 +1997,7 @@ build_dsdt(GArray *table_data, GArray *linker, uint32_t nr_mem = machine->ram_slots; int root_bus_limit = 0xFF; PCIBus *root_bus = NULL; + bool has_pci_bus = false; int i; dsdt = init_aml_allocator(); @@ -2016,7 +2019,6 @@ build_dsdt(GArray *table_data, GArray *linker, build_piix4_pm(dsdt); build_piix4_isa_bridge(dsdt); build_isa_devices_aml(dsdt); - build_piix4_pci_hotplug(dsdt); build_piix4_pci0_int(dsdt); } else { sb_scope = aml_scope("_SB"); @@ -2040,6 +2042,11 @@ build_dsdt(GArray *table_data, GArray *linker, root_bus = PC_MACHINE(machine)->bus; assert(root_bus); + pci_for_each_bus(root_bus, acpi_test_pci_bus, &has_pci_bus); + if (pm->pcihp_bridge_en && has_pci_bus) { + build_acpi_pci_hotplug(dsdt); + } + build_cpu_hotplug_aml(dsdt); build_memory_hotplug_aml(dsdt, nr_mem, pm->mem_hp_io_base, pm->mem_hp_io_len); @@ -2050,7 +2057,7 @@ build_dsdt(GArray *table_data, GArray *linker, aml_append(scope, aml_method("_L00", 0, AML_NOTSERIALIZED)); - if (misc->is_piix4) { + if (pm->pcihp_bridge_en && has_pci_bus) { method = aml_method("_E01", 0, AML_NOTSERIALIZED); aml_append(method, aml_acquire(aml_name("\\_SB.PCI0.BLCK"), 0xFFFF)); @@ -2188,7 +2195,7 @@ build_dsdt(GArray *table_data, GArray *linker, g_ptr_array_free(mem_ranges, true); /* reserve PCIHP resources */ - if (pm->pcihp_io_len) { + if (pm->pcihp_bridge_en && has_pci_bus && pm->pcihp_io_len) { dev = aml_device("PHPR"); aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A06"))); aml_append(dev, @@ -2326,10 +2333,14 @@ build_dsdt(GArray *table_data, GArray *linker, { Aml *scope = aml_scope("PCI0"); + bool scope_used = false; - /* Scan all PCI buses. Generate tables to support hotplug. */ - build_append_pci_bus_devices(scope, root_bus, - pm->pcihp_bridge_en); + if (pm->pcihp_bridge_en && has_pci_bus) { + /* Scan all PCI buses. Generate tables to support hotplug. */ + build_append_pci_bus_devices(scope, root_bus, + pm->pcihp_bridge_en); + scope_used = true; + } if (misc->tpm_version != TPM_VERSION_UNSPEC) { dev = aml_device("ISA.TPM"); @@ -2346,8 +2357,12 @@ build_dsdt(GArray *table_data, GArray *linker, /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */ aml_append(dev, aml_name_decl("_CRS", crs)); aml_append(scope, dev); + scope_used = true; + } + + if (scope_used) { + aml_append(sb_scope, scope); } - aml_append(sb_scope, scope); } aml_append(dsdt, sb_scope); } @@ -2870,7 +2885,7 @@ void acpi_setup(void) build_state = g_malloc0(sizeof *build_state); - acpi_set_pci_info(); + acpi_set_pci_info(pcms->bus); acpi_build_tables_init(&tables); acpi_build(&tables, MACHINE(pcms));
Emit acpi code for pci hotplug on all PC machines: - if legacy pci hotpug is enabled (pcihp_bridge_en) - if there is at least one legacy pci bridge in current system - for PCI buses only (skip PCIe buses) Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> --- hw/i386/acpi-build.c | 49 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-)