Re: [RFC PATCH 3/5] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to q35

[Igor Mammedov]

On Thu,  9 Jul 2020 00:46:13 +0200
Julia Suvorova <jusual@redhat.com> wrote:

> Implement notifications and gpe to support q35 ACPI PCI hot-plug.
> The addresses specified in [1] remain the same to make fewer changes.
> 
> [1] docs/spec/acpi_pci_hotplug.txt

CCing Gerd his opinion on reusing piix4 IO port range for q35

 
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
>  hw/i386/acpi-build.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 11c598f955..5c5ad88ad6 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -201,10 +201,6 @@ static void acpi_get_pm_info(MachineState *machine, 
> AcpiPmInfo *pm)
>          /* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
>          pm->fadt.rev = 1;
>          pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
> -        pm->pcihp_io_base =
> -            object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> -        pm->pcihp_io_len =
> -            object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
>      }
>      if (lpc) {
>          struct AcpiGenericAddress r = { .space_id = AML_AS_SYSTEM_IO,
> @@ -214,6 +210,10 @@ static void acpi_get_pm_info(MachineState *machine, 
> AcpiPmInfo *pm)
>          pm->fadt.flags |= 1 << ACPI_FADT_F_RESET_REG_SUP;
>          pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
>      }
> +    pm->pcihp_io_base =
> +        object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> +    pm->pcihp_io_len =
> +        object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
>  
>      /* The above need not be conditional on machine type because the reset 
> port
>       * happens to be the same on PIIX (pc) and ICH9 (q35). */
> @@ -472,7 +472,7 @@ static void build_append_pci_bus_devices(Aml 
> *parent_scope, PCIBus *bus,
>          QLIST_FOREACH(sec, &bus->child, sibling) {
>              int32_t devfn = sec->parent_dev->devfn;
>  
> -            if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
> +            if (pci_bus_is_root(sec)) {
>                  continue;
>              }
>  
> @@ -1586,7 +1586,12 @@ static void build_piix4_pci_hotplug(Aml *table)
>      aml_append(table, scope);
>  }
>  
> -static Aml *build_q35_osc_method(void)
> +static void build_q35_pci_hotplug(Aml *table)
> +{
> +    build_piix4_pci_hotplug(table);
> +}

s/build_piix4_pci_hotplug/build_i386_acpi_pci_hotplug/

and reuse it in both cases, instead of adding wrapper?

> +
> +static Aml *build_q35_osc_method(AcpiPmInfo *pm)
>  {
>      Aml *if_ctx;
>      Aml *if_ctx2;
> @@ -1698,6 +1703,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          build_hpet_aml(dsdt);
>          build_q35_isa_bridge(dsdt);
>          build_isa_devices_aml(dsdt);
> +        build_q35_pci_hotplug(dsdt);
>          build_q35_pci0_int(dsdt);
>          if (pcms->smbus && !pcmc->do_not_add_smb_acpi) {
>              build_smb0(dsdt, pcms->smbus, ICH9_SMB_DEV, ICH9_SMB_FUNC);
> @@ -1724,7 +1730,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>      {
>          aml_append(scope, aml_name_decl("_HID", aml_string("ACPI0006")));
>  
> -        if (misc->is_piix4) {
> +        if (misc->is_piix4 || pm->pcihp_bridge_en) {
>              method = aml_method("_E01", 0, AML_NOTSERIALIZED);
>              aml_append(method,
>                  aml_acquire(aml_name("\\_SB.PCI0.BLCK"), 0xFFFF));