On 1/12/23 13:10, Daniel P. Berrangé wrote:
On Thu, Jan 05, 2023 at 03:53:11PM +0100, Pierre Morel wrote:Reporting the current topology informations to the admin through the QEMU monitor. Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx> --- qapi/machine-target.json | 66 ++++++++++++++++++++++++++++++++++ include/monitor/hmp.h | 1 + hw/s390x/cpu-topology.c | 76 ++++++++++++++++++++++++++++++++++++++++ hmp-commands-info.hx | 16 +++++++++ 4 files changed, 159 insertions(+) diff --git a/qapi/machine-target.json b/qapi/machine-target.json index 75b0aa254d..927618a78f 100644 --- a/qapi/machine-target.json +++ b/qapi/machine-target.json @@ -371,3 +371,69 @@ }, 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } } + +## +# @S390CpuTopology: +# +# CPU Topology information +# +# @drawer: the destination drawer where to move the vCPU +# +# @book: the destination book where to move the vCPU +# +# @socket: the destination socket where to move the vCPU +# +# @polarity: optional polarity, default is last polarity set by the guest +# +# @dedicated: optional, if the vCPU is dedicated to a real CPU +# +# @origin: offset of the first bit of the core mask +# +# @mask: mask of the cores sharing the same topology +# +# Since: 8.0 +## +{ 'struct': 'S390CpuTopology', + 'data': { + 'drawer': 'int', + 'book': 'int', + 'socket': 'int', + 'polarity': 'int', + 'dedicated': 'bool', + 'origin': 'int', + 'mask': 'str' + }, + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } +} + +## +# @query-topology: +# +# Return information about CPU Topology +# +# Returns a @CpuTopology instance describing the CPU Toplogy +# being currently used by QEMU. +# +# Since: 8.0 +# +# Example: +# +# -> { "execute": "cpu-topology" } +# <- {"return": [ +# { +# "drawer": 0, +# "book": 0, +# "socket": 0, +# "polarity": 0, +# "dedicated": true, +# "origin": 0, +# "mask": 0xc000000000000000, +# }, +# ] +# } +# +## +{ 'command': 'query-topology', + 'returns': ['S390CpuTopology'], + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } +}IIUC, you're using @mask as a way to compress the array returned from query-topology, so that it doesn't have any repeated elements with the same data. I guess I can understand that desire when the core count can get very large, this can have a large saving. The downside of using @mask, is that now you require the caller to parse the string to turn it into a bitmask and expand the data. Generally this is considered a bit of an anti-pattern in QAPI design - we don't want callers to have to further parse the data to extract information, we want to directly consumable from the parsed JSON doc.
Not exactly, the mask is computed by the firmware to provide it to the guest and is already available when querying the topology. But I understand that for the QAPI user the mask is not the right solution, standard coma separated values like (1,3,5,7-11) would be much easier to read.
We already have 'query-cpus-fast' wich returns one entry for each CPU. In fact why do we need to add query-topology at all. Can't we just add book-id / drawer-id / polarity / dedicated to the query-cpus-fast result ?
Yes we can, I think we should, however when there are a lot of CPU it will be complicated to find the CPU sharing the same socket and the same attributes.
I think having both would be interesting. What do you think? regards, Pierre
With regards, Daniel
-- Pierre Morel IBM Lab Boeblingen
