diff mbox

[v4,02/10] pc: init pcms->apic_id_limit once and use it throughout pc.c

Message ID 1456495168-144510-3-git-send-email-imammedo@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Igor Mammedov Feb. 26, 2016, 1:59 p.m. UTC 
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 45 +++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

Comments

Marcel Apfelbaum Feb. 28, 2016, 8:11 p.m. UTC | #1 
On 02/26/2016 03:59 PM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   hw/i386/pc.c | 45 +++++++++++++++++++--------------------------
>   1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0aeefd2..151a64c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
>       }
>   }
>
> -/* Calculates the limit to CPU APIC ID values
> - *
> - * This function returns the limit for the APIC ID value, so that all
> - * CPU APIC IDs are < pc_apic_id_limit().
> - *
> - * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
> - */
> -static unsigned int pc_apic_id_limit(unsigned int max_cpus)
> -{
> -    return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> -}
> -
>   static void pc_build_smbios(FWCfgState *fw_cfg)
>   {
>       uint8_t *smbios_tables, *smbios_anchor;
> @@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
>       }
>   }
>
> -static FWCfgState *bochs_bios_init(AddressSpace *as)
> +static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
>   {
>       FWCfgState *fw_cfg;
>       uint64_t *numa_fw_cfg;
>       int i, j;
> -    unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
>
>       fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
>
> @@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
>        * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is
>        *     the APIC ID, not the "CPU index"
>        */
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit);
> +    fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
>       fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>       fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
>                        acpi_tables, acpi_tables_len);
> @@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
>        * of nodes, one word for each VCPU->node and one word for each node to
>        * hold the amount of memory.
>        */
> -    numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
> +    numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
>       numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
>       for (i = 0; i < max_cpus; i++) {
>           unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> -        assert(apic_id < apic_id_limit);
> +        assert(apic_id < pcms->apic_id_limit);
>           for (j = 0; j < nb_numa_nodes; j++) {
>               if (test_bit(i, numa_info[j].node_cpu)) {
>                   numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
> @@ -803,10 +790,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
>           }
>       }
>       for (i = 0; i < nb_numa_nodes; i++) {
> -        numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem);
> +        numa_fw_cfg[pcms->apic_id_limit + 1 + i] =
> +            cpu_to_le64(numa_info[i].node_mem);
>       }
>       fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
> -                     (1 + apic_id_limit + nb_numa_nodes) *
> +                     (1 + pcms->apic_id_limit + nb_numa_nodes) *
>                        sizeof(*numa_fw_cfg));
>
>       return fw_cfg;
> @@ -1120,7 +1108,6 @@ void pc_cpus_init(PCMachineState *pcms)
>       int i;
>       X86CPU *cpu = NULL;
>       MachineState *machine = MACHINE(pcms);
> -    unsigned long apic_id_limit;
>
>       /* init CPUs */
>       if (machine->cpu_model == NULL) {
> @@ -1131,10 +1118,17 @@ void pc_cpus_init(PCMachineState *pcms)
>   #endif
>       }
>
> -    apic_id_limit = pc_apic_id_limit(max_cpus);
> -    if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> -        error_report("max_cpus is too large. APIC ID of last CPU is %lu",
> -                     apic_id_limit - 1);
> +    /* Calculates the limit to CPU APIC ID values
> +     *
> +     * Limit for the APIC ID value, so that all
> +     * CPU APIC IDs are < pcms->apic_id_limit.
> +     *
> +     * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
> +     */
> +    pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> +    if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> +        error_report("max_cpus is too large. APIC ID of last CPU is %u",
> +                     pcms->apic_id_limit - 1);
>           exit(1);
>       }
>
> @@ -1187,7 +1181,6 @@ void pc_guest_info_init(PCMachineState *pcms)
>   {
>       int i, j;
>
> -    pcms->apic_id_limit = pc_apic_id_limit(max_cpus);
>       pcms->apic_xrupt_override = kvm_allows_irq0_override();
>       pcms->numa_nodes = nb_numa_nodes;
>       pcms->node_mem = g_malloc0(pcms->numa_nodes *
> @@ -1372,7 +1365,7 @@ void pc_memory_init(PCMachineState *pcms,
>                                           option_rom_mr,
>                                           1);
>
> -    fw_cfg = bochs_bios_init(&address_space_memory);
> +    fw_cfg = bochs_bios_init(&address_space_memory, pcms);
>
>       rom_set_fw(fw_cfg);
>
>


Looks OK to me.

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0aeefd2..151a64c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -700,18 +700,6 @@  static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
     }
 }
 
