* [PATCH v2 1/2] KVM: SVM: Inhibit APIC virtualization for X2APIC guest
@ 2020-02-28 8:59 Oliver Upton
2020-02-28 8:59 ` [PATCH v2 2/2] KVM: SVM: Enable AVIC by default Oliver Upton
0 siblings, 1 reply; 8+ messages in thread
From: Oliver Upton @ 2020-02-28 8:59 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Jim Mattson, Oliver Upton
The AVIC does not support guest use of the x2APIC interface. Currently,
KVM simply chooses to squash the x2APIC feature in the guest's CPUID
If the AVIC is enabled. Doing so prevents KVM from running a guest
with greater than 255 vCPUs, as such a guest necessitates the use
of the x2APIC interface.
Instead, inhibit AVIC enablement on a per-VM basis whenever the x2APIC
feature is set in the guest's CPUID.
Signed-off-by: Oliver Upton <oupton@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>
---
Parent commit: a93236fcbe1d ("KVM: s390: rstify new ioctls in api.rst")
v1 => v2:
- Adopt Paolo's suggestion to inhibit the AVIC by default rather than
requiring opt-in
- Drop now unnecessary module parameter
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/svm.c | 15 +++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 98959e8cd448..9d40132a3ae2 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -890,6 +890,7 @@ enum kvm_irqchip_mode {
#define APICV_INHIBIT_REASON_NESTED 2
#define APICV_INHIBIT_REASON_IRQWIN 3
#define APICV_INHIBIT_REASON_PIT_REINJ 4
+#define APICV_INHIBIT_REASON_X2APIC 5
struct kvm_arch {
unsigned long n_used_mmu_pages;
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ad3f5b178a03..b82a500bccb7 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -6027,7 +6027,13 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
if (!kvm_vcpu_apicv_active(vcpu))
return;
- guest_cpuid_clear(vcpu, X86_FEATURE_X2APIC);
+ /*
+ * AVIC does not work with an x2APIC mode guest. If the X2APIC feature
+ * is exposed to the guest, disable AVIC.
+ */
+ if (guest_cpuid_has(vcpu, X86_FEATURE_X2APIC))
+ kvm_request_apicv_update(vcpu->kvm, false,
+ APICV_INHIBIT_REASON_X2APIC);
/*
* Currently, AVIC does not work with nested virtualization.
@@ -6043,10 +6049,6 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
{
switch (func) {
- case 0x1:
- if (avic)
- entry->ecx &= ~F(X2APIC);
- break;
case 0x80000001:
if (nested)
entry->ecx |= (1 << 2); /* Set SVM bit */
@@ -7370,7 +7372,8 @@ static bool svm_check_apicv_inhibit_reasons(ulong bit)
BIT(APICV_INHIBIT_REASON_HYPERV) |
BIT(APICV_INHIBIT_REASON_NESTED) |
BIT(APICV_INHIBIT_REASON_IRQWIN) |
- BIT(APICV_INHIBIT_REASON_PIT_REINJ);
+ BIT(APICV_INHIBIT_REASON_PIT_REINJ) |
+ BIT(APICV_INHIBIT_REASON_X2APIC);
return supported & BIT(bit);
}
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/2] KVM: SVM: Enable AVIC by default
2020-02-28 8:59 [PATCH v2 1/2] KVM: SVM: Inhibit APIC virtualization for X2APIC guest Oliver Upton
@ 2020-02-28 8:59 ` Oliver Upton
2020-02-28 19:40 ` Jim Mattson
0 siblings, 1 reply; 8+ messages in thread
From: Oliver Upton @ 2020-02-28 8:59 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Jim Mattson, Oliver Upton
Switch the default value of the 'avic' module parameter to 1.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Oliver Upton <oupton@google.com>
---
arch/x86/kvm/svm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index b82a500bccb7..70d2df13eb02 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -358,7 +358,7 @@ static int nested = true;
module_param(nested, int, S_IRUGO);
/* enable / disable AVIC */
-static int avic;
+static int avic = 1;
#ifdef CONFIG_X86_LOCAL_APIC
module_param(avic, int, S_IRUGO);
#endif
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: SVM: Enable AVIC by default
2020-02-28 8:59 ` [PATCH v2 2/2] KVM: SVM: Enable AVIC by default Oliver Upton
@ 2020-02-28 19:40 ` Jim Mattson
2020-02-28 22:47 ` Wei Huang
0 siblings, 1 reply; 8+ messages in thread
From: Jim Mattson @ 2020-02-28 19:40 UTC (permalink / raw)
To: Oliver Upton; +Cc: kvm list, Paolo Bonzini, Tom Lendacky
On Fri, Feb 28, 2020 at 12:59 AM Oliver Upton <oupton@google.com> wrote:
>
> Switch the default value of the 'avic' module parameter to 1.
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
> arch/x86/kvm/svm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index b82a500bccb7..70d2df13eb02 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -358,7 +358,7 @@ static int nested = true;
> module_param(nested, int, S_IRUGO);
>
> /* enable / disable AVIC */
> -static int avic;
> +static int avic = 1;
> #ifdef CONFIG_X86_LOCAL_APIC
> module_param(avic, int, S_IRUGO);
> #endif
> --
> 2.25.1.481.gfbce0eb801-goog
How extensively has this been tested? Why hasn't someone from AMD
suggested this change?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: SVM: Enable AVIC by default
2020-02-28 19:40 ` Jim Mattson
@ 2020-02-28 22:47 ` Wei Huang
2020-03-02 16:19 ` Paolo Bonzini
2020-03-02 17:40 ` Jim Mattson
0 siblings, 2 replies; 8+ messages in thread
From: Wei Huang @ 2020-02-28 22:47 UTC (permalink / raw)
To: Jim Mattson, Oliver Upton; +Cc: kvm list, Paolo Bonzini, Tom Lendacky
On 2/28/20 1:40 PM, Jim Mattson wrote:
> On Fri, Feb 28, 2020 at 12:59 AM Oliver Upton <oupton@google.com> wrote:
>>
>> Switch the default value of the 'avic' module parameter to 1.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Oliver Upton <oupton@google.com>
>> ---
>> arch/x86/kvm/svm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index b82a500bccb7..70d2df13eb02 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -358,7 +358,7 @@ static int nested = true;
>> module_param(nested, int, S_IRUGO);
>>
>> /* enable / disable AVIC */
>> -static int avic;
>> +static int avic = 1;
>> #ifdef CONFIG_X86_LOCAL_APIC
>> module_param(avic, int, S_IRUGO);
>> #endif
>> --
>> 2.25.1.481.gfbce0eb801-goog
>
> How extensively has this been tested? Why hasn't someone from AMD
> suggested this change?
I personally don't suggest enable AVIC by default. There are cases of
slow AVIC doorbell delivery, due to delivery path and contention under a
large number of guest cores.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: SVM: Enable AVIC by default
2020-02-28 22:47 ` Wei Huang
@ 2020-03-02 16:19 ` Paolo Bonzini
2020-03-02 17:11 ` Wei Huang
2020-03-02 17:40 ` Jim Mattson
1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2020-03-02 16:19 UTC (permalink / raw)
To: Wei Huang, Jim Mattson, Oliver Upton; +Cc: kvm list, Tom Lendacky
On 28/02/20 23:47, Wei Huang wrote:
>> How extensively has this been tested? Why hasn't someone from AMD
>> suggested this change?
>
> I personally don't suggest enable AVIC by default. There are cases of
> slow AVIC doorbell delivery, due to delivery path and contention under a
> large number of guest cores.
To clarify, this is a hardware issue, right?
Note that in practice this patch series wouldn't change much, because
x2apic is enabled by default by userspace (it has better performance
than memory-mapped APIC) and patch 1 in turn inhibits APICv if the guest
has the X2APIC CPUID bit set.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: SVM: Enable AVIC by default
2020-03-02 16:19 ` Paolo Bonzini
@ 2020-03-02 17:11 ` Wei Huang
2020-03-02 17:35 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: Wei Huang @ 2020-03-02 17:11 UTC (permalink / raw)
To: Paolo Bonzini, Wei Huang, Jim Mattson, Oliver Upton
Cc: kvm list, Tom Lendacky
On 3/2/20 10:19 AM, Paolo Bonzini wrote:
> On 28/02/20 23:47, Wei Huang wrote:
>>> How extensively has this been tested? Why hasn't someone from AMD
>>> suggested this change?
>>
>> I personally don't suggest enable AVIC by default. There are cases of
>> slow AVIC doorbell delivery, due to delivery path and contention under a
>> large number of guest cores.
>
> To clarify, this is a hardware issue, right?
>
> Note that in practice this patch series wouldn't change much, because
> x2apic is enabled by default by userspace (it has better performance
> than memory-mapped APIC) and patch 1 in turn inhibits APICv if the guest
> has the X2APIC CPUID bit set.
QEMU will work fine with this option ON due to x2APIC, just like what
you said. If you feel other emulators using KVM will behave similarly, I
can revoke my concern.
>
> Paolo
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: SVM: Enable AVIC by default
2020-03-02 17:11 ` Wei Huang
@ 2020-03-02 17:35 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2020-03-02 17:35 UTC (permalink / raw)
To: Wei Huang, Wei Huang, Jim Mattson, Oliver Upton; +Cc: kvm list, Tom Lendacky
On 02/03/20 18:11, Wei Huang wrote:
>>> I personally don't suggest enable AVIC by default. There are cases of
>>> slow AVIC doorbell delivery, due to delivery path and contention under a
>>> large number of guest cores.
>> To clarify, this is a hardware issue, right?
>>
>> Note that in practice this patch series wouldn't change much, because
>> x2apic is enabled by default by userspace (it has better performance
>> than memory-mapped APIC) and patch 1 in turn inhibits APICv if the guest
>> has the X2APIC CPUID bit set.
> QEMU will work fine with this option ON due to x2APIC, just like what
> you said. If you feel other emulators using KVM will behave similarly, I
> can revoke my concern.
No, it's okay for now to leave it disabled. It would be nice if this
information would be more easily available than by you lurking on the
mailing list. :)
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] KVM: SVM: Enable AVIC by default
2020-02-28 22:47 ` Wei Huang
2020-03-02 16:19 ` Paolo Bonzini
@ 2020-03-02 17:40 ` Jim Mattson
1 sibling, 0 replies; 8+ messages in thread
From: Jim Mattson @ 2020-03-02 17:40 UTC (permalink / raw)
To: Wei Huang
Cc: Oliver Upton, kvm list, Paolo Bonzini, Tom Lendacky, David Rientjes
On Fri, Feb 28, 2020 at 2:47 PM Wei Huang <whuang2@amd.com> wrote:
> I personally don't suggest enable AVIC by default. There are cases of
> slow AVIC doorbell delivery, due to delivery path and contention under a
> large number of guest cores.
Under what conditions would you suggest enabling AVIC?
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-02 17:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28 8:59 [PATCH v2 1/2] KVM: SVM: Inhibit APIC virtualization for X2APIC guest Oliver Upton
2020-02-28 8:59 ` [PATCH v2 2/2] KVM: SVM: Enable AVIC by default Oliver Upton
2020-02-28 19:40 ` Jim Mattson
2020-02-28 22:47 ` Wei Huang
2020-03-02 16:19 ` Paolo Bonzini
2020-03-02 17:11 ` Wei Huang
2020-03-02 17:35 ` Paolo Bonzini
2020-03-02 17:40 ` Jim Mattson
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.