Re: [RFC PATCH 0/4] cpu: Drop CPUState::nr_cores

[Igor Mammedov]

On Thu,  5 Dec 2024 09:57:12 -0500
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> The series is motivated by auditing the usage of CPUState::nr_cores and
> CPUState::nr_threads, which is motivated by [1].
> 
> The initial goal is to initialize nr_threads and nr_cores earlier for
> x86, which leads to patches [2] and [3]. Patch [2] touches all the ARCHes
> and patch [3] looks hacky. At last Igor suggested to audit nr_threads and
> nr_cores, and only set them in the pre_plug() callback for the ARCHes that
> really need them[1].
> 
> By audting nr_threads and nr_cores, I found nr_cores is only used by
> x86. So we can introduce a x86 specific one and initialize in
> x86_cpu_pre_plug(), then drop nr_cores totally.
> 
> However for nr_threads, it's used by MIPS and PPC as well[4]. There are
> two options:
> 1. maintain separate substitute in X86/MIPS/PPS, so we can drop
> CPUState::nr_threads like for CPUState::nr_cores.
> 
> 2. keep CPUState::nr_threads and find place (or introduce pre_plug()) to
> initialize them earlier for MIPS/PPC.
> 
> I would like to seek for opinions for which one is prefered. 
> 
> This series implments the drop for CPUState::nr_cores. Though it doesn't
> help on the initial goal without the solution for nr_threads, I think it
> is still a good cleanup?
> 
> BTW, by initializing nr_threads and nr_cores earlier than
> qemu_init_vcpu(), it also unblocks [5].

With minor fixes included mentioned during review

Acked-by: Igor Mammedov <imammedo@redhat.com>

> 
> 
> [1] 
> https://lore.kernel.org/qemu-devel/20241125103857.78a23715@imammedo.users.ipa.redhat.com/
> [2] 
> https://lore.kernel.org/qemu-devel/5f8db586-cdda-4d00-be02-f9880a20e1a3@redhat.com/
> [3] 
> https://lore.kernel.org/qemu-devel/20241122160317.4070177-1-xiaoyao.li@intel.com/
> [4] 
> 045f9cb1-2b17-4b2c-985f-3c34e3626b36@intel.com/">https://lore.kernel.org/qemu-devel/045f9cb1-2b17-4b2c-985f-3c34e3626b36@intel.com/
> [5] 
> https://lore.kernel.org/qemu-devel/20231128171239.69b6d7b1@imammedo.users.ipa.redhat.com/
> 
> Xiaoyao Li (4):
>   i386/topology: Update the comment of x86_apicid_from_topo_ids()
>   i386: Extract a common fucntion to setup value of
>     MSR_CORE_THREAD_COUNT
>   i386: Track cores_per_module in CPUX86State
>   cpu: Remove nr_cores from struct CPUState
> 
>  hw/core/cpu-common.c                 |  1 -
>  hw/i386/x86-common.c                 |  4 +++-
>  include/hw/core/cpu.h                |  2 --
>  include/hw/i386/topology.h           |  5 +++--
>  system/cpus.c                        |  1 -
>  target/i386/cpu.c                    |  2 +-
>  target/i386/cpu.h                    | 16 ++++++++++++++++
>  target/i386/hvf/x86_emu.c            |  3 +--
>  target/i386/kvm/kvm.c                |  5 +----
>  target/i386/tcg/sysemu/misc_helper.c |  3 +--
>  10 files changed, 26 insertions(+), 16 deletions(-)
>