[PATCH] target/i386: support KVM_FEATURE_ASYNC_PF_INT

Vitaly Kuznetsov posted 1 patch 3 years, 7 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200908141206.357450-1-vkuznets@redhat.com
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, Richard Henderson <rth@twiddle.net>, Paolo Bonzini <pbonzini@redhat.com>
target/i386/cpu.c     |  4 +++-
target/i386/cpu.h     |  1 +
target/i386/kvm.c     | 10 ++++++++++
target/i386/machine.c | 19 +++++++++++++++++++
4 files changed, 33 insertions(+), 1 deletion(-)
[PATCH] target/i386: support KVM_FEATURE_ASYNC_PF_INT
Posted by Vitaly Kuznetsov 3 years, 7 months ago
Linux-5.8 introduced interrupt based mechanism for 'page ready' events
delivery and disabled the old, #PF based one (see commit 2635b5c4a0e4
"KVM: x86: interrupt based APF 'page ready' event delivery"). Linux
guest switches to using in in 5.9 (see commit b1d405751cd5 "KVM: x86:
Switch KVM guest to using interrupts for page ready APF delivery").
The feature has a new KVM_FEATURE_ASYNC_PF_INT bit assigned and
the interrupt vector is set in MSR_KVM_ASYNC_PF_INT MSR. Support this
in QEMU.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
- Note, Linux-5.9-rc4 is currently broken (see
https://lore.kernel.org/kvm/20200908135350.355053-3-vkuznets@redhat.com/T/#u)
but I hope it will get fixed before 5.9 is released.
---
 target/i386/cpu.c     |  4 +++-
 target/i386/cpu.h     |  1 +
 target/i386/kvm.c     | 10 ++++++++++
 target/i386/machine.c | 19 +++++++++++++++++++
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 49d89585288d..76037843511f 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",
-            "kvm-poll-control", "kvm-pv-sched-yield", NULL, NULL,
+            "kvm-poll-control", "kvm-pv-sched-yield", "kvm-asyncpf-int", NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             "kvmclock-stable-bit", NULL, NULL, NULL,
@@ -4209,6 +4209,7 @@ static PropValue kvm_default_props[] = {
     { "kvmclock", "on" },
     { "kvm-nopiodelay", "on" },
     { "kvm-asyncpf", "on" },
+    { "kvm-asyncpf-int", "on" },
     { "kvm-steal-time", "on" },
     { "kvm-pv-eoi", "on" },
     { "kvmclock-stable-bit", "on" },
@@ -7092,6 +7093,7 @@ static void x86_cpu_initfn(Object *obj)
     object_property_add_alias(obj, "kvm_nopiodelay", obj, "kvm-nopiodelay");
     object_property_add_alias(obj, "kvm_mmu", obj, "kvm-mmu");
     object_property_add_alias(obj, "kvm_asyncpf", obj, "kvm-asyncpf");
+    object_property_add_alias(obj, "kvm_asyncpf_int", obj, "kvm-asyncpf-int");
     object_property_add_alias(obj, "kvm_steal_time", obj, "kvm-steal-time");
     object_property_add_alias(obj, "kvm_pv_eoi", obj, "kvm-pv-eoi");
     object_property_add_alias(obj, "kvm_pv_unhalt", obj, "kvm-pv-unhalt");
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d3097be6a50a..18e99e9b392a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1492,6 +1492,7 @@ typedef struct CPUX86State {
     uint64_t wall_clock_msr;
     uint64_t steal_time_msr;
     uint64_t async_pf_en_msr;
+    uint64_t async_pf_int_msr;
     uint64_t pv_eoi_en_msr;
     uint64_t poll_control_msr;
 
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 205b68bc0ce8..b26370662075 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -287,6 +287,7 @@ static const struct kvm_para_features {
     { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
     { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
     { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
+    { KVM_CAP_ASYNC_PF_INT, KVM_FEATURE_ASYNC_PF_INT },
 };
 
 static int get_para_features(KVMState *s)
@@ -2820,6 +2821,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
             kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
         }
+        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
+            kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_int_msr);
+        }
         if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
             kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, env->pv_eoi_en_msr);
         }
@@ -3205,6 +3209,9 @@ static int kvm_get_msrs(X86CPU *cpu)
     if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
         kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, 0);
     }
+    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
+        kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, 0);
+    }
     if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
         kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, 0);
     }
@@ -3445,6 +3452,9 @@ static int kvm_get_msrs(X86CPU *cpu)
         case MSR_KVM_ASYNC_PF_EN:
             env->async_pf_en_msr = msrs[i].data;
             break;
+        case MSR_KVM_ASYNC_PF_INT:
+            env->async_pf_int_msr = msrs[i].data;
+            break;
         case MSR_KVM_PV_EOI_EN:
             env->pv_eoi_en_msr = msrs[i].data;
             break;
diff --git a/target/i386/machine.c b/target/i386/machine.c
index b1acf7d0ef46..233e46bb70b5 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -394,6 +394,13 @@ static bool async_pf_msr_needed(void *opaque)
     return cpu->env.async_pf_en_msr != 0;
 }
 
+static bool async_pf_int_msr_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+
+    return cpu->env.async_pf_int_msr != 0;
+}
+
 static bool pv_eoi_msr_needed(void *opaque)
 {
     X86CPU *cpu = opaque;
@@ -467,6 +474,17 @@ static const VMStateDescription vmstate_async_pf_msr = {
     }
 };
 
+static const VMStateDescription vmstate_async_pf_int_msr = {
+    .name = "cpu/async_pf_int_msr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = async_pf_int_msr_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(env.async_pf_int_msr, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_pv_eoi_msr = {
     .name = "cpu/async_pv_eoi_msr",
     .version_id = 1,
@@ -1438,6 +1456,7 @@ VMStateDescription vmstate_x86_cpu = {
     .subsections = (const VMStateDescription*[]) {
         &vmstate_exception_info,
         &vmstate_async_pf_msr,
+        &vmstate_async_pf_int_msr,
         &vmstate_pv_eoi_msr,
         &vmstate_steal_time_msr,
         &vmstate_poll_control_msr,
-- 
2.25.4


Re: [PATCH] target/i386: support KVM_FEATURE_ASYNC_PF_INT
Posted by Paolo Bonzini 3 years, 7 months ago
On 08/09/20 16:12, Vitaly Kuznetsov wrote:
> Linux-5.8 introduced interrupt based mechanism for 'page ready' events
> delivery and disabled the old, #PF based one (see commit 2635b5c4a0e4
> "KVM: x86: interrupt based APF 'page ready' event delivery"). Linux
> guest switches to using in in 5.9 (see commit b1d405751cd5 "KVM: x86:
> Switch KVM guest to using interrupts for page ready APF delivery").
> The feature has a new KVM_FEATURE_ASYNC_PF_INT bit assigned and
> the interrupt vector is set in MSR_KVM_ASYNC_PF_INT MSR. Support this
> in QEMU.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> - Note, Linux-5.9-rc4 is currently broken (see
> https://lore.kernel.org/kvm/20200908135350.355053-3-vkuznets@redhat.com/T/#u)
> but I hope it will get fixed before 5.9 is released.
> ---
>  target/i386/cpu.c     |  4 +++-
>  target/i386/cpu.h     |  1 +
>  target/i386/kvm.c     | 10 ++++++++++
>  target/i386/machine.c | 19 +++++++++++++++++++
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 49d89585288d..76037843511f 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",
> -            "kvm-poll-control", "kvm-pv-sched-yield", NULL, NULL,
> +            "kvm-poll-control", "kvm-pv-sched-yield", "kvm-asyncpf-int", NULL,
>              NULL, NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              "kvmclock-stable-bit", NULL, NULL, NULL,
> @@ -4209,6 +4209,7 @@ static PropValue kvm_default_props[] = {
>      { "kvmclock", "on" },
>      { "kvm-nopiodelay", "on" },
>      { "kvm-asyncpf", "on" },
> +    { "kvm-asyncpf-int", "on" },
>      { "kvm-steal-time", "on" },
>      { "kvm-pv-eoi", "on" },
>      { "kvmclock-stable-bit", "on" },
> @@ -7092,6 +7093,7 @@ static void x86_cpu_initfn(Object *obj)
>      object_property_add_alias(obj, "kvm_nopiodelay", obj, "kvm-nopiodelay");
>      object_property_add_alias(obj, "kvm_mmu", obj, "kvm-mmu");
>      object_property_add_alias(obj, "kvm_asyncpf", obj, "kvm-asyncpf");
> +    object_property_add_alias(obj, "kvm_asyncpf_int", obj, "kvm-asyncpf-int");
>      object_property_add_alias(obj, "kvm_steal_time", obj, "kvm-steal-time");
>      object_property_add_alias(obj, "kvm_pv_eoi", obj, "kvm-pv-eoi");
>      object_property_add_alias(obj, "kvm_pv_unhalt", obj, "kvm-pv-unhalt");
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index d3097be6a50a..18e99e9b392a 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1492,6 +1492,7 @@ typedef struct CPUX86State {
>      uint64_t wall_clock_msr;
>      uint64_t steal_time_msr;
>      uint64_t async_pf_en_msr;
> +    uint64_t async_pf_int_msr;
>      uint64_t pv_eoi_en_msr;
>      uint64_t poll_control_msr;
>  
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 205b68bc0ce8..b26370662075 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -287,6 +287,7 @@ static const struct kvm_para_features {
>      { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
>      { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
>      { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
> +    { KVM_CAP_ASYNC_PF_INT, KVM_FEATURE_ASYNC_PF_INT },
>  };
>  
>  static int get_para_features(KVMState *s)
> @@ -2820,6 +2821,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>          if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
>              kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
>          }
> +        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
> +            kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_int_msr);
> +        }
>          if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
>              kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, env->pv_eoi_en_msr);
>          }
> @@ -3205,6 +3209,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>      if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
>          kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, 0);
>      }
> +    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
> +        kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, 0);
> +    }
>      if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
>          kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, 0);
>      }
> @@ -3445,6 +3452,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>          case MSR_KVM_ASYNC_PF_EN:
>              env->async_pf_en_msr = msrs[i].data;
>              break;
> +        case MSR_KVM_ASYNC_PF_INT:
> +            env->async_pf_int_msr = msrs[i].data;
> +            break;
>          case MSR_KVM_PV_EOI_EN:
>              env->pv_eoi_en_msr = msrs[i].data;
>              break;
> diff --git a/target/i386/machine.c b/target/i386/machine.c
> index b1acf7d0ef46..233e46bb70b5 100644
> --- a/target/i386/machine.c
> +++ b/target/i386/machine.c
> @@ -394,6 +394,13 @@ static bool async_pf_msr_needed(void *opaque)
>      return cpu->env.async_pf_en_msr != 0;
>  }
>  
> +static bool async_pf_int_msr_needed(void *opaque)
> +{
> +    X86CPU *cpu = opaque;
> +
> +    return cpu->env.async_pf_int_msr != 0;
> +}
> +
>  static bool pv_eoi_msr_needed(void *opaque)
>  {
>      X86CPU *cpu = opaque;
> @@ -467,6 +474,17 @@ static const VMStateDescription vmstate_async_pf_msr = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_async_pf_int_msr = {
> +    .name = "cpu/async_pf_int_msr",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = async_pf_int_msr_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(env.async_pf_int_msr, X86CPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_pv_eoi_msr = {
>      .name = "cpu/async_pv_eoi_msr",
>      .version_id = 1,
> @@ -1438,6 +1456,7 @@ VMStateDescription vmstate_x86_cpu = {
>      .subsections = (const VMStateDescription*[]) {
>          &vmstate_exception_info,
>          &vmstate_async_pf_msr,
> +        &vmstate_async_pf_int_msr,
>          &vmstate_pv_eoi_msr,
>          &vmstate_steal_time_msr,
>          &vmstate_poll_control_msr,
> 

Queued, thanks.

Paolo


Re: [PATCH] target/i386: support KVM_FEATURE_ASYNC_PF_INT
Posted by Paolo Bonzini 3 years, 6 months ago
On 12/09/20 08:02, Paolo Bonzini wrote:
> @@ -4209,6 +4209,7 @@ static PropValue kvm_default_props[] = {
>      { "kvmclock", "on" },
>      { "kvm-nopiodelay", "on" },
>      { "kvm-asyncpf", "on" },
> +    { "kvm-asyncpf-int", "on" },
>      { "kvm-steal-time", "on" },
>      { "kvm-pv-eoi", "on" },

This would warn on old kernels.  I removed it.

Paolo

Re: [PATCH] target/i386: support KVM_FEATURE_ASYNC_PF_INT
Posted by Vitaly Kuznetsov 3 years, 6 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 12/09/20 08:02, Paolo Bonzini wrote:
>> @@ -4209,6 +4209,7 @@ static PropValue kvm_default_props[] = {
>>      { "kvmclock", "on" },
>>      { "kvm-nopiodelay", "on" },
>>      { "kvm-asyncpf", "on" },
>> +    { "kvm-asyncpf-int", "on" },
>>      { "kvm-steal-time", "on" },
>>      { "kvm-pv-eoi", "on" },
>
> This would warn on old kernels.  I removed it.
>

With Eduardo's comment updated (thanks) it's clear it should not be
there. Thanks!

-- 
Vitaly