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.ostrovsky@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
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.ostrovsky@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