KVM: x86: Treat x2APIC's ICR as a 64-bit register, not two 32-bit regs

Emulate the x2APIC ICR as a single 64-bit register, as opposed to forking
it across ICR and ICR2 as two 32-bit registers.  This mirrors hardware
behavior for Intel's upcoming IPI virtualization support, which does not
split the access.

Previous versions of Intel's SDM and AMD's APM don't explicitly state
exactly how ICR is reflected in the vAPIC page for x2APIC, KVM just
happened to speculate incorrectly.

Handling the upcoming behavior is necessary in order to maintain
backwards compatibility with KVM_{G,S}ET_LAPIC, e.g. failure to shuffle
the 64-bit ICR to ICR+ICR2 and vice versa would break live migration if
IPI virtualization support isn't symmetrical across the source and dest.

Cc: Zeng Guang <guang.zeng@intel.com>
Cc: Chao Gao <chao.gao@intel.com>
Cc: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-Id: <20220204214205.3306634-10-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 05a6f69fe589e6542256de2cfd64e855b71e2866..3e6a53210cc42f53b4221a4d55c0c0b8b9f87b30 100644 (file)
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -68,6 +68,29 @@ static bool lapic_timer_advance_dynamic __read_mostly;
 /* step-by-step approximation to mitigate fluctuation */
 #define LAPIC_TIMER_ADVANCE_ADJUST_STEP 8
 
+static __always_inline u64 __kvm_lapic_get_reg64(char *regs, int reg)
+{
+       BUILD_BUG_ON(reg != APIC_ICR);
+       return *((u64 *) (regs + reg));
+}
+
+static __always_inline u64 kvm_lapic_get_reg64(struct kvm_lapic *apic, int reg)
+{
+       return __kvm_lapic_get_reg64(apic->regs, reg);
+}
+
+static __always_inline void __kvm_lapic_set_reg64(char *regs, int reg, u64 val)
+{
+       BUILD_BUG_ON(reg != APIC_ICR);
+       *((u64 *) (regs + reg)) = val;
+}
+
+static __always_inline void kvm_lapic_set_reg64(struct kvm_lapic *apic,
+                                               int reg, u64 val)
+{
+       __kvm_lapic_set_reg64(apic->regs, reg, val);
+}
+
 static inline int apic_test_vector(int vec, void *bitmap)
 {
        return test_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
@@ -1397,7 +1420,6 @@ static int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
                APIC_REGS_MASK(APIC_IRR, APIC_ISR_NR) |
                APIC_REG_MASK(APIC_ESR) |
                APIC_REG_MASK(APIC_ICR) |
-               APIC_REG_MASK(APIC_ICR2) |
                APIC_REG_MASK(APIC_LVTT) |
                APIC_REG_MASK(APIC_LVTTHMR) |
                APIC_REG_MASK(APIC_LVTPC) |
@@ -1408,9 +1430,16 @@ static int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
                APIC_REG_MASK(APIC_TMCCT) |
                APIC_REG_MASK(APIC_TDCR);
 
-       /* ARBPRI is not valid on x2APIC */
+       /*
+        * ARBPRI and ICR2 are not valid in x2APIC mode.  WARN if KVM reads ICR
+        * in x2APIC mode as it's an 8-byte register in x2APIC and needs to be
+        * manually handled by the caller.
+        */
        if (!apic_x2apic_mode(apic))
-               valid_reg_mask |= APIC_REG_MASK(APIC_ARBPRI);
+               valid_reg_mask |= APIC_REG_MASK(APIC_ARBPRI) |
+                                 APIC_REG_MASK(APIC_ICR2);
+       else
+               WARN_ON_ONCE(offset == APIC_ICR);
 
        if (alignment + len > 4)
                return 1;
@@ -2054,16 +2083,18 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
                break;
        }
        case APIC_ICR:
+               WARN_ON_ONCE(apic_x2apic_mode(apic));
+
                /* No delay here, so we always clear the pending bit */
                val &= ~APIC_ICR_BUSY;
                kvm_apic_send_ipi(apic, val, kvm_lapic_get_reg(apic, APIC_ICR2));
                kvm_lapic_set_reg(apic, APIC_ICR, val);
                break;
-
        case APIC_ICR2:
-               if (!apic_x2apic_mode(apic))
-                       val &= 0xff000000;
-               kvm_lapic_set_reg(apic, APIC_ICR2, val);
+               if (apic_x2apic_mode(apic))
+                       ret = 1;
+               else
+                       kvm_lapic_set_reg(apic, APIC_ICR2, val & 0xff000000);
                break;
 
        case APIC_LVT0:
@@ -2353,8 +2384,12 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
        if (!apic_x2apic_mode(apic))
                kvm_apic_set_ldr(apic, 0);
        kvm_lapic_set_reg(apic, APIC_ESR, 0);
