Re: [PATCH v6 2/4] hw/arm/virt: Consider SMP configuration in CPU topology

[Gavin Shan]

Hi Drew,

On 4/21/22 5:02 PM, Andrew Jones wrote:
> On Wed, Apr 20, 2022 at 10:24:46PM +0800, Gavin Shan wrote:
> ...
> > With amend to the command lines, the following one is used and below error
> > is raised from the test. The error is mentioned in the commit log in
> > PATCH[v7 2/4].
> >
> >      -machine smp.cpus=2                                   \
> >      -numa node,nodeid=0,memdev=ram -numa node,nodeid=1    \
> >      -numa cpu,node-id=1,thread-id=0                       \
> >      -numa cpu,node-id=0,thread-id=1
> >
> >      qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found
> >      (reported from hw/core/machine.c::machine_set_cpu_numa_node())
> >
> > After the changes to virt_possible_cpu_arch_ids() is applied, "thread-id=1"
> > isn't valid any more. The CPU topology becomes like below. Note that
> > mc->smp_props.prefer_sockets is true on arm/virt machine.
>
> prefer_sockets is only true for mach-virt 6.1 and older. It's false for
> 6.2 and later.

Yeah, @perfer_socket is false in last mach-virt-7.0 , which is used in
this test case. So we prefer CPU core over sockets, as explained in
hw/core/machine.c::machine_parse_smp_config(). The CPU topology
becomes like below instead, but 'thread-id=1' is still invalid.

      index    socket   cluster    core    thread
      --------------------------------------------
        0        0        0         0        0
        1        0        0         1        0

> >      index    socket   cluster    core    thread
> >      --------------------------------------------
> >        0        0        0         0        0
> >        1        1        0         0        0
> >
> > With the amended command lines, the topology changes again so
> > that "thread-id=1" is valid:
> >
> >      index    socket   cluster    core    thread
> >      --------------------------------------------
> >        0        0        0         0        0
> >        1        0        0         0        1
> >
> > It should be ok to split the test/qtest/aarch64_numa_cpu() changes into
> > a separate patch and put it before this one. In that case, the specified
> > smp.{socket, cluster, core, threads} isn't used by arm/virt machine yet,
> > and 'thread-id=2' should be still valid. Lets do this if I need post v8.
> > Otherwise, I guess it's also fine to squash the test/qtest/aarch64_numa_cpu()
> > changes into PATCH[2/4], as we're doing.
> >
> > > > > >             "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
> > > > > >             "-numa cpu,node-id=1,thread-id=0 "
> > > > > >             "-numa cpu,node-id=0,thread-id=1");

Thanks,
Gavin