diff mbox series

[v14,09/11] qapi/s390/cpu topology: monitor query topology information

Message ID 20230105145313.168489-10-pmorel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: CPU Topology | expand

Commit Message

Pierre Morel Jan. 5, 2023, 2:53 p.m. UTC
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(+)

Comments

Thomas Huth Jan. 12, 2023, 11:48 a.m. UTC | #1
On 05/01/2023 15.53, Pierre Morel wrote:
> Reporting the current topology informations to the admin through
> the QEMU monitor.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
...
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 754b1e8408..5730a47f71 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -993,3 +993,19 @@ SRST
>     ``info virtio-queue-element`` *path* *queue* [*index*]
>       Display element of a given virtio queue
>   ERST
> +
> +#if defined(TARGET_S390X) && defined(CONFIG_KVM)
> +    {
> +        .name       = "query-topology",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "Show information about CPU topology",
> +        .cmd        = hmp_query_topology,
> +        .flags      = "p",
> +    },
> +
> +SRST
> +  ``info query-topology``

"info query-topology" sounds weird ... I'd maybe rather call it only "info 
topology" or "info cpu-topology" here.

  Thomas


> +    Show information about CPU topology
> +ERST
> +#endif
Daniel P. Berrangé Jan. 12, 2023, 12:10 p.m. UTC | #2
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.

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 ?

With regards,
Daniel
Nina Schoetterl-Glausch Jan. 12, 2023, 5:27 p.m. UTC | #3
On Thu, 2023-01-12 at 12:10 +0000, Daniel P. Berrangé wrote

[...]
> 
> 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 ?

Is there an existing command for setting cpu properties, also?
> 
> With regards,
> Daniel
Daniel P. Berrangé Jan. 12, 2023, 5:30 p.m. UTC | #4
On Thu, Jan 12, 2023 at 06:27:04PM +0100, Nina Schoetterl-Glausch wrote:
> On Thu, 2023-01-12 at 12:10 +0000, Daniel P. Berrangé wrote
> 
> [...]
> > 
> > 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 ?
> 
> Is there an existing command for setting cpu properties, also?

No, there's nothing applicable that I recall for runtime property
changes on CPUs.

With regards,
Daniel
Pierre Morel Jan. 18, 2023, 3:58 p.m. UTC | #5
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.

> 
> 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 Jan. 18, 2023, 3:59 p.m. UTC | #6
On 1/12/23 12:48, Thomas Huth wrote:
> On 05/01/2023 15.53, Pierre Morel wrote:
>> Reporting the current topology informations to the admin through
>> the QEMU monitor.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
> ...
>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
>> index 754b1e8408..5730a47f71 100644
>> --- a/hmp-commands-info.hx
>> +++ b/hmp-commands-info.hx
>> @@ -993,3 +993,19 @@ SRST
>>     ``info virtio-queue-element`` *path* *queue* [*index*]
>>       Display element of a given virtio queue
>>   ERST
>> +
>> +#if defined(TARGET_S390X) && defined(CONFIG_KVM)
>> +    {
>> +        .name       = "query-topology",
>> +        .args_type  = "",
>> +        .params     = "",
>> +        .help       = "Show information about CPU topology",
>> +        .cmd        = hmp_query_topology,
>> +        .flags      = "p",
>> +    },
>> +
>> +SRST
>> +  ``info query-topology``
> 
> "info query-topology" sounds weird ... I'd maybe rather call it only 
> "info topology" or "info cpu-topology" here.

info cpu-topology looks good for me.

thanks.

Regards,
Pierre

> 
>   Thomas
> 
> 
>> +    Show information about CPU topology
>> +ERST
>> +#endif
>
Daniel P. Berrangé Jan. 18, 2023, 4:08 p.m. UTC | #7
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.

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

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


With regards,
Daniel
Pierre Morel Jan. 18, 2023, 4:57 p.m. UTC | #8
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
diff mbox series

Patch

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' ] }
+}
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 15c36bf549..0b3c758231 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -145,5 +145,6 @@  void hmp_human_readable_text_helper(Monitor *mon,
 void hmp_info_stats(Monitor *mon, const QDict *qdict);
 void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict);
 void hmp_change_topology(Monitor *mon, const QDict *qdict);
+void hmp_query_topology(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
index 0faffe657e..c3748654ff 100644
--- a/hw/s390x/cpu-topology.c
+++ b/hw/s390x/cpu-topology.c
@@ -524,3 +524,79 @@  void hmp_change_topology(Monitor *mon, const QDict *qdict)
         return;
     }
 }
+
+static S390CpuTopologyList *s390_cpu_topology_list(void)
+{
+    S390CpuTopologyList *head = NULL;
+    S390TopologyEntry *entry;
+
+    QTAILQ_FOREACH_REVERSE(entry, &s390_topology.list, next) {
+        S390CpuTopology *item = g_new0(typeof(*item), 1);
+
+        item->drawer = entry->id.drawer;
+        item->book = entry->id.book;
+        item->socket = entry->id.socket;
+        item->polarity = entry->id.p;
+        if (entry->id.d) {
+            item->dedicated = true;
+        }
+        item->origin = entry->id.origin;
+        item->mask = g_strdup_printf("0x%016lx", entry->mask);
+
+        QAPI_LIST_PREPEND(head, item);
+    }
+    return head;
+}
+
+S390CpuTopologyList *qmp_query_topology(Error **errp)
+{
+    if (!s390_has_topology()) {
+        error_setg(errp, "This machine doesn't support topology");
+        return NULL;
+    }
+
+    return s390_cpu_topology_list();
+}
+
+void hmp_query_topology(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    S390CpuTopologyList *l = qmp_query_topology(&err);
+
+    if (hmp_handle_error(mon, err)) {
+        return;
+    }
+
+    monitor_printf(mon, "CPU Topology:\n");
+    while (l) {
+        uint64_t d = -1UL;
+        uint64_t b = -1UL;
+        uint64_t s = -1UL;
+        uint64_t p = -1UL;
+        uint64_t dd = -1UL;
+
+        if (d != l->value->drawer) {
+            monitor_printf(mon, "  drawer   : \"%" PRIu64 "\"\n",
+                           l->value->drawer);
+        }
+        if (b != l->value->book) {
+            monitor_printf(mon, "  book     : \"%" PRIu64 "\"\n",
+                           l->value->book);
+        }
+        if (s != l->value->socket) {
+            monitor_printf(mon, "  socket   : \"%" PRIu64 "\"\n",
+                           l->value->socket);
+        }
+        if (p != l->value->polarity) {
+            monitor_printf(mon, "  polarity : \"%" PRIu64 "\"\n",
+                           l->value->polarity);
+        }
+        if (dd != l->value->dedicated) {
+            monitor_printf(mon, "  dedicated: \"%d\"\n", l->value->dedicated);
+        }
+        monitor_printf(mon, "  mask  : \"%s\"\n", l->value->mask);
+
+
+        l = l->next;
+    }
+}
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 754b1e8408..5730a47f71 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -993,3 +993,19 @@  SRST
   ``info virtio-queue-element`` *path* *queue* [*index*]
     Display element of a given virtio queue
 ERST
+
+#if defined(TARGET_S390X) && defined(CONFIG_KVM)
+    {
+        .name       = "query-topology",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Show information about CPU topology",
+        .cmd        = hmp_query_topology,
+        .flags      = "p",
+    },
+
+SRST
+  ``info query-topology``
+    Show information about CPU topology
+ERST
+#endif