| Message ID | 20220221021922.733373-13-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introducing AMD x2APIC Virtualization (x2AVIC) support. | expand |
On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote: > Currently, AVIC is inactive when booting a VM w/ x2APIC support. > With x2AVIC support, the APICV_INHIBIT_REASON_X2APIC can be removed. The commit title is a bit misleading - the inhibit reason is not removed, but rather AVIC is not inhibited when x2avic is present. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > arch/x86/kvm/svm/avic.c | 21 +++++++++++++++++++++ > arch/x86/kvm/svm/svm.c | 18 ++---------------- > arch/x86/kvm/svm/svm.h | 1 + > 3 files changed, 24 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 3306b74f1d8b..874c89f8fd47 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -21,6 +21,7 @@ > > #include <asm/irq_remapping.h> > > +#include "cpuid.h" > #include "trace.h" > #include "lapic.h" > #include "x86.h" > @@ -176,6 +177,26 @@ void avic_vm_destroy(struct kvm *kvm) > spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags); > } > > +void avic_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu, int nested) > +{ > + /* > + * If the X2APIC feature is exposed to the guest, > + * disable AVIC unless X2AVIC mode is enabled. > + */ > + if (avic_mode == AVIC_MODE_X1 && > + 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. > + * So, we disable AVIC when cpuid for SVM is set in the L1 guest. > + */ > + if (nested && guest_cpuid_has(vcpu, X86_FEATURE_SVM)) > + kvm_request_apicv_update(vcpu->kvm, false, > + APICV_INHIBIT_REASON_NESTED); BTW, now that I am thinking about it, it would be nice to be able to force the AVIC_MODE_X1 even if x2avic is present, for debug purposes from a module param. Just a suggestion. > +} > + > int avic_vm_init(struct kvm *kvm) > { > unsigned long flags; > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index afca26aa1f40..b7bc6cd74aba 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -3992,23 +3992,9 @@ static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > vcpu->arch.reserved_gpa_bits &= ~(1UL << (best->ebx & 0x3f)); > } > > - if (kvm_vcpu_apicv_active(vcpu)) { > - /* > - * 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); > + if (kvm_vcpu_apicv_active(vcpu)) > + avic_vcpu_after_set_cpuid(vcpu, nested); > > - /* > - * Currently, AVIC does not work with nested virtualization. > - * So, we disable AVIC when cpuid for SVM is set in the L1 guest. > - */ > - if (nested && guest_cpuid_has(vcpu, X86_FEATURE_SVM)) > - kvm_request_apicv_update(vcpu->kvm, false, > - APICV_INHIBIT_REASON_NESTED); > - } > init_vmcb_after_set_cpuid(vcpu); > } > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index 41514df5107e..aea80abe9186 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -578,6 +578,7 @@ void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data); > void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu); > void avic_vcpu_put(struct kvm_vcpu *vcpu); > void avic_post_state_restore(struct kvm_vcpu *vcpu); > +void avic_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu, int nested); > void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu); > void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu); > bool svm_check_apicv_inhibit_reasons(ulong bit); Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
Hi Maxim, On 2/25/22 3:06 AM, Maxim Levitsky wrote: > On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote: >> Currently, AVIC is inactive when booting a VM w/ x2APIC support. >> With x2AVIC support, the APICV_INHIBIT_REASON_X2APIC can be removed. > The commit title is a bit misleading - the inhibit reason is not removed, > but rather AVIC is not inhibited when x2avic is present. > I'll fix the commit message. >> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com> >> --- >> arch/x86/kvm/svm/avic.c | 21 +++++++++++++++++++++ >> arch/x86/kvm/svm/svm.c | 18 ++---------------- >> arch/x86/kvm/svm/svm.h | 1 + >> 3 files changed, 24 insertions(+), 16 deletions(-) >> >> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c >> index 3306b74f1d8b..874c89f8fd47 100644 >> --- a/arch/x86/kvm/svm/avic.c >> +++ b/arch/x86/kvm/svm/avic.c >> @@ -21,6 +21,7 @@ >> >> #include <asm/irq_remapping.h> >> >> +#include "cpuid.h" >> #include "trace.h" >> #include "lapic.h" >> #include "x86.h" >> @@ -176,6 +177,26 @@ void avic_vm_destroy(struct kvm *kvm) >> spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags); >> } >> >> +void avic_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu, int nested) >> +{ >> + /* >> + * If the X2APIC feature is exposed to the guest, >> + * disable AVIC unless X2AVIC mode is enabled. >> + */ >> + if (avic_mode == AVIC_MODE_X1 && >> + 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. >> + * So, we disable AVIC when cpuid for SVM is set in the L1 guest. >> + */ >> + if (nested && guest_cpuid_has(vcpu, X86_FEATURE_SVM)) >> + kvm_request_apicv_update(vcpu->kvm, false, >> + APICV_INHIBIT_REASON_NESTED); > BTW, now that I am thinking about it, it would be nice to be able to force > the AVIC_MODE_X1 even if x2avic is present, for debug purposes from a module > param. Just a suggestion. Actually, we can force AVIC_MODE_X1 on a VM by disabling Guest x2APIC mode via QEMU option. That should work for debugging purpose, right? Regards, Suravee
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index 3306b74f1d8b..874c89f8fd47 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -21,6 +21,7 @@ #include <asm/irq_remapping.h> +#include "cpuid.h" #include "trace.h" #include "lapic.h" #include "x86.h" @@ -176,6 +177,26 @@ void avic_vm_destroy(struct kvm *kvm) spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags); } +void avic_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu, int nested) +{ + /* + * If the X2APIC feature is exposed to the guest, + * disable AVIC unless X2AVIC mode is enabled. + */ + if (avic_mode == AVIC_MODE_X1 && + 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. + * So, we disable AVIC when cpuid for SVM is set in the L1 guest. + */ + if (nested && guest_cpuid_has(vcpu, X86_FEATURE_SVM)) + kvm_request_apicv_update(vcpu->kvm, false, + APICV_INHIBIT_REASON_NESTED); +} + int avic_vm_init(struct kvm *kvm) { unsigned long flags; diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index afca26aa1f40..b7bc6cd74aba 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -3992,23 +3992,9 @@ static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) vcpu->arch.reserved_gpa_bits &= ~(1UL << (best->ebx & 0x3f)); } - if (kvm_vcpu_apicv_active(vcpu)) { - /* - * 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); + if (kvm_vcpu_apicv_active(vcpu)) + avic_vcpu_after_set_cpuid(vcpu, nested); - /* - * Currently, AVIC does not work with nested virtualization. - * So, we disable AVIC when cpuid for SVM is set in the L1 guest. - */ - if (nested && guest_cpuid_has(vcpu, X86_FEATURE_SVM)) - kvm_request_apicv_update(vcpu->kvm, false, - APICV_INHIBIT_REASON_NESTED); - } init_vmcb_after_set_cpuid(vcpu); } diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index 41514df5107e..aea80abe9186 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -578,6 +578,7 @@ void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data); void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu); void avic_vcpu_put(struct kvm_vcpu *vcpu); void avic_post_state_restore(struct kvm_vcpu *vcpu); +void avic_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu, int nested); void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu); void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu); bool svm_check_apicv_inhibit_reasons(ulong bit);
Currently, AVIC is inactive when booting a VM w/ x2APIC support. With x2AVIC support, the APICV_INHIBIT_REASON_X2APIC can be removed. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- arch/x86/kvm/svm/avic.c | 21 +++++++++++++++++++++ arch/x86/kvm/svm/svm.c | 18 ++---------------- arch/x86/kvm/svm/svm.h | 1 + 3 files changed, 24 insertions(+), 16 deletions(-)