svm: Fix AVIC DFR and LDR handling
authorSuthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>
Tue, 29 Jan 2019 08:08:42 +0000 (08:08 +0000)
committerPaolo Bonzini <pbonzini@redhat.com>
Wed, 20 Feb 2019 21:48:20 +0000 (22:48 +0100)
Current SVM AVIC driver makes two incorrect assumptions:
  1. APIC LDR register cannot be zero
  2. APIC DFR for all vCPUs must be the same

LDR=0 means the local APIC does not support logical destination mode.
Therefore, the driver should mark any previously assigned logical APIC ID
table entry as invalid, and return success.  Also, DFR is specific to
a particular local APIC, and can be different among all vCPUs
(as observed on Windows 10).

These incorrect assumptions cause Windows 10 and FreeBSD VMs to fail
to boot with AVIC enabled. So, instead of flush the whole logical APIC ID
table, handle DFR and LDR for each vCPU independently.

Fixes: 18f40c53e10f ('svm: Add VMEXIT handlers for AVIC')
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Reported-by: Julian Stecklina <jsteckli@amazon.de>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
arch/x86/kvm/svm.c

index f13a3a24d3609e03b2bd849c0058ff41a670f2ca..01b66bbcdd7afebb218878e65ac70a62bfa3aa19 100644 (file)
@@ -145,7 +145,6 @@ struct kvm_svm {
 
        /* Struct members for AVIC */
        u32 avic_vm_id;
-       u32 ldr_mode;
        struct page *avic_logical_id_table_page;
        struct page *avic_physical_id_table_page;
        struct hlist_node hnode;
@@ -236,6 +235,7 @@ struct vcpu_svm {
        bool nrips_enabled      : 1;
 
        u32 ldr_reg;
+       u32 dfr_reg;
        struct page *avic_backing_page;
        u64 *avic_physical_id_cache;
        bool avic_is_running;
@@ -2106,6 +2106,7 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
 
        INIT_LIST_HEAD(&svm->ir_list);
        spin_lock_init(&svm->ir_list_lock);
+       svm->dfr_reg = APIC_DFR_FLAT;
 
        return ret;
 }
@@ -4565,8 +4566,7 @@ static u32 *avic_get_logical_id_entry(struct kvm_vcpu *vcpu, u32 ldr, bool flat)
        return &logical_apic_id_table[index];
 }
 
-static int avic_ldr_write(struct kvm_vcpu *vcpu, u8 g_physical_id, u32 ldr,
-                         bool valid)
+static int avic_ldr_write(struct kvm_vcpu *vcpu, u8 g_physical_id, u32 ldr)
 {
        bool flat;
        u32 *entry, new_entry;
@@ -4579,31 +4579,39 @@ static int avic_ldr_write(struct kvm_vcpu *vcpu, u8 g_physical_id, u32 ldr,
        new_entry = READ_ONCE(*entry);
        new_entry &= ~AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK;
        new_entry |= (g_physical_id & AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK);
-       if (valid)
-               new_entry |= AVIC_LOGICAL_ID_ENTRY_VALID_MASK;
-       else
-               new_entry &= ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK;
+       new_entry |= AVIC_LOGICAL_ID_ENTRY_VALID_MASK;
        WRITE_ONCE(*entry, new_entry);
 
        return 0;
 }
 
+static void avic_invalidate_logical_id_entry(struct kvm_vcpu *vcpu)
+{
+       struct vcpu_svm *svm = to_svm(vcpu);
+       bool flat = svm->dfr_reg == APIC_DFR_FLAT;
+       u32 *entry = avic_get_logical_id_entry(vcpu, svm->ldr_reg, flat);
+
+       if (entry)
+               WRITE_ONCE(*entry, (u32) ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK);
+}
+
 static int avic_handle_ldr_update(struct kvm_vcpu *vcpu)
 {
-       int ret;
+       int ret = 0;
        struct vcpu_svm *svm = to_svm(vcpu);
        u32 ldr = kvm_lapic_get_reg(vcpu->arch.apic, APIC_LDR);
 
-       if (!ldr)
-               return 1;
+       if (ldr == svm->ldr_reg)
+               return 0;
 
-       ret = avic_ldr_write(vcpu, vcpu->vcpu_id, ldr, true);
-       if (ret && svm->ldr_reg) {
-               avic_ldr_write(vcpu, 0, svm->ldr_reg, false);
-               svm->ldr_reg = 0;
-       } else {
+       avic_invalidate_logical_id_entry(vcpu);
+
+       if (ldr)
+               ret = avic_ldr_write(vcpu, vcpu->vcpu_id, ldr);
+
+       if (!ret)
                svm->ldr_reg = ldr;
-       }
+
        return ret;
 }
 
@@ -4637,27 +4645,16 @@ static int avic_handle_apic_id_update(struct kvm_vcpu *vcpu)
        return 0;
 }
 
-static int avic_handle_dfr_update(struct kvm_vcpu *vcpu)
+static void avic_handle_dfr_update(struct kvm_vcpu *vcpu)
 {
        struct vcpu_svm *svm = to_svm(vcpu);
-       struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm);
        u32 dfr = kvm_lapic_get_reg(vcpu->arch.apic, APIC_DFR);
-       u32 mod = (dfr >> 28) & 0xf;
-
-       /*
-        * We assume that all local APICs are using the same type.
-        * If this changes, we need to flush the AVIC logical
-        * APID id table.
-        */
-       if (kvm_svm->ldr_mode == mod)
-               return 0;
 
-       clear_page(page_address(kvm_svm->avic_logical_id_table_page));
-       kvm_svm->ldr_mode = mod;
+       if (svm->dfr_reg == dfr)
+               return;
 
-       if (svm->ldr_reg)
-               avic_handle_ldr_update(vcpu);
-       return 0;
+       avic_invalidate_logical_id_entry(vcpu);
+       svm->dfr_reg = dfr;
 }
 
 static int avic_unaccel_trap_write(struct vcpu_svm *svm)
@@ -6163,8 +6160,7 @@ static inline void avic_post_state_restore(struct kvm_vcpu *vcpu)
 {
        if (avic_handle_apic_id_update(vcpu) != 0)
                return;
-       if (avic_handle_dfr_update(vcpu) != 0)
-               return;
+       avic_handle_dfr_update(vcpu);
        avic_handle_ldr_update(vcpu);
 }