diff mbox series

Fix CPUID_Fn8000001E_EBX for AMD

Message ID 20210630082551.12956-1-chengjiayao@bytedance.com (mailing list archive)
State New, archived
Headers show
Series Fix CPUID_Fn8000001E_EBX for AMD | expand

Commit Message

成家瑶 June 30, 2021, 8:25 a.m. UTC
According to AMD64 Arch Programmer's Manual Appendix D,
bits 7:0 in Fn8000_001E_EBX should be physical core(s) per logical processor, not per die.

Signed-off-by: Jade Cheng <chengjiayao@bytedance.com>
---
 target/i386/cpu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

成家瑶 July 15, 2021, 6:15 a.m. UTC | #1
Hello,

I am Jade Cheng working for ByteDance, sending this email is aimed at ping
you guys to check the patch I submitted earlier, link is attached below:
https://patchew.org/QEMU/20210630082551.12956-1-chengjiayao@bytedance.com/

Please do me a favor to give it a review, and let me know if you have any
concerns.
Stay safe and have a nice day = )
Best,
Jiayao (Jade) Cheng
On Wed, Jun 30, 2021, 16:26 <chengjiayao@bytedance.com> wrote:
According to AMD64 Arch Programmer's Manual Appendix D, bits 7:0 in
Fn8000_001E_EBX should be physical core(s) per logical processor, not per
die. Signed-off-by: Jade Cheng <chengjiayao@bytedance.com> ---
target/i386/cpu.c | 8 +++++++- 1 file changed, 7 insertions(+), 1
deletion(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
a9fe1662d3..417f5ba81f 100644 --- a/target/i386/cpu.c +++
b/target/i386/cpu.c @@ -381,7 +381,13 @@ static void
encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info, * NOTE:
CoreId is already part of apic_id. Just use it. We can * use all the 8 bits
to represent the core_id here. */ - *ebx = ((topo_info->threads_per_core -
1) << 8) | (topo_ids.core_id & 0xFF); + uint32_t core_id =
topo_ids.core_id; + + if (IS_AMD_CPU(&cpu->env)) { + core_id =
topo_ids.core_id + topo_ids.die_id * topo_info->cores_per_die; + } + + *ebx
= ((topo_info->threads_per_core - 1) << 8) | (core_id & 0xFF); /* *
CPUID_Fn8000001E_ECX [Node Identifiers] (NodeId) -- 2.24.3 (Apple Git-128)
Eduardo Habkost July 26, 2021, 9 p.m. UTC | #2
CCing the original author of that code (Babu Moger).

On Wed, Jun 30, 2021 at 04:25:51PM +0800, Jade Cheng wrote:
> According to AMD64 Arch Programmer's Manual Appendix D,
> bits 7:0 in Fn8000_001E_EBX should be physical core(s) per logical processor, not per die.

Do you mean physical cores per package/socket?

> 
> Signed-off-by: Jade Cheng <chengjiayao@bytedance.com>

Do you have a pointer to the specific paragraph of the
documentation that states that?

https://www.amd.com/system/files/TechDocs/24594.pdf
page 634 says:

  CPUID Fn8000_001E_EBX Compute Unit Identifiers
  [...]
  7:0 ComputeUnitId Compute unit ID. Identifies a Compute Unit,
                    which may be one or more physical cores that each implement
                    one or more logical processors

I don't see any content referencing physical cores per logical
processor, or physical cores per package/socket.

Which problem are you trying to fix here?


> ---
>  target/i386/cpu.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a9fe1662d3..417f5ba81f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -381,7 +381,13 @@ static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
>       *  NOTE: CoreId is already part of apic_id. Just use it. We can
>       *  use all the 8 bits to represent the core_id here.
>       */
> -    *ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.core_id & 0xFF);
> +    uint32_t core_id = topo_ids.core_id;
> +
> +    if (IS_AMD_CPU(&cpu->env)) {
> +        core_id = topo_ids.core_id + topo_ids.die_id * topo_info->cores_per_die;
> +    }
> +
> +    *ebx = ((topo_info->threads_per_core - 1) << 8) | (core_id & 0xFF);
>  
>      /*
>       * CPUID_Fn8000001E_ECX [Node Identifiers] (NodeId)
> -- 
> 2.24.3 (Apple Git-128)
>
diff mbox series

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a9fe1662d3..417f5ba81f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -381,7 +381,13 @@  static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
      *  NOTE: CoreId is already part of apic_id. Just use it. We can
      *  use all the 8 bits to represent the core_id here.
      */
-    *ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.core_id & 0xFF);
+    uint32_t core_id = topo_ids.core_id;
+
+    if (IS_AMD_CPU(&cpu->env)) {
+        core_id = topo_ids.core_id + topo_ids.die_id * topo_info->cores_per_die;
+    }
+
+    *ebx = ((topo_info->threads_per_core - 1) << 8) | (core_id & 0xFF);
 
     /*
      * CPUID_Fn8000001E_ECX [Node Identifiers] (NodeId)