Re: [RFC PATCH 2/4] i386: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT

[Philippe Mathieu-Daudé]

Hi Xiaoyao,

On 5/12/24 15:57, Xiaoyao Li wrote:
> There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
> Extract a common function for it.
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   target/i386/cpu.h                    | 11 +++++++++++
>   target/i386/hvf/x86_emu.c            |  3 +--
>   target/i386/kvm/kvm.c                |  5 +----
>   target/i386/tcg/sysemu/misc_helper.c |  3 +--
>   4 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 4c239a6970fd..5795a497e567 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2390,6 +2390,17 @@ static inline void cpu_x86_load_seg_cache_sipi(X86CPU 
> *cpu,
>       cs->halted = 0;
>   }
>   
> +static inline uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)

Please do not add more inlined functions in this huge file, the
inlining performance isn't justified at all.

target/i386/cpu-sysemu.c looks the correct place for this helper.

> +{
> +    CPUState *cs = CPU(cpu);
> +    uint64_t val;
> +
> +    val = cs->nr_threads * cs->nr_cores;  /* thread count, bits 15..0 */
> +    val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 31..16 */

Personally I'd change to:

       return deposit64(cs->nr_threads * cs->nr_cores, 16, 16,
                        cs->nr_cores);

> +
> +    return val;
> +}