-/* Calculates the limit to CPU APIC ID values
- *
- * This function returns the limit for the APIC ID value, so that all
- * CPU APIC IDs are < pc_apic_id_limit().
- *
- * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
- */
-static unsigned int pc_apic_id_limit(unsigned int max_cpus)
-{
-    return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
-}
-
 static void pc_build_smbios(FWCfgState *fw_cfg)
 {
     uint8_t *smbios_tables, *smbios_anchor;
@@ -749,12 +737,11 @@  static void pc_build_smbios(FWCfgState *fw_cfg)
     }
 }
 
-static FWCfgState *bochs_bios_init(AddressSpace *as)
+static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
 {
     FWCfgState *fw_cfg;
     uint64_t *numa_fw_cfg;
     int i, j;
-    unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
     fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
 
@@ -772,7 +759,7 @@  static FWCfgState *bochs_bios_init(AddressSpace *as)
      * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is
      *     the APIC ID, not the "CPU index"
      */
-    fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
     fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
                      acpi_tables, acpi_tables_len);
@@ -790,11 +777,11 @@  static FWCfgState *bochs_bios_init(AddressSpace *as)
      * of nodes, one word for each VCPU->node and one word for each node to
      * hold the amount of memory.
      */
-    numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
+    numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
     numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
     for (i = 0; i < max_cpus; i++) {
         unsigned int apic_id = x86_cpu_apic_id_from_index(i);
-        assert(apic_id < apic_id_limit);
+        assert(apic_id < pcms->apic_id_limit);
         for (j = 0; j < nb_numa_nodes; j++) {
             if (test_bit(i, numa_info[j].node_cpu)) {
                 numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
@@ -803,10 +790,11 @@  static FWCfgState *bochs_bios_init(AddressSpace *as)
         }
     }
     for (i = 0; i < nb_numa_nodes; i++) {
-        numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem);
+        numa_fw_cfg[pcms->apic_id_limit + 1 + i] =
+            cpu_to_le64(numa_info[i].node_mem);
     }
     fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
-                     (1 + apic_id_limit + nb_numa_nodes) *
+                     (1 + pcms->apic_id_limit + nb_numa_nodes) *
                      sizeof(*numa_fw_cfg));
 
     return fw_cfg;
@@ -1120,7 +1108,6 @@  void pc_cpus_init(PCMachineState *pcms)
     int i;
     X86CPU *cpu = NULL;
     MachineState *machine = MACHINE(pcms);
-    unsigned long apic_id_limit;
 
     /* init CPUs */
     if (machine->cpu_model == NULL) {
@@ -1131,10 +1118,17 @@  void pc_cpus_init(PCMachineState *pcms)
 #endif
     }
 
-    apic_id_limit = pc_apic_id_limit(max_cpus);
-    if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
-        error_report("max_cpus is too large. APIC ID of last CPU is %lu",
-                     apic_id_limit - 1);
+    /* Calculates the limit to CPU APIC ID values
+     *
+     * Limit for the APIC ID value, so that all
+     * CPU APIC IDs are < pcms->apic_id_limit.
+     *
+     * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
+     */
+    pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
+    if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
+        error_report("max_cpus is too large. APIC ID of last CPU is %u",
+                     pcms->apic_id_limit - 1);
         exit(1);
     }
 
@@ -1187,7 +1181,6 @@  void pc_guest_info_init(PCMachineState *pcms)
 {
     int i, j;
 
-    pcms->apic_id_limit = pc_apic_id_limit(max_cpus);
     pcms->apic_xrupt_override = kvm_allows_irq0_override();
     pcms->numa_nodes = nb_numa_nodes;
     pcms->node_mem = g_malloc0(pcms->numa_nodes *
@@ -1372,7 +1365,7 @@  void pc_memory_init(PCMachineState *pcms,
                                         option_rom_mr,
                                         1);
 
-    fw_cfg = bochs_bios_init(&address_space_memory);
+    fw_cfg = bochs_bios_init(&address_space_memory, pcms);
 
     rom_set_fw(fw_cfg);