|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen master] SVM: clean up svm_vmcb_isvalid()
commit 136d46e548db1c346c3e14b8d6d5b771fc30ca41
Author: Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Mon Jun 12 09:32:14 2017 +0200
Commit: Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Jun 12 09:32:14 2017 +0200
SVM: clean up svm_vmcb_isvalid()
- correct CR3, CR4, and EFER checks
- delete bogus nested paging check
- add vcpu parameter (to include in log messages) and constify vmcb one
- use bool/true/false
- use accessors (and local variables to improve code readability)
- adjust formatting
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
---
xen/arch/x86/hvm/svm/nestedsvm.c | 4 +-
xen/arch/x86/hvm/svm/svmdebug.c | 131 ++++++++++++++-------------------
xen/include/asm-x86/hvm/svm/svmdebug.h | 4 +-
3 files changed, 61 insertions(+), 78 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index f46e346..8fd9c23 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -658,13 +658,13 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct
cpu_user_regs *regs)
/* Cleanbits */
n2vmcb->cleanbits.bytes = 0;
- rc = svm_vmcb_isvalid(__func__, ns_vmcb, 1);
+ rc = svm_vmcb_isvalid(__func__, ns_vmcb, v, true);
if (rc) {
gdprintk(XENLOG_ERR, "virtual vmcb invalid\n");
return NSVM_ERROR_VVMCB;
}
- rc = svm_vmcb_isvalid(__func__, n2vmcb, 1);
+ rc = svm_vmcb_isvalid(__func__, n2vmcb, v, true);
if (rc) {
gdprintk(XENLOG_ERR, "n2vmcb invalid\n");
return NSVM_ERROR_VMENTRY;
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index 961e098..a3f8685 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -16,6 +16,7 @@
*
*/
+#include <xen/sched.h>
#include <asm/processor.h>
#include <asm/msr-index.h>
#include <asm/hvm/svm/svmdebug.h>
@@ -87,93 +88,75 @@ void svm_vmcb_dump(const char *from, const struct
vmcb_struct *vmcb)
svm_dump_sel(" TR", &vmcb->tr);
}
-bool_t
-svm_vmcb_isvalid(const char *from, struct vmcb_struct *vmcb,
- bool_t verbose)
+bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
+ const struct vcpu *v, bool verbose)
{
- bool_t ret = 0; /* ok */
-
-#define PRINTF(...) \
- if (verbose) { ret = 1; printk("%s: ", from); printk(__VA_ARGS__); \
- } else return 1;
-
- if ((vmcb->_efer & EFER_SVME) == 0) {
- PRINTF("EFER: SVME bit not set (%#"PRIx64")\n", vmcb->_efer);
- }
-
- if ((vmcb->_cr0 & X86_CR0_CD) == 0 && (vmcb->_cr0 & X86_CR0_NW) != 0) {
- PRINTF("CR0: CD bit is zero and NW bit set (%#"PRIx64")\n",
- vmcb->_cr0);
- }
-
- if ((vmcb->_cr0 >> 32U) != 0) {
- PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n",
- vmcb->_cr0);
- }
-
- if ((vmcb->_cr3 & 0x7) != 0) {
- PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", vmcb->_cr3);
- }
- if ((vmcb->_efer & EFER_LMA) && (vmcb->_cr3 & 0xfe) != 0) {
- PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", vmcb->_cr3);
- }
-
- if ((vmcb->_cr4 >> 19U) != 0) {
- PRINTF("CR4: bits [63:19] are not zero (%#"PRIx64")\n",
- vmcb->_cr4);
- }
-
- if (((vmcb->_cr4 >> 11U) & 0x7fU) != 0) {
- PRINTF("CR4: bits [17:11] are not zero (%#"PRIx64")\n",
- vmcb->_cr4);
- }
-
- if ((vmcb->_dr6 >> 32U) != 0) {
+ bool ret = false; /* ok */
+ unsigned long cr0 = vmcb_get_cr0(vmcb);
+ unsigned long cr3 = vmcb_get_cr3(vmcb);
+ unsigned long cr4 = vmcb_get_cr4(vmcb);
+ uint64_t efer = vmcb_get_efer(vmcb);
+
+#define PRINTF(fmt, args...) do { \
+ if ( !verbose ) return true; \
+ ret = true; \
+ printk(XENLOG_GUEST "%pv[%s]: " fmt, v, from, ## args); \
+} while (0)
+
+ if ( !(efer & EFER_SVME) )
+ PRINTF("EFER: SVME bit not set (%#"PRIx64")\n", efer);
+
+ if ( !(cr0 & X86_CR0_CD) && (cr0 & X86_CR0_NW) )
+ PRINTF("CR0: CD bit is zero and NW bit set (%#"PRIx64")\n", cr0);
+
+ if ( cr0 >> 32 )
+ PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n", cr0);
+
+ if ( (cr0 & X86_CR0_PG) &&
+ ((cr3 & 7) ||
+ ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
+ ((efer & EFER_LMA) &&
+ (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
+ PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
+
+ if ( cr4 & ~hvm_cr4_guest_valid_bits(v, false) )
+ PRINTF("CR4: invalid bits are set (%#"PRIx64", valid: %#"PRIx64")\n",
+ cr4, hvm_cr4_guest_valid_bits(v, false));
+
+ if ( vmcb_get_dr6(vmcb) >> 32 )
PRINTF("DR6: bits [63:32] are not zero (%#"PRIx64")\n",
- vmcb->_dr6);
- }
+ vmcb_get_dr6(vmcb));
- if ((vmcb->_dr7 >> 32U) != 0) {
+ if ( vmcb_get_dr7(vmcb) >> 32 )
PRINTF("DR7: bits [63:32] are not zero (%#"PRIx64")\n",
- vmcb->_dr7);
- }
+ vmcb_get_dr7(vmcb));
- if ((vmcb->_efer >> 15U) != 0) {
- PRINTF("EFER: bits [63:15] are not zero (%#"PRIx64")\n",
- vmcb->_efer);
- }
+ if ( efer & ~(EFER_SCE | EFER_LME | EFER_LMA | EFER_NX | EFER_SVME |
+ EFER_LMSLE | EFER_FFXSE) )
+ PRINTF("EFER: undefined bits are not zero (%#"PRIx64")\n", efer);
- if ((vmcb->_efer & EFER_LME) != 0 && ((vmcb->_cr0 & X86_CR0_PG) != 0)) {
- if ((vmcb->_cr4 & X86_CR4_PAE) == 0) {
- PRINTF("EFER_LME and CR0.PG are both set and CR4.PAE is zero.\n");
- }
- if ((vmcb->_cr0 & X86_CR0_PE) == 0) {
- PRINTF("EFER_LME and CR0.PG are both set and CR0.PE is zero.\n");
- }
- }
+ if ( hvm_efer_valid(v, efer, -1) )
+ PRINTF("EFER: %s (%"PRIx64")\n", hvm_efer_valid(v, efer, -1), efer);
- if ((vmcb->_efer & EFER_LME) != 0
- && (vmcb->_cr0 & X86_CR0_PG) != 0
- && (vmcb->_cr4 & X86_CR4_PAE) != 0
- && (vmcb->cs.attr.fields.l != 0)
- && (vmcb->cs.attr.fields.db != 0))
+ if ( (efer & EFER_LME) && (cr0 & X86_CR0_PG) )
{
- PRINTF("EFER_LME, CR0.PG, CR4.PAE, CS.L and CS.D are all non-zero.\n");
+ if ( !(cr4 & X86_CR4_PAE) )
+ PRINTF("EFER_LME and CR0.PG are both set and CR4.PAE is zero\n");
+ if ( !(cr0 & X86_CR0_PE) )
+ PRINTF("EFER_LME and CR0.PG are both set and CR0.PE is zero\n");
}
- if ((vmcb->_general2_intercepts & GENERAL2_INTERCEPT_VMRUN) == 0) {
+ if ( (efer & EFER_LME) && (cr0 & X86_CR0_PG) && (cr4 & X86_CR4_PAE) &&
+ vmcb->cs.attr.fields.l && vmcb->cs.attr.fields.db )
+ PRINTF("EFER_LME, CR0.PG, CR4.PAE, CS.L and CS.D are all non-zero\n");
+
+ if ( !(vmcb_get_general2_intercepts(vmcb) & GENERAL2_INTERCEPT_VMRUN) )
PRINTF("GENERAL2_INTERCEPT: VMRUN intercept bit is clear
(%#"PRIx32")\n",
- vmcb->_general2_intercepts);
- }
+ vmcb_get_general2_intercepts(vmcb));
- if (vmcb->eventinj.fields.resvd1 != 0) {
+ if ( vmcb->eventinj.fields.resvd1 )
PRINTF("eventinj: MBZ bits are set (%#"PRIx64")\n",
- vmcb->eventinj.bytes);
- }
-
- if (vmcb->_np_enable && vmcb->_h_cr3 == 0) {
- PRINTF("nested paging enabled but host cr3 is 0\n");
- }
+ vmcb->eventinj.bytes);
#undef PRINTF
return ret;
diff --git a/xen/include/asm-x86/hvm/svm/svmdebug.h
b/xen/include/asm-x86/hvm/svm/svmdebug.h
index 92a7c2b..658cdd3 100644
--- a/xen/include/asm-x86/hvm/svm/svmdebug.h
+++ b/xen/include/asm-x86/hvm/svm/svmdebug.h
@@ -23,7 +23,7 @@
#include <asm/hvm/svm/vmcb.h>
void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb);
-bool_t svm_vmcb_isvalid(const char *from, struct vmcb_struct *vmcb,
- bool_t verbose);
+bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
+ const struct vcpu *v, bool verbose);
#endif /* __ASM_X86_HVM_SVM_SVMDEBUG_H__ */
--
generated by git-patchbot for /home/xen/git/xen.git#master
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |