[PATCH 2/4] KVM: SVM: Fixes setting V_IRQ while AVIC is still enabled
From: Suravee Suthikulpanit
Date: Wed May 06 2020 - 09:18:29 EST
The commit 64b5bd270426 ("KVM: nSVM: ignore L1 interrupt window
while running L2 with V_INTR_MASKING=1") introduced a WARN_ON,
which checks if AVIC is enabled when trying to set V_IRQ
in the VMCB for enabling irq window.
The following warning is triggered because the requesting vcpu
(to deactivate AVIC) does not get to process APICv update request
for itself until the next #vmexit.
WARNING: CPU: 0 PID: 118232 at arch/x86/kvm/svm/svm.c:1372 enable_irq_window+0x6a/0xa0 [kvm_amd]
RIP: 0010:enable_irq_window+0x6a/0xa0 [kvm_amd]
Call Trace:
kvm_arch_vcpu_ioctl_run+0x6e3/0x1b50 [kvm]
? kvm_vm_ioctl_irq_line+0x27/0x40 [kvm]
? _copy_to_user+0x26/0x30
? kvm_vm_ioctl+0xb3e/0xd90 [kvm]
? set_next_entity+0x78/0xc0
kvm_vcpu_ioctl+0x236/0x610 [kvm]
ksys_ioctl+0x8a/0xc0
__x64_sys_ioctl+0x1a/0x20
do_syscall_64+0x58/0x210
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Fixes by sending APICV update request to all other vcpus, and
immediately update APIC for itself.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
Link: https://lkml.org/lkml/2020/5/2/167
Fixes: 64b5bd270426 ("KVM: nSVM: ignore L1 interrupt window while running L2 with V_INTR_MASKING=1")
---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/hyperv.c | 4 +++-
arch/x86/kvm/i8254.c | 4 ++--
arch/x86/kvm/svm/avic.c | 2 +-
arch/x86/kvm/svm/svm.c | 6 ++++--
arch/x86/kvm/x86.c | 14 ++++++++++++--
6 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index dd84085..e7115dd 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1503,7 +1503,7 @@ gpa_t kvm_mmu_gva_to_gpa_system(struct kvm_vcpu *vcpu, gva_t gva,
void kvm_apicv_init(struct kvm *kvm, bool enable);
void kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu);
void kvm_request_apicv_update(struct kvm *kvm, bool activate,
- unsigned long bit);
+ unsigned long bit, struct kvm_vcpu *except);
int kvm_emulate_hypercall(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 54d4b98..a007ca5 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -779,7 +779,9 @@ int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages)
* not compatible with APICV, so request
* to deactivate APICV permanently.
*/
- kvm_request_apicv_update(vcpu->kvm, false, APICV_INHIBIT_REASON_HYPERV);
+ kvm_request_apicv_update(vcpu->kvm, false,
+ APICV_INHIBIT_REASON_HYPERV,
+ vcpu);
synic->active = true;
synic->dont_zero_synic_pages = dont_zero_synic_pages;
return 0;
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index febca33..c524081 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -305,14 +305,14 @@ void kvm_pit_set_reinject(struct kvm_pit *pit, bool reinject)
*/
if (reinject) {
kvm_request_apicv_update(kvm, false,
- APICV_INHIBIT_REASON_PIT_REINJ);
+ APICV_INHIBIT_REASON_PIT_REINJ, NULL);
/* The initial state is preserved while ps->reinject == 0. */
kvm_pit_reset_reinject(pit);
kvm_register_irq_ack_notifier(kvm, &ps->irq_ack_notifier);
kvm_register_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
} else {
kvm_request_apicv_update(kvm, true,
- APICV_INHIBIT_REASON_PIT_REINJ);
+ APICV_INHIBIT_REASON_PIT_REINJ, NULL);
kvm_unregister_irq_ack_notifier(kvm, &ps->irq_ack_notifier);
kvm_unregister_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
}
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index e80daa9..4b1574e 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -594,7 +594,7 @@ void svm_toggle_avic_for_irq_window(struct kvm_vcpu *vcpu, bool activate)
srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
kvm_request_apicv_update(vcpu->kvm, activate,
- APICV_INHIBIT_REASON_IRQWIN);
+ APICV_INHIBIT_REASON_IRQWIN, vcpu);
vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 2f379ba..c2117cd 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3502,7 +3502,8 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
*/
if (guest_cpuid_has(vcpu, X86_FEATURE_X2APIC))
kvm_request_apicv_update(vcpu->kvm, false,
- APICV_INHIBIT_REASON_X2APIC);
+ APICV_INHIBIT_REASON_X2APIC,
+ vcpu);
/*
* Currently, AVIC does not work with nested virtualization.
@@ -3510,7 +3511,8 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
*/
if (nested && guest_cpuid_has(vcpu, X86_FEATURE_SVM))
kvm_request_apicv_update(vcpu->kvm, false,
- APICV_INHIBIT_REASON_NESTED);
+ APICV_INHIBIT_REASON_NESTED,
+ vcpu);
}
static bool svm_has_wbinvd_exit(void)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index df473f9..8f69ad0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8083,7 +8083,8 @@ void kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu)
* locked, because it calls __x86_set_memory_region() which does
* synchronize_srcu(&kvm->srcu).
*/
-void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
+void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit,
+ struct kvm_vcpu *except)
{
unsigned long old, new, expected;
@@ -8110,7 +8111,16 @@ void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
trace_kvm_apicv_update_request(activate, bit);
if (kvm_x86_ops.pre_update_apicv_exec_ctrl)
kvm_x86_ops.pre_update_apicv_exec_ctrl(kvm, activate);
- kvm_make_all_cpus_request(kvm, KVM_REQ_APICV_UPDATE);
+
+ /*
+ * Sending request to update APICV for all other vcpus,
+ * while update the calling vcpu immediately instead of
+ * waiting for another #VMEXIT to handle the request.
+ */
+ kvm_make_all_cpus_request_except(kvm, KVM_REQ_APICV_UPDATE,
+ except);
+ if (except)
+ kvm_vcpu_update_apicv(except);
}
EXPORT_SYMBOL_GPL(kvm_request_apicv_update);
--
1.8.3.1