Igor, On 1/28/20 9:49 AM, Igor Mammedov wrote: > On Tue, 03 Dec 2019 18:37:21 -0600 > Babu Moger <babu.mo...@amd.com> wrote: > >> Initialize all the parameters in one function initialize_topo_info. >> >> Signed-off-by: Babu Moger <babu.mo...@amd.com> >> Reviewed-by: Eduardo Habkost <ehabk...@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; >> >