On Thu, 13 Feb 2020 12:16:51 -0600
Babu Moger <babu.mo...@amd.com> wrote:
> Initialize all the parameters in one function init_topo_info.
is it possible to squash it in 2/16
>
> Move the data structure X86CPUTopoIDs and X86CPUTopoInfo into
> x86.h.
A reason why it's moved should be here.
>
> Signed-off-by: Babu Moger <babu.mo...@amd.com>
> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
> ---
> hw/i386/pc.c | 4 +---
> hw/i386/x86.c | 14 +++-----------
> include/hw/i386/topology.h | 26 ++++++++++----------------
> include/hw/i386/x86.h | 17 +++++++++++++++++
> 4 files changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2adf7f6afa..9803413dd9 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1749,9 +1749,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
> return;
> }
>
> - topo_info.dies_per_pkg = x86ms->smp_dies;
> - topo_info.cores_per_die = smp_cores;
> - topo_info.threads_per_core = smp_threads;
> + init_topo_info(&topo_info, x86ms);
>
> env->nr_dies = x86ms->smp_dies;
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index f18cab8e5c..083effb2f5 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -63,15 +63,12 @@ static size_t pvh_start_addr;
> uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
> unsigned int cpu_index)
> {
> - MachineState *ms = MACHINE(x86ms);
> X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms);
> X86CPUTopoInfo topo_info;
> uint32_t correct_id;
> static bool warned;
>
> - topo_info.dies_per_pkg = x86ms->smp_dies;
> - topo_info.cores_per_die = ms->smp.cores;
> - topo_info.threads_per_core = ms->smp.threads;
> + init_topo_info(&topo_info, x86ms);
>
> correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
> if (x86mc->compat_apic_id_mode) {
> @@ -146,10 +143,7 @@ int64_t x86_get_default_cpu_node_id(const MachineState
> *ms, int idx)
> X86MachineState *x86ms = X86_MACHINE(ms);
> X86CPUTopoInfo topo_info;
>
> - topo_info.dies_per_pkg = x86ms->smp_dies;
> - topo_info.cores_per_die = ms->smp.cores;
> - topo_info.threads_per_core = ms->smp.threads;
> -
> + init_topo_info(&topo_info, x86ms);
>
> assert(idx < ms->possible_cpus->len);
> x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
> @@ -177,9 +171,7 @@ const CPUArchIdList
> *x86_possible_cpu_arch_ids(MachineState *ms)
> sizeof(CPUArchId) * max_cpus);
> ms->possible_cpus->len = max_cpus;
>
> - topo_info.dies_per_pkg = x86ms->smp_dies;
> - topo_info.cores_per_die = ms->smp.cores;
> - topo_info.threads_per_core = ms->smp.threads;
> + init_topo_info(&topo_info, x86ms);
>
> for (i = 0; i < ms->possible_cpus->len; i++) {
> X86CPUTopoIDs topo_ids;
> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> index ba52d49079..ef0ab0b6a3 100644
> --- a/include/hw/i386/topology.h
> +++ b/include/hw/i386/topology.h
> @@ -40,23 +40,17 @@
>
>
> #include "qemu/bitops.h"
> +#include "hw/i386/x86.h"
>
> -/* APIC IDs can be 32-bit, but beware: APIC IDs > 255 require x2APIC support
> - */
> -typedef uint32_t apic_id_t;
> -
> -typedef struct X86CPUTopoIDs {
> - unsigned pkg_id;
> - unsigned die_id;
> - unsigned core_id;
> - unsigned smt_id;
> -} X86CPUTopoIDs;
> -
> -typedef struct X86CPUTopoInfo {
> - unsigned dies_per_pkg;
> - unsigned cores_per_die;
> - unsigned threads_per_core;
> -} X86CPUTopoInfo;
> +static inline void init_topo_info(X86CPUTopoInfo *topo_info,
> + const X86MachineState *x86ms)
> +{
> + MachineState *ms = MACHINE(x86ms);
> +
> + topo_info->dies_per_pkg = x86ms->smp_dies;
> + topo_info->cores_per_die = ms->smp.cores;
> + topo_info->threads_per_core = ms->smp.threads;
> +}
>
> /* Return the bit width needed for 'count' IDs
> */
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index 4b84917885..ad62b01cf2 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -36,6 +36,23 @@ typedef struct {
> bool compat_apic_id_mode;
> } X86MachineClass;
>
> +/* APIC IDs can be 32-bit, but beware: APIC IDs > 255 require x2APIC support
> + */
> +typedef uint32_t apic_id_t;
> +
> +typedef struct X86CPUTopoIDs {
> + unsigned pkg_id;
> + unsigned die_id;
> + unsigned core_id;
> + unsigned smt_id;
> +} X86CPUTopoIDs;
> +
> +typedef struct X86CPUTopoInfo {
> + unsigned dies_per_pkg;
> + unsigned cores_per_die;
> + unsigned threads_per_core;
> +} X86CPUTopoInfo;
> +
> typedef struct {
> /*< private >*/
> MachineState parent;
>
