accel/tcg/user-exec-stub.c | 4 +++ hw/core/cpu-common.c | 2 +- include/hw/core/cpu.h | 8 +++++ system/cpus.c | 6 +++- target/alpha/cpu.c | 2 ++ target/arm/cpu.c | 2 ++ target/avr/cpu.c | 2 ++ target/hexagon/cpu.c | 2 ++ target/hppa/cpu.c | 2 ++ target/i386/cpu.c | 61 +++++++++++++++++++------------------- target/loongarch/cpu.c | 2 ++ target/m68k/cpu.c | 2 ++ target/microblaze/cpu.c | 2 ++ target/mips/cpu.c | 2 ++ target/openrisc/cpu.c | 2 ++ target/ppc/cpu_init.c | 2 ++ target/riscv/cpu.c | 2 ++ target/rx/cpu.c | 2 ++ target/s390x/cpu.c | 2 ++ target/sh4/cpu.c | 2 ++ target/sparc/cpu.c | 2 ++ target/tricore/cpu.c | 2 ++ target/xtensa/cpu.c | 2 ++ 23 files changed, 85 insertions(+), 32 deletions(-)
This series is extracted from TDX QEMU v6[1] series per Paolo's request.
It is originally motivated by x86 TDX to track CPUID_HT in env->features[]
which requires nr_cores and nr_cores being initialized earlier than in
qemu_init_vcpu().
Initialize of nr_cores and nr_threads earlier in x86's cpu_realizefn()
can make it work for x86 but it's duplicated with the initialization in
qemu_init_vcpu() which are used by all the ARCHes. Since initialize them
earlier also work for other ARCHes, introduce qemu_init_early_vcpu() to
hold the initialization of nr_cores and nr_threads and call it at the
beginning in realizefn() for each ARCH.
Note, I only tested it for x86 ARCH. Please help test on other ARCHes.
The following patch 2 - 4 are x86 specific.
[1] https://lore.kernel.org/qemu-devel/CABgObfZVxaQL4FSJX396kAJ67Qp=XhEWwcmv+NQZCbdpfbV9xg@mail.gmail.com/
Xiaoyao Li (4):
cpu: Introduce qemu_early_init_vcpu() to initialize nr_cores and
nr_threads inside it
i386/cpu: Set up CPUID_HT in x86_cpu_expand_features() instead of
cpu_x86_cpuid()
i386/cpu: Set and track CPUID_EXT3_CMP_LEG in
env->features[FEAT_8000_0001_ECX]
i386/cpu: Rectify the comment on order dependency on qemu_init_vcpu()
accel/tcg/user-exec-stub.c | 4 +++
hw/core/cpu-common.c | 2 +-
include/hw/core/cpu.h | 8 +++++
system/cpus.c | 6 +++-
target/alpha/cpu.c | 2 ++
target/arm/cpu.c | 2 ++
target/avr/cpu.c | 2 ++
target/hexagon/cpu.c | 2 ++
target/hppa/cpu.c | 2 ++
target/i386/cpu.c | 61 +++++++++++++++++++-------------------
target/loongarch/cpu.c | 2 ++
target/m68k/cpu.c | 2 ++
target/microblaze/cpu.c | 2 ++
target/mips/cpu.c | 2 ++
target/openrisc/cpu.c | 2 ++
target/ppc/cpu_init.c | 2 ++
target/riscv/cpu.c | 2 ++
target/rx/cpu.c | 2 ++
target/s390x/cpu.c | 2 ++
target/sh4/cpu.c | 2 ++
target/sparc/cpu.c | 2 ++
target/tricore/cpu.c | 2 ++
target/xtensa/cpu.c | 2 ++
23 files changed, 85 insertions(+), 32 deletions(-)
--
2.34.1
On 08.11.24 08:06, Xiaoyao Li wrote: > This series is extracted from TDX QEMU v6[1] series per Paolo's request. > > It is originally motivated by x86 TDX to track CPUID_HT in env->features[] > which requires nr_cores and nr_cores being initialized earlier than in "and nr_threads" > qemu_init_vcpu(). > > Initialize of nr_cores and nr_threads earlier in x86's cpu_realizefn() > can make it work for x86 but it's duplicated with the initialization in > qemu_init_vcpu() which are used by all the ARCHes. Since initialize them > earlier also work for other ARCHes, introduce qemu_init_early_vcpu() to > hold the initialization of nr_cores and nr_threads and call it at the > beginning in realizefn() for each ARCH. > > Note, I only tested it for x86 ARCH. Please help test on other ARCHes. [...] > accel/tcg/user-exec-stub.c | 4 +++ > hw/core/cpu-common.c | 2 +- > include/hw/core/cpu.h | 8 +++++ > system/cpus.c | 6 +++- > target/alpha/cpu.c | 2 ++ > target/arm/cpu.c | 2 ++ > target/avr/cpu.c | 2 ++ > target/hexagon/cpu.c | 2 ++ > target/hppa/cpu.c | 2 ++ > target/i386/cpu.c | 61 +++++++++++++++++++------------------- > target/loongarch/cpu.c | 2 ++ > target/m68k/cpu.c | 2 ++ > target/microblaze/cpu.c | 2 ++ > target/mips/cpu.c | 2 ++ > target/openrisc/cpu.c | 2 ++ > target/ppc/cpu_init.c | 2 ++ > target/riscv/cpu.c | 2 ++ > target/rx/cpu.c | 2 ++ > target/s390x/cpu.c | 2 ++ > target/sh4/cpu.c | 2 ++ > target/sparc/cpu.c | 2 ++ > target/tricore/cpu.c | 2 ++ > target/xtensa/cpu.c | 2 ++ > 23 files changed, 85 insertions(+), 32 deletions(-) Hm. It looks like this belongs into the parent realize function. But the "bad thing" is that we call the parent realize function after we try realizing the derived CPU. Could it go into cpu_common_initfn()? -- Cheers, David / dhildenb
On 11/11/2024 6:49 PM, David Hildenbrand wrote: > On 08.11.24 08:06, Xiaoyao Li wrote: >> This series is extracted from TDX QEMU v6[1] series per Paolo's request. >> >> It is originally motivated by x86 TDX to track CPUID_HT in env- >> >features[] >> which requires nr_cores and nr_cores being initialized earlier than in > > "and nr_threads" > >> qemu_init_vcpu(). >> >> Initialize of nr_cores and nr_threads earlier in x86's cpu_realizefn() >> can make it work for x86 but it's duplicated with the initialization in >> qemu_init_vcpu() which are used by all the ARCHes. Since initialize them >> earlier also work for other ARCHes, introduce qemu_init_early_vcpu() to >> hold the initialization of nr_cores and nr_threads and call it at the >> beginning in realizefn() for each ARCH. >> >> Note, I only tested it for x86 ARCH. Please help test on other ARCHes. > > [...] > >> accel/tcg/user-exec-stub.c | 4 +++ >> hw/core/cpu-common.c | 2 +- >> include/hw/core/cpu.h | 8 +++++ >> system/cpus.c | 6 +++- >> target/alpha/cpu.c | 2 ++ >> target/arm/cpu.c | 2 ++ >> target/avr/cpu.c | 2 ++ >> target/hexagon/cpu.c | 2 ++ >> target/hppa/cpu.c | 2 ++ >> target/i386/cpu.c | 61 +++++++++++++++++++------------------- >> target/loongarch/cpu.c | 2 ++ >> target/m68k/cpu.c | 2 ++ >> target/microblaze/cpu.c | 2 ++ >> target/mips/cpu.c | 2 ++ >> target/openrisc/cpu.c | 2 ++ >> target/ppc/cpu_init.c | 2 ++ >> target/riscv/cpu.c | 2 ++ >> target/rx/cpu.c | 2 ++ >> target/s390x/cpu.c | 2 ++ >> target/sh4/cpu.c | 2 ++ >> target/sparc/cpu.c | 2 ++ >> target/tricore/cpu.c | 2 ++ >> target/xtensa/cpu.c | 2 ++ >> 23 files changed, 85 insertions(+), 32 deletions(-) > > Hm. It looks like this belongs into the parent realize function. But the > "bad thing" is that we call the parent realize function after we try > realizing the derived CPU. > > Could it go into cpu_common_initfn()? > It can, I think. I'll move them into cpu_common_initfn() in v2 to avoid touching all the ARCHes.
On 11/21/24 17:24, Xiaoyao Li wrote: >> Could it go into cpu_common_initfn()? > > It can, I think. > > I'll move them into cpu_common_initfn() in v2 to avoid touching all the > ARCHes. It does look better than the alternative of duplicating code. On the other hand qemu_init_vcpu is already duplicated and I'm not sure I like relying on qdev_get_machine() inside the instance_init function... Paolo
On 11/22/2024 2:52 AM, Paolo Bonzini wrote: > On 11/21/24 17:24, Xiaoyao Li wrote: >>> Could it go into cpu_common_initfn()? >> >> It can, I think. >> >> I'll move them into cpu_common_initfn() in v2 to avoid touching all >> the ARCHes. > > It does look better than the alternative of duplicating code. > > On the other hand qemu_init_vcpu is already duplicated and I'm not sure > I like relying on qdev_get_machine() inside the instance_init function... I had the same concern. But it seems all the ARCHes should create MACHINE before VCPUs. So it seems OK to qdev_get_machine() inside the instance_init function. But I'm not sure if there is any case to create VCPU standalone. I think we can check if qdev_get_machine() gets a valid result. If not, fall back to assign nr_cores and nr_threads to 1. Anyway, please let me know your preference, this series or a v2 to implement it inside cpu_common_initfn(). > Paolo >
On 22.11.24 03:40, Xiaoyao Li wrote: > On 11/22/2024 2:52 AM, Paolo Bonzini wrote: >> On 11/21/24 17:24, Xiaoyao Li wrote: >>>> Could it go into cpu_common_initfn()? >>> >>> It can, I think. >>> >>> I'll move them into cpu_common_initfn() in v2 to avoid touching all >>> the ARCHes. >> >> It does look better than the alternative of duplicating code. >> >> On the other hand qemu_init_vcpu is already duplicated and I'm not sure >> I like relying on qdev_get_machine() inside the instance_init function... > Good point. > I had the same concern. > > But it seems all the ARCHes should create MACHINE before VCPUs. So it > seems OK to qdev_get_machine() inside the instance_init function. But > I'm not sure if there is any case to create VCPU standalone. There are, for example on s390x in create_cpu_model_list(). I recall there are ways to start QEMU without any machine and trigger that code. (or maybe this was just for the test environment with a special test machine ...) > > I think we can check if qdev_get_machine() gets a valid result. If not, > fall back to assign nr_cores and nr_threads to 1. That sounds reasonable to me. -- Cheers, David / dhildenb
Il ven 22 nov 2024, 10:44 David Hildenbrand <david@redhat.com> ha scritto: > > I think we can check if qdev_get_machine() gets a valid result. If not, > > fall back to assign nr_cores and nr_threads to 1. > > That sounds reasonable to me. > Another possibility is to add a cpu_realize() function that sets two properties and then calls qdev_realize(). It touches all architectures but not in the sense of adding new code, just changing it. And it is more code to write though, so if Xiaoyao prefers that I apply v1 I am okay with that. Paolo > Cheers, > > David / dhildenb > >
On 21/11/24 17:24, Xiaoyao Li wrote: > On 11/11/2024 6:49 PM, David Hildenbrand wrote: >> On 08.11.24 08:06, Xiaoyao Li wrote: >>> This series is extracted from TDX QEMU v6[1] series per Paolo's request. >>> >>> It is originally motivated by x86 TDX to track CPUID_HT in env- >>> >features[] >>> which requires nr_cores and nr_cores being initialized earlier than in >> >> "and nr_threads" >> >>> qemu_init_vcpu(). >>> >>> Initialize of nr_cores and nr_threads earlier in x86's cpu_realizefn() >>> can make it work for x86 but it's duplicated with the initialization in >>> qemu_init_vcpu() which are used by all the ARCHes. Since initialize them >>> earlier also work for other ARCHes, introduce qemu_init_early_vcpu() to >>> hold the initialization of nr_cores and nr_threads and call it at the >>> beginning in realizefn() for each ARCH. >>> >>> Note, I only tested it for x86 ARCH. Please help test on other ARCHes. >> >> [...] >> >>> accel/tcg/user-exec-stub.c | 4 +++ >>> hw/core/cpu-common.c | 2 +- >>> include/hw/core/cpu.h | 8 +++++ >>> system/cpus.c | 6 +++- >>> target/alpha/cpu.c | 2 ++ >>> target/arm/cpu.c | 2 ++ >>> target/avr/cpu.c | 2 ++ >>> target/hexagon/cpu.c | 2 ++ >>> target/hppa/cpu.c | 2 ++ >>> target/i386/cpu.c | 61 +++++++++++++++++++------------------- >>> target/loongarch/cpu.c | 2 ++ >>> target/m68k/cpu.c | 2 ++ >>> target/microblaze/cpu.c | 2 ++ >>> target/mips/cpu.c | 2 ++ >>> target/openrisc/cpu.c | 2 ++ >>> target/ppc/cpu_init.c | 2 ++ >>> target/riscv/cpu.c | 2 ++ >>> target/rx/cpu.c | 2 ++ >>> target/s390x/cpu.c | 2 ++ >>> target/sh4/cpu.c | 2 ++ >>> target/sparc/cpu.c | 2 ++ >>> target/tricore/cpu.c | 2 ++ >>> target/xtensa/cpu.c | 2 ++ >>> 23 files changed, 85 insertions(+), 32 deletions(-) >> >> Hm. It looks like this belongs into the parent realize function. But >> the "bad thing" is that we call the parent realize function after we >> try realizing the derived CPU. >> >> Could it go into cpu_common_initfn()? >> > > It can, I think. > > I'll move them into cpu_common_initfn() in v2 to avoid touching all the > ARCHes. This seems a x86 issue, it also bugs me: https://lore.kernel.org/qemu-devel/20231128171239.69b6d7b1@imammedo.users.ipa.redhat.com/ IMO we need to make vCPU creation steps more explicit.
© 2016 - 2024 Red Hat, Inc.