kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch QEMU] kvm: i386: halt poll control MSR support
@ 2019-06-03 23:04 Marcelo Tosatti
  2019-07-15  9:23 ` [Qemu-devel] " Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2019-06-03 23:04 UTC (permalink / raw)
  To: kvm-devel, qemu-devel; +Cc: Paolo Bonzini, Radim Krčmář

(CC'ing qemu devel)

Add support for halt poll control MSR: save/restore, migration
and new feature name.

The purpose of this MSR is to allow the guest to disable
host halt poll.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

diff --git a/include/standard-headers/asm-x86/kvm_para.h b/include/standard-headers/asm-x86/kvm_para.h
index 35cd8d6..e171514 100644
--- a/include/standard-headers/asm-x86/kvm_para.h
+++ b/include/standard-headers/asm-x86/kvm_para.h
@@ -29,6 +29,7 @@
 #define KVM_FEATURE_PV_TLB_FLUSH	9
 #define KVM_FEATURE_ASYNC_PF_VMEXIT	10
 #define KVM_FEATURE_PV_SEND_IPI	11
+#define KVM_FEATURE_POLL_CONTROL	12
 
 #define KVM_HINTS_REALTIME      0
 
@@ -47,6 +48,7 @@
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
 #define MSR_KVM_STEAL_TIME  0x4b564d03
 #define MSR_KVM_PV_EOI_EN      0x4b564d04
+#define MSR_KVM_POLL_CONTROL	0x4b564d05
 
 struct kvm_steal_time {
 	uint64_t steal;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c1ab86d..1ca6944 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -903,7 +903,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
             "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
             NULL, "kvm-pv-tlb-flush", NULL, "kvm-pv-ipi",
-            NULL, NULL, NULL, NULL,
+            "kvm-poll-control", NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             "kvmclock-stable-bit", NULL, NULL, NULL,
@@ -3001,6 +3001,7 @@ static PropValue kvm_default_props[] = {
     { "kvm-asyncpf", "on" },
     { "kvm-steal-time", "on" },
     { "kvm-pv-eoi", "on" },
+    { "kvm-poll-control", "on" },
     { "kvmclock-stable-bit", "on" },
     { "x2apic", "on" },
     { "acpi", "off" },
@@ -5660,6 +5661,8 @@ static void x86_cpu_initfn(Object *obj)
     object_property_add_alias(obj, "kvm_steal_time", obj, "kvm-steal-time", &error_abort);
     object_property_add_alias(obj, "kvm_pv_eoi", obj, "kvm-pv-eoi", &error_abort);
     object_property_add_alias(obj, "kvm_pv_unhalt", obj, "kvm-pv-unhalt", &error_abort);
+    object_property_add_alias(obj, "kvm_poll_control", obj, "kvm-poll-control",
+                              &error_abort);
     object_property_add_alias(obj, "svm_lock", obj, "svm-lock", &error_abort);
     object_property_add_alias(obj, "nrip_save", obj, "nrip-save", &error_abort);
     object_property_add_alias(obj, "tsc_scale", obj, "tsc-scale", &error_abort);
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index bd06523..21ed2f8 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1241,6 +1241,7 @@ typedef struct CPUX86State {
     uint64_t steal_time_msr;
     uint64_t async_pf_en_msr;
     uint64_t pv_eoi_en_msr;
+    uint64_t poll_control_msr;
 
     /* Partition-wide HV MSRs, will be updated only on the first vcpu */
     uint64_t msr_hv_hypercall;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3b29ce5..a5e9cdf 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1369,6 +1369,8 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
 
         hyperv_x86_synic_reset(cpu);
     }
+    /* enabled by default */
+    env->poll_control_msr = 1;
 }
 
 void kvm_arch_do_init_vcpu(X86CPU *cpu)
@@ -2059,6 +2061,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
             kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr);
         }
+
+        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
+            kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, env->poll_control_msr);
+        }
+
         if (has_architectural_pmu_version > 0) {
             if (has_architectural_pmu_version > 1) {
                 /* Stop the counter.  */
@@ -2443,6 +2450,9 @@ static int kvm_get_msrs(X86CPU *cpu)
     if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
         kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 0);
     }
+    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
+        kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, 1);
+    }
     if (has_architectural_pmu_version > 0) {
         if (has_architectural_pmu_version > 1) {
             kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
@@ -2677,6 +2687,10 @@ static int kvm_get_msrs(X86CPU *cpu)
         case MSR_KVM_STEAL_TIME:
             env->steal_time_msr = msrs[i].data;
             break;
+        case MSR_KVM_POLL_CONTROL: {
+            env->poll_control_msr = msrs[i].data;
+            break;
+        }
         case MSR_CORE_PERF_FIXED_CTR_CTRL:
             env->msr_fixed_ctr_ctrl = msrs[i].data;
             break;
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 225b5d4..1c23e5e 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -323,6 +323,14 @@ static bool steal_time_msr_needed(void *opaque)
     return cpu->env.steal_time_msr != 0;
 }
 
+/* Poll control MSR enabled by default */
+static bool poll_control_msr_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+
+    return cpu->env.poll_control_msr != 1;
+}
+
 static const VMStateDescription vmstate_steal_time_msr = {
     .name = "cpu/steal_time_msr",
     .version_id = 1,
@@ -356,6 +364,17 @@ static const VMStateDescription vmstate_pv_eoi_msr = {
     }
 };
 
+static const VMStateDescription vmstate_poll_control_msr = {
+    .name = "cpu/poll_control_msr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = poll_control_msr_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(env.poll_control_msr, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool fpop_ip_dp_needed(void *opaque)
 {
     X86CPU *cpu = opaque;
@@ -1062,6 +1081,7 @@ VMStateDescription vmstate_x86_cpu = {
         &vmstate_async_pf_msr,
         &vmstate_pv_eoi_msr,
         &vmstate_steal_time_msr,
+        &vmstate_poll_control_msr,
         &vmstate_fpop_ip_dp,
         &vmstate_msr_tsc_adjust,
         &vmstate_msr_tscdeadline,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [patch QEMU] kvm: i386: halt poll control MSR support
  2019-06-03 23:04 [patch QEMU] kvm: i386: halt poll control MSR support Marcelo Tosatti
@ 2019-07-15  9:23 ` Paolo Bonzini
  2019-07-16 19:52   ` Mark Kanda
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2019-07-15  9:23 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm-devel, qemu-devel; +Cc: Radim Krčmář

On 04/06/19 01:04, Marcelo Tosatti wrote:
> (CC'ing qemu devel)
> 
> Add support for halt poll control MSR: save/restore, migration
> and new feature name.
> 
> The purpose of this MSR is to allow the guest to disable
> host halt poll.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> diff --git a/include/standard-headers/asm-x86/kvm_para.h b/include/standard-headers/asm-x86/kvm_para.h
> index 35cd8d6..e171514 100644
> --- a/include/standard-headers/asm-x86/kvm_para.h
> +++ b/include/standard-headers/asm-x86/kvm_para.h
> @@ -29,6 +29,7 @@
>  #define KVM_FEATURE_PV_TLB_FLUSH	9
>  #define KVM_FEATURE_ASYNC_PF_VMEXIT	10
>  #define KVM_FEATURE_PV_SEND_IPI	11
> +#define KVM_FEATURE_POLL_CONTROL	12
>  
>  #define KVM_HINTS_REALTIME      0
>  
> @@ -47,6 +48,7 @@
>  #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
>  #define MSR_KVM_STEAL_TIME  0x4b564d03
>  #define MSR_KVM_PV_EOI_EN      0x4b564d04
> +#define MSR_KVM_POLL_CONTROL	0x4b564d05
>  
>  struct kvm_steal_time {
>  	uint64_t steal;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index c1ab86d..1ca6944 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -903,7 +903,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>              "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
>              "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
>              NULL, "kvm-pv-tlb-flush", NULL, "kvm-pv-ipi",
> -            NULL, NULL, NULL, NULL,
> +            "kvm-poll-control", NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              "kvmclock-stable-bit", NULL, NULL, NULL,
> @@ -3001,6 +3001,7 @@ static PropValue kvm_default_props[] = {
>      { "kvm-asyncpf", "on" },
>      { "kvm-steal-time", "on" },
>      { "kvm-pv-eoi", "on" },
> +    { "kvm-poll-control", "on" },
>      { "kvmclock-stable-bit", "on" },
>      { "x2apic", "on" },
>      { "acpi", "off" },
> @@ -5660,6 +5661,8 @@ static void x86_cpu_initfn(Object *obj)
>      object_property_add_alias(obj, "kvm_steal_time", obj, "kvm-steal-time", &error_abort);
>      object_property_add_alias(obj, "kvm_pv_eoi", obj, "kvm-pv-eoi", &error_abort);
>      object_property_add_alias(obj, "kvm_pv_unhalt", obj, "kvm-pv-unhalt", &error_abort);
> +    object_property_add_alias(obj, "kvm_poll_control", obj, "kvm-poll-control",
> +                              &error_abort);
>      object_property_add_alias(obj, "svm_lock", obj, "svm-lock", &error_abort);
>      object_property_add_alias(obj, "nrip_save", obj, "nrip-save", &error_abort);
>      object_property_add_alias(obj, "tsc_scale", obj, "tsc-scale", &error_abort);
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index bd06523..21ed2f8 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1241,6 +1241,7 @@ typedef struct CPUX86State {
>      uint64_t steal_time_msr;
>      uint64_t async_pf_en_msr;
>      uint64_t pv_eoi_en_msr;
> +    uint64_t poll_control_msr;
>  
>      /* Partition-wide HV MSRs, will be updated only on the first vcpu */
>      uint64_t msr_hv_hypercall;
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 3b29ce5..a5e9cdf 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1369,6 +1369,8 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
>  
>          hyperv_x86_synic_reset(cpu);
>      }
> +    /* enabled by default */
> +    env->poll_control_msr = 1;
>  }
>  
>  void kvm_arch_do_init_vcpu(X86CPU *cpu)
> @@ -2059,6 +2061,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>          if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
>              kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr);
>          }
> +
> +        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
> +            kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, env->poll_control_msr);
> +        }
> +
>          if (has_architectural_pmu_version > 0) {
>              if (has_architectural_pmu_version > 1) {
>                  /* Stop the counter.  */
> @@ -2443,6 +2450,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>      if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
>          kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 0);
>      }
> +    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
> +        kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, 1);
> +    }
>      if (has_architectural_pmu_version > 0) {
>          if (has_architectural_pmu_version > 1) {
>              kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
> @@ -2677,6 +2687,10 @@ static int kvm_get_msrs(X86CPU *cpu)
>          case MSR_KVM_STEAL_TIME:
>              env->steal_time_msr = msrs[i].data;
>              break;
> +        case MSR_KVM_POLL_CONTROL: {
> +            env->poll_control_msr = msrs[i].data;
> +            break;
> +        }
>          case MSR_CORE_PERF_FIXED_CTR_CTRL:
>              env->msr_fixed_ctr_ctrl = msrs[i].data;
>              break;
> diff --git a/target/i386/machine.c b/target/i386/machine.c
> index 225b5d4..1c23e5e 100644
> --- a/target/i386/machine.c
> +++ b/target/i386/machine.c
> @@ -323,6 +323,14 @@ static bool steal_time_msr_needed(void *opaque)
>      return cpu->env.steal_time_msr != 0;
>  }
>  
> +/* Poll control MSR enabled by default */
> +static bool poll_control_msr_needed(void *opaque)
> +{
> +    X86CPU *cpu = opaque;
> +
> +    return cpu->env.poll_control_msr != 1;
> +}
> +
>  static const VMStateDescription vmstate_steal_time_msr = {
>      .name = "cpu/steal_time_msr",
>      .version_id = 1,
> @@ -356,6 +364,17 @@ static const VMStateDescription vmstate_pv_eoi_msr = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_poll_control_msr = {
> +    .name = "cpu/poll_control_msr",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = poll_control_msr_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(env.poll_control_msr, X86CPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static bool fpop_ip_dp_needed(void *opaque)
>  {
>      X86CPU *cpu = opaque;
> @@ -1062,6 +1081,7 @@ VMStateDescription vmstate_x86_cpu = {
>          &vmstate_async_pf_msr,
>          &vmstate_pv_eoi_msr,
>          &vmstate_steal_time_msr,
> +        &vmstate_poll_control_msr,
>          &vmstate_fpop_ip_dp,
>          &vmstate_msr_tsc_adjust,
>          &vmstate_msr_tscdeadline,
> 

Queued, thanks.  Sorry for missing it until now.

Paolo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [patch QEMU] kvm: i386: halt poll control MSR support
  2019-07-15  9:23 ` [Qemu-devel] " Paolo Bonzini
@ 2019-07-16 19:52   ` Mark Kanda
  2019-07-16 21:09     ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Kanda @ 2019-07-16 19:52 UTC (permalink / raw)
  To: Paolo Bonzini, Marcelo Tosatti, kvm-devel, qemu-devel
  Cc: Radim Krčmář

[-- Attachment #1: Type: text/plain, Size: 7488 bytes --]

Hi all,

If the host doesn't support halt polling, this patch seems to break 
libvirt save/restore:

"
virsh # save halt-poll-vm halt-poll-vm.sav --running --verbose
Save: [100 %]
Domain halt-poll-vm saved to halt-poll-vm.sav

virsh # restore halt-poll-vm.sav
error: Failed to restore domain from halt-poll-vm.sav
error: operation failed: guest CPU doesn't match specification
"

I believe this occurs because libvirt rejects the restore if there are 
filtered features, which is the case if halt polling is enabled on a 
host which doesn't support it (halt polling is enabled 'by default').

As such, I think we should only enable halt polling if it is supported 
on the host - see the attached patch.

...thoughts?

Thanks,
-Mark

On 7/15/2019 4:23 AM, Paolo Bonzini wrote:
> On 04/06/19 01:04, Marcelo Tosatti wrote:
>> (CC'ing qemu devel)
>>
>> Add support for halt poll control MSR: save/restore, migration
>> and new feature name.
>>
>> The purpose of this MSR is to allow the guest to disable
>> host halt poll.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> diff --git a/include/standard-headers/asm-x86/kvm_para.h b/include/standard-headers/asm-x86/kvm_para.h
>> index 35cd8d6..e171514 100644
>> --- a/include/standard-headers/asm-x86/kvm_para.h
>> +++ b/include/standard-headers/asm-x86/kvm_para.h
>> @@ -29,6 +29,7 @@
>>   #define KVM_FEATURE_PV_TLB_FLUSH	9
>>   #define KVM_FEATURE_ASYNC_PF_VMEXIT	10
>>   #define KVM_FEATURE_PV_SEND_IPI	11
>> +#define KVM_FEATURE_POLL_CONTROL	12
>>   
>>   #define KVM_HINTS_REALTIME      0
>>   
>> @@ -47,6 +48,7 @@
>>   #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
>>   #define MSR_KVM_STEAL_TIME  0x4b564d03
>>   #define MSR_KVM_PV_EOI_EN      0x4b564d04
>> +#define MSR_KVM_POLL_CONTROL	0x4b564d05
>>   
>>   struct kvm_steal_time {
>>   	uint64_t steal;
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index c1ab86d..1ca6944 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -903,7 +903,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>>               "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
>>               "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
>>               NULL, "kvm-pv-tlb-flush", NULL, "kvm-pv-ipi",
>> -            NULL, NULL, NULL, NULL,
>> +            "kvm-poll-control", NULL, NULL, NULL,
>>               NULL, NULL, NULL, NULL,
>>               NULL, NULL, NULL, NULL,
>>               "kvmclock-stable-bit", NULL, NULL, NULL,
>> @@ -3001,6 +3001,7 @@ static PropValue kvm_default_props[] = {
>>       { "kvm-asyncpf", "on" },
>>       { "kvm-steal-time", "on" },
>>       { "kvm-pv-eoi", "on" },
>> +    { "kvm-poll-control", "on" },
>>       { "kvmclock-stable-bit", "on" },
>>       { "x2apic", "on" },
>>       { "acpi", "off" },
>> @@ -5660,6 +5661,8 @@ static void x86_cpu_initfn(Object *obj)
>>       object_property_add_alias(obj, "kvm_steal_time", obj, "kvm-steal-time", &error_abort);
>>       object_property_add_alias(obj, "kvm_pv_eoi", obj, "kvm-pv-eoi", &error_abort);
>>       object_property_add_alias(obj, "kvm_pv_unhalt", obj, "kvm-pv-unhalt", &error_abort);
>> +    object_property_add_alias(obj, "kvm_poll_control", obj, "kvm-poll-control",
>> +                              &error_abort);
>>       object_property_add_alias(obj, "svm_lock", obj, "svm-lock", &error_abort);
>>       object_property_add_alias(obj, "nrip_save", obj, "nrip-save", &error_abort);
>>       object_property_add_alias(obj, "tsc_scale", obj, "tsc-scale", &error_abort);
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index bd06523..21ed2f8 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -1241,6 +1241,7 @@ typedef struct CPUX86State {
>>       uint64_t steal_time_msr;
>>       uint64_t async_pf_en_msr;
>>       uint64_t pv_eoi_en_msr;
>> +    uint64_t poll_control_msr;
>>   
>>       /* Partition-wide HV MSRs, will be updated only on the first vcpu */
>>       uint64_t msr_hv_hypercall;
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 3b29ce5..a5e9cdf 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1369,6 +1369,8 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
>>   
>>           hyperv_x86_synic_reset(cpu);
>>       }
>> +    /* enabled by default */
>> +    env->poll_control_msr = 1;
>>   }
>>   
>>   void kvm_arch_do_init_vcpu(X86CPU *cpu)
>> @@ -2059,6 +2061,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>>           if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
>>               kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr);
>>           }
>> +
>> +        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
>> +            kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, env->poll_control_msr);
>> +        }
>> +
>>           if (has_architectural_pmu_version > 0) {
>>               if (has_architectural_pmu_version > 1) {
>>                   /* Stop the counter.  */
>> @@ -2443,6 +2450,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>>       if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
>>           kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 0);
>>       }
>> +    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
>> +        kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, 1);
>> +    }
>>       if (has_architectural_pmu_version > 0) {
>>           if (has_architectural_pmu_version > 1) {
>>               kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
>> @@ -2677,6 +2687,10 @@ static int kvm_get_msrs(X86CPU *cpu)
>>           case MSR_KVM_STEAL_TIME:
>>               env->steal_time_msr = msrs[i].data;
>>               break;
>> +        case MSR_KVM_POLL_CONTROL: {
>> +            env->poll_control_msr = msrs[i].data;
>> +            break;
>> +        }
>>           case MSR_CORE_PERF_FIXED_CTR_CTRL:
>>               env->msr_fixed_ctr_ctrl = msrs[i].data;
>>               break;
>> diff --git a/target/i386/machine.c b/target/i386/machine.c
>> index 225b5d4..1c23e5e 100644
>> --- a/target/i386/machine.c
>> +++ b/target/i386/machine.c
>> @@ -323,6 +323,14 @@ static bool steal_time_msr_needed(void *opaque)
>>       return cpu->env.steal_time_msr != 0;
>>   }
>>   
>> +/* Poll control MSR enabled by default */
>> +static bool poll_control_msr_needed(void *opaque)
>> +{
>> +    X86CPU *cpu = opaque;
>> +
>> +    return cpu->env.poll_control_msr != 1;
>> +}
>> +
>>   static const VMStateDescription vmstate_steal_time_msr = {
>>       .name = "cpu/steal_time_msr",
>>       .version_id = 1,
>> @@ -356,6 +364,17 @@ static const VMStateDescription vmstate_pv_eoi_msr = {
>>       }
>>   };
>>   
>> +static const VMStateDescription vmstate_poll_control_msr = {
>> +    .name = "cpu/poll_control_msr",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = poll_control_msr_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT64(env.poll_control_msr, X86CPU),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>   static bool fpop_ip_dp_needed(void *opaque)
>>   {
>>       X86CPU *cpu = opaque;
>> @@ -1062,6 +1081,7 @@ VMStateDescription vmstate_x86_cpu = {
>>           &vmstate_async_pf_msr,
>>           &vmstate_pv_eoi_msr,
>>           &vmstate_steal_time_msr,
>> +        &vmstate_poll_control_msr,
>>           &vmstate_fpop_ip_dp,
>>           &vmstate_msr_tsc_adjust,
>>           &vmstate_msr_tscdeadline,
>>
> 
> Queued, thanks.  Sorry for missing it until now.
> 
> Paolo
> 
> 

[-- Attachment #2: 0001-Only-enable-the-halt-poll-control-MSR-if-it-is-suppo.patch --]
[-- Type: text/plain, Size: 2269 bytes --]

From ac83cb52be3fbe226645e780500adb767f9bcfd2 Mon Sep 17 00:00:00 2001
From: Mark Kanda <mark.kanda@oracle.com>
Date: Tue, 16 Jul 2019 14:46:11 -0500
Subject: [PATCH QEMU] Only enable the halt poll control MSR if it is supported
 by the host

The halt poll control MSR should only be enabled on hosts which
support it.

Fixes: ("kvm: i386: halt poll control MSR support")

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
 target/i386/cpu.c     | 8 +++++++-
 target/i386/kvm.c     | 2 --
 target/i386/machine.c | 1 -
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a8bafdb8b9..dacbf7a9fe 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2838,7 +2838,6 @@ static PropValue kvm_default_props[] = {
     { "kvm-asyncpf", "on" },
     { "kvm-steal-time", "on" },
     { "kvm-pv-eoi", "on" },
-    { "kvm-poll-control", "on" },
     { "kvmclock-stable-bit", "on" },
     { "x2apic", "on" },
     { "acpi", "off" },
@@ -5109,6 +5108,13 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
         env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
     }
 
+    /* Enable the halt poll control MSR if it is supported by the host */
+    if (x86_cpu_get_supported_feature_word(FEAT_KVM, cpu->migratable) &
+        (1 << KVM_FEATURE_POLL_CONTROL)) {
+        env->features[FEAT_KVM] |= 1 << KVM_FEATURE_POLL_CONTROL;
+        env->poll_control_msr = 1;
+    }
+
 out:
     if (local_err != NULL) {
         error_propagate(errp, local_err);
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index cb22684139..81dd5d2c1b 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1796,8 +1796,6 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
 
         hyperv_x86_synic_reset(cpu);
     }
-    /* enabled by default */
-    env->poll_control_msr = 1;
 }
 
 void kvm_arch_do_init_vcpu(X86CPU *cpu)
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 20077a8a5a..9d6095b264 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -394,7 +394,6 @@ static bool steal_time_msr_needed(void *opaque)
     return cpu->env.steal_time_msr != 0;
 }
 
-/* Poll control MSR enabled by default */
 static bool poll_control_msr_needed(void *opaque)
 {
     X86CPU *cpu = opaque;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [patch QEMU] kvm: i386: halt poll control MSR support
  2019-07-16 19:52   ` Mark Kanda
