| Message ID | 20230105145313.168489-2-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: CPU Topology | expand |
On 05/01/2023 15.53, Pierre Morel wrote: > S390 adds two new SMP levels, drawers and books to the CPU > topology. > The S390 CPU have specific toplogy features like dedication > and polarity to give to the guest indications on the host > vCPUs scheduling and help the guest take the best decisions > on the scheduling of threads on the vCPUs. > > Let us provide the SMP properties with books and drawers levels > and S390 CPU with dedication and polarity, > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- ... > diff --git a/qapi/machine.json b/qapi/machine.json > index b9228a5e46..ff8f2b0e84 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -900,13 +900,15 @@ > # a CPU is being hotplugged. > # > # @node-id: NUMA node ID the CPU belongs to > -# @socket-id: socket number within node/board the CPU belongs to > +# @drawer-id: drawer number within node/board the CPU belongs to > +# @book-id: book number within drawer/node/board the CPU belongs to > +# @socket-id: socket number within book/node/board the CPU belongs to I think the new entries need a "(since 8.0)" comment (similar to die-id and cluster-id below). Other question: Do we have "node-id"s on s390x? If not, is that similar to books or drawers, i.e. just another word? If so, we should maybe rather re-use "nodes" instead of introducing a new name for the same thing? > # @die-id: die number within socket the CPU belongs to (since 4.1) > # @cluster-id: cluster number within die the CPU belongs to (since 7.1) > # @core-id: core number within cluster the CPU belongs to > # @thread-id: thread number within core the CPU belongs to > # > -# Note: currently there are 6 properties that could be present > +# Note: currently there are 8 properties that could be present > # but management should be prepared to pass through other > # properties with device_add command to allow for future > # interface extension. This also requires the filed names to be kept in > @@ -916,6 +918,8 @@ > ## > { 'struct': 'CpuInstanceProperties', > 'data': { '*node-id': 'int', > + '*drawer-id': 'int', > + '*book-id': 'int', > '*socket-id': 'int', > '*die-id': 'int', > '*cluster-id': 'int', > @@ -1465,6 +1469,10 @@ > # > # @cpus: number of virtual CPUs in the virtual machine > # > +# @drawers: number of drawers in the CPU topology > +# > +# @books: number of books in the CPU topology > +# These also need a "(since 8.0)" comment at the end. > # @sockets: number of sockets in the CPU topology > # > # @dies: number of dies per socket in the CPU topology > @@ -1481,6 +1489,8 @@ > ## > { 'struct': 'SMPConfiguration', 'data': { > '*cpus': 'int', > + '*drawers': 'int', > + '*books': 'int', > '*sockets': 'int', > '*dies': 'int', > '*clusters': 'int', ... > diff --git a/qemu-options.hx b/qemu-options.hx > index 7f99d15b23..8dc9a4c052 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -250,11 +250,13 @@ SRST > ERST > > DEF("smp", HAS_ARG, QEMU_OPTION_smp, > - "-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n" > + "-smp [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n" This line now got too long. Please add a newline inbetween. > " set the number of initial CPUs to 'n' [default=1]\n" > " maxcpus= maximum number of total CPUs, including\n" > " offline CPUs for hotplug, etc\n" > - " sockets= number of sockets on the machine board\n" > + " drawers= number of drawers on the machine board\n" > + " books= number of books in one drawer\n" > + " sockets= number of sockets in one book\n" > " dies= number of dies in one socket\n" > " clusters= number of clusters in one die\n" > " cores= number of cores in one cluster\n" Thomas
On Thu, 2023-01-05 at 15:53 +0100, Pierre Morel wrote: > S390 adds two new SMP levels, drawers and books to the CPU > topology. > The S390 CPU have specific toplogy features like dedication > and polarity to give to the guest indications on the host > vCPUs scheduling and help the guest take the best decisions > on the scheduling of threads on the vCPUs. > > Let us provide the SMP properties with books and drawers levels > and S390 CPU with dedication and polarity, > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > qapi/machine.json | 14 ++++++++-- > include/hw/boards.h | 10 ++++++- > include/hw/s390x/cpu-topology.h | 23 ++++++++++++++++ > target/s390x/cpu.h | 6 +++++ > hw/core/machine-smp.c | 48 ++++++++++++++++++++++++++++----- > hw/core/machine.c | 4 +++ > hw/s390x/s390-virtio-ccw.c | 2 ++ > softmmu/vl.c | 6 +++++ > target/s390x/cpu.c | 10 +++++++ > qemu-options.hx | 6 +++-- > 10 files changed, 117 insertions(+), 12 deletions(-) > create mode 100644 include/hw/s390x/cpu-topology.h > [...] > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 7d6d01325b..39ea63a416 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -131,6 +131,12 @@ struct CPUArchState { > > #if !defined(CONFIG_USER_ONLY) > uint32_t core_id; /* PoP "CPU address", same as cpu_index */ > + int32_t socket_id; > + int32_t book_id; > + int32_t drawer_id; > + int32_t dedicated; > + int32_t polarity; If I understood the architecture correctly, the polarity is a property of the configuration, not the cpus. So this should be vertical_entitlement, and there should be a machine (?) property specifying if the polarity is horizontal or vertical. > + int32_t cpu_type; > uint64_t cpuid; > #endif > [...]
On 1/10/23 12:37, Thomas Huth wrote: > On 05/01/2023 15.53, Pierre Morel wrote: >> S390 adds two new SMP levels, drawers and books to the CPU >> topology. >> The S390 CPU have specific toplogy features like dedication >> and polarity to give to the guest indications on the host >> vCPUs scheduling and help the guest take the best decisions >> on the scheduling of threads on the vCPUs. >> >> Let us provide the SMP properties with books and drawers levels >> and S390 CPU with dedication and polarity, >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- > ... >> diff --git a/qapi/machine.json b/qapi/machine.json >> index b9228a5e46..ff8f2b0e84 100644 >> --- a/qapi/machine.json >> +++ b/qapi/machine.json >> @@ -900,13 +900,15 @@ >> # a CPU is being hotplugged. >> # >> # @node-id: NUMA node ID the CPU belongs to >> -# @socket-id: socket number within node/board the CPU belongs to >> +# @drawer-id: drawer number within node/board the CPU belongs to >> +# @book-id: book number within drawer/node/board the CPU belongs to >> +# @socket-id: socket number within book/node/board the CPU belongs to > > I think the new entries need a "(since 8.0)" comment (similar to die-id > and cluster-id below). right > > Other question: Do we have "node-id"s on s390x? If not, is that similar > to books or drawers, i.e. just another word? If so, we should maybe > rather re-use "nodes" instead of introducing a new name for the same thing? We have theoretically nodes-id on s390x, it is the level 5 of the topology, above drawers. Currently it is not used in s390x topology, the maximum level returned to a LPAR host is 4. I suppose that it adds a possibility to link several s390x with a fast network. > >> # @die-id: die number within socket the CPU belongs to (since 4.1) >> # @cluster-id: cluster number within die the CPU belongs to (since 7.1) >> # @core-id: core number within cluster the CPU belongs to >> # @thread-id: thread number within core the CPU belongs to >> # >> -# Note: currently there are 6 properties that could be present >> +# Note: currently there are 8 properties that could be present >> # but management should be prepared to pass through other >> # properties with device_add command to allow for future >> # interface extension. This also requires the filed names to >> be kept in >> @@ -916,6 +918,8 @@ >> ## >> { 'struct': 'CpuInstanceProperties', >> 'data': { '*node-id': 'int', >> + '*drawer-id': 'int', >> + '*book-id': 'int', >> '*socket-id': 'int', >> '*die-id': 'int', >> '*cluster-id': 'int', >> @@ -1465,6 +1469,10 @@ >> # >> # @cpus: number of virtual CPUs in the virtual machine >> # >> +# @drawers: number of drawers in the CPU topology >> +# >> +# @books: number of books in the CPU topology >> +# > > These also need a "(since 8.0)" comment at the end. right again, I will add this. > >> # @sockets: number of sockets in the CPU topology >> # >> # @dies: number of dies per socket in the CPU topology >> @@ -1481,6 +1489,8 @@ >> ## >> { 'struct': 'SMPConfiguration', 'data': { >> '*cpus': 'int', >> + '*drawers': 'int', >> + '*books': 'int', >> '*sockets': 'int', >> '*dies': 'int', >> '*clusters': 'int', > ... >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 7f99d15b23..8dc9a4c052 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -250,11 +250,13 @@ SRST >> ERST >> DEF("smp", HAS_ARG, QEMU_OPTION_smp, >> - "-smp >> [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n" >> + "-smp >> [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n" > > This line now got too long. Please add a newline inbetween. OK Thanks. Regards, Pierre
On 1/13/23 17:58, Nina Schoetterl-Glausch wrote: > On Thu, 2023-01-05 at 15:53 +0100, Pierre Morel wrote: >> S390 adds two new SMP levels, drawers and books to the CPU >> topology. >> The S390 CPU have specific toplogy features like dedication >> and polarity to give to the guest indications on the host >> vCPUs scheduling and help the guest take the best decisions >> on the scheduling of threads on the vCPUs. >> >> Let us provide the SMP properties with books and drawers levels >> and S390 CPU with dedication and polarity, >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> qapi/machine.json | 14 ++++++++-- >> include/hw/boards.h | 10 ++++++- >> include/hw/s390x/cpu-topology.h | 23 ++++++++++++++++ >> target/s390x/cpu.h | 6 +++++ >> hw/core/machine-smp.c | 48 ++++++++++++++++++++++++++++----- >> hw/core/machine.c | 4 +++ >> hw/s390x/s390-virtio-ccw.c | 2 ++ >> softmmu/vl.c | 6 +++++ >> target/s390x/cpu.c | 10 +++++++ >> qemu-options.hx | 6 +++-- >> 10 files changed, 117 insertions(+), 12 deletions(-) >> create mode 100644 include/hw/s390x/cpu-topology.h >> > [...] > >> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >> index 7d6d01325b..39ea63a416 100644 >> --- a/target/s390x/cpu.h >> +++ b/target/s390x/cpu.h >> @@ -131,6 +131,12 @@ struct CPUArchState { >> >> #if !defined(CONFIG_USER_ONLY) >> uint32_t core_id; /* PoP "CPU address", same as cpu_index */ >> + int32_t socket_id; >> + int32_t book_id; >> + int32_t drawer_id; >> + int32_t dedicated; >> + int32_t polarity; > > If I understood the architecture correctly, the polarity is a property of the configuration, > not the cpus. So this should be vertical_entitlement, and there should be a machine (?) property > specifying if the polarity is horizontal or vertical. You are right, considering PTF only, the documentation says PTF([01]) does the following: "... a process is initiated to place all CPUs in the configuration into the polarization specified by the function code, ..." So on one side the polarization property is explicitly set on the CPU, and on the other side all CPU are supposed to be in the same polarization state. So yes we can make the horizontal/vertical a machine property. However, we do not need to set this tunable as the documentation says that the machine always start with horizontal polarization. On the other hand the documentation mixes a lot vertical with different entitlement and horizontal polarization, for TLE order and slacks so I prefer to keep the complete description of the polarization as CPU properties in case we miss something. PTF([01]) are no performance bottle neck and the number of CPU is likely to be small, even a maximum of 248 is possible KVM warns above 16 CPU so the loop for setting all CPU inside PTF interception is not very problematic I think. Doing like you say should simplify PTF interception (no loop) but complicates (some more if/else) TLE handling and QMP information display on CPU. So I will have a look at the implications and answer again on this. Thanks, Regards, Pierre
On Mon, 2023-01-16 at 18:28 +0100, Pierre Morel wrote: > > On 1/13/23 17:58, Nina Schoetterl-Glausch wrote: > > On Thu, 2023-01-05 at 15:53 +0100, Pierre Morel wrote: > > > S390 adds two new SMP levels, drawers and books to the CPU > > > topology. > > > The S390 CPU have specific toplogy features like dedication > > > and polarity to give to the guest indications on the host > > > vCPUs scheduling and help the guest take the best decisions > > > on the scheduling of threads on the vCPUs. > > > > > > Let us provide the SMP properties with books and drawers levels > > > and S390 CPU with dedication and polarity, > > > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > > --- > > > qapi/machine.json | 14 ++++++++-- > > > include/hw/boards.h | 10 ++++++- > > > include/hw/s390x/cpu-topology.h | 23 ++++++++++++++++ > > > target/s390x/cpu.h | 6 +++++ > > > hw/core/machine-smp.c | 48 ++++++++++++++++++++++++++++----- > > > hw/core/machine.c | 4 +++ > > > hw/s390x/s390-virtio-ccw.c | 2 ++ > > > softmmu/vl.c | 6 +++++ > > > target/s390x/cpu.c | 10 +++++++ > > > qemu-options.hx | 6 +++-- > > > 10 files changed, 117 insertions(+), 12 deletions(-) > > > create mode 100644 include/hw/s390x/cpu-topology.h > > > > > [...] > > > > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > > > index 7d6d01325b..39ea63a416 100644 > > > --- a/target/s390x/cpu.h > > > +++ b/target/s390x/cpu.h > > > @@ -131,6 +131,12 @@ struct CPUArchState { > > > > > > #if !defined(CONFIG_USER_ONLY) > > > uint32_t core_id; /* PoP "CPU address", same as cpu_index */ > > > + int32_t socket_id; > > > + int32_t book_id; > > > + int32_t drawer_id; > > > + int32_t dedicated; > > > + int32_t polarity; > > > > If I understood the architecture correctly, the polarity is a property of the configuration, > > not the cpus. So this should be vertical_entitlement, and there should be a machine (?) property > > specifying if the polarity is horizontal or vertical. > > You are right, considering PTF only, the documentation says PTF([01]) > does the following: > > "... a process is initiated to place all CPUs in the configuration into > the polarization specified by the function code, ..." > > So on one side the polarization property is explicitly set on the CPU, > and on the other side all CPU are supposed to be in the same > polarization state. I'm worried about STSI showing both horizontal and vertical CPUs at the same time. I don't know if this is allowed. If it is not, you need a way to switch between those atomically, which is harder if every CPU has this property. > > So yes we can make the horizontal/vertical a machine property. > However, we do not need to set this tunable as the documentation says > that the machine always start with horizontal polarization. > > On the other hand the documentation mixes a lot vertical with different > entitlement and horizontal polarization, for TLE order and slacks so I > prefer to keep the complete description of the polarization as CPU > properties in case we miss something. > > PTF([01]) are no performance bottle neck and the number of CPU is likely > to be small, even a maximum of 248 is possible KVM warns above 16 CPU so > the loop for setting all CPU inside PTF interception is not very > problematic I think. Yeah, I'm not worried about that. > > Doing like you say should simplify PTF interception (no loop) but > complicates (some more if/else) TLE handling and QMP information display > on CPU. > So I will have a look at the implications and answer again on this. > > Thanks, > > Regards, > Pierre >
On 16/01/2023 18.28, Pierre Morel wrote: > > > On 1/13/23 17:58, Nina Schoetterl-Glausch wrote: >> On Thu, 2023-01-05 at 15:53 +0100, Pierre Morel wrote: >>> S390 adds two new SMP levels, drawers and books to the CPU >>> topology. >>> The S390 CPU have specific toplogy features like dedication >>> and polarity to give to the guest indications on the host >>> vCPUs scheduling and help the guest take the best decisions >>> on the scheduling of threads on the vCPUs. >>> >>> Let us provide the SMP properties with books and drawers levels >>> and S390 CPU with dedication and polarity, >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> --- ... > PTF([01]) are no performance bottle neck and the number of CPU is likely to > be small, even a maximum of 248 is possible KVM warns above 16 CPU so the > loop for setting all CPU inside PTF interception is not very problematic I > think. KVM warns if you try to use more than the number of physical CPUs that you have, not at hard-coded 16 CPUs. So if you've got an LPAR with 248 CPUs, it's perfectly fine to use also 248 CPUs for your guest. Thomas
On 16/01/2023 17.32, Pierre Morel wrote: > > On 1/10/23 12:37, Thomas Huth wrote: ... >> Other question: Do we have "node-id"s on s390x? If not, is that similar to >> books or drawers, i.e. just another word? If so, we should maybe rather >> re-use "nodes" instead of introducing a new name for the same thing? > > We have theoretically nodes-id on s390x, it is the level 5 of the topology, > above drawers. > Currently it is not used in s390x topology, the maximum level returned to a > LPAR host is 4. > I suppose that it adds a possibility to link several s390x with a fast network. Ok, thanks. So if nodes are indeed a concept on top of the other layers, and we do not really support these on s390x yet, should we maybe forbid them on s390x like we recently did with the "threads" in the recent machine types? ... to avoid that users try to use them with the wrong expectations? Thomas
On 1/16/23 21:34, Nina Schoetterl-Glausch wrote: > On Mon, 2023-01-16 at 18:28 +0100, Pierre Morel wrote: >> >> On 1/13/23 17:58, Nina Schoetterl-Glausch wrote: >>> On Thu, 2023-01-05 at 15:53 +0100, Pierre Morel wrote: >>>> S390 adds two new SMP levels, drawers and books to the CPU >>>> topology. >>>> The S390 CPU have specific toplogy features like dedication >>>> and polarity to give to the guest indications on the host >>>> vCPUs scheduling and help the guest take the best decisions >>>> on the scheduling of threads on the vCPUs. >>>> >>>> Let us provide the SMP properties with books and drawers levels >>>> and S390 CPU with dedication and polarity, >>>> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> --- >>>> qapi/machine.json | 14 ++++++++-- >>>> include/hw/boards.h | 10 ++++++- >>>> include/hw/s390x/cpu-topology.h | 23 ++++++++++++++++ >>>> target/s390x/cpu.h | 6 +++++ >>>> hw/core/machine-smp.c | 48 ++++++++++++++++++++++++++++----- >>>> hw/core/machine.c | 4 +++ >>>> hw/s390x/s390-virtio-ccw.c | 2 ++ >>>> softmmu/vl.c | 6 +++++ >>>> target/s390x/cpu.c | 10 +++++++ >>>> qemu-options.hx | 6 +++-- >>>> 10 files changed, 117 insertions(+), 12 deletions(-) >>>> create mode 100644 include/hw/s390x/cpu-topology.h >>>> >>> [...] >>> >>>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >>>> index 7d6d01325b..39ea63a416 100644 >>>> --- a/target/s390x/cpu.h >>>> +++ b/target/s390x/cpu.h >>>> @@ -131,6 +131,12 @@ struct CPUArchState { >>>> >>>> #if !defined(CONFIG_USER_ONLY) >>>> uint32_t core_id; /* PoP "CPU address", same as cpu_index */ >>>> + int32_t socket_id; >>>> + int32_t book_id; >>>> + int32_t drawer_id; >>>> + int32_t dedicated; >>>> + int32_t polarity; >>> >>> If I understood the architecture correctly, the polarity is a property of the configuration, >>> not the cpus. So this should be vertical_entitlement, and there should be a machine (?) property >>> specifying if the polarity is horizontal or vertical. >> >> You are right, considering PTF only, the documentation says PTF([01]) >> does the following: >> >> "... a process is initiated to place all CPUs in the configuration into >> the polarization specified by the function code, ..." >> >> So on one side the polarization property is explicitly set on the CPU, >> and on the other side all CPU are supposed to be in the same >> polarization state. > > I'm worried about STSI showing both horizontal and vertical CPUs at the same time. > I don't know if this is allowed. > If it is not, you need a way to switch between those atomically, which is harder > if every CPU has this property. The documentation explicitly provides the order in which to show the TLE when both vertical and horizontal TLE are display inside the SYSIB. So it seems expected by the architecture. I understand that it is because the documentation says that the instruction finished before all the CPU did change polarity. But I think you are right on the point that: - It is only transitional - it does not make sense for my point of view to have both at the same time. In QEMU, as these are two interceptions we will always finish the PTF interception before we start the STSI interception, so we will never see this happen. Question are: 1) Should we do better than the architecture? 2) If yes, are we sure it is better? Regards, Pierre
diff --git a/qapi/machine.json b/qapi/machine.json index b9228a5e46..ff8f2b0e84 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -900,13 +900,15 @@ # a CPU is being hotplugged. # # @node-id: NUMA node ID the CPU belongs to -# @socket-id: socket number within node/board the CPU belongs to +# @drawer-id: drawer number within node/board the CPU belongs to +# @book-id: book number within drawer/node/board the CPU belongs to +# @socket-id: socket number within book/node/board the CPU belongs to # @die-id: die number within socket the CPU belongs to (since 4.1) # @cluster-id: cluster number within die the CPU belongs to (since 7.1) # @core-id: core number within cluster the CPU belongs to # @thread-id: thread number within core the CPU belongs to # -# Note: currently there are 6 properties that could be present +# Note: currently there are 8 properties that could be present # but management should be prepared to pass through other # properties with device_add command to allow for future # interface extension. This also requires the filed names to be kept in @@ -916,6 +918,8 @@ ## { 'struct': 'CpuInstanceProperties', 'data': { '*node-id': 'int', + '*drawer-id': 'int', + '*book-id': 'int', '*socket-id': 'int', '*die-id': 'int', '*cluster-id': 'int', @@ -1465,6 +1469,10 @@ # # @cpus: number of virtual CPUs in the virtual machine # +# @drawers: number of drawers in the CPU topology +# +# @books: number of books in the CPU topology +# # @sockets: number of sockets in the CPU topology # # @dies: number of dies per socket in the CPU topology @@ -1481,6 +1489,8 @@ ## { 'struct': 'SMPConfiguration', 'data': { '*cpus': 'int', + '*drawers': 'int', + '*books': 'int', '*sockets': 'int', '*dies': 'int', '*clusters': 'int', diff --git a/include/hw/boards.h b/include/hw/boards.h index d18d6d0073..239fc017b0 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -130,11 +130,15 @@ typedef struct { * @prefer_sockets - whether sockets are preferred over cores in smp parsing * @dies_supported - whether dies are supported by the machine * @clusters_supported - whether clusters are supported by the machine + * @books_supported - whether books are supported by the machine + * @drawers_supported - whether drawers are supported by the machine */ typedef struct { bool prefer_sockets; bool dies_supported; bool clusters_supported; + bool books_supported; + bool drawers_supported; } SMPCompatProps; /** @@ -299,7 +303,9 @@ typedef struct DeviceMemoryState { /** * CpuTopology: * @cpus: the number of present logical processors on the machine - * @sockets: the number of sockets on the machine + * @drawers: the number of drawers on the machine + * @books: the number of books in one drawer + * @sockets: the number of sockets in one book * @dies: the number of dies in one socket * @clusters: the number of clusters in one die * @cores: the number of cores in one cluster @@ -308,6 +314,8 @@ typedef struct DeviceMemoryState { */ typedef struct CpuTopology { unsigned int cpus; + unsigned int drawers; + unsigned int books; unsigned int sockets; unsigned int dies; unsigned int clusters; diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h new file mode 100644 index 0000000000..d945b57fc3 --- /dev/null +++ b/include/hw/s390x/cpu-topology.h @@ -0,0 +1,23 @@ +/* + * CPU Topology + * + * Copyright IBM Corp. 2022 + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at + * your option) any later version. See the COPYING file in the top-level + * directory. + */ +#ifndef HW_S390X_CPU_TOPOLOGY_H +#define HW_S390X_CPU_TOPOLOGY_H + +#define S390_TOPOLOGY_CPU_IFL 0x03 + +#define S390_TOPOLOGY_POLARITY_HORIZONTAL 0x00 +#define S390_TOPOLOGY_POLARITY_VERTICAL_LOW 0x01 +#define S390_TOPOLOGY_POLARITY_VERTICAL_MEDIUM 0x02 +#define S390_TOPOLOGY_POLARITY_VERTICAL_HIGH 0x03 + +#define S390_TOPOLOGY_SHARED 0x00 +#define S390_TOPOLOGY_DEDICATED 0x01 + +#endif diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 7d6d01325b..39ea63a416 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -131,6 +131,12 @@ struct CPUArchState { #if !defined(CONFIG_USER_ONLY) uint32_t core_id; /* PoP "CPU address", same as cpu_index */ + int32_t socket_id; + int32_t book_id; + int32_t drawer_id; + int32_t dedicated; + int32_t polarity; + int32_t cpu_type; uint64_t cpuid; #endif diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index b39ed21e65..edabfdb67e 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -31,6 +31,14 @@ static char *cpu_hierarchy_to_string(MachineState *ms) MachineClass *mc = MACHINE_GET_CLASS(ms); GString *s = g_string_new(NULL); + if (mc->smp_props.drawers_supported) { + g_string_append_printf(s, " * drawers (%u)", ms->smp.drawers); + } + + if (mc->smp_props.books_supported) { + g_string_append_printf(s, " * books (%u)", ms->smp.books); + } + g_string_append_printf(s, "sockets (%u)", ms->smp.sockets); if (mc->smp_props.dies_supported) { @@ -73,6 +81,8 @@ void machine_parse_smp_config(MachineState *ms, { MachineClass *mc = MACHINE_GET_CLASS(ms); unsigned cpus = config->has_cpus ? config->cpus : 0; + unsigned drawers = config->has_drawers ? config->drawers : 0; + unsigned books = config->has_books ? config->books : 0; unsigned sockets = config->has_sockets ? config->sockets : 0; unsigned dies = config->has_dies ? config->dies : 0; unsigned clusters = config->has_clusters ? config->clusters : 0; @@ -85,6 +95,8 @@ void machine_parse_smp_config(MachineState *ms, * explicit configuration like "cpus=0" is not allowed. */ if ((config->has_cpus && config->cpus == 0) || + (config->has_drawers && config->drawers == 0) || + (config->has_books && config->books == 0) || (config->has_sockets && config->sockets == 0) || (config->has_dies && config->dies == 0) || (config->has_clusters && config->clusters == 0) || @@ -111,6 +123,19 @@ void machine_parse_smp_config(MachineState *ms, dies = dies > 0 ? dies : 1; clusters = clusters > 0 ? clusters : 1; + if (!mc->smp_props.books_supported && books > 1) { + error_setg(errp, "books not supported by this machine's CPU topology"); + return; + } + books = books > 0 ? books : 1; + + if (!mc->smp_props.drawers_supported && drawers > 1) { + error_setg(errp, + "drawers not supported by this machine's CPU topology"); + return; + } + drawers = drawers > 0 ? drawers : 1; + /* compute missing values based on the provided ones */ if (cpus == 0 && maxcpus == 0) { sockets = sockets > 0 ? sockets : 1; @@ -124,33 +149,41 @@ void machine_parse_smp_config(MachineState *ms, if (sockets == 0) { cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; - sockets = maxcpus / (dies * clusters * cores * threads); + sockets = maxcpus / + (drawers * books * dies * clusters * cores * threads); } else if (cores == 0) { threads = threads > 0 ? threads : 1; - cores = maxcpus / (sockets * dies * clusters * threads); + cores = maxcpus / + (drawers * books * sockets * dies * clusters * threads); } } else { /* prefer cores over sockets since 6.2 */ if (cores == 0) { sockets = sockets > 0 ? sockets : 1; threads = threads > 0 ? threads : 1; - cores = maxcpus / (sockets * dies * clusters * threads); + cores = maxcpus / + (drawers * books * sockets * dies * clusters * threads); } else if (sockets == 0) { threads = threads > 0 ? threads : 1; - sockets = maxcpus / (dies * clusters * cores * threads); + sockets = maxcpus / + (drawers * books * dies * clusters * cores * threads); } } /* try to calculate omitted threads at last */ if (threads == 0) { - threads = maxcpus / (sockets * dies * clusters * cores); + threads = maxcpus / + (drawers * books * sockets * dies * clusters * cores); } } - maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads; + maxcpus = maxcpus > 0 ? maxcpus : drawers * books * sockets * dies * + clusters * cores * threads; cpus = cpus > 0 ? cpus : maxcpus; ms->smp.cpus = cpus; + ms->smp.drawers = drawers; + ms->smp.books = books; ms->smp.sockets = sockets; ms->smp.dies = dies; ms->smp.clusters = clusters; @@ -159,7 +192,8 @@ void machine_parse_smp_config(MachineState *ms, ms->smp.max_cpus = maxcpus; /* sanity-check of the computed topology */ - if (sockets * dies * clusters * cores * threads != maxcpus) { + if (drawers * books * sockets * dies * clusters * cores * threads != + maxcpus) { g_autofree char *topo_msg = cpu_hierarchy_to_string(ms); error_setg(errp, "Invalid CPU topology: " "product of the hierarchy must match maxcpus: " diff --git a/hw/core/machine.c b/hw/core/machine.c index f589b92909..3c781fed07 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -827,6 +827,8 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name, MachineState *ms = MACHINE(obj); SMPConfiguration *config = &(SMPConfiguration){ .has_cpus = true, .cpus = ms->smp.cpus, + .has_drawers = true, .drawers = ms->smp.drawers, + .has_books = true, .books = ms->smp.books, .has_sockets = true, .sockets = ms->smp.sockets, .has_dies = true, .dies = ms->smp.dies, .has_clusters = true, .clusters = ms->smp.clusters, @@ -1092,6 +1094,8 @@ static void machine_initfn(Object *obj) /* default to mc->default_cpus */ ms->smp.cpus = mc->default_cpus; ms->smp.max_cpus = mc->default_cpus; + ms->smp.drawers = 1; + ms->smp.books = 1; ms->smp.sockets = 1; ms->smp.dies = 1; ms->smp.clusters = 1; diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index f22f61b8b6..f3cc845d3b 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -733,6 +733,8 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) mc->no_sdcard = 1; mc->max_cpus = S390_MAX_CPUS; mc->has_hotpluggable_cpus = true; + mc->smp_props.books_supported = true; + mc->smp_props.drawers_supported = true; assert(!mc->get_hotplug_handler); mc->get_hotplug_handler = s390_get_hotplug_handler; mc->cpu_index_to_instance_props = s390_cpu_index_to_props; diff --git a/softmmu/vl.c b/softmmu/vl.c index 798e1dc933..e894ab32b0 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -724,6 +724,12 @@ static QemuOptsList qemu_smp_opts = { { .name = "cpus", .type = QEMU_OPT_NUMBER, + }, { + .name = "drawers", + .type = QEMU_OPT_NUMBER, + }, { + .name = "books", + .type = QEMU_OPT_NUMBER, }, { .name = "sockets", .type = QEMU_OPT_NUMBER, diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index 96562c516d..c9f980dbb5 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -35,6 +35,7 @@ #include "fpu/softfloat-helpers.h" #include "disas/capstone.h" #include "sysemu/tcg.h" +#include "hw/s390x/cpu-topology.h" #define CR0_RESET 0xE0UL #define CR14_RESET 0xC2000000UL; @@ -257,6 +258,15 @@ static gchar *s390_gdb_arch_name(CPUState *cs) static Property s390x_cpu_properties[] = { #if !defined(CONFIG_USER_ONLY) DEFINE_PROP_UINT32("core-id", S390CPU, env.core_id, 0), + DEFINE_PROP_INT32("socket-id", S390CPU, env.socket_id, -1), + DEFINE_PROP_INT32("book-id", S390CPU, env.book_id, -1), + DEFINE_PROP_INT32("drawer-id", S390CPU, env.drawer_id, -1), + DEFINE_PROP_INT32("dedicated", S390CPU, env.dedicated, + S390_TOPOLOGY_SHARED), + DEFINE_PROP_INT32("polarity", S390CPU, env.polarity, + S390_TOPOLOGY_POLARITY_HORIZONTAL), + DEFINE_PROP_INT32("cpu-type", S390CPU, env.cpu_type, + S390_TOPOLOGY_CPU_IFL), #endif DEFINE_PROP_END_OF_LIST() }; diff --git a/qemu-options.hx b/qemu-options.hx index 7f99d15b23..8dc9a4c052 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -250,11 +250,13 @@ SRST ERST DEF("smp", HAS_ARG, QEMU_OPTION_smp, - "-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n" + "-smp [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n" " set the number of initial CPUs to 'n' [default=1]\n" " maxcpus= maximum number of total CPUs, including\n" " offline CPUs for hotplug, etc\n" - " sockets= number of sockets on the machine board\n" + " drawers= number of drawers on the machine board\n" + " books= number of books in one drawer\n" + " sockets= number of sockets in one book\n" " dies= number of dies in one socket\n" " clusters= number of clusters in one die\n" " cores= number of cores in one cluster\n"
S390 adds two new SMP levels, drawers and books to the CPU topology. The S390 CPU have specific toplogy features like dedication and polarity to give to the guest indications on the host vCPUs scheduling and help the guest take the best decisions on the scheduling of threads on the vCPUs. Let us provide the SMP properties with books and drawers levels and S390 CPU with dedication and polarity, Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- qapi/machine.json | 14 ++++++++-- include/hw/boards.h | 10 ++++++- include/hw/s390x/cpu-topology.h | 23 ++++++++++++++++ target/s390x/cpu.h | 6 +++++ hw/core/machine-smp.c | 48 ++++++++++++++++++++++++++++----- hw/core/machine.c | 4 +++ hw/s390x/s390-virtio-ccw.c | 2 ++ softmmu/vl.c | 6 +++++ target/s390x/cpu.c | 10 +++++++ qemu-options.hx | 6 +++-- 10 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 include/hw/s390x/cpu-topology.h