[PATCH] nestedhvm: xentrace support

[PATCH] nestedhvm: xentrace support

[Christoph Egger]

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


Tracing facility for nested virtualization

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

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

[-- Attachment #2: xen_nh_tracing.diff --]
[-- Type: text/plain, Size: 4230 bytes --]

Tracing facility for nested virtualization

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

diff -r 2b16ca86c211 -r 3635528a091b tools/xentrace/formats
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -35,9 +35,12 @@ 0x0002800d  CPU%(cpu)d  %(tsc)d (+%(relt
 0x0002800e  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  switch_infprev    [ old_domid = 0x%(1)08x, runtime = %(2)d ]
 0x0002800f  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  switch_infnext    [ new_domid = 0x%(1)08x, time = %(2)d, r_time = %(3)d ]
 
-0x00081001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMENTRY
+0x00081001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMENTRY     [ rIP = 0x%(1)08x ]
 0x00081002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMEXIT      [ exitcode = 0x%(1)08x, rIP  = 0x%(2)08x ]
 0x00081102  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMEXIT      [ exitcode = 0x%(1)08x, rIP  = 0x%(3)08x%(2)08x ]
+0x00081401  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  nVMENTRY    [ rIP = 0x%(1)08x ]
+0x00081402  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  nVMEXIT     [ exitcode = 0x%(1)08x, rIP  = 0x%(2)08x ]
+0x00081502  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  nVMEXIT     [ exitcode = 0x%(1)08x, rIP  = 0x%(3)08x%(2)08x ]
 0x00082001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  PF_XEN      [ errorcode = 0x%(2)02x, virt = 0x%(1)08x ]
 0x00082101  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  PF_XEN      [ errorcode = 0x%(3)02x, virt = 0x%(2)08x%(1)08x ]
 0x00082002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  PF_INJECT   [ errorcode = 0x%(1)02x, virt = 0x%(2)08x ]
diff -r 2b16ca86c211 -r 3635528a091b xen/arch/x86/hvm/svm/entry.S
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -178,7 +178,7 @@ svm_stgi_label:
         jmp  svm_asm_do_resume
 
 .Lsvm_trace:
-        call svm_trace_vmentry
+        call_with_regs(svm_trace_vmentry)
         jmp  .Lsvm_trace_done
 
 .Lsvm_nsvm_no_p2m:
diff -r 2b16ca86c211 -r 3635528a091b xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1679,6 +1679,20 @@ asmlinkage void svm_vmexit_handler(struc
 
     exit_reason = vmcb->exitcode;
 
+    if ( tb_init_done ) {
+        if ( hvm_long_mode_enabled(v) ) {
+            HVMTRACE_ND(VMEXIT64 | (TRC_HVM_NESTEDFLAG * vcpu_guestmode),
+                    1/*cycles*/, 3, exit_reason,
+                    (uint32_t)regs->eip, (uint32_t)((uint64_t)regs->eip >> 32),
+                    0, 0, 0);
+        } else {
+            HVMTRACE_ND(VMEXIT | (TRC_HVM_NESTEDFLAG * vcpu_guestmode),
+                    1/*cycles*/, 2, exit_reason,
+                    (uint32_t)regs->eip,
+                    0, 0, 0, 0);
+        }
+    }
+
     if ( vcpu_guestmode ) {
         enum nestedhvm_vmexits nsret;
         struct nestedvcpu *nv = &vcpu_nestedhvm(v);
@@ -1739,15 +1753,6 @@ asmlinkage void svm_vmexit_handler(struc
         }
     }
 
-    if ( hvm_long_mode_enabled(v) )
-        HVMTRACE_ND(VMEXIT64, 1/*cycles*/, 3, exit_reason,
-                    (uint32_t)regs->eip, (uint32_t)((uint64_t)regs->eip >> 32),
-                    0, 0, 0);
-    else
-        HVMTRACE_ND(VMEXIT, 1/*cycles*/, 2, exit_reason,
-                    (uint32_t)regs->eip, 
-                    0, 0, 0, 0);
-
     if ( unlikely(exit_reason == VMEXIT_INVALID) )
     {
         svm_vmcb_dump(__func__, vmcb);
@@ -2025,9 +2030,12 @@ asmlinkage void svm_vmexit_handler(struc
     vmcb_set_vintr(vmcb, intr);
 }
 
-asmlinkage void svm_trace_vmentry(void)
+asmlinkage void svm_trace_vmentry(struct cpu_user_regs *regs)
 {
-    HVMTRACE_ND (VMENTRY, 1/*cycles*/, 0, 0, 0, 0, 0, 0, 0);
+    struct vcpu *v = current;
+    HVMTRACE_ND(VMENTRY | (TRC_HVM_NESTEDFLAG * !!(nestedhvm_vcpu_in_guestmode(v))),
+               1/*cycles*/, 1, regs->rip,
+               0, 0, 0, 0, 0);
 }
   
 /*
diff -r 2b16ca86c211 -r 3635528a091b xen/include/public/trace.h
--- a/xen/include/public/trace.h
+++ b/xen/include/public/trace.h
@@ -123,6 +123,7 @@
 #define TRC_SHADOW_RESYNC_ONLY                (TRC_SHADOW + 15)
 
 /* trace events per subclass */
+#define TRC_HVM_NESTEDFLAG      (0x400)
 #define TRC_HVM_VMENTRY         (TRC_HVM_ENTRYEXIT + 0x01)
 #define TRC_HVM_VMEXIT          (TRC_HVM_ENTRYEXIT + 0x02)
 #define TRC_HVM_VMEXIT64        (TRC_HVM_ENTRYEXIT + TRC_64_FLAG + 0x02)

[-- 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] nestedhvm: xentrace support

[Keir Fraser]

On 13/04/2011 16:34, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 
> Tracing facility for nested virtualization
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

I fixed this, by adding an extra argument to HVMTRACE_ND() -- it wasn't
really valid to make the first argument be an arbitarry C expression, due to
argument concatenation that then went on in the macro body.

I also discarded the change to the VMENTRY record to add the guest EIP.
Firstly, it was incomplete, because it did not handle 64-bit RIP. Secondly,
it does not logically belong in this patch. Finally, George may have a
reason for not including RIP in VMENTRY records (e.g., not that useful
compared with the extra space taken up in trace records). So you'd need to
submit a new patch and get an Ack from George (who is currently on holiday).

 -- Keir

Re: [PATCH] nestedhvm: xentrace support

[Christoph Egger]

On 04/14/11 16:00, Keir Fraser wrote:
> On 13/04/2011 16:34, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>
>> Tracing facility for nested virtualization
>>
>> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>
>
> I fixed this, by adding an extra argument to HVMTRACE_ND() -- it wasn't
> really valid to make the first argument be an arbitarry C expression, due to
> argument concatenation that then went on in the macro body.

Thanks for your effort.

> I also discarded the change to the VMENTRY record to add the guest EIP.
> Firstly, it was incomplete, because it did not handle 64-bit RIP. Secondly,
> it does not logically belong in this patch. Finally, George may have a
> reason for not including RIP in VMENTRY records (e.g., not that useful
> compared with the extra space taken up in trace records). So you'd need to
> submit a new patch and get an Ack from George (who is currently on holiday).

Ok. If he has one I would like to know it.
For nested virtualization it is at least good debugging information to 
see if the l2 guest loops somewhere or not.

Christoph


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

Re: [PATCH] nestedhvm: xentrace support

[Keir Fraser]

On 14/04/2011 15:36, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> I also discarded the change to the VMENTRY record to add the guest EIP.
>> Firstly, it was incomplete, because it did not handle 64-bit RIP. Secondly,
>> it does not logically belong in this patch. Finally, George may have a
>> reason for not including RIP in VMENTRY records (e.g., not that useful
>> compared with the extra space taken up in trace records). So you'd need to
>> submit a new patch and get an Ack from George (who is currently on holiday).
> 
> Ok. If he has one I would like to know it.
> For nested virtualization it is at least good debugging information to
> see if the l2 guest loops somewhere or not.

He might at least agree to it in the L2 case only. Anyhow, that's his call
not mine.

 -- Keir

Re: [PATCH] nestedhvm: xentrace support

[George Dunlap]

On Thu, Apr 14, 2011 at 3:36 PM, Christoph Egger
<Christoph.Egger@amd.com> wrote:
>> Finally, George may have a
>> reason for not including RIP in VMENTRY records (e.g., not that useful
>> compared with the extra space taken up in trace records). So you'd need to
>> submit a new patch and get an Ack from George (who is currently on
>> holiday).

Yeah, the general principle is just to not log duplicate information,
or information which can be inferred.  VMENTRY logs happen a *lot*,
and so a small change has a non-negligible impact on trace size.  To
this point, I haven't needed the RIP on VMENTRY, since having it on
VMEXIT is generally sufficient.

> Ok. If he has one I would like to know it.
> For nested virtualization it is at least good debugging information to see
> if the l2 guest loops somewhere or not.

Can't you get this information from looking at the RIP on the
corresponding VMEXIT?

 -George