Re: [PATCH] Fix CPUID_Fn8000001E_EBX for AMD

[Eduardo Habkost]

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)
> 

-- 
Eduardo