[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Patch v1 24/29] qmp: add QMP interface "query-cpu-mode
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [Patch v1 24/29] qmp: add QMP interface "query-cpu-model-expansion" |
Date: |
Tue, 2 Aug 2016 17:04:05 +0200 |
> > +# A CPU model consists of the name of a CPU definition, to which
> > +# delta changes are applied (e.g. features added/removed). Most magic
> > values
> > +# that an architecture might require should be hidden behind the name.
> > +# However, if required, architectures can expose relevant properties.
> > +#
> > +# @name: the name of the CPU definition the model is based on
> > +# @props: #optional a dictionary of properties to be applied
>
> Should we make it explicit that we are talking about QOM
> properties?
Yes makes sense.
>
> > +#
> > +# Since: 2.8.0
> > +##
> > +{ 'struct': 'CpuModelInfo',
> > + 'data': { 'name': 'str',
> > + '*props': 'any' } }
> > +
> > +##
> > +# @CpuModelExpansionType
> > +#
> > +# An enumeration of CPU model expansion types.
> > +#
> > +# @static: Expand to a static CPU model, a combination of a static base
> > +# model name and property delta changes. As the static base model
> > will
> > +# never change, the expanded CPU model will be the same,
> > independant of
> > +# QEMU version or compatibility machines. Therefore, the
> > resulting
>
> We could be more explicit about the guarantees: "independent of
> QEMU version, machine type, machine options, and accelerator
> options".
agreed.
>
> > +# model can be used by tooling without having to specify a
> > +# compatibility machine - e.g. when displaying the "host" model.
> > +# All static CPU models are migration-safe.
>
> This is cool. Unfortunately we are not going to support it in x86
> very soon because we don't have any static CPU models.
Well, it's all about finding a minimum set of features that one can work with.
I assume e.g. a Phenom also always has a minimum set of features?
>
> > +#
> > +# @full: Expand all properties. The produced model is not guaranteed to be
> > +# migration-safe, but allows tooling to get an insight and work with
> > +# model details.
>
> I wonder if we really need to document it broadly as "not
> guaranteed to be migration-safe". The returned data will be
> migration-safe (but not static) if the CPU model being expanded
> is migration-safe, won't it?
Actually I don't think so.
Imagine expanding host: featA=true, featB=false
Now, if going to another QEMU version, there might be featC known.
So -host,featA=on,featB=off will implicitly enable featC and is therefore
not be migration-safe. You would have to disable featC for compatibility
machines on the host model. Is something like that done? I don't think so
(and at least s390x won't do it right now).
But, I can simply get rid of that remark.
>
> Also: I wonder what should be the return value for "name" when
> expansion type is "full" and we don't have any static CPU models
> (like on x86). e.g. returning:
> { name: "host", props: { foo: "on", bar: "on", ... }
> would make the interface not directly usable for the expansion of
> <cpu mode="host-model">. Maybe that means we have to add at least
> one static CPU model to x86, too?
I'd simply copy the name then. That's what I had in mind.
(actually I don't do it on s390x because it's easier to just rely
on the output of our conversion function).
> > +
> > +
> > +##
> > +# @query-cpu-model-expansion:
> > +#
> > +# Expands the given CPU model to different granularities, allowing tooling
> > +# to get an understanding what a specific CPU model looks like in QEMU
> > +# under a certain QEMU machine.
>
> Maybe "expands a given CPU model (or a combination of CPU model +
> additional options) to different granularities".
>
> > +#
> > +# Expanding CPU models is in general independant of the accelerator, except
> > +# for models like "host" that explicitly rely on an accelerator and can
> > +# vary in different configurations.
>
> Unfortunately this is not true in x86. Documenting it this way
> might give people the wrong expectations.
>
> Specific guarantees from arch-specific code could be documented
> somewhere, but: where?
>
> Maybe arch-specific guarantees should actually become
> query-cpu-definitions fields (like we have "static" and
> "migration-safe" today). If people are really interested in
> accelerator-independent data, we need
Okay, so this could be extended later than.
>
>
> > This interface can therefore also be used
> > +# to query the "host" CPU model.
> > +#
> > +# Note: This interface should not be used when global properties of CPU
> > classes
> > +# are changed (e.g. via "-cpu ...").
>
> We could simply enumerate all cases that could affect the return
> value. e.g.:
>
> # The data returned by this command may be affected by:
> #
> # * QEMU version: CPU models may look different depending on the
> # QEMU version. (Except for CPU models reported as "static"
> # in query-cpu-definitions.)
> # * machine-type: CPU model expansion may look different depending
> # on the machine-type. (Except for CPU models reported as "static"
> # in query-cpu-definitions.)
> # * machine options (including accelerator): in some
> # architectures, CPU models may look different depending on
> # machine and accelerator options. (Except for CPU models
> # reported as "static" in query-cpu-definitions.)
> # * "-cpu" arguments and global properties: arguments to the -cpu
> # option and global properties may affect expansion of CPU
> # models. Using query-cpu-model-expansion while using "-cpu"
> # or global properties is not advised.
>
Yes, that's better.
>
> > +#
> > +# s390x supports expanding of all CPU models with all expansion types.
> > Other
> > +# architectures are not supported yet.
>
> I think this paragraph is likely to get obsolete very soon (as
> people may forget to update it when implementing the new
> interface on other architectures). Also, the paragraph is not
> true until patch 27/29 is applied.
>
> Maybe write it as "Some architectures may not support all
> expansion types".
Agreed. And most likely x86 won't support expanding all CPU models I assume?
>
> Patch 27/29 could add "s390x supports both 'static' and 'full'
> expansion types". I wouldn't document it as "supports all
> expansion types" because CpuModelExpansionType may be extended in
> the future.
Agreed.
Thanks!
David
- [Qemu-devel] [Patch v1 15/29] s390x/sclp: indicate sclp features, (continued)
- [Qemu-devel] [Patch v1 15/29] s390x/sclp: indicate sclp features, David Hildenbrand, 2016/08/02
- [Qemu-devel] [Patch v1 03/29] s390x/cpumodel: expose CPU class properties, David Hildenbrand, 2016/08/02
- [Qemu-devel] [Patch v1 07/29] s390x/cpumodel: introduce CPU feature group definitions, David Hildenbrand, 2016/08/02
- [Qemu-devel] [Patch v1 14/29] s390x/sclp: introduce sclp feature blocks, David Hildenbrand, 2016/08/02
- [Qemu-devel] [Patch v1 12/29] s390x/cpumodel: check and apply the CPU model, David Hildenbrand, 2016/08/02
- [Qemu-devel] [Patch v1 18/29] s390x/sclp: propagate hmfai, David Hildenbrand, 2016/08/02
- [Qemu-devel] [Patch v1 24/29] qmp: add QMP interface "query-cpu-model-expansion", David Hildenbrand, 2016/08/02
[Qemu-devel] [Patch v1 09/29] s390x/cpumodel: store the CPU model in the CPU instance, David Hildenbrand, 2016/08/02
[Qemu-devel] [Patch v1 11/29] s390x/cpumodel: let the CPU model handle feature checks, David Hildenbrand, 2016/08/02
[Qemu-devel] [Patch v1 20/29] s390x/kvm: allow runtime-instrumentation for "none" machine, David Hildenbrand, 2016/08/02
[Qemu-devel] [Patch v1 19/29] linux-headers: update against kvm/next, David Hildenbrand, 2016/08/02
[Qemu-devel] [Patch v1 21/29] s390x/kvm: implement CPU model support, David Hildenbrand, 2016/08/02
[Qemu-devel] [Patch v1 22/29] s390x/kvm: disable host model for existing compat machines, David Hildenbrand, 2016/08/02
[Qemu-devel] [Patch v1 25/29] qmp: add QMP interface "query-cpu-model-comparison", David Hildenbrand, 2016/08/02