| Message ID | 1460017232-17429-11-git-send-email-Suravee.Suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2016-04-07 03:20-0500, Suravee Suthikulpanit: > When enable AVIC: > * Do not intercept CR8 since this should be handled by AVIC HW. > * Also, we don't need to sync cr8/V_TPR and APIC backing page. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > @@ -4069,7 +4070,8 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr) > - if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) > + if ((is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) || > + svm_vcpu_avic_enabled(svm)) > @@ -4255,14 +4257,15 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu) > static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu) > { > struct vcpu_svm *svm = to_svm(vcpu); > - u64 cr8; > + struct kvm_lapic *apic = vcpu->arch.apic; > > - if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) > + if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK) && Should be "||" at the end of line, like above. (Naming this condition would reduce the chance of errors.) > + svm_vcpu_avic_enabled(svm)) > return; > > - cr8 = kvm_get_cr8(vcpu); > svm->vmcb->control.int_ctl &= ~V_TPR_MASK; > - svm->vmcb->control.int_ctl |= cr8 & V_TPR_MASK; > + svm->vmcb->control.int_ctl |= (kvm_apic_get_reg(apic, > + APIC_TASKPRI) >> 4) & V_TPR_MASK; kvm_get_cr8 takes a different path without lapic_in_kernel (when avic cannot be enabled), so the original code was better. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/04/2016 16:18, Radim Kr?má? wrote: >> > @@ -4069,7 +4070,8 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr) >> > - if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) >> > + if ((is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) || >> > + svm_vcpu_avic_enabled(svm)) >> > @@ -4255,14 +4257,15 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu) >> > static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu) >> > { >> > struct vcpu_svm *svm = to_svm(vcpu); >> > - u64 cr8; >> > + struct kvm_lapic *apic = vcpu->arch.apic; >> > >> > - if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) >> > + if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK) && > Should be "||" at the end of line, like above. > > (Naming this condition would reduce the chance of errors.) > I think it's just "is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)" that should become a static inline. It is used also in update_cr8_intercept. Then something like if (svm_in_nested_interrupt_shadow(vcpu) && svm_vcpu_avic_enabled(svm)) return; makes little sense and stands out much better. In fact, because nested SVM and AVIC have nothing to do with each other, it's even better to write it like if (svm_in_nested_interrupt_shadow(vcpu)) return; if (svm_vcpu_avic_enabled(svm)) return; Thanks, Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 74b0751..bccf6cb 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1092,7 +1092,8 @@ static void init_vmcb(struct vcpu_svm *svm) set_cr_intercept(svm, INTERCEPT_CR0_WRITE); set_cr_intercept(svm, INTERCEPT_CR3_WRITE); set_cr_intercept(svm, INTERCEPT_CR4_WRITE); - set_cr_intercept(svm, INTERCEPT_CR8_WRITE); + if (!svm_vcpu_avic_enabled(svm)) + set_cr_intercept(svm, INTERCEPT_CR8_WRITE); set_dr_intercepts(svm); @@ -4069,7 +4070,8 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr) { struct vcpu_svm *svm = to_svm(vcpu); - if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if ((is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) || + svm_vcpu_avic_enabled(svm)) return; clr_cr_intercept(svm, INTERCEPT_CR8_WRITE); @@ -4255,14 +4257,15 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu) static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); - u64 cr8; + struct kvm_lapic *apic = vcpu->arch.apic; - if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK) && + svm_vcpu_avic_enabled(svm)) return; - cr8 = kvm_get_cr8(vcpu); svm->vmcb->control.int_ctl &= ~V_TPR_MASK; - svm->vmcb->control.int_ctl |= cr8 & V_TPR_MASK; + svm->vmcb->control.int_ctl |= (kvm_apic_get_reg(apic, + APIC_TASKPRI) >> 4) & V_TPR_MASK; } static void svm_complete_interrupts(struct vcpu_svm *svm)
When enable AVIC: * Do not intercept CR8 since this should be handled by AVIC HW. * Also, we don't need to sync cr8/V_TPR and APIC backing page. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- arch/x86/kvm/svm.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)