[Qemu-devel] [PATCH v2] arm: virt-acpi: each MADT.GICC entry as enabled unconditionally

[Qemu-devel] [PATCH v2] arm: virt-acpi: each MADT.GICC entry as enabled unconditionally

[Igor Mammedov]

in current impl. condition

build_madt() {
  ...
  if (test_bit(i, cpuinfo->found_cpus))

is always true since loop handles only present CPUs
in range [0..smp_cpus).
But to fill usless cpuinfo->found_cpus we do unnecessary
scan over QOM tree to find the same CPUs.
So mark GICC as present always and drop not needed
code that fills cpuinfo->found_cpus.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v2:
  - drop unused VIRT_ACPI_CPU_ID_LIMIT macro
v1:
It's just simple cleanup but I'm trying to generalize
a bit CPU related ACPI tables and as part of it get rid
of found_cpus bitmap and if possible cpu_index usage
in ACPI parts of code.
---
 hw/arm/virt-acpi-build.c         | 26 +++-----------------------
 include/hw/arm/virt-acpi-build.h |  1 -
 2 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 87fbe7c..3ed39fc 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -46,20 +46,6 @@
 #define ARM_SPI_BASE 32
 #define ACPI_POWER_BUTTON_DEVICE "PWRB"
 
-typedef struct VirtAcpiCpuInfo {
-    DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
-} VirtAcpiCpuInfo;
-
-static void virt_acpi_get_cpu_info(VirtAcpiCpuInfo *cpuinfo)
-{
-    CPUState *cpu;
-
-    memset(cpuinfo->found_cpus, 0, sizeof cpuinfo->found_cpus);
-    CPU_FOREACH(cpu) {
-        set_bit(cpu->cpu_index, cpuinfo->found_cpus);
-    }
-}
-
 static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
 {
     uint16_t i;
@@ -458,8 +444,7 @@ build_gtdt(GArray *table_data, GArray *linker)
 
 /* MADT */
 static void
-build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
-           VirtAcpiCpuInfo *cpuinfo)
+build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
 {
     int madt_start = table_data->len;
     const MemMapEntry *memmap = guest_info->memmap;
@@ -489,9 +474,7 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
         gicc->cpu_interface_number = i;
         gicc->arm_mpidr = armcpu->mp_affinity;
         gicc->uid = i;
-        if (test_bit(i, cpuinfo->found_cpus)) {
-            gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED);
-        }
+        gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED);
     }
 
     if (guest_info->gic_version == 3) {
@@ -599,11 +582,8 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
 {
     GArray *table_offsets;
     unsigned dsdt, rsdt;
-    VirtAcpiCpuInfo cpuinfo;
     GArray *tables_blob = tables->table_data;
 
-    virt_acpi_get_cpu_info(&cpuinfo);
-
     table_offsets = g_array_new(false, true /* clear */,
                                         sizeof(uint32_t));
 
@@ -630,7 +610,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
     build_fadt(tables_blob, tables->linker, dsdt);
 
     acpi_add_table(table_offsets, tables_blob);
-    build_madt(tables_blob, tables->linker, guest_info, &cpuinfo);
+    build_madt(tables_blob, tables->linker, guest_info);
 
     acpi_add_table(table_offsets, tables_blob);
     build_gtdt(tables_blob, tables->linker);
diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
index 744b666..7d3700e 100644
--- a/include/hw/arm/virt-acpi-build.h
+++ b/include/hw/arm/virt-acpi-build.h
@@ -23,7 +23,6 @@
 #include "qemu-common.h"
 #include "hw/arm/virt.h"
 
-#define VIRT_ACPI_CPU_ID_LIMIT 8
 #define ACPI_GICC_ENABLED 1
 
 typedef struct VirtGuestInfo {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2] arm: virt-acpi: each MADT.GICC entry as enabled unconditionally

[Peter Maydell]

On 1 February 2016 at 10:48, Igor Mammedov <imammedo@redhat.com> wrote:
> in current impl. condition
>
> build_madt() {
>   ...
>   if (test_bit(i, cpuinfo->found_cpus))
>
> is always true since loop handles only present CPUs
> in range [0..smp_cpus).
> But to fill usless cpuinfo->found_cpus we do unnecessary
> scan over QOM tree to find the same CPUs.
> So mark GICC as present always and drop not needed
> code that fills cpuinfo->found_cpus.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v2:
>   - drop unused VIRT_ACPI_CPU_ID_LIMIT macro
> v1:
> It's just simple cleanup but I'm trying to generalize
> a bit CPU related ACPI tables and as part of it get rid
> of found_cpus bitmap and if possible cpu_index usage
> in ACPI parts of code.
> ---
>  hw/arm/virt-acpi-build.c         | 26 +++-----------------------
>  include/hw/arm/virt-acpi-build.h |  1 -
>  2 files changed, 3 insertions(+), 24 deletions(-)



Applied to target-arm.next, thanks.

-- PMM