Re: [PATCH v3 04/18] hw/i386: Introduce initialize_topo_info to initialize X86CPUTopoInfo
Igor,
On 1/28/20 9:49 AM, Igor Mammedov wrote:
> On Tue, 03 Dec 2019 18:37:21 -0600
> Babu Moger wrote:
>
>> Initialize all the parameters in one function initialize_topo_info.
>>
>> Signed-off-by: Babu Moger
>> Reviewed-by: Eduardo Habkost
>> ---
>> 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(_info, pcms, ms);
>>
>> correct_id = x86_apicid_from_cpu_idx(_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(_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(_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(_info, pcms, ms);
>> +
>> ms->possible_cpus->len = max_cpus;
>> for (i = 0; i < ms->possible_cpus->len; i++) {
>> X86CPUTopoIDs topo_ids;
>>
>
Re: [PATCH v3 04/18] hw/i386: Introduce initialize_topo_info to initialize X86CPUTopoInfo
On Tue, 03 Dec 2019 18:37:21 -0600
Babu Moger wrote:
> Initialize all the parameters in one function initialize_topo_info.
>
> Signed-off-by: Babu Moger
> Reviewed-by: Eduardo Habkost
> ---
> 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(_info, pcms, ms);
>
> correct_id = x86_apicid_from_cpu_idx(_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(_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(_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(_info, pcms, ms);
> +
> ms->possible_cpus->len = max_cpus;
> for (i = 0; i < ms->possible_cpus->len; i++) {
> X86CPUTopoIDs topo_ids;
>
[PATCH v3 04/18] hw/i386: Introduce initialize_topo_info to initialize X86CPUTopoInfo
Initialize all the parameters in one function initialize_topo_info.
Signed-off-by: Babu Moger
Reviewed-by: Eduardo Habkost
---
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,
+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(_info, pcms, ms);
correct_id = x86_apicid_from_cpu_idx(_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(_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(_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(_info, pcms, ms);
+
ms->possible_cpus->len = max_cpus;
for (i = 0; i < ms->possible_cpus->len; i++) {
X86CPUTopoIDs topo_ids;
