Re: [PATCH RESEND 07/18] i386: Support modules_per_die in X86CPUTopoInfo

2023-02-15 Thread Zhao Liu
On Thu, Feb 16, 2023 at 10:34:24AM +0800, wangyanan (Y) wrote:
> Date: Thu, 16 Feb 2023 10:34:24 +0800
> From: "wangyanan (Y)" 
> Subject: Re: [PATCH RESEND 07/18] i386: Support modules_per_die in
>  X86CPUTopoInfo
> 
> Hi Zhao,
> 
> 在 2023/2/13 17:36, Zhao Liu 写道:
> > From: Zhuocheng Ding 
> > 
> > Support module level in i386 cpu topology structure "X86CPUTopoInfo".
> > 
> > Before updating APIC ID parsing rule with module level, the
> > apicid_core_width() temporarily combines the core and module levels
> > together.
> If we dont merge this one with the followed patches, then nits below
> may be meaningful.
> > At present, we don't expose module level in CPUID.1FH because currently
> > linux (v6.2-rc6) doesn't support module level. And exposing module and
> > die levels at the same time in CPUID.1FH will cause linux to calculate
> > the wrong die_id. The module level should be exposed until the real
> > machine has the module level in CPUID.1FH.
> > 
> > In addition, update topology structure in test-x86-apicid.c.
> > 
> > Signed-off-by: Zhuocheng Ding 
> > Co-developed-by: Zhao Liu 
> > Signed-off-by: Zhao Liu 
> > ---
> >   hw/i386/x86.c|  3 ++-
> >   include/hw/i386/topology.h   | 13 ---
> >   target/i386/cpu.c| 17 --
> >   tests/unit/test-x86-apicid.c | 45 +++-
> >   4 files changed, 46 insertions(+), 32 deletions(-)
> > 
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index ae1bb562d6e2..1c069ff56ae7 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -71,7 +71,8 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
> >   MachineState *ms = MACHINE(x86ms);
> >   topo_info->dies_per_pkg = ms->smp.dies;
> > -topo_info->cores_per_die = ms->smp.cores;
> > +topo_info->modules_per_die = ms->smp.clusters;
> > +topo_info->cores_per_module = ms->smp.cores;
> Here we can ensure that topo_info->modules_per_die is always 1, so...
> >   topo_info->threads_per_core = ms->smp.threads;
> >   }
> > diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> > index 81573f6cfde0..bbb00dc4aad8 100644
> > --- a/include/hw/i386/topology.h
> > +++ b/include/hw/i386/topology.h
> > @@ -54,7 +54,8 @@ typedef struct X86CPUTopoIDs {
> >   typedef struct X86CPUTopoInfo {
> >   unsigned dies_per_pkg;
> > -unsigned cores_per_die;
> > +unsigned modules_per_die;
> > +unsigned cores_per_module;
> >   unsigned threads_per_core;
> >   } X86CPUTopoInfo;
> > @@ -78,7 +79,12 @@ static inline unsigned apicid_smt_width(X86CPUTopoInfo 
> > *topo_info)
> >*/
> >   static inline unsigned apicid_core_width(X86CPUTopoInfo *topo_info)
> >   {
> > -return apicid_bitwidth_for_count(topo_info->cores_per_die);
> > +/*
> > + * TODO: Will separate module info from core_width when update
> > + * APIC ID with module level.
> > + */
> > +return apicid_bitwidth_for_count(topo_info->cores_per_module *
> > + topo_info->modules_per_die);
> >   }
> ...We can directly add apicid_module_width (which returns 0 so far)
> and apicid_module_offset here which don't rely on the APIC ID rule
> change, and avoid the "TODO..".
> 
> Then patch 8 and 10 are about module_id, so can be merged.
> Is this good?

Thanks Yanan, this makes sense. I'll do it.

Zhao

> 
> Thanks,
> Yanan
> >   /* Bit width of the Die_ID field */
> > @@ -128,7 +134,8 @@ static inline void x86_topo_ids_from_idx(X86CPUTopoInfo 
> > *topo_info,
> >X86CPUTopoIDs *topo_ids)
> >   {
> >   unsigned nr_dies = topo_info->dies_per_pkg;
> > -unsigned nr_cores = topo_info->cores_per_die;
> > +unsigned nr_cores = topo_info->cores_per_module *
> > +topo_info->modules_per_die;
> >   unsigned nr_threads = topo_info->threads_per_core;
> >   topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 61ec9a7499b8..6f3d114c7d12 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -336,7 +336,9 @@ static void encode_cache_cpuid801d(CPUCacheInfo 
> > *cache,
> >   /* L3 is shared among multiple cores */
> >   if (cache->level == 3) {
> > -l3_threads = topo_info-&g

Re: [PATCH RESEND 07/18] i386: Support modules_per_die in X86CPUTopoInfo

2023-02-15 Thread wangyanan (Y)

Hi Zhao,

在 2023/2/13 17:36, Zhao Liu 写道:

From: Zhuocheng Ding 

Support module level in i386 cpu topology structure "X86CPUTopoInfo".

Before updating APIC ID parsing rule with module level, the
apicid_core_width() temporarily combines the core and module levels
together.

If we dont merge this one with the followed patches, then nits below
may be meaningful.

At present, we don't expose module level in CPUID.1FH because currently
linux (v6.2-rc6) doesn't support module level. And exposing module and
die levels at the same time in CPUID.1FH will cause linux to calculate
the wrong die_id. The module level should be exposed until the real
machine has the module level in CPUID.1FH.

In addition, update topology structure in test-x86-apicid.c.

Signed-off-by: Zhuocheng Ding 
Co-developed-by: Zhao Liu 
Signed-off-by: Zhao Liu 
---
  hw/i386/x86.c|  3 ++-
  include/hw/i386/topology.h   | 13 ---
  target/i386/cpu.c| 17 --
  tests/unit/test-x86-apicid.c | 45 +++-
  4 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ae1bb562d6e2..1c069ff56ae7 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -71,7 +71,8 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
  MachineState *ms = MACHINE(x86ms);
  
  topo_info->dies_per_pkg = ms->smp.dies;

-topo_info->cores_per_die = ms->smp.cores;
+topo_info->modules_per_die = ms->smp.clusters;
+topo_info->cores_per_module = ms->smp.cores;

Here we can ensure that topo_info->modules_per_die is always 1, so...

  topo_info->threads_per_core = ms->smp.threads;
  }
  
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h

index 81573f6cfde0..bbb00dc4aad8 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -54,7 +54,8 @@ typedef struct X86CPUTopoIDs {
  
  typedef struct X86CPUTopoInfo {

  unsigned dies_per_pkg;
-unsigned cores_per_die;
+unsigned modules_per_die;
+unsigned cores_per_module;
  unsigned threads_per_core;
  } X86CPUTopoInfo;
  
@@ -78,7 +79,12 @@ static inline unsigned apicid_smt_width(X86CPUTopoInfo *topo_info)

   */
  static inline unsigned apicid_core_width(X86CPUTopoInfo *topo_info)
  {
-return apicid_bitwidth_for_count(topo_info->cores_per_die);
+/*
+ * TODO: Will separate module info from core_width when update
+ * APIC ID with module level.
+ */
+return apicid_bitwidth_for_count(topo_info->cores_per_module *
+ topo_info->modules_per_die);
  }

...We can directly add apicid_module_width (which returns 0 so far)
and apicid_module_offset here which don't rely on the APIC ID rule
change, and avoid the "TODO..".

Then patch 8 and 10 are about module_id, so can be merged.
Is this good?

Thanks,
Yanan

  /* Bit width of the Die_ID field */
@@ -128,7 +134,8 @@ static inline void x86_topo_ids_from_idx(X86CPUTopoInfo 
*topo_info,
   X86CPUTopoIDs *topo_ids)
  {
  unsigned nr_dies = topo_info->dies_per_pkg;
-unsigned nr_cores = topo_info->cores_per_die;
+unsigned nr_cores = topo_info->cores_per_module *
+topo_info->modules_per_die;
  unsigned nr_threads = topo_info->threads_per_core;
  
  topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 61ec9a7499b8..6f3d114c7d12 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -336,7 +336,9 @@ static void encode_cache_cpuid801d(CPUCacheInfo *cache,
  
  /* L3 is shared among multiple cores */

  if (cache->level == 3) {
-l3_threads = topo_info->cores_per_die * topo_info->threads_per_core;
+l3_threads = topo_info->modules_per_die *
+ topo_info->cores_per_module *
+ topo_info->threads_per_core;
  *eax |= (l3_threads - 1) << 14;
  } else {
  *eax |= ((topo_info->threads_per_core - 1) << 14);
@@ -5218,11 +5220,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
  uint32_t cpus_per_pkg;
  
  topo_info.dies_per_pkg = env->nr_dies;

-topo_info.cores_per_die = cs->nr_cores / env->nr_dies;
+topo_info.modules_per_die = env->nr_modules;
+topo_info.cores_per_module = cs->nr_cores / env->nr_dies / env->nr_modules;
  topo_info.threads_per_core = cs->nr_threads;
  
-cpus_per_pkg = topo_info.dies_per_pkg * topo_info.cores_per_die *

-   topo_info.threads_per_core;
+cpus_per_pkg = topo_info.dies_per_pkg * topo_info.modules_per_die *
+   topo_info.cores_per_module * topo_info.threads_per_core;
  
  /* Calculate & apply limits for different index ranges */

  if (index >= 0xC000) {
@@ -5298,8 +5301,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
  if (*eax & 31) {
  int 

Re: [PATCH RESEND 07/18] i386: Support modules_per_die in X86CPUTopoInfo

2023-02-15 Thread Zhao Liu
On Wed, Feb 15, 2023 at 06:38:31PM +0800, wangyanan (Y) wrote:
> Date: Wed, 15 Feb 2023 18:38:31 +0800
> From: "wangyanan (Y)" 
> Subject: Re: [PATCH RESEND 07/18] i386: Support modules_per_die in
>  X86CPUTopoInfo
> 
> 在 2023/2/13 17:36, Zhao Liu 写道:
> > From: Zhuocheng Ding 
> > 
> > Support module level in i386 cpu topology structure "X86CPUTopoInfo".
> > 
> > Before updating APIC ID parsing rule with module level, the
> > apicid_core_width() temporarily combines the core and module levels
> > together.
> > 
> > At present, we don't expose module level in CPUID.1FH because currently
> > linux (v6.2-rc6) doesn't support module level. And exposing module and
> > die levels at the same time in CPUID.1FH will cause linux to calculate
> > the wrong die_id. The module level should be exposed until the real
> > machine has the module level in CPUID.1FH.
> > 
> > In addition, update topology structure in test-x86-apicid.c.
> > 
> > Signed-off-by: Zhuocheng Ding 
> > Co-developed-by: Zhao Liu 
> > Signed-off-by: Zhao Liu 
> > ---
> >   hw/i386/x86.c|  3 ++-
> >   include/hw/i386/topology.h   | 13 ---
> >   target/i386/cpu.c| 17 --
> >   tests/unit/test-x86-apicid.c | 45 +++-
> >   4 files changed, 46 insertions(+), 32 deletions(-)
> > 
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index ae1bb562d6e2..1c069ff56ae7 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -71,7 +71,8 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
> >   MachineState *ms = MACHINE(x86ms);
> >   topo_info->dies_per_pkg = ms->smp.dies;
> > -topo_info->cores_per_die = ms->smp.cores;
> > +topo_info->modules_per_die = ms->smp.clusters;
> > +topo_info->cores_per_module = ms->smp.cores;
> >   topo_info->threads_per_core = ms->smp.threads;
> >   }
> > diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> > index 81573f6cfde0..bbb00dc4aad8 100644
> > --- a/include/hw/i386/topology.h
> > +++ b/include/hw/i386/topology.h
> > @@ -54,7 +54,8 @@ typedef struct X86CPUTopoIDs {
> >   typedef struct X86CPUTopoInfo {
> >   unsigned dies_per_pkg;
> > -unsigned cores_per_die;
> > +unsigned modules_per_die;
> > +unsigned cores_per_module;
> >   unsigned threads_per_core;
> >   } X86CPUTopoInfo;
> > @@ -78,7 +79,12 @@ static inline unsigned apicid_smt_width(X86CPUTopoInfo 
> > *topo_info)
> >*/
> >   static inline unsigned apicid_core_width(X86CPUTopoInfo *topo_info)
> >   {
> > -return apicid_bitwidth_for_count(topo_info->cores_per_die);
> > +/*
> > + * TODO: Will separate module info from core_width when update
> > + * APIC ID with module level.
> > + */
> > +return apicid_bitwidth_for_count(topo_info->cores_per_module *
> > + topo_info->modules_per_die);
> >   }
> >   /* Bit width of the Die_ID field */
> > @@ -128,7 +134,8 @@ static inline void x86_topo_ids_from_idx(X86CPUTopoInfo 
> > *topo_info,
> >X86CPUTopoIDs *topo_ids)
> >   {
> >   unsigned nr_dies = topo_info->dies_per_pkg;
> > -unsigned nr_cores = topo_info->cores_per_die;
> > +unsigned nr_cores = topo_info->cores_per_module *
> > +topo_info->modules_per_die;
> >   unsigned nr_threads = topo_info->threads_per_core;
> >   topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 61ec9a7499b8..6f3d114c7d12 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -336,7 +336,9 @@ static void encode_cache_cpuid801d(CPUCacheInfo 
> > *cache,
> >   /* L3 is shared among multiple cores */
> >   if (cache->level == 3) {
> > -l3_threads = topo_info->cores_per_die * 
> > topo_info->threads_per_core;
> > +l3_threads = topo_info->modules_per_die *
> > + topo_info->cores_per_module *
> > + topo_info->threads_per_core;
> >   *eax |= (l3_threads - 1) << 14;
> >   } else {
> >   *eax |= ((topo_info->threads_per_core - 1) << 14);
> > @@ -5218,11 +5220,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t 
> > index, uint32_t count,
> > 

Re: [PATCH RESEND 07/18] i386: Support modules_per_die in X86CPUTopoInfo

2023-02-15 Thread wangyanan (Y)

在 2023/2/13 17:36, Zhao Liu 写道:

From: Zhuocheng Ding 

Support module level in i386 cpu topology structure "X86CPUTopoInfo".

Before updating APIC ID parsing rule with module level, the
apicid_core_width() temporarily combines the core and module levels
together.

At present, we don't expose module level in CPUID.1FH because currently
linux (v6.2-rc6) doesn't support module level. And exposing module and
die levels at the same time in CPUID.1FH will cause linux to calculate
the wrong die_id. The module level should be exposed until the real
machine has the module level in CPUID.1FH.

In addition, update topology structure in test-x86-apicid.c.

Signed-off-by: Zhuocheng Ding 
Co-developed-by: Zhao Liu 
Signed-off-by: Zhao Liu 
---
  hw/i386/x86.c|  3 ++-
  include/hw/i386/topology.h   | 13 ---
  target/i386/cpu.c| 17 --
  tests/unit/test-x86-apicid.c | 45 +++-
  4 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ae1bb562d6e2..1c069ff56ae7 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -71,7 +71,8 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
  MachineState *ms = MACHINE(x86ms);
  
  topo_info->dies_per_pkg = ms->smp.dies;

-topo_info->cores_per_die = ms->smp.cores;
+topo_info->modules_per_die = ms->smp.clusters;
+topo_info->cores_per_module = ms->smp.cores;
  topo_info->threads_per_core = ms->smp.threads;
  }
  
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h

index 81573f6cfde0..bbb00dc4aad8 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -54,7 +54,8 @@ typedef struct X86CPUTopoIDs {
  
  typedef struct X86CPUTopoInfo {

  unsigned dies_per_pkg;
-unsigned cores_per_die;
+unsigned modules_per_die;
+unsigned cores_per_module;
  unsigned threads_per_core;
  } X86CPUTopoInfo;
  
@@ -78,7 +79,12 @@ static inline unsigned apicid_smt_width(X86CPUTopoInfo *topo_info)

   */
  static inline unsigned apicid_core_width(X86CPUTopoInfo *topo_info)
  {
-return apicid_bitwidth_for_count(topo_info->cores_per_die);
+/*
+ * TODO: Will separate module info from core_width when update
+ * APIC ID with module level.
+ */
+return apicid_bitwidth_for_count(topo_info->cores_per_module *
+ topo_info->modules_per_die);
  }
  
  /* Bit width of the Die_ID field */

@@ -128,7 +134,8 @@ static inline void x86_topo_ids_from_idx(X86CPUTopoInfo 
*topo_info,
   X86CPUTopoIDs *topo_ids)
  {
  unsigned nr_dies = topo_info->dies_per_pkg;
-unsigned nr_cores = topo_info->cores_per_die;
+unsigned nr_cores = topo_info->cores_per_module *
+topo_info->modules_per_die;
  unsigned nr_threads = topo_info->threads_per_core;
  
  topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 61ec9a7499b8..6f3d114c7d12 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -336,7 +336,9 @@ static void encode_cache_cpuid801d(CPUCacheInfo *cache,
  
  /* L3 is shared among multiple cores */

  if (cache->level == 3) {
-l3_threads = topo_info->cores_per_die * topo_info->threads_per_core;
+l3_threads = topo_info->modules_per_die *
+ topo_info->cores_per_module *
+ topo_info->threads_per_core;
  *eax |= (l3_threads - 1) << 14;
  } else {
  *eax |= ((topo_info->threads_per_core - 1) << 14);
@@ -5218,11 +5220,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
  uint32_t cpus_per_pkg;
  
  topo_info.dies_per_pkg = env->nr_dies;

-topo_info.cores_per_die = cs->nr_cores / env->nr_dies;
+topo_info.modules_per_die = env->nr_modules;
+topo_info.cores_per_module = cs->nr_cores / env->nr_dies / env->nr_modules;
  topo_info.threads_per_core = cs->nr_threads;
  
-cpus_per_pkg = topo_info.dies_per_pkg * topo_info.cores_per_die *

-   topo_info.threads_per_core;
+cpus_per_pkg = topo_info.dies_per_pkg * topo_info.modules_per_die *
+   topo_info.cores_per_module * topo_info.threads_per_core;
  
  /* Calculate & apply limits for different index ranges */

  if (index >= 0xC000) {
@@ -5298,8 +5301,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
  if (*eax & 31) {
  int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14);
  int vcpus_per_socket = cpus_per_pkg;
-int cores_per_socket = topo_info.cores_per_die *
-   topo_info.dies_per_pkg;
+int cores_per_socket = cpus_per_pkg /
+   topo_info.threads_per_core;

As mentioned in patch 5, cores_per_socket can be function-global.

  

[PATCH RESEND 07/18] i386: Support modules_per_die in X86CPUTopoInfo

2023-02-13 Thread Zhao Liu
From: Zhuocheng Ding 

Support module level in i386 cpu topology structure "X86CPUTopoInfo".

Before updating APIC ID parsing rule with module level, the
apicid_core_width() temporarily combines the core and module levels
together.

At present, we don't expose module level in CPUID.1FH because currently
linux (v6.2-rc6) doesn't support module level. And exposing module and
die levels at the same time in CPUID.1FH will cause linux to calculate
the wrong die_id. The module level should be exposed until the real
machine has the module level in CPUID.1FH.

In addition, update topology structure in test-x86-apicid.c.

Signed-off-by: Zhuocheng Ding 
Co-developed-by: Zhao Liu 
Signed-off-by: Zhao Liu 
---
 hw/i386/x86.c|  3 ++-
 include/hw/i386/topology.h   | 13 ---
 target/i386/cpu.c| 17 --
 tests/unit/test-x86-apicid.c | 45 +++-
 4 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ae1bb562d6e2..1c069ff56ae7 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -71,7 +71,8 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
 MachineState *ms = MACHINE(x86ms);
 
 topo_info->dies_per_pkg = ms->smp.dies;
-topo_info->cores_per_die = ms->smp.cores;
+topo_info->modules_per_die = ms->smp.clusters;
+topo_info->cores_per_module = ms->smp.cores;
 topo_info->threads_per_core = ms->smp.threads;
 }
 
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 81573f6cfde0..bbb00dc4aad8 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -54,7 +54,8 @@ typedef struct X86CPUTopoIDs {
 
 typedef struct X86CPUTopoInfo {
 unsigned dies_per_pkg;
-unsigned cores_per_die;
+unsigned modules_per_die;
+unsigned cores_per_module;
 unsigned threads_per_core;
 } X86CPUTopoInfo;
 
@@ -78,7 +79,12 @@ static inline unsigned apicid_smt_width(X86CPUTopoInfo 
*topo_info)
  */
 static inline unsigned apicid_core_width(X86CPUTopoInfo *topo_info)
 {
-return apicid_bitwidth_for_count(topo_info->cores_per_die);
+/*
+ * TODO: Will separate module info from core_width when update
+ * APIC ID with module level.
+ */
+return apicid_bitwidth_for_count(topo_info->cores_per_module *
+ topo_info->modules_per_die);
 }
 
 /* Bit width of the Die_ID field */
@@ -128,7 +134,8 @@ static inline void x86_topo_ids_from_idx(X86CPUTopoInfo 
*topo_info,
  X86CPUTopoIDs *topo_ids)
 {
 unsigned nr_dies = topo_info->dies_per_pkg;
-unsigned nr_cores = topo_info->cores_per_die;
+unsigned nr_cores = topo_info->cores_per_module *
+topo_info->modules_per_die;
 unsigned nr_threads = topo_info->threads_per_core;
 
 topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 61ec9a7499b8..6f3d114c7d12 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -336,7 +336,9 @@ static void encode_cache_cpuid801d(CPUCacheInfo *cache,
 
 /* L3 is shared among multiple cores */
 if (cache->level == 3) {
-l3_threads = topo_info->cores_per_die * topo_info->threads_per_core;
+l3_threads = topo_info->modules_per_die *
+ topo_info->cores_per_module *
+ topo_info->threads_per_core;
 *eax |= (l3_threads - 1) << 14;
 } else {
 *eax |= ((topo_info->threads_per_core - 1) << 14);
@@ -5218,11 +5220,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 uint32_t cpus_per_pkg;
 
 topo_info.dies_per_pkg = env->nr_dies;
-topo_info.cores_per_die = cs->nr_cores / env->nr_dies;
+topo_info.modules_per_die = env->nr_modules;
+topo_info.cores_per_module = cs->nr_cores / env->nr_dies / env->nr_modules;
 topo_info.threads_per_core = cs->nr_threads;
 
-cpus_per_pkg = topo_info.dies_per_pkg * topo_info.cores_per_die *
-   topo_info.threads_per_core;
+cpus_per_pkg = topo_info.dies_per_pkg * topo_info.modules_per_die *
+   topo_info.cores_per_module * topo_info.threads_per_core;
 
 /* Calculate & apply limits for different index ranges */
 if (index >= 0xC000) {
@@ -5298,8 +5301,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 if (*eax & 31) {
 int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14);
 int vcpus_per_socket = cpus_per_pkg;
-int cores_per_socket = topo_info.cores_per_die *
-   topo_info.dies_per_pkg;
+int cores_per_socket = cpus_per_pkg /
+   topo_info.threads_per_core;
 if (cores_per_socket > 1) {
 *eax &= ~0xFC00;
 *eax |= (pow2ceil(cores_per_socket) - 1) << 26;
@@