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

Ying Fang posted 12 patches 3 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
device_tree.c                |  24 +++++++
hw/acpi/aml-build.c          |  68 +++++++++++++++++++
hw/arm/virt-acpi-build.c     |  99 +++++++++++++++++++++++++--
hw/arm/virt.c                | 128 ++++++++++++++++++++++++++++++++++-
include/hw/acpi/acpi-defs.h  |  14 ++++
include/hw/acpi/aml-build.h  |  11 +++
include/hw/arm/virt.h        |   1 +
include/sysemu/device_tree.h |   1 +
linux-headers/linux/kvm.h    |   3 +
target/arm/cpu.c             |  42 ++++++++++++
target/arm/cpu.h             |  27 ++++++++
target/arm/kvm32.c           |  46 ++++++++++---
target/arm/kvm64.c           |  46 ++++++++++---
13 files changed, 488 insertions(+), 22 deletions(-)
[RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Ying Fang 3 years, 7 months ago
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
KVM_ARM_SET_MP_AFFINITY is introduced so that userspace can set MPIDR
according to the topology specified [1]. To describe the cpu topology
both fdt and ACPI are supported. To describe the cpu cache information,
a default cache hierarchy is given and can be made configurable later.
The cpu topology is built according to processor hierarchy node structure.
The cpu cache information is built according to cache type structure.

This patch series is partially based on the patches posted by Andrew Jone
years ago [2], I jumped in on it since some OS vendor cooperative partners
are eager for it. Thanks for Andrew's contribution. Please feel free to reply
to me if there is anything improper.

[1] https://patchwork.kernel.org/cover/11781317
[2] https://patchwork.ozlabs.org/project/qemu-devel/cover/20180704124923.32483-1-drjones@redhat.com

Andrew Jones (2):
  device_tree: add qemu_fdt_add_path
  hw/arm/virt: DT: add cpu-map

Ying Fang (10):
  linux headers: Update linux header with KVM_ARM_SET_MP_AFFINITY
  target/arm/kvm64: make MPIDR consistent with CPU Topology
  target/arm/kvm32: make MPIDR consistent with CPU Topology
  hw/arm/virt-acpi-build: distinguish possible and present cpus
  hw/acpi/aml-build: add processor hierarchy node structure
  hw/arm/virt-acpi-build: add PPTT table
  target/arm/cpu: Add CPU cache description for arm
  hw/arm/virt: add fdt cache information
  hw/acpi/aml-build: build ACPI CPU cache topology information
  hw/arm/virt-acpi-build: Enable CPU cache topology

 device_tree.c                |  24 +++++++
 hw/acpi/aml-build.c          |  68 +++++++++++++++++++
 hw/arm/virt-acpi-build.c     |  99 +++++++++++++++++++++++++--
 hw/arm/virt.c                | 128 ++++++++++++++++++++++++++++++++++-
 include/hw/acpi/acpi-defs.h  |  14 ++++
 include/hw/acpi/aml-build.h  |  11 +++
 include/hw/arm/virt.h        |   1 +
 include/sysemu/device_tree.h |   1 +
 linux-headers/linux/kvm.h    |   3 +
 target/arm/cpu.c             |  42 ++++++++++++
 target/arm/cpu.h             |  27 ++++++++
 target/arm/kvm32.c           |  46 ++++++++++---
 target/arm/kvm64.c           |  46 ++++++++++---
 13 files changed, 488 insertions(+), 22 deletions(-)

-- 
2.23.0


RE: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Zengtao (B) 3 years, 6 months ago
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/ 

> KVM_ARM_SET_MP_AFFINITY is introduced so that userspace can set
> MPIDR
> according to the topology specified [1]. To describe the cpu topology
> both fdt and ACPI are supported. To describe the cpu cache
> information,
> a default cache hierarchy is given and can be made configurable later.
> The cpu topology is built according to processor hierarchy node
> structure.
> The cpu cache information is built according to cache type structure.
> 
> This patch series is partially based on the patches posted by Andrew
> Jone
> years ago [2], I jumped in on it since some OS vendor cooperative
> partners
> are eager for it. Thanks for Andrew's contribution. Please feel free to
> reply
> to me if there is anything improper.
> 
> [1] https://patchwork.kernel.org/cover/11781317
> [2]
> https://patchwork.ozlabs.org/project/qemu-devel/cover/2018070412
> 4923.32483-1-drjones@redhat.com
> 
> Andrew Jones (2):
>   device_tree: add qemu_fdt_add_path
>   hw/arm/virt: DT: add cpu-map
> 
> Ying Fang (10):
>   linux headers: Update linux header with
> KVM_ARM_SET_MP_AFFINITY
>   target/arm/kvm64: make MPIDR consistent with CPU Topology
>   target/arm/kvm32: make MPIDR consistent with CPU Topology
>   hw/arm/virt-acpi-build: distinguish possible and present cpus
>   hw/acpi/aml-build: add processor hierarchy node structure
>   hw/arm/virt-acpi-build: add PPTT table
>   target/arm/cpu: Add CPU cache description for arm
>   hw/arm/virt: add fdt cache information
>   hw/acpi/aml-build: build ACPI CPU cache topology information
>   hw/arm/virt-acpi-build: Enable CPU cache topology
> 
>  device_tree.c                |  24 +++++++
>  hw/acpi/aml-build.c          |  68 +++++++++++++++++++
>  hw/arm/virt-acpi-build.c     |  99
> +++++++++++++++++++++++++--
>  hw/arm/virt.c                | 128
> ++++++++++++++++++++++++++++++++++-
>  include/hw/acpi/acpi-defs.h  |  14 ++++
>  include/hw/acpi/aml-build.h  |  11 +++
>  include/hw/arm/virt.h        |   1 +
>  include/sysemu/device_tree.h |   1 +
>  linux-headers/linux/kvm.h    |   3 +
>  target/arm/cpu.c             |  42 ++++++++++++
>  target/arm/cpu.h             |  27 ++++++++
>  target/arm/kvm32.c           |  46 ++++++++++---
>  target/arm/kvm64.c           |  46 ++++++++++---
>  13 files changed, 488 insertions(+), 22 deletions(-)
> 
> --
> 2.23.0
> 


Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Andrew Jones 3 years, 6 months ago
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,
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
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.

Thanks,
drew


Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Ying Fang 3 years, 6 months ago

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.

> 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

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

> 
> Thanks,
> drew
> 
> .
> 
Thanks Ying.

Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Andrew Jones 3 years, 6 months ago
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.

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.

Thanks,
drew


Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Ying Fang 3 years, 6 months ago

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.

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

Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Andrew Jones 3 years, 6 months ago
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.

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


Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Ying Fang 3 years, 6 months ago

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

Re: [RFC PATCH 00/12] hw/arm/virt: Introduce cpu and cache topology support
Posted by Andrew Jones 3 years, 6 months ago
On Tue, Oct 20, 2020 at 10:52:11AM +0800, Ying Fang wrote:
> 
> 
> 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.
>

Right, and it's probably sufficient to just switch to this function
for assigning affinity fields of MPIDRs for both TCG and KVM. Currently
it's only for TCG, as the comment in it explains, but it does the same
thing as KVM anyway. So, while nothing should change from the view of
the guest, userspace gains control over the MPIDRs, and that allows it
to build VCPU topologies more smoothly and in advance of VCPU creation.

Thanks,
drew