Hi Radim,
On 9/8/17 08:53, Radim Krčmář wrote:
2017-09-05 22:39-0500, Suravee Suthikulpanit:
SVM AVIC hardware accelerates guest write to APIC_EOI register
(for edge-trigger interrupt), which means it does not trap to KVM.
So, only enable SVM AVIC only in split irqchip mode.
(e.g. launching qemu w/ option '-machine kernel_irqchip=split').
Yeah, hacking TMR to get the VM exit could result in future bugs.
We have to push split irqchip as the deafult in userspaces with this
change.
Actually, I'm not quite sure about the advantages/disadvantages with split
irqchip, and how it would affect other cases, and why it was not used as default
currently.
Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
---
arch/x86/kvm/svm.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d1b3eb4..7ce191b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1292,7 +1291,7 @@ static void init_vmcb(struct vcpu_svm *svm)
set_intercept(svm, INTERCEPT_PAUSE);
}
- if (avic)
+ if (kvm_vcpu_apicv_active(&svm->vcpu))
avic_init_vmcb(svm);
/*
@@ -1594,6 +1593,12 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
if (!avic)
return 0;
+ if (!irqchip_split(svm->vcpu.kvm)) {
The other place used kvm_vcpu_apicv_active() instead of checking
irqchip_split() directly, so I think it would be better to be consistent
and do it here too.
I'd also like if this workaround used !irqchip_split() exactly once.
Ok, I'll clean up in V2.
+ pr_debug("%s: Disable AVIC due to non-split irqchip.\n",
+ __func__);
There is going to be too much of those. pr_debug_once() would be a
better notification. We can also report it in svm_get_enable_apicv().
pr_debug_once does not use dynamic debug APIs. I think I can call pr_debug only
when vcpu_id == 0.
+ return 0;
+ }
+
ret = avic_init_backing_page(&svm->vcpu);
if (ret)
return ret;
@@ -4388,7 +4393,7 @@ static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
static bool svm_get_enable_apicv(struct kvm_vcpu *vcpu)
{
- return avic;
+ return avic && irqchip_split(vcpu->kvm);
}
static void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
@@ -4405,7 +4410,7 @@ static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
struct vcpu_svm *svm = to_svm(vcpu);
struct vmcb *vmcb = svm->vmcb;
- if (!avic)
+ if (!avic || !irqchip_split(svm->vcpu.kvm))
Seems like we want !kvm_vcpu_apicv_active() here too.
Right.
return;
vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
(separate bug: refresh should be able to enable avic as well.)
thanks.
Thanks,
Suravee
