|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v5 1/4] 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@xxxxxxxx>
---
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:
x86-HVM-x2APIC-misc.patch _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |