Re: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT

[Xiaoyao Li]

On 1/3/2025 4:52 PM, Philippe Mathieu-Daudé wrote:
> On 28/12/24 18:37, Philippe Mathieu-Daudé wrote:
> > On 19/12/24 12:01, 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>
> > > ---
> > > Changes in v2:
> > > - move the implementation of cpu_x86_get_msr_core_thread_count() to
> > >    target/i386/cpu-sysemu.c;
> > > ---
> > >   target/i386/cpu-sysemu.c             | 11 +++++++++++
> > >   target/i386/cpu.h                    |  2 ++
> > >   target/i386/hvf/x86_emu.c            |  3 +--
> > >   target/i386/kvm/kvm.c                |  5 +----
> > >   target/i386/tcg/sysemu/misc_helper.c |  3 +--
> > >   5 files changed, 16 insertions(+), 8 deletions(-)
> >
> >
> > > +uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)
> > > +{
> > > +    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 */
> > > +
> > > +    return val;
> >
> > Alternatively:
> >
> >         return deposit64(cs->nr_threads * cs->nr_cores, 16, 16,
> >                          cs->nr_cores);

I would rather keep it as-is as suguested by Philippe[1].

(deposit64() is really a new thing for i386)

[1] 
https://lore.kernel.org/qemu-devel/20241210173524.48e203a3@imammedo.users.ipa.redhat.com/

> > > +}
>
> Typo "function" in patch subject.

thanks for catching it!