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

[Ying Fang <fangying1@huawei.com>]

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
>>>
>>> .
>>>
>>
> 
> .
>