Skip to contentSkip to conversation

New HVMOPs for PV-on-HVM drivers

PATCH New Tim Deegan 2 participants 6 messages Jul 9, 2010

lore
Share & tools
lore

Tim Deegan

author
#0 lore raw

PATCH 1/1 New HVMOPs for PV-on-HVM drivers

These patches add two HVM operations that can be used by the XCP PV drivers.

author
#1 lore raw

PATCH 1/1 Add a new HVMOP which allows an HVM guest to get the current

Xen absolute system time, so that it can use SCHEDOP_poll in a sensible fashion. HVM PV drivers can't use the normal PV clock because they might have TSC offsets that hey don't know about.

Signed-off-by Tim Deegan <[email protected]>

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1278682007 -3600
# Node ID dc3138d60b40baf38016a9b58f438598580a407f
# Parent a672af698bc387ea92070fc3ef337e8c2fbb716e Add a new HVMOP which allows an HVM guest to get the current Xen absolute system time, so that it can use SCHEDOP_poll in a sensible fashion. HVM PV drivers can't use the normal PV clock because they might have TSC offsets that hey don't know about.

Signed-off-by Tim Deegan <[email protected]>
xen/arch/x86/hvm/hvm.c +9 -0
diff -r a672af698bc3 -r dc3138d60b40 xen/arch/x86/hvm/hvm.c
@@ -3188,6 +3188,15 @@
         break;
     }
 
+    case HVMOP_get_time: {
+        xen_hvm_get_time_t gxt;
+
+        gxt.now = NOW();
+        if ( copy_to_guest(arg, &gxt, 1) )
+            rc = -EFAULT;
+        break;
+    }
+
     default:
     {
         gdprintk(XENLOG_WARNING, "Bad HVM op %ld.\n", op);
xen/include/public/hvm/hvm_op.h +8 -0
diff -r a672af698bc3 -r dc3138d60b40 xen/include/public/hvm/hvm_op.h
@@ -138,6 +138,14 @@
 typedef struct xen_hvm_pagetable_dying xen_hvm_pagetable_dying_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_pagetable_dying_t);
 
+/* Get the current Xen time, in nanoseconds since system boot. */
+#define HVMOP_get_time              10
+struct xen_hvm_get_time {
+    uint64_t now;      /* OUT */
+};
+typedef struct xen_hvm_get_time xen_hvm_get_time_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_time_t);
+
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
 
 #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

author
#2 lore raw

PATCH 1/1 Add a hypercall to allow HVM PV drivers to insert xentrace records

In reply to Tim Deegan
Signed-off-by Tim Deegan <[email protected]>

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1278682148 -3600
# Node ID ed89d064acc184b0c10a60ffb4c67c493dfb3c8b
# Parent dc3138d60b40baf38016a9b58f438598580a407f
Add a hypercall to allow HVM PV drivers to insert xentrace records.

Signed-off-by Tim Deegan <[email protected]>
xen/arch/x86/hvm/hvm.c +16 -0
diff -r dc3138d60b40 -r ed89d064acc1 xen/arch/x86/hvm/hvm.c
@@ -3197,6 +3197,22 @@
         break;
     }
 
+    case HVMOP_xentrace: {
+        xen_hvm_xentrace_t tr;
+
+        if ( copy_from_guest(&tr, arg, 1 ) )
+            return -EFAULT;
+
+        if ( tr.extra_bytes > 28 || tr.event >= 0x1000 )
+            return -EINVAL;
+
+        /* Cycles will be taken at the vmexit and vmenter */
+        trace_var(tr.event | TRC_GUEST, 0 /*!cycles*/,
+                  tr.extra_bytes,
+                  (unsigned char *)tr.extra);
+        break;
+    }
+
     default:
     {
         gdprintk(XENLOG_WARNING, "Bad HVM op %ld.\n", op);
xen/include/public/hvm/hvm_op.h +8 -0
diff -r dc3138d60b40 -r ed89d064acc1 xen/include/public/hvm/hvm_op.h
@@ -146,6 +146,14 @@
 typedef struct xen_hvm_get_time xen_hvm_get_time_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_time_t);
 
+#define HVMOP_xentrace              11
+struct xen_hvm_xentrace {
+    uint16_t event, extra_bytes;
+    uint8_t extra[28];
+};
+typedef struct xen_hvm_xentrace xen_hvm_xentrace_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_xentrace_t);
+
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
 
 #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */
xen/include/public/trace.h +1 -0
diff -r dc3138d60b40 -r ed89d064acc1 xen/include/public/trace.h
@@ -39,6 +39,7 @@
 #define TRC_PV       0x0020f000    /* Xen PV traces            */
 #define TRC_SHADOW   0x0040f000    /* Xen shadow tracing       */
 #define TRC_PM       0x0080f000    /* Xen power management trace */
