Re: [PATCH v1 0/4] Initialize nr_cores and nr_threads early and related clearup

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.