# HG changeset patch
# User kaf24@xxxxxxxxxxxxxxxxxxxx
# Node ID 26eff2448966a9e25f6776fe56a88a0acddc35c2
# Parent f614ea56143c469bd0e72c77d658b20bc818168c
libxc: xc_ptrace cleanups
General Cleanups
* Use { after if consistently in xc_ptrace.c and xc_ptrace_core.c
(But not in xc_ptrace_core() which should be removed shortly)
* Remove duplicate code and centralise around xc_ptrace.h
* Avoid ifing values covered by case in xc_ptrace()
- PTRACE_GETREGS, PTRACE_GETFPREGS and PTRACE_GETFPXREGS are grouped into
a single case, and then with the exception of a call to FETCH_REGS(),
different code is executed based on ifing the values covered by the
case. The PTRACE_GETFPREGS and PTRACE_GETFPXREGS code is actually a
duplicate. This patch breaks the code out to two different cases.
Error Handling
* Eliminate FETCH_REGS macro as it forces several functions
to have an otherwise uneeded error_out label, mittigating
any code savins.
* Rework error handling in xc_ptrace().
- Remove FETCH_REGS as above
- Make sure that all dom0 errors are caught
- Make sure errno is always set on error
* Eliminate gotos in xc_ptrace_core.c that do nothing but return
Signed-Off-By: Horms <horms@xxxxxxxxxxxx>
diff -r f614ea56143c -r 26eff2448966 tools/libxc/xc_ptrace.c
--- a/tools/libxc/xc_ptrace.c Mon Mar 6 10:04:37 2006
+++ b/tools/libxc/xc_ptrace.c Mon Mar 6 11:05:44 2006
@@ -7,9 +7,35 @@
#include "xc_private.h"
#include "xg_private.h"
-#include <thread_db.h>
#include "xc_ptrace.h"
+const char const * ptrace_names[] = {
+ "PTRACE_TRACEME",
+ "PTRACE_PEEKTEXT",
+ "PTRACE_PEEKDATA",
+ "PTRACE_PEEKUSER",
+ "PTRACE_POKETEXT",
+ "PTRACE_POKEDATA",
+ "PTRACE_POKEUSER",
+ "PTRACE_CONT",
+ "PTRACE_KILL",
+ "PTRACE_SINGLESTEP",
+ "PTRACE_INVALID",
+ "PTRACE_INVALID",
+ "PTRACE_GETREGS",
+ "PTRACE_SETREGS",
+ "PTRACE_GETFPREGS",
+ "PTRACE_SETFPREGS",
+ "PTRACE_ATTACH",
+ "PTRACE_DETACH",
+ "PTRACE_GETFPXREGS",
+ "PTRACE_SETFPXREGS",
+ "PTRACE_INVALID",
+ "PTRACE_INVALID",
+ "PTRACE_INVALID",
+ "PTRACE_INVALID",
+ "PTRACE_SYSCALL",
+};
/* XXX application state */
static long nr_pages = 0;
@@ -32,7 +58,8 @@
if (online)
*online = 0;
- if ( !(regs_valid & (1 << cpu)) ) {
+ if ( !(regs_valid & (1 << cpu)) )
+ {
retval = xc_vcpu_getcontext(xc_handle, current_domid,
cpu, &ctxt[cpu]);
if ( retval )
@@ -50,9 +77,6 @@
return retval;
}
-#define FETCH_REGS(cpu) if (fetch_regs(xc_handle, cpu, NULL)) goto error_out;
-
-
static struct thr_ev_handlers {
thr_ev_handler_t td_create;
thr_ev_handler_t td_death;
@@ -95,14 +119,12 @@
*cpumap = 0;
for (i = 0; i <= d->max_vcpu_id; i++) {
if ((retval = fetch_regs(xc_handle, i, &online)))
- goto error_out;
+ return retval;
if (online)
*cpumap |= (1 << i);
}
return 0;
- error_out:
- return retval;
}
/*
@@ -118,7 +140,8 @@
int index;
while ( (index = ffsll(changed_cpumap)) ) {
- if ( cpumap & (1 << (index - 1)) ) {
+ if ( cpumap & (1 << (index - 1)) )
+ {
if (handlers.td_create) handlers.td_create(index - 1);
} else {
printf("thread death: %d\n", index - 1);
@@ -143,34 +166,32 @@
uint64_t *l3, *l2, *l1;
static void *v;
- FETCH_REGS(cpu);
+ if (fetch_regs(xc_handle, cpu, NULL))
+ return NULL;
l3 = xc_map_foreign_range(
xc_handle, current_domid, PAGE_SIZE, PROT_READ, ctxt[cpu].ctrlreg[3]
>> PAGE_SHIFT);
if ( l3 == NULL )
- goto error_out;
+ return NULL;
l2p = l3[l3_table_offset_pae(va)] >> PAGE_SHIFT;
l2 = xc_map_foreign_range(xc_handle, current_domid, PAGE_SIZE, PROT_READ,
l2p);
if ( l2 == NULL )
- goto error_out;
+ return NULL;
l1p = l2[l2_table_offset_pae(va)] >> PAGE_SHIFT;
l1 = xc_map_foreign_range(xc_handle, current_domid, PAGE_SIZE, perm, l1p);
if ( l1 == NULL )
- goto error_out;
+ return NULL;
p = l1[l1_table_offset_pae(va)] >> PAGE_SHIFT;
if ( v != NULL )
munmap(v, PAGE_SIZE);
v = xc_map_foreign_range(xc_handle, current_domid, PAGE_SIZE, perm, p);
if ( v == NULL )
- goto error_out;
+ return NULL;
return (void *)((unsigned long)v | (va & (PAGE_SIZE - 1)));
-
- error_out:
- return NULL;
}
static void *
@@ -215,17 +236,18 @@
if ( (page_array = malloc(nr_pages * sizeof(unsigned long))) == NULL )
{
printf("Could not allocate memory\n");
- goto error_out;
+ return NULL;
}
if ( xc_get_pfn_list(xc_handle, current_domid,
page_array, nr_pages) != nr_pages )
{
printf("Could not get the page frame list\n");
- goto error_out;
+ return NULL;
}
}
- FETCH_REGS(cpu);
+ if (fetch_regs(xc_handle, cpu, NULL))
+ return NULL;
if ( ctxt[cpu].ctrlreg[3] != cr3_phys[cpu] )
{
@@ -236,10 +258,10 @@
xc_handle, current_domid, PAGE_SIZE, PROT_READ,
cr3_phys[cpu] >> PAGE_SHIFT);
if ( cr3_virt[cpu] == NULL )
- goto error_out;
+ return NULL;
}
if ( (pde = cr3_virt[cpu][vtopdi(va)]) == 0 )
- goto error_out;
+ return NULL;
if ( (ctxt[cpu].flags & VGCF_HVM_GUEST) && paging_enabled(&ctxt[cpu]) )
pde = page_array[pde >> PAGE_SHIFT] << PAGE_SHIFT;
if ( pde != pde_phys[cpu] )
@@ -251,10 +273,10 @@
xc_handle, current_domid, PAGE_SIZE, PROT_READ,
pde_phys[cpu] >> PAGE_SHIFT);
if ( pde_virt[cpu] == NULL )
- goto error_out;
+ return NULL;
}
if ( (page = pde_virt[cpu][vtopti(va)]) == 0 )
- goto error_out;
+ return NULL;
if ( (ctxt[cpu].flags & VGCF_HVM_GUEST) && paging_enabled(&ctxt[cpu]) )
page = page_array[page >> PAGE_SHIFT] << PAGE_SHIFT;
if ( (page != page_phys[cpu]) || (perm != prev_perm[cpu]) )
@@ -268,15 +290,12 @@
if ( page_virt[cpu] == NULL )
{
page_phys[cpu] = 0;
- goto error_out;
+ return NULL;
}
prev_perm[cpu] = perm;
}
return (void *)(((unsigned long)page_virt[cpu]) | (va & BSD_PAGE_MASK));
-
- error_out:
- return NULL;
}
int
@@ -335,7 +354,6 @@
long edata)
{
DECLARE_DOM0_OP;
- int status = 0;
struct gdb_regs pt;
long retval = 0;
unsigned long *guest_va;
@@ -353,10 +371,7 @@
guest_va = (unsigned long *)map_domain_va(
xc_handle, cpu, addr, PROT_READ);
if ( guest_va == NULL )
- {
- status = EFAULT;
- goto error_out;
- }
+ goto out_error;
retval = *guest_va;
break;
@@ -365,38 +380,30 @@
/* XXX assume that all CPUs have the same address space */
guest_va = (unsigned long *)map_domain_va(
xc_handle, cpu, addr, PROT_READ|PROT_WRITE);
- if ( guest_va == NULL ) {
- status = EFAULT;
- goto error_out;
- }
+ if ( guest_va == NULL )
+ goto out_error;
*guest_va = (unsigned long)data;
break;
case PTRACE_GETREGS:
+ if (fetch_regs(xc_handle, cpu, NULL))
+ goto out_error;
+ SET_PT_REGS(pt, ctxt[cpu].user_regs);
+ memcpy(data, &pt, sizeof(struct gdb_regs));
+ break;
+
case PTRACE_GETFPREGS:
case PTRACE_GETFPXREGS:
-
- FETCH_REGS(cpu);
- if ( request == PTRACE_GETREGS )
- {
- SET_PT_REGS(pt, ctxt[cpu].user_regs);
- memcpy(data, &pt, sizeof(struct gdb_regs));
- }
- else if (request == PTRACE_GETFPREGS)
- {
- memcpy(data, &ctxt[cpu].fpu_ctxt, sizeof(ctxt[cpu].fpu_ctxt));
- }
- else /*if (request == PTRACE_GETFPXREGS)*/
- {
- memcpy(data, &ctxt[cpu].fpu_ctxt, sizeof(ctxt[cpu].fpu_ctxt));
- }
+ if (fetch_regs(xc_handle, cpu, NULL))
+ goto out_error;
+ memcpy(data, &ctxt[cpu].fpu_ctxt, sizeof(ctxt[cpu].fpu_ctxt));
break;
case PTRACE_SETREGS:
SET_XC_REGS(((struct gdb_regs *)data), ctxt[cpu].user_regs);
- retval = xc_vcpu_setcontext(xc_handle, current_domid, cpu, &ctxt[cpu]);
- if (retval)
- goto error_out;
+ if ((retval = xc_vcpu_setcontext(xc_handle, current_domid, cpu,
+ &ctxt[cpu])))
+ goto out_error_dom0;
break;
case PTRACE_SINGLESTEP:
@@ -404,12 +411,9 @@
* during single-stepping - but that just seems retarded
*/
ctxt[cpu].user_regs.eflags |= PSL_T;
- retval = xc_vcpu_setcontext(xc_handle, current_domid, cpu, &ctxt[cpu]);
- if ( retval )
- {
- perror("dom0 op failed");
- goto error_out;
- }
+ if ((retval = xc_vcpu_setcontext(xc_handle, current_domid, cpu,
+ &ctxt[cpu])))
+ goto out_error_dom0;
/* FALLTHROUGH */
case PTRACE_CONT:
@@ -418,16 +422,15 @@
{
FOREACH_CPU(cpumap, index) {
cpu = index - 1;
- FETCH_REGS(cpu);
+ if (fetch_regs(xc_handle, cpu, NULL))
+ goto out_error;
/* Clear trace flag */
- if ( ctxt[cpu].user_regs.eflags & PSL_T ) {
+ if ( ctxt[cpu].user_regs.eflags & PSL_T )
+ {
ctxt[cpu].user_regs.eflags &= ~PSL_T;
- retval = xc_vcpu_setcontext(xc_handle, current_domid,
- cpu, &ctxt[cpu]);
- if ( retval ) {
- perror("dom0 op failed");
- goto error_out;
- }
+ if ((retval = xc_vcpu_setcontext(xc_handle, current_domid,
+ cpu, &ctxt[cpu])))
+ goto out_error_dom0;
}
}
}
@@ -436,10 +439,13 @@
op.cmd = DOM0_SETDEBUGGING;
op.u.setdebugging.domain = current_domid;
op.u.setdebugging.enable = 0;
- retval = do_dom0_op(xc_handle, &op);
+ if ((retval = do_dom0_op(xc_handle, &op)))
+ goto out_error_dom0;
}
regs_valid = 0;
- xc_domain_unpause(xc_handle, current_domid > 0 ? current_domid :
-current_domid);
+ if ((retval = xc_domain_unpause(xc_handle, current_domid > 0 ?
+ current_domid : -current_domid)))
+ goto out_error_dom0;
break;
case PTRACE_ATTACH:
@@ -448,19 +454,16 @@
op.u.getdomaininfo.domain = current_domid;
retval = do_dom0_op(xc_handle, &op);
if ( retval || (op.u.getdomaininfo.domain != current_domid) )
- {
- perror("dom0 op failed");
- goto error_out;
- }
+ goto out_error_dom0;
if ( op.u.getdomaininfo.flags & DOMFLAGS_PAUSED )
- {
printf("domain currently paused\n");
- } else
- retval = xc_domain_pause(xc_handle, current_domid);
+ else if ((retval = xc_domain_pause(xc_handle, current_domid)))
+ goto out_error_dom0;
op.cmd = DOM0_SETDEBUGGING;
op.u.setdebugging.domain = current_domid;
op.u.setdebugging.enable = 1;
- retval = do_dom0_op(xc_handle, &op);
+ if ((retval = do_dom0_op(xc_handle, &op)))
+ goto out_error_dom0;
if (get_online_cpumap(xc_handle, &op.u.getdomaininfo, &cpumap))
printf("get_online_cpumap failed\n");
@@ -478,21 +481,20 @@
printf("unsupported xc_ptrace request %s\n", ptrace_names[request]);
#endif
/* XXX not yet supported */
- status = ENOSYS;
- break;
+ errno = ENOSYS;
+ return -1;
case PTRACE_TRACEME:
printf("PTRACE_TRACEME is an invalid request under Xen\n");
- status = EINVAL;
- }
-
- if ( status )
- {
- errno = status;
- retval = -1;
- }
-
- error_out:
+ goto out_error;
+ }
+
+ return retval;
+
+ out_error_dom0:
+ perror("dom0 op failed");
+ out_error:
+ errno = EINVAL;
return retval;
}
diff -r f614ea56143c -r 26eff2448966 tools/libxc/xc_ptrace.h
--- a/tools/libxc/xc_ptrace.h Mon Mar 6 10:04:37 2006
+++ b/tools/libxc/xc_ptrace.h Mon Mar 6 11:05:44 2006
@@ -1,5 +1,7 @@
#ifndef XC_PTRACE_
#define XC_PTRACE_
+
+#include <thread_db.h>
#ifdef XC_PTRACE_PRIVATE
#define X86_CR0_PE 0x00000001 /* Enable Protected Mode (RW) */
@@ -8,33 +10,7 @@
#define PDRSHIFT 22
#define PSL_T 0x00000100 /* trace enable bit */
-char * ptrace_names[] = {
- "PTRACE_TRACEME",
- "PTRACE_PEEKTEXT",
- "PTRACE_PEEKDATA",
- "PTRACE_PEEKUSER",
- "PTRACE_POKETEXT",
- "PTRACE_POKEDATA",
- "PTRACE_POKEUSER",
- "PTRACE_CONT",
- "PTRACE_KILL",
- "PTRACE_SINGLESTEP",
- "PTRACE_INVALID",
- "PTRACE_INVALID",
- "PTRACE_GETREGS",
- "PTRACE_SETREGS",
- "PTRACE_GETFPREGS",
- "PTRACE_SETFPREGS",
- "PTRACE_ATTACH",
- "PTRACE_DETACH",
- "PTRACE_GETFPXREGS",
- "PTRACE_SETFPXREGS",
- "PTRACE_INVALID",
- "PTRACE_INVALID",
- "PTRACE_INVALID",
- "PTRACE_INVALID",
- "PTRACE_SYSCALL",
-};
+extern const char const * ptrace_names[];
struct gdb_regs {
long ebx; /* 0 */
diff -r f614ea56143c -r 26eff2448966 tools/libxc/xc_ptrace_core.c
--- a/tools/libxc/xc_ptrace_core.c Mon Mar 6 10:04:37 2006
+++ b/tools/libxc/xc_ptrace_core.c Mon Mar 6 11:05:44 2006
@@ -1,81 +1,12 @@
+#define XC_PTRACE_PRIVATE
+
#include <sys/ptrace.h>
#include <sys/wait.h>
#include "xc_private.h"
+#include "xc_ptrace.h"
#include <time.h>
-#define BSD_PAGE_MASK (PAGE_SIZE-1)
-#define PDRSHIFT 22
#define VCPU 0 /* XXX */
-
-/*
- * long
- * ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data);
- */
-
-struct gdb_regs {
- long ebx; /* 0 */
- long ecx; /* 4 */
- long edx; /* 8 */
- long esi; /* 12 */
- long edi; /* 16 */
- long ebp; /* 20 */
- long eax; /* 24 */
- int xds; /* 28 */
- int xes; /* 32 */
- int xfs; /* 36 */
- int xgs; /* 40 */
- long orig_eax; /* 44 */
- long eip; /* 48 */
- int xcs; /* 52 */
- long eflags; /* 56 */
- long esp; /* 60 */
- int xss; /* 64 */
-};
-
-#define printval(x) printf("%s = %lx\n", #x, (long)x);
-#define SET_PT_REGS(pt, xc) \
-{ \
- pt.ebx = xc.ebx; \
- pt.ecx = xc.ecx; \
- pt.edx = xc.edx; \
- pt.esi = xc.esi; \
- pt.edi = xc.edi; \
- pt.ebp = xc.ebp; \
- pt.eax = xc.eax; \
- pt.eip = xc.eip; \
- pt.xcs = xc.cs; \
- pt.eflags = xc.eflags; \
- pt.esp = xc.esp; \
- pt.xss = xc.ss; \
- pt.xes = xc.es; \
- pt.xds = xc.ds; \
- pt.xfs = xc.fs; \
- pt.xgs = xc.gs; \
-}
-
-#define SET_XC_REGS(pt, xc) \
-{ \
- xc.ebx = pt->ebx; \
- xc.ecx = pt->ecx; \
- xc.edx = pt->edx; \
- xc.esi = pt->esi; \
- xc.edi = pt->edi; \
- xc.ebp = pt->ebp; \
- xc.eax = pt->eax; \
- xc.eip = pt->eip; \
- xc.cs = pt->xcs; \
- xc.eflags = pt->eflags; \
- xc.esp = pt->esp; \
- xc.ss = pt->xss; \
- xc.es = pt->xes; \
- xc.ds = pt->xds; \
- xc.fs = pt->xfs; \
- xc.gs = pt->xgs; \
-}
-
-
-#define vtopdi(va) ((va) >> PDRSHIFT)
-#define vtopti(va) (((va) >> PAGE_SHIFT) & 0x3ff)
/* XXX application state */
@@ -120,12 +51,12 @@
if (v == MAP_FAILED)
{
perror("mmap failed");
- goto error_out;
+ return NULL;
}
cr3_virt[cpu] = v;
}
if ((pde = cr3_virt[cpu][vtopdi(va)]) == 0) /* logical address */
- goto error_out;
+ return NULL;
if (ctxt[cpu].flags & VGCF_HVM_GUEST)
pde = p2m_array[pde >> PAGE_SHIFT] << PAGE_SHIFT;
if (pde != pde_phys[cpu])
@@ -137,11 +68,11 @@
NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, domfd,
map_mtop_offset(pde_phys[cpu]));
if (v == MAP_FAILED)
- goto error_out;
+ return NULL;
pde_virt[cpu] = v;
}
if ((page = pde_virt[cpu][vtopti(va)]) == 0) /* logical address */
- goto error_out;
+ return NULL;
if (ctxt[cpu].flags & VGCF_HVM_GUEST)
page = p2m_array[page >> PAGE_SHIFT] << PAGE_SHIFT;
if (page != page_phys[cpu])
@@ -152,17 +83,15 @@
v = mmap(
NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, domfd,
map_mtop_offset(page_phys[cpu]));
- if (v == MAP_FAILED) {
+ if (v == MAP_FAILED)
+ {
printf("cr3 %lx pde %lx page %lx pti %lx\n", cr3[cpu], pde, page,
vtopti(va));
page_phys[cpu] = 0;
- goto error_out;
+ return NULL;
}
page_virt[cpu] = v;
}
return (void *)(((unsigned long)page_virt[cpu]) | (va & BSD_PAGE_MASK));
-
- error_out:
- return 0;
}
int
@@ -172,12 +101,12 @@
int *status,
int options)
{
- int retval = -1;
int nr_vcpus;
int i;
xc_core_header_t header;
- if (nr_pages == 0) {
+ if (nr_pages == 0)
+ {
if (read(domfd, &header, sizeof(header)) != sizeof(header))
return -1;
@@ -193,17 +122,19 @@
for (i = 0; i < nr_vcpus; i++) {
cr3[i] = ctxt[i].ctrlreg[3];
}
- if ((p2m_array = malloc(nr_pages * sizeof(unsigned long))) == NULL) {
+ if ((p2m_array = malloc(nr_pages * sizeof(unsigned long))) == NULL)
+ {
printf("Could not allocate p2m_array\n");
- goto error_out;
+ return -1;
}
if (read(domfd, p2m_array, sizeof(unsigned long)*nr_pages) !=
sizeof(unsigned long)*nr_pages)
return -1;
- if ((m2p_array = malloc((1<<20) * sizeof(unsigned long))) == NULL) {
+ if ((m2p_array = malloc((1<<20) * sizeof(unsigned long))) == NULL)
+ {
printf("Could not allocate m2p array\n");
- goto error_out;
+ return -1;
}
bzero(m2p_array, sizeof(unsigned long)* 1 << 20);
@@ -212,10 +143,7 @@
}
}
- retval = 0;
- error_out:
- return retval;
-
+ return 0;
}
long
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home