Starting with commit 84e848fd7a16 ("x86/hvm: disallow access to unknown MSRs") accesses to unhandled MSRs result in #GP sent to the guest. This caused a regression for Solaris who tries to acccess MSR_RAPL_POWER_UNIT and (unlike, for example, Linux) does not catch exceptions when accessing MSRs that potentially may not be present.
Instead of special-casing RAPL registers we decide what to do when any non-emulated MSR is accessed based on ignore_msrs field of msr_policy. Signed-off-by: Boris Ostrovsky <boris.ostrov...@oracle.com> --- Changes in v2: * define x86_emul_guest_msr_access() and use it to determine whether emulated instruction is rd/wrmsr. * Don't use ignore_msrs for MSR accesses that are not guest's rd/wrmsr. * Clear @val for writes too in guest_unhandled_msr() xen/arch/x86/hvm/svm/svm.c | 10 ++++------ xen/arch/x86/hvm/vmx/vmx.c | 10 ++++------ xen/arch/x86/msr.c | 28 ++++++++++++++++++++++++++++ xen/arch/x86/pv/emul-priv-op.c | 10 ++++++---- xen/arch/x86/x86_emulate/x86_emulate.h | 6 ++++++ xen/include/asm-x86/msr.h | 3 +++ 6 files changed, 51 insertions(+), 16 deletions(-) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index b819897a4a9f..7b59885b2619 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1965,8 +1965,8 @@ static int svm_msr_read_intercept(unsigned int msr, uint64_t *msr_content) break; default: - gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", msr); - goto gpf; + if ( guest_unhandled_msr(v, msr, msr_content, false, true) ) + goto gpf; } HVM_DBG_LOG(DBG_LEVEL_MSR, "returns: ecx=%x, msr_value=%"PRIx64, @@ -2151,10 +2151,8 @@ static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content) break; default: - gdprintk(XENLOG_WARNING, - "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n", - msr, msr_content); - goto gpf; + if ( guest_unhandled_msr(v, msr, &msr_content, true, true) ) + goto gpf; } return X86EMUL_OKAY; diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 2d4475ee3de2..87baca57d33f 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -3017,8 +3017,8 @@ static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content) break; } - gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", msr); - goto gp_fault; + if ( guest_unhandled_msr(curr, msr, msr_content, false, true) ) + goto gp_fault; } done: @@ -3319,10 +3319,8 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) is_last_branch_msr(msr) ) break; - gdprintk(XENLOG_WARNING, - "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n", - msr, msr_content); - goto gp_fault; + if ( guest_unhandled_msr(v, msr, &msr_content, true, true) ) + goto gp_fault; } return X86EMUL_OKAY; diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c index 433d16c80728..a57d838f642b 100644 --- a/xen/arch/x86/msr.c +++ b/xen/arch/x86/msr.c @@ -164,6 +164,34 @@ int init_vcpu_msr_policy(struct vcpu *v) return 0; } +/* Returns true if policy requires #GP to the guest. */ +bool guest_unhandled_msr(const struct vcpu *v, uint32_t msr, uint64_t *val, + bool is_write, bool is_guest_msr_access) +{ + u8 ignore_msrs = v->domain->arch.msr->ignore_msrs; + + /* + * Accesses to unimplemented MSRs as part of emulation of instructions + * other than guest's RDMSR/WRMSR should never succeed. + */ + if ( !is_guest_msr_access ) + ignore_msrs = MSR_UNHANDLED_NEVER; + + if ( unlikely(ignore_msrs != MSR_UNHANDLED_NEVER) ) + *val = 0; + + if ( likely(ignore_msrs != MSR_UNHANDLED_SILENT) ) + { + if ( is_write ) + gdprintk(XENLOG_WARNING, "WRMSR 0x%08x val 0x%016"PRIx64 + " unimplemented\n", msr, *val); + else + gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", msr); + } + + return (ignore_msrs == MSR_UNHANDLED_NEVER); +} + int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val) { const struct vcpu *curr = current; diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c index dbceed8a05fd..6b378dbe2239 100644 --- a/xen/arch/x86/pv/emul-priv-op.c +++ b/xen/arch/x86/pv/emul-priv-op.c @@ -984,7 +984,9 @@ static int read_msr(unsigned int reg, uint64_t *val, } /* fall through */ default: - gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", reg); + if ( !guest_unhandled_msr(curr, reg, val, false, + x86_emul_guest_msr_access(ctxt)) ) + return X86EMUL_OKAY; break; normal: @@ -1146,9 +1148,9 @@ static int write_msr(unsigned int reg, uint64_t val, } /* fall through */ default: - gdprintk(XENLOG_WARNING, - "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n", - reg, val); + if ( !guest_unhandled_msr(curr, reg, &val, true, + x86_emul_guest_msr_access(ctxt)) ) + return X86EMUL_OKAY; break; invalid: diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h index d8fb3a990933..06e6b7479f37 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -850,4 +850,10 @@ static inline void x86_emul_reset_event(struct x86_emulate_ctxt *ctxt) ctxt->event = (struct x86_event){}; } +static inline bool x86_emul_guest_msr_access(struct x86_emulate_ctxt *ctxt) +{ + return ctxt->opcode == X86EMUL_OPC(0x0f, 0x32) || /* RDMSR */ + ctxt->opcode == X86EMUL_OPC(0x0f, 0x30); /* WRMSR */ +} + #endif /* __X86_EMULATE_H__ */ diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h index 16f95e734428..e7d69ad5bf29 100644 --- a/xen/include/asm-x86/msr.h +++ b/xen/include/asm-x86/msr.h @@ -345,5 +345,8 @@ int init_vcpu_msr_policy(struct vcpu *v); */ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val); int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val); +bool guest_unhandled_msr(const struct vcpu *v, uint32_t msr, + uint64_t *val, bool is_write, + bool is_guest_msr_access); #endif /* __ASM_MSR_H */ -- 1.8.3.1