x86/vlapic: fix two flaws in emulating MSR_IA32_APICBASE

According to SDM Chapter ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER (APIC)
-> Extended XAPIC (x2APIC) -> x2APIC State Transitions, The existing code to
handle guest's writing MSR_IA32_APICBASE has two flaws:
1. Transition from x2APIC Mode to Disabled Mode is allowed but wrongly
disabled currently. Fix it by removing the related check.
2. Transition from x2APIC Mode to xAPIC Mode is illegal but wrongly allowed
currently. Considering changing ENABLE bit of the MSR has been handled,
it can be fixed by only allowing transition from xAPIC Mode to x2APIC Mode
(the other two transitions: from x2APIC mode to xAPIC Mode, from disabled mode
to invalid state (EN=0, EXTD=1) are disabled).

Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index cf8ee50a97d4b594203ffc048988506eb8a14efb..4320c6e30a4f0f97b0e29d2f4e8138eeb5785ac9 100644 (file)
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1003,14 +1003,12 @@ bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
         }
         else
         {
-            if ( unlikely(vlapic_x2apic_mode(vlapic)) )
-                return 0;
             vlapic->hw.disabled |= VLAPIC_HW_DISABLED;
             pt_may_unmask_irq(vlapic_domain(vlapic), NULL);
         }
     }
-    else if ( !(value & MSR_IA32_APICBASE_ENABLE) &&
-              unlikely(value & MSR_IA32_APICBASE_EXTD) )
+    else if ( ((vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_EXTD) &&
+              unlikely(!vlapic_xapic_mode(vlapic)) )
         return 0;
 
     vlapic->hw.apic_base_msr = value;

diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h
index 4656293228c1f46ddc60d8fb48b7b93ac44caa81..a63fcd56b863106f590bdbe6331ecba5c5f75c55 100644 (file)
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -51,8 +51,12 @@
 
 #define vlapic_base_address(vlapic)                             \
     ((vlapic)->hw.apic_base_msr & MSR_IA32_APICBASE_BASE)
+/* Only check EXTD bit as EXTD can't be set if it is disabled by hardware */
 #define vlapic_x2apic_mode(vlapic)                              \
     ((vlapic)->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD)
+#define vlapic_xapic_mode(vlapic)                               \
+    (!vlapic_hw_disabled(vlapic) && \
+     !((vlapic)->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD))
 
 /*
  * Generic APIC bitmap vector update & search routines.