On Tue, 31 May 2016 20:48:37 +0300
Marcel Apfelbaum <mar...@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 <mar...@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));