+#define TRC_GUEST    0x0800f000    /* Guest-generated traces   */
 #define TRC_ALL      0x0ffff000
 #define TRC_HD_TO_EVENT(x) ((x)&0x0fffffff)
 #define TRC_HD_CYCLE_FLAG (1UL<<31)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#3 lore raw

Re: [PATCH 2 of 2] Add a hypercall to allow HVM PV drivers to insert xentrace records

Quoted 2 lines
Quoted 1 line
Quoted 1 line · Tim Deegan

+ if ( tr.extra_bytes > 28 || tr.event >= 0x1000 )

Couldn't you use TRACE_EXTRA_MAX * sizeof(u32) instead of the literal 28?

The literal 0x1000 also doesn't look nice - I'm no sure though whether this really is (1 << TRC_SUBCLS_SHIFT).

Jan

author
#4 lore raw

Re: [PATCH 2 of 2] Add a hypercall to allow HVM PV drivers to insert xentrace records

Quoted 5 lines · Jan Beulich
Quoted 2 lines
Quoted 1 line
Quoted 1 line · Tim Deegan

+ if ( tr.extra_bytes > 28 || tr.event >= 0x1000 )

Couldn't you use TRACE_EXTRA_MAX * sizeof(u32) instead of the literal 28?

Indeed I should.

Quoted 2 lines

The literal 0x1000 also doesn't look nice - I'm no sure though whether this really is (1 << TRC_SUBCLS_SHIFT).

Yes it is (the guest's not allowed to define the class or subclass of the trace entry). How about the attached patch instead?

Cheers,

Tim.

Signature · 3 lines
Tim Deegan <[email protected]>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)
xen/arch/x86/hvm/hvm.c +17 -0
diff -r dc3138d60b40 xen/arch/x86/hvm/hvm.c
@@ -3197,6 +3197,23 @@
         break;
     }
 
+    case HVMOP_xentrace: {
+        xen_hvm_xentrace_t tr;
+
+        if ( copy_from_guest(&tr, arg, 1 ) )
+            return -EFAULT;
+
+        if ( tr.extra_bytes > sizeof(tr.extra)
+             || (tr.event & ~((1u<<TRC_SUBCLS_SHIFT)-1)) )
+            return -EINVAL;
+
+        /* Cycles will be taken at the vmexit and vmenter */
+        trace_var(tr.event | TRC_GUEST, 0 /*!cycles*/,
+                  tr.extra_bytes,
+                  (unsigned char *)tr.extra);
+        break;
+    }
+
     default:
     {
         gdprintk(XENLOG_WARNING, "Bad HVM op %ld.\n", op);
xen/include/public/hvm/hvm_op.h +9 -0
diff -r dc3138d60b40 xen/include/public/hvm/hvm_op.h
@@ -22,6 +22,7 @@
 #define __XEN_PUBLIC_HVM_HVM_OP_H__
 
 #include "../xen.h"
+#include "../trace.h"
 
 /* Get/set subcommands: extra argument == pointer to xen_hvm_param struct. */
 #define HVMOP_set_param           0

@@ -146,6 +147,14 @@
 typedef struct xen_hvm_get_time xen_hvm_get_time_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_time_t);
 
+#define HVMOP_xentrace              11
+struct xen_hvm_xentrace {
+    uint16_t event, extra_bytes;
+    uint8_t extra[TRACE_EXTRA_MAX * sizeof(uint32_t)];
+};
+typedef struct xen_hvm_xentrace xen_hvm_xentrace_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_xentrace_t);
+
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
 
 #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */
xen/include/public/trace.h +1 -0
diff -r dc3138d60b40 xen/include/public/trace.h
@@ -39,6 +39,7 @@
 #define TRC_PV       0x0020f000    /* Xen PV traces            */
 #define TRC_SHADOW   0x0040f000    /* Xen shadow tracing       */
 #define TRC_PM       0x0080f000    /* Xen power management trace */
+#define TRC_GUEST    0x0800f000    /* Guest-generated traces   */
 #define TRC_ALL      0x0ffff000
 #define TRC_HD_TO_EVENT(x) ((x)&0x0fffffff)
 #define TRC_HD_CYCLE_FLAG (1UL<<31)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#5 lore raw

Re: [PATCH 2 of 2] Add a hypercall to allow HVM PV drivers to insert xentrace records

Quoted 8 lines
Quoted 1 line
Quoted 1 line · Tim Deegan
Quoted 5 lines · Jan Beulich
Quoted 2 lines
Quoted 1 line
Quoted 1 line · Tim Deegan

+ if ( tr.extra_bytes > 28 || tr.event >= 0x1000 )

Couldn't you use TRACE_EXTRA_MAX * sizeof(u32) instead of the literal 28?

Quoted 7 lines

Indeed I should.

Quoted 2 lines

The literal 0x1000 also doesn't look nice - I'm no sure though whether this really is (1 << TRC_SUBCLS_SHIFT).

Yes it is (the guest's not allowed to define the class or subclass of the trace entry). How about the attached patch instead?

Great, thanks!

Jan