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 <ehabkost@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 <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org