| Message ID | 20220308163926.563994-10-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introducing AMD x2APIC Virtualization (x2AVIC) support. | expand |
On Tue, 2022-03-08 at 10:39 -0600, Suravee Suthikulpanit wrote: > When APIC mode is updated (e.g. from xAPIC to x2APIC), > KVM needs to update AVIC settings accordingly, whic is > handled by svm_refresh_apicv_exec_ctrl(). > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > arch/x86/kvm/svm/avic.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 7e5a39a8e698..53559b8dfa52 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -625,7 +625,24 @@ void avic_post_state_restore(struct kvm_vcpu *vcpu) > > void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu) > { > - return; > + struct vcpu_svm *svm = to_svm(vcpu); > + > + if (!lapic_in_kernel(vcpu) || (avic_mode == AVIC_MODE_NONE)) > + return; > + > + if (kvm_get_apic_mode(vcpu) == LAPIC_MODE_INVALID) > + WARN_ONCE(true, "Invalid local APIC state"); > + > + svm->vmcb->control.avic_vapic_bar = svm->vcpu.arch.apic_base & > + VMCB_AVIC_APIC_BAR_MASK; No need for that - APIC base relocation doesn't work when AVIC is enabled, since the page which contains it has to be marked R/W in NPT, which we only do for the default APIC base. I recently removed the code from AVIC which still tried to set the 'avic_vapic_bar' like this. > + kvm_vcpu_update_apicv(&svm->vcpu); > + > + /* > + * The VM could be running w/ AVIC activated switching from APIC > + * to x2APIC mode. We need to all refresh to make sure that all > + * x2AVIC configuration are being done. Why? When AVIC is un-inhibited later then the svm_refresh_apicv_exec_ctrl will be called again and switch to x2avic mode I think. When AVIC is inhibited, then regardless of x2apic mode, VMCB must not have any avic bits set, and all x2apic msrs should be read/write intercepted., thus I don't think that svm_refresh_apicv_exec_ctrl should be force called. > + */ > + svm_refresh_apicv_exec_ctrl(&svm->vcpu); > } > > void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr) Best regards, Maxim Levitsky
Hi Maxim, On 3/24/22 10:35 PM, Maxim Levitsky wrote: > On Tue, 2022-03-08 at 10:39 -0600, Suravee Suthikulpanit wrote: >> When APIC mode is updated (e.g. from xAPIC to x2APIC), >> KVM needs to update AVIC settings accordingly, whic is >> handled by svm_refresh_apicv_exec_ctrl(). >> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> --- >> arch/x86/kvm/svm/avic.c | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c >> index 7e5a39a8e698..53559b8dfa52 100644 >> --- a/arch/x86/kvm/svm/avic.c >> +++ b/arch/x86/kvm/svm/avic.c >> @@ -625,7 +625,24 @@ void avic_post_state_restore(struct kvm_vcpu *vcpu) >> >> void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu) >> { >> - return; >> + struct vcpu_svm *svm = to_svm(vcpu); >> + >> + if (!lapic_in_kernel(vcpu) || (avic_mode == AVIC_MODE_NONE)) >> + return; >> + >> + if (kvm_get_apic_mode(vcpu) == LAPIC_MODE_INVALID) >> + WARN_ONCE(true, "Invalid local APIC state"); >> + >> + svm->vmcb->control.avic_vapic_bar = svm->vcpu.arch.apic_base & >> + VMCB_AVIC_APIC_BAR_MASK; > > No need for that - APIC base relocation doesn't work when AVIC is enabled, > since the page which contains it has to be marked R/W in NPT, which we > only do for the default APIC base. > > I recently removed the code from AVIC which still tried to set the > 'avic_vapic_bar' like this. Got it. I'll remove this part. > >> + kvm_vcpu_update_apicv(&svm->vcpu); >> + >> + /* >> + * The VM could be running w/ AVIC activated switching from APIC >> + * to x2APIC mode. We need to all refresh to make sure that all >> + * x2AVIC configuration are being done. > > Why? When AVIC is un-inhibited later then the svm_refresh_apicv_exec_ctrl will be called > again and switch to x2avic mode I think. Current version does not disable AVIC when APIC is disabled, which happens during APIC mode switching (i.e. xAPIC -> disabled -> x2APIC). This needs to be fixed. Then we can remove the force refresh. > When AVIC is inhibited, then regardless of x2apic mode, VMCB must not have > any avic bits set, and all x2apic msrs should be read/write intercepted., > thus I don't think that svm_refresh_apicv_exec_ctrl should be force called. The refresh is normally called only when there is APICV update request (e.g. kvm_request_apicv_update(APICV_INHIBIT_REASON_IRQWIN)), which could happen or not. However, I have reworked this part. The svm_refresh_apicv_exec_ctrl() force is no longer needed. Regards, Suravee
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index 7e5a39a8e698..53559b8dfa52 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -625,7 +625,24 @@ void avic_post_state_restore(struct kvm_vcpu *vcpu) void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu) { - return; + struct vcpu_svm *svm = to_svm(vcpu); + + if (!lapic_in_kernel(vcpu) || (avic_mode == AVIC_MODE_NONE)) + return; + + if (kvm_get_apic_mode(vcpu) == LAPIC_MODE_INVALID) + WARN_ONCE(true, "Invalid local APIC state"); + + svm->vmcb->control.avic_vapic_bar = svm->vcpu.arch.apic_base & + VMCB_AVIC_APIC_BAR_MASK; + kvm_vcpu_update_apicv(&svm->vcpu); + + /* + * The VM could be running w/ AVIC activated switching from APIC + * to x2APIC mode. We need to all refresh to make sure that all + * x2AVIC configuration are being done. + */ + svm_refresh_apicv_exec_ctrl(&svm->vcpu); } void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
When APIC mode is updated (e.g. from xAPIC to x2APIC), KVM needs to update AVIC settings accordingly, whic is handled by svm_refresh_apicv_exec_ctrl(). Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- arch/x86/kvm/svm/avic.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)