@ 2019-07-16 21:09     ` Paolo Bonzini
  2019-07-16 21:15       ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2019-07-16 21:09 UTC (permalink / raw)
  To: Mark Kanda, Marcelo Tosatti, kvm-devel, qemu-devel
  Cc: Radim Krčmář

On 16/07/19 21:52, Mark Kanda wrote:
> 
> As such, I think we should only enable halt polling if it is supported
> on the host - see the attached patch.
> 
> ...thoughts?

No, it should not be enabled by default at all, at least not until we
can require kernel 5.2.  My mistake, sorry.  Can you post a patch?

Paolo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [patch QEMU] kvm: i386: halt poll control MSR support
  2019-07-16 21:09     ` Paolo Bonzini
@ 2019-07-16 21:15       ` Paolo Bonzini
  2019-07-16 21:21         ` Mark Kanda
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2019-07-16 21:15 UTC (permalink / raw)
  To: Mark Kanda, Marcelo Tosatti, kvm-devel, qemu-devel
  Cc: Radim Krčmář

On 16/07/19 23:09, Paolo Bonzini wrote:
>> As such, I think we should only enable halt polling if it is supported
>> on the host - see the attached patch.
>>
>> ...thoughts?
> No, it should not be enabled by default at all, at least not until we
> can require kernel 5.2.  My mistake, sorry.  Can you post a patch?

Doh, nevermind.  This is not included in 4.1 so there's time to fix it.
 Pfew. :)

Paolo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [patch QEMU] kvm: i386: halt poll control MSR support
  2019-07-16 21:15       ` Paolo Bonzini
@ 2019-07-16 21:21         ` Mark Kanda
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Kanda @ 2019-07-16 21:21 UTC (permalink / raw)
  To: Paolo Bonzini, Marcelo Tosatti, kvm-devel, qemu-devel
  Cc: Radim Krčmář

On 7/16/2019 4:15 PM, Paolo Bonzini wrote:
> On 16/07/19 23:09, Paolo Bonzini wrote:
>>> As such, I think we should only enable halt polling if it is supported
>>> on the host - see the attached patch.
>>>
>>> ...thoughts?
>> No, it should not be enabled by default at all, at least not until we
>> can require kernel 5.2.  My mistake, sorry.  Can you post a patch?
> 
> Doh, nevermind.  This is not included in 4.1 so there's time to fix it.
>   Pfew. :)
> 

:)

I'll post a patch regardless.

Thanks/regards,
-Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-07-16 21:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03 23:04 [patch QEMU] kvm: i386: halt poll control MSR support Marcelo Tosatti
2019-07-15  9:23 ` [Qemu-devel] " Paolo Bonzini
2019-07-16 19:52   ` Mark Kanda
2019-07-16 21:09     ` Paolo Bonzini
2019-07-16 21:15       ` Paolo Bonzini
2019-07-16 21:21         ` Mark Kanda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).