Re: [PATCH v2 1/1] target-i386: Fix default Hypervisor level for hypervisor-vendor=kvm.
On Thu, Sep 20, 2012 at 04:06:27PM -0400, Don Slutz wrote:
From http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html
EAX should be KVM_CPUID_FEATURES (0x4001) not 0.
Added hypervisor-vendor=kvm0 to get the older CPUID result. kvm1 selects the
newer one.
Why not just make hypervisor-vendor=kvm control only the hypervisor
vendor string, and support something like kvm-hypervisor-level=0 to
restore the old cpuid_hv_level=0 behavior?
This is similar to the kvmclock case: it would allow us to make
hypervisor-vendor=kvm use saner values as default, but letting old
machine-types to override it for compatibility if required.
Signed-off-by: Don Slutz d...@cloudswitch.com
---
target-i386/cpu.c | 17 +
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 72a8442..e51b2b1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1250,9 +1250,12 @@ static char *x86_cpuid_get_hv_vendor(Object *obj,
Error **errp)
} else if (!strcmp(value, CPUID_HV_VENDOR_XEN)
env-cpuid_hv_level == CPUID_HV_LEVEL_XEN) {
pstrcpy(value, sizeof(value), xen);
-} else if (!strcmp(value, CPUID_HV_VENDOR_KVM)
- env-cpuid_hv_level == 0) {
-pstrcpy(value, sizeof(value), kvm);
+} else if (!strcmp(value, CPUID_HV_VENDOR_KVM)) {
+if (env-cpuid_hv_level == CPUID_HV_LEVEL_KVM) {
+pstrcpy(value, sizeof(value), kvm1);
+} else if (env-cpuid_hv_level == 0) {
+pstrcpy(value, sizeof(value), kvm0);
+}
}
return value;
}
@@ -1288,7 +1291,13 @@ static void x86_cpuid_set_hv_vendor(Object *obj, const
char *value,
env-cpuid_hv_level = CPUID_HV_LEVEL_XEN;
}
pstrcpy(adj_value, sizeof(adj_value), CPUID_HV_VENDOR_XEN);
-} else if (!strcmp(value, kvm)) {
+} else if (!strcmp(value, kvm) || !strcmp(value, kvm1)) {
+if (env-cpuid_hv_level == 0) {
+env-cpuid_hv_level = CPUID_HV_LEVEL_KVM;
+}
+pstrcpy(adj_value, sizeof(adj_value), CPUID_HV_VENDOR_KVM);
+} else if (!strcmp(value, kvm0)) {
+env-cpuid_hv_level = 0;
pstrcpy(adj_value, sizeof(adj_value), CPUID_HV_VENDOR_KVM);
} else {
pstrcpy(adj_value, sizeof(adj_value), value);
--
1.7.1
--
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] target-i386: Fix default Hypervisor level for hypervisor-vendor=kvm.
On 09/21/12 10:18, Eduardo Habkost wrote: On Thu, Sep 20, 2012 at 04:06:27PM -0400, Don Slutz wrote: From http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html EAX should be KVM_CPUID_FEATURES (0x4001) not 0. Added hypervisor-vendor=kvm0 to get the older CPUID result. kvm1 selects the newer one. Why not just make hypervisor-vendor=kvm control only the hypervisor vendor string, and support something like kvm-hypervisor-level=0 to restore the old cpuid_hv_level=0 behavior? -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 Does this. This is similar to the kvmclock case: it would allow us to make hypervisor-vendor=kvm use saner values as default, but letting old machine-types to override it for compatibility if required. Right now since I am using env-cpuid_hv_level == 0 as a flag. This means that: -cpu host,hypervisor-level=0,hypervisor-vendor=kvm -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 end up with different CPUID data (Which I do not like). I will fix this in the next round. Did you want me to drop kvm0 and kvm1? -Don [...] -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] target-i386: Fix default Hypervisor level for hypervisor-vendor=kvm.
On 09/21/12 16:49, Eduardo Habkost wrote: On Fri, Sep 21, 2012 at 04:26:58PM -0400, Don Slutz wrote: On 09/21/12 10:18, Eduardo Habkost wrote: On Thu, Sep 20, 2012 at 04:06:27PM -0400, Don Slutz wrote: From http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html EAX should be KVM_CPUID_FEATURES (0x4001) not 0. Added hypervisor-vendor=kvm0 to get the older CPUID result. kvm1 selects the newer one. Why not just make hypervisor-vendor=kvm control only the hypervisor vendor string, and support something like kvm-hypervisor-level=0 to restore the old cpuid_hv_level=0 behavior? -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 Does this. Good. :-) This is similar to the kvmclock case: it would allow us to make hypervisor-vendor=kvm use saner values as default, but letting old machine-types to override it for compatibility if required. Right now since I am using env-cpuid_hv_level == 0 as a flag. This means that: -cpu host,hypervisor-level=0,hypervisor-vendor=kvm -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 end up with different CPUID data (Which I do not like). I will fix this in the next round. Right. This has to be fixed. Did you want me to drop kvm0 and kvm1? Yes, if level is already configurable using the hypervisor-level property, I don't see the need for kvm0 and kvm1. If you make kvm_arch_init_vcpu() actually use those fields, you will end up implementing what's required to allow migration compatibility to be kept (the only thing missing is to make the CPU class a child of DeviceState, and add hypervisor-level=0 to the existing machine-types). :-) You mean like http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03402.html and http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03405.html already change kvm_arch_init_vcpu(). I did not know that I need to make the CPU class a child of DeviceState. Nor that I needed to add hypervisor-vendor=kvm,hypervisor-level=0 to the existing machine-types. Since without specifying hypervisor-level=0 it defaults to 0 and kvm_arch_init_vcpu() will default to setting hypervisor-vendor=kvm. -Don -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] target-i386: Fix default Hypervisor level for hypervisor-vendor=kvm.
On Fri, Sep 21, 2012 at 05:28:27PM -0400, Don Slutz wrote: On 09/21/12 16:49, Eduardo Habkost wrote: On Fri, Sep 21, 2012 at 04:26:58PM -0400, Don Slutz wrote: On 09/21/12 10:18, Eduardo Habkost wrote: On Thu, Sep 20, 2012 at 04:06:27PM -0400, Don Slutz wrote: From http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html EAX should be KVM_CPUID_FEATURES (0x4001) not 0. Added hypervisor-vendor=kvm0 to get the older CPUID result. kvm1 selects the newer one. Why not just make hypervisor-vendor=kvm control only the hypervisor vendor string, and support something like kvm-hypervisor-level=0 to restore the old cpuid_hv_level=0 behavior? -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 Does this. Good. :-) This is similar to the kvmclock case: it would allow us to make hypervisor-vendor=kvm use saner values as default, but letting old machine-types to override it for compatibility if required. Right now since I am using env-cpuid_hv_level == 0 as a flag. This means that: -cpu host,hypervisor-level=0,hypervisor-vendor=kvm -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 end up with different CPUID data (Which I do not like). I will fix this in the next round. Right. This has to be fixed. Did you want me to drop kvm0 and kvm1? Yes, if level is already configurable using the hypervisor-level property, I don't see the need for kvm0 and kvm1. If you make kvm_arch_init_vcpu() actually use those fields, you will end up implementing what's required to allow migration compatibility to be kept (the only thing missing is to make the CPU class a child of DeviceState, and add hypervisor-level=0 to the existing machine-types). :-) You mean like http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03402.html and http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03405.html already change kvm_arch_init_vcpu(). Yes! Sorry, I hadn't reviewed all of your previous series yet. :-) I did not know that I need to make the CPU class a child of DeviceState. You don't. But we plan to do that, eventually, so we can put CPU compatibility properties into machine-types. Nor that I needed to add hypervisor-vendor=kvm,hypervisor-level=0 to the existing machine-types. Since without specifying hypervisor-level=0 it defaults to 0 and kvm_arch_init_vcpu() will default to setting hypervisor-vendor=kvm. What I would like to do is: to change the default to 0x4001 (that's the correct value), but make the pc-1.1 and older machine-types keep the hypervisor-level=0 default for compatibility. (But to be able to do that, we need to first make the CPU class a child of DeviceState, so that's something to be done later) -- Eduardo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] target-i386: Fix default Hypervisor level for hypervisor-vendor=kvm.
On 09/21/12 17:53, Eduardo Habkost wrote: On Fri, Sep 21, 2012 at 05:28:27PM -0400, Don Slutz wrote: On 09/21/12 16:49, Eduardo Habkost wrote: On Fri, Sep 21, 2012 at 04:26:58PM -0400, Don Slutz wrote: On 09/21/12 10:18, Eduardo Habkost wrote: On Thu, Sep 20, 2012 at 04:06:27PM -0400, Don Slutz wrote: From http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html EAX should be KVM_CPUID_FEATURES (0x4001) not 0. Added hypervisor-vendor=kvm0 to get the older CPUID result. kvm1 selects the newer one. Why not just make hypervisor-vendor=kvm control only the hypervisor vendor string, and support something like kvm-hypervisor-level=0 to restore the old cpuid_hv_level=0 behavior? -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 Does this. Good. :-) This is similar to the kvmclock case: it would allow us to make hypervisor-vendor=kvm use saner values as default, but letting old machine-types to override it for compatibility if required. Right now since I am using env-cpuid_hv_level == 0 as a flag. This means that: -cpu host,hypervisor-level=0,hypervisor-vendor=kvm -cpu host,hypervisor-vendor=kvm,hypervisor-level=0 end up with different CPUID data (Which I do not like). I will fix this in the next round. Right. This has to be fixed. Did you want me to drop kvm0 and kvm1? Yes, if level is already configurable using the hypervisor-level property, I don't see the need for kvm0 and kvm1. If you make kvm_arch_init_vcpu() actually use those fields, you will end up implementing what's required to allow migration compatibility to be kept (the only thing missing is to make the CPU class a child of DeviceState, and add hypervisor-level=0 to the existing machine-types). :-) You mean like http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03402.html and http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03405.html already change kvm_arch_init_vcpu(). Yes! Sorry, I hadn't reviewed all of your previous series yet. :-) I did not know that I need to make the CPU class a child of DeviceState. You don't. But we plan to do that, eventually, so we can put CPU compatibility properties into machine-types. Nor that I needed to add hypervisor-vendor=kvm,hypervisor-level=0 to the existing machine-types. Since without specifying hypervisor-level=0 it defaults to 0 and kvm_arch_init_vcpu() will default to setting hypervisor-vendor=kvm. What I would like to do is: to change the default to 0x4001 (that's the correct value), but make the pc-1.1 and older machine-types keep the hypervisor-level=0 default for compatibility. (But to be able to do that, we need to first make the CPU class a child of DeviceState, so that's something to be done later) Now that ([PATCH v5 00/17] Allow changing...): http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03810.html is released, this contains all that is left of the patch set. So there will not be a v3. This patch set is withdrawn. -Don -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] target-i386: Fix default Hypervisor level for hypervisor-vendor=kvm.
From http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html
EAX should be KVM_CPUID_FEATURES (0x4001) not 0.
Added hypervisor-vendor=kvm0 to get the older CPUID result. kvm1 selects the
newer one.
Signed-off-by: Don Slutz d...@cloudswitch.com
---
target-i386/cpu.c | 17 +
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 72a8442..e51b2b1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1250,9 +1250,12 @@ static char *x86_cpuid_get_hv_vendor(Object *obj, Error
**errp)
} else if (!strcmp(value, CPUID_HV_VENDOR_XEN)
env-cpuid_hv_level == CPUID_HV_LEVEL_XEN) {
pstrcpy(value, sizeof(value), xen);
-} else if (!strcmp(value, CPUID_HV_VENDOR_KVM)
- env-cpuid_hv_level == 0) {
-pstrcpy(value, sizeof(value), kvm);
+} else if (!strcmp(value, CPUID_HV_VENDOR_KVM)) {
+if (env-cpuid_hv_level == CPUID_HV_LEVEL_KVM) {
+pstrcpy(value, sizeof(value), kvm1);
+} else if (env-cpuid_hv_level == 0) {
+pstrcpy(value, sizeof(value), kvm0);
+}
}
return value;
}
@@ -1288,7 +1291,13 @@ static void x86_cpuid_set_hv_vendor(Object *obj, const
char *value,
env-cpuid_hv_level = CPUID_HV_LEVEL_XEN;
}
pstrcpy(adj_value, sizeof(adj_value), CPUID_HV_VENDOR_XEN);
-} else if (!strcmp(value, kvm)) {
+} else if (!strcmp(value, kvm) || !strcmp(value, kvm1)) {
+if (env-cpuid_hv_level == 0) {
+env-cpuid_hv_level = CPUID_HV_LEVEL_KVM;
+}
+pstrcpy(adj_value, sizeof(adj_value), CPUID_HV_VENDOR_KVM);
+} else if (!strcmp(value, kvm0)) {
+env-cpuid_hv_level = 0;
pstrcpy(adj_value, sizeof(adj_value), CPUID_HV_VENDOR_KVM);
} else {
pstrcpy(adj_value, sizeof(adj_value), value);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
