Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support

[Ying Fang]

On 10/16/2020 6:07 PM, Andrew Jones wrote:
> On Fri, Oct 16, 2020 at 05:40:02PM +0800, Ying Fang wrote:
> > On 10/15/2020 3:59 PM, Andrew Jones wrote:
> > > On Thu, Oct 15, 2020 at 10:07:16AM +0800, Ying Fang wrote:
> > > > On 10/14/2020 2:08 AM, Andrew Jones wrote:
> > > > > On Tue, Oct 13, 2020 at 12:11:20PM +0000, Zengtao (B) wrote:
> > > > > > Cc valentin
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Qemu-devel
> > > > > > > [mailto:qemu-devel-bounces+prime.zeng=hisilicon.com@nongnu.org]
> > > > > > > On Behalf Of Ying Fang
> > > > > > > Sent: Thursday, September 17, 2020 11:20 AM
> > > > > > > To: qemu-devel@nongnu.org
> > > > > > > Cc: peter.maydell@linaro.org; drjones@redhat.com; Zhanghailiang;
> > > > > > > Chenzhendong (alex); shannon.zhaosl@gmail.com;
> > > > > > > qemu-arm@nongnu.org; alistair.francis@wdc.com; fangying;
> > > > > > > imammedo@redhat.com
> > > > > > > Subject: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache
> > > > > > > topology support
> > > > > > >
> > > > > > > An accurate cpu topology may help improve the cpu scheduler's
> > > > > > > decision
> > > > > > > making when dealing with multi-core system. So cpu topology
> > > > > > > description
> > > > > > > is helpful to provide guest with the right view. Cpu cache information
> > > > > > > may
> > > > > > > also have slight impact on the sched domain, and even userspace
> > > > > > > software
> > > > > > > may check the cpu cache information to do some optimizations. Thus
> > > > > > > this patch
> > > > > > > series is posted to provide cpu and cache topology support for arm.
> > > > > > >
> > > > > > > To make the cpu topology consistent with MPIDR, an vcpu ioctl
> > > > > >
> > > > > > For aarch64, the cpu topology don't depends on the MPDIR.
> > > > > > See https://patchwork.kernel.org/patch/11744387/
> > > > >
> > > > > The topology should not be inferred from the MPIDR Aff fields,
> > > >
> > > > MPIDR is abused by ARM OEM manufactures. It is only used as a
> > > > identifer for a specific cpu, not representation of the topology.
> > >
> > > Right, which is why I stated topology should not be inferred from
> > > it.
> > >
> > > > > but MPIDR is the CPU identifier. When describing a topology
> > > > > with ACPI or DT the CPU elements in the topology description
> > > > > must map to actual CPUs. MPIDR is that mapping link. KVM
> > > > > currently determines what the MPIDR of a VCPU is. If KVM
> > > >
> > > > KVM currently assigns MPIDR with vcpu->vcpu_id which mapped
> > > > into affinity levels. See reset_mpidr in sys_regs.c
> > >
> > > I know, but how KVM assigns MPIDRs today is not really important
> > > to KVM userspace. KVM userspace shouldn't depend on a KVM
> > > algorithm, as it could change.
> > >
> > > > > userspace is going to determine the VCPU topology, then it
> > > > > also needs control over the MPIDR values, otherwise it
> > > > > becomes quite messy trying to get the mapping right.
> > > > If we are going to control MPIDR, shall we assign MPIDR with
> > > > vcpu_id or map topology hierarchy into affinity levels or any
> > > > other link schema ?
> > >
> > > We can assign them to whatever we want, as long as they're
> > > unique and as long as Aff0 is assigned per the GIC requirements,
> > > e.g. GICv3 requires that Aff0 be from 0 to 0xf. Also, when
> > > pinning VCPUs to PCPUs we should ensure that MPIDRs with matching
> > > Aff3,Aff2,Aff1 fields should actually be peers with respect to
> > > the GIC.
> >
> > Still not clear why vCPU's MPIDR need to match pPCPU's GIC affinity.
> > Maybe I should read spec for GICv3.
>
> Look at how IPIs are efficiently sent to "peers", where the definition
> of a peer is that only Aff0 differs in its MPIDR. But, gicv3's
> optimizations can only handle 16 peers. If we want pinned VCPUs to
> have the same performance as PCPUS, then we should maintain this
> Aff0 limit.

Yes I see. I think *virt_cpu_mp_affinity* in qemu has limit
on the clustersz. It groups every 16 vCPUs into a cluster
and then mapped into the first two affinity levels.

Thanks.
Ying.

> Thanks,
> drew
>
> > > We shouldn't try to encode topology in the MPIDR in any way,
> > > so we might as well simply increment a counter to assign them,
> > > which could possibly be the same as the VCPU ID.
> >
> > Hmm, then we can leave it as it is.
> >
> > > Thanks,
> > > drew
> > >
> > > .
>
> .