On Tue, 10 Sep 2024 12:01:05 +0100
Salil Mehta <salil.me...@huawei.com> wrote:
> HI Zhao,
A few trivial comments inline.
>
> > From: Zhao Liu <zhao1....@intel.com>
> > Sent: Monday, September 9, 2024 4:28 PM
> > To: Salil Mehta <salil.me...@huawei.com>
> >
> > 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.
>
>
> Agreed. It is same in ARM as well. The difference could be in how hardware
> threads are implemented at microarchitecture level. Nevertheless, we do
> have such virtual configurations, and the meaning of *threads* as-in QOM
> topology (socket,cluster,core,thread) is virtualized similar to the hardware
> threads in host. And One should be able to configure threads support in the
> virtual environment, regardless whether or not underlying hardware
> supports threads. That's my take.
>
> Other aspect is how we then expose these threads to the guest. The guest
> kernel (just like host kernel) should gather topology information using
> ACPI PPTT Table (This is ARM specific?).
Not ARM specific, but not used by x86 in practice (I believe some risc-v boards
use it).
https://lore.kernel.org/linux-riscv/20240617131425.7526-3-cuiyun...@bytedance.com/
> Later is populated by the Qemu
> (just like by firmware for the host kernel) by making use of the virtual
> topology. ARM guest kernel, in absence of PPTT support can detect
> presence of hardware threads by reading MT Bit within the MPIDR_EL1
> register.
Sadly no it can't. Lots of CPUs cores that are single thread set that
bit anyway (so it's garbage and PPTT / DT is the only source of truth)
https://lore.kernel.org/all/caffo_h7vuehqv15epf+_zvrbdhc3jrejkkovshzhgcxnk+n...@mail.gmail.com/T/
Jonathan
