* [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-17 8:24 ` Wanpeng Li
0 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-04-17 8:24 UTC (permalink / raw)
To: qemu-devel, kvm
Cc: Paolo Bonzini, Eduardo Habkost, Radim Krčmář
From: Wanpeng Li <wanpengli@tencent.com>
This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
in order that to improve latency in some workloads.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
| 6 +++++-
target/i386/cpu.h | 2 ++
target/i386/kvm.c | 16 ++++++++++++++++
3 files changed, 23 insertions(+), 1 deletion(-)
--git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index a167be8..857df15 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_S390_GS 140
#define KVM_CAP_S390_AIS 141
#define KVM_CAP_SPAPR_TCE_VFIO 142
-#define KVM_CAP_X86_GUEST_MWAIT 143
+#define KVM_CAP_X86_DISABLE_EXITS 143
#define KVM_CAP_ARM_USER_IRQ 144
#define KVM_CAP_S390_CMMA_MIGRATION 145
#define KVM_CAP_PPC_FWNMI 146
@@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
#define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
#define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
+#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
+#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
+#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
+
/* Available with KVM_CAP_ARM_USER_IRQ */
/* Bits for run->s.regs.device_irq_level */
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1b219fa..965de1b 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
#define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
+#define KVM_PV_UNHALT (1U << 7)
+
#define KVM_HINTS_DEDICATED (1U << 0)
#define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6c49954..3e99830 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
}
+ if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
+ int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
+
+ if (disable_exits) {
+ disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
+ KVM_X86_DISABLE_EXITS_HLT |
+ KVM_X86_DISABLE_EXITS_PAUSE);
+ if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
+ disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
+ }
+ }
+ if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
+ error_report("kvm: DISABLE EXITS not supported");
+ }
+ }
+
qemu_add_vm_change_state_handler(cpu_update_state, env);
c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
--
2.7.4
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-17 8:24 ` Wanpeng Li
0 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-04-17 8:24 UTC (permalink / raw)
To: qemu-devel, kvm
Cc: Paolo Bonzini, Radim Krčmář, Eduardo Habkost
From: Wanpeng Li <wanpengli@tencent.com>
This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
in order that to improve latency in some workloads.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
| 6 +++++-
target/i386/cpu.h | 2 ++
target/i386/kvm.c | 16 ++++++++++++++++
3 files changed, 23 insertions(+), 1 deletion(-)
--git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index a167be8..857df15 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_S390_GS 140
#define KVM_CAP_S390_AIS 141
#define KVM_CAP_SPAPR_TCE_VFIO 142
-#define KVM_CAP_X86_GUEST_MWAIT 143
+#define KVM_CAP_X86_DISABLE_EXITS 143
#define KVM_CAP_ARM_USER_IRQ 144
#define KVM_CAP_S390_CMMA_MIGRATION 145
#define KVM_CAP_PPC_FWNMI 146
@@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
#define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
#define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
+#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
+#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
+#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
+
/* Available with KVM_CAP_ARM_USER_IRQ */
/* Bits for run->s.regs.device_irq_level */
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1b219fa..965de1b 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
#define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
+#define KVM_PV_UNHALT (1U << 7)
+
#define KVM_HINTS_DEDICATED (1U << 0)
#define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6c49954..3e99830 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
}
+ if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
+ int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
+
+ if (disable_exits) {
+ disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
+ KVM_X86_DISABLE_EXITS_HLT |
+ KVM_X86_DISABLE_EXITS_PAUSE);
+ if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
+ disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
+ }
+ }
+ if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
+ error_report("kvm: DISABLE EXITS not supported");
+ }
+ }
+
qemu_add_vm_change_state_handler(cpu_update_state, env);
c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
--
2.7.4
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-17 8:24 ` [Qemu-devel] " Wanpeng Li
@ 2018-04-17 8:28 ` no-reply
-1 siblings, 0 replies; 50+ messages in thread
From: no-reply @ 2018-04-17 8:28 UTC (permalink / raw)
To: kernellwp; +Cc: famz, ehabkost, kvm, rkrcmar, qemu-devel, pbonzini
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 1523953455-28053-1-git-send-email-wanpengli@tencent.com
Subject: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1523953455-28053-1-git-send-email-wanpengli@tencent.com -> patchew/1523953455-28053-1-git-send-email-wanpengli@tencent.com
Switched to a new branch 'test'
61dd49b0a1 i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
=== OUTPUT BEGIN ===
Checking PATCH 1/1: i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS...
WARNING: line over 80 characters
#65: FILE: target/i386/kvm.c:1033:
+ int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
ERROR: line over 90 characters
#75: FILE: target/i386/kvm.c:1043:
+ if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
total: 1 errors, 1 warnings, 48 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-17 8:28 ` no-reply
0 siblings, 0 replies; 50+ messages in thread
From: no-reply @ 2018-04-17 8:28 UTC (permalink / raw)
To: kernellwp; +Cc: famz, qemu-devel, kvm, pbonzini, ehabkost, rkrcmar
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 1523953455-28053-1-git-send-email-wanpengli@tencent.com
Subject: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1523953455-28053-1-git-send-email-wanpengli@tencent.com -> patchew/1523953455-28053-1-git-send-email-wanpengli@tencent.com
Switched to a new branch 'test'
61dd49b0a1 i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
=== OUTPUT BEGIN ===
Checking PATCH 1/1: i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS...
WARNING: line over 80 characters
#65: FILE: target/i386/kvm.c:1033:
+ int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
ERROR: line over 90 characters
#75: FILE: target/i386/kvm.c:1043:
+ if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
total: 1 errors, 1 warnings, 48 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-17 8:24 ` [Qemu-devel] " Wanpeng Li
@ 2018-04-17 18:08 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-04-17 18:08 UTC (permalink / raw)
To: Wanpeng Li
Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel, kvm,
Radim Krčmář
On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
>
> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
> in order that to improve latency in some workloads.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>
> linux-headers/linux/kvm.h | 6 +++++-
> target/i386/cpu.h | 2 ++
> target/i386/kvm.c | 16 ++++++++++++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index a167be8..857df15 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_S390_GS 140
> #define KVM_CAP_S390_AIS 141
> #define KVM_CAP_SPAPR_TCE_VFIO 142
> -#define KVM_CAP_X86_GUEST_MWAIT 143
> +#define KVM_CAP_X86_DISABLE_EXITS 143
> #define KVM_CAP_ARM_USER_IRQ 144
> #define KVM_CAP_S390_CMMA_MIGRATION 145
> #define KVM_CAP_PPC_FWNMI 146
> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>
> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
> +
> /* Available with KVM_CAP_ARM_USER_IRQ */
>
> /* Bits for run->s.regs.device_irq_level */
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 1b219fa..965de1b 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>
> +#define KVM_PV_UNHALT (1U << 7)
> +
Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
> #define KVM_HINTS_DEDICATED (1U << 0)
>
BTW I wonder whether we should switch to a value from
kvm_para.h? I'll send a version to do it, pls take a look.
> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6c49954..3e99830 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
> }
>
> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
> +
> + if (disable_exits) {
> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> + KVM_X86_DISABLE_EXITS_HLT |
> + KVM_X86_DISABLE_EXITS_PAUSE);
> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> + }
> + }
> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
> + error_report("kvm: DISABLE EXITS not supported");
> + }
> + }
> +
> qemu_add_vm_change_state_handler(cpu_update_state, env);
>
> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> --
> 2.7.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-17 18:08 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-04-17 18:08 UTC (permalink / raw)
To: Wanpeng Li
Cc: qemu-devel, kvm, Paolo Bonzini, Radim Krčmář,
Eduardo Habkost
On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
>
> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
> in order that to improve latency in some workloads.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>
> linux-headers/linux/kvm.h | 6 +++++-
> target/i386/cpu.h | 2 ++
> target/i386/kvm.c | 16 ++++++++++++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index a167be8..857df15 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_S390_GS 140
> #define KVM_CAP_S390_AIS 141
> #define KVM_CAP_SPAPR_TCE_VFIO 142
> -#define KVM_CAP_X86_GUEST_MWAIT 143
> +#define KVM_CAP_X86_DISABLE_EXITS 143
> #define KVM_CAP_ARM_USER_IRQ 144
> #define KVM_CAP_S390_CMMA_MIGRATION 145
> #define KVM_CAP_PPC_FWNMI 146
> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>
> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
> +
> /* Available with KVM_CAP_ARM_USER_IRQ */
>
> /* Bits for run->s.regs.device_irq_level */
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 1b219fa..965de1b 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>
> +#define KVM_PV_UNHALT (1U << 7)
> +
Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
> #define KVM_HINTS_DEDICATED (1U << 0)
>
BTW I wonder whether we should switch to a value from
kvm_para.h? I'll send a version to do it, pls take a look.
> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6c49954..3e99830 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
> }
>
> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
> +
> + if (disable_exits) {
> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> + KVM_X86_DISABLE_EXITS_HLT |
> + KVM_X86_DISABLE_EXITS_PAUSE);
> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> + }
> + }
> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
> + error_report("kvm: DISABLE EXITS not supported");
> + }
> + }
> +
> qemu_add_vm_change_state_handler(cpu_update_state, env);
>
> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> --
> 2.7.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-17 8:24 ` [Qemu-devel] " Wanpeng Li
@ 2018-04-17 20:59 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-04-17 20:59 UTC (permalink / raw)
To: Wanpeng Li; +Cc: Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
>
> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
> in order that to improve latency in some workloads.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>
> linux-headers/linux/kvm.h | 6 +++++-
> target/i386/cpu.h | 2 ++
> target/i386/kvm.c | 16 ++++++++++++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index a167be8..857df15 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_S390_GS 140
> #define KVM_CAP_S390_AIS 141
> #define KVM_CAP_SPAPR_TCE_VFIO 142
> -#define KVM_CAP_X86_GUEST_MWAIT 143
> +#define KVM_CAP_X86_DISABLE_EXITS 143
> #define KVM_CAP_ARM_USER_IRQ 144
> #define KVM_CAP_S390_CMMA_MIGRATION 145
> #define KVM_CAP_PPC_FWNMI 146
> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>
> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
> +
> /* Available with KVM_CAP_ARM_USER_IRQ */
>
> /* Bits for run->s.regs.device_irq_level */
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 1b219fa..965de1b 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>
> +#define KVM_PV_UNHALT (1U << 7)
> +
> #define KVM_HINTS_DEDICATED (1U << 0)
>
> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6c49954..3e99830 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
> }
>
> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
> +
> + if (disable_exits) {
> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> + KVM_X86_DISABLE_EXITS_HLT |
> + KVM_X86_DISABLE_EXITS_PAUSE);
> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> + }
In the future, if we decide to enable kvm-pv-unhalt by default,
should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
automatically, or should we require an explicit
"kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
For today's defaults, this patch solves the problem, only one
thing is missing before I give my R-b: we need to clearly
document what exactly are the consequences and requirements of
setting kvm-hint-dedicated=on (I'm not sure if the best place for
this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> + }
> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
> + error_report("kvm: DISABLE EXITS not supported");
> + }
> + }
> +
> qemu_add_vm_change_state_handler(cpu_update_state, env);
>
> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-17 20:59 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-04-17 20:59 UTC (permalink / raw)
To: Wanpeng Li; +Cc: qemu-devel, kvm, Paolo Bonzini, Radim Krčmář
On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
>
> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
> in order that to improve latency in some workloads.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>
> linux-headers/linux/kvm.h | 6 +++++-
> target/i386/cpu.h | 2 ++
> target/i386/kvm.c | 16 ++++++++++++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index a167be8..857df15 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_S390_GS 140
> #define KVM_CAP_S390_AIS 141
> #define KVM_CAP_SPAPR_TCE_VFIO 142
> -#define KVM_CAP_X86_GUEST_MWAIT 143
> +#define KVM_CAP_X86_DISABLE_EXITS 143
> #define KVM_CAP_ARM_USER_IRQ 144
> #define KVM_CAP_S390_CMMA_MIGRATION 145
> #define KVM_CAP_PPC_FWNMI 146
> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>
> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
> +
> /* Available with KVM_CAP_ARM_USER_IRQ */
>
> /* Bits for run->s.regs.device_irq_level */
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 1b219fa..965de1b 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>
> +#define KVM_PV_UNHALT (1U << 7)
> +
> #define KVM_HINTS_DEDICATED (1U << 0)
>
> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6c49954..3e99830 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
> }
>
> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
> +
> + if (disable_exits) {
> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> + KVM_X86_DISABLE_EXITS_HLT |
> + KVM_X86_DISABLE_EXITS_PAUSE);
> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> + }
In the future, if we decide to enable kvm-pv-unhalt by default,
should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
automatically, or should we require an explicit
"kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
For today's defaults, this patch solves the problem, only one
thing is missing before I give my R-b: we need to clearly
document what exactly are the consequences and requirements of
setting kvm-hint-dedicated=on (I'm not sure if the best place for
this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> + }
> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
> + error_report("kvm: DISABLE EXITS not supported");
> + }
> + }
> +
> qemu_add_vm_change_state_handler(cpu_update_state, env);
>
> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-17 18:08 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-04-18 1:09 ` Wanpeng Li
-1 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-04-18 1:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel@nongnu.org Developers,
kvm, Radim Krčmář
2018-04-18 2:08 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
> On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpengli@tencent.com>
>>
>> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
>> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
>> in order that to improve latency in some workloads.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
>> ---
>>
>> linux-headers/linux/kvm.h | 6 +++++-
>> target/i386/cpu.h | 2 ++
>> target/i386/kvm.c | 16 ++++++++++++++++
>> 3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>> index a167be8..857df15 100644
>> --- a/linux-headers/linux/kvm.h
>> +++ b/linux-headers/linux/kvm.h
>> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
>> #define KVM_CAP_S390_GS 140
>> #define KVM_CAP_S390_AIS 141
>> #define KVM_CAP_SPAPR_TCE_VFIO 142
>> -#define KVM_CAP_X86_GUEST_MWAIT 143
>> +#define KVM_CAP_X86_DISABLE_EXITS 143
>> #define KVM_CAP_ARM_USER_IRQ 144
>> #define KVM_CAP_S390_CMMA_MIGRATION 145
>> #define KVM_CAP_PPC_FWNMI 146
>> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
>> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
>> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>>
>> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
>> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
>> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
>> +
>> /* Available with KVM_CAP_ARM_USER_IRQ */
>>
>> /* Bits for run->s.regs.device_irq_level */
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index 1b219fa..965de1b 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
>> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>>
>> +#define KVM_PV_UNHALT (1U << 7)
>> +
>
> Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
>
>> #define KVM_HINTS_DEDICATED (1U << 0)
>>
>
> BTW I wonder whether we should switch to a value from
> kvm_para.h? I'll send a version to do it, pls take a look.
Yeah, your patchset looks good.
Regards,
Wanpeng Li
>
>
>> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 6c49954..3e99830 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> }
>> }
>>
>> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
>> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
>> +
>> + if (disable_exits) {
>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> + KVM_X86_DISABLE_EXITS_HLT |
>> + KVM_X86_DISABLE_EXITS_PAUSE);
>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>> + }
>> + }
>> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
>> + error_report("kvm: DISABLE EXITS not supported");
>> + }
>> + }
>> +
>> qemu_add_vm_change_state_handler(cpu_update_state, env);
>>
>> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
>> --
>> 2.7.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-18 1:09 ` Wanpeng Li
0 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-04-18 1:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel@nongnu.org Developers, kvm, Paolo Bonzini,
Radim Krčmář,
Eduardo Habkost
2018-04-18 2:08 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
> On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpengli@tencent.com>
>>
>> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
>> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
>> in order that to improve latency in some workloads.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
>> ---
>>
>> linux-headers/linux/kvm.h | 6 +++++-
>> target/i386/cpu.h | 2 ++
>> target/i386/kvm.c | 16 ++++++++++++++++
>> 3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>> index a167be8..857df15 100644
>> --- a/linux-headers/linux/kvm.h
>> +++ b/linux-headers/linux/kvm.h
>> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
>> #define KVM_CAP_S390_GS 140
>> #define KVM_CAP_S390_AIS 141
>> #define KVM_CAP_SPAPR_TCE_VFIO 142
>> -#define KVM_CAP_X86_GUEST_MWAIT 143
>> +#define KVM_CAP_X86_DISABLE_EXITS 143
>> #define KVM_CAP_ARM_USER_IRQ 144
>> #define KVM_CAP_S390_CMMA_MIGRATION 145
>> #define KVM_CAP_PPC_FWNMI 146
>> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
>> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
>> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>>
>> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
>> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
>> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
>> +
>> /* Available with KVM_CAP_ARM_USER_IRQ */
>>
>> /* Bits for run->s.regs.device_irq_level */
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index 1b219fa..965de1b 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
>> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>>
>> +#define KVM_PV_UNHALT (1U << 7)
>> +
>
> Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
>
>> #define KVM_HINTS_DEDICATED (1U << 0)
>>
>
> BTW I wonder whether we should switch to a value from
> kvm_para.h? I'll send a version to do it, pls take a look.
Yeah, your patchset looks good.
Regards,
Wanpeng Li
>
>
>> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 6c49954..3e99830 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> }
>> }
>>
>> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
>> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
>> +
>> + if (disable_exits) {
>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> + KVM_X86_DISABLE_EXITS_HLT |
>> + KVM_X86_DISABLE_EXITS_PAUSE);
>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>> + }
>> + }
>> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
>> + error_report("kvm: DISABLE EXITS not supported");
>> + }
>> + }
>> +
>> qemu_add_vm_change_state_handler(cpu_update_state, env);
>>
>> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
>> --
>> 2.7.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-17 20:59 ` [Qemu-devel] " Eduardo Habkost
@ 2018-04-18 1:20 ` Wanpeng Li
-1 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-04-18 1:20 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, qemu-devel@nongnu.org Developers, kvm,
Radim Krčmář
2018-04-18 4:59 GMT+08:00 Eduardo Habkost <ehabkost@redhat.com>:
> On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
[.../...]
>>
>> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
>> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
>> +
>> + if (disable_exits) {
>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> + KVM_X86_DISABLE_EXITS_HLT |
>> + KVM_X86_DISABLE_EXITS_PAUSE);
>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>> + }
>
> In the future, if we decide to enable kvm-pv-unhalt by default,
> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> automatically, or should we require an explicit
> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
>
> For today's defaults, this patch solves the problem, only one
> thing is missing before I give my R-b: we need to clearly
> document what exactly are the consequences and requirements of
> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
What's your opinion, Paolo?
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-18 1:20 ` Wanpeng Li
0 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-04-18 1:20 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel@nongnu.org Developers, kvm, Paolo Bonzini,
Radim Krčmář
2018-04-18 4:59 GMT+08:00 Eduardo Habkost <ehabkost@redhat.com>:
> On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
[.../...]
>>
>> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
>> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
>> +
>> + if (disable_exits) {
>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> + KVM_X86_DISABLE_EXITS_HLT |
>> + KVM_X86_DISABLE_EXITS_PAUSE);
>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>> + }
>
> In the future, if we decide to enable kvm-pv-unhalt by default,
> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> automatically, or should we require an explicit
> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
>
> For today's defaults, this patch solves the problem, only one
> thing is missing before I give my R-b: we need to clearly
> document what exactly are the consequences and requirements of
> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
What's your opinion, Paolo?
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-17 20:59 ` [Qemu-devel] " Eduardo Habkost
@ 2018-04-19 15:48 ` Paolo Bonzini
-1 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-04-19 15:48 UTC (permalink / raw)
To: Eduardo Habkost, Wanpeng Li; +Cc: qemu-devel, kvm, Radim Krčmář
On 17/04/2018 22:59, Eduardo Habkost wrote:
>> + if (disable_exits) {
>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> + KVM_X86_DISABLE_EXITS_HLT |
>> + KVM_X86_DISABLE_EXITS_PAUSE);
>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>> + }
>
> In the future, if we decide to enable kvm-pv-unhalt by default,
> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> automatically, or should we require an explicit
> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
It should be automatic.
> For today's defaults, this patch solves the problem, only one
> thing is missing before I give my R-b: we need to clearly
> document what exactly are the consequences and requirements of
> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
I don't think we have a good place for this kind of documentation,
unfortunately. Right now it is mentioned in
Documentation/virtual/kvm/cpuid.txt.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-19 15:48 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-04-19 15:48 UTC (permalink / raw)
To: Eduardo Habkost, Wanpeng Li; +Cc: qemu-devel, kvm, Radim Krčmář
On 17/04/2018 22:59, Eduardo Habkost wrote:
>> + if (disable_exits) {
>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> + KVM_X86_DISABLE_EXITS_HLT |
>> + KVM_X86_DISABLE_EXITS_PAUSE);
>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>> + }
>
> In the future, if we decide to enable kvm-pv-unhalt by default,
> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> automatically, or should we require an explicit
> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
It should be automatic.
> For today's defaults, this patch solves the problem, only one
> thing is missing before I give my R-b: we need to clearly
> document what exactly are the consequences and requirements of
> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
I don't think we have a good place for this kind of documentation,
unfortunately. Right now it is mentioned in
Documentation/virtual/kvm/cpuid.txt.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-19 15:48 ` [Qemu-devel] " Paolo Bonzini
@ 2018-04-19 19:56 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-04-19 19:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> On 17/04/2018 22:59, Eduardo Habkost wrote:
> >> + if (disable_exits) {
> >> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> >> + KVM_X86_DISABLE_EXITS_HLT |
> >> + KVM_X86_DISABLE_EXITS_PAUSE);
> >> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> >> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> >> + }
> >
> > In the future, if we decide to enable kvm-pv-unhalt by default,
> > should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > automatically, or should we require an explicit
> > "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
>
> It should be automatic.
>
> > For today's defaults, this patch solves the problem, only one
> > thing is missing before I give my R-b: we need to clearly
> > document what exactly are the consequences and requirements of
> > setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > this is qemu-options.hx, x86_cpu_list(), or somewhere else).
>
> I don't think we have a good place for this kind of documentation,
> unfortunately. Right now it is mentioned in
> Documentation/virtual/kvm/cpuid.txt.
With this patch, the QEMU option will do more than just setting
the CPUID bit, that's why I miss more detailed documentation on
the QEMU side. But I agree we have no obvious place for that
documentation.
In the worst case we can just add a code comment on top of
feature_word_info[FEAT_KVM_HINTS].feat_names warning that
kvm-hint-dedicated won't just enable the flag on CPUID and has
other side-effects.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-19 19:56 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-04-19 19:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> On 17/04/2018 22:59, Eduardo Habkost wrote:
> >> + if (disable_exits) {
> >> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> >> + KVM_X86_DISABLE_EXITS_HLT |
> >> + KVM_X86_DISABLE_EXITS_PAUSE);
> >> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> >> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> >> + }
> >
> > In the future, if we decide to enable kvm-pv-unhalt by default,
> > should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > automatically, or should we require an explicit
> > "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
>
> It should be automatic.
>
> > For today's defaults, this patch solves the problem, only one
> > thing is missing before I give my R-b: we need to clearly
> > document what exactly are the consequences and requirements of
> > setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > this is qemu-options.hx, x86_cpu_list(), or somewhere else).
>
> I don't think we have a good place for this kind of documentation,
> unfortunately. Right now it is mentioned in
> Documentation/virtual/kvm/cpuid.txt.
With this patch, the QEMU option will do more than just setting
the CPUID bit, that's why I miss more detailed documentation on
the QEMU side. But I agree we have no obvious place for that
documentation.
In the worst case we can just add a code comment on top of
feature_word_info[FEAT_KVM_HINTS].feat_names warning that
kvm-hint-dedicated won't just enable the flag on CPUID and has
other side-effects.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-19 19:56 ` [Qemu-devel] " Eduardo Habkost
@ 2018-04-19 21:32 ` Paolo Bonzini
-1 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-04-19 21:32 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On 19/04/2018 21:56, Eduardo Habkost wrote:
> On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
>> On 17/04/2018 22:59, Eduardo Habkost wrote:
>>>> + if (disable_exits) {
>>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>>>> + KVM_X86_DISABLE_EXITS_HLT |
>>>> + KVM_X86_DISABLE_EXITS_PAUSE);
>>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>>>> + }
>>>
>>> In the future, if we decide to enable kvm-pv-unhalt by default,
>>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
>>> automatically, or should we require an explicit
>>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
>>
>> It should be automatic.
>>
>>> For today's defaults, this patch solves the problem, only one
>>> thing is missing before I give my R-b: we need to clearly
>>> document what exactly are the consequences and requirements of
>>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
>>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
>>
>> I don't think we have a good place for this kind of documentation,
>> unfortunately. Right now it is mentioned in
>> Documentation/virtual/kvm/cpuid.txt.
>
> With this patch, the QEMU option will do more than just setting
> the CPUID bit, that's why I miss more detailed documentation on
> the QEMU side. But I agree we have no obvious place for that
> documentation.
>
> In the worst case we can just add a code comment on top of
> feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> kvm-hint-dedicated won't just enable the flag on CPUID and has
> other side-effects.
Maybe we should use "-realtime dedicated=on" instead of, or in addition
to kvm-hint-dedicated=on?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-19 21:32 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-04-19 21:32 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On 19/04/2018 21:56, Eduardo Habkost wrote:
> On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
>> On 17/04/2018 22:59, Eduardo Habkost wrote:
>>>> + if (disable_exits) {
>>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>>>> + KVM_X86_DISABLE_EXITS_HLT |
>>>> + KVM_X86_DISABLE_EXITS_PAUSE);
>>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
>>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
>>>> + }
>>>
>>> In the future, if we decide to enable kvm-pv-unhalt by default,
>>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
>>> automatically, or should we require an explicit
>>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
>>
>> It should be automatic.
>>
>>> For today's defaults, this patch solves the problem, only one
>>> thing is missing before I give my R-b: we need to clearly
>>> document what exactly are the consequences and requirements of
>>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
>>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
>>
>> I don't think we have a good place for this kind of documentation,
>> unfortunately. Right now it is mentioned in
>> Documentation/virtual/kvm/cpuid.txt.
>
> With this patch, the QEMU option will do more than just setting
> the CPUID bit, that's why I miss more detailed documentation on
> the QEMU side. But I agree we have no obvious place for that
> documentation.
>
> In the worst case we can just add a code comment on top of
> feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> kvm-hint-dedicated won't just enable the flag on CPUID and has
> other side-effects.
Maybe we should use "-realtime dedicated=on" instead of, or in addition
to kvm-hint-dedicated=on?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-19 21:32 ` [Qemu-devel] " Paolo Bonzini
@ 2018-04-19 21:53 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-04-19 21:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> On 19/04/2018 21:56, Eduardo Habkost wrote:
> > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> >>>> + if (disable_exits) {
> >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> >>>> + KVM_X86_DISABLE_EXITS_HLT |
> >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> >>>> + }
> >>>
> >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> >>> automatically, or should we require an explicit
> >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> >>
> >> It should be automatic.
> >>
> >>> For today's defaults, this patch solves the problem, only one
> >>> thing is missing before I give my R-b: we need to clearly
> >>> document what exactly are the consequences and requirements of
> >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> >>
> >> I don't think we have a good place for this kind of documentation,
> >> unfortunately. Right now it is mentioned in
> >> Documentation/virtual/kvm/cpuid.txt.
> >
> > With this patch, the QEMU option will do more than just setting
> > the CPUID bit, that's why I miss more detailed documentation on
> > the QEMU side. But I agree we have no obvious place for that
> > documentation.
> >
> > In the worst case we can just add a code comment on top of
> > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > other side-effects.
>
> Maybe we should use "-realtime dedicated=on" instead of, or in addition
> to kvm-hint-dedicated=on?
Maybe it's a better idea than overloading an option that is only
expected to control a CPUID bit.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-04-19 21:53 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-04-19 21:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> On 19/04/2018 21:56, Eduardo Habkost wrote:
> > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> >>>> + if (disable_exits) {
> >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> >>>> + KVM_X86_DISABLE_EXITS_HLT |
> >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> >>>> + }
> >>>
> >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> >>> automatically, or should we require an explicit
> >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> >>
> >> It should be automatic.
> >>
> >>> For today's defaults, this patch solves the problem, only one
> >>> thing is missing before I give my R-b: we need to clearly
> >>> document what exactly are the consequences and requirements of
> >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> >>
> >> I don't think we have a good place for this kind of documentation,
> >> unfortunately. Right now it is mentioned in
> >> Documentation/virtual/kvm/cpuid.txt.
> >
> > With this patch, the QEMU option will do more than just setting
> > the CPUID bit, that's why I miss more detailed documentation on
> > the QEMU side. But I agree we have no obvious place for that
> > documentation.
> >
> > In the worst case we can just add a code comment on top of
> > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > other side-effects.
>
> Maybe we should use "-realtime dedicated=on" instead of, or in addition
> to kvm-hint-dedicated=on?
Maybe it's a better idea than overloading an option that is only
expected to control a CPUID bit.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-18 1:09 ` [Qemu-devel] " Wanpeng Li
@ 2018-05-11 21:57 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-11 21:57 UTC (permalink / raw)
To: Wanpeng Li
Cc: Paolo Bonzini, Radim Krčmář,
Eduardo Habkost, kvm, qemu-devel@nongnu.org Developers
On Wed, Apr 18, 2018 at 09:09:19AM +0800, Wanpeng Li wrote:
> 2018-04-18 2:08 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
> > On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
> >> From: Wanpeng Li <wanpengli@tencent.com>
> >>
> >> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
> >> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
> >> in order that to improve latency in some workloads.
> >>
> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >> Cc: Radim Krčmář <rkrcmar@redhat.com>
> >> Cc: Eduardo Habkost <ehabkost@redhat.com>
> >> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> >> ---
> >>
> >> linux-headers/linux/kvm.h | 6 +++++-
> >> target/i386/cpu.h | 2 ++
> >> target/i386/kvm.c | 16 ++++++++++++++++
> >> 3 files changed, 23 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> >> index a167be8..857df15 100644
> >> --- a/linux-headers/linux/kvm.h
> >> +++ b/linux-headers/linux/kvm.h
> >> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
> >> #define KVM_CAP_S390_GS 140
> >> #define KVM_CAP_S390_AIS 141
> >> #define KVM_CAP_SPAPR_TCE_VFIO 142
> >> -#define KVM_CAP_X86_GUEST_MWAIT 143
> >> +#define KVM_CAP_X86_DISABLE_EXITS 143
> >> #define KVM_CAP_ARM_USER_IRQ 144
> >> #define KVM_CAP_S390_CMMA_MIGRATION 145
> >> #define KVM_CAP_PPC_FWNMI 146
> >> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
> >> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
> >> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
> >>
> >> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
> >> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
> >> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
> >> +
> >> /* Available with KVM_CAP_ARM_USER_IRQ */
> >>
> >> /* Bits for run->s.regs.device_irq_level */
> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> >> index 1b219fa..965de1b 100644
> >> --- a/target/i386/cpu.h
> >> +++ b/target/i386/cpu.h
> >> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> >> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
> >> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
> >>
> >> +#define KVM_PV_UNHALT (1U << 7)
> >> +
> >
> > Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
> >
> >> #define KVM_HINTS_DEDICATED (1U << 0)
> >>
> >
> > BTW I wonder whether we should switch to a value from
> > kvm_para.h? I'll send a version to do it, pls take a look.
>
> Yeah, your patchset looks good.
>
> Regards,
> Wanpeng Li
Do you plan to rebase your patch and upstream it or do you expect me to
do it?
> >
> >
> >> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
> >> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> >> index 6c49954..3e99830 100644
> >> --- a/target/i386/kvm.c
> >> +++ b/target/i386/kvm.c
> >> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >> }
> >> }
> >>
> >> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
> >> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
> >> +
> >> + if (disable_exits) {
> >> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> >> + KVM_X86_DISABLE_EXITS_HLT |
> >> + KVM_X86_DISABLE_EXITS_PAUSE);
> >> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> >> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> >> + }
> >> + }
> >> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
> >> + error_report("kvm: DISABLE EXITS not supported");
> >> + }
> >> + }
> >> +
> >> qemu_add_vm_change_state_handler(cpu_update_state, env);
> >>
> >> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> >> --
> >> 2.7.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-11 21:57 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-11 21:57 UTC (permalink / raw)
To: Wanpeng Li
Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel@nongnu.org Developers,
kvm, Radim Krčmář
On Wed, Apr 18, 2018 at 09:09:19AM +0800, Wanpeng Li wrote:
> 2018-04-18 2:08 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
> > On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
> >> From: Wanpeng Li <wanpengli@tencent.com>
> >>
> >> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
> >> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
> >> in order that to improve latency in some workloads.
> >>
> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >> Cc: Radim Krčmář <rkrcmar@redhat.com>
> >> Cc: Eduardo Habkost <ehabkost@redhat.com>
> >> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> >> ---
> >>
> >> linux-headers/linux/kvm.h | 6 +++++-
> >> target/i386/cpu.h | 2 ++
> >> target/i386/kvm.c | 16 ++++++++++++++++
> >> 3 files changed, 23 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> >> index a167be8..857df15 100644
> >> --- a/linux-headers/linux/kvm.h
> >> +++ b/linux-headers/linux/kvm.h
> >> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
> >> #define KVM_CAP_S390_GS 140
> >> #define KVM_CAP_S390_AIS 141
> >> #define KVM_CAP_SPAPR_TCE_VFIO 142
> >> -#define KVM_CAP_X86_GUEST_MWAIT 143
> >> +#define KVM_CAP_X86_DISABLE_EXITS 143
> >> #define KVM_CAP_ARM_USER_IRQ 144
> >> #define KVM_CAP_S390_CMMA_MIGRATION 145
> >> #define KVM_CAP_PPC_FWNMI 146
> >> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
> >> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
> >> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
> >>
> >> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
> >> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
> >> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
> >> +
> >> /* Available with KVM_CAP_ARM_USER_IRQ */
> >>
> >> /* Bits for run->s.regs.device_irq_level */
> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> >> index 1b219fa..965de1b 100644
> >> --- a/target/i386/cpu.h
> >> +++ b/target/i386/cpu.h
> >> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> >> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
> >> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
> >>
> >> +#define KVM_PV_UNHALT (1U << 7)
> >> +
> >
> > Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
> >
> >> #define KVM_HINTS_DEDICATED (1U << 0)
> >>
> >
> > BTW I wonder whether we should switch to a value from
> > kvm_para.h? I'll send a version to do it, pls take a look.
>
> Yeah, your patchset looks good.
>
> Regards,
> Wanpeng Li
Do you plan to rebase your patch and upstream it or do you expect me to
do it?
> >
> >
> >> #define CPUID_8000_0008_EBX_IBPB (1U << 12) /* Indirect Branch Prediction Barrier */
> >> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> >> index 6c49954..3e99830 100644
> >> --- a/target/i386/kvm.c
> >> +++ b/target/i386/kvm.c
> >> @@ -1029,6 +1029,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >> }
> >> }
> >>
> >> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
> >> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
> >> +
> >> + if (disable_exits) {
> >> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> >> + KVM_X86_DISABLE_EXITS_HLT |
> >> + KVM_X86_DISABLE_EXITS_PAUSE);
> >> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> >> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> >> + }
> >> + }
> >> + if (kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS, 0, disable_exits)) {
> >> + error_report("kvm: DISABLE EXITS not supported");
> >> + }
> >> + }
> >> +
> >> qemu_add_vm_change_state_handler(cpu_update_state, env);
> >>
> >> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> >> --
> >> 2.7.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-04-19 21:53 ` [Qemu-devel] " Eduardo Habkost
@ 2018-05-11 22:12 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-11 22:12 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > >>>> + if (disable_exits) {
> > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > >>>> + }
> > >>>
> > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > >>> automatically, or should we require an explicit
> > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > >>
> > >> It should be automatic.
> > >>
> > >>> For today's defaults, this patch solves the problem, only one
> > >>> thing is missing before I give my R-b: we need to clearly
> > >>> document what exactly are the consequences and requirements of
> > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > >>
> > >> I don't think we have a good place for this kind of documentation,
> > >> unfortunately. Right now it is mentioned in
> > >> Documentation/virtual/kvm/cpuid.txt.
> > >
> > > With this patch, the QEMU option will do more than just setting
> > > the CPUID bit, that's why I miss more detailed documentation on
> > > the QEMU side. But I agree we have no obvious place for that
> > > documentation.
> > >
> > > In the worst case we can just add a code comment on top of
> > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > other side-effects.
> >
> > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > to kvm-hint-dedicated=on?
>
> Maybe it's a better idea than overloading an option that is only
> expected to control a CPUID bit.
Well -realtime would be a bit confusing in that it enables mlock by
default.
>From pure API point of view, hint-dedicated looks good since
it seems to say "optimize for a dedicated host CPU" and
it's a hint in that guests keep working if you violate this
slightly once in a while.
But I agree there's a problem: right now "kvm-" means "KVM PV"
as opposed to e.g. HV enlightened gusts.
So if you enable hyperv and also want to disable halt existing,
what then? What should kvm-hint-dedicated=on do?
So how about a new hint-dedicated=on cpu flag? We can have that set
kvm-hint-dedicated if kvm PV is enabled.
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-11 22:12 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-11 22:12 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > >>>> + if (disable_exits) {
> > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > >>>> + }
> > >>>
> > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > >>> automatically, or should we require an explicit
> > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > >>
> > >> It should be automatic.
> > >>
> > >>> For today's defaults, this patch solves the problem, only one
> > >>> thing is missing before I give my R-b: we need to clearly
> > >>> document what exactly are the consequences and requirements of
> > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > >>
> > >> I don't think we have a good place for this kind of documentation,
> > >> unfortunately. Right now it is mentioned in
> > >> Documentation/virtual/kvm/cpuid.txt.
> > >
> > > With this patch, the QEMU option will do more than just setting
> > > the CPUID bit, that's why I miss more detailed documentation on
> > > the QEMU side. But I agree we have no obvious place for that
> > > documentation.
> > >
> > > In the worst case we can just add a code comment on top of
> > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > other side-effects.
> >
> > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > to kvm-hint-dedicated=on?
>
> Maybe it's a better idea than overloading an option that is only
> expected to control a CPUID bit.
Well -realtime would be a bit confusing in that it enables mlock by
default.
>From pure API point of view, hint-dedicated looks good since
it seems to say "optimize for a dedicated host CPU" and
it's a hint in that guests keep working if you violate this
slightly once in a while.
But I agree there's a problem: right now "kvm-" means "KVM PV"
as opposed to e.g. HV enlightened gusts.
So if you enable hyperv and also want to disable halt existing,
what then? What should kvm-hint-dedicated=on do?
So how about a new hint-dedicated=on cpu flag? We can have that set
kvm-hint-dedicated if kvm PV is enabled.
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-11 21:57 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-12 0:49 ` Wanpeng Li
-1 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-05-12 0:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Radim Krčmář,
Eduardo Habkost, kvm, qemu-devel@nongnu.org Developers
2018-05-12 5:57 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
> On Wed, Apr 18, 2018 at 09:09:19AM +0800, Wanpeng Li wrote:
>> 2018-04-18 2:08 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
>> > On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
>> >> From: Wanpeng Li <wanpengli@tencent.com>
>> >>
>> >> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
>> >> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
>> >> in order that to improve latency in some workloads.
>> >>
>> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> >> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> >> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> >> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
>> >> ---
>> >>
>> >> linux-headers/linux/kvm.h | 6 +++++-
>> >> target/i386/cpu.h | 2 ++
>> >> target/i386/kvm.c | 16 ++++++++++++++++
>> >> 3 files changed, 23 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>> >> index a167be8..857df15 100644
>> >> --- a/linux-headers/linux/kvm.h
>> >> +++ b/linux-headers/linux/kvm.h
>> >> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
>> >> #define KVM_CAP_S390_GS 140
>> >> #define KVM_CAP_S390_AIS 141
>> >> #define KVM_CAP_SPAPR_TCE_VFIO 142
>> >> -#define KVM_CAP_X86_GUEST_MWAIT 143
>> >> +#define KVM_CAP_X86_DISABLE_EXITS 143
>> >> #define KVM_CAP_ARM_USER_IRQ 144
>> >> #define KVM_CAP_S390_CMMA_MIGRATION 145
>> >> #define KVM_CAP_PPC_FWNMI 146
>> >> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
>> >> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
>> >> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>> >>
>> >> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
>> >> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
>> >> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
>> >> +
>> >> /* Available with KVM_CAP_ARM_USER_IRQ */
>> >>
>> >> /* Bits for run->s.regs.device_irq_level */
>> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> >> index 1b219fa..965de1b 100644
>> >> --- a/target/i386/cpu.h
>> >> +++ b/target/i386/cpu.h
>> >> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>> >> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
>> >> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>> >>
>> >> +#define KVM_PV_UNHALT (1U << 7)
>> >> +
>> >
>> > Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
>> >
>> >> #define KVM_HINTS_DEDICATED (1U << 0)
>> >>
>> >
>> > BTW I wonder whether we should switch to a value from
>> > kvm_para.h? I'll send a version to do it, pls take a look.
>>
>> Yeah, your patchset looks good.
>>
>> Regards,
>> Wanpeng Li
>
> Do you plan to rebase your patch and upstream it or do you expect me to
> do it?
You can pick it since I am too busy recently. Thanks Michael!
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-12 0:49 ` Wanpeng Li
0 siblings, 0 replies; 50+ messages in thread
From: Wanpeng Li @ 2018-05-12 0:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel@nongnu.org Developers,
kvm, Radim Krčmář
2018-05-12 5:57 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
> On Wed, Apr 18, 2018 at 09:09:19AM +0800, Wanpeng Li wrote:
>> 2018-04-18 2:08 GMT+08:00 Michael S. Tsirkin <mst@redhat.com>:
>> > On Tue, Apr 17, 2018 at 01:24:15AM -0700, Wanpeng Li wrote:
>> >> From: Wanpeng Li <wanpengli@tencent.com>
>> >>
>> >> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
>> >> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
>> >> in order that to improve latency in some workloads.
>> >>
>> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> >> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> >> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> >> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
>> >> ---
>> >>
>> >> linux-headers/linux/kvm.h | 6 +++++-
>> >> target/i386/cpu.h | 2 ++
>> >> target/i386/kvm.c | 16 ++++++++++++++++
>> >> 3 files changed, 23 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>> >> index a167be8..857df15 100644
>> >> --- a/linux-headers/linux/kvm.h
>> >> +++ b/linux-headers/linux/kvm.h
>> >> @@ -925,7 +925,7 @@ struct kvm_ppc_resize_hpt {
>> >> #define KVM_CAP_S390_GS 140
>> >> #define KVM_CAP_S390_AIS 141
>> >> #define KVM_CAP_SPAPR_TCE_VFIO 142
>> >> -#define KVM_CAP_X86_GUEST_MWAIT 143
>> >> +#define KVM_CAP_X86_DISABLE_EXITS 143
>> >> #define KVM_CAP_ARM_USER_IRQ 144
>> >> #define KVM_CAP_S390_CMMA_MIGRATION 145
>> >> #define KVM_CAP_PPC_FWNMI 146
>> >> @@ -1508,6 +1508,10 @@ struct kvm_assigned_msix_entry {
>> >> #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
>> >> #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
>> >>
>> >> +#define KVM_X86_DISABLE_EXITS_MWAIT (1 << 0)
>> >> +#define KVM_X86_DISABLE_EXITS_HLT (1 << 1)
>> >> +#define KVM_X86_DISABLE_EXITS_PAUSE (1 << 2)
>> >> +
>> >> /* Available with KVM_CAP_ARM_USER_IRQ */
>> >>
>> >> /* Bits for run->s.regs.device_irq_level */
>> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> >> index 1b219fa..965de1b 100644
>> >> --- a/target/i386/cpu.h
>> >> +++ b/target/i386/cpu.h
>> >> @@ -685,6 +685,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>> >> #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
>> >> #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>> >>
>> >> +#define KVM_PV_UNHALT (1U << 7)
>> >> +
>> >
>> > Why don't we use KVM_FEATURE_PV_UNHALT from kvm_para.h?
>> >
>> >> #define KVM_HINTS_DEDICATED (1U << 0)
>> >>
>> >
>> > BTW I wonder whether we should switch to a value from
>> > kvm_para.h? I'll send a version to do it, pls take a look.
>>
>> Yeah, your patchset looks good.
>>
>> Regards,
>> Wanpeng Li
>
> Do you plan to rebase your patch and upstream it or do you expect me to
> do it?
You can pick it since I am too busy recently. Thanks Michael!
Regards,
Wanpeng Li
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-11 22:12 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-16 12:34 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-16 12:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > >>>> + if (disable_exits) {
> > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > >>>> + }
> > > >>>
> > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > >>> automatically, or should we require an explicit
> > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > >>
> > > >> It should be automatic.
> > > >>
> > > >>> For today's defaults, this patch solves the problem, only one
> > > >>> thing is missing before I give my R-b: we need to clearly
> > > >>> document what exactly are the consequences and requirements of
> > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > >>
> > > >> I don't think we have a good place for this kind of documentation,
> > > >> unfortunately. Right now it is mentioned in
> > > >> Documentation/virtual/kvm/cpuid.txt.
> > > >
> > > > With this patch, the QEMU option will do more than just setting
> > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > the QEMU side. But I agree we have no obvious place for that
> > > > documentation.
> > > >
> > > > In the worst case we can just add a code comment on top of
> > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > other side-effects.
> > >
> > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > to kvm-hint-dedicated=on?
> >
> > Maybe it's a better idea than overloading an option that is only
> > expected to control a CPUID bit.
>
> Well -realtime would be a bit confusing in that it enables mlock by
> default.
>
> From pure API point of view, hint-dedicated looks good since
> it seems to say "optimize for a dedicated host CPU" and
> it's a hint in that guests keep working if you violate this
> slightly once in a while.
>
> But I agree there's a problem: right now "kvm-" means "KVM PV"
> as opposed to e.g. HV enlightened gusts.
> So if you enable hyperv and also want to disable halt existing,
> what then? What should kvm-hint-dedicated=on do?
>
> So how about a new hint-dedicated=on cpu flag? We can have that set
> kvm-hint-dedicated if kvm PV is enabled.
Using a boolean flag that is _not_ considered a CPUID feature
flag would be better. Using the CPUID feature flag name risks
having management software enabling the flag by accident (as it
will get included in query-cpu-model-* queries). A separate
boolean flag would make this clearer.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 12:34 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-16 12:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > >>>> + if (disable_exits) {
> > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > >>>> + }
> > > >>>
> > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > >>> automatically, or should we require an explicit
> > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > >>
> > > >> It should be automatic.
> > > >>
> > > >>> For today's defaults, this patch solves the problem, only one
> > > >>> thing is missing before I give my R-b: we need to clearly
> > > >>> document what exactly are the consequences and requirements of
> > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > >>
> > > >> I don't think we have a good place for this kind of documentation,
> > > >> unfortunately. Right now it is mentioned in
> > > >> Documentation/virtual/kvm/cpuid.txt.
> > > >
> > > > With this patch, the QEMU option will do more than just setting
> > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > the QEMU side. But I agree we have no obvious place for that
> > > > documentation.
> > > >
> > > > In the worst case we can just add a code comment on top of
> > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > other side-effects.
> > >
> > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > to kvm-hint-dedicated=on?
> >
> > Maybe it's a better idea than overloading an option that is only
> > expected to control a CPUID bit.
>
> Well -realtime would be a bit confusing in that it enables mlock by
> default.
>
> From pure API point of view, hint-dedicated looks good since
> it seems to say "optimize for a dedicated host CPU" and
> it's a hint in that guests keep working if you violate this
> slightly once in a while.
>
> But I agree there's a problem: right now "kvm-" means "KVM PV"
> as opposed to e.g. HV enlightened gusts.
> So if you enable hyperv and also want to disable halt existing,
> what then? What should kvm-hint-dedicated=on do?
>
> So how about a new hint-dedicated=on cpu flag? We can have that set
> kvm-hint-dedicated if kvm PV is enabled.
Using a boolean flag that is _not_ considered a CPUID feature
flag would be better. Using the CPUID feature flag name risks
having management software enabling the flag by accident (as it
will get included in query-cpu-model-* queries). A separate
boolean flag would make this clearer.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-11 22:12 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-16 12:44 ` Paolo Bonzini
-1 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-05-16 12:44 UTC (permalink / raw)
To: Michael S. Tsirkin, Eduardo Habkost
Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On 12/05/2018 00:12, Michael S. Tsirkin wrote:
>> Maybe it's a better idea than overloading an option that is only
>> expected to control a CPUID bit.
> Well -realtime would be a bit confusing in that it enables mlock by
> default.
Currently, the only suboption of "-realtime" is mlock, which means that
the only three valid uses of it are
-realtime mlock=on
-realtime mlock=off
-realtime ''
We can change the default, I think. Only the third would change
meaning, and it's a slightly crazy way to use the option.
> From pure API point of view, hint-dedicated looks good since
> it seems to say "optimize for a dedicated host CPU" and
> it's a hint in that guests keep working if you violate this
> slightly once in a while.
>
> But I agree there's a problem: right now "kvm-" means "KVM PV"
> as opposed to e.g. HV enlightened gusts.
> So if you enable hyperv and also want to disable halt existing,
> what then? What should kvm-hint-dedicated=on do?
kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
disables the vmexits. You can use the two independently.
Thanks,
Paolo
> So how about a new hint-dedicated=on cpu flag? We can have that set
> kvm-hint-dedicated if kvm PV is enabled.
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 12:44 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-05-16 12:44 UTC (permalink / raw)
To: Michael S. Tsirkin, Eduardo Habkost
Cc: Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On 12/05/2018 00:12, Michael S. Tsirkin wrote:
>> Maybe it's a better idea than overloading an option that is only
>> expected to control a CPUID bit.
> Well -realtime would be a bit confusing in that it enables mlock by
> default.
Currently, the only suboption of "-realtime" is mlock, which means that
the only three valid uses of it are
-realtime mlock=on
-realtime mlock=off
-realtime ''
We can change the default, I think. Only the third would change
meaning, and it's a slightly crazy way to use the option.
> From pure API point of view, hint-dedicated looks good since
> it seems to say "optimize for a dedicated host CPU" and
> it's a hint in that guests keep working if you violate this
> slightly once in a while.
>
> But I agree there's a problem: right now "kvm-" means "KVM PV"
> as opposed to e.g. HV enlightened gusts.
> So if you enable hyperv and also want to disable halt existing,
> what then? What should kvm-hint-dedicated=on do?
kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
disables the vmexits. You can use the two independently.
Thanks,
Paolo
> So how about a new hint-dedicated=on cpu flag? We can have that set
> kvm-hint-dedicated if kvm PV is enabled.
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 12:44 ` [Qemu-devel] " Paolo Bonzini
@ 2018-05-16 14:22 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 14:22 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Wanpeng Li, Radim Krčmář,
Eduardo Habkost, kvm, qemu-devel
On Wed, May 16, 2018 at 02:44:24PM +0200, Paolo Bonzini wrote:
> On 12/05/2018 00:12, Michael S. Tsirkin wrote:
> >> Maybe it's a better idea than overloading an option that is only
> >> expected to control a CPUID bit.
> > Well -realtime would be a bit confusing in that it enables mlock by
> > default.
>
> Currently, the only suboption of "-realtime" is mlock, which means that
> the only three valid uses of it are
>
> -realtime mlock=on
> -realtime mlock=off
> -realtime ''
>
> We can change the default, I think. Only the third would change
> meaning, and it's a slightly crazy way to use the option.
>
> > From pure API point of view, hint-dedicated looks good since
> > it seems to say "optimize for a dedicated host CPU" and
> > it's a hint in that guests keep working if you violate this
> > slightly once in a while.
> >
> > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > as opposed to e.g. HV enlightened gusts.
> > So if you enable hyperv and also want to disable halt existing,
> > what then? What should kvm-hint-dedicated=on do?
>
> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> disables the vmexits. You can use the two independently.
But when would you want to use the two independently?
> Thanks,
>
> Paolo
>
> > So how about a new hint-dedicated=on cpu flag? We can have that set
> > kvm-hint-dedicated if kvm PV is enabled.
> >
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 14:22 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 14:22 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Eduardo Habkost, Wanpeng Li, qemu-devel, kvm,
Radim Krčmář
On Wed, May 16, 2018 at 02:44:24PM +0200, Paolo Bonzini wrote:
> On 12/05/2018 00:12, Michael S. Tsirkin wrote:
> >> Maybe it's a better idea than overloading an option that is only
> >> expected to control a CPUID bit.
> > Well -realtime would be a bit confusing in that it enables mlock by
> > default.
>
> Currently, the only suboption of "-realtime" is mlock, which means that
> the only three valid uses of it are
>
> -realtime mlock=on
> -realtime mlock=off
> -realtime ''
>
> We can change the default, I think. Only the third would change
> meaning, and it's a slightly crazy way to use the option.
>
> > From pure API point of view, hint-dedicated looks good since
> > it seems to say "optimize for a dedicated host CPU" and
> > it's a hint in that guests keep working if you violate this
> > slightly once in a while.
> >
> > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > as opposed to e.g. HV enlightened gusts.
> > So if you enable hyperv and also want to disable halt existing,
> > what then? What should kvm-hint-dedicated=on do?
>
> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> disables the vmexits. You can use the two independently.
But when would you want to use the two independently?
> Thanks,
>
> Paolo
>
> > So how about a new hint-dedicated=on cpu flag? We can have that set
> > kvm-hint-dedicated if kvm PV is enabled.
> >
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 12:34 ` [Qemu-devel] " Eduardo Habkost
@ 2018-05-16 14:26 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 14:26 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > >>>> + if (disable_exits) {
> > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > >>>> + }
> > > > >>>
> > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > >>> automatically, or should we require an explicit
> > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > >>
> > > > >> It should be automatic.
> > > > >>
> > > > >>> For today's defaults, this patch solves the problem, only one
> > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > >>> document what exactly are the consequences and requirements of
> > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > >>
> > > > >> I don't think we have a good place for this kind of documentation,
> > > > >> unfortunately. Right now it is mentioned in
> > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > >
> > > > > With this patch, the QEMU option will do more than just setting
> > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > documentation.
> > > > >
> > > > > In the worst case we can just add a code comment on top of
> > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > other side-effects.
> > > >
> > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > to kvm-hint-dedicated=on?
> > >
> > > Maybe it's a better idea than overloading an option that is only
> > > expected to control a CPUID bit.
> >
> > Well -realtime would be a bit confusing in that it enables mlock by
> > default.
> >
> > From pure API point of view, hint-dedicated looks good since
> > it seems to say "optimize for a dedicated host CPU" and
> > it's a hint in that guests keep working if you violate this
> > slightly once in a while.
> >
> > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > as opposed to e.g. HV enlightened gusts.
> > So if you enable hyperv and also want to disable halt existing,
> > what then? What should kvm-hint-dedicated=on do?
> >
> > So how about a new hint-dedicated=on cpu flag? We can have that set
> > kvm-hint-dedicated if kvm PV is enabled.
>
> Using a boolean flag that is _not_ considered a CPUID feature
> flag would be better. Using the CPUID feature flag name risks
> having management software enabling the flag by accident (as it
> will get included in query-cpu-model-* queries). A separate
> boolean flag would make this clearer.
Can we remove all hints from query-cpu-model queries?
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 14:26 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 14:26 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > >>>> + if (disable_exits) {
> > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > >>>> + }
> > > > >>>
> > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > >>> automatically, or should we require an explicit
> > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > >>
> > > > >> It should be automatic.
> > > > >>
> > > > >>> For today's defaults, this patch solves the problem, only one
> > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > >>> document what exactly are the consequences and requirements of
> > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > >>
> > > > >> I don't think we have a good place for this kind of documentation,
> > > > >> unfortunately. Right now it is mentioned in
> > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > >
> > > > > With this patch, the QEMU option will do more than just setting
> > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > documentation.
> > > > >
> > > > > In the worst case we can just add a code comment on top of
> > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > other side-effects.
> > > >
> > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > to kvm-hint-dedicated=on?
> > >
> > > Maybe it's a better idea than overloading an option that is only
> > > expected to control a CPUID bit.
> >
> > Well -realtime would be a bit confusing in that it enables mlock by
> > default.
> >
> > From pure API point of view, hint-dedicated looks good since
> > it seems to say "optimize for a dedicated host CPU" and
> > it's a hint in that guests keep working if you violate this
> > slightly once in a while.
> >
> > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > as opposed to e.g. HV enlightened gusts.
> > So if you enable hyperv and also want to disable halt existing,
> > what then? What should kvm-hint-dedicated=on do?
> >
> > So how about a new hint-dedicated=on cpu flag? We can have that set
> > kvm-hint-dedicated if kvm PV is enabled.
>
> Using a boolean flag that is _not_ considered a CPUID feature
> flag would be better. Using the CPUID feature flag name risks
> having management software enabling the flag by accident (as it
> will get included in query-cpu-model-* queries). A separate
> boolean flag would make this clearer.
Can we remove all hints from query-cpu-model queries?
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 14:22 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-16 15:04 ` Paolo Bonzini
-1 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-05-16 15:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wanpeng Li, Radim Krčmář,
Eduardo Habkost, kvm, qemu-devel
On 16/05/2018 16:22, Michael S. Tsirkin wrote:
>> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
>> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
>> disables the vmexits. You can use the two independently.
>
> But when would you want to use the two independently?
1) For testing
2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
you may still want to use -realtime dedicated-cpus=on to get better
performance on the new one.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 15:04 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2018-05-16 15:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eduardo Habkost, Wanpeng Li, qemu-devel, kvm,
Radim Krčmář
On 16/05/2018 16:22, Michael S. Tsirkin wrote:
>> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
>> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
>> disables the vmexits. You can use the two independently.
>
> But when would you want to use the two independently?
1) For testing
2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
you may still want to use -realtime dedicated-cpus=on to get better
performance on the new one.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 15:04 ` [Qemu-devel] " Paolo Bonzini
@ 2018-05-16 15:13 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 15:13 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Wanpeng Li, Radim Krčmář,
Eduardo Habkost, kvm, qemu-devel
On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> >> disables the vmexits. You can use the two independently.
> >
> > But when would you want to use the two independently?
>
> 1) For testing
>
> 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> you may still want to use -realtime dedicated-cpus=on to get better
> performance on the new one.
>
> Paolo
For the second purpose, can't we handle this using machine types?
--
MST
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 15:13 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 15:13 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Eduardo Habkost, Wanpeng Li, qemu-devel, kvm,
Radim Krčmář
On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> >> disables the vmexits. You can use the two independently.
> >
> > But when would you want to use the two independently?
>
> 1) For testing
>
> 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> you may still want to use -realtime dedicated-cpus=on to get better
> performance on the new one.
>
> Paolo
For the second purpose, can't we handle this using machine types?
--
MST
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 15:13 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-16 15:33 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-16 15:33 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 06:13:17PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> > On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> > >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> > >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> > >> disables the vmexits. You can use the two independently.
> > >
> > > But when would you want to use the two independently?
> >
> > 1) For testing
> >
> > 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> > you may still want to use -realtime dedicated-cpus=on to get better
> > performance on the new one.
> >
> > Paolo
>
> For the second purpose, can't we handle this using machine types?
Machine-type compatibility code deals with defaults when options
are omitted, not for making the meaning of explicit options
depend on the machine-type.
e.g. having "-machine pc-q35-2.11 -cpu ...,+kvm-hint-dedicated=on"
not expose the CPUID bit that was explicitly requested in the
command-line would be a bad idea.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 15:33 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-16 15:33 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 06:13:17PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> > On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> > >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> > >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> > >> disables the vmexits. You can use the two independently.
> > >
> > > But when would you want to use the two independently?
> >
> > 1) For testing
> >
> > 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> > you may still want to use -realtime dedicated-cpus=on to get better
> > performance on the new one.
> >
> > Paolo
>
> For the second purpose, can't we handle this using machine types?
Machine-type compatibility code deals with defaults when options
are omitted, not for making the meaning of explicit options
depend on the machine-type.
e.g. having "-machine pc-q35-2.11 -cpu ...,+kvm-hint-dedicated=on"
not expose the CPUID bit that was explicitly requested in the
command-line would be a bad idea.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 15:33 ` [Qemu-devel] " Eduardo Habkost
@ 2018-05-16 16:21 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 16:21 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 12:33:50PM -0300, Eduardo Habkost wrote:
> On Wed, May 16, 2018 at 06:13:17PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> > > On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> > > >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> > > >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> > > >> disables the vmexits. You can use the two independently.
> > > >
> > > > But when would you want to use the two independently?
> > >
> > > 1) For testing
> > >
> > > 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> > > you may still want to use -realtime dedicated-cpus=on to get better
> > > performance on the new one.
> > >
> > > Paolo
> >
> > For the second purpose, can't we handle this using machine types?
>
> Machine-type compatibility code deals with defaults when options
> are omitted, not for making the meaning of explicit options
> depend on the machine-type.
>
> e.g. having "-machine pc-q35-2.11 -cpu ...,+kvm-hint-dedicated=on"
> not expose the CPUID bit that was explicitly requested in the
> command-line would be a bad idea.
Why? We have machine type affecting guest visible device behaviours
for years.
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 16:21 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-16 16:21 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 12:33:50PM -0300, Eduardo Habkost wrote:
> On Wed, May 16, 2018 at 06:13:17PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> > > On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> > > >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> > > >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> > > >> disables the vmexits. You can use the two independently.
> > > >
> > > > But when would you want to use the two independently?
> > >
> > > 1) For testing
> > >
> > > 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> > > you may still want to use -realtime dedicated-cpus=on to get better
> > > performance on the new one.
> > >
> > > Paolo
> >
> > For the second purpose, can't we handle this using machine types?
>
> Machine-type compatibility code deals with defaults when options
> are omitted, not for making the meaning of explicit options
> depend on the machine-type.
>
> e.g. having "-machine pc-q35-2.11 -cpu ...,+kvm-hint-dedicated=on"
> not expose the CPUID bit that was explicitly requested in the
> command-line would be a bad idea.
Why? We have machine type affecting guest visible device behaviours
for years.
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 16:21 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-16 17:20 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-16 17:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 07:21:04PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2018 at 12:33:50PM -0300, Eduardo Habkost wrote:
> > On Wed, May 16, 2018 at 06:13:17PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> > > > On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> > > > >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> > > > >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> > > > >> disables the vmexits. You can use the two independently.
> > > > >
> > > > > But when would you want to use the two independently?
> > > >
> > > > 1) For testing
> > > >
> > > > 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> > > > you may still want to use -realtime dedicated-cpus=on to get better
> > > > performance on the new one.
> > > >
> > > > Paolo
> > >
> > > For the second purpose, can't we handle this using machine types?
> >
> > Machine-type compatibility code deals with defaults when options
> > are omitted, not for making the meaning of explicit options
> > depend on the machine-type.
> >
> > e.g. having "-machine pc-q35-2.11 -cpu ...,+kvm-hint-dedicated=on"
> > not expose the CPUID bit that was explicitly requested in the
> > command-line would be a bad idea.
>
> Why? We have machine type affecting guest visible device behaviours
> for years.
Do you have an example where a machine-type overrides an option
explicitly set by the user?
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-16 17:20 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-16 17:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 07:21:04PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2018 at 12:33:50PM -0300, Eduardo Habkost wrote:
> > On Wed, May 16, 2018 at 06:13:17PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 16, 2018 at 05:04:41PM +0200, Paolo Bonzini wrote:
> > > > On 16/05/2018 16:22, Michael S. Tsirkin wrote:
> > > > >> kvm-hint-dedicated=on only sets the CPUID bit, which Linux for example
> > > > >> uses that to disable pv spinlocks. "-realtime dedicated-cpus=on" only
> > > > >> disables the vmexits. You can use the two independently.
> > > > >
> > > > > But when would you want to use the two independently?
> > > >
> > > > 1) For testing
> > > >
> > > > 2) When some of your QEMUs are too old to support kvm-hint-dedicated=on,
> > > > you may still want to use -realtime dedicated-cpus=on to get better
> > > > performance on the new one.
> > > >
> > > > Paolo
> > >
> > > For the second purpose, can't we handle this using machine types?
> >
> > Machine-type compatibility code deals with defaults when options
> > are omitted, not for making the meaning of explicit options
> > depend on the machine-type.
> >
> > e.g. having "-machine pc-q35-2.11 -cpu ...,+kvm-hint-dedicated=on"
> > not expose the CPUID bit that was explicitly requested in the
> > command-line would be a bad idea.
>
> Why? We have machine type affecting guest visible device behaviours
> for years.
Do you have an example where a machine-type overrides an option
explicitly set by the user?
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-16 14:26 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-17 17:34 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-17 17:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 05:26:40PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> > On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > > >>>> + if (disable_exits) {
> > > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > > >>>> + }
> > > > > >>>
> > > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > > >>> automatically, or should we require an explicit
> > > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > > >>
> > > > > >> It should be automatic.
> > > > > >>
> > > > > >>> For today's defaults, this patch solves the problem, only one
> > > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > > >>> document what exactly are the consequences and requirements of
> > > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > > >>
> > > > > >> I don't think we have a good place for this kind of documentation,
> > > > > >> unfortunately. Right now it is mentioned in
> > > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > > >
> > > > > > With this patch, the QEMU option will do more than just setting
> > > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > > documentation.
> > > > > >
> > > > > > In the worst case we can just add a code comment on top of
> > > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > > other side-effects.
> > > > >
> > > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > > to kvm-hint-dedicated=on?
> > > >
> > > > Maybe it's a better idea than overloading an option that is only
> > > > expected to control a CPUID bit.
> > >
> > > Well -realtime would be a bit confusing in that it enables mlock by
> > > default.
> > >
> > > From pure API point of view, hint-dedicated looks good since
> > > it seems to say "optimize for a dedicated host CPU" and
> > > it's a hint in that guests keep working if you violate this
> > > slightly once in a while.
> > >
> > > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > > as opposed to e.g. HV enlightened gusts.
> > > So if you enable hyperv and also want to disable halt existing,
> > > what then? What should kvm-hint-dedicated=on do?
> > >
> > > So how about a new hint-dedicated=on cpu flag? We can have that set
> > > kvm-hint-dedicated if kvm PV is enabled.
> >
> > Using a boolean flag that is _not_ considered a CPUID feature
> > flag would be better. Using the CPUID feature flag name risks
> > having management software enabling the flag by accident (as it
> > will get included in query-cpu-model-* queries). A separate
> > boolean flag would make this clearer.
>
> Can we remove all hints from query-cpu-model queries?
We already do (see usage of EatureWordInfo::no_autoenable_flags).
This is just a matter of making the configuration option
decoupled from the CPUID code, to avoid surprises elsewhere.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-17 17:34 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-17 17:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Wed, May 16, 2018 at 05:26:40PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> > On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > > >>>> + if (disable_exits) {
> > > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > > >>>> + }
> > > > > >>>
> > > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > > >>> automatically, or should we require an explicit
> > > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > > >>
> > > > > >> It should be automatic.
> > > > > >>
> > > > > >>> For today's defaults, this patch solves the problem, only one
> > > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > > >>> document what exactly are the consequences and requirements of
> > > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > > >>
> > > > > >> I don't think we have a good place for this kind of documentation,
> > > > > >> unfortunately. Right now it is mentioned in
> > > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > > >
> > > > > > With this patch, the QEMU option will do more than just setting
> > > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > > documentation.
> > > > > >
> > > > > > In the worst case we can just add a code comment on top of
> > > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > > other side-effects.
> > > > >
> > > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > > to kvm-hint-dedicated=on?
> > > >
> > > > Maybe it's a better idea than overloading an option that is only
> > > > expected to control a CPUID bit.
> > >
> > > Well -realtime would be a bit confusing in that it enables mlock by
> > > default.
> > >
> > > From pure API point of view, hint-dedicated looks good since
> > > it seems to say "optimize for a dedicated host CPU" and
> > > it's a hint in that guests keep working if you violate this
> > > slightly once in a while.
> > >
> > > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > > as opposed to e.g. HV enlightened gusts.
> > > So if you enable hyperv and also want to disable halt existing,
> > > what then? What should kvm-hint-dedicated=on do?
> > >
> > > So how about a new hint-dedicated=on cpu flag? We can have that set
> > > kvm-hint-dedicated if kvm PV is enabled.
> >
> > Using a boolean flag that is _not_ considered a CPUID feature
> > flag would be better. Using the CPUID feature flag name risks
> > having management software enabling the flag by accident (as it
> > will get included in query-cpu-model-* queries). A separate
> > boolean flag would make this clearer.
>
> Can we remove all hints from query-cpu-model queries?
We already do (see usage of EatureWordInfo::no_autoenable_flags).
This is just a matter of making the configuration option
decoupled from the CPUID code, to avoid surprises elsewhere.
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-17 17:34 ` [Qemu-devel] " Eduardo Habkost
@ 2018-05-17 17:57 ` Michael S. Tsirkin
-1 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-17 17:57 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Thu, May 17, 2018 at 02:34:28PM -0300, Eduardo Habkost wrote:
> On Wed, May 16, 2018 at 05:26:40PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> > > On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > > > >>>> + if (disable_exits) {
> > > > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > > > >>>> + }
> > > > > > >>>
> > > > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > > > >>> automatically, or should we require an explicit
> > > > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > > > >>
> > > > > > >> It should be automatic.
> > > > > > >>
> > > > > > >>> For today's defaults, this patch solves the problem, only one
> > > > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > > > >>> document what exactly are the consequences and requirements of
> > > > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > > > >>
> > > > > > >> I don't think we have a good place for this kind of documentation,
> > > > > > >> unfortunately. Right now it is mentioned in
> > > > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > > > >
> > > > > > > With this patch, the QEMU option will do more than just setting
> > > > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > > > documentation.
> > > > > > >
> > > > > > > In the worst case we can just add a code comment on top of
> > > > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > > > other side-effects.
> > > > > >
> > > > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > > > to kvm-hint-dedicated=on?
> > > > >
> > > > > Maybe it's a better idea than overloading an option that is only
> > > > > expected to control a CPUID bit.
> > > >
> > > > Well -realtime would be a bit confusing in that it enables mlock by
> > > > default.
> > > >
> > > > From pure API point of view, hint-dedicated looks good since
> > > > it seems to say "optimize for a dedicated host CPU" and
> > > > it's a hint in that guests keep working if you violate this
> > > > slightly once in a while.
> > > >
> > > > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > > > as opposed to e.g. HV enlightened gusts.
> > > > So if you enable hyperv and also want to disable halt existing,
> > > > what then? What should kvm-hint-dedicated=on do?
> > > >
> > > > So how about a new hint-dedicated=on cpu flag? We can have that set
> > > > kvm-hint-dedicated if kvm PV is enabled.
> > >
> > > Using a boolean flag that is _not_ considered a CPUID feature
> > > flag would be better. Using the CPUID feature flag name risks
> > > having management software enabling the flag by accident (as it
> > > will get included in query-cpu-model-* queries). A separate
> > > boolean flag would make this clearer.
> >
> > Can we remove all hints from query-cpu-model queries?
>
> We already do (see usage of EatureWordInfo::no_autoenable_flags).
> This is just a matter of making the configuration option
> decoupled from the CPUID code, to avoid surprises elsewhere.
It it too late to drop the hint flag and rename to a -realtime option?
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-17 17:57 ` Michael S. Tsirkin
0 siblings, 0 replies; 50+ messages in thread
From: Michael S. Tsirkin @ 2018-05-17 17:57 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Thu, May 17, 2018 at 02:34:28PM -0300, Eduardo Habkost wrote:
> On Wed, May 16, 2018 at 05:26:40PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> > > On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > > > >>>> + if (disable_exits) {
> > > > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > > > >>>> + }
> > > > > > >>>
> > > > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > > > >>> automatically, or should we require an explicit
> > > > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > > > >>
> > > > > > >> It should be automatic.
> > > > > > >>
> > > > > > >>> For today's defaults, this patch solves the problem, only one
> > > > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > > > >>> document what exactly are the consequences and requirements of
> > > > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > > > >>
> > > > > > >> I don't think we have a good place for this kind of documentation,
> > > > > > >> unfortunately. Right now it is mentioned in
> > > > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > > > >
> > > > > > > With this patch, the QEMU option will do more than just setting
> > > > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > > > documentation.
> > > > > > >
> > > > > > > In the worst case we can just add a code comment on top of
> > > > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > > > other side-effects.
> > > > > >
> > > > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > > > to kvm-hint-dedicated=on?
> > > > >
> > > > > Maybe it's a better idea than overloading an option that is only
> > > > > expected to control a CPUID bit.
> > > >
> > > > Well -realtime would be a bit confusing in that it enables mlock by
> > > > default.
> > > >
> > > > From pure API point of view, hint-dedicated looks good since
> > > > it seems to say "optimize for a dedicated host CPU" and
> > > > it's a hint in that guests keep working if you violate this
> > > > slightly once in a while.
> > > >
> > > > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > > > as opposed to e.g. HV enlightened gusts.
> > > > So if you enable hyperv and also want to disable halt existing,
> > > > what then? What should kvm-hint-dedicated=on do?
> > > >
> > > > So how about a new hint-dedicated=on cpu flag? We can have that set
> > > > kvm-hint-dedicated if kvm PV is enabled.
> > >
> > > Using a boolean flag that is _not_ considered a CPUID feature
> > > flag would be better. Using the CPUID feature flag name risks
> > > having management software enabling the flag by accident (as it
> > > will get included in query-cpu-model-* queries). A separate
> > > boolean flag would make this clearer.
> >
> > Can we remove all hints from query-cpu-model queries?
>
> We already do (see usage of EatureWordInfo::no_autoenable_flags).
> This is just a matter of making the configuration option
> decoupled from the CPUID code, to avoid surprises elsewhere.
It it too late to drop the hint flag and rename to a -realtime option?
> --
> Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
2018-05-17 17:57 ` [Qemu-devel] " Michael S. Tsirkin
@ 2018-05-17 18:18 ` Eduardo Habkost
-1 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-17 18:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wanpeng Li, Paolo Bonzini, qemu-devel, kvm, Radim Krčmář
On Thu, May 17, 2018 at 08:57:04PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 17, 2018 at 02:34:28PM -0300, Eduardo Habkost wrote:
> > On Wed, May 16, 2018 at 05:26:40PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> > > > On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > > > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > > > > >>>> + if (disable_exits) {
> > > > > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > > > > >>>> + }
> > > > > > > >>>
> > > > > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > > > > >>> automatically, or should we require an explicit
> > > > > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > > > > >>
> > > > > > > >> It should be automatic.
> > > > > > > >>
> > > > > > > >>> For today's defaults, this patch solves the problem, only one
> > > > > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > > > > >>> document what exactly are the consequences and requirements of
> > > > > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > > > > >>
> > > > > > > >> I don't think we have a good place for this kind of documentation,
> > > > > > > >> unfortunately. Right now it is mentioned in
> > > > > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > > > > >
> > > > > > > > With this patch, the QEMU option will do more than just setting
> > > > > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > > > > documentation.
> > > > > > > >
> > > > > > > > In the worst case we can just add a code comment on top of
> > > > > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > > > > other side-effects.
> > > > > > >
> > > > > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > > > > to kvm-hint-dedicated=on?
> > > > > >
> > > > > > Maybe it's a better idea than overloading an option that is only
> > > > > > expected to control a CPUID bit.
> > > > >
> > > > > Well -realtime would be a bit confusing in that it enables mlock by
> > > > > default.
> > > > >
> > > > > From pure API point of view, hint-dedicated looks good since
> > > > > it seems to say "optimize for a dedicated host CPU" and
> > > > > it's a hint in that guests keep working if you violate this
> > > > > slightly once in a while.
> > > > >
> > > > > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > > > > as opposed to e.g. HV enlightened gusts.
> > > > > So if you enable hyperv and also want to disable halt existing,
> > > > > what then? What should kvm-hint-dedicated=on do?
> > > > >
> > > > > So how about a new hint-dedicated=on cpu flag? We can have that set
> > > > > kvm-hint-dedicated if kvm PV is enabled.
> > > >
> > > > Using a boolean flag that is _not_ considered a CPUID feature
> > > > flag would be better. Using the CPUID feature flag name risks
> > > > having management software enabling the flag by accident (as it
> > > > will get included in query-cpu-model-* queries). A separate
> > > > boolean flag would make this clearer.
> > >
> > > Can we remove all hints from query-cpu-model queries?
> >
> > We already do (see usage of EatureWordInfo::no_autoenable_flags).
> > This is just a matter of making the configuration option
> > decoupled from the CPUID code, to avoid surprises elsewhere.
>
> It it too late to drop the hint flag and rename to a -realtime option?
We can't remove support for "-cpu ...,kvm-pv-dedicated=on"
without a deprecation notice, as it's already in v2.12.0.
But it's not too late to make other side-effects (e.g. disabling
VM exits) be controlled by -realtime or other command-line
option. It's also not too late to move kvm-pv-dedicated from
the feat_names array to x86_cpu_properties to avoid confusion.
The existing behavior of "-cpu ...,kvm-hint-dedicated=on" is to
only set the CPUID bit and do nothing else that could have
unwanted side-effects (like disabling VM exits).
Do you see a problem in simply keeping the existing behavior?
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS
@ 2018-05-17 18:18 ` Eduardo Habkost
0 siblings, 0 replies; 50+ messages in thread
From: Eduardo Habkost @ 2018-05-17 18:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Wanpeng Li, qemu-devel, kvm, Radim Krčmář
On Thu, May 17, 2018 at 08:57:04PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 17, 2018 at 02:34:28PM -0300, Eduardo Habkost wrote:
> > On Wed, May 16, 2018 at 05:26:40PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 16, 2018 at 09:34:52AM -0300, Eduardo Habkost wrote:
> > > > On Sat, May 12, 2018 at 01:12:59AM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, Apr 19, 2018 at 06:53:20PM -0300, Eduardo Habkost wrote:
> > > > > > On Thu, Apr 19, 2018 at 11:32:16PM +0200, Paolo Bonzini wrote:
> > > > > > > On 19/04/2018 21:56, Eduardo Habkost wrote:
> > > > > > > > On Thu, Apr 19, 2018 at 05:48:57PM +0200, Paolo Bonzini wrote:
> > > > > > > >> On 17/04/2018 22:59, Eduardo Habkost wrote:
> > > > > > > >>>> + if (disable_exits) {
> > > > > > > >>>> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
> > > > > > > >>>> + KVM_X86_DISABLE_EXITS_HLT |
> > > > > > > >>>> + KVM_X86_DISABLE_EXITS_PAUSE);
> > > > > > > >>>> + if (env->user_features[FEAT_KVM] & KVM_PV_UNHALT) {
> > > > > > > >>>> + disable_exits &= ~KVM_X86_DISABLE_EXITS_HLT;
> > > > > > > >>>> + }
> > > > > > > >>>
> > > > > > > >>> In the future, if we decide to enable kvm-pv-unhalt by default,
> > > > > > > >>> should "-cpu ...,kvm-hint-dedicated=on" disable kvm-pv-unhalt
> > > > > > > >>> automatically, or should we require an explicit
> > > > > > > >>> "kvm-hint-dedicated=on,kvm-pv-unhalt=off" option?
> > > > > > > >>
> > > > > > > >> It should be automatic.
> > > > > > > >>
> > > > > > > >>> For today's defaults, this patch solves the problem, only one
> > > > > > > >>> thing is missing before I give my R-b: we need to clearly
> > > > > > > >>> document what exactly are the consequences and requirements of
> > > > > > > >>> setting kvm-hint-dedicated=on (I'm not sure if the best place for
> > > > > > > >>> this is qemu-options.hx, x86_cpu_list(), or somewhere else).
> > > > > > > >>
> > > > > > > >> I don't think we have a good place for this kind of documentation,
> > > > > > > >> unfortunately. Right now it is mentioned in
> > > > > > > >> Documentation/virtual/kvm/cpuid.txt.
> > > > > > > >
> > > > > > > > With this patch, the QEMU option will do more than just setting
> > > > > > > > the CPUID bit, that's why I miss more detailed documentation on
> > > > > > > > the QEMU side. But I agree we have no obvious place for that
> > > > > > > > documentation.
> > > > > > > >
> > > > > > > > In the worst case we can just add a code comment on top of
> > > > > > > > feature_word_info[FEAT_KVM_HINTS].feat_names warning that
> > > > > > > > kvm-hint-dedicated won't just enable the flag on CPUID and has
> > > > > > > > other side-effects.
> > > > > > >
> > > > > > > Maybe we should use "-realtime dedicated=on" instead of, or in addition
> > > > > > > to kvm-hint-dedicated=on?
> > > > > >
> > > > > > Maybe it's a better idea than overloading an option that is only
> > > > > > expected to control a CPUID bit.
> > > > >
> > > > > Well -realtime would be a bit confusing in that it enables mlock by
> > > > > default.
> > > > >
> > > > > From pure API point of view, hint-dedicated looks good since
> > > > > it seems to say "optimize for a dedicated host CPU" and
> > > > > it's a hint in that guests keep working if you violate this
> > > > > slightly once in a while.
> > > > >
> > > > > But I agree there's a problem: right now "kvm-" means "KVM PV"
> > > > > as opposed to e.g. HV enlightened gusts.
> > > > > So if you enable hyperv and also want to disable halt existing,
> > > > > what then? What should kvm-hint-dedicated=on do?
> > > > >
> > > > > So how about a new hint-dedicated=on cpu flag? We can have that set
> > > > > kvm-hint-dedicated if kvm PV is enabled.
> > > >
> > > > Using a boolean flag that is _not_ considered a CPUID feature
> > > > flag would be better. Using the CPUID feature flag name risks
> > > > having management software enabling the flag by accident (as it
> > > > will get included in query-cpu-model-* queries). A separate
> > > > boolean flag would make this clearer.
> > >
> > > Can we remove all hints from query-cpu-model queries?
> >
> > We already do (see usage of EatureWordInfo::no_autoenable_flags).
> > This is just a matter of making the configuration option
> > decoupled from the CPUID code, to avoid surprises elsewhere.
>
> It it too late to drop the hint flag and rename to a -realtime option?
We can't remove support for "-cpu ...,kvm-pv-dedicated=on"
without a deprecation notice, as it's already in v2.12.0.
But it's not too late to make other side-effects (e.g. disabling
VM exits) be controlled by -realtime or other command-line
option. It's also not too late to move kvm-pv-dedicated from
the feat_names array to x86_cpu_properties to avoid confusion.
The existing behavior of "-cpu ...,kvm-hint-dedicated=on" is to
only set the CPUID bit and do nothing else that could have
unwanted side-effects (like disabling VM exits).
Do you see a problem in simply keeping the existing behavior?
--
Eduardo
^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2018-05-17 18:18 UTC | newest]
Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-17 8:24 [PATCH RESEND v2] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS Wanpeng Li
2018-04-17 8:24 ` [Qemu-devel] " Wanpeng Li
2018-04-17 8:28 ` no-reply
2018-04-17 8:28 ` [Qemu-devel] " no-reply
2018-04-17 18:08 ` Michael S. Tsirkin
2018-04-17 18:08 ` [Qemu-devel] " Michael S. Tsirkin
2018-04-18 1:09 ` Wanpeng Li
2018-04-18 1:09 ` [Qemu-devel] " Wanpeng Li
2018-05-11 21:57 ` Michael S. Tsirkin
2018-05-11 21:57 ` [Qemu-devel] " Michael S. Tsirkin
2018-05-12 0:49 ` Wanpeng Li
2018-05-12 0:49 ` [Qemu-devel] " Wanpeng Li
2018-04-17 20:59 ` Eduardo Habkost
2018-04-17 20:59 ` [Qemu-devel] " Eduardo Habkost
2018-04-18 1:20 ` Wanpeng Li
2018-04-18 1:20 ` [Qemu-devel] " Wanpeng Li
2018-04-19 15:48 ` Paolo Bonzini
2018-04-19 15:48 ` [Qemu-devel] " Paolo Bonzini
2018-04-19 19:56 ` Eduardo Habkost
2018-04-19 19:56 ` [Qemu-devel] " Eduardo Habkost
2018-04-19 21:32 ` Paolo Bonzini
2018-04-19 21:32 ` [Qemu-devel] " Paolo Bonzini
2018-04-19 21:53 ` Eduardo Habkost
2018-04-19 21:53 ` [Qemu-devel] " Eduardo Habkost
2018-05-11 22:12 ` Michael S. Tsirkin
2018-05-11 22:12 ` [Qemu-devel] " Michael S. Tsirkin
2018-05-16 12:34 ` Eduardo Habkost
2018-05-16 12:34 ` [Qemu-devel] " Eduardo Habkost
2018-05-16 14:26 ` Michael S. Tsirkin
2018-05-16 14:26 ` [Qemu-devel] " Michael S. Tsirkin
2018-05-17 17:34 ` Eduardo Habkost
2018-05-17 17:34 ` [Qemu-devel] " Eduardo Habkost
2018-05-17 17:57 ` Michael S. Tsirkin
2018-05-17 17:57 ` [Qemu-devel] " Michael S. Tsirkin
2018-05-17 18:18 ` Eduardo Habkost
2018-05-17 18:18 ` [Qemu-devel] " Eduardo Habkost
2018-05-16 12:44 ` Paolo Bonzini
2018-05-16 12:44 ` [Qemu-devel] " Paolo Bonzini
2018-05-16 14:22 ` Michael S. Tsirkin
2018-05-16 14:22 ` [Qemu-devel] " Michael S. Tsirkin
2018-05-16 15:04 ` Paolo Bonzini
2018-05-16 15:04 ` [Qemu-devel] " Paolo Bonzini
2018-05-16 15:13 ` Michael S. Tsirkin
2018-05-16 15:13 ` [Qemu-devel] " Michael S. Tsirkin
2018-05-16 15:33 ` Eduardo Habkost
2018-05-16 15:33 ` [Qemu-devel] " Eduardo Habkost
2018-05-16 16:21 ` Michael S. Tsirkin
2018-05-16 16:21 ` [Qemu-devel] " Michael S. Tsirkin
2018-05-16 17:20 ` Eduardo Habkost
2018-05-16 17:20 ` [Qemu-devel] " Eduardo Habkost
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.