diff mbox

[RFC,2/7] hw/acpi: simplify dsdt building code

Message ID 1464716918-29689-3-git-send-email-marcel@redhat.com
State New
Headers show

Commit Message

Marcel Apfelbaum May 31, 2016, 5:48 p.m. UTC
Get the pci root-bus once and pass it around.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/acpi-build.c | 58 +++++++++++++++++++++++-----------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

Comments

Igor Mammedov June 17, 2016, 8:39 a.m. UTC | #1
On Tue, 31 May 2016 20:48:33 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:

> Get the pci root-bus once and pass it around.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@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 <imammedo@redhat.com>
diff mbox

Patch

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);
+
     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();
-            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);
 
-                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);
     }