| Message ID | 20200228085905.22495-2-oupton@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] KVM: SVM: Inhibit APIC virtualization for X2APIC guest | expand |
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?
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. >
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
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 >
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
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?
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
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(-)