Message ID | 157541984181.46157.12341489595513709747.stgit@naples-babu.amd.com |
---|---|
State | New |
Headers | show |
Series | APIC ID fixes for AMD EPYC CPU models | expand |
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; >
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 --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;