xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: xen-devel <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Keir Fraser <keir@xen.org>
Subject: [PATCH v5 1/4] x86/HVM: fix miscellaneous aspects of x2APIC emulation
Date: Wed, 24 Sep 2014 16:34:39 +0100	[thread overview]
Message-ID: <542300AF02000078000386B2@mail.emea.novell.com> (raw)
In-Reply-To: <5422FF2E0200007800038693@mail.emea.novell.com>

[-- Attachment #1: Type: text/plain, Size: 11078 bytes --]

- generate #GP on invalid APIC base MSR transitions
- fail reads from the EOI and self-IPI registers (which are write-only)
- handle self-IPI writes and the ICR2 half of ICR writes largely in
  hvm_x2apic_msr_write() and (for self-IPI only) vlapic_apicv_write()
- don't permit MMIO-based access in x2APIC mode
- filter writes to read-only registers in hvm_x2apic_msr_write(),
  allowing conditionals to be dropped from vlapic_reg_write()
- don't ignore upper half of MSR-based write to ESR being non-zero
- don't ignore other writes to reserved bits
- VMX's EXIT_REASON_APIC_WRITE must not result in #GP (this exit being
  trap-like, this exception would get raised on the wrong RIP)
- make hvm_x2apic_msr_read() produce X86EMUL_* return codes just like
  hvm_x2apic_msr_write() does (benign to the only caller)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v5: Force #GP to be raised on reserved bit writes. Drop APIC_TMCCT from
    set of writable registers. Drop message previously issued on x2APIC
    ESR writes.
v3: Also handle APIC_EOI in hvm_x2apic_msr_read() as pointed out by
    Andrew. Filter MMIO-based accesses in vlapic_range() when in x2APIC
    mode. Move x2APIC special casing from vlapic_reg_write() to
    hvm_x2apic_msr_write(). Don't open-code vlapic_x2apic_mode().
v2: Split from main patch.

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4499,7 +4499,8 @@ int hvm_msr_write_intercept(unsigned int
         break;
 
     case MSR_IA32_APICBASE:
-        vlapic_msr_set(vcpu_vlapic(v), msr_content);
+        if ( !vlapic_msr_set(vcpu_vlapic(v), msr_content) )
+            goto gp_fault;
         break;
 
     case MSR_IA32_TSC_DEADLINE:
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -45,11 +45,11 @@
 #define VLAPIC_LVT_NUM                  6
 
 #define LVT_MASK \
-    APIC_LVT_MASKED | APIC_SEND_PENDING | APIC_VECTOR_MASK
+    (APIC_LVT_MASKED | APIC_SEND_PENDING | APIC_VECTOR_MASK)
 
 #define LINT_MASK   \
-    LVT_MASK | APIC_MODE_MASK | APIC_INPUT_POLARITY |\
-    APIC_LVT_REMOTE_IRR | APIC_LVT_LEVEL_TRIGGER
+    (LVT_MASK | APIC_MODE_MASK | APIC_INPUT_POLARITY |\
+    APIC_LVT_REMOTE_IRR | APIC_LVT_LEVEL_TRIGGER)
 
 static const unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] =
 {
@@ -614,7 +614,7 @@ int hvm_x2apic_msr_read(struct vcpu *v, 
     uint32_t low, high = 0, offset = (msr - MSR_IA32_APICBASE_MSR) << 4;
 
     if ( !vlapic_x2apic_mode(vlapic) )
-        return 1;
+        return X86EMUL_UNHANDLEABLE;
 
     vlapic_read_aligned(vlapic, offset, &low);
     switch ( offset )
@@ -627,12 +627,15 @@ int hvm_x2apic_msr_read(struct vcpu *v, 
         vlapic_read_aligned(vlapic, APIC_ICR2, &high);
         break;
 
+    case APIC_EOI:
     case APIC_ICR2:
-        return 1;
+    case APIC_SELF_IPI:
+        return X86EMUL_UNHANDLEABLE;
     }
 
     *msr_content = (((uint64_t)high) << 32) | low;
-    return 0;
+
+    return X86EMUL_OKAY;
 }
 
 static void vlapic_pt_cb(struct vcpu *v, void *data)
@@ -656,10 +659,7 @@ static int vlapic_reg_write(struct vcpu 
     switch ( offset )
     {
     case APIC_ID:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            vlapic_set_reg(vlapic, APIC_ID, val);
-        else
-            rc = X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_ID, val);
         break;
 
     case APIC_TASKPRI:
@@ -671,17 +671,11 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_LDR:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
-        else
-            rc = X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
         break;
 
     case APIC_DFR:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
-        else
-            rc = X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
         break;
 
     case APIC_SPIV:
@@ -708,21 +702,6 @@ static int vlapic_reg_write(struct vcpu 
         }
         break;
 
-    case APIC_ESR:
-        if ( vlapic_x2apic_mode(vlapic) && (val != 0) )
-        {
-            gdprintk(XENLOG_ERR, "Local APIC write ESR with non-zero %lx\n",
-                    val);
-            rc = X86EMUL_UNHANDLEABLE;
-        }
-        break;
-
-    case APIC_SELF_IPI:
-        rc = vlapic_x2apic_mode(vlapic)
-            ? vlapic_reg_write(v, APIC_ICR, 0x40000 | (val & 0xff))
-            : X86EMUL_UNHANDLEABLE;
-        break;
-
     case APIC_ICR:
         val &= ~(1 << 12); /* always clear the pending bit */
         vlapic_ipi(vlapic, val, vlapic_get_reg(vlapic, APIC_ICR2));
@@ -730,9 +709,7 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_ICR2:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            val &= 0xff000000;
-        vlapic_set_reg(vlapic, APIC_ICR2, val);
+        vlapic_set_reg(vlapic, APIC_ICR2, val & 0xff000000);
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
@@ -877,8 +854,16 @@ static int vlapic_write(struct vcpu *v, 
 
 int vlapic_apicv_write(struct vcpu *v, unsigned int offset)
 {
-    uint32_t val = vlapic_get_reg(vcpu_vlapic(v), offset);
-    return vlapic_reg_write(v, offset, val);
+    struct vlapic *vlapic = vcpu_vlapic(v);
+    uint32_t val = vlapic_get_reg(vlapic, offset);
+
+    if ( !vlapic_x2apic_mode(vlapic) )
+        return vlapic_reg_write(v, offset, val);
+
+    if ( offset != APIC_SELF_IPI )
+        return X86EMUL_UNHANDLEABLE;
+
+    return vlapic_reg_write(v, APIC_ICR, APIC_DEST_SELF | (uint8_t)val);
 }
 
 int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t msr_content)
@@ -891,16 +876,69 @@ int hvm_x2apic_msr_write(struct vcpu *v,
 
     switch ( offset )
     {
-        int rc;
+    case APIC_TASKPRI:
+        if ( msr_content & ~APIC_TPRI_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_SPIV:
+        if ( msr_content & ~(APIC_VECTOR_MASK | APIC_SPIV_APIC_ENABLED |
+                             (VLAPIC_VERSION & APIC_LVR_DIRECTED_EOI
+                              ? APIC_SPIV_DIRECTED_EOI : 0)) )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVTT:
+        if ( msr_content & ~(LVT_MASK | APIC_TIMER_MODE_MASK) )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVTTHMR:
+    case APIC_LVTPC:
+    case APIC_CMCI:
+        if ( msr_content & ~(LVT_MASK | APIC_MODE_MASK) )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVT0:
+    case APIC_LVT1:
+        if ( msr_content & ~LINT_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVTERR:
+        if ( msr_content & ~LVT_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_TMICT:
+        break;
+
+    case APIC_TDCR:
+        if ( msr_content & ~APIC_TDR_DIV_1 )
+            return X86EMUL_UNHANDLEABLE;
+        break;
 
     case APIC_ICR:
-        rc = vlapic_reg_write(v, APIC_ICR2, (uint32_t)(msr_content >> 32));
-        if ( rc )
-            return rc;
+        if ( (uint32_t)msr_content & ~(APIC_VECTOR_MASK | APIC_MODE_MASK |
+                                       APIC_DEST_MASK | APIC_INT_ASSERT |
+                                       APIC_INT_LEVELTRIG | APIC_SHORT_MASK) )
+            return X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32);
         break;
 
-    case APIC_ICR2:
-        return X86EMUL_UNHANDLEABLE;
+    case APIC_SELF_IPI:
+        if ( msr_content & ~APIC_VECTOR_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        offset = APIC_ICR;
+        msr_content = APIC_DEST_SELF | (msr_content & APIC_VECTOR_MASK);
+        break;
+
+    case APIC_EOI:
+    case APIC_ESR:
+        if ( msr_content )
+    default:
+            return X86EMUL_UNHANDLEABLE;
     }
 
     return vlapic_reg_write(v, offset, (uint32_t)msr_content);
@@ -910,7 +948,10 @@ static int vlapic_range(struct vcpu *v, 
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
     unsigned long offset  = addr - vlapic_base_address(vlapic);
-    return (!vlapic_hw_disabled(vlapic) && (offset < PAGE_SIZE));
+
+    return !vlapic_hw_disabled(vlapic) &&
+           !vlapic_x2apic_mode(vlapic) &&
+           (offset < PAGE_SIZE);
 }
 
 const struct hvm_mmio_handler vlapic_mmio_handler = {
@@ -919,10 +960,12 @@ const struct hvm_mmio_handler vlapic_mmi
     .write_handler = vlapic_write
 };
 
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
 {
     if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE )
     {
+        if ( unlikely(value & MSR_IA32_APICBASE_EXTD) )
+            return 0;
         if ( value & MSR_IA32_APICBASE_ENABLE )
         {
             vlapic_reset(vlapic);
@@ -931,10 +974,15 @@ void vlapic_msr_set(struct vlapic *vlapi
         }
         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) )
+        return 0;
 
     vlapic->hw.apic_base_msr = value;
 
@@ -949,6 +997,8 @@ void vlapic_msr_set(struct vlapic *vlapi
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                 "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+
+    return 1;
 }
 
 uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
@@ -1232,6 +1282,10 @@ static int lapic_load_hidden(struct doma
     if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 ) 
         return -EINVAL;
 
+    if ( !(s->hw.apic_base_msr & MSR_IA32_APICBASE_ENABLE) &&
+         unlikely(vlapic_x2apic_mode(s)) )
+        return -EINVAL;
+
     vmx_vlapic_msr_changed(v);
 
     return 0;
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3110,8 +3110,7 @@ void vmx_vmexit_handler(struct cpu_user_
         break;
 
     case EXIT_REASON_APIC_WRITE:
-        if ( vmx_handle_apic_write() )
-            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+        vmx_handle_apic_write();
         break;
 
     case EXIT_REASON_ACCESS_GDTR_OR_IDTR:
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -106,7 +106,7 @@ void vlapic_destroy(struct vcpu *v);
 
 void vlapic_reset(struct vlapic *vlapic);
 
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
 void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
 uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 



[-- Attachment #2: x86-HVM-x2APIC-misc.patch --]
[-- Type: text/plain, Size: 11132 bytes --]

x86/HVM: fix miscellaneous aspects of x2APIC emulation

- generate #GP on invalid APIC base MSR transitions
- fail reads from the EOI and self-IPI registers (which are write-only)
- handle self-IPI writes and the ICR2 half of ICR writes largely in
  hvm_x2apic_msr_write() and (for self-IPI only) vlapic_apicv_write()
- don't permit MMIO-based access in x2APIC mode
- filter writes to read-only registers in hvm_x2apic_msr_write(),
  allowing conditionals to be dropped from vlapic_reg_write()
- don't ignore upper half of MSR-based write to ESR being non-zero
- don't ignore other writes to reserved bits
- VMX's EXIT_REASON_APIC_WRITE must not result in #GP (this exit being
  trap-like, this exception would get raised on the wrong RIP)
- make hvm_x2apic_msr_read() produce X86EMUL_* return codes just like
  hvm_x2apic_msr_write() does (benign to the only caller)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v5: Force #GP to be raised on reserved bit writes. Drop APIC_TMCCT from
    set of writable registers. Drop message previously issued on x2APIC
    ESR writes.
v3: Also handle APIC_EOI in hvm_x2apic_msr_read() as pointed out by
    Andrew. Filter MMIO-based accesses in vlapic_range() when in x2APIC
    mode. Move x2APIC special casing from vlapic_reg_write() to
    hvm_x2apic_msr_write(). Don't open-code vlapic_x2apic_mode().
v2: Split from main patch.

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4499,7 +4499,8 @@ int hvm_msr_write_intercept(unsigned int
         break;
 
     case MSR_IA32_APICBASE:
-        vlapic_msr_set(vcpu_vlapic(v), msr_content);
+        if ( !vlapic_msr_set(vcpu_vlapic(v), msr_content) )
+            goto gp_fault;
         break;
 
     case MSR_IA32_TSC_DEADLINE:
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -45,11 +45,11 @@
 #define VLAPIC_LVT_NUM                  6
 
 #define LVT_MASK \
-    APIC_LVT_MASKED | APIC_SEND_PENDING | APIC_VECTOR_MASK
+    (APIC_LVT_MASKED | APIC_SEND_PENDING | APIC_VECTOR_MASK)
 
 #define LINT_MASK   \
-    LVT_MASK | APIC_MODE_MASK | APIC_INPUT_POLARITY |\
-    APIC_LVT_REMOTE_IRR | APIC_LVT_LEVEL_TRIGGER
+    (LVT_MASK | APIC_MODE_MASK | APIC_INPUT_POLARITY |\
+    APIC_LVT_REMOTE_IRR | APIC_LVT_LEVEL_TRIGGER)
 
 static const unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] =
 {
@@ -614,7 +614,7 @@ int hvm_x2apic_msr_read(struct vcpu *v, 
     uint32_t low, high = 0, offset = (msr - MSR_IA32_APICBASE_MSR) << 4;
 
     if ( !vlapic_x2apic_mode(vlapic) )
-        return 1;
+        return X86EMUL_UNHANDLEABLE;
 
     vlapic_read_aligned(vlapic, offset, &low);
     switch ( offset )
@@ -627,12 +627,15 @@ int hvm_x2apic_msr_read(struct vcpu *v, 
         vlapic_read_aligned(vlapic, APIC_ICR2, &high);
         break;
 
+    case APIC_EOI:
     case APIC_ICR2:
-        return 1;
+    case APIC_SELF_IPI:
+        return X86EMUL_UNHANDLEABLE;
     }
 
     *msr_content = (((uint64_t)high) << 32) | low;
-    return 0;
+
+    return X86EMUL_OKAY;
 }
 
 static void vlapic_pt_cb(struct vcpu *v, void *data)
@@ -656,10 +659,7 @@ static int vlapic_reg_write(struct vcpu 
     switch ( offset )
     {
     case APIC_ID:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            vlapic_set_reg(vlapic, APIC_ID, val);
-        else
-            rc = X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_ID, val);
         break;
 
     case APIC_TASKPRI:
@@ -671,17 +671,11 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_LDR:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
-        else
-            rc = X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
         break;
 
     case APIC_DFR:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
-        else
-            rc = X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
         break;
 
     case APIC_SPIV:
@@ -708,21 +702,6 @@ static int vlapic_reg_write(struct vcpu 
         }
         break;
 
-    case APIC_ESR:
-        if ( vlapic_x2apic_mode(vlapic) && (val != 0) )
-        {
-            gdprintk(XENLOG_ERR, "Local APIC write ESR with non-zero %lx\n",
-                    val);
-            rc = X86EMUL_UNHANDLEABLE;
-        }
-        break;
-
-    case APIC_SELF_IPI:
-        rc = vlapic_x2apic_mode(vlapic)
-            ? vlapic_reg_write(v, APIC_ICR, 0x40000 | (val & 0xff))
-            : X86EMUL_UNHANDLEABLE;
-        break;
-
     case APIC_ICR:
         val &= ~(1 << 12); /* always clear the pending bit */
         vlapic_ipi(vlapic, val, vlapic_get_reg(vlapic, APIC_ICR2));
@@ -730,9 +709,7 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_ICR2:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            val &= 0xff000000;
-        vlapic_set_reg(vlapic, APIC_ICR2, val);
+        vlapic_set_reg(vlapic, APIC_ICR2, val & 0xff000000);
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
@@ -877,8 +854,16 @@ static int vlapic_write(struct vcpu *v, 
 
 int vlapic_apicv_write(struct vcpu *v, unsigned int offset)
 {
-    uint32_t val = vlapic_get_reg(vcpu_vlapic(v), offset);
-    return vlapic_reg_write(v, offset, val);
+    struct vlapic *vlapic = vcpu_vlapic(v);
+    uint32_t val = vlapic_get_reg(vlapic, offset);
+
+    if ( !vlapic_x2apic_mode(vlapic) )
+        return vlapic_reg_write(v, offset, val);
+
+    if ( offset != APIC_SELF_IPI )
+        return X86EMUL_UNHANDLEABLE;
+
+    return vlapic_reg_write(v, APIC_ICR, APIC_DEST_SELF | (uint8_t)val);
 }
 
 int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t msr_content)
@@ -891,16 +876,69 @@ int hvm_x2apic_msr_write(struct vcpu *v,
 
     switch ( offset )
     {
-        int rc;
+    case APIC_TASKPRI:
+        if ( msr_content & ~APIC_TPRI_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_SPIV:
+        if ( msr_content & ~(APIC_VECTOR_MASK | APIC_SPIV_APIC_ENABLED |
+                             (VLAPIC_VERSION & APIC_LVR_DIRECTED_EOI
+                              ? APIC_SPIV_DIRECTED_EOI : 0)) )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVTT:
+        if ( msr_content & ~(LVT_MASK | APIC_TIMER_MODE_MASK) )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVTTHMR:
+    case APIC_LVTPC:
+    case APIC_CMCI:
+        if ( msr_content & ~(LVT_MASK | APIC_MODE_MASK) )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVT0:
+    case APIC_LVT1:
+        if ( msr_content & ~LINT_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_LVTERR:
+        if ( msr_content & ~LVT_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        break;
+
+    case APIC_TMICT:
+        break;
+
+    case APIC_TDCR:
+        if ( msr_content & ~APIC_TDR_DIV_1 )
+            return X86EMUL_UNHANDLEABLE;
+        break;
 
     case APIC_ICR:
-        rc = vlapic_reg_write(v, APIC_ICR2, (uint32_t)(msr_content >> 32));
-        if ( rc )
-            return rc;
+        if ( (uint32_t)msr_content & ~(APIC_VECTOR_MASK | APIC_MODE_MASK |
+                                       APIC_DEST_MASK | APIC_INT_ASSERT |
+                                       APIC_INT_LEVELTRIG | APIC_SHORT_MASK) )
+            return X86EMUL_UNHANDLEABLE;
+        vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32);
         break;
 
-    case APIC_ICR2:
-        return X86EMUL_UNHANDLEABLE;
+    case APIC_SELF_IPI:
+        if ( msr_content & ~APIC_VECTOR_MASK )
+            return X86EMUL_UNHANDLEABLE;
+        offset = APIC_ICR;
+        msr_content = APIC_DEST_SELF | (msr_content & APIC_VECTOR_MASK);
+        break;
+
+    case APIC_EOI:
+    case APIC_ESR:
+        if ( msr_content )
+    default:
+            return X86EMUL_UNHANDLEABLE;
     }
 
     return vlapic_reg_write(v, offset, (uint32_t)msr_content);
@@ -910,7 +948,10 @@ static int vlapic_range(struct vcpu *v, 
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
     unsigned long offset  = addr - vlapic_base_address(vlapic);
-    return (!vlapic_hw_disabled(vlapic) && (offset < PAGE_SIZE));
+
+    return !vlapic_hw_disabled(vlapic) &&
+           !vlapic_x2apic_mode(vlapic) &&
+           (offset < PAGE_SIZE);
 }
 
 const struct hvm_mmio_handler vlapic_mmio_handler = {
@@ -919,10 +960,12 @@ const struct hvm_mmio_handler vlapic_mmi
     .write_handler = vlapic_write
 };
 
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
 {
     if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE )
     {
+        if ( unlikely(value & MSR_IA32_APICBASE_EXTD) )
+            return 0;
         if ( value & MSR_IA32_APICBASE_ENABLE )
         {
             vlapic_reset(vlapic);
@@ -931,10 +974,15 @@ void vlapic_msr_set(struct vlapic *vlapi
         }
         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) )
+        return 0;
 
     vlapic->hw.apic_base_msr = value;
 
@@ -949,6 +997,8 @@ void vlapic_msr_set(struct vlapic *vlapi
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                 "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+
+    return 1;
 }
 
 uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
@@ -1232,6 +1282,10 @@ static int lapic_load_hidden(struct doma
     if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 ) 
         return -EINVAL;
 
+    if ( !(s->hw.apic_base_msr & MSR_IA32_APICBASE_ENABLE) &&
+         unlikely(vlapic_x2apic_mode(s)) )
+        return -EINVAL;
+
     vmx_vlapic_msr_changed(v);
 
     return 0;
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3110,8 +3110,7 @@ void vmx_vmexit_handler(struct cpu_user_
         break;
 
     case EXIT_REASON_APIC_WRITE:
-        if ( vmx_handle_apic_write() )
-            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+        vmx_handle_apic_write();
         break;
 
     case EXIT_REASON_ACCESS_GDTR_OR_IDTR:
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -106,7 +106,7 @@ void vlapic_destroy(struct vcpu *v);
 
 void vlapic_reset(struct vlapic *vlapic);
 
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
 void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
 uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

  reply	other threads:[~2014-09-24 15:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-24 15:28 [PATCH v5 0/4] x86/HVM: fix various aspects of APIC emulation Jan Beulich
2014-09-24 15:34 ` Jan Beulich [this message]
2014-09-24 15:57   ` [PATCH v5 1/4] x86/HVM: fix miscellaneous aspects of x2APIC emulation Andrew Cooper
2014-09-25  8:42     ` Jan Beulich
2014-09-25 11:26       ` Andrew Cooper
2014-09-24 15:35 ` [PATCH v5 2/4] x86/HVM: fix ID handling " Jan Beulich
2014-09-24 15:59   ` Andrew Cooper
2014-09-24 15:36 ` [PATCH v5 3/4] x86/vlapic: a few type adjustments Jan Beulich
2014-09-24 16:02   ` Andrew Cooper
2014-09-24 15:37 ` [PATCH v5 4/4] x86/vlapic: don't silently accept bad vectors Jan Beulich
2014-09-25 11:17 ` [PATCH v5 0/4] x86/HVM: fix various aspects of APIC emulation Tim Deegan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=542300AF02000078000386B2@mail.emea.novell.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=keir@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).