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

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 12/5/2024 3:30 PM, Zhao Liu wrote:
> I'm also very sorry, but I have a slightly different opinion...
> 
>>   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(-)
>>
> 
> I have some doubts about the necessity of changing the initialization of
> nr_cores/nr_threads, because you can access the machine's topology info
> via machine_topo_get_threads_per_socket(), which gives the same result as
> `nr_cores * nr_threads`.

yeah, it works. The goal is to maintain HT in env->features[].

The problem is, as Igor objects, accessing MachineState from CPU 
context. This is what I'm working on to avoid for the next version.

> Especially, the TDX feature check hook is also within the context of
> `current_machine`, so why not check if TDX's HT is consistent with QEMU's
> emulation in the TDX hook?
> 
> For this reason, and based on my comment on patch 2, I think checking HT
> in the TDX hook or even ignoring HT, would be a more straightforward and
> less impactful solution.

Though it's motivated by TDX series, the goal is not TDX specific. The 
goal is to track features in env->features[] instead of manually 
generating a one-off value in cpu_x86_cpuid().

> -Zhao
>