Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add feature-words property
On Mon, 22 Apr 2013 16:00:17 -0300
Eduardo Habkost ehabk...@redhat.com 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
patch doesn't apply to current qom-cpu, more comments below.
Example output:
$ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1]
--property=feature-words
item[0].cpuid-register: EDX
item[0].cpuid-input-eax: 2147483658
item[0].features: 0
item[1].cpuid-register: EAX
item[1].cpuid-input-eax: 1073741825
item[1].features: 0
item[2].cpuid-register: EDX
item[2].cpuid-input-eax: 3221225473
item[2].features: 0
item[3].cpuid-register: ECX
item[3].cpuid-input-eax: 2147483649
item[3].features: 101
item[4].cpuid-register: EDX
item[4].cpuid-input-eax: 2147483649
item[4].features: 563346425
item[5].cpuid-register: EBX
item[5].cpuid-input-eax: 7
item[5].features: 0
item[5].cpuid-input-ecx: 0
could item be represented as CPUID function in format used in Intel's SDM?
item[5].CPUID: EAX=7h,ECX=0h
item[5].EBX: 0
item[5].EAX: 0
for simplicity/uniformity ECX could be not optional, always present, and
ignored when not needed.
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
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(-)
diff --git a/.gitignore b/.gitignore
index 487813a..71408e3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -21,6 +21,8 @@ linux-headers/asm
qapi-generated
qapi-types.[ch]
qapi-visit.[ch]
+cpu-qapi-types.[ch]
+cpu-qapi-visit.[ch]
still needed?
qmp-commands.h
qmp-marshal.c
qemu-doc.html
diff --git a/Makefile.objs b/Makefile.objs
index a473348..1835d37 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -78,10 +78,15 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
##
# qapi
-common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
+common-obj-y += qmp-marshal.o
common-obj-y += qmp.o hmp.o
endif
+##
+# some qapi visitors are used by both system and user emulation:
+
+common-obj-y += qapi-visit.o qapi-types.o
+
###
# Target-independent parts used in system and user emulation
common-obj-y += qemu-log.o
diff --git a/qapi-schema.json b/qapi-schema.json
index 751d3c2..424434f 100644
--- a/qapi-schema.json
+++ 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
+##
+{ 'enum': 'X86CPURegister32',
+ 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
+
+##
+# @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' } }
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 314931e..757749c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -30,6 +30,8 @@
#include qemu/config-file.h
#include qapi/qmp/qerror.h
+#include qapi-types.h
+#include qapi-visit.h
#include qapi/visitor.h
#include
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add feature-words property
On Fri, May 03, 2013 at 01:34:23PM +0200, Igor Mammedov wrote: On Mon, 22 Apr 2013 16:00:17 -0300 Eduardo Habkost ehabk...@redhat.com 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 patch doesn't apply to current qom-cpu, more comments below. Example output: $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=feature-words item[0].cpuid-register: EDX item[0].cpuid-input-eax: 2147483658 item[0].features: 0 item[1].cpuid-register: EAX item[1].cpuid-input-eax: 1073741825 item[1].features: 0 item[2].cpuid-register: EDX item[2].cpuid-input-eax: 3221225473 item[2].features: 0 item[3].cpuid-register: ECX item[3].cpuid-input-eax: 2147483649 item[3].features: 101 item[4].cpuid-register: EDX item[4].cpuid-input-eax: 2147483649 item[4].features: 563346425 item[5].cpuid-register: EBX item[5].cpuid-input-eax: 7 item[5].features: 0 item[5].cpuid-input-ecx: 0 could item be represented as CPUID function in format used in Intel's SDM? We could, but maybe it would make the interface harder to use and not easier? Even when two feature words are returned in the same CPUID leaf, they are independent and separate feature-words that must be checked individually by libvirt, so I believe returning one feature-word per array-item makes more sense. Having an extra item in the array would make it clear for libvirt that QEMU has a new feature-word that libvirt doesn't know about, and easier to spot than an extra field in an existing array item. item[5].CPUID: EAX=7h,ECX=0h What would be the data type of this CPUID field? Are you suggesting returning a string to be parsed manually? item[5].EBX: 0 item[5].EAX: 0 for simplicity/uniformity ECX could be not optional, always present, and ignored when not needed. Then how would we represent the fact that ECX is optional? If ECX is not important for a given CPUID leaf, I don't see what's the point of including it with a fake value. Note that this interface is not supposed to be a comprehensive CPUID dump with all CPUID bits returned by the CPU, but just a list of CPU capability words that happen to be returned inside CPUID leaves. 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 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(-) diff --git a/.gitignore b/.gitignore index 487813a..71408e3 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,8 @@ linux-headers/asm qapi-generated qapi-types.[ch] qapi-visit.[ch] +cpu-qapi-types.[ch] +cpu-qapi-visit.[ch] still needed? Not needed anymore. Thanks for spotting it. qmp-commands.h qmp-marshal.c qemu-doc.html diff --git a/Makefile.objs b/Makefile.objs index a473348..1835d37 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -78,10 +78,15 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y) ## # qapi -common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o +common-obj-y += qmp-marshal.o common-obj-y += qmp.o hmp.o endif +## +# some qapi visitors are used by both system and user emulation: + +common-obj-y += qapi-visit.o qapi-types.o + ### # Target-independent parts used in system and user emulation common-obj-y += qemu-log.o diff --git a/qapi-schema.json b/qapi-schema.json index 751d3c2..424434f 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3505,3 +3505,34 @@ '*asl_compiler_rev': 'uint32',
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add feature-words property
On 05/03/2013 07:17 AM, Eduardo Habkost wrote:
We could, but maybe it would make the interface harder to use and not
easier?
Even when two feature words are returned in the same CPUID leaf, they
are independent and separate feature-words that must be checked
individually by libvirt, so I believe returning one feature-word per
array-item makes more sense. Having an extra item in the array would
make it clear for libvirt that QEMU has a new feature-word that libvirt
doesn't know about, and easier to spot than an extra field in an
existing array item.
Firmly agree - bundling multiple features into one array item is not nice.
item[5].CPUID: EAX=7h,ECX=0h
What would be the data type of this CPUID field? Are you suggesting
returning a string to be parsed manually?
Anything that requires parsing to break into pieces on the receiving end
implies that it was not correctly represented in JSON in the first
place. I'd much rather see it kept as multiple fields.
+for (w = 0; w FEATURE_WORDS; w++) {
+FeatureWordInfo *wi = feature_word_info[w];
+X86CPUFeatureWordInfo *qwi = word_infos[w];
+qwi-cpuid_input_eax = wi-cpuid_eax;
+qwi-has_cpuid_input_ecx = wi-cpuid_needs_ecx;
+qwi-cpuid_input_ecx = wi-cpuid_ecx;
+qwi-cpuid_register = x86_reg_info_32[wi-cpuid_reg].qapi_enum;
Is there way not to use qapi_enum at all and use name instead?
Are you suggesting making the qapi interface be string-based instead of
using an enum? Why?
enum-based is better than string based. That way, when we add
introspection in qemu 1.6, libvirt can see what enum values to expect,
instead of having an open-ended set of strings with no idea what strings
will be present.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add feature-words property
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
