[PATCH] svm: debug improvements

[PATCH] svm: debug improvements

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]


Hi!

Attached patch moves vmcb dump function into
its own svm debug files.
Add svm_vmcb_isvalid() to svm code to validate
the vmcb.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

P.S.: svm_vmcb_isvalid() is extracted from my
nested virtualization patches.

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_svmdebug.diff --]
[-- Type: text/x-diff, Size: 14235 bytes --]

diff -r e2f5e4f3481c xen/arch/x86/hvm/svm/Makefile
--- a/xen/arch/x86/hvm/svm/Makefile	Tue Jun 22 16:22:30 2010 +0100
+++ b/xen/arch/x86/hvm/svm/Makefile	Wed Jun 23 14:40:08 2010 +0200
@@ -3,5 +3,6 @@ obj-y += emulate.o
 obj-y += entry.o
 obj-y += intr.o
 obj-y += svm.o
+obj-y += svmdebug.o
 obj-y += vmcb.o
 obj-y += vpmu.o
diff -r e2f5e4f3481c xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c	Tue Jun 22 16:22:30 2010 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c	Wed Jun 23 14:40:08 2010 +0200
@@ -48,6 +48,7 @@
 #include <asm/hvm/svm/vmcb.h>
 #include <asm/hvm/svm/emulate.h>
 #include <asm/hvm/svm/intr.h>
+#include <asm/hvm/svm/svmdebug.h>
 #include <asm/x86_emulate.h>
 #include <public/sched.h>
 #include <asm/hvm/vpt.h>
@@ -1451,7 +1452,7 @@ asmlinkage void svm_vmexit_handler(struc
 
     if ( unlikely(exit_reason == VMEXIT_INVALID) )
     {
-        svm_dump_vmcb(__func__, vmcb);
+        svm_vmcb_dump(__func__, vmcb);
         goto exit_and_crash;
     }
 
diff -r e2f5e4f3481c xen/arch/x86/hvm/svm/svmdebug.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/arch/x86/hvm/svm/svmdebug.c	Wed Jun 23 14:40:08 2010 +0200
@@ -0,0 +1,185 @@
+/*
+ * svmdebug.c: debug functions
+ * Copyright (c) 2010, Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ */
+
+#include <asm/processor.h>
+#include <asm/msr-index.h>
+#include <asm/hvm/svm/svmdebug.h>
+
+static void svm_dump_sel(const char *name, svm_segment_register_t *s)
+{
+    printk("%s: sel=0x%04x, attr=0x%04x, limit=0x%08x, base=0x%016llx\n", 
+           name, s->sel, s->attr.bytes, s->limit,
+           (unsigned long long)s->base);
+}
+
+void svm_vmcb_dump(const char *from, struct vmcb_struct *vmcb)
+{
+    printk("Dumping guest's current state at %s...\n", from);
+    printk("Size of VMCB = %d, address = %p\n", 
+            (int) sizeof(struct vmcb_struct), vmcb);
+
+    printk("cr_intercepts = 0x%08x dr_intercepts = 0x%08x "
+           "exception_intercepts = 0x%08x\n", 
+           vmcb->cr_intercepts, vmcb->dr_intercepts, 
+           vmcb->exception_intercepts);
+    printk("general1_intercepts = 0x%08x general2_intercepts = 0x%08x\n", 
+           vmcb->general1_intercepts, vmcb->general2_intercepts);
+    printk("iopm_base_pa = %016llx msrpm_base_pa = 0x%016llx tsc_offset = "
+            "0x%016llx\n", 
+           (unsigned long long) vmcb->iopm_base_pa,
+           (unsigned long long) vmcb->msrpm_base_pa,
+           (unsigned long long) vmcb->tsc_offset);
+    printk("tlb_control = 0x%08x vintr = 0x%016llx interrupt_shadow = "
+            "0x%016llx\n", vmcb->tlb_control,
+           (unsigned long long) vmcb->vintr.bytes,
+           (unsigned long long) vmcb->interrupt_shadow);
+    printk("exitcode = 0x%016llx exitintinfo = 0x%016llx\n", 
+           (unsigned long long) vmcb->exitcode,
+           (unsigned long long) vmcb->exitintinfo.bytes);
+    printk("exitinfo1 = 0x%016llx exitinfo2 = 0x%016llx \n",
+           (unsigned long long) vmcb->exitinfo1,
+           (unsigned long long) vmcb->exitinfo2);
+    printk("np_enable = 0x%016llx guest_asid = 0x%03x\n", 
+           (unsigned long long) vmcb->np_enable, vmcb->guest_asid);
+    printk("cpl = %d efer = 0x%016llx star = 0x%016llx lstar = 0x%016llx\n", 
+           vmcb->cpl, (unsigned long long) vmcb->efer,
+           (unsigned long long) vmcb->star, (unsigned long long) vmcb->lstar);
+    printk("CR0 = 0x%016llx CR2 = 0x%016llx\n",
+           (unsigned long long) vmcb->cr0, (unsigned long long) vmcb->cr2);
+    printk("CR3 = 0x%016llx CR4 = 0x%016llx\n", 
+           (unsigned long long) vmcb->cr3, (unsigned long long) vmcb->cr4);
+    printk("RSP = 0x%016llx  RIP = 0x%016llx\n", 
+           (unsigned long long) vmcb->rsp, (unsigned long long) vmcb->rip);
+    printk("RAX = 0x%016llx  RFLAGS=0x%016llx\n",
+           (unsigned long long) vmcb->rax, (unsigned long long) vmcb->rflags);
+    printk("DR6 = 0x%016llx, DR7 = 0x%016llx\n", 
+           (unsigned long long) vmcb->dr6, (unsigned long long) vmcb->dr7);
+    printk("CSTAR = 0x%016llx SFMask = 0x%016llx\n",
+           (unsigned long long) vmcb->cstar, 
+           (unsigned long long) vmcb->sfmask);
+    printk("KernGSBase = 0x%016llx PAT = 0x%016llx \n", 
+           (unsigned long long) vmcb->kerngsbase,
+           (unsigned long long) vmcb->g_pat);
+    printk("H_CR3 = 0x%016llx\n", (unsigned long long)vmcb->h_cr3);
+
+    /* print out all the selectors */
+    svm_dump_sel("CS", &vmcb->cs);
+    svm_dump_sel("DS", &vmcb->ds);
+    svm_dump_sel("SS", &vmcb->ss);
+    svm_dump_sel("ES", &vmcb->es);
+    svm_dump_sel("FS", &vmcb->fs);
+    svm_dump_sel("GS", &vmcb->gs);
+    svm_dump_sel("GDTR", &vmcb->gdtr);
+    svm_dump_sel("LDTR", &vmcb->ldtr);
+    svm_dump_sel("IDTR", &vmcb->idtr);
+    svm_dump_sel("TR", &vmcb->tr);
+}
+
+bool_t
+svm_vmcb_isvalid(const char *from, struct vmcb_struct *vmcb,
+                 bool_t 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 (0x%"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 (0x%"PRIx64")\n",
+                vmcb->cr0);
+    }
+
+    if ((vmcb->cr0 >> 32U) != 0) {
+        PRINTF("CR0: bits [63:32] are not zero (0x%"PRIx64")\n",
+                vmcb->cr0);
+    }
+
+    if ((vmcb->cr3 & 0x7) != 0) {
+        PRINTF("CR3: MBZ bits are set (0x%"PRIx64")\n", vmcb->cr3);
+    }
+    if ((vmcb->efer & EFER_LMA) && (vmcb->cr3 & 0xfe) != 0) {
+        PRINTF("CR3: MBZ bits are set (0x%"PRIx64")\n", vmcb->cr3);
+    }
+
+    if ((vmcb->cr4 >> 11U) != 0) {
+        PRINTF("CR4: bits [63:11] are not zero (0x%"PRIx64")\n",
+                vmcb->cr4);
+    }
+
+    if ((vmcb->dr6 >> 32U) != 0) {
+        PRINTF("DR6: bits [63:32] are not zero (0x%"PRIx64")\n",
+                vmcb->dr6);
+    }
+
+    if ((vmcb->dr7 >> 32U) != 0) {
+        PRINTF("DR7: bits [63:32] are not zero (0x%"PRIx64")\n",
+                vmcb->dr7);
+    }
+
+    if ((vmcb->efer >> 15U) != 0) {
+        PRINTF("EFER: bits [63:15] are not zero (0x%"PRIx64")\n",
+                vmcb->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 ((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))
+    {
+        PRINTF("EFER_LME, CR0.PG, CR4.PAE, CS.L and CS.D are all non-zero.\n");
+    }
+
+    if ((vmcb->general2_intercepts & GENERAL2_INTERCEPT_VMRUN) == 0) {
+        PRINTF("GENERAL2_INTERCEPT: VMRUN intercept bit is clear (0x%"PRIx32")\n",
+            vmcb->general2_intercepts);
+    }
+
+    if (vmcb->eventinj.fields.resvd1 != 0) {
+        PRINTF("eventinj: MBZ bits are set (0x%"PRIx64")\n",
+                vmcb->eventinj.bytes);
+    }
+
+#undef PRINTF
+    return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff -r e2f5e4f3481c xen/arch/x86/hvm/svm/vmcb.c
--- a/xen/arch/x86/hvm/svm/vmcb.c	Tue Jun 22 16:22:30 2010 +0100
+++ b/xen/arch/x86/hvm/svm/vmcb.c	Wed Jun 23 14:40:08 2010 +0200
@@ -33,6 +33,7 @@
 #include <asm/hvm/svm/svm.h>
 #include <asm/hvm/svm/intr.h>
 #include <asm/hvm/svm/asid.h>
+#include <asm/hvm/svm/svmdebug.h>
 #include <xen/event.h>
 #include <xen/kernel.h>
 #include <xen/domain_page.h>
@@ -301,76 +302,6 @@ void svm_destroy_vmcb(struct vcpu *v)
     arch_svm->vmcb = NULL;
 }
 
-static void svm_dump_sel(char *name, svm_segment_register_t *s)
-{
-    printk("%s: sel=0x%04x, attr=0x%04x, limit=0x%08x, base=0x%016llx\n", 
-           name, s->sel, s->attr.bytes, s->limit,
-           (unsigned long long)s->base);
-}
-
-void svm_dump_vmcb(const char *from, struct vmcb_struct *vmcb)
-{
-    printk("Dumping guest's current state at %s...\n", from);
-    printk("Size of VMCB = %d, address = %p\n", 
-            (int) sizeof(struct vmcb_struct), vmcb);
-
-    printk("cr_intercepts = 0x%08x dr_intercepts = 0x%08x "
-           "exception_intercepts = 0x%08x\n", 
-           vmcb->cr_intercepts, vmcb->dr_intercepts, 
-           vmcb->exception_intercepts);
-    printk("general1_intercepts = 0x%08x general2_intercepts = 0x%08x\n", 
-           vmcb->general1_intercepts, vmcb->general2_intercepts);
-    printk("iopm_base_pa = %016llx msrpm_base_pa = 0x%016llx tsc_offset = "
-            "0x%016llx\n", 
-           (unsigned long long) vmcb->iopm_base_pa,
-           (unsigned long long) vmcb->msrpm_base_pa,
-           (unsigned long long) vmcb->tsc_offset);
-    printk("tlb_control = 0x%08x vintr = 0x%016llx interrupt_shadow = "
-            "0x%016llx\n", vmcb->tlb_control,
-           (unsigned long long) vmcb->vintr.bytes,
-           (unsigned long long) vmcb->interrupt_shadow);
-    printk("exitcode = 0x%016llx exitintinfo = 0x%016llx\n", 
-           (unsigned long long) vmcb->exitcode,
-           (unsigned long long) vmcb->exitintinfo.bytes);
-    printk("exitinfo1 = 0x%016llx exitinfo2 = 0x%016llx \n",
-           (unsigned long long) vmcb->exitinfo1,
-           (unsigned long long) vmcb->exitinfo2);
-    printk("np_enable = 0x%016llx guest_asid = 0x%03x\n", 
-           (unsigned long long) vmcb->np_enable, vmcb->guest_asid);
-    printk("cpl = %d efer = 0x%016llx star = 0x%016llx lstar = 0x%016llx\n", 
-           vmcb->cpl, (unsigned long long) vmcb->efer,
-           (unsigned long long) vmcb->star, (unsigned long long) vmcb->lstar);
-    printk("CR0 = 0x%016llx CR2 = 0x%016llx\n",
-           (unsigned long long) vmcb->cr0, (unsigned long long) vmcb->cr2);
-    printk("CR3 = 0x%016llx CR4 = 0x%016llx\n", 
-           (unsigned long long) vmcb->cr3, (unsigned long long) vmcb->cr4);
-    printk("RSP = 0x%016llx  RIP = 0x%016llx\n", 
-           (unsigned long long) vmcb->rsp, (unsigned long long) vmcb->rip);
-    printk("RAX = 0x%016llx  RFLAGS=0x%016llx\n",
-           (unsigned long long) vmcb->rax, (unsigned long long) vmcb->rflags);
-    printk("DR6 = 0x%016llx, DR7 = 0x%016llx\n", 
-           (unsigned long long) vmcb->dr6, (unsigned long long) vmcb->dr7);
-    printk("CSTAR = 0x%016llx SFMask = 0x%016llx\n",
-           (unsigned long long) vmcb->cstar, 
-           (unsigned long long) vmcb->sfmask);
-    printk("KernGSBase = 0x%016llx PAT = 0x%016llx \n", 
-           (unsigned long long) vmcb->kerngsbase,
-           (unsigned long long) vmcb->g_pat);
-    printk("H_CR3 = 0x%016llx\n", (unsigned long long)vmcb->h_cr3);
-
-    /* print out all the selectors */
-    svm_dump_sel("CS", &vmcb->cs);
-    svm_dump_sel("DS", &vmcb->ds);
-    svm_dump_sel("SS", &vmcb->ss);
-    svm_dump_sel("ES", &vmcb->es);
-    svm_dump_sel("FS", &vmcb->fs);
-    svm_dump_sel("GS", &vmcb->gs);
-    svm_dump_sel("GDTR", &vmcb->gdtr);
-    svm_dump_sel("LDTR", &vmcb->ldtr);
-    svm_dump_sel("IDTR", &vmcb->idtr);
-    svm_dump_sel("TR", &vmcb->tr);
-}
-
 static void vmcb_dump(unsigned char ch)
 {
     struct domain *d;
@@ -388,7 +319,7 @@ static void vmcb_dump(unsigned char ch)
         for_each_vcpu ( d, v )
         {
             printk("\tVCPU %d\n", v->vcpu_id);
-            svm_dump_vmcb("key_handler", v->arch.hvm_svm.vmcb);
+            svm_vmcb_dump("key_handler", v->arch.hvm_svm.vmcb);
         }
     }
 
diff -r e2f5e4f3481c xen/include/asm-x86/hvm/svm/svm.h
--- a/xen/include/asm-x86/hvm/svm/svm.h	Tue Jun 22 16:22:30 2010 +0100
+++ b/xen/include/asm-x86/hvm/svm/svm.h	Wed Jun 23 14:40:08 2010 +0200
@@ -29,8 +29,6 @@
 #include <asm/i387.h>
 #include <asm/hvm/vpmu.h>
 
-void svm_dump_vmcb(const char *from, struct vmcb_struct *vmcb);
-
 #define SVM_REG_EAX (0) 
 #define SVM_REG_ECX (1) 
 #define SVM_REG_EDX (2) 
diff -r e2f5e4f3481c xen/include/asm-x86/hvm/svm/svmdebug.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/include/asm-x86/hvm/svm/svmdebug.h	Wed Jun 23 14:40:08 2010 +0200
@@ -0,0 +1,30 @@
+/*
+ * svmdebug.h: SVM related debug defintions
+ * Copyright (c) 2010, AMD Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ */
+
+#ifndef __ASM_X86_HVM_SVM_SVMDEBUG_H__
+#define __ASM_X86_HVM_SVM_SVMDEBUG_H__
+
+#include <asm/types.h>
+#include <asm/hvm/svm/vmcb.h>
+
+void svm_vmcb_dump(const char *from, struct vmcb_struct *vmcb);
+bool_t svm_vmcb_isvalid(const char *from, struct vmcb_struct *vmcb,
+                        bool_t verbose);
+
+#endif /* __ASM_X86_HVM_SVM_SVMDEBUG_H__ */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] svm: debug improvements

[Keir Fraser]

On 23/06/2010 13:49, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> Attached patch moves vmcb dump function into
> its own svm debug files.
> Add svm_vmcb_isvalid() to svm code to validate
> the vmcb.

I won't take patches to add things that aren't used. Unless they are used
later in the same patch series, all posted with intention to be applied at
the same time. Otherwise the function is dead code, and I love to delete
dead code.

 -- Keir

Re: [PATCH] svm: debug improvements

[Christoph Egger]

On Wednesday 23 June 2010 15:48:28 Keir Fraser wrote:
> On 23/06/2010 13:49, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > Attached patch moves vmcb dump function into
> > its own svm debug files.
> > Add svm_vmcb_isvalid() to svm code to validate
> > the vmcb.
>
> I won't take patches to add things that aren't used. Unless they are used
> later in the same patch series,

Yes, that is the case. It will be used in the nested virtualization patch 
series. Do you prefer me to resend it as part of the patch series?

> all posted with intention to be applied at 
> the same time. Otherwise the function is dead code, and I love to delete
> dead code.

Me, too.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] svm: debug improvements

[Keir Fraser]

On 24/06/2010 11:33, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> I won't take patches to add things that aren't used. Unless they are used
>> later in the same patch series,
> 
> Yes, that is the case. It will be used in the nested virtualization patch
> series. Do you prefer me to resend it as part of the patch series?

Yes indeed.

 K.