| Message ID | 20220902075531.188916-1-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | s390x: CPU Topology | expand |
I found this version much easier to understand than the previous one. You could consider splitting up the series into two. One that introduces support for STSI, PTF, migration, etc. And a second one that adds support for the maximum-MNist facility and drawers and books. This would also make bisecting a bit nicer because it moves the feature enablement closer to the commits adding the support. Right now, with this series, the topology is static and cannot change. Most of the value of making the topology visible to the guest is for it to mirror reality, and a static topology is a hindrance for that. I'm completely fine with having a static topology as a stepping stone to a dynamic one. However I think we should have a rough plan or maybe even a prototype for how we turn a static into a dynamic topology before we merge this series in order to avoid designing us into a corner. What do you think? On Fri, 2022-09-02 at 09:55 +0200, Pierre Morel wrote: > Hi, > > The implementation of the CPU Topology in QEMU has been drastically > modified since the last patch series and the number of LOCs has been > greatly reduced. > > Unnecessary objects have been removed, only a single S390Topology object > is created to support migration and reset. > > Also a documentation has been added to the series. > > > To use these patches, you will need Linux V6-rc1 or newer. > > Mainline patches needed are: > > f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report > 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function > 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF fac.. > > Currently this code is for KVM only, I have no idea if it is interesting > to provide a TCG patch. If ever it will be done in another series. > > To have a better understanding of the S390x CPU Topology and its > implementation in QEMU you can have a look at the documentation in the > last patch. > > New in this series > ================== > > s390x/cpus: Make absence of multithreading clear > > This patch makes clear that CPU-multithreading is not supported in > the guest. > > s390x/cpu topology: core_id sets s390x CPU topology > > This patch uses the core_id to build the container topology > and the placement of the CPU inside the container. > > s390x/cpu topology: reporting the CPU topology to the guest > > This patch is based on the fact that the CPU type for guests > is always IFL, CPUs are always dedicated and the polarity is > always horizontal. > This may change in the future. > > hw/core: introducing drawer and books for s390x > s390x/cpu: reporting drawers and books topology to the guest > > These two patches extend the topology handling to add two > new containers levels above sockets: books and drawers. > > The subject of the last patches is clear enough (I hope). > > Regards, > Pierre > > Pierre Morel (10): > s390x/cpus: Make absence of multithreading clear > s390x/cpu topology: core_id sets s390x CPU topology > s390x/cpu topology: reporting the CPU topology to the guest > hw/core: introducing drawer and books for s390x > s390x/cpu: reporting drawers and books topology to the guest > s390x/cpu_topology: resetting the Topology-Change-Report > s390x/cpu_topology: CPU topology migration > target/s390x: interception of PTF instruction > s390x/cpu_topology: activating CPU topology > docs/s390x: document s390x cpu topology > > docs/system/s390x/cpu_topology.rst | 88 +++++++++ > hw/core/machine-smp.c | 48 ++++- > hw/core/machine.c | 9 + > hw/s390x/cpu-topology.c | 293 +++++++++++++++++++++++++++++ > hw/s390x/meson.build | 1 + > hw/s390x/s390-virtio-ccw.c | 61 +++++- > include/hw/boards.h | 11 ++ > include/hw/s390x/cpu-topology.h | 53 ++++++ > include/hw/s390x/s390-virtio-ccw.h | 7 + > qapi/machine.json | 14 +- > qemu-options.hx | 6 +- > softmmu/vl.c | 6 + > target/s390x/cpu-sysemu.c | 15 ++ > target/s390x/cpu.h | 51 +++++ > target/s390x/cpu_topology.c | 150 +++++++++++++++ > target/s390x/kvm/kvm.c | 56 +++++- > target/s390x/kvm/kvm_s390x.h | 1 + > target/s390x/meson.build | 1 + > 18 files changed, 858 insertions(+), 13 deletions(-) > create mode 100644 docs/system/s390x/cpu_topology.rst > create mode 100644 hw/s390x/cpu-topology.c > create mode 100644 include/hw/s390x/cpu-topology.h > create mode 100644 target/s390x/cpu_topology.c >
On 9/12/22 16:38, Janis Schoetterl-Glausch wrote: > I found this version much easier to understand than the previous one. > > You could consider splitting up the series into two. > One that introduces support for STSI, PTF, migration, etc. > And a second one that adds support for the maximum-MNist facility and > drawers and books. I agree, sending the first part as supporting only sockets already is an enhancement that could be proposed on its own. > > This would also make bisecting a bit nicer because it moves the feature > enablement closer to the commits adding the support. > > Right now, with this series, the topology is static and cannot change. If we ignore changes on hotplug, yes we consider the topology fixed and that the vCPU does not migrate. > Most of the value of making the topology visible to the guest is for it > to mirror reality, and a static topology is a hindrance for that. > I'm completely fine with having a static topology as a stepping stone > to a dynamic one. > However I think we should have a rough plan or maybe even a prototype > for how we turn a static into a dynamic topology before we merge this > series in order to avoid designing us into a corner. > What do you think? Yes, we can discuss this internally before moving there. Thanks, Pierre
Am 02.09.22 um 09:55 schrieb Pierre Morel: > Hi, > > The implementation of the CPU Topology in QEMU has been drastically > modified since the last patch series and the number of LOCs has been > greatly reduced. > > Unnecessary objects have been removed, only a single S390Topology object > is created to support migration and reset. > > Also a documentation has been added to the series. > > > To use these patches, you will need Linux V6-rc1 or newer. > > Mainline patches needed are: > > f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report > 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function > 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF fac.. > > Currently this code is for KVM only, I have no idea if it is interesting > to provide a TCG patch. If ever it will be done in another series. > > To have a better understanding of the S390x CPU Topology and its > implementation in QEMU you can have a look at the documentation in the > last patch. > > New in this series > ================== > > s390x/cpus: Make absence of multithreading clear > > This patch makes clear that CPU-multithreading is not supported in > the guest. > > s390x/cpu topology: core_id sets s390x CPU topology > > This patch uses the core_id to build the container topology > and the placement of the CPU inside the container. > > s390x/cpu topology: reporting the CPU topology to the guest > > This patch is based on the fact that the CPU type for guests > is always IFL, CPUs are always dedicated and the polarity is > always horizontal. > This may change in the future. > > hw/core: introducing drawer and books for s390x > s390x/cpu: reporting drawers and books topology to the guest > > These two patches extend the topology handling to add two > new containers levels above sockets: books and drawers. > > The subject of the last patches is clear enough (I hope). > > Regards, > Pierre > > Pierre Morel (10): > s390x/cpus: Make absence of multithreading clear > s390x/cpu topology: core_id sets s390x CPU topology > s390x/cpu topology: reporting the CPU topology to the guest > hw/core: introducing drawer and books for s390x > s390x/cpu: reporting drawers and books topology to the guest > s390x/cpu_topology: resetting the Topology-Change-Report > s390x/cpu_topology: CPU topology migration > target/s390x: interception of PTF instruction > s390x/cpu_topology: activating CPU topology Do we really need a machine property? As far as I can see, old QEMU cannot activate the ctop facility with old and new kernel unless it enables CAP_S390_CPU_TOPOLOGY. I do get oldqemu .... -cpu z14,ctop=on qemu-system-s390x: Some features requested in the CPU model are not available in the configuration: ctop With the newer QEMU we can. So maybe we can simply have a topology (and then a cpu model feature) in new QEMUs and non in old. the cpu model would then also fence migration from enabled to disabled.
On 11/16/22 17:51, Christian Borntraeger wrote: > Am 02.09.22 um 09:55 schrieb Pierre Morel: >> Hi, >> >> The implementation of the CPU Topology in QEMU has been drastically >> modified since the last patch series and the number of LOCs has been >> greatly reduced. >> >> Unnecessary objects have been removed, only a single S390Topology object >> is created to support migration and reset. >> >> Also a documentation has been added to the series. >> >> >> To use these patches, you will need Linux V6-rc1 or newer. >> >> Mainline patches needed are: >> >> f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report >> 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function >> 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF >> fac.. >> >> Currently this code is for KVM only, I have no idea if it is interesting >> to provide a TCG patch. If ever it will be done in another series. >> >> To have a better understanding of the S390x CPU Topology and its >> implementation in QEMU you can have a look at the documentation in the >> last patch. >> >> New in this series >> ================== >> >> s390x/cpus: Make absence of multithreading clear >> >> This patch makes clear that CPU-multithreading is not supported in >> the guest. >> >> s390x/cpu topology: core_id sets s390x CPU topology >> >> This patch uses the core_id to build the container topology >> and the placement of the CPU inside the container. >> >> s390x/cpu topology: reporting the CPU topology to the guest >> >> This patch is based on the fact that the CPU type for guests >> is always IFL, CPUs are always dedicated and the polarity is >> always horizontal. >> This may change in the future. >> >> hw/core: introducing drawer and books for s390x >> s390x/cpu: reporting drawers and books topology to the guest >> >> These two patches extend the topology handling to add two >> new containers levels above sockets: books and drawers. >> >> The subject of the last patches is clear enough (I hope). >> >> Regards, >> Pierre >> >> Pierre Morel (10): >> s390x/cpus: Make absence of multithreading clear >> s390x/cpu topology: core_id sets s390x CPU topology >> s390x/cpu topology: reporting the CPU topology to the guest >> hw/core: introducing drawer and books for s390x >> s390x/cpu: reporting drawers and books topology to the guest >> s390x/cpu_topology: resetting the Topology-Change-Report >> s390x/cpu_topology: CPU topology migration >> target/s390x: interception of PTF instruction >> s390x/cpu_topology: activating CPU topology > > > Do we really need a machine property? As far as I can see, old QEMU > cannot activate the ctop facility with old and new kernel unless it > enables CAP_S390_CPU_TOPOLOGY. I do get > oldqemu .... -cpu z14,ctop=on > qemu-system-s390x: Some features requested in the CPU model are not > available in the configuration: ctop > > With the newer QEMU we can. So maybe we can simply have a topology (and > then a cpu model feature) in new QEMUs and non in old. the cpu model > would then also fence migration from enabled to disabled. OK, I can check this. In this case migration with topology will be if I understand correctly: NEW_QEMU/old_machine <-> NEW_QEMU/old_machine OK While OLD_QEMU/old_machine <-> NEW_QEMU/old_machine KO NEW_QEMU/old_machine <-> OLD_QEMU/old_machine KO Is this something we can accept? regards, Pierre
On 11/17/22 10:31, Pierre Morel wrote: > > > On 11/16/22 17:51, Christian Borntraeger wrote: >> Am 02.09.22 um 09:55 schrieb Pierre Morel: >>> Hi, >>> >>> The implementation of the CPU Topology in QEMU has been drastically >>> modified since the last patch series and the number of LOCs has been >>> greatly reduced. >>> >>> Unnecessary objects have been removed, only a single S390Topology object >>> is created to support migration and reset. >>> >>> Also a documentation has been added to the series. >>> >>> >>> To use these patches, you will need Linux V6-rc1 or newer. >>> >>> Mainline patches needed are: >>> >>> f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report >>> 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function >>> 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF >>> fac.. >>> >>> Currently this code is for KVM only, I have no idea if it is interesting >>> to provide a TCG patch. If ever it will be done in another series. >>> >>> To have a better understanding of the S390x CPU Topology and its >>> implementation in QEMU you can have a look at the documentation in the >>> last patch. >>> >>> New in this series >>> ================== >>> >>> s390x/cpus: Make absence of multithreading clear >>> >>> This patch makes clear that CPU-multithreading is not supported in >>> the guest. >>> >>> s390x/cpu topology: core_id sets s390x CPU topology >>> >>> This patch uses the core_id to build the container topology >>> and the placement of the CPU inside the container. >>> >>> s390x/cpu topology: reporting the CPU topology to the guest >>> >>> This patch is based on the fact that the CPU type for guests >>> is always IFL, CPUs are always dedicated and the polarity is >>> always horizontal. >>> This may change in the future. >>> >>> hw/core: introducing drawer and books for s390x >>> s390x/cpu: reporting drawers and books topology to the guest >>> >>> These two patches extend the topology handling to add two >>> new containers levels above sockets: books and drawers. >>> >>> The subject of the last patches is clear enough (I hope). >>> >>> Regards, >>> Pierre >>> >>> Pierre Morel (10): >>> s390x/cpus: Make absence of multithreading clear >>> s390x/cpu topology: core_id sets s390x CPU topology >>> s390x/cpu topology: reporting the CPU topology to the guest >>> hw/core: introducing drawer and books for s390x >>> s390x/cpu: reporting drawers and books topology to the guest >>> s390x/cpu_topology: resetting the Topology-Change-Report >>> s390x/cpu_topology: CPU topology migration >>> target/s390x: interception of PTF instruction >>> s390x/cpu_topology: activating CPU topology >> >> >> Do we really need a machine property? As far as I can see, old QEMU >> cannot activate the ctop facility with old and new kernel unless it >> enables CAP_S390_CPU_TOPOLOGY. I do get >> oldqemu .... -cpu z14,ctop=on >> qemu-system-s390x: Some features requested in the CPU model are not >> available in the configuration: ctop >> >> With the newer QEMU we can. So maybe we can simply have a topology (and >> then a cpu model feature) in new QEMUs and non in old. the cpu model >> would then also fence migration from enabled to disabled. > > OK, I can check this. > In this case migration with topology will be if I understand correctly: > > NEW_QEMU/old_machine <-> NEW_QEMU/old_machine OK > While > OLD_QEMU/old_machine <-> NEW_QEMU/old_machine KO > NEW_QEMU/old_machine <-> OLD_QEMU/old_machine KO I forgot to say that I mean in the examples above without using a flag. Of course using a flag like -ctop=off in NEW_QEMU/new_machine allows to migrate from and to old_machines in an old QEMU. Also I had the same behavior already in V9 by having a VMState without the creation of a machine property, a new cpu feature and a new cpu flag. > > Is this something we can accept? > > regards, > Pierre >
Gentle ping. Did I understand the problem or am I wrong? On 11/17/22 17:38, Pierre Morel wrote: > > > On 11/17/22 10:31, Pierre Morel wrote: >> >> >> On 11/16/22 17:51, Christian Borntraeger wrote: >>> Am 02.09.22 um 09:55 schrieb Pierre Morel: >>>> Hi, >>>> >>>> The implementation of the CPU Topology in QEMU has been drastically >>>> modified since the last patch series and the number of LOCs has been >>>> greatly reduced. >>>> >>>> Unnecessary objects have been removed, only a single S390Topology >>>> object >>>> is created to support migration and reset. >>>> >>>> Also a documentation has been added to the series. >>>> >>>> >>>> To use these patches, you will need Linux V6-rc1 or newer. >>>> >>>> Mainline patches needed are: >>>> >>>> f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report >>>> 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function >>>> 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF >>>> fac.. >>>> >>>> Currently this code is for KVM only, I have no idea if it is >>>> interesting >>>> to provide a TCG patch. If ever it will be done in another series. >>>> >>>> To have a better understanding of the S390x CPU Topology and its >>>> implementation in QEMU you can have a look at the documentation in the >>>> last patch. >>>> >>>> New in this series >>>> ================== >>>> >>>> s390x/cpus: Make absence of multithreading clear >>>> >>>> This patch makes clear that CPU-multithreading is not supported in >>>> the guest. >>>> >>>> s390x/cpu topology: core_id sets s390x CPU topology >>>> >>>> This patch uses the core_id to build the container topology >>>> and the placement of the CPU inside the container. >>>> >>>> s390x/cpu topology: reporting the CPU topology to the guest >>>> >>>> This patch is based on the fact that the CPU type for guests >>>> is always IFL, CPUs are always dedicated and the polarity is >>>> always horizontal. >>>> This may change in the future. >>>> >>>> hw/core: introducing drawer and books for s390x >>>> s390x/cpu: reporting drawers and books topology to the guest >>>> >>>> These two patches extend the topology handling to add two >>>> new containers levels above sockets: books and drawers. >>>> >>>> The subject of the last patches is clear enough (I hope). >>>> >>>> Regards, >>>> Pierre >>>> >>>> Pierre Morel (10): >>>> s390x/cpus: Make absence of multithreading clear >>>> s390x/cpu topology: core_id sets s390x CPU topology >>>> s390x/cpu topology: reporting the CPU topology to the guest >>>> hw/core: introducing drawer and books for s390x >>>> s390x/cpu: reporting drawers and books topology to the guest >>>> s390x/cpu_topology: resetting the Topology-Change-Report >>>> s390x/cpu_topology: CPU topology migration >>>> target/s390x: interception of PTF instruction >>>> s390x/cpu_topology: activating CPU topology >>> >>> >>> Do we really need a machine property? As far as I can see, old QEMU >>> cannot activate the ctop facility with old and new kernel unless it >>> enables CAP_S390_CPU_TOPOLOGY. I do get >>> oldqemu .... -cpu z14,ctop=on >>> qemu-system-s390x: Some features requested in the CPU model are not >>> available in the configuration: ctop >>> >>> With the newer QEMU we can. So maybe we can simply have a topology (and >>> then a cpu model feature) in new QEMUs and non in old. the cpu model >>> would then also fence migration from enabled to disabled. >> >> OK, I can check this. >> In this case migration with topology will be if I understand correctly: >> >> NEW_QEMU/old_machine <-> NEW_QEMU/old_machine OK >> While >> OLD_QEMU/old_machine <-> NEW_QEMU/old_machine KO >> NEW_QEMU/old_machine <-> OLD_QEMU/old_machine KO > > I forgot to say that I mean in the examples above without using a flag. > > Of course using a flag like -ctop=off in NEW_QEMU/new_machine allows > to migrate from and to old_machines in an old QEMU. > > Also I had the same behavior already in V9 by having a VMState without > the creation of a machine property, a new cpu feature and a new cpu flag. > > > > > > >> >> Is this something we can accept? >> >> regards, >> Pierre >> >
On 11/24/22 10:25, Pierre Morel wrote: > > Gentle ping. > > Did I understand the problem or am I wrong? > I guess I was wrong, so I send a new series next week. Regards, Pierre > > On 11/17/22 17:38, Pierre Morel wrote: >> >> >> On 11/17/22 10:31, Pierre Morel wrote: >>> >>> >>> On 11/16/22 17:51, Christian Borntraeger wrote: >>>> Am 02.09.22 um 09:55 schrieb Pierre Morel: >>>>> Hi, >>>>> >>>>> The implementation of the CPU Topology in QEMU has been drastically >>>>> modified since the last patch series and the number of LOCs has been >>>>> greatly reduced. >>>>> >>>>> Unnecessary objects have been removed, only a single S390Topology >>>>> object >>>>> is created to support migration and reset. >>>>> >>>>> Also a documentation has been added to the series. >>>>> >>>>> >>>>> To use these patches, you will need Linux V6-rc1 or newer. >>>>> >>>>> Mainline patches needed are: >>>>> >>>>> f5ecfee94493 2022-07-20 KVM: s390: resetting the >>>>> Topology-Change-Report >>>>> 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function >>>>> 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and >>>>> SIIF fac.. >>>>> >>>>> Currently this code is for KVM only, I have no idea if it is >>>>> interesting >>>>> to provide a TCG patch. If ever it will be done in another series. >>>>> >>>>> To have a better understanding of the S390x CPU Topology and its >>>>> implementation in QEMU you can have a look at the documentation in the >>>>> last patch. >>>>> >>>>> New in this series >>>>> ================== >>>>> >>>>> s390x/cpus: Make absence of multithreading clear >>>>> >>>>> This patch makes clear that CPU-multithreading is not supported in >>>>> the guest. >>>>> >>>>> s390x/cpu topology: core_id sets s390x CPU topology >>>>> >>>>> This patch uses the core_id to build the container topology >>>>> and the placement of the CPU inside the container. >>>>> >>>>> s390x/cpu topology: reporting the CPU topology to the guest >>>>> >>>>> This patch is based on the fact that the CPU type for guests >>>>> is always IFL, CPUs are always dedicated and the polarity is >>>>> always horizontal. >>>>> This may change in the future. >>>>> >>>>> hw/core: introducing drawer and books for s390x >>>>> s390x/cpu: reporting drawers and books topology to the guest >>>>> >>>>> These two patches extend the topology handling to add two >>>>> new containers levels above sockets: books and drawers. >>>>> >>>>> The subject of the last patches is clear enough (I hope). >>>>> >>>>> Regards, >>>>> Pierre >>>>> >>>>> Pierre Morel (10): >>>>> s390x/cpus: Make absence of multithreading clear >>>>> s390x/cpu topology: core_id sets s390x CPU topology >>>>> s390x/cpu topology: reporting the CPU topology to the guest >>>>> hw/core: introducing drawer and books for s390x >>>>> s390x/cpu: reporting drawers and books topology to the guest >>>>> s390x/cpu_topology: resetting the Topology-Change-Report >>>>> s390x/cpu_topology: CPU topology migration >>>>> target/s390x: interception of PTF instruction >>>>> s390x/cpu_topology: activating CPU topology >>>> >>>> >>>> Do we really need a machine property? As far as I can see, old QEMU >>>> cannot activate the ctop facility with old and new kernel unless it >>>> enables CAP_S390_CPU_TOPOLOGY. I do get >>>> oldqemu .... -cpu z14,ctop=on >>>> qemu-system-s390x: Some features requested in the CPU model are not >>>> available in the configuration: ctop >>>> >>>> With the newer QEMU we can. So maybe we can simply have a topology (and >>>> then a cpu model feature) in new QEMUs and non in old. the cpu model >>>> would then also fence migration from enabled to disabled. >>> >>> OK, I can check this. >>> In this case migration with topology will be if I understand correctly: >>> >>> NEW_QEMU/old_machine <-> NEW_QEMU/old_machine OK >>> While >>> OLD_QEMU/old_machine <-> NEW_QEMU/old_machine KO >>> NEW_QEMU/old_machine <-> OLD_QEMU/old_machine KO >> >> I forgot to say that I mean in the examples above without using a flag. >> >> Of course using a flag like -ctop=off in NEW_QEMU/new_machine allows >> to migrate from and to old_machines in an old QEMU. >> >> Also I had the same behavior already in V9 by having a VMState without >> the creation of a machine property, a new cpu feature and a new cpu flag. >> >> >> >> >> >> >>> >>> Is this something we can accept? >>> >>> regards, >>> Pierre >>> >> >