[Xen-devel] [PATCH v2] vlapic: fix two flaws in emulating MSR_IA32_APICBASE

[Chao Gao <chao.gao@xxxxxxxxx>]

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@xxxxxxxxx>
---
v2:
- Modify macro vlapic_xapic_mode()

---
 xen/arch/x86/hvm/vlapic.c        | 6 ++----
 xen/include/asm-x86/hvm/vlapic.h | 4 ++++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index cf8ee50..4320c6e 100644
--- 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 4656293..a63fcd5 100644
--- 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.
-- 
1.8.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel