| Message ID | 20210518144339.1987982-4-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: hyper-v: Conditionally allow SynIC with APICv/AVIC | expand |
On Tue, May 18, 2021, Vitaly Kuznetsov wrote: > Unify VMX and SVM code by moving APICv/AVIC enablement tracking to common > 'enable_apicv' variable. Note: unlike APICv, AVIC is disabled by default. > > No functional change intended. > > Suggested-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- ... > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 8c3918a11826..0d6ec34d1e4b 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -185,6 +185,10 @@ module_param(vls, int, 0444); > static int vgif = true; > module_param(vgif, int, 0444); > > +/* enable / disable AVIC */ > +static int avic; > +module_param(avic, int, 0444); We should opportunistically make avic a "bool". > + > bool __read_mostly dump_invalid_vmcb; > module_param(dump_invalid_vmcb, bool, 0644); > > @@ -1009,14 +1013,15 @@ static __init int svm_hardware_setup(void) > nrips = false; > } > > - if (avic) { > - if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) { > - avic = false; > - } else { > - pr_info("AVIC enabled\n"); > + if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) > + avic = false; > > - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > - } > + /* 'enable_apicv' is common between VMX/SVM but the defaults differ */ -1 for not throwing Jim under the bus :-) Nits aside, Reviewed-by: Sean Christopherson <seanjc@google.com>
Sean Christopherson <seanjc@google.com> writes: > On Tue, May 18, 2021, Vitaly Kuznetsov wrote: ... >> + /* 'enable_apicv' is common between VMX/SVM but the defaults differ */ > > -1 for not throwing Jim under the bus :-) I bought the 'messenger' argument :-) > > Nits aside, > > Reviewed-by: Sean Christopherson <seanjc@google.com> > Thanks!
On 18/05/21 22:39, Sean Christopherson wrote: >> +/* enable / disable AVIC */ >> +static int avic; >> +module_param(avic, int, 0444); > We should opportunistically make avic a "bool". > And also: diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 11714c22c9f1..48cb498ff070 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -185,9 +185,12 @@ module_param(vls, int, 0444); static int vgif = true; module_param(vgif, int, 0444); -/* enable / disable AVIC */ -static int avic; -module_param(avic, int, 0444); +/* + * enable / disable AVIC. Because the defaults differ for APICv + * support between VMX and SVM we cannot use module_param_named. + */ +static bool avic; +module_param(avic, bool, 0444); bool __read_mostly dump_invalid_vmcb; module_param(dump_invalid_vmcb, bool, 0644); @@ -1013,11 +1016,7 @@ static __init int svm_hardware_setup(void) nrips = false; } - if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) - avic = false; - - /* 'enable_apicv' is common between VMX/SVM but the defaults differ */ - enable_apicv = avic; + enable_apicv = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); if (enable_apicv) { pr_info("AVIC enabled\n"); The "if" can come back when AVIC is enabled by default. Paolo
On Mon, May 24, 2021, Paolo Bonzini wrote: > On 18/05/21 22:39, Sean Christopherson wrote: > > > +/* enable / disable AVIC */ > > > +static int avic; > > > +module_param(avic, int, 0444); > > We should opportunistically make avic a "bool". > > > > And also: > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 11714c22c9f1..48cb498ff070 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -185,9 +185,12 @@ module_param(vls, int, 0444); > static int vgif = true; > module_param(vgif, int, 0444); > -/* enable / disable AVIC */ > -static int avic; > -module_param(avic, int, 0444); > +/* > + * enable / disable AVIC. Because the defaults differ for APICv > + * support between VMX and SVM we cannot use module_param_named. > + */ > +static bool avic; > +module_param(avic, bool, 0444); > bool __read_mostly dump_invalid_vmcb; > module_param(dump_invalid_vmcb, bool, 0644); > @@ -1013,11 +1016,7 @@ static __init int svm_hardware_setup(void) > nrips = false; > } > - if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) > - avic = false; > - > - /* 'enable_apicv' is common between VMX/SVM but the defaults differ */ > - enable_apicv = avic; > + enable_apicv = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); > if (enable_apicv) { > pr_info("AVIC enabled\n"); > > The "if" can come back when AVIC is enabled by default. But "avic" is connected to the module param, even if it's off by default its effective value should be reflected in sysfs. E.g. the user may incorrectly think AVIC is in use if they set avic=1 but the CPU doesn't support AVIC. Forcing the user to check /proc/cpuinfo or look for "AVIC enabled" in dmesg is kludgy at best.
On 24/05/21 18:52, Sean Christopherson wrote: > On Mon, May 24, 2021, Paolo Bonzini wrote: >> On 18/05/21 22:39, Sean Christopherson wrote: >>>> +/* enable / disable AVIC */ >>>> +static int avic; >>>> +module_param(avic, int, 0444); >>> We should opportunistically make avic a "bool". >>> >> >> And also: >> >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >> index 11714c22c9f1..48cb498ff070 100644 >> --- a/arch/x86/kvm/svm/svm.c >> +++ b/arch/x86/kvm/svm/svm.c >> @@ -185,9 +185,12 @@ module_param(vls, int, 0444); >> static int vgif = true; >> module_param(vgif, int, 0444); >> -/* enable / disable AVIC */ >> -static int avic; >> -module_param(avic, int, 0444); >> +/* >> + * enable / disable AVIC. Because the defaults differ for APICv >> + * support between VMX and SVM we cannot use module_param_named. >> + */ >> +static bool avic; >> +module_param(avic, bool, 0444); >> bool __read_mostly dump_invalid_vmcb; >> module_param(dump_invalid_vmcb, bool, 0644); >> @@ -1013,11 +1016,7 @@ static __init int svm_hardware_setup(void) >> nrips = false; >> } >> - if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) >> - avic = false; >> - >> - /* 'enable_apicv' is common between VMX/SVM but the defaults differ */ >> - enable_apicv = avic; >> + enable_apicv = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); >> if (enable_apicv) { >> pr_info("AVIC enabled\n"); >> >> The "if" can come back when AVIC is enabled by default. > > But "avic" is connected to the module param, even if it's off by default its > effective value should be reflected in sysfs. E.g. the user may incorrectly > think AVIC is in use if they set avic=1 but the CPU doesn't support AVIC. > Forcing the user to check /proc/cpuinfo or look for "AVIC enabled" in dmesg is > kludgy at best. Indeed -- I even tested the above, but only before realizing that module_param_named would change the default. So for now this needs to be "enable_apicv = avic = ...", and later it can become just enable_apicv &= npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); Paolo
On Tue, 2021-05-18 at 16:43 +0200, Vitaly Kuznetsov wrote: > Unify VMX and SVM code by moving APICv/AVIC enablement tracking to common > 'enable_apicv' variable. Note: unlike APICv, AVIC is disabled by default. > > No functional change intended. > > Suggested-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/svm/avic.c | 14 +++++--------- > arch/x86/kvm/svm/svm.c | 23 ++++++++++++++--------- > arch/x86/kvm/svm/svm.h | 2 -- > arch/x86/kvm/vmx/capabilities.h | 1 - > arch/x86/kvm/vmx/vmx.c | 1 - > arch/x86/kvm/x86.c | 3 +++ > 7 files changed, 23 insertions(+), 22 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 55efbacfc244..a2197fcf0e7c 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1422,6 +1422,7 @@ struct kvm_arch_async_pf { > extern u32 __read_mostly kvm_nr_uret_msrs; > extern u64 __read_mostly host_efer; > extern bool __read_mostly allow_smaller_maxphyaddr; > +extern bool __read_mostly enable_apicv; > extern struct kvm_x86_ops kvm_x86_ops; > > #define KVM_X86_OP(func) \ > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 1c1bf911e02b..05cd0b128b02 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -27,10 +27,6 @@ > #include "irq.h" > #include "svm.h" > > -/* enable / disable AVIC */ > -int avic; > -module_param(avic, int, S_IRUGO); > - > #define SVM_AVIC_DOORBELL 0xc001011b > > #define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF) > @@ -124,7 +120,7 @@ void avic_vm_destroy(struct kvm *kvm) > unsigned long flags; > struct kvm_svm *kvm_svm = to_kvm_svm(kvm); > > - if (!avic) > + if (!enable_apicv) > return; > > if (kvm_svm->avic_logical_id_table_page) > @@ -147,7 +143,7 @@ int avic_vm_init(struct kvm *kvm) > struct page *l_page; > u32 vm_id; > > - if (!avic) > + if (!enable_apicv) > return 0; > > /* Allocating physical APIC ID table (4KB) */ > @@ -569,7 +565,7 @@ int avic_init_vcpu(struct vcpu_svm *svm) > int ret; > struct kvm_vcpu *vcpu = &svm->vcpu; > > - if (!avic || !irqchip_in_kernel(vcpu->kvm)) > + if (!enable_apicv || !irqchip_in_kernel(vcpu->kvm)) > return 0; > > ret = avic_init_backing_page(vcpu); > @@ -593,7 +589,7 @@ void avic_post_state_restore(struct kvm_vcpu *vcpu) > > void svm_toggle_avic_for_irq_window(struct kvm_vcpu *vcpu, bool activate) > { > - if (!avic || !lapic_in_kernel(vcpu)) > + if (!enable_apicv || !lapic_in_kernel(vcpu)) > return; > > srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); > @@ -653,7 +649,7 @@ void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu) > struct vmcb *vmcb = svm->vmcb; > bool activated = kvm_vcpu_apicv_active(vcpu); > > - if (!avic) > + if (!enable_apicv) > return; > > if (activated) { > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 8c3918a11826..0d6ec34d1e4b 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -185,6 +185,10 @@ module_param(vls, int, 0444); > static int vgif = true; > module_param(vgif, int, 0444); > > +/* enable / disable AVIC */ > +static int avic; > +module_param(avic, int, 0444); > + > bool __read_mostly dump_invalid_vmcb; > module_param(dump_invalid_vmcb, bool, 0644); > > @@ -1009,14 +1013,15 @@ static __init int svm_hardware_setup(void) > nrips = false; > } > > - if (avic) { > - if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) { > - avic = false; > - } else { > - pr_info("AVIC enabled\n"); > + if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) > + avic = false; > > - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > - } > + /* 'enable_apicv' is common between VMX/SVM but the defaults differ */ > + enable_apicv = avic; > + if (enable_apicv) { > + pr_info("AVIC enabled\n"); > + > + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > } > > if (vls) { > @@ -4427,13 +4432,13 @@ static int svm_vm_init(struct kvm *kvm) > if (!pause_filter_count || !pause_filter_thresh) > kvm->arch.pause_in_guest = true; > > - if (avic) { > + if (enable_apicv) { > int ret = avic_vm_init(kvm); > if (ret) > return ret; > } > > - kvm_apicv_init(kvm, avic); > + kvm_apicv_init(kvm, enable_apicv); > return 0; > } > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index e44567ceb865..a514b490db4a 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -479,8 +479,6 @@ extern struct kvm_x86_nested_ops svm_nested_ops; > > #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL > > -extern int avic; > - > static inline void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data) > { > svm->vmcb->control.avic_vapic_bar = data & VMCB_AVIC_APIC_BAR_MASK; > diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h > index aa0e7872fcc9..4705ad55abb5 100644 > --- a/arch/x86/kvm/vmx/capabilities.h > +++ b/arch/x86/kvm/vmx/capabilities.h > @@ -12,7 +12,6 @@ extern bool __read_mostly enable_ept; > extern bool __read_mostly enable_unrestricted_guest; > extern bool __read_mostly enable_ept_ad_bits; > extern bool __read_mostly enable_pml; > -extern bool __read_mostly enable_apicv; > extern int __read_mostly pt_mode; > > #define PT_MODE_SYSTEM 0 > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 4bceb5ca3a89..5e9ba10e9c2d 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -101,7 +101,6 @@ module_param(emulate_invalid_guest_state, bool, S_IRUGO); > static bool __read_mostly fasteoi = 1; > module_param(fasteoi, bool, S_IRUGO); > > -bool __read_mostly enable_apicv = 1; > module_param(enable_apicv, bool, S_IRUGO); > > /* > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 9b6bca616929..23fdbba6b394 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -209,6 +209,9 @@ EXPORT_SYMBOL_GPL(host_efer); > bool __read_mostly allow_smaller_maxphyaddr = 0; > EXPORT_SYMBOL_GPL(allow_smaller_maxphyaddr); > > +bool __read_mostly enable_apicv = true; Nitpick: I don't like this asymmetry. VMX and the common code uses the enable_apicv module param and variable, while SVM uses avic, which sets the enable_apicv variable. I'll prefer both VMX and SVM have their own private variable for their avic/apicv module param, which should set a common variable later. Best regards, Maxim Levitsky > +EXPORT_SYMBOL_GPL(enable_apicv); > + > u64 __read_mostly host_xss; > EXPORT_SYMBOL_GPL(host_xss); > u64 __read_mostly supported_xss;
On Wed, May 26, 2021, Maxim Levitsky wrote: > On Tue, 2021-05-18 at 16:43 +0200, Vitaly Kuznetsov wrote: > > Unify VMX and SVM code by moving APICv/AVIC enablement tracking to common > > 'enable_apicv' variable. Note: unlike APICv, AVIC is disabled by default. > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 9b6bca616929..23fdbba6b394 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -209,6 +209,9 @@ EXPORT_SYMBOL_GPL(host_efer); > > bool __read_mostly allow_smaller_maxphyaddr = 0; > > EXPORT_SYMBOL_GPL(allow_smaller_maxphyaddr); > > > > +bool __read_mostly enable_apicv = true; > > Nitpick: I don't like this asymmetry. > > VMX and the common code uses the enable_apicv module param and variable, > while SVM uses avic, which sets the enable_apicv variable. > > I'll prefer both VMX and SVM have their own private variable for their > avic/apicv module param, which should set a common variable later. I don't love the intermediate "avic" either, but there isn't a good alternative. Forcing VMX to also use an intermediate doesn't make much sense, we'd be penalizing ourselves in the form of unnecessary complexity just because AVIC needs to be disabled by default for reasons KVM can't fix. As for the asymmetry, I actually like it because it makes "avic" stand out and highlights that there is weirdness with enabling AVIC.
On Wed, 2021-05-26 at 15:07 +0000, Sean Christopherson wrote: > On Wed, May 26, 2021, Maxim Levitsky wrote: > > On Tue, 2021-05-18 at 16:43 +0200, Vitaly Kuznetsov wrote: > > > Unify VMX and SVM code by moving APICv/AVIC enablement tracking to common > > > 'enable_apicv' variable. Note: unlike APICv, AVIC is disabled by default. > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > > index 9b6bca616929..23fdbba6b394 100644 > > > --- a/arch/x86/kvm/x86.c > > > +++ b/arch/x86/kvm/x86.c > > > @@ -209,6 +209,9 @@ EXPORT_SYMBOL_GPL(host_efer); > > > bool __read_mostly allow_smaller_maxphyaddr = 0; > > > EXPORT_SYMBOL_GPL(allow_smaller_maxphyaddr); > > > > > > +bool __read_mostly enable_apicv = true; > > > > Nitpick: I don't like this asymmetry. > > > > VMX and the common code uses the enable_apicv module param and variable, > > while SVM uses avic, which sets the enable_apicv variable. > > > > I'll prefer both VMX and SVM have their own private variable for their > > avic/apicv module param, which should set a common variable later. > > I don't love the intermediate "avic" either, but there isn't a good alternative. > Forcing VMX to also use an intermediate doesn't make much sense, we'd be penalizing > ourselves in the form of unnecessary complexity just because AVIC needs to be > disabled by default for reasons KVM can't fix. This is also something we should eventually reconsider. These days, the AVIC works quite well and disables itself when needed. When do you think it will be the time to enable it by default? > > As for the asymmetry, I actually like it because it makes "avic" stand out and > highlights that there is weirdness with enabling AVIC. You mean that it is disabled by default? Anyway I don't have that strong opinion on this, so let it be like this. Best regards, Maxim Levitsky >
On 26/05/21 17:52, Maxim Levitsky wrote: >> I don't love the intermediate "avic" either, but there isn't a good alternative. >> Forcing VMX to also use an intermediate doesn't make much sense, we'd be penalizing >> ourselves in the form of unnecessary complexity just because AVIC needs to be >> disabled by default for reasons KVM can't fix. > This is also something we should eventually reconsider. > These days, the AVIC works quite well and disables itself when needed. > When do you think it will be the time to enable it by default? We can probably enable it, but most guests won't use it because of x2apic. Paolo
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 55efbacfc244..a2197fcf0e7c 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1422,6 +1422,7 @@ struct kvm_arch_async_pf { extern u32 __read_mostly kvm_nr_uret_msrs; extern u64 __read_mostly host_efer; extern bool __read_mostly allow_smaller_maxphyaddr; +extern bool __read_mostly enable_apicv; extern struct kvm_x86_ops kvm_x86_ops; #define KVM_X86_OP(func) \ diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index 1c1bf911e02b..05cd0b128b02 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -27,10 +27,6 @@ #include "irq.h" #include "svm.h" -/* enable / disable AVIC */ -int avic; -module_param(avic, int, S_IRUGO); - #define SVM_AVIC_DOORBELL 0xc001011b #define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF) @@ -124,7 +120,7 @@ void avic_vm_destroy(struct kvm *kvm) unsigned long flags; struct kvm_svm *kvm_svm = to_kvm_svm(kvm); - if (!avic) + if (!enable_apicv) return; if (kvm_svm->avic_logical_id_table_page) @@ -147,7 +143,7 @@ int avic_vm_init(struct kvm *kvm) struct page *l_page; u32 vm_id; - if (!avic) + if (!enable_apicv) return 0; /* Allocating physical APIC ID table (4KB) */ @@ -569,7 +565,7 @@ int avic_init_vcpu(struct vcpu_svm *svm) int ret; struct kvm_vcpu *vcpu = &svm->vcpu; - if (!avic || !irqchip_in_kernel(vcpu->kvm)) + if (!enable_apicv || !irqchip_in_kernel(vcpu->kvm)) return 0; ret = avic_init_backing_page(vcpu); @@ -593,7 +589,7 @@ void avic_post_state_restore(struct kvm_vcpu *vcpu) void svm_toggle_avic_for_irq_window(struct kvm_vcpu *vcpu, bool activate) { - if (!avic || !lapic_in_kernel(vcpu)) + if (!enable_apicv || !lapic_in_kernel(vcpu)) return; srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); @@ -653,7 +649,7 @@ void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu) struct vmcb *vmcb = svm->vmcb; bool activated = kvm_vcpu_apicv_active(vcpu); - if (!avic) + if (!enable_apicv) return; if (activated) { diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 8c3918a11826..0d6ec34d1e4b 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -185,6 +185,10 @@ module_param(vls, int, 0444); static int vgif = true; module_param(vgif, int, 0444); +/* enable / disable AVIC */ +static int avic; +module_param(avic, int, 0444); + bool __read_mostly dump_invalid_vmcb; module_param(dump_invalid_vmcb, bool, 0644); @@ -1009,14 +1013,15 @@ static __init int svm_hardware_setup(void) nrips = false; } - if (avic) { - if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) { - avic = false; - } else { - pr_info("AVIC enabled\n"); + if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) + avic = false; - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); - } + /* 'enable_apicv' is common between VMX/SVM but the defaults differ */ + enable_apicv = avic; + if (enable_apicv) { + pr_info("AVIC enabled\n"); + + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); } if (vls) { @@ -4427,13 +4432,13 @@ static int svm_vm_init(struct kvm *kvm) if (!pause_filter_count || !pause_filter_thresh) kvm->arch.pause_in_guest = true; - if (avic) { + if (enable_apicv) { int ret = avic_vm_init(kvm); if (ret) return ret; } - kvm_apicv_init(kvm, avic); + kvm_apicv_init(kvm, enable_apicv); return 0; } diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index e44567ceb865..a514b490db4a 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -479,8 +479,6 @@ extern struct kvm_x86_nested_ops svm_nested_ops; #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL -extern int avic; - static inline void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data) { svm->vmcb->control.avic_vapic_bar = data & VMCB_AVIC_APIC_BAR_MASK; diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h index aa0e7872fcc9..4705ad55abb5 100644 --- a/arch/x86/kvm/vmx/capabilities.h +++ b/arch/x86/kvm/vmx/capabilities.h @@ -12,7 +12,6 @@ extern bool __read_mostly enable_ept; extern bool __read_mostly enable_unrestricted_guest; extern bool __read_mostly enable_ept_ad_bits; extern bool __read_mostly enable_pml; -extern bool __read_mostly enable_apicv; extern int __read_mostly pt_mode; #define PT_MODE_SYSTEM 0 diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 4bceb5ca3a89..5e9ba10e9c2d 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -101,7 +101,6 @@ module_param(emulate_invalid_guest_state, bool, S_IRUGO); static bool __read_mostly fasteoi = 1; module_param(fasteoi, bool, S_IRUGO); -bool __read_mostly enable_apicv = 1; module_param(enable_apicv, bool, S_IRUGO); /* diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 9b6bca616929..23fdbba6b394 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -209,6 +209,9 @@ EXPORT_SYMBOL_GPL(host_efer); bool __read_mostly allow_smaller_maxphyaddr = 0; EXPORT_SYMBOL_GPL(allow_smaller_maxphyaddr); +bool __read_mostly enable_apicv = true; +EXPORT_SYMBOL_GPL(enable_apicv); + u64 __read_mostly host_xss; EXPORT_SYMBOL_GPL(host_xss); u64 __read_mostly supported_xss;
Unify VMX and SVM code by moving APICv/AVIC enablement tracking to common 'enable_apicv' variable. Note: unlike APICv, AVIC is disabled by default. No functional change intended. Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/svm/avic.c | 14 +++++--------- arch/x86/kvm/svm/svm.c | 23 ++++++++++++++--------- arch/x86/kvm/svm/svm.h | 2 -- arch/x86/kvm/vmx/capabilities.h | 1 - arch/x86/kvm/vmx/vmx.c | 1 - arch/x86/kvm/x86.c | 3 +++ 7 files changed, 23 insertions(+), 22 deletions(-)