On Tue, Dec 13, 2016 at 10:47:38AM +0100, Markus Armbruster wrote:
I'm not familiar with CPU model expansion, but here goes anyway.
Eduardo Habkost <ehabkost(a)redhat.com> writes:
> On x86, "-cpu host" enables some features that can't be
> represented by a static CPU model definition: cache info
> passthrough ("host-cache-info") and PMU passthrough ("pmu").
This
> means a type=static expansion of "host" can't include those
> features.
>
> A type=full expansion of "host", on the other hand, can include
> those features, but then the returned data won't be a static CPU
> model representation.
>
> Add a note to the documentation explaining that when using CPU
> models that include non-migration-safe features, users need to
> choose being precision and safety: a precise expansion of the CPU
s/being/between/
> model (full) won't be safe (static), (because they would include
s/they/it/
Oops. Thanks!
> pmu=on and host-cache-info=on), and a safe (static) expansion of
> the CPU model won't be precise.
>
> Architectures where CPU model expansion is always migration-safe
> (e.g. s390x) can simply do what they already do, and set
> 'migration-safe' and 'static' to true.
This patch does that exactly for s390x. The next patch looks like it
does something for x86. What about other targets? I recommend to have
this commit message explain briefly what this patch does, what later
patches in this series do, and what's left for later (if anything).
s390x is the only architecture implementing
query-cpu-model-expansion by now.
I will add a comment clarifying that, anyway.
> Cc: Cornelia Huck <cornelia.huck(a)de.ibm.com>
> Cc: Christian Borntraeger <borntraeger(a)de.ibm.com>
> Cc: David Hildenbrand <david(a)redhat.com>
> Cc: libvir-list(a)redhat.com
> Cc: Jiri Denemark <jdenemar(a)redhat.com>
> Cc: "Jason J. Herne" <jjherne(a)linux.vnet.ibm.com>
> Cc: Markus Armbruster <armbru(a)redhat.com>
> Cc: Eric Blake <eblake(a)redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
> ---
> qapi-schema.json | 25 ++++++++++++++++++++++++-
> target-s390x/cpu_models.c | 4 ++++
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8d113f8..a102534 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3291,6 +3291,15 @@
> # migration-safe, but allows tooling to get an insight and work with
> # model details.
> #
> +# Note: When a non-migration-safe CPU model is expanded in static mode, some
> +# features enabled by the CPU model may be omitted, because they can't be
> +# implemented by a static CPU model definition (e.g. cache info passthrough and
> +# PMU passthrough in x86). If you need an accurate representation of the
> +# features enabled by a non-migration-safe CPU model, use @full. If you need a
> +# static representation that will keep ABI compatibility even when changing QEMU
> +# version or machine-type, use @static (but keep in mind that some features may
> +# be omitted).
> +#
> # Since: 2.8.0
> ##
> { 'enum': 'CpuModelExpansionType',
> @@ -3304,10 +3313,24 @@
> #
> # @model: the expanded CpuModelInfo.
> #
> +# @migration-safe: the expanded CPU model in @model is a migration-safe
> +# CPU model. See @CpuDefinitionInfo.migration-safe.
> +# If expansion type was @static, this is always true.
> +# (since 2.9)
> +#
> +# @static: the expanded CPU model in @model is a static CPU model.
> +# See @CpuDefinitionInfo.static. If expansion type was @static,
> +# this is always true.
> +# (since 2.9)
> +#
> +# query-cpu-model-expansion with static expansion type should always
> +# return a static and migration-safe expansion.
> +#
> # Since: 2.8.0
> ##
> { 'struct': 'CpuModelExpansionInfo',
> - 'data': { 'model': 'CpuModelInfo' } }
> + 'data': { 'model': 'CpuModelInfo', 'static':
'bool',
> + 'migration-safe': 'bool' } }
>
>
> ##
> diff --git a/target-s390x/cpu_models.c b/target-s390x/cpu_models.c
> index 5b66d33..f934add 100644
> --- a/target-s390x/cpu_models.c
> +++ b/target-s390x/cpu_models.c
> @@ -448,6 +448,10 @@ CpuModelExpansionInfo
*arch_query_cpu_model_expansion(CpuModelExpansionType type
> /* convert it back to a static representation */
> expansion_info = g_malloc0(sizeof(*expansion_info));
> expansion_info->model = g_malloc0(sizeof(*expansion_info->model));
> +
> + /* We always expand to a static and migration-safe CpuModelInfo */
> + expansion_info->q_static = true;
> + expansion_info->migration_safe = true;
> cpu_info_from_model(expansion_info->model, &s390_model, delta_changes);
> return expansion_info;
> }
--
Eduardo