[PATCH] Make xentrace hypercall a bit more sane

[PATCH] Make xentrace hypercall a bit more sane

[George Dunlap]

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

xentrace, setsize and the hypervisor don't seem to be on the same
page.  If tracing is not enabled, the hypercall will only allow
TBUF_SET_SIZE and TBUF_ENABLE.  However, set_size calls TBUF_GET_INFO,
and xentrace wants to call TBUF_SET_EVT_MASK and TBUF_SET_CPU_MASK
before calling TBUF_ENABLE.

I don't see any reason not to call SET_*_MASK and GET_INFO if tracing
is not enabled.    If the buffer hasn't been allocated yet, GET_INFO
should just return something invalid in the mfn field.  Anyone calling
should check this value.

Comments?
 -George

[-- Attachment #2: xentrace.patch --]
[-- Type: text/x-patch, Size: 1032 bytes --]

diff -r acbdec33f666 xen/common/trace.c
--- a/xen/common/trace.c	Thu Jun 01 16:08:36 2006 -0400
+++ b/xen/common/trace.c	Thu Jun 01 16:13:11 2006 -0400
@@ -178,21 +178,18 @@ int tb_control(dom0_tbufcontrol_t *tbc)
 
     spin_lock(&lock);
 
-    if ( !tb_init_done &&
-         (tbc->op != DOM0_TBUF_SET_SIZE) &&
-         (tbc->op != DOM0_TBUF_ENABLE) )
-    {
-        spin_unlock(&lock);
-        return -EINVAL;
-    }
-
     switch ( tbc->op )
     {
     case DOM0_TBUF_GET_INFO:
         tbc->cpu_mask   = tb_cpu_mask;
         tbc->evt_mask   = tb_event_mask;
-        tbc->buffer_mfn = __pa(t_bufs[0]) >> PAGE_SHIFT;
-        tbc->size       = opt_tbuf_size * PAGE_SIZE;
+        if( opt_tbuf_size ) {
+            tbc->buffer_mfn = __pa(t_bufs[0]) >> PAGE_SHIFT;
+            tbc->size       = opt_tbuf_size * PAGE_SIZE;
+        } else {
+            tbc->buffer_mfn = (unsigned long)-1;
+            tbc->size       = 0;
+        }
         break;
     case DOM0_TBUF_SET_CPU_MASK:
         tb_cpu_mask = tbc->cpu_mask;

[-- 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] Make xentrace hypercall a bit more sane

[Rob Gardner]

George Dunlap wrote:
> xentrace, setsize and the hypervisor don't seem to be on the same
> page.  If tracing is not enabled, the hypercall will only allow
> TBUF_SET_SIZE and TBUF_ENABLE.  However, set_size calls TBUF_GET_INFO,
> and xentrace wants to call TBUF_SET_EVT_MASK and TBUF_SET_CPU_MASK
> before calling TBUF_ENABLE.
>
> I don't see any reason not to call SET_*_MASK and GET_INFO if tracing
> is not enabled.    If the buffer hasn't been allocated yet, GET_INFO
> should just return something invalid in the mfn field.  Anyone calling
> should check this value.
>

Sounds benign enough. I never liked the behavior of disallowing the 
get_info call.

Rob