On Wed, Sep 04, 2024 at 05:37:21PM +0000, Salil Mehta wrote:
> Date: Wed, 4 Sep 2024 17:37:21 +0000
> From: Salil Mehta <salil.me...@huawei.com>
> Subject: RE: [PATCH RFC V3 01/29] arm/virt,target/arm: Add new ARMCPU
> {socket,cluster,core,thread}-id property
>
> Hi Zhao,
>
> > From: zhao1....@intel.com <zhao1....@intel.com>
> > Sent: Wednesday, September 4, 2024 3:43 PM
> > To: Salil Mehta <salil.me...@huawei.com>
> >
> > Hi Salil,
> >
> > On Mon, Aug 19, 2024 at 11:53:52AM +0000, Salil Mehta wrote:
> > > Date: Mon, 19 Aug 2024 11:53:52 +0000
> > > From: Salil Mehta <salil.me...@huawei.com>
> > > Subject: RE: [PATCH RFC V3 01/29] arm/virt,target/arm: Add new ARMCPU
> > > {socket,cluster,core,thread}-id property
> >
> > [snip]
> >
> > > > > NULL); @@ -2708,6 +2716,7 @@ static const CPUArchIdList
> > > > *virt_possible_cpu_arch_ids(MachineState *ms)
> > > > > {
> > > > > int n;
> > > > > unsigned int max_cpus = ms->smp.max_cpus;
> > > > > + unsigned int smp_threads = ms->smp.threads;
> > > > > VirtMachineState *vms = VIRT_MACHINE(ms);
> > > > > MachineClass *mc = MACHINE_GET_CLASS(vms);
> > > > >
> > > > > @@ -2721,6 +2730,7 @@ static const CPUArchIdList
> > > > *virt_possible_cpu_arch_ids(MachineState *ms)
> > > > > ms->possible_cpus->len = max_cpus;
> > > > > for (n = 0; n < ms->possible_cpus->len; n++) {
> > > > > ms->possible_cpus->cpus[n].type = ms->cpu_type;
> > > > > + ms->possible_cpus->cpus[n].vcpus_count = smp_threads;
> > > > > ms->possible_cpus->cpus[n].arch_id =
> > > > > virt_cpu_mp_affinity(vms, n);
> > > > >
> > > >
> > > > Why @vcpus_count is initialized to @smp_threads? it needs to be
> > > > documented in the commit log.
> > >
> > >
> > > Because every thread internally amounts to a vCPU in QOM and which is
> > > in 1:1 relationship with KVM vCPU. AFAIK, QOM does not strictly
> > > follows any architecture. Once you start to get into details of
> > > threads there are many aspects of shared resources one will have to
> > > consider and these can vary across different implementations of
> > architecture.
> >
> > For SPAPR CPU, the granularity of >possible_cpus->cpus[] is "core", and for
> > x86, it's "thread" granularity.
>
>
> We have threads per-core at microarchitecture level in ARM as well. But each
> thread appears like a vCPU to OS and AFAICS there are no special attributes
> attached to it. SMT can be enabled/disabled at firmware and should get
> reflected in the configuration accordingly i.e. value of *threads-per-core*
> changes between 1 and 'N'. This means 'vcpus_count' has to reflect the
> correct configuration. But I think threads lack proper representation
> in Qemu QOM.
In topology related part, SMT (of x86) usually represents the logical
processor level. And thread has the same meaning. To change these
meanings is also possible, but I think it should be based on the actual
use case. we can consider the complexity of the implementation when
there is a need.
> In Qemu, each vCPU reflects an execution context (which gets uniquely mapped
> to KVM vCPU). AFAICS, we only have *CPUState* (Struct ArchCPU) as a
> placeholder
> for this execution context and there is no *ThreadState* (derived out of
> Struct CPUState). Hence, we've to map all the threads as QOM vCPUs. This
> means
> the array of present or possible CPUs represented by 'struct CPUArchIdList'
> contains
> all execution contexts which actually might be vCPU or a thread. Hence, usage
> of
> *vcpus_count* seems quite superficial to me frankly.
>
> Also, AFAICS, KVM does not have the concept of the threads and only has
> KVM vCPUs, but you are still allowed to specify the topology with sockets,
> dies,
> clusters, cores, threads in most architectures.
There are some uses for topology, such as it affects scheduling behavior,
and it affects feature emulation, etc.
> > And smp.threads means how many threads in one core, so for x86, the
> > vcpus_count of a "thread" is 1, and for spapr, the vcpus_count of a "core"
> > equals to smp.threads.
>
>
> Sure, but does the KVM specifies this?
At least as you said, KVM (for x86) doesn't consider higher-level topologies
at the moment, but that's not to say that it won't in the future, as certain
registers do have topology dependencies.
> and how does these threads map to the QOM vCPU objects or execution context?
Each CPU object will create a (software) thread, you can refer the
function "kvm_start_vcpu_thread(CPUState *cpu)", which will be called
when CPU object realizes.
> AFAICS there is nothing but 'CPUState'
> which will be made part of the possible vCPU list 'struct CPUArchIdList'.
As I said, an example is spapr ("spapr_possible_cpu_arch_ids()"), which
maps possible_cpu to core object. However, this is a very specific
example, and like Igor's slides said, I understand it's an architectural
requirement.
> >
> > IIUC, your granularity is still "thread", so that this filed should be 1.
>
>
> Well, again we need more discussion on this. I've stated my concerns against
> doing this. User should be allowed to create virtual topology which will
> include 'threads' as one of the parameter.
>
I don't seem to understand...There is a “threads” parameter in -smp, does
this not satisfy your use case?
Regards,
Zhao
