x86/HVM: fix miscellaneous aspects of x2APIC emulation - generate #GP on invalid APIC base MSR transitions - fail reads from the self-IPI register (which is write-only) - handle self-IPI writes and the ICR2 half of ICR writes largely in hvm_x2apic_msr_write() Signed-off-by: Jan Beulich --- 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 @@ -602,6 +602,7 @@ int hvm_x2apic_msr_read(struct vcpu *v, break; case APIC_ICR2: + case APIC_SELF_IPI: return 1; } @@ -692,9 +693,7 @@ static int vlapic_reg_write(struct vcpu break; case APIC_SELF_IPI: - rc = vlapic_x2apic_mode(vlapic) - ? vlapic_reg_write(v, APIC_ICR, 0x40000 | (val & 0xff)) - : X86EMUL_UNHANDLEABLE; + rc = X86EMUL_UNHANDLEABLE; break; case APIC_ICR: @@ -704,9 +703,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 */ @@ -865,16 +862,17 @@ int hvm_x2apic_msr_write(struct vcpu *v, switch ( offset ) { - int rc; - case APIC_ICR: - rc = vlapic_reg_write(v, APIC_ICR2, (uint32_t)(msr_content >> 32)); - if ( rc ) - return rc; + vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32); break; case APIC_ICR2: return X86EMUL_UNHANDLEABLE; + + case APIC_SELF_IPI: + offset = APIC_ICR; + msr_content = APIC_DEST_SELF | (uint8_t)msr_content; + break; } return vlapic_reg_write(v, offset, (uint32_t)msr_content); @@ -893,10 +891,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); @@ -905,10 +905,15 @@ void vlapic_msr_set(struct vlapic *vlapi } else { + if ( unlikely(vlapic->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD) ) + 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; @@ -923,6 +928,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) @@ -1206,6 +1213,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(s->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD) ) + return -EINVAL; + vmx_vlapic_msr_changed(v); return 0; --- 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);