On 04/22/2013 01:00 PM, Eduardo Habkost wrote: > This property will be useful for libvirt, as libvirt already has logic > based on low-level feature bits (not feature names), so it will be > really easy to convert the current libvirt logic to something using the > "feature-words" property. > > The property will have two main use cases: > - Checking host capabilities, by checking the features of the "host" > CPU model > - Checking which features are enabled on each CPU model >
> item[6].cpuid-register: ECX
> item[6].cpuid-input-eax: 1
> item[6].features: 2155880449
> item[7].cpuid-register: EDX
> item[7].cpuid-input-eax: 1
> item[7].features: 126614521
I'm guessing the corresponding JSON passed over QMP would look something
like:
[ ...
{ "cpuid-register": "ECX",
"cpuid-input-eax": 1,
"features": 2155880449 },
{ "cpuid-register": "EDX",
"cpuid-input-eax": 1,
"features": 126614521 } ]
which libvirt can reasonably parse.
>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> ---
> Changes v1 -> v2:
> * Merge the non-qapi and qapi patches, to keep series simpler
> * Use the feature word array series as base, so we don't have
> to set the feature word values one-by-one in the code
> * Change type name of property from "x86-cpu-feature-words" to
> "X86CPUFeatureWordInfo"
> * Remove cpu-qapi-schema.json and simply add the type definitions
> to qapi-schema.json, to keep the changes simpler
> * This required compiling qapi-types.o and qapi-visit.o into
> *-user as well
> ---
> .gitignore | 2 ++
> Makefile.objs | 7 +++++-
> qapi-schema.json | 31 ++++++++++++++++++++++++
> target-i386/cpu.c | 70
> +++++++++++++++++++++++++++++++++++++++++++++----------
> 4 files changed, 97 insertions(+), 13 deletions(-)
I'm not sure I'm the best person to review cpu.c, but I can at least
review the interface from what libvirt plans on using:
> +++ b/qapi-schema.json
> @@ -3505,3 +3505,34 @@
> '*asl_compiler_rev': 'uint32',
> '*file': 'str',
> '*data': 'str' }}
> +
> +# @X86CPURegister32
> +#
> +# A X86 32-bit register
> +#
> +# Since: 1.5
Yes, I'd still like to get this into 1.5. On some enums, we have called
out doc-text for each enum value; but I'm fine with your choice here to
omit that.
> +##
> +{ 'enum': 'X86CPURegister32',
> + 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
Any reason you favored ALL-CAPS names? But it doesn't matter to me, as
long as we stick to the name once baked into a release.
> +
> +##
> +# @X86CPUFeatureWordInfo
> +#
> +# Information about a X86 CPU feature word
> +#
> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature
> word
> +#
> +# @cpuid-input-ecx: #optional Input ECX value for CPUID instruction for that
> +# feature word
> +#
> +# @cpuid-register: Output register containing the feature bits
> +#
> +# @features: value of output register, containing the feature bits
> +#
> +# Since: 1.5
> +##
> +{ 'type': 'X86CPUFeatureWordInfo',
> + 'data': { 'cpuid-input-eax': 'int',
> + '*cpuid-input-ecx': 'int',
> + 'cpuid-register': 'X86CPURegister32',
> + 'features': 'int' } }
Looks reasonable for the interface side of things.
>
> +static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
Indentation looks off.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
