|
From: | Pierre Morel |
Subject: | Re: [PATCH v14 09/11] qapi/s390/cpu topology: monitor query topology information |
Date: | Wed, 18 Jan 2023 17:57:21 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 |
On 1/18/23 17:08, Daniel P. Berrangé wrote:
On Wed, Jan 18, 2023 at 04:58:05PM +0100, Pierre Morel wrote: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@linux.ibm.com> --- 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.That is still inventing a second level data format for an attribute that needs to be parsed, and its arguably more complex.
OK, I think I am too focused on my vision of the topology. I add the new attributes to the query-cpus-fast
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.It shouldn't be that hard to populate a hash table, using the set of socket + attributes you want as the hash key.
It is not a problem.
I think having both would be interesting.IMHO this is undesirable if we can make query-cpus-fast report sufficient information, as it gives a maint burden to QEMU and is confusing to consumers to QEMU to have multiple commands with largely overlapping functionality.
right. Thanks Daniel. Regards, Pierre
With regards, Daniel
-- Pierre Morel IBM Lab Boeblingen
[Prev in Thread] | Current Thread | [Next in Thread] |