diff mbox series

[v3,04/18] hw/i386: Introduce initialize_topo_info to initialize X86CPUTopoInfo

Message ID 157541984181.46157.12341489595513709747.stgit@naples-babu.amd.com (mailing list archive)
State New, archived
Headers show
Series APIC ID fixes for AMD EPYC CPU models | expand

Commit Message

Moger, Babu Dec. 4, 2019, 12:37 a.m. UTC 
Initialize all the parameters in one function initialize_topo_info.

Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c |   28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

Comments

Igor Mammedov Jan. 28, 2020, 3:49 p.m. UTC | #1 
On Tue, 03 Dec 2019 18:37:21 -0600
Babu Moger <babu.moger@amd.com> wrote:

> Initialize all the parameters in one function initialize_topo_info.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/i386/pc.c |   28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8c23b1e8c9..cafbdafa76 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -866,6 +866,15 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
>      x86_cpu_set_a20(cpu, level);
>  }
>  
> +static inline void initialize_topo_info(X86CPUTopoInfo *topo_info,
> +                                        PCMachineState *pcms,

maybe use 'const'

> +                                        const MachineState *ms)
'ms' is the same thing as 'pcms', so why pass it around separately?

you can just do
   MachineState *ms = MACHINE(pcms)
inside of function

> +{
> +    topo_info->dies_per_pkg = pcms->smp_dies;
> +    topo_info->cores_per_die = ms->smp.cores;
> +    topo_info->threads_per_core = ms->smp.threads;
> +}
> +
>  /* Calculates initial APIC ID for a specific CPU index
>   *
>   * Currently we need to be able to calculate the APIC ID from the CPU index
> @@ -882,9 +891,7 @@ static uint32_t x86_cpu_apic_id_from_index(PCMachineState *pcms,
>      uint32_t correct_id;
>      static bool warned;
>  
> -    topo_info.dies_per_pkg = pcms->smp_dies;
> -    topo_info.cores_per_die = ms->smp.cores;
> -    topo_info.threads_per_core = ms->smp.threads;
> +    initialize_topo_info(&topo_info, pcms, ms);
>  
>      correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
>      if (pcmc->compat_apic_id_mode) {
> @@ -2231,9 +2238,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>          return;
>      }
>  
> -    topo_info.dies_per_pkg = pcms->smp_dies;
> -    topo_info.cores_per_die = smp_cores;
> -    topo_info.threads_per_core = smp_threads;
> +    initialize_topo_info(&topo_info, pcms, ms);
>  
>      env->nr_dies = pcms->smp_dies;
>  
> @@ -2702,9 +2707,7 @@ static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx)
>     PCMachineState *pcms = PC_MACHINE(ms);
>     X86CPUTopoInfo topo_info;
>  
> -   topo_info.dies_per_pkg = pcms->smp_dies;
> -   topo_info.cores_per_die = ms->smp.cores;
> -   topo_info.threads_per_core = ms->smp.threads;
> +   initialize_topo_info(&topo_info, pcms, ms);
>  
>     assert(idx < ms->possible_cpus->len);
>     x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
> @@ -2719,10 +2722,6 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>      X86CPUTopoInfo topo_info;
>      int i;
>  
> -    topo_info.dies_per_pkg = pcms->smp_dies;
> -    topo_info.cores_per_die = ms->smp.cores;
> -    topo_info.threads_per_core = ms->smp.threads;
> -
>      if (ms->possible_cpus) {
>          /*
>           * make sure that max_cpus hasn't changed since the first use, i.e.
> @@ -2734,6 +2733,9 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>  
>      ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
>                                    sizeof(CPUArchId) * max_cpus);
> +
> +    initialize_topo_info(&topo_info, pcms, ms);
> +
>      ms->possible_cpus->len = max_cpus;
>      for (i = 0; i < ms->possible_cpus->len; i++) {
>          X86CPUTopoIDs topo_ids;
>
Moger, Babu Jan. 28, 2020, 4:42 p.m. UTC | #2 
Igor,

On 1/28/20 9:49 AM, Igor Mammedov wrote:
> On Tue, 03 Dec 2019 18:37:21 -0600
> Babu Moger <babu.moger@amd.com> wrote:
> 
>> Initialize all the parameters in one function initialize_topo_info.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  hw/i386/pc.c |   28 +++++++++++++++-------------
>>  1 file changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 8c23b1e8c9..cafbdafa76 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -866,6 +866,15 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
>>      x86_cpu_set_a20(cpu, level);
>>  }
>>  
>> +static inline void initialize_topo_info(X86CPUTopoInfo *topo_info,
>> +                                        PCMachineState *pcms,
> 
> maybe use 'const'
> 
>> +                                        const MachineState *ms)
> 'ms' is the same thing as 'pcms', so why pass it around separately?
> 
> you can just do
>    MachineState *ms = MACHINE(pcms)
> inside of function

Yes. We can do that. Thanks

> 
>> +{
>> +    topo_info->dies_per_pkg = pcms->smp_dies;
>> +    topo_info->cores_per_die = ms->smp.cores;
>> +    topo_info->threads_per_core = ms->smp.threads;
>> +}
>> +
>>  /* Calculates initial APIC ID for a specific CPU index
>>   *
>>   * Currently we need to be able to calculate the APIC ID from the CPU index
>> @@ -882,9 +891,7 @@ static uint32_t x86_cpu_apic_id_from_index(PCMachineState *pcms,
>>      uint32_t correct_id;
>>      static bool warned;
>>  
>> -    topo_info.dies_per_pkg = pcms->smp_dies;
>> -    topo_info.cores_per_die = ms->smp.cores;
>> -    topo_info.threads_per_core = ms->smp.threads;
>> +    initialize_topo_info(&topo_info, pcms, ms);
>>  
>>      correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
>>      if (pcmc->compat_apic_id_mode) {
>> @@ -2231,9 +2238,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>>          return;
>>      }
>>  
>> -    topo_info.dies_per_pkg = pcms->smp_dies;
>> -    topo_info.cores_per_die = smp_cores;
>> -    topo_info.threads_per_core = smp_threads;
>> +    initialize_topo_info(&topo_info, pcms, ms);
>>  
>>      env->nr_dies = pcms->smp_dies;
>>  
>> @@ -2702,9 +2707,7 @@ static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx)
>>     PCMachineState *pcms = PC_MACHINE(ms);
>>     X86CPUTopoInfo topo_info;
>>  
>> -   topo_info.dies_per_pkg = pcms->smp_dies;
>> -   topo_info.cores_per_die = ms->smp.cores;
>> -   topo_info.threads_per_core = ms->smp.threads;
>> +   initialize_topo_info(&topo_info, pcms, ms);
>>  
>>     assert(idx < ms->possible_cpus->len);
>>     x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
>> @@ -2719,10 +2722,6 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>>      X86CPUTopoInfo topo_info;
>>      int i;
>>  
>> -    topo_info.dies_per_pkg = pcms->smp_dies;
>> -    topo_info.cores_per_die = ms->smp.cores;
>> -    topo_info.threads_per_core = ms->smp.threads;
>> -
>>      if (ms->possible_cpus) {
>>          /*
>>           * make sure that max_cpus hasn't changed since the first use, i.e.
>> @@ -2734,6 +2733,9 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>>  
>>      ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
>>                                    sizeof(CPUArchId) * max_cpus);
>> +
>> +    initialize_topo_info(&topo_info, pcms, ms);
>> +
>>      ms->possible_cpus->len = max_cpus;
>>      for (i = 0; i < ms->possible_cpus->len; i++) {
>>          X86CPUTopoIDs topo_ids;
>>
>
diff mbox series

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8c23b1e8c9..cafbdafa76 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -866,6 +866,15 @@  static void handle_a20_line_change(void *opaque, int irq, int level)
     x86_cpu_set_a20(cpu, level);
 }
 
+static inline void initialize_topo_info(X86CPUTopoInfo *topo_info,
+                                        PCMachineState *pcms,
+                                        const MachineState *ms)
+{
+    topo_info->dies_per_pkg = pcms->smp_dies;
+    topo_info->cores_per_die = ms->smp.cores;
+    topo_info->threads_per_core = ms->smp.threads;
+}
+
 /* Calculates initial APIC ID for a specific CPU index
  *
  * Currently we need to be able to calculate the APIC ID from the CPU index
@@ -882,9 +891,7 @@  static uint32_t x86_cpu_apic_id_from_index(PCMachineState *pcms,
     uint32_t correct_id;
     static bool warned;
 
-    topo_info.dies_per_pkg = pcms->smp_dies;
-    topo_info.cores_per_die = ms->smp.cores;
-    topo_info.threads_per_core = ms->smp.threads;
+    initialize_topo_info(&topo_info, pcms, ms);
 
     correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
     if (pcmc->compat_apic_id_mode) {
@@ -2231,9 +2238,7 @@  static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
 
-    topo_info.dies_per_pkg = pcms->smp_dies;
-    topo_info.cores_per_die = smp_cores;
-    topo_info.threads_per_core = smp_threads;
+    initialize_topo_info(&topo_info, pcms, ms);
 
     env->nr_dies = pcms->smp_dies;
 
@@ -2702,9 +2707,7 @@  static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx)
    PCMachineState *pcms = PC_MACHINE(ms);
    X86CPUTopoInfo topo_info;
 
-   topo_info.dies_per_pkg = pcms->smp_dies;
-   topo_info.cores_per_die = ms->smp.cores;
-   topo_info.threads_per_core = ms->smp.threads;
+   initialize_topo_info(&topo_info, pcms, ms);
 
    assert(idx < ms->possible_cpus->len);
    x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
@@ -2719,10 +2722,6 @@  static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
     X86CPUTopoInfo topo_info;
     int i;
 
-    topo_info.dies_per_pkg = pcms->smp_dies;
-    topo_info.cores_per_die = ms->smp.cores;
-    topo_info.threads_per_core = ms->smp.threads;
-
     if (ms->possible_cpus) {
         /*
          * make sure that max_cpus hasn't changed since the first use, i.e.
@@ -2734,6 +2733,9 @@  static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
 
     ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
                                   sizeof(CPUArchId) * max_cpus);
+
+    initialize_topo_info(&topo_info, pcms, ms);
+
     ms->possible_cpus->len = max_cpus;
     for (i = 0; i < ms->possible_cpus->len; i++) {
         X86CPUTopoIDs topo_ids;