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(-)
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].
[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] 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(-)
--
2.34.1
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] 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(-) >
On 12/31/2024 12:11 AM, Igor Mammedov wrote: > 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> Appreciated for your Ack, Igor! There is a v2[*], could you take a look at that? [*] https://lore.kernel.org/qemu-devel/20241219110125.1266461-1-xiaoyao.li@intel.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] 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(-) >> >
© 2016 - 2025 Red Hat, Inc.