* [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-07 15:13 ` Arthur Chunqi Li
0 siblings, 0 replies; 18+ messages in thread
From: Arthur Chunqi Li @ 2013-07-07 15:13 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: gleb, pbonzini, jan.kiszka, nyh, Arthur Chunqi Li
The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
to clear this MSR when reset vCPU and keep the value of it when
migration. This patch add this feature.
Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
---
target-i386/cpu.h | 2 ++
target-i386/kvm.c | 4 ++++
target-i386/machine.c | 22 ++++++++++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 62e3547..a418e17 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -301,6 +301,7 @@
#define MSR_IA32_APICBASE_BSP (1<<8)
#define MSR_IA32_APICBASE_ENABLE (1<<11)
#define MSR_IA32_APICBASE_BASE (0xfffff<<12)
+#define MSR_IA32_FEATURE_CONTROL 0x0000003a
#define MSR_TSC_ADJUST 0x0000003b
#define MSR_IA32_TSCDEADLINE 0x6e0
@@ -813,6 +814,7 @@ typedef struct CPUX86State {
uint64_t mcg_status;
uint64_t msr_ia32_misc_enable;
+ uint64_t msr_ia32_feature_control;
/* exception/interrupt handling */
int error_code;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 39f4fbb..3cb2161 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
if (hyperv_vapic_recommended()) {
kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
}
+ kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
}
if (env->mcg_cap) {
int i;
@@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
if (has_msr_misc_enable) {
msrs[n++].index = MSR_IA32_MISC_ENABLE;
}
+ msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
if (!env->tsc_valid) {
msrs[n++].index = MSR_IA32_TSC;
@@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
case MSR_IA32_MISC_ENABLE:
env->msr_ia32_misc_enable = msrs[i].data;
break;
+ case MSR_IA32_FEATURE_CONTROL:
+ env->msr_ia32_feature_control = msrs[i].data;
default:
if (msrs[i].index >= MSR_MC0_CTL &&
msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 3659db9..94ca914 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
}
+static bool feature_control_needed(void *opaque)
+{
+ X86CPU *cpu = opaque;
+ CPUX86State *env = &cpu->env;
+
+ return env->msr_ia32_feature_control != 0;
+}
+
static const VMStateDescription vmstate_msr_ia32_misc_enable = {
.name = "cpu/msr_ia32_misc_enable",
.version_id = 1,
@@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
}
};
+static const VMStateDescription vmstate_msr_ia32_feature_control = {
+ .name = "cpu/msr_ia32_feature_control",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField []) {
+ VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
const VMStateDescription vmstate_x86_cpu = {
.name = "cpu",
.version_id = 12,
@@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
}, {
.vmsd = &vmstate_msr_ia32_misc_enable,
.needed = misc_enable_needed,
+ }, {
+ .vmsd = &vmstate_msr_ia32_feature_control,
+ .needed = feature_control_needed,
} , {
/* empty */
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-07 15:13 ` Arthur Chunqi Li
0 siblings, 0 replies; 18+ messages in thread
From: Arthur Chunqi Li @ 2013-07-07 15:13 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, Arthur Chunqi Li, jan.kiszka, gleb, nyh
The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
to clear this MSR when reset vCPU and keep the value of it when
migration. This patch add this feature.
Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
---
target-i386/cpu.h | 2 ++
target-i386/kvm.c | 4 ++++
target-i386/machine.c | 22 ++++++++++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 62e3547..a418e17 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -301,6 +301,7 @@
#define MSR_IA32_APICBASE_BSP (1<<8)
#define MSR_IA32_APICBASE_ENABLE (1<<11)
#define MSR_IA32_APICBASE_BASE (0xfffff<<12)
+#define MSR_IA32_FEATURE_CONTROL 0x0000003a
#define MSR_TSC_ADJUST 0x0000003b
#define MSR_IA32_TSCDEADLINE 0x6e0
@@ -813,6 +814,7 @@ typedef struct CPUX86State {
uint64_t mcg_status;
uint64_t msr_ia32_misc_enable;
+ uint64_t msr_ia32_feature_control;
/* exception/interrupt handling */
int error_code;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 39f4fbb..3cb2161 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
if (hyperv_vapic_recommended()) {
kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
}
+ kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
}
if (env->mcg_cap) {
int i;
@@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
if (has_msr_misc_enable) {
msrs[n++].index = MSR_IA32_MISC_ENABLE;
}
+ msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
if (!env->tsc_valid) {
msrs[n++].index = MSR_IA32_TSC;
@@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
case MSR_IA32_MISC_ENABLE:
env->msr_ia32_misc_enable = msrs[i].data;
break;
+ case MSR_IA32_FEATURE_CONTROL:
+ env->msr_ia32_feature_control = msrs[i].data;
default:
if (msrs[i].index >= MSR_MC0_CTL &&
msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 3659db9..94ca914 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
}
+static bool feature_control_needed(void *opaque)
+{
+ X86CPU *cpu = opaque;
+ CPUX86State *env = &cpu->env;
+
+ return env->msr_ia32_feature_control != 0;
+}
+
static const VMStateDescription vmstate_msr_ia32_misc_enable = {
.name = "cpu/msr_ia32_misc_enable",
.version_id = 1,
@@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
}
};
+static const VMStateDescription vmstate_msr_ia32_feature_control = {
+ .name = "cpu/msr_ia32_feature_control",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField []) {
+ VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
const VMStateDescription vmstate_x86_cpu = {
.name = "cpu",
.version_id = 12,
@@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
}, {
.vmsd = &vmstate_msr_ia32_misc_enable,
.needed = misc_enable_needed,
+ }, {
+ .vmsd = &vmstate_msr_ia32_feature_control,
+ .needed = feature_control_needed,
} , {
/* empty */
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-07 15:13 ` [Qemu-devel] [ " Arthur Chunqi Li
@ 2013-07-15 5:44 ` Arthur Chunqi Li
-1 siblings, 0 replies; 18+ messages in thread
From: Arthur Chunqi Li @ 2013-07-15 5:44 UTC (permalink / raw)
To: kvm, qemu-devel
Cc: Paolo Bonzini, Arthur Chunqi Li, Jan Kiszka, Gleb Natapov, nyh
Hi Gleb and Paolo,
What is the status of this patch since the relevant patch for KVM is
accepted? These two patches must cooperate to fix the bug.
Arthur
On Sun, Jul 7, 2013 at 11:13 PM, Arthur Chunqi Li <yzt356@gmail.com> wrote:
> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> to clear this MSR when reset vCPU and keep the value of it when
> migration. This patch add this feature.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> target-i386/cpu.h | 2 ++
> target-i386/kvm.c | 4 ++++
> target-i386/machine.c | 22 ++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 62e3547..a418e17 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -301,6 +301,7 @@
> #define MSR_IA32_APICBASE_BSP (1<<8)
> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> #define MSR_TSC_ADJUST 0x0000003b
> #define MSR_IA32_TSCDEADLINE 0x6e0
>
> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>
> uint64_t mcg_status;
> uint64_t msr_ia32_misc_enable;
> + uint64_t msr_ia32_feature_control;
>
> /* exception/interrupt handling */
> int error_code;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 39f4fbb..3cb2161 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> if (hyperv_vapic_recommended()) {
> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> }
> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> }
> if (env->mcg_cap) {
> int i;
> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> if (has_msr_misc_enable) {
> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> }
> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>
> if (!env->tsc_valid) {
> msrs[n++].index = MSR_IA32_TSC;
> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> case MSR_IA32_MISC_ENABLE:
> env->msr_ia32_misc_enable = msrs[i].data;
> break;
> + case MSR_IA32_FEATURE_CONTROL:
> + env->msr_ia32_feature_control = msrs[i].data;
> default:
> if (msrs[i].index >= MSR_MC0_CTL &&
> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 3659db9..94ca914 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> }
>
> +static bool feature_control_needed(void *opaque)
> +{
> + X86CPU *cpu = opaque;
> + CPUX86State *env = &cpu->env;
> +
> + return env->msr_ia32_feature_control != 0;
> +}
> +
> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> .name = "cpu/msr_ia32_misc_enable",
> .version_id = 1,
> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> }
> };
>
> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> + .name = "cpu/msr_ia32_feature_control",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_x86_cpu = {
> .name = "cpu",
> .version_id = 12,
> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> }, {
> .vmsd = &vmstate_msr_ia32_misc_enable,
> .needed = misc_enable_needed,
> + }, {
> + .vmsd = &vmstate_msr_ia32_feature_control,
> + .needed = feature_control_needed,
> } , {
> /* empty */
> }
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-15 5:44 ` Arthur Chunqi Li
0 siblings, 0 replies; 18+ messages in thread
From: Arthur Chunqi Li @ 2013-07-15 5:44 UTC (permalink / raw)
To: kvm, qemu-devel
Cc: Paolo Bonzini, Arthur Chunqi Li, Jan Kiszka, Gleb Natapov, nyh
Hi Gleb and Paolo,
What is the status of this patch since the relevant patch for KVM is
accepted? These two patches must cooperate to fix the bug.
Arthur
On Sun, Jul 7, 2013 at 11:13 PM, Arthur Chunqi Li <yzt356@gmail.com> wrote:
> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> to clear this MSR when reset vCPU and keep the value of it when
> migration. This patch add this feature.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> target-i386/cpu.h | 2 ++
> target-i386/kvm.c | 4 ++++
> target-i386/machine.c | 22 ++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 62e3547..a418e17 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -301,6 +301,7 @@
> #define MSR_IA32_APICBASE_BSP (1<<8)
> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> #define MSR_TSC_ADJUST 0x0000003b
> #define MSR_IA32_TSCDEADLINE 0x6e0
>
> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>
> uint64_t mcg_status;
> uint64_t msr_ia32_misc_enable;
> + uint64_t msr_ia32_feature_control;
>
> /* exception/interrupt handling */
> int error_code;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 39f4fbb..3cb2161 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> if (hyperv_vapic_recommended()) {
> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> }
> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> }
> if (env->mcg_cap) {
> int i;
> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> if (has_msr_misc_enable) {
> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> }
> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>
> if (!env->tsc_valid) {
> msrs[n++].index = MSR_IA32_TSC;
> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> case MSR_IA32_MISC_ENABLE:
> env->msr_ia32_misc_enable = msrs[i].data;
> break;
> + case MSR_IA32_FEATURE_CONTROL:
> + env->msr_ia32_feature_control = msrs[i].data;
> default:
> if (msrs[i].index >= MSR_MC0_CTL &&
> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 3659db9..94ca914 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> }
>
> +static bool feature_control_needed(void *opaque)
> +{
> + X86CPU *cpu = opaque;
> + CPUX86State *env = &cpu->env;
> +
> + return env->msr_ia32_feature_control != 0;
> +}
> +
> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> .name = "cpu/msr_ia32_misc_enable",
> .version_id = 1,
> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> }
> };
>
> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> + .name = "cpu/msr_ia32_feature_control",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_x86_cpu = {
> .name = "cpu",
> .version_id = 12,
> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> }, {
> .vmsd = &vmstate_msr_ia32_misc_enable,
> .needed = misc_enable_needed,
> + }, {
> + .vmsd = &vmstate_msr_ia32_feature_control,
> + .needed = feature_control_needed,
> } , {
> /* empty */
> }
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-15 5:44 ` [Qemu-devel] " Arthur Chunqi Li
@ 2013-07-15 5:50 ` Gleb Natapov
-1 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-15 5:50 UTC (permalink / raw)
To: Arthur Chunqi Li
Cc: kvm, qemu-devel, Paolo Bonzini, Jan Kiszka, Juan Quintela,
Eduardo Habkost
On Mon, Jul 15, 2013 at 01:44:01PM +0800, Arthur Chunqi Li wrote:
> Hi Gleb and Paolo,
> What is the status of this patch since the relevant patch for KVM is
> accepted? These two patches must cooperate to fix the bug.
>
Need some reviews from migration and machine type experts. Copying Juan
and Eduardo.
> Arthur
>
> On Sun, Jul 7, 2013 at 11:13 PM, Arthur Chunqi Li <yzt356@gmail.com> wrote:
> > The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> > to clear this MSR when reset vCPU and keep the value of it when
> > migration. This patch add this feature.
> >
> > Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> > ---
> > target-i386/cpu.h | 2 ++
> > target-i386/kvm.c | 4 ++++
> > target-i386/machine.c | 22 ++++++++++++++++++++++
> > 3 files changed, 28 insertions(+)
> >
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 62e3547..a418e17 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -301,6 +301,7 @@
> > #define MSR_IA32_APICBASE_BSP (1<<8)
> > #define MSR_IA32_APICBASE_ENABLE (1<<11)
> > #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> > +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> > #define MSR_TSC_ADJUST 0x0000003b
> > #define MSR_IA32_TSCDEADLINE 0x6e0
> >
> > @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> >
> > uint64_t mcg_status;
> > uint64_t msr_ia32_misc_enable;
> > + uint64_t msr_ia32_feature_control;
> >
> > /* exception/interrupt handling */
> > int error_code;
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 39f4fbb..3cb2161 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> > if (hyperv_vapic_recommended()) {
> > kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> > }
> > + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> > }
> > if (env->mcg_cap) {
> > int i;
> > @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> > if (has_msr_misc_enable) {
> > msrs[n++].index = MSR_IA32_MISC_ENABLE;
> > }
> > + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> >
> > if (!env->tsc_valid) {
> > msrs[n++].index = MSR_IA32_TSC;
> > @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> > case MSR_IA32_MISC_ENABLE:
> > env->msr_ia32_misc_enable = msrs[i].data;
> > break;
> > + case MSR_IA32_FEATURE_CONTROL:
> > + env->msr_ia32_feature_control = msrs[i].data;
> > default:
> > if (msrs[i].index >= MSR_MC0_CTL &&
> > msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index 3659db9..94ca914 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> > return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> > }
> >
> > +static bool feature_control_needed(void *opaque)
> > +{
> > + X86CPU *cpu = opaque;
> > + CPUX86State *env = &cpu->env;
> > +
> > + return env->msr_ia32_feature_control != 0;
> > +}
> > +
> > static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > .name = "cpu/msr_ia32_misc_enable",
> > .version_id = 1,
> > @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > }
> > };
> >
> > +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> > + .name = "cpu/msr_ia32_feature_control",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .minimum_version_id_old = 1,
> > + .fields = (VMStateField []) {
> > + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > const VMStateDescription vmstate_x86_cpu = {
> > .name = "cpu",
> > .version_id = 12,
> > @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> > }, {
> > .vmsd = &vmstate_msr_ia32_misc_enable,
> > .needed = misc_enable_needed,
> > + }, {
> > + .vmsd = &vmstate_msr_ia32_feature_control,
> > + .needed = feature_control_needed,
> > } , {
> > /* empty */
> > }
> > --
> > 1.7.9.5
> >
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-15 5:50 ` Gleb Natapov
0 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-15 5:50 UTC (permalink / raw)
To: Arthur Chunqi Li
Cc: Eduardo Habkost, kvm, Juan Quintela, qemu-devel, Jan Kiszka,
Paolo Bonzini
On Mon, Jul 15, 2013 at 01:44:01PM +0800, Arthur Chunqi Li wrote:
> Hi Gleb and Paolo,
> What is the status of this patch since the relevant patch for KVM is
> accepted? These two patches must cooperate to fix the bug.
>
Need some reviews from migration and machine type experts. Copying Juan
and Eduardo.
> Arthur
>
> On Sun, Jul 7, 2013 at 11:13 PM, Arthur Chunqi Li <yzt356@gmail.com> wrote:
> > The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> > to clear this MSR when reset vCPU and keep the value of it when
> > migration. This patch add this feature.
> >
> > Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> > ---
> > target-i386/cpu.h | 2 ++
> > target-i386/kvm.c | 4 ++++
> > target-i386/machine.c | 22 ++++++++++++++++++++++
> > 3 files changed, 28 insertions(+)
> >
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 62e3547..a418e17 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -301,6 +301,7 @@
> > #define MSR_IA32_APICBASE_BSP (1<<8)
> > #define MSR_IA32_APICBASE_ENABLE (1<<11)
> > #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> > +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> > #define MSR_TSC_ADJUST 0x0000003b
> > #define MSR_IA32_TSCDEADLINE 0x6e0
> >
> > @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> >
> > uint64_t mcg_status;
> > uint64_t msr_ia32_misc_enable;
> > + uint64_t msr_ia32_feature_control;
> >
> > /* exception/interrupt handling */
> > int error_code;
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 39f4fbb..3cb2161 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> > if (hyperv_vapic_recommended()) {
> > kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> > }
> > + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> > }
> > if (env->mcg_cap) {
> > int i;
> > @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> > if (has_msr_misc_enable) {
> > msrs[n++].index = MSR_IA32_MISC_ENABLE;
> > }
> > + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> >
> > if (!env->tsc_valid) {
> > msrs[n++].index = MSR_IA32_TSC;
> > @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> > case MSR_IA32_MISC_ENABLE:
> > env->msr_ia32_misc_enable = msrs[i].data;
> > break;
> > + case MSR_IA32_FEATURE_CONTROL:
> > + env->msr_ia32_feature_control = msrs[i].data;
> > default:
> > if (msrs[i].index >= MSR_MC0_CTL &&
> > msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index 3659db9..94ca914 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> > return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> > }
> >
> > +static bool feature_control_needed(void *opaque)
> > +{
> > + X86CPU *cpu = opaque;
> > + CPUX86State *env = &cpu->env;
> > +
> > + return env->msr_ia32_feature_control != 0;
> > +}
> > +
> > static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > .name = "cpu/msr_ia32_misc_enable",
> > .version_id = 1,
> > @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > }
> > };
> >
> > +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> > + .name = "cpu/msr_ia32_feature_control",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .minimum_version_id_old = 1,
> > + .fields = (VMStateField []) {
> > + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > const VMStateDescription vmstate_x86_cpu = {
> > .name = "cpu",
> > .version_id = 12,
> > @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> > }, {
> > .vmsd = &vmstate_msr_ia32_misc_enable,
> > .needed = misc_enable_needed,
> > + }, {
> > + .vmsd = &vmstate_msr_ia32_feature_control,
> > + .needed = feature_control_needed,
> > } , {
> > /* empty */
> > }
> > --
> > 1.7.9.5
> >
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-07 15:13 ` [Qemu-devel] [ " Arthur Chunqi Li
@ 2013-07-16 11:42 ` Gleb Natapov
-1 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-16 11:42 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: kvm, qemu-devel, pbonzini, jan.kiszka, Juan Quintela
On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> to clear this MSR when reset vCPU and keep the value of it when
> migration. This patch add this feature.
>
So what happens if we migrate from qemu that does not have this patch
to qemu that does? Since msr_ia32_feature_control will not be migrated
it will not be set on the destination so destination will not be able to
use nested vmx. Since nested vmx is experimental it may be to early for
us to care about it though, and nested vmx does not work with migration
anyway.
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> target-i386/cpu.h | 2 ++
> target-i386/kvm.c | 4 ++++
> target-i386/machine.c | 22 ++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 62e3547..a418e17 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -301,6 +301,7 @@
> #define MSR_IA32_APICBASE_BSP (1<<8)
> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> #define MSR_TSC_ADJUST 0x0000003b
> #define MSR_IA32_TSCDEADLINE 0x6e0
>
> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>
> uint64_t mcg_status;
> uint64_t msr_ia32_misc_enable;
> + uint64_t msr_ia32_feature_control;
>
> /* exception/interrupt handling */
> int error_code;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 39f4fbb..3cb2161 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> if (hyperv_vapic_recommended()) {
> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> }
> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> }
> if (env->mcg_cap) {
> int i;
> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> if (has_msr_misc_enable) {
> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> }
> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>
> if (!env->tsc_valid) {
> msrs[n++].index = MSR_IA32_TSC;
> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> case MSR_IA32_MISC_ENABLE:
> env->msr_ia32_misc_enable = msrs[i].data;
> break;
> + case MSR_IA32_FEATURE_CONTROL:
> + env->msr_ia32_feature_control = msrs[i].data;
> default:
> if (msrs[i].index >= MSR_MC0_CTL &&
> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 3659db9..94ca914 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> }
>
> +static bool feature_control_needed(void *opaque)
> +{
> + X86CPU *cpu = opaque;
> + CPUX86State *env = &cpu->env;
> +
> + return env->msr_ia32_feature_control != 0;
> +}
> +
> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> .name = "cpu/msr_ia32_misc_enable",
> .version_id = 1,
> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> }
> };
>
> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> + .name = "cpu/msr_ia32_feature_control",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_x86_cpu = {
> .name = "cpu",
> .version_id = 12,
> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> }, {
> .vmsd = &vmstate_msr_ia32_misc_enable,
> .needed = misc_enable_needed,
> + }, {
> + .vmsd = &vmstate_msr_ia32_feature_control,
> + .needed = feature_control_needed,
> } , {
> /* empty */
> }
> --
> 1.7.9.5
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-16 11:42 ` Gleb Natapov
0 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-16 11:42 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: pbonzini, jan.kiszka, qemu-devel, kvm, Juan Quintela
On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> to clear this MSR when reset vCPU and keep the value of it when
> migration. This patch add this feature.
>
So what happens if we migrate from qemu that does not have this patch
to qemu that does? Since msr_ia32_feature_control will not be migrated
it will not be set on the destination so destination will not be able to
use nested vmx. Since nested vmx is experimental it may be to early for
us to care about it though, and nested vmx does not work with migration
anyway.
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> target-i386/cpu.h | 2 ++
> target-i386/kvm.c | 4 ++++
> target-i386/machine.c | 22 ++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 62e3547..a418e17 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -301,6 +301,7 @@
> #define MSR_IA32_APICBASE_BSP (1<<8)
> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> #define MSR_TSC_ADJUST 0x0000003b
> #define MSR_IA32_TSCDEADLINE 0x6e0
>
> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>
> uint64_t mcg_status;
> uint64_t msr_ia32_misc_enable;
> + uint64_t msr_ia32_feature_control;
>
> /* exception/interrupt handling */
> int error_code;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 39f4fbb..3cb2161 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> if (hyperv_vapic_recommended()) {
> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> }
> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> }
> if (env->mcg_cap) {
> int i;
> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> if (has_msr_misc_enable) {
> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> }
> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>
> if (!env->tsc_valid) {
> msrs[n++].index = MSR_IA32_TSC;
> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> case MSR_IA32_MISC_ENABLE:
> env->msr_ia32_misc_enable = msrs[i].data;
> break;
> + case MSR_IA32_FEATURE_CONTROL:
> + env->msr_ia32_feature_control = msrs[i].data;
> default:
> if (msrs[i].index >= MSR_MC0_CTL &&
> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 3659db9..94ca914 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> }
>
> +static bool feature_control_needed(void *opaque)
> +{
> + X86CPU *cpu = opaque;
> + CPUX86State *env = &cpu->env;
> +
> + return env->msr_ia32_feature_control != 0;
> +}
> +
> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> .name = "cpu/msr_ia32_misc_enable",
> .version_id = 1,
> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> }
> };
>
> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> + .name = "cpu/msr_ia32_feature_control",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_x86_cpu = {
> .name = "cpu",
> .version_id = 12,
> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> }, {
> .vmsd = &vmstate_msr_ia32_misc_enable,
> .needed = misc_enable_needed,
> + }, {
> + .vmsd = &vmstate_msr_ia32_feature_control,
> + .needed = feature_control_needed,
> } , {
> /* empty */
> }
> --
> 1.7.9.5
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-16 11:42 ` [Qemu-devel] [ " Gleb Natapov
@ 2013-07-16 11:56 ` Arthur Chunqi Li
-1 siblings, 0 replies; 18+ messages in thread
From: Arthur Chunqi Li @ 2013-07-16 11:56 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, qemu-devel, Paolo Bonzini, Jan Kiszka, Juan Quintela
On Tue, Jul 16, 2013 at 7:42 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
>> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
>> to clear this MSR when reset vCPU and keep the value of it when
>> migration. This patch add this feature.
>>
> So what happens if we migrate from qemu that does not have this patch
> to qemu that does? Since msr_ia32_feature_control will not be migrated
> it will not be set on the destination so destination will not be able to
> use nested vmx. Since nested vmx is experimental it may be to early for
> us to care about it though, and nested vmx does not work with migration
> anyway.
In my test, if migration doesn't care about msr_ia32_feature_control,
the value will be set to 0 in the destination VM and this may cause
some logical confusions, but the VMX running on it may not aware of
this (if migration nested vmx is supported in the future) because once
VMX initialized, it will not check this msr any more in normal cases.
This is also a complex problem since we don't know how many states
like this msr need to be migrated related to nested virt. If there're
a lot of states need migrating, it is better to reconstruct the
relevant codes. But now this patch is enough.
Besides, though migration is not supported in nested vmx, we should
keep the machine state consistent during migration. So this patch is
also meaningful.
Arthur
>
>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>> ---
>> target-i386/cpu.h | 2 ++
>> target-i386/kvm.c | 4 ++++
>> target-i386/machine.c | 22 ++++++++++++++++++++++
>> 3 files changed, 28 insertions(+)
>>
>> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> index 62e3547..a418e17 100644
>> --- a/target-i386/cpu.h
>> +++ b/target-i386/cpu.h
>> @@ -301,6 +301,7 @@
>> #define MSR_IA32_APICBASE_BSP (1<<8)
>> #define MSR_IA32_APICBASE_ENABLE (1<<11)
>> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
>> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
>> #define MSR_TSC_ADJUST 0x0000003b
>> #define MSR_IA32_TSCDEADLINE 0x6e0
>>
>> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>>
>> uint64_t mcg_status;
>> uint64_t msr_ia32_misc_enable;
>> + uint64_t msr_ia32_feature_control;
>>
>> /* exception/interrupt handling */
>> int error_code;
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 39f4fbb..3cb2161 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>> if (hyperv_vapic_recommended()) {
>> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
>> }
>> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
>> }
>> if (env->mcg_cap) {
>> int i;
>> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
>> if (has_msr_misc_enable) {
>> msrs[n++].index = MSR_IA32_MISC_ENABLE;
>> }
>> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>>
>> if (!env->tsc_valid) {
>> msrs[n++].index = MSR_IA32_TSC;
>> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
>> case MSR_IA32_MISC_ENABLE:
>> env->msr_ia32_misc_enable = msrs[i].data;
>> break;
>> + case MSR_IA32_FEATURE_CONTROL:
>> + env->msr_ia32_feature_control = msrs[i].data;
>> default:
>> if (msrs[i].index >= MSR_MC0_CTL &&
>> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
>> diff --git a/target-i386/machine.c b/target-i386/machine.c
>> index 3659db9..94ca914 100644
>> --- a/target-i386/machine.c
>> +++ b/target-i386/machine.c
>> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
>> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
>> }
>>
>> +static bool feature_control_needed(void *opaque)
>> +{
>> + X86CPU *cpu = opaque;
>> + CPUX86State *env = &cpu->env;
>> +
>> + return env->msr_ia32_feature_control != 0;
>> +}
>> +
>> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> .name = "cpu/msr_ia32_misc_enable",
>> .version_id = 1,
>> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> }
>> };
>>
>> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
>> + .name = "cpu/msr_ia32_feature_control",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField []) {
>> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> const VMStateDescription vmstate_x86_cpu = {
>> .name = "cpu",
>> .version_id = 12,
>> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
>> }, {
>> .vmsd = &vmstate_msr_ia32_misc_enable,
>> .needed = misc_enable_needed,
>> + }, {
>> + .vmsd = &vmstate_msr_ia32_feature_control,
>> + .needed = feature_control_needed,
>> } , {
>> /* empty */
>> }
>> --
>> 1.7.9.5
>
> --
> Gleb.
--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-16 11:56 ` Arthur Chunqi Li
0 siblings, 0 replies; 18+ messages in thread
From: Arthur Chunqi Li @ 2013-07-16 11:56 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Paolo Bonzini, Jan Kiszka, qemu-devel, kvm, Juan Quintela
On Tue, Jul 16, 2013 at 7:42 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
>> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
>> to clear this MSR when reset vCPU and keep the value of it when
>> migration. This patch add this feature.
>>
> So what happens if we migrate from qemu that does not have this patch
> to qemu that does? Since msr_ia32_feature_control will not be migrated
> it will not be set on the destination so destination will not be able to
> use nested vmx. Since nested vmx is experimental it may be to early for
> us to care about it though, and nested vmx does not work with migration
> anyway.
In my test, if migration doesn't care about msr_ia32_feature_control,
the value will be set to 0 in the destination VM and this may cause
some logical confusions, but the VMX running on it may not aware of
this (if migration nested vmx is supported in the future) because once
VMX initialized, it will not check this msr any more in normal cases.
This is also a complex problem since we don't know how many states
like this msr need to be migrated related to nested virt. If there're
a lot of states need migrating, it is better to reconstruct the
relevant codes. But now this patch is enough.
Besides, though migration is not supported in nested vmx, we should
keep the machine state consistent during migration. So this patch is
also meaningful.
Arthur
>
>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>> ---
>> target-i386/cpu.h | 2 ++
>> target-i386/kvm.c | 4 ++++
>> target-i386/machine.c | 22 ++++++++++++++++++++++
>> 3 files changed, 28 insertions(+)
>>
>> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> index 62e3547..a418e17 100644
>> --- a/target-i386/cpu.h
>> +++ b/target-i386/cpu.h
>> @@ -301,6 +301,7 @@
>> #define MSR_IA32_APICBASE_BSP (1<<8)
>> #define MSR_IA32_APICBASE_ENABLE (1<<11)
>> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
>> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
>> #define MSR_TSC_ADJUST 0x0000003b
>> #define MSR_IA32_TSCDEADLINE 0x6e0
>>
>> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>>
>> uint64_t mcg_status;
>> uint64_t msr_ia32_misc_enable;
>> + uint64_t msr_ia32_feature_control;
>>
>> /* exception/interrupt handling */
>> int error_code;
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 39f4fbb..3cb2161 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>> if (hyperv_vapic_recommended()) {
>> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
>> }
>> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
>> }
>> if (env->mcg_cap) {
>> int i;
>> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
>> if (has_msr_misc_enable) {
>> msrs[n++].index = MSR_IA32_MISC_ENABLE;
>> }
>> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>>
>> if (!env->tsc_valid) {
>> msrs[n++].index = MSR_IA32_TSC;
>> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
>> case MSR_IA32_MISC_ENABLE:
>> env->msr_ia32_misc_enable = msrs[i].data;
>> break;
>> + case MSR_IA32_FEATURE_CONTROL:
>> + env->msr_ia32_feature_control = msrs[i].data;
>> default:
>> if (msrs[i].index >= MSR_MC0_CTL &&
>> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
>> diff --git a/target-i386/machine.c b/target-i386/machine.c
>> index 3659db9..94ca914 100644
>> --- a/target-i386/machine.c
>> +++ b/target-i386/machine.c
>> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
>> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
>> }
>>
>> +static bool feature_control_needed(void *opaque)
>> +{
>> + X86CPU *cpu = opaque;
>> + CPUX86State *env = &cpu->env;
>> +
>> + return env->msr_ia32_feature_control != 0;
>> +}
>> +
>> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> .name = "cpu/msr_ia32_misc_enable",
>> .version_id = 1,
>> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> }
>> };
>>
>> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
>> + .name = "cpu/msr_ia32_feature_control",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField []) {
>> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> const VMStateDescription vmstate_x86_cpu = {
>> .name = "cpu",
>> .version_id = 12,
>> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
>> }, {
>> .vmsd = &vmstate_msr_ia32_misc_enable,
>> .needed = misc_enable_needed,
>> + }, {
>> + .vmsd = &vmstate_msr_ia32_feature_control,
>> + .needed = feature_control_needed,
>> } , {
>> /* empty */
>> }
>> --
>> 1.7.9.5
>
> --
> Gleb.
--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-16 11:42 ` [Qemu-devel] [ " Gleb Natapov
@ 2013-07-16 11:56 ` Paolo Bonzini
-1 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2013-07-16 11:56 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Arthur Chunqi Li, kvm, qemu-devel, jan.kiszka, Juan Quintela
Il 16/07/2013 13:42, Gleb Natapov ha scritto:
> On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
>> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
>> to clear this MSR when reset vCPU and keep the value of it when
>> migration. This patch add this feature.
>>
> So what happens if we migrate from qemu that does not have this patch
> to qemu that does? Since msr_ia32_feature_control will not be migrated
> it will not be set on the destination so destination will not be able to
> use nested vmx.
Yes, that's the same as with every subsection.
> Since nested vmx is experimental it may be to early for
> us to care about it though, and nested vmx does not work with migration
> anyway.
Exactly. The next time you start KVM, it will set the MSR to 5 again.
Paolo
>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>> ---
>> target-i386/cpu.h | 2 ++
>> target-i386/kvm.c | 4 ++++
>> target-i386/machine.c | 22 ++++++++++++++++++++++
>> 3 files changed, 28 insertions(+)
>>
>> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> index 62e3547..a418e17 100644
>> --- a/target-i386/cpu.h
>> +++ b/target-i386/cpu.h
>> @@ -301,6 +301,7 @@
>> #define MSR_IA32_APICBASE_BSP (1<<8)
>> #define MSR_IA32_APICBASE_ENABLE (1<<11)
>> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
>> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
>> #define MSR_TSC_ADJUST 0x0000003b
>> #define MSR_IA32_TSCDEADLINE 0x6e0
>>
>> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>>
>> uint64_t mcg_status;
>> uint64_t msr_ia32_misc_enable;
>> + uint64_t msr_ia32_feature_control;
>>
>> /* exception/interrupt handling */
>> int error_code;
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 39f4fbb..3cb2161 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>> if (hyperv_vapic_recommended()) {
>> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
>> }
>> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
>> }
>> if (env->mcg_cap) {
>> int i;
>> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
>> if (has_msr_misc_enable) {
>> msrs[n++].index = MSR_IA32_MISC_ENABLE;
>> }
>> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>>
>> if (!env->tsc_valid) {
>> msrs[n++].index = MSR_IA32_TSC;
>> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
>> case MSR_IA32_MISC_ENABLE:
>> env->msr_ia32_misc_enable = msrs[i].data;
>> break;
>> + case MSR_IA32_FEATURE_CONTROL:
>> + env->msr_ia32_feature_control = msrs[i].data;
>> default:
>> if (msrs[i].index >= MSR_MC0_CTL &&
>> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
>> diff --git a/target-i386/machine.c b/target-i386/machine.c
>> index 3659db9..94ca914 100644
>> --- a/target-i386/machine.c
>> +++ b/target-i386/machine.c
>> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
>> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
>> }
>>
>> +static bool feature_control_needed(void *opaque)
>> +{
>> + X86CPU *cpu = opaque;
>> + CPUX86State *env = &cpu->env;
>> +
>> + return env->msr_ia32_feature_control != 0;
>> +}
>> +
>> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> .name = "cpu/msr_ia32_misc_enable",
>> .version_id = 1,
>> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> }
>> };
>>
>> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
>> + .name = "cpu/msr_ia32_feature_control",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField []) {
>> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> const VMStateDescription vmstate_x86_cpu = {
>> .name = "cpu",
>> .version_id = 12,
>> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
>> }, {
>> .vmsd = &vmstate_msr_ia32_misc_enable,
>> .needed = misc_enable_needed,
>> + }, {
>> + .vmsd = &vmstate_msr_ia32_feature_control,
>> + .needed = feature_control_needed,
>> } , {
>> /* empty */
>> }
>> --
>> 1.7.9.5
>
> --
> Gleb.
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-16 11:56 ` Paolo Bonzini
0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2013-07-16 11:56 UTC (permalink / raw)
To: Gleb Natapov; +Cc: jan.kiszka, Arthur Chunqi Li, qemu-devel, kvm, Juan Quintela
Il 16/07/2013 13:42, Gleb Natapov ha scritto:
> On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
>> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
>> to clear this MSR when reset vCPU and keep the value of it when
>> migration. This patch add this feature.
>>
> So what happens if we migrate from qemu that does not have this patch
> to qemu that does? Since msr_ia32_feature_control will not be migrated
> it will not be set on the destination so destination will not be able to
> use nested vmx.
Yes, that's the same as with every subsection.
> Since nested vmx is experimental it may be to early for
> us to care about it though, and nested vmx does not work with migration
> anyway.
Exactly. The next time you start KVM, it will set the MSR to 5 again.
Paolo
>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>> ---
>> target-i386/cpu.h | 2 ++
>> target-i386/kvm.c | 4 ++++
>> target-i386/machine.c | 22 ++++++++++++++++++++++
>> 3 files changed, 28 insertions(+)
>>
>> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> index 62e3547..a418e17 100644
>> --- a/target-i386/cpu.h
>> +++ b/target-i386/cpu.h
>> @@ -301,6 +301,7 @@
>> #define MSR_IA32_APICBASE_BSP (1<<8)
>> #define MSR_IA32_APICBASE_ENABLE (1<<11)
>> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
>> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
>> #define MSR_TSC_ADJUST 0x0000003b
>> #define MSR_IA32_TSCDEADLINE 0x6e0
>>
>> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>>
>> uint64_t mcg_status;
>> uint64_t msr_ia32_misc_enable;
>> + uint64_t msr_ia32_feature_control;
>>
>> /* exception/interrupt handling */
>> int error_code;
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 39f4fbb..3cb2161 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>> if (hyperv_vapic_recommended()) {
>> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
>> }
>> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
>> }
>> if (env->mcg_cap) {
>> int i;
>> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
>> if (has_msr_misc_enable) {
>> msrs[n++].index = MSR_IA32_MISC_ENABLE;
>> }
>> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>>
>> if (!env->tsc_valid) {
>> msrs[n++].index = MSR_IA32_TSC;
>> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
>> case MSR_IA32_MISC_ENABLE:
>> env->msr_ia32_misc_enable = msrs[i].data;
>> break;
>> + case MSR_IA32_FEATURE_CONTROL:
>> + env->msr_ia32_feature_control = msrs[i].data;
>> default:
>> if (msrs[i].index >= MSR_MC0_CTL &&
>> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
>> diff --git a/target-i386/machine.c b/target-i386/machine.c
>> index 3659db9..94ca914 100644
>> --- a/target-i386/machine.c
>> +++ b/target-i386/machine.c
>> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
>> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
>> }
>>
>> +static bool feature_control_needed(void *opaque)
>> +{
>> + X86CPU *cpu = opaque;
>> + CPUX86State *env = &cpu->env;
>> +
>> + return env->msr_ia32_feature_control != 0;
>> +}
>> +
>> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> .name = "cpu/msr_ia32_misc_enable",
>> .version_id = 1,
>> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>> }
>> };
>>
>> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
>> + .name = "cpu/msr_ia32_feature_control",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField []) {
>> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> const VMStateDescription vmstate_x86_cpu = {
>> .name = "cpu",
>> .version_id = 12,
>> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
>> }, {
>> .vmsd = &vmstate_msr_ia32_misc_enable,
>> .needed = misc_enable_needed,
>> + }, {
>> + .vmsd = &vmstate_msr_ia32_feature_control,
>> + .needed = feature_control_needed,
>> } , {
>> /* empty */
>> }
>> --
>> 1.7.9.5
>
> --
> Gleb.
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-16 11:56 ` [Qemu-devel] [ " Arthur Chunqi Li
@ 2013-07-16 12:01 ` Gleb Natapov
-1 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-16 12:01 UTC (permalink / raw)
To: Arthur Chunqi Li
Cc: kvm, qemu-devel, Paolo Bonzini, Jan Kiszka, Juan Quintela
On Tue, Jul 16, 2013 at 07:56:25PM +0800, Arthur Chunqi Li wrote:
> On Tue, Jul 16, 2013 at 7:42 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> >> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> >> to clear this MSR when reset vCPU and keep the value of it when
> >> migration. This patch add this feature.
> >>
> > So what happens if we migrate from qemu that does not have this patch
> > to qemu that does? Since msr_ia32_feature_control will not be migrated
> > it will not be set on the destination so destination will not be able to
> > use nested vmx. Since nested vmx is experimental it may be to early for
> > us to care about it though, and nested vmx does not work with migration
> > anyway.
> In my test, if migration doesn't care about msr_ia32_feature_control,
> the value will be set to 0 in the destination VM and this may cause
> some logical confusions, but the VMX running on it may not aware of
> this (if migration nested vmx is supported in the future) because once
> VMX initialized, it will not check this msr any more in normal cases.
>
With vmm_exclusive=0 kvm does vmxon/vmxoff while running. But lest not
worry about nested kvm migration for now. There are much harder problems
to overcome before it will work.
> This is also a complex problem since we don't know how many states
> like this msr need to be migrated related to nested virt. If there're
> a lot of states need migrating, it is better to reconstruct the
> relevant codes. But now this patch is enough.
>
> Besides, though migration is not supported in nested vmx, we should
> keep the machine state consistent during migration. So this patch is
> also meaningful.
>
> Arthur
> >
> >> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> >> ---
> >> target-i386/cpu.h | 2 ++
> >> target-i386/kvm.c | 4 ++++
> >> target-i386/machine.c | 22 ++++++++++++++++++++++
> >> 3 files changed, 28 insertions(+)
> >>
> >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> >> index 62e3547..a418e17 100644
> >> --- a/target-i386/cpu.h
> >> +++ b/target-i386/cpu.h
> >> @@ -301,6 +301,7 @@
> >> #define MSR_IA32_APICBASE_BSP (1<<8)
> >> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> >> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> >> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> >> #define MSR_TSC_ADJUST 0x0000003b
> >> #define MSR_IA32_TSCDEADLINE 0x6e0
> >>
> >> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> >>
> >> uint64_t mcg_status;
> >> uint64_t msr_ia32_misc_enable;
> >> + uint64_t msr_ia32_feature_control;
> >>
> >> /* exception/interrupt handling */
> >> int error_code;
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index 39f4fbb..3cb2161 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >> if (hyperv_vapic_recommended()) {
> >> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> >> }
> >> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> >> }
> >> if (env->mcg_cap) {
> >> int i;
> >> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> >> if (has_msr_misc_enable) {
> >> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> >> }
> >> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> >>
> >> if (!env->tsc_valid) {
> >> msrs[n++].index = MSR_IA32_TSC;
> >> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> >> case MSR_IA32_MISC_ENABLE:
> >> env->msr_ia32_misc_enable = msrs[i].data;
> >> break;
> >> + case MSR_IA32_FEATURE_CONTROL:
> >> + env->msr_ia32_feature_control = msrs[i].data;
> >> default:
> >> if (msrs[i].index >= MSR_MC0_CTL &&
> >> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> >> diff --git a/target-i386/machine.c b/target-i386/machine.c
> >> index 3659db9..94ca914 100644
> >> --- a/target-i386/machine.c
> >> +++ b/target-i386/machine.c
> >> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> >> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> >> }
> >>
> >> +static bool feature_control_needed(void *opaque)
> >> +{
> >> + X86CPU *cpu = opaque;
> >> + CPUX86State *env = &cpu->env;
> >> +
> >> + return env->msr_ia32_feature_control != 0;
> >> +}
> >> +
> >> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> >> .name = "cpu/msr_ia32_misc_enable",
> >> .version_id = 1,
> >> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> >> }
> >> };
> >>
> >> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> >> + .name = "cpu/msr_ia32_feature_control",
> >> + .version_id = 1,
> >> + .minimum_version_id = 1,
> >> + .minimum_version_id_old = 1,
> >> + .fields = (VMStateField []) {
> >> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> >> + VMSTATE_END_OF_LIST()
> >> + }
> >> +};
> >> +
> >> const VMStateDescription vmstate_x86_cpu = {
> >> .name = "cpu",
> >> .version_id = 12,
> >> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> >> }, {
> >> .vmsd = &vmstate_msr_ia32_misc_enable,
> >> .needed = misc_enable_needed,
> >> + }, {
> >> + .vmsd = &vmstate_msr_ia32_feature_control,
> >> + .needed = feature_control_needed,
> >> } , {
> >> /* empty */
> >> }
> >> --
> >> 1.7.9.5
> >
> > --
> > Gleb.
>
>
>
> --
> Arthur Chunqi Li
> Department of Computer Science
> School of EECS
> Peking University
> Beijing, China
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-16 12:01 ` Gleb Natapov
0 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-16 12:01 UTC (permalink / raw)
To: Arthur Chunqi Li
Cc: Paolo Bonzini, Jan Kiszka, qemu-devel, kvm, Juan Quintela
On Tue, Jul 16, 2013 at 07:56:25PM +0800, Arthur Chunqi Li wrote:
> On Tue, Jul 16, 2013 at 7:42 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> >> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> >> to clear this MSR when reset vCPU and keep the value of it when
> >> migration. This patch add this feature.
> >>
> > So what happens if we migrate from qemu that does not have this patch
> > to qemu that does? Since msr_ia32_feature_control will not be migrated
> > it will not be set on the destination so destination will not be able to
> > use nested vmx. Since nested vmx is experimental it may be to early for
> > us to care about it though, and nested vmx does not work with migration
> > anyway.
> In my test, if migration doesn't care about msr_ia32_feature_control,
> the value will be set to 0 in the destination VM and this may cause
> some logical confusions, but the VMX running on it may not aware of
> this (if migration nested vmx is supported in the future) because once
> VMX initialized, it will not check this msr any more in normal cases.
>
With vmm_exclusive=0 kvm does vmxon/vmxoff while running. But lest not
worry about nested kvm migration for now. There are much harder problems
to overcome before it will work.
> This is also a complex problem since we don't know how many states
> like this msr need to be migrated related to nested virt. If there're
> a lot of states need migrating, it is better to reconstruct the
> relevant codes. But now this patch is enough.
>
> Besides, though migration is not supported in nested vmx, we should
> keep the machine state consistent during migration. So this patch is
> also meaningful.
>
> Arthur
> >
> >> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> >> ---
> >> target-i386/cpu.h | 2 ++
> >> target-i386/kvm.c | 4 ++++
> >> target-i386/machine.c | 22 ++++++++++++++++++++++
> >> 3 files changed, 28 insertions(+)
> >>
> >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> >> index 62e3547..a418e17 100644
> >> --- a/target-i386/cpu.h
> >> +++ b/target-i386/cpu.h
> >> @@ -301,6 +301,7 @@
> >> #define MSR_IA32_APICBASE_BSP (1<<8)
> >> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> >> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> >> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> >> #define MSR_TSC_ADJUST 0x0000003b
> >> #define MSR_IA32_TSCDEADLINE 0x6e0
> >>
> >> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> >>
> >> uint64_t mcg_status;
> >> uint64_t msr_ia32_misc_enable;
> >> + uint64_t msr_ia32_feature_control;
> >>
> >> /* exception/interrupt handling */
> >> int error_code;
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index 39f4fbb..3cb2161 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >> if (hyperv_vapic_recommended()) {
> >> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> >> }
> >> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> >> }
> >> if (env->mcg_cap) {
> >> int i;
> >> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> >> if (has_msr_misc_enable) {
> >> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> >> }
> >> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> >>
> >> if (!env->tsc_valid) {
> >> msrs[n++].index = MSR_IA32_TSC;
> >> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> >> case MSR_IA32_MISC_ENABLE:
> >> env->msr_ia32_misc_enable = msrs[i].data;
> >> break;
> >> + case MSR_IA32_FEATURE_CONTROL:
> >> + env->msr_ia32_feature_control = msrs[i].data;
> >> default:
> >> if (msrs[i].index >= MSR_MC0_CTL &&
> >> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> >> diff --git a/target-i386/machine.c b/target-i386/machine.c
> >> index 3659db9..94ca914 100644
> >> --- a/target-i386/machine.c
> >> +++ b/target-i386/machine.c
> >> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> >> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> >> }
> >>
> >> +static bool feature_control_needed(void *opaque)
> >> +{
> >> + X86CPU *cpu = opaque;
> >> + CPUX86State *env = &cpu->env;
> >> +
> >> + return env->msr_ia32_feature_control != 0;
> >> +}
> >> +
> >> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> >> .name = "cpu/msr_ia32_misc_enable",
> >> .version_id = 1,
> >> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> >> }
> >> };
> >>
> >> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> >> + .name = "cpu/msr_ia32_feature_control",
> >> + .version_id = 1,
> >> + .minimum_version_id = 1,
> >> + .minimum_version_id_old = 1,
> >> + .fields = (VMStateField []) {
> >> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> >> + VMSTATE_END_OF_LIST()
> >> + }
> >> +};
> >> +
> >> const VMStateDescription vmstate_x86_cpu = {
> >> .name = "cpu",
> >> .version_id = 12,
> >> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> >> }, {
> >> .vmsd = &vmstate_msr_ia32_misc_enable,
> >> .needed = misc_enable_needed,
> >> + }, {
> >> + .vmsd = &vmstate_msr_ia32_feature_control,
> >> + .needed = feature_control_needed,
> >> } , {
> >> /* empty */
> >> }
> >> --
> >> 1.7.9.5
> >
> > --
> > Gleb.
>
>
>
> --
> Arthur Chunqi Li
> Department of Computer Science
> School of EECS
> Peking University
> Beijing, China
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-16 12:01 ` [Qemu-devel] [ " Gleb Natapov
@ 2013-07-18 12:42 ` Eduardo Habkost
-1 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2013-07-18 12:42 UTC (permalink / raw)
To: Gleb Natapov
Cc: Arthur Chunqi Li, Paolo Bonzini, Jan Kiszka, qemu-devel, kvm,
Juan Quintela
On Tue, Jul 16, 2013 at 03:01:58PM +0300, Gleb Natapov wrote:
> On Tue, Jul 16, 2013 at 07:56:25PM +0800, Arthur Chunqi Li wrote:
> > On Tue, Jul 16, 2013 at 7:42 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > > On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> > >> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> > >> to clear this MSR when reset vCPU and keep the value of it when
> > >> migration. This patch add this feature.
> > >>
> > > So what happens if we migrate from qemu that does not have this patch
> > > to qemu that does? Since msr_ia32_feature_control will not be migrated
> > > it will not be set on the destination so destination will not be able to
> > > use nested vmx. Since nested vmx is experimental it may be to early for
> > > us to care about it though, and nested vmx does not work with migration
> > > anyway.
> > In my test, if migration doesn't care about msr_ia32_feature_control,
> > the value will be set to 0 in the destination VM and this may cause
> > some logical confusions, but the VMX running on it may not aware of
> > this (if migration nested vmx is supported in the future) because once
> > VMX initialized, it will not check this msr any more in normal cases.
> >
> With vmm_exclusive=0 kvm does vmxon/vmxoff while running. But lest not
> worry about nested kvm migration for now. There are much harder problems
> to overcome before it will work.
>
> > This is also a complex problem since we don't know how many states
> > like this msr need to be migrated related to nested virt. If there're
> > a lot of states need migrating, it is better to reconstruct the
> > relevant codes. But now this patch is enough.
> >
> > Besides, though migration is not supported in nested vmx, we should
> > keep the machine state consistent during migration. So this patch is
> > also meaningful.
I'm assuming that even "qemu-1.6 -machine pc-1.5" is not expected to
allow migration to a qemu-1.5 binary. Is that OK for everybody, or
should we support backwards migration?
Other than that, the patch looks good to me. If migrating from a version
that doesn't have the patch, we are just going to get the same behavior
we had before.
> >
> > Arthur
> > >
> > >> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> > >> ---
> > >> target-i386/cpu.h | 2 ++
> > >> target-i386/kvm.c | 4 ++++
> > >> target-i386/machine.c | 22 ++++++++++++++++++++++
> > >> 3 files changed, 28 insertions(+)
> > >>
> > >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > >> index 62e3547..a418e17 100644
> > >> --- a/target-i386/cpu.h
> > >> +++ b/target-i386/cpu.h
> > >> @@ -301,6 +301,7 @@
> > >> #define MSR_IA32_APICBASE_BSP (1<<8)
> > >> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> > >> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> > >> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> > >> #define MSR_TSC_ADJUST 0x0000003b
> > >> #define MSR_IA32_TSCDEADLINE 0x6e0
> > >>
> > >> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> > >>
> > >> uint64_t mcg_status;
> > >> uint64_t msr_ia32_misc_enable;
> > >> + uint64_t msr_ia32_feature_control;
> > >>
> > >> /* exception/interrupt handling */
> > >> int error_code;
> > >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > >> index 39f4fbb..3cb2161 100644
> > >> --- a/target-i386/kvm.c
> > >> +++ b/target-i386/kvm.c
> > >> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> > >> if (hyperv_vapic_recommended()) {
> > >> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> > >> }
> > >> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> > >> }
> > >> if (env->mcg_cap) {
> > >> int i;
> > >> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> > >> if (has_msr_misc_enable) {
> > >> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> > >> }
> > >> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> > >>
> > >> if (!env->tsc_valid) {
> > >> msrs[n++].index = MSR_IA32_TSC;
> > >> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> > >> case MSR_IA32_MISC_ENABLE:
> > >> env->msr_ia32_misc_enable = msrs[i].data;
> > >> break;
> > >> + case MSR_IA32_FEATURE_CONTROL:
> > >> + env->msr_ia32_feature_control = msrs[i].data;
> > >> default:
> > >> if (msrs[i].index >= MSR_MC0_CTL &&
> > >> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> > >> diff --git a/target-i386/machine.c b/target-i386/machine.c
> > >> index 3659db9..94ca914 100644
> > >> --- a/target-i386/machine.c
> > >> +++ b/target-i386/machine.c
> > >> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> > >> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> > >> }
> > >>
> > >> +static bool feature_control_needed(void *opaque)
> > >> +{
> > >> + X86CPU *cpu = opaque;
> > >> + CPUX86State *env = &cpu->env;
> > >> +
> > >> + return env->msr_ia32_feature_control != 0;
> > >> +}
> > >> +
> > >> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > >> .name = "cpu/msr_ia32_misc_enable",
> > >> .version_id = 1,
> > >> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > >> }
> > >> };
> > >>
> > >> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> > >> + .name = "cpu/msr_ia32_feature_control",
> > >> + .version_id = 1,
> > >> + .minimum_version_id = 1,
> > >> + .minimum_version_id_old = 1,
> > >> + .fields = (VMStateField []) {
> > >> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> > >> + VMSTATE_END_OF_LIST()
> > >> + }
> > >> +};
> > >> +
> > >> const VMStateDescription vmstate_x86_cpu = {
> > >> .name = "cpu",
> > >> .version_id = 12,
> > >> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> > >> }, {
> > >> .vmsd = &vmstate_msr_ia32_misc_enable,
> > >> .needed = misc_enable_needed,
> > >> + }, {
> > >> + .vmsd = &vmstate_msr_ia32_feature_control,
> > >> + .needed = feature_control_needed,
> > >> } , {
> > >> /* empty */
> > >> }
> > >> --
> > >> 1.7.9.5
> > >
> > > --
> > > Gleb.
> >
> >
> >
> > --
> > Arthur Chunqi Li
> > Department of Computer Science
> > School of EECS
> > Peking University
> > Beijing, China
>
> --
> Gleb.
>
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-18 12:42 ` Eduardo Habkost
0 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2013-07-18 12:42 UTC (permalink / raw)
To: Gleb Natapov
Cc: kvm, Juan Quintela, Arthur Chunqi Li, qemu-devel, Jan Kiszka,
Paolo Bonzini
On Tue, Jul 16, 2013 at 03:01:58PM +0300, Gleb Natapov wrote:
> On Tue, Jul 16, 2013 at 07:56:25PM +0800, Arthur Chunqi Li wrote:
> > On Tue, Jul 16, 2013 at 7:42 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > > On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> > >> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> > >> to clear this MSR when reset vCPU and keep the value of it when
> > >> migration. This patch add this feature.
> > >>
> > > So what happens if we migrate from qemu that does not have this patch
> > > to qemu that does? Since msr_ia32_feature_control will not be migrated
> > > it will not be set on the destination so destination will not be able to
> > > use nested vmx. Since nested vmx is experimental it may be to early for
> > > us to care about it though, and nested vmx does not work with migration
> > > anyway.
> > In my test, if migration doesn't care about msr_ia32_feature_control,
> > the value will be set to 0 in the destination VM and this may cause
> > some logical confusions, but the VMX running on it may not aware of
> > this (if migration nested vmx is supported in the future) because once
> > VMX initialized, it will not check this msr any more in normal cases.
> >
> With vmm_exclusive=0 kvm does vmxon/vmxoff while running. But lest not
> worry about nested kvm migration for now. There are much harder problems
> to overcome before it will work.
>
> > This is also a complex problem since we don't know how many states
> > like this msr need to be migrated related to nested virt. If there're
> > a lot of states need migrating, it is better to reconstruct the
> > relevant codes. But now this patch is enough.
> >
> > Besides, though migration is not supported in nested vmx, we should
> > keep the machine state consistent during migration. So this patch is
> > also meaningful.
I'm assuming that even "qemu-1.6 -machine pc-1.5" is not expected to
allow migration to a qemu-1.5 binary. Is that OK for everybody, or
should we support backwards migration?
Other than that, the patch looks good to me. If migrating from a version
that doesn't have the patch, we are just going to get the same behavior
we had before.
> >
> > Arthur
> > >
> > >> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> > >> ---
> > >> target-i386/cpu.h | 2 ++
> > >> target-i386/kvm.c | 4 ++++
> > >> target-i386/machine.c | 22 ++++++++++++++++++++++
> > >> 3 files changed, 28 insertions(+)
> > >>
> > >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > >> index 62e3547..a418e17 100644
> > >> --- a/target-i386/cpu.h
> > >> +++ b/target-i386/cpu.h
> > >> @@ -301,6 +301,7 @@
> > >> #define MSR_IA32_APICBASE_BSP (1<<8)
> > >> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> > >> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> > >> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> > >> #define MSR_TSC_ADJUST 0x0000003b
> > >> #define MSR_IA32_TSCDEADLINE 0x6e0
> > >>
> > >> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> > >>
> > >> uint64_t mcg_status;
> > >> uint64_t msr_ia32_misc_enable;
> > >> + uint64_t msr_ia32_feature_control;
> > >>
> > >> /* exception/interrupt handling */
> > >> int error_code;
> > >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > >> index 39f4fbb..3cb2161 100644
> > >> --- a/target-i386/kvm.c
> > >> +++ b/target-i386/kvm.c
> > >> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> > >> if (hyperv_vapic_recommended()) {
> > >> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> > >> }
> > >> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> > >> }
> > >> if (env->mcg_cap) {
> > >> int i;
> > >> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> > >> if (has_msr_misc_enable) {
> > >> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> > >> }
> > >> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> > >>
> > >> if (!env->tsc_valid) {
> > >> msrs[n++].index = MSR_IA32_TSC;
> > >> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> > >> case MSR_IA32_MISC_ENABLE:
> > >> env->msr_ia32_misc_enable = msrs[i].data;
> > >> break;
> > >> + case MSR_IA32_FEATURE_CONTROL:
> > >> + env->msr_ia32_feature_control = msrs[i].data;
> > >> default:
> > >> if (msrs[i].index >= MSR_MC0_CTL &&
> > >> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> > >> diff --git a/target-i386/machine.c b/target-i386/machine.c
> > >> index 3659db9..94ca914 100644
> > >> --- a/target-i386/machine.c
> > >> +++ b/target-i386/machine.c
> > >> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> > >> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> > >> }
> > >>
> > >> +static bool feature_control_needed(void *opaque)
> > >> +{
> > >> + X86CPU *cpu = opaque;
> > >> + CPUX86State *env = &cpu->env;
> > >> +
> > >> + return env->msr_ia32_feature_control != 0;
> > >> +}
> > >> +
> > >> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > >> .name = "cpu/msr_ia32_misc_enable",
> > >> .version_id = 1,
> > >> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> > >> }
> > >> };
> > >>
> > >> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> > >> + .name = "cpu/msr_ia32_feature_control",
> > >> + .version_id = 1,
> > >> + .minimum_version_id = 1,
> > >> + .minimum_version_id_old = 1,
> > >> + .fields = (VMStateField []) {
> > >> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> > >> + VMSTATE_END_OF_LIST()
> > >> + }
> > >> +};
> > >> +
> > >> const VMStateDescription vmstate_x86_cpu = {
> > >> .name = "cpu",
> > >> .version_id = 12,
> > >> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> > >> }, {
> > >> .vmsd = &vmstate_msr_ia32_misc_enable,
> > >> .needed = misc_enable_needed,
> > >> + }, {
> > >> + .vmsd = &vmstate_msr_ia32_feature_control,
> > >> + .needed = feature_control_needed,
> > >> } , {
> > >> /* empty */
> > >> }
> > >> --
> > >> 1.7.9.5
> > >
> > > --
> > > Gleb.
> >
> >
> >
> > --
> > Arthur Chunqi Li
> > Department of Computer Science
> > School of EECS
> > Peking University
> > Beijing, China
>
> --
> Gleb.
>
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
2013-07-07 15:13 ` [Qemu-devel] [ " Arthur Chunqi Li
@ 2013-07-25 10:09 ` Gleb Natapov
-1 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-25 10:09 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: kvm, qemu-devel, pbonzini, jan.kiszka, nyh
On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> to clear this MSR when reset vCPU and keep the value of it when
> migration. This patch add this feature.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
Applied, thanks.
> ---
> target-i386/cpu.h | 2 ++
> target-i386/kvm.c | 4 ++++
> target-i386/machine.c | 22 ++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 62e3547..a418e17 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -301,6 +301,7 @@
> #define MSR_IA32_APICBASE_BSP (1<<8)
> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> #define MSR_TSC_ADJUST 0x0000003b
> #define MSR_IA32_TSCDEADLINE 0x6e0
>
> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>
> uint64_t mcg_status;
> uint64_t msr_ia32_misc_enable;
> + uint64_t msr_ia32_feature_control;
>
> /* exception/interrupt handling */
> int error_code;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 39f4fbb..3cb2161 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> if (hyperv_vapic_recommended()) {
> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> }
> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> }
> if (env->mcg_cap) {
> int i;
> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> if (has_msr_misc_enable) {
> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> }
> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>
> if (!env->tsc_valid) {
> msrs[n++].index = MSR_IA32_TSC;
> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> case MSR_IA32_MISC_ENABLE:
> env->msr_ia32_misc_enable = msrs[i].data;
> break;
> + case MSR_IA32_FEATURE_CONTROL:
> + env->msr_ia32_feature_control = msrs[i].data;
> default:
> if (msrs[i].index >= MSR_MC0_CTL &&
> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 3659db9..94ca914 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> }
>
> +static bool feature_control_needed(void *opaque)
> +{
> + X86CPU *cpu = opaque;
> + CPUX86State *env = &cpu->env;
> +
> + return env->msr_ia32_feature_control != 0;
> +}
> +
> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> .name = "cpu/msr_ia32_misc_enable",
> .version_id = 1,
> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> }
> };
>
> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> + .name = "cpu/msr_ia32_feature_control",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_x86_cpu = {
> .name = "cpu",
> .version_id = 12,
> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> }, {
> .vmsd = &vmstate_msr_ia32_misc_enable,
> .needed = misc_enable_needed,
> + }, {
> + .vmsd = &vmstate_msr_ia32_feature_control,
> + .needed = feature_control_needed,
> } , {
> /* empty */
> }
> --
> 1.7.9.5
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration
@ 2013-07-25 10:09 ` Gleb Natapov
0 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2013-07-25 10:09 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: pbonzini, jan.kiszka, qemu-devel, kvm, nyh
On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> to clear this MSR when reset vCPU and keep the value of it when
> migration. This patch add this feature.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
Applied, thanks.
> ---
> target-i386/cpu.h | 2 ++
> target-i386/kvm.c | 4 ++++
> target-i386/machine.c | 22 ++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 62e3547..a418e17 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -301,6 +301,7 @@
> #define MSR_IA32_APICBASE_BSP (1<<8)
> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> #define MSR_TSC_ADJUST 0x0000003b
> #define MSR_IA32_TSCDEADLINE 0x6e0
>
> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
>
> uint64_t mcg_status;
> uint64_t msr_ia32_misc_enable;
> + uint64_t msr_ia32_feature_control;
>
> /* exception/interrupt handling */
> int error_code;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 39f4fbb..3cb2161 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> if (hyperv_vapic_recommended()) {
> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> }
> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> }
> if (env->mcg_cap) {
> int i;
> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> if (has_msr_misc_enable) {
> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> }
> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
>
> if (!env->tsc_valid) {
> msrs[n++].index = MSR_IA32_TSC;
> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> case MSR_IA32_MISC_ENABLE:
> env->msr_ia32_misc_enable = msrs[i].data;
> break;
> + case MSR_IA32_FEATURE_CONTROL:
> + env->msr_ia32_feature_control = msrs[i].data;
> default:
> if (msrs[i].index >= MSR_MC0_CTL &&
> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 3659db9..94ca914 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> }
>
> +static bool feature_control_needed(void *opaque)
> +{
> + X86CPU *cpu = opaque;
> + CPUX86State *env = &cpu->env;
> +
> + return env->msr_ia32_feature_control != 0;
> +}
> +
> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> .name = "cpu/msr_ia32_misc_enable",
> .version_id = 1,
> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> }
> };
>
> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> + .name = "cpu/msr_ia32_feature_control",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_x86_cpu = {
> .name = "cpu",
> .version_id = 12,
> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> }, {
> .vmsd = &vmstate_msr_ia32_misc_enable,
> .needed = misc_enable_needed,
> + }, {
> + .vmsd = &vmstate_msr_ia32_feature_control,
> + .needed = feature_control_needed,
> } , {
> /* empty */
> }
> --
> 1.7.9.5
--
Gleb.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-07-25 10:09 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-07 15:13 [[Qemu-devel] [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration Arthur Chunqi Li
2013-07-07 15:13 ` [Qemu-devel] [ " Arthur Chunqi Li
2013-07-15 5:44 ` Arthur Chunqi Li
2013-07-15 5:44 ` [Qemu-devel] " Arthur Chunqi Li
2013-07-15 5:50 ` [[Qemu-devel] " Gleb Natapov
2013-07-15 5:50 ` [Qemu-devel] [ " Gleb Natapov
2013-07-16 11:42 ` [[Qemu-devel] " Gleb Natapov
2013-07-16 11:42 ` [Qemu-devel] [ " Gleb Natapov
2013-07-16 11:56 ` [[Qemu-devel] " Arthur Chunqi Li
2013-07-16 11:56 ` [Qemu-devel] [ " Arthur Chunqi Li
2013-07-16 12:01 ` [[Qemu-devel] " Gleb Natapov
2013-07-16 12:01 ` [Qemu-devel] [ " Gleb Natapov
2013-07-18 12:42 ` Eduardo Habkost
2013-07-18 12:42 ` Eduardo Habkost
2013-07-16 11:56 ` [[Qemu-devel] " Paolo Bonzini
2013-07-16 11:56 ` [Qemu-devel] [ " Paolo Bonzini
2013-07-25 10:09 ` [[Qemu-devel] " Gleb Natapov
2013-07-25 10:09 ` [Qemu-devel] [ " Gleb Natapov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.