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