-       kvm_lapic_set_reg(apic, APIC_ICR, 0);
-       kvm_lapic_set_reg(apic, APIC_ICR2, 0);
+       if (!apic_x2apic_mode(apic)) {
+               kvm_lapic_set_reg(apic, APIC_ICR, 0);
+               kvm_lapic_set_reg(apic, APIC_ICR2, 0);
+       } else {
+               kvm_lapic_set_reg64(apic, APIC_ICR, 0);
+       }
        kvm_lapic_set_reg(apic, APIC_TDCR, 0);
        kvm_lapic_set_reg(apic, APIC_TMICT, 0);
        for (i = 0; i < 8; i++) {
@@ -2571,6 +2606,7 @@ static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu,
        if (apic_x2apic_mode(vcpu->arch.apic)) {
                u32 *id = (u32 *)(s->regs + APIC_ID);
                u32 *ldr = (u32 *)(s->regs + APIC_LDR);
+               u64 icr;
 
                if (vcpu->kvm->arch.x2apic_format) {
                        if (*id != vcpu->vcpu_id)
@@ -2582,9 +2618,21 @@ static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu,
                                *id <<= 24;
                }
 
-               /* In x2APIC mode, the LDR is fixed and based on the id */
-               if (set)
+               /*
+                * In x2APIC mode, the LDR is fixed and based on the id.  And
+                * ICR is internally a single 64-bit register, but needs to be
+                * split to ICR+ICR2 in userspace for backwards compatibility.
+                */
+               if (set) {
                        *ldr = kvm_apic_calc_x2apic_ldr(*id);
+
+                       icr = __kvm_lapic_get_reg(s->regs, APIC_ICR) |
+                             (u64)__kvm_lapic_get_reg(s->regs, APIC_ICR2) << 32;
+                       __kvm_lapic_set_reg64(s->regs, APIC_ICR, icr);
+               } else {
+                       icr = __kvm_lapic_get_reg64(s->regs, APIC_ICR);
+                       __kvm_lapic_set_reg(s->regs, APIC_ICR2, icr >> 32);
+               }
        }
 
        return 0;
@@ -2774,27 +2822,43 @@ int kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr)
        return 0;
 }
 
+int kvm_x2apic_icr_write(struct kvm_lapic *apic, u64 data)
+{
+       data &= ~APIC_ICR_BUSY;
+
+       kvm_apic_send_ipi(apic, (u32)data, (u32)(data >> 32));
+       kvm_lapic_set_reg64(apic, APIC_ICR, data);
+       trace_kvm_apic_write(APIC_ICR, data);
+       return 0;
+}
+
 static int kvm_lapic_msr_read(struct kvm_lapic *apic, u32 reg, u64 *data)
 {
-       u32 low, high = 0;
+       u32 low;
 
-       if (kvm_lapic_reg_read(apic, reg, 4, &low))
-               return 1;
+       if (reg == APIC_ICR) {
+               *data = kvm_lapic_get_reg64(apic, APIC_ICR);
+               return 0;
+       }
 
-       if (reg == APIC_ICR &&
-           WARN_ON_ONCE(kvm_lapic_reg_read(apic, APIC_ICR2, 4, &high)))
+       if (kvm_lapic_reg_read(apic, reg, 4, &low))
                return 1;
 
-       *data = (((u64)high) << 32) | low;
+       *data = low;
 
        return 0;
 }
 
 static int kvm_lapic_msr_write(struct kvm_lapic *apic, u32 reg, u64 data)
 {
-       /* For 64-bit ICR writes, set ICR2 (dest) before ICR (command). */
+       /*
+        * ICR is a 64-bit register in x2APIC mode (and Hyper'v PV vAPIC) and
+        * can be written as such, all other registers remain accessible only
+        * through 32-bit reads/writes.
+        */
        if (reg == APIC_ICR)
-               kvm_lapic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
+               return kvm_x2apic_icr_write(apic, data);
+
        return kvm_lapic_reg_write(apic, reg, (u32)data);
 }
 
@@ -2806,9 +2870,6 @@ int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
        if (!lapic_in_kernel(vcpu) || !apic_x2apic_mode(apic))
                return 1;
 
-       if (reg == APIC_ICR2)
-               return 1;
-
        return kvm_lapic_msr_write(apic, reg, data);
 }
 
@@ -2820,7 +2881,7 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
        if (!lapic_in_kernel(vcpu) || !apic_x2apic_mode(apic))
                return 1;
 
-       if (reg == APIC_DFR || reg == APIC_ICR2)
+       if (reg == APIC_DFR)
                return 1;
 
        return kvm_lapic_msr_read(apic, reg, data);