Re: [Qemu-devel] [RFC 2 PATCH 15/16] i386: Fix pkg_id offset for epyc mode

[Eduardo Habkost]

On Fri, Sep 06, 2019 at 07:13:22PM +, Moger, Babu wrote:
> Signed-off-by: Babu Moger 
> ---
>  target/i386/cpu.c |   24 +++-
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f25491a029..f8b1fc5c07 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4094,9 +4094,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
> uint32_t *eax, uint32_t *ebx,
> uint32_t *ecx, uint32_t *edx)
>  {
> +MachineState *ms = MACHINE(qdev_get_machine());
>  X86CPU *cpu = env_archcpu(env);
>  CPUState *cs = env_cpu(env);
> -uint32_t die_offset;
> +uint32_t die_offset, pkg_offset;
>  uint32_t limit;
>  uint32_t signature[3];
>  
> @@ -4119,6 +4120,21 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>  index = env->cpuid_level;
>  }
>  
> +if (ms->epyc) {
> +X86CPUTopoInfo topo_info = {
> +.numa_nodes = nb_numa_nodes,
> +.nr_sockets = ms->smp.sockets,
> +.nr_cores = ms->smp.cores,
> +.nr_threads = ms->smp.threads,
> +};

Why aren't you using initialize_topo_info() here?

> +unsigned nodes = nodes_in_pkg(_info);
> +pkg_offset = apicid_pkg_offset_epyc(nodes, MAX_CCX, MAX_CORES_IN_CCX,
> +cs->nr_threads);
> +} else {
> +pkg_offset = apicid_pkg_offset(env->nr_dies, cs->nr_cores,
> +   cs->nr_threads);
> +}

I think we won't need this at all if we simply:

1) Add nodes_per_pkg to X86CPUTopoInfo;
2) make nodes_per_pkg configurable;
3) just use topo->nr_dies and topo->nr_cores instead of those
   constants.

-- 
Eduardo

[Qemu-devel] [RFC 2 PATCH 15/16] i386: Fix pkg_id offset for epyc mode

[Moger, Babu]

Signed-off-by: Babu Moger 
---
 target/i386/cpu.c |   24 +++-
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f25491a029..f8b1fc5c07 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4094,9 +4094,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx)
 {
+MachineState *ms = MACHINE(qdev_get_machine());
 X86CPU *cpu = env_archcpu(env);
 CPUState *cs = env_cpu(env);
-uint32_t die_offset;
+uint32_t die_offset, pkg_offset;
 uint32_t limit;
 uint32_t signature[3];
 
@@ -4119,6 +4120,21 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 index = env->cpuid_level;
 }
 
+if (ms->epyc) {
+X86CPUTopoInfo topo_info = {
+.numa_nodes = nb_numa_nodes,
+.nr_sockets = ms->smp.sockets,
+.nr_cores = ms->smp.cores,
+.nr_threads = ms->smp.threads,
+};
+unsigned nodes = nodes_in_pkg(_info);
+pkg_offset = apicid_pkg_offset_epyc(nodes, MAX_CCX, MAX_CORES_IN_CCX,
+cs->nr_threads);
+} else {
+pkg_offset = apicid_pkg_offset(env->nr_dies, cs->nr_cores,
+   cs->nr_threads);
+}
+
 switch(index) {
 case 0:
 *eax = env->cpuid_level;
@@ -4275,8 +4291,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
 break;
 case 1:
-*eax = apicid_pkg_offset(env->nr_dies,
- cs->nr_cores, cs->nr_threads);
+*eax = pkg_offset;
 *ebx = cs->nr_cores * cs->nr_threads;
 *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
 break;
@@ -4310,8 +4325,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
 break;
 case 2:
-*eax = apicid_pkg_offset(env->nr_dies, cs->nr_cores,
-   cs->nr_threads);
+*eax = pkg_offset;
 *ebx = env->nr_dies * cs->nr_cores * cs->nr_threads;
 *ecx |= CPUID_TOPOLOGY_LEVEL_DIE;
 break;