Re: [RFC PATCH v2 23/29] target/arm/qmp: add query-arm-cpu-props-info

[Markus Armbruster <armbru@redhat.com>]

Khushit Shah <khushit.shah@nutanix.com> writes:

> Introduce 'query-arm-cpu-props-info' QMP command. This command returns
> information about ARM CPU model properties, including their name, type,
> supported values on the given host, and composite parent if any.
>
> This provides management layer necessary information to determine
> migration compatibility off a VM on different host.
>
> Signed-off-by: Khushit Shah <khushit.shah@nutanix.com>
> ---
>  qapi/misc-arm.json              | 41 ++++++++++++++++
>  stubs/qmp-arm-gic.c             |  6 +++
>  target/arm/arm-cpu-props-stub.c | 13 +++++
>  target/arm/arm-cpu-props.c      | 85 +++++++++++++++++++++++++++++++++
>  target/arm/arm-cpu-props.h      |  5 ++
>  target/arm/arm-qmp-cmds.c       | 44 +++++++++++++++++
>  target/arm/cpu-idregs.c         | 71 +++++++++++++++++++++++++++
>  target/arm/cpu-idregs.h         |  8 ++++
>  8 files changed, 273 insertions(+)
>
> diff --git a/qapi/misc-arm.json b/qapi/misc-arm.json
> index 4dc66d00e5..1a3fb140c4 100644
> --- a/qapi/misc-arm.json
> +++ b/qapi/misc-arm.json
> @@ -46,6 +46,47 @@
>  ##
>  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
>  
> +##
> +# @CpuPropInfo:

CpuPropertyInfo for consistency with type names such as ObjectPropertyInfo.

> +#
> +# Information about a specific ARM CPU model property.
> +#
> +# @name: the name of the property
> +#
> +# @type: the type of the property, any of ('string', 'boolean',
> +#     'number').
> +#
> +# @supported-values: the set of values the host hardware supports
> +#     for the property, a list of strings.

I figure the set of values depends on the host, i.e. it isn't known at
compile time.  Correct?

Are the possible property names known at compile time?

List of strings is fine when the values are strings.  When they're not,
the interface requires its users to parse strings, which is something we
try hard to avoid in QAPI/QMP.

> +#
> +# @composite: name of the encapsulating composite property if any.
> +#
> +# Since: 11.1
> +##
> +{ 'struct': 'CpuPropInfo',
> +  'data': { 'name': 'str',
> +            'type': 'str',
> +            'supported-values': 'any',

Description above claims "list of strings".  If that's accurate, it
should be ['str'] here.  Else, the description needs fixing.

> +            '*composite': 'str' } }
> +
> +##
> +# @query-arm-cpu-props-info:
> +#
> +# Returns information about most of ARM CPU named model properties.

Imperative mood, please: Return information ...

> +# This does not include composite properties or properties already
> +# defined by QEMU like (sveNNNN, pmu, etc), This only returns all
> +# the newly defined properties like ('feat_*', 'hw_prop_*', etc).

"newly defined" compared to what?

> +#
> +# With this upper management layers can find out what property values

Comma after this, I think.

> +# are supported on the given host.
> +#
> +# Requires KVM to be enabled.

What happens when it isn't?

> +#
> +# Since: 11.1
> +##
> +{ 'command': 'query-arm-cpu-props-info',
> +  'returns': ['CpuPropInfo'] }
> +
>  ##
>  # @SsidSizeMode:
>  #

[...]