From: Don Slutz <dslutz@verizon.com>
Change from v1 to v2:
title was: xenctx: Add an option to output more registers.
Processed review comments.
Jan Beulich:
Split 1 change into 12.
Switch to enum for is_kernel_text(), renamed to is_kernel_addr().
Renamed vars like memAddr to mem_addr.
Ian Campbell:
More on is_kernel_text().
Don Slutz (12):
xenctx: Clean up stack trace when hypercall_page not in symbol table
xenctx: Add -2 (--two-pages) option to switch stack size to 8KiB
xenctx: Output ascii version of stack also.
xenctx: Add stack address to dump, switch to 16 bytes per line.
xenctx: Change print_symbol to do the space before.
xenctx: More info on failed to map page.
xenctx: Add stack addr to call trace.
xenctx: Add -d <daddr> option to dump memory at daddr as a stack.
xenctx: Add -m <maddr> option to dump memory at maddr.
xenctx: change is_kernel_text() into is_kernel_addr().
xenctx: Dump registers via hvm info if available.
xenctx: Add optional fCPU.
tools/xentrace/xenctx.c | 628 +++++++++++++++++++++++++++++++++++++++++------
1 files changed, 549 insertions(+), 79 deletions(-)
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 01/12] xenctx: Clean up stack trace when hypercall_page not in symbol table
From: Don Slutz <Don@CloudSwitch.com>
Before:
Call Trace:
[<ffffffff8006b2b0>] default_idle+0x29 <--
[<ffffffff80048d19>] cpu_idle+0x95
[<ffffffff803e7801>] start_kernel+0x220
[<0000000000000000>] startup_64+0x80000000
[<ffffffff803e722f>] x86_64_start_kernel+0x22f
[<0000000000000000>] startup_64+0x80000000
[<0000000000000000>] startup_64+0x80000000
[<0000000000000000>] startup_64+0x80000000
[<0000000000000000>] startup_64+0x80000000
After:
Call Trace:
[<ffffffff8006b2b0>] default_idle+0x29 <--
[<ffffffff80048d19>] cpu_idle+0x95
[<ffffffff803e7801>] start_kernel+0x220
[<ffffffff803e722f>] x86_64_start_kernel+0x22f
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 060e480..10292fa 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -83,8 +83,9 @@ static int is_kernel_text(guest_word_t addr)
if (addr >= kernel_stext &&
addr <= kernel_etext)
return 1;
- if (addr >= kernel_hypercallpage &&
- addr <= kernel_hypercallpage + 4096)
+ if (kernel_hypercallpage &&
+ (addr >= kernel_hypercallpage &&
+ addr <= kernel_hypercallpage + 4096))
return 1;
if (addr >= kernel_sinittext &&
addr <= kernel_einittext)
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 02/12] xenctx: Add -2 (--two-pages) option to switch stack size to 8KiB
From: Don Slutz <Don@CloudSwitch.com>
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 10292fa..06a8850 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -35,6 +35,7 @@ static struct xenctx {
int frame_ptrs;
int stack_trace;
int disp_all;
+ int two_pages;
int all_vcpus;
int self_paused;
xc_dominfo_t dominfo;
@@ -650,9 +651,11 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE)
& ~((guest_word_t) XC_PAGE_SIZE - 1));
+ if (xenctx.two_pages)
+ stack_limit += XC_PAGE_SIZE;
printf("\n");
printf("Stack:\n");
- for (i=1; i<5 && stack < stack_limit; i++) {
+ for (i=1; i<10 && stack < stack_limit; i++) {
while(stack < stack_limit && stack < stack_pointer(ctx) +
i*32) {
p = map_page(ctx, vcpu, stack);
if (!p)
@@ -821,6 +824,7 @@ static void usage(void)
printf(" -k, --kernel-start\n");
printf(" set user/kernel split. (default
0xc0000000)\n");
printf(" -a --all display more registers\n");
+ printf(" -2, --two-pages assume the kernel was compiled with 8KiB
stacks.\n");
printf(" -C --all-vcpus print info for all vcpus\n");
}
@@ -828,12 +832,13 @@ int main(int argc, char **argv)
{
int ch;
int ret;
- static const char *sopts = "fs:hak:SC";
+ static const char *sopts = "fs:hak:SC2";
static const struct option lopts[] = {
{"stack-trace", 0, NULL, ''S''},
{"symbol-table", 1, NULL, ''s''},
{"frame-pointers", 0, NULL, ''f''},
{"kernel-start", 1, NULL, ''k''},
+ {"two-pages", 0, NULL, ''2''},
{"all", 0, NULL, ''a''},
{"all-vcpus", 0, NULL, ''C''},
{"help", 0, NULL, ''h''},
@@ -857,6 +862,9 @@ int main(int argc, char **argv)
case ''a'':
xenctx.disp_all = 1;
break;
+ case ''2'':
+ xenctx.two_pages = 1;
+ break;
case ''C'':
xenctx.all_vcpus = 1;
break;
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 03/12] xenctx: Output ascii version of stack also.
From: Don Slutz <Don@CloudSwitch.com>
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 27 ++++++++++++++++++++++++++-
1 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 06a8850..dabce16 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -640,6 +640,8 @@ static int print_code(vcpu_guest_context_any_t *ctx, int
vcpu)
return 0;
}
+#define BYTES_PER_LINE 32
+
static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
{
guest_word_t stack = stack_pointer(ctx);
@@ -647,6 +649,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
guest_word_t frame;
guest_word_t word;
guest_word_t *p;
+ guest_word_t ascii[BYTES_PER_LINE/4];
+ unsigned char *bytep;
int i;
stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE)
@@ -656,15 +660,36 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
printf("\n");
printf("Stack:\n");
for (i=1; i<10 && stack < stack_limit; i++) {
- while(stack < stack_limit && stack < stack_pointer(ctx) +
i*32) {
+ int j = 0;
+ int k;
+
+ while(stack < stack_limit && stack < stack_pointer(ctx) +
i*BYTES_PER_LINE) {
p = map_page(ctx, vcpu, stack);
if (!p)
return -1;
word = read_stack_word(p, width);
+ ascii[j++] = word;
printf(" ");
print_stack_word(word, width);
stack += width;
}
+ printf(" ");
+ for (k = j; k < BYTES_PER_LINE/width; k++)
+ printf("%s ", width == 8
+ ? " "
+ : " ");
+ for (k = 0; k < j; k++) {
+ int l;
+
+ bytep = (unsigned char*)&ascii[k];
+ for (l = 0; l < width; l++) {
+ if ((*bytep < 127) && (*bytep >= 32))
+ printf("%c", *bytep);
+ else
+ printf(".");
+ bytep++;
+ }
+ }
printf("\n");
}
printf("\n");
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 04/12] xenctx: Add stack address to dump, switch to 16 bytes per line.
From: Don Slutz <Don@CloudSwitch.com>
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index dabce16..a20281e 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -640,7 +640,7 @@ static int print_code(vcpu_guest_context_any_t *ctx, int
vcpu)
return 0;
}
-#define BYTES_PER_LINE 32
+#define BYTES_PER_LINE 16
static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
{
@@ -663,6 +663,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
int j = 0;
int k;
+ print_stack_word(stack, width);
+ printf(":");
while(stack < stack_limit && stack < stack_pointer(ctx) +
i*BYTES_PER_LINE) {
p = map_page(ctx, vcpu, stack);
if (!p)
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 05/12] xenctx: Change print_symbol to do the space before.
From: Don Slutz <Don@CloudSwitch.com>
This stops the output of an extra space at the end of the line.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index a20281e..5e2354c 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -158,9 +158,9 @@ static void print_symbol(guest_word_t addr)
return;
if (addr==s->address)
- printf("%s ", s->name);
+ printf(" %s", s->name);
else
- printf("%s+%#x ", s->name, (unsigned int)(addr -
s->address));
+ printf(" %s+%#x", s->name, (unsigned int)(addr -
s->address));
}
static void read_symbol_table(const char *symtab)
@@ -287,7 +287,7 @@ static void print_ctx_32(vcpu_guest_context_x86_32_t *ctx)
{
struct cpu_user_regs_x86_32 *regs = &ctx->user_regs;
- printf("cs:eip: %04x:%08x ", regs->cs, regs->eip);
+ printf("cs:eip: %04x:%08x", regs->cs, regs->eip);
print_symbol(regs->eip);
print_flags(regs->eflags);
printf("ss:esp: %04x:%08x\n", regs->ss, regs->esp);
@@ -316,7 +316,7 @@ static void print_ctx_32on64(vcpu_guest_context_x86_64_t
*ctx)
{
struct cpu_user_regs_x86_64 *regs = &ctx->user_regs;
- printf("cs:eip: %04x:%08x ", regs->cs,
(uint32_t)regs->eip);
+ printf("cs:eip: %04x:%08x", regs->cs, (uint32_t)regs->eip);
print_symbol((uint32_t)regs->eip);
print_flags((uint32_t)regs->eflags);
printf("ss:esp: %04x:%08x\n", regs->ss,
(uint32_t)regs->esp);
@@ -345,7 +345,7 @@ static void print_ctx_64(vcpu_guest_context_x86_64_t *ctx)
{
struct cpu_user_regs_x86_64 *regs = &ctx->user_regs;
- printf("rip: %016"PRIx64" ", regs->rip);
+ printf("rip: %016"PRIx64, regs->rip);
print_symbol(regs->rip);
print_flags(regs->rflags);
printf("rsp: %016"PRIx64"\n", regs->rsp);
@@ -443,7 +443,7 @@ static void print_ctx_32(vcpu_guest_context_t *ctx)
{
vcpu_guest_core_regs_t *regs = &ctx->user_regs;
- printf("PC: %08"PRIx32" ", regs->pc32);
+ printf("PC: %08"PRIx32, regs->pc32);
print_symbol(regs->pc32);
printf("\n");
printf("CPSR: %08"PRIx32"\n", regs->cpsr);
@@ -495,7 +495,7 @@ static void print_ctx_64(vcpu_guest_context_t *ctx)
{
vcpu_guest_core_regs_t *regs = &ctx->user_regs;
- printf("PC: %016"PRIx64" ", regs->pc64);
+ printf("PC: %016"PRIx64, regs->pc64);
print_symbol(regs->pc64);
printf("\n");
@@ -702,7 +702,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
printf("Call Trace:\n");
printf("%c [<", xenctx.stack_trace ? ''*'' :
'' '');
print_stack_word(instr_pointer(ctx), width);
- printf(">] ");
+ printf(">]");
print_symbol(instr_pointer(ctx));
printf(" <--\n");
@@ -742,7 +742,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
word = read_stack_word(p, width);
printf("%c [<", xenctx.stack_trace ?
''|'' : '' '');
print_stack_word(word, width);
- printf(">] ");
+ printf(">]");
print_symbol(word);
printf("\n");
stack += width;
@@ -758,7 +758,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
if (is_kernel_text(word)) {
printf(" [<");
print_stack_word(word, width);
- printf(">] ");
+ printf(">]");
print_symbol(word);
printf("\n");
} else if (xenctx.stack_trace) {
--
1.7.1
From: Don Slutz <Don@CloudSwitch.com>
Also output an extra new line since we may be in the middle of output.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 5e2354c..dcf431c 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -593,7 +593,7 @@ static void *map_page(vcpu_guest_context_any_t *ctx, int
vcpu, guest_word_t virt
mapped = xc_map_foreign_range(xenctx.xc_handle, xenctx.domid, XC_PAGE_SIZE,
PROT_READ, mfn);
if (mapped == NULL) {
- fprintf(stderr, "failed to map page.\n");
+ fprintf(stderr, "\nfailed to map page for
"FMT_32B_WORD".\n", virt);
return NULL;
}
--
1.7.1
From: Don Slutz <Don@CloudSwitch.com>
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 20 ++++++++++++++------
1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index dcf431c..4dc6574 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -700,6 +700,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
printf("Stack Trace:\n");
else
printf("Call Trace:\n");
+ printf("%s ", width == 8
+ ? " "
+ : " ");
printf("%c [<", xenctx.stack_trace ? ''*'' :
'' '');
print_stack_word(instr_pointer(ctx), width);
printf(">]");
@@ -715,9 +718,10 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
p = map_page(ctx, vcpu, stack);
if (!p)
return -1;
- printf("| ");
+ print_stack_word(stack, width);
+ printf(": | ");
print_stack_word(read_stack_word(p, width), width);
- printf(" \n");
+ printf("\n");
stack += width;
}
} else {
@@ -729,7 +733,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
return -1;
frame = read_stack_word(p, width);
if (xenctx.stack_trace) {
- printf("|-- ");
+ print_stack_word(stack, width);
+ printf(": |-- ");
print_stack_word(read_stack_word(p, width), width);
printf("\n");
}
@@ -740,7 +745,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
if (!p)
return -1;
word = read_stack_word(p, width);
- printf("%c [<", xenctx.stack_trace ?
''|'' : '' '');
+ print_stack_word(stack, width);
+ printf(": %c [<", xenctx.stack_trace ?
''|'' : '' '');
print_stack_word(word, width);
printf(">]");
print_symbol(word);
@@ -756,13 +762,15 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
return -1;
word = read_stack_word(p, width);
if (is_kernel_text(word)) {
- printf(" [<");
+ print_stack_word(stack, width);
+ printf(": [<");
print_stack_word(word, width);
printf(">]");
print_symbol(word);
printf("\n");
} else if (xenctx.stack_trace) {
- printf(" ");
+ print_stack_word(stack, width);
+ printf(": ");
print_stack_word(word, width);
printf("\n");
}
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 08/12] xenctx: Add -d <daddr> option to dump memory at daddr as a stack.
From: Don Slutz <Don@CloudSwitch.com>
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 85 ++++++++++++++++++++++++++++-------------------
1 files changed, 51 insertions(+), 34 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 4dc6574..233f537 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -642,18 +642,21 @@ static int print_code(vcpu_guest_context_any_t *ctx, int
vcpu)
#define BYTES_PER_LINE 16
-static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
+static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width,
guest_word_t stk_addr)
{
- guest_word_t stack = stack_pointer(ctx);
+ guest_word_t stk_addr_start = stack_pointer(ctx);
+ guest_word_t stack;
guest_word_t stack_limit;
guest_word_t frame;
guest_word_t word;
- guest_word_t *p;
guest_word_t ascii[BYTES_PER_LINE/4];
unsigned char *bytep;
int i;
- stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE)
+ if (stk_addr && !xenctx.frame_ptrs)
+ stk_addr_start = stk_addr;
+ stack = stk_addr_start;
+ stack_limit = ((stk_addr_start + XC_PAGE_SIZE)
& ~((guest_word_t) XC_PAGE_SIZE - 1));
if (xenctx.two_pages)
stack_limit += XC_PAGE_SIZE;
@@ -665,8 +668,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
print_stack_word(stack, width);
printf(":");
- while(stack < stack_limit && stack < stack_pointer(ctx) +
i*BYTES_PER_LINE) {
- p = map_page(ctx, vcpu, stack);
+ while(stack < stack_limit && stack < stk_addr_start +
i*BYTES_PER_LINE) {
+ void *p = map_page(ctx, vcpu, stack);
if (!p)
return -1;
word = read_stack_word(p, width);
@@ -700,16 +703,19 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
printf("Stack Trace:\n");
else
printf("Call Trace:\n");
- printf("%s ", width == 8
- ? " "
- : " ");
- printf("%c [<", xenctx.stack_trace ? ''*'' :
'' '');
- print_stack_word(instr_pointer(ctx), width);
- printf(">]");
-
- print_symbol(instr_pointer(ctx));
- printf(" <--\n");
+ if (!stk_addr || xenctx.frame_ptrs) {
+ printf("%s ", width == 8
+ ? " "
+ : " ");
+ printf("%c [<", xenctx.stack_trace ? ''*''
: '' '');
+ print_stack_word(instr_pointer(ctx), width);
+ printf(">]");
+ print_symbol(instr_pointer(ctx));
+ printf(" <--\n");
+ }
if (xenctx.frame_ptrs) {
+ void *p;
+
stack = stack_pointer(ctx);
frame = frame_pointer(ctx);
while(frame && stack < stack_limit) {
@@ -755,9 +761,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
}
}
} else {
- stack = stack_pointer(ctx);
+ stack = stk_addr_start;
while(stack < stack_limit) {
- p = map_page(ctx, vcpu, stack);
+ void *p = map_page(ctx, vcpu, stack);
if (!p)
return -1;
word = read_stack_word(p, width);
@@ -781,7 +787,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width)
}
#endif
-static void dump_ctx(int vcpu)
+static void dump_ctx(int vcpu, guest_word_t stk_addr)
{
vcpu_guest_context_any_t ctx;
@@ -820,17 +826,21 @@ static void dump_ctx(int vcpu)
}
#endif
- print_ctx(&ctx);
+ if (!stk_addr) {
+ print_ctx(&ctx);
+ }
#ifndef NO_TRANSLATION
- if (print_code(&ctx, vcpu))
- return;
- if (is_kernel_text(instr_pointer(&ctx)))
- if (print_stack(&ctx, vcpu, guest_word_size))
- return;
+ if (!stk_addr) {
+ print_code(&ctx, vcpu);
+ if (is_kernel_text(instr_pointer(&ctx)))
+ print_stack(&ctx, vcpu, guest_word_size, stk_addr);
+ } else {
+ print_stack(&ctx, vcpu, guest_word_size, stk_addr);
+ }
#endif
}
-static void dump_all_vcpus(void)
+static void dump_all_vcpus(guest_word_t stk_addr)
{
xc_vcpuinfo_t vinfo;
int vcpu;
@@ -839,7 +849,7 @@ static void dump_all_vcpus(void)
if ( xc_vcpu_getinfo(xenctx.xc_handle, xenctx.domid, vcpu, &vinfo)
)
continue;
if ( vinfo.online )
- dump_ctx(vcpu);
+ dump_ctx(vcpu, stk_addr);
}
}
@@ -855,31 +865,35 @@ static void usage(void)
printf(" frame pointers.\n");
printf(" -s SYMTAB, --symbol-table=SYMTAB\n");
printf(" read symbol table from SYMTAB.\n");
- printf(" -S --stack-trace print a complete stack trace.\n");
- printf(" -k, --kernel-start\n");
- printf(" set user/kernel split. (default
0xc0000000)\n");
- printf(" -a --all display more registers\n");
+ printf(" -S, --stack-trace print a complete stack trace.\n");
+ printf(" -k kaddr, --kernel-start=kaddr\n");
+ printf(" set user/kernel split at kaddr. (default
0xc0000000)\n");
+ printf(" -d daddr, --dump-as-stack=daddr\n");
+ printf(" dump memory as a stack at daddr.\n");
+ printf(" -a, --all display more registers\n");
printf(" -2, --two-pages assume the kernel was compiled with 8KiB
stacks.\n");
- printf(" -C --all-vcpus print info for all vcpus\n");
+ printf(" -C, --all-vcpus print info for all vcpus\n");
}
int main(int argc, char **argv)
{
int ch;
int ret;
- static const char *sopts = "fs:hak:SC2";
+ static const char *sopts = "fs:hak:SCd:2";
static const struct option lopts[] = {
{"stack-trace", 0, NULL, ''S''},
{"symbol-table", 1, NULL, ''s''},
{"frame-pointers", 0, NULL, ''f''},
{"kernel-start", 1, NULL, ''k''},
{"two-pages", 0, NULL, ''2''},
+ {"dump-as-stack", 1, NULL, ''d''},
{"all", 0, NULL, ''a''},
{"all-vcpus", 0, NULL, ''C''},
{"help", 0, NULL, ''h''},
{0, 0, 0, 0}
};
const char *symbol_table = NULL;
+ guest_word_t stk_addr = 0;
int vcpu = 0;
@@ -906,6 +920,9 @@ int main(int argc, char **argv)
case ''k'':
kernel_start = strtoull(optarg, NULL, 0);
break;
+ case ''d'':
+ stk_addr = strtoull(optarg, NULL, 0);
+ break;
case ''h'':
usage();
exit(-1);
@@ -956,9 +973,9 @@ int main(int argc, char **argv)
}
if (xenctx.all_vcpus)
- dump_all_vcpus();
+ dump_all_vcpus(stk_addr);
else
- dump_ctx(vcpu);
+ dump_ctx(vcpu, stk_addr);
if (xenctx.self_paused) {
ret = xc_domain_unpause(xenctx.xc_handle, xenctx.domid);
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 09/12] xenctx: Add -m <maddr> option to dump memory at maddr.
From: Don Slutz <Don@CloudSwitch.com>
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 126 ++++++++++++++++++++++++++++++++++++++++------
1 files changed, 109 insertions(+), 17 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 233f537..d6f8482 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -609,6 +609,33 @@ static guest_word_t read_stack_word(guest_word_t *src, int
width)
return word;
}
+static guest_word_t read_mem_word(vcpu_guest_context_any_t *ctx, int vcpu,
guest_word_t virt, int width)
+{
+ if ((virt & 7) == 0) {
+ guest_word_t *p = map_page(ctx, vcpu, virt);
+
+ if (p)
+ return read_stack_word(p, width);
+ else
+ return -1;
+ } else {
+ guest_word_t word = -1;
+ char *src, *dst;
+ int i;
+
+ dst = (char*)&word;
+ for(i = 0; i < width; i++) {
+ src = map_page(ctx, vcpu, virt + i);
+ if (src)
+ *dst++ = *src;
+ else
+ return word;
+ }
+ return word;
+ }
+}
+
+
static void print_stack_word(guest_word_t word, int width)
{
if (width == 4)
@@ -617,6 +644,58 @@ static void print_stack_word(guest_word_t word, int width)
printf(FMT_64B_WORD, word);
}
+#define BYTES_PER_LINE 16
+
+static void print_mem(vcpu_guest_context_any_t *ctx, int vcpu, int width,
guest_word_t mem_addr)
+{
+ guest_word_t instr;
+ guest_word_t instr_start;
+ guest_word_t word;
+ guest_word_t ascii[BYTES_PER_LINE/4];
+ unsigned char *bytep;
+ int i;
+
+ instr_start = mem_addr;
+ instr = mem_addr;
+ printf("Memory (addr %08llx)\n", instr);
+ for (i=1; i<10; i++) {
+ int j = 0;
+ int k;
+
+ print_stack_word(instr, width);
+ printf(":");
+ while(instr < instr_start + i*BYTES_PER_LINE) {
+ void *p = map_page(ctx, vcpu, instr);
+ if (!p)
+ return;
+ word = read_mem_word(ctx, vcpu, instr, width);
+ ascii[j++] = word;
+ printf(" ");
+ print_stack_word(word, width);
+ instr += width;
+ }
+ printf(" ");
+ for (k = j; k < BYTES_PER_LINE/width; k++)
+ printf("%s ", width == 8
+ ? " "
+ : " ");
+ for (k = 0; k < j; k++) {
+ int l;
+
+ bytep = (unsigned char*)&ascii[k];
+ for (l = 0; l < width; l++) {
+ if ((*bytep < 127) && (*bytep >= 32))
+ printf("%c", *bytep);
+ else
+ printf(".");
+ bytep++;
+ }
+ }
+ printf("\n");
+ }
+ printf("\n");
+}
+
static int print_code(vcpu_guest_context_any_t *ctx, int vcpu)
{
guest_word_t instr;
@@ -640,8 +719,6 @@ static int print_code(vcpu_guest_context_any_t *ctx, int
vcpu)
return 0;
}
-#define BYTES_PER_LINE 16
-
static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width,
guest_word_t stk_addr)
{
guest_word_t stk_addr_start = stack_pointer(ctx);
@@ -787,7 +864,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
}
#endif
-static void dump_ctx(int vcpu, guest_word_t stk_addr)
+static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr)
{
vcpu_guest_context_any_t ctx;
@@ -826,21 +903,29 @@ static void dump_ctx(int vcpu, guest_word_t stk_addr)
}
#endif
- if (!stk_addr) {
- print_ctx(&ctx);
- }
+ if (mem_addr) {
+ print_mem(&ctx, vcpu, guest_word_size, mem_addr);
#ifndef NO_TRANSLATION
- if (!stk_addr) {
- print_code(&ctx, vcpu);
- if (is_kernel_text(instr_pointer(&ctx)))
+ if (stk_addr)
print_stack(&ctx, vcpu, guest_word_size, stk_addr);
+#endif
} else {
- print_stack(&ctx, vcpu, guest_word_size, stk_addr);
- }
+ if (!stk_addr) {
+ print_ctx(&ctx);
+ }
+#ifndef NO_TRANSLATION
+ if (!stk_addr) {
+ print_code(&ctx, vcpu);
+ if (is_kernel_text(instr_pointer(&ctx)))
+ print_stack(&ctx, vcpu, guest_word_size, stk_addr);
+ } else {
+ print_stack(&ctx, vcpu, guest_word_size, stk_addr);
+ }
#endif
+ }
}
-static void dump_all_vcpus(guest_word_t stk_addr)
+static void dump_all_vcpus(guest_word_t mem_addr, guest_word_t stk_addr)
{
xc_vcpuinfo_t vinfo;
int vcpu;
@@ -849,7 +934,7 @@ static void dump_all_vcpus(guest_word_t stk_addr)
if ( xc_vcpu_getinfo(xenctx.xc_handle, xenctx.domid, vcpu, &vinfo)
)
continue;
if ( vinfo.online )
- dump_ctx(vcpu, stk_addr);
+ dump_ctx(vcpu, mem_addr, stk_addr);
}
}
@@ -868,6 +953,8 @@ static void usage(void)
printf(" -S, --stack-trace print a complete stack trace.\n");
printf(" -k kaddr, --kernel-start=kaddr\n");
printf(" set user/kernel split at kaddr. (default
0xc0000000)\n");
+ printf(" -m maddr, --memory=maddr\n");
+ printf(" dump memory at maddr.\n");
printf(" -d daddr, --dump-as-stack=daddr\n");
printf(" dump memory as a stack at daddr.\n");
printf(" -a, --all display more registers\n");
@@ -879,13 +966,14 @@ int main(int argc, char **argv)
{
int ch;
int ret;
- static const char *sopts = "fs:hak:SCd:2";
+ static const char *sopts = "fs:hak:SCm:d:2";
static const struct option lopts[] = {
{"stack-trace", 0, NULL, ''S''},
{"symbol-table", 1, NULL, ''s''},
{"frame-pointers", 0, NULL, ''f''},
{"kernel-start", 1, NULL, ''k''},
{"two-pages", 0, NULL, ''2''},
+ {"memory", 1, NULL, ''m''},
{"dump-as-stack", 1, NULL, ''d''},
{"all", 0, NULL, ''a''},
{"all-vcpus", 0, NULL, ''C''},
@@ -893,6 +981,7 @@ int main(int argc, char **argv)
{0, 0, 0, 0}
};
const char *symbol_table = NULL;
+ guest_word_t mem_addr = 0;
guest_word_t stk_addr = 0;
int vcpu = 0;
@@ -920,6 +1009,9 @@ int main(int argc, char **argv)
case ''k'':
kernel_start = strtoull(optarg, NULL, 0);
break;
+ case ''m'':
+ mem_addr = strtoull(optarg, NULL, 0);
+ break;
case ''d'':
stk_addr = strtoull(optarg, NULL, 0);
break;
@@ -952,7 +1044,7 @@ int main(int argc, char **argv)
read_symbol_table(symbol_table);
xenctx.xc_handle = xc_interface_open(0,0,0); /* for accessing control
interface */
- if (xenctx.xc_handle < 0) {
+ if (!xenctx.xc_handle) {
perror("xc_interface_open");
exit(-1);
}
@@ -973,9 +1065,9 @@ int main(int argc, char **argv)
}
if (xenctx.all_vcpus)
- dump_all_vcpus(stk_addr);
+ dump_all_vcpus(mem_addr, stk_addr);
else
- dump_ctx(vcpu, stk_addr);
+ dump_ctx(vcpu, mem_addr, stk_addr);
if (xenctx.self_paused) {
ret = xc_domain_unpause(xenctx.xc_handle, xenctx.domid);
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 10/12] xenctx: change is_kernel_text() into is_kernel_addr().
From: Don Slutz <Don@CloudSwitch.com>
A new enum has been added to allow the caller to determine if this
kernel address is a text on data address. This is currenlty not
used, but will be in the next patch.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 70 ++++++++++++++++++++++++++++++-----------------
1 files changed, 45 insertions(+), 25 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index d6f8482..6ec9c74 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -41,6 +41,15 @@ static struct xenctx {
xc_dominfo_t dominfo;
} xenctx;
+/* Note: the order of these matter.
+ * NOT_KERNEL_ADDR must be < both KERNEL_DATA_ADDR and KERNEL_TEXT_ADDR.
+ * KERNEL_DATA_ADDR must be < KERNEL_TEXT_ADDR. */
+typedef enum type_of_addr_ {
+ NOT_KERNEL_ADDR,
+ KERNEL_DATA_ADDR,
+ KERNEL_TEXT_ADDR,
+} type_of_addr;
+
#if defined (__i386__) || defined (__x86_64__)
typedef unsigned long long guest_word_t;
#define FMT_32B_WORD "%08llx"
@@ -69,6 +78,7 @@ struct symbol {
} *symbol_table = NULL;
guest_word_t kernel_stext, kernel_etext, kernel_sinittext, kernel_einittext,
kernel_hypercallpage;
+guest_word_t kernel_text, kernel_end;
#if defined (__i386__)
unsigned long long kernel_start = 0xc0000000;
@@ -76,22 +86,28 @@ unsigned long long kernel_start = 0xc0000000;
unsigned long long kernel_start = 0xffffffff80000000UL;
#endif
-static int is_kernel_text(guest_word_t addr)
+static type_of_addr is_kernel_addr(guest_word_t addr)
{
- if (symbol_table == NULL)
- return (addr > kernel_start);
+ if (symbol_table == NULL) {
+ if (addr > kernel_start)
+ return KERNEL_TEXT_ADDR;
+ else
+ return NOT_KERNEL_ADDR;
+ }
if (addr >= kernel_stext &&
addr <= kernel_etext)
- return 1;
- if (kernel_hypercallpage &&
- (addr >= kernel_hypercallpage &&
- addr <= kernel_hypercallpage + 4096))
- return 1;
+ return KERNEL_TEXT_ADDR;
+ if (kernel_hypercallpage && (addr >= kernel_hypercallpage
&&
+ addr <= kernel_hypercallpage + 4096))
+ return KERNEL_TEXT_ADDR;
if (addr >= kernel_sinittext &&
addr <= kernel_einittext)
- return 1;
- return 0;
+ return KERNEL_TEXT_ADDR;
+ if (addr >= kernel_text &&
+ addr <= kernel_end)
+ return KERNEL_DATA_ADDR;
+ return NOT_KERNEL_ADDR;
}
#if 0
@@ -145,11 +161,11 @@ static struct symbol *lookup_symbol(guest_word_t address)
return s->next && s->next->address <= address ?
s->next : s;
}
-static void print_symbol(guest_word_t addr)
+static void print_symbol(guest_word_t addr, type_of_addr type)
{
struct symbol *s;
- if (!is_kernel_text(addr))
+ if (is_kernel_addr(addr) < type)
return;
s = lookup_symbol(addr);
@@ -217,6 +233,10 @@ static void read_symbol_table(const char *symtab)
kernel_stext = symbol->address;
else if (strcmp(symbol->name, "_etext") == 0)
kernel_etext = symbol->address;
+ else if (strcmp(symbol->name, "_text") == 0)
+ kernel_text = symbol->address;
+ else if (strcmp(symbol->name, "_end") == 0)
+ kernel_end = symbol->address;
else if (strcmp(symbol->name, "_sinittext") == 0)
kernel_sinittext = symbol->address;
else if (strcmp(symbol->name, "_einittext") == 0)
@@ -288,7 +308,7 @@ static void print_ctx_32(vcpu_guest_context_x86_32_t *ctx)
struct cpu_user_regs_x86_32 *regs = &ctx->user_regs;
printf("cs:eip: %04x:%08x", regs->cs, regs->eip);
- print_symbol(regs->eip);
+ print_symbol(regs->eip, KERNEL_TEXT_ADDR);
print_flags(regs->eflags);
printf("ss:esp: %04x:%08x\n", regs->ss, regs->esp);
@@ -317,7 +337,7 @@ static void print_ctx_32on64(vcpu_guest_context_x86_64_t
*ctx)
struct cpu_user_regs_x86_64 *regs = &ctx->user_regs;
printf("cs:eip: %04x:%08x", regs->cs, (uint32_t)regs->eip);
- print_symbol((uint32_t)regs->eip);
+ print_symbol((uint32_t)regs->eip, KERNEL_TEXT_ADDR);
print_flags((uint32_t)regs->eflags);
printf("ss:esp: %04x:%08x\n", regs->ss,
(uint32_t)regs->esp);
@@ -346,7 +366,7 @@ static void print_ctx_64(vcpu_guest_context_x86_64_t *ctx)
struct cpu_user_regs_x86_64 *regs = &ctx->user_regs;
printf("rip: %016"PRIx64, regs->rip);
- print_symbol(regs->rip);
+ print_symbol(regs->rip, KERNEL_TEXT_ADDR);
print_flags(regs->rflags);
printf("rsp: %016"PRIx64"\n", regs->rsp);
@@ -444,7 +464,7 @@ static void print_ctx_32(vcpu_guest_context_t *ctx)
vcpu_guest_core_regs_t *regs = &ctx->user_regs;
printf("PC: %08"PRIx32, regs->pc32);
- print_symbol(regs->pc32);
+ print_symbol(regs->pc32, KERNEL_TEXT_ADDR);
printf("\n");
printf("CPSR: %08"PRIx32"\n", regs->cpsr);
printf("USR: SP:%08"PRIx32"
LR:%08"PRIx32"\n",
@@ -496,7 +516,7 @@ static void print_ctx_64(vcpu_guest_context_t *ctx)
vcpu_guest_core_regs_t *regs = &ctx->user_regs;
printf("PC: %016"PRIx64, regs->pc64);
- print_symbol(regs->pc64);
+ print_symbol(regs->pc64, KERNEL_TEXT_ADDR);
printf("\n");
printf("LR: %016"PRIx64"zn", regs->x30);
@@ -749,7 +769,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
void *p = map_page(ctx, vcpu, stack);
if (!p)
return -1;
- word = read_stack_word(p, width);
+ word = read_mem_word(ctx, vcpu, stack, width);
ascii[j++] = word;
printf(" ");
print_stack_word(word, width);
@@ -787,7 +807,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
printf("%c [<", xenctx.stack_trace ? ''*''
: '' '');
print_stack_word(instr_pointer(ctx), width);
printf(">]");
- print_symbol(instr_pointer(ctx));
+ print_symbol(instr_pointer(ctx), KERNEL_TEXT_ADDR);
printf(" <--\n");
}
if (xenctx.frame_ptrs) {
@@ -818,7 +838,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
if (xenctx.stack_trace) {
print_stack_word(stack, width);
printf(": |-- ");
- print_stack_word(read_stack_word(p, width), width);
+ print_stack_word(frame, width);
printf("\n");
}
stack += width;
@@ -832,7 +852,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
printf(": %c [<", xenctx.stack_trace ?
''|'' : '' '');
print_stack_word(word, width);
printf(">]");
- print_symbol(word);
+ print_symbol(word, KERNEL_TEXT_ADDR);
printf("\n");
stack += width;
}
@@ -843,13 +863,13 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
void *p = map_page(ctx, vcpu, stack);
if (!p)
return -1;
- word = read_stack_word(p, width);
- if (is_kernel_text(word)) {
+ word = read_mem_word(ctx, vcpu, stack, width);
+ if (is_kernel_addr(word) >= KERNEL_TEXT_ADDR) {
print_stack_word(stack, width);
printf(": [<");
print_stack_word(word, width);
printf(">]");
- print_symbol(word);
+ print_symbol(word, KERNEL_TEXT_ADDR);
printf("\n");
} else if (xenctx.stack_trace) {
print_stack_word(stack, width);
@@ -916,7 +936,7 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr,
guest_word_t stk_addr)
#ifndef NO_TRANSLATION
if (!stk_addr) {
print_code(&ctx, vcpu);
- if (is_kernel_text(instr_pointer(&ctx)))
+ if (is_kernel_addr(instr_pointer(&ctx)) >= KERNEL_TEXT_ADDR)
print_stack(&ctx, vcpu, guest_word_size, stk_addr);
} else {
print_stack(&ctx, vcpu, guest_word_size, stk_addr);
--
1.7.1
Don Slutz
2013-Nov-06 20:08 UTC
[PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
From: Don Slutz <Don@CloudSwitch.com>
This allows the output of registers that are not available in the
not HVM view.
Look up symbols for various registers as kernel data.
Also a minor check that ip, sp, and cr3 are the same.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 284 ++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 282 insertions(+), 2 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 6ec9c74..d3cbb22 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -415,6 +415,247 @@ static void print_ctx(vcpu_guest_context_any_t *ctx)
print_ctx_64(&ctx->x64);
}
+#if defined(__i386__) || defined(__x86_64__)
+static void print_cpuctx_regs64(struct hvm_hw_cpu *ctx)
+{
+ printf("rip: %016"PRIx64, ctx->rip);
+ print_symbol(ctx->rip, KERNEL_TEXT_ADDR);
+ print_flags(ctx->rflags);
+ printf("rsp: %016"PRIx64"\n", ctx->rsp);
+
+ printf("rax: %016"PRIx64"\t", ctx->rax);
+ printf("rcx: %016"PRIx64"\t", ctx->rcx);
+ printf("rdx: %016"PRIx64"\n", ctx->rdx);
+
+ printf("rbx: %016"PRIx64"\t", ctx->rbx);
+ printf("rsi: %016"PRIx64"\t", ctx->rsi);
+ printf("rdi: %016"PRIx64"\n", ctx->rdi);
+
+ printf("rbp: %016"PRIx64"\t", ctx->rbp);
+ printf(" r8: %016"PRIx64"\t", ctx->r8);
+ printf(" r9: %016"PRIx64"\n", ctx->r9);
+
+ printf("r10: %016"PRIx64"\t", ctx->r10);
+ printf("r11: %016"PRIx64"\t", ctx->r11);
+ printf("r12: %016"PRIx64"\n", ctx->r12);
+
+ printf("r13: %016"PRIx64"\t", ctx->r13);
+ printf("r14: %016"PRIx64"\t", ctx->r14);
+ printf("r15: %016"PRIx64"\n", ctx->r15);
+
+ printf(" cs: %04x @ %016"PRIx64, ctx->cs_sel,
ctx->cs_base);
+ if (ctx->cs_base)
+ print_symbol(ctx->cs_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->cs_limit,
ctx->cs_arbytes);
+ printf(" ss: %04x @ %016"PRIx64, ctx->ss_sel,
ctx->ss_base);
+ if (ctx->ss_base)
+ print_symbol(ctx->ss_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->ss_limit,
ctx->ss_arbytes);
+ printf(" ds: %04x @ %016"PRIx64, ctx->ds_sel,
ctx->ds_base);
+ if (ctx->ds_base)
+ print_symbol(ctx->ds_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->ds_limit,
ctx->ds_arbytes);
+ printf(" es: %04x @ %016"PRIx64, ctx->es_sel,
ctx->es_base);
+ if (ctx->es_base)
+ print_symbol(ctx->es_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->es_limit,
ctx->es_arbytes);
+ printf(" fs: %04x @ %016"PRIx64, ctx->fs_sel,
ctx->fs_base);
+ if (ctx->fs_base)
+ print_symbol(ctx->fs_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->fs_limit,
ctx->fs_arbytes);
+ printf(" gs: %04x @ %016"PRIx64"\\%016"PRIx64,
ctx->gs_sel,
+ ctx->gs_base, ctx->shadow_gs);
+ if (ctx->gs_base)
+ print_symbol(ctx->gs_base, KERNEL_DATA_ADDR);
+ printf("\\");
+ if (ctx->shadow_gs)
+ print_symbol(ctx->shadow_gs, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->gs_limit,
ctx->gs_arbytes);
+
+ if (xenctx.disp_all) {
+ printf(" tr: %04x @ %016"PRIx64, ctx->tr_sel,
ctx->tr_base);
+ if (ctx->tr_base)
+ print_symbol(ctx->tr_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->tr_limit,
ctx->tr_arbytes);
+ printf(" ldt: %04x @ %016"PRIx64, ctx->ldtr_sel,
ctx->ldtr_base);
+ if (ctx->ldtr_base)
+ print_symbol(ctx->ldtr_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->ldtr_limit,
ctx->ldtr_arbytes);
+ printf("\n");
+ printf("cr0: %08"PRIx64"\n", ctx->cr0);
+ printf("cr2: %08"PRIx64, ctx->cr2);
+ print_symbol(ctx->cr2, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("cr3: %08"PRIx64"\n", ctx->cr3);
+ printf("cr4: %08"PRIx64"\n", ctx->cr4);
+ printf("\n");
+ printf("dr0: %08"PRIx64, ctx->dr0);
+ print_symbol(ctx->dr0, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr1: %08"PRIx64, ctx->dr1);
+ print_symbol(ctx->dr1, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr2: %08"PRIx64, ctx->dr2);
+ print_symbol(ctx->dr2, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr3: %08"PRIx64, ctx->dr3);
+ print_symbol(ctx->dr3, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr6: %08"PRIx64"\n", ctx->dr6);
+ printf("dr7: %08"PRIx64"\n", ctx->dr7);
+ printf("\n");
+ printf("gdt: %016"PRIx64"/%lx", ctx->gdtr_base,
(long)(ctx->gdtr_limit));
+ print_symbol(ctx->gdtr_base, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("idt: %016"PRIx64"/%lx", ctx->idtr_base,
(long)(ctx->idtr_limit));
+ print_symbol(ctx->idtr_base, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("\n");
+ printf(" EFER: %08"PRIx64" flags:
%08"PRIx64"\n",
+ ctx->msr_efer, ctx->msr_flags);
+ printf("SYSENTER cs: %08"PRIx64" esp:
%08"PRIx64" eip: %08"PRIx64,
+ ctx->sysenter_cs, ctx->sysenter_esp,
ctx->sysenter_eip);
+ print_symbol(ctx->sysenter_eip, KERNEL_TEXT_ADDR);
+ printf("\n");
+ printf(" STAR: %08"PRIx64" SFMASK:
%08"PRIx64"\n",
+ ctx->msr_star, ctx->msr_syscall_mask);
+ printf("LSTAR: %08"PRIx64, ctx->msr_lstar);
+ print_symbol(ctx->msr_lstar, KERNEL_TEXT_ADDR);
+ printf("\n");
+ printf("CSTAR: %08"PRIx64, ctx->msr_cstar);
+ print_symbol(ctx->msr_cstar, KERNEL_TEXT_ADDR);
+ printf("\n");
+ printf("\n");
+ printf("\n");
+ printf(" tsc: %016"PRIx64"\n", ctx->tsc);
+ printf("\n");
+ printf("\n");
+ printf("pending_event: %lx pending_vector: %lx\n",
+ (long)(ctx->pending_event), (long)(ctx->pending_vector));
+ printf("pending error_code: %lx\n\n",
(long)(ctx->error_code));
+ }
+}
+
+static void print_cpuctx_regs32(struct hvm_hw_cpu *ctx)
+{
+ printf("cs:eip: %04x:%08x", ctx->cs_sel,
(uint32_t)ctx->rip);
+ print_symbol((uint32_t)ctx->rip, KERNEL_TEXT_ADDR);
+ print_flags((uint32_t)ctx->rflags);
+ printf("ss:esp: %04x:%08x\n", ctx->ss_sel,
(uint32_t)ctx->rsp);
+ printf("\n");
+
+ printf("eax: %08x\t", (uint32_t)ctx->rax);
+ printf("ebx: %08x\t", (uint32_t)ctx->rbx);
+ printf("ecx: %08x\t", (uint32_t)ctx->rcx);
+ printf("edx: %08x\n", (uint32_t)ctx->rdx);
+
+ printf("esi: %08x\t", (uint32_t)ctx->rsi);
+ printf("edi: %08x\t", (uint32_t)ctx->rdi);
+ printf("ebp: %08x\n", (uint32_t)ctx->rbp);
+ printf("\n");
+
+ printf(" cs: %04x @ %08"PRIx64, ctx->cs_sel,
ctx->cs_base);
+ if (ctx->cs_base)
+ print_symbol(ctx->cs_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->cs_limit,
ctx->cs_arbytes);
+ printf(" ss: %04x @ %08"PRIx64, ctx->ss_sel,
ctx->ss_base);
+ if (ctx->ss_base)
+ print_symbol(ctx->ss_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->ss_limit,
ctx->ss_arbytes);
+ printf(" ds: %04x @ %08"PRIx64, ctx->ds_sel,
ctx->ds_base);
+ if (ctx->ds_base)
+ print_symbol(ctx->ds_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->ds_limit,
ctx->ds_arbytes);
+ printf(" es: %04x @ %08"PRIx64, ctx->es_sel,
ctx->es_base);
+ if (ctx->es_base)
+ print_symbol(ctx->es_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->es_limit,
ctx->es_arbytes);
+ printf(" fs: %04x @ %08"PRIx64, ctx->fs_sel,
ctx->fs_base);
+ if (ctx->fs_base)
+ print_symbol(ctx->fs_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->fs_limit,
ctx->fs_arbytes);
+ printf(" gs: %04x @ %08"PRIx64"\\%08"PRIx64,
ctx->gs_sel,
+ ctx->gs_base, ctx->shadow_gs);
+ if (ctx->gs_base)
+ print_symbol(ctx->gs_base, KERNEL_DATA_ADDR);
+ printf("\\");
+ if (ctx->shadow_gs)
+ print_symbol(ctx->shadow_gs, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->gs_limit,
ctx->gs_arbytes);
+
+ if (xenctx.disp_all) {
+ printf(" tr: %04x @ %08"PRIx64, ctx->tr_sel,
ctx->tr_base);
+ if (ctx->tr_base)
+ print_symbol(ctx->tr_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->tr_limit,
ctx->tr_arbytes);
+ printf(" ldt: %04x @ %08"PRIx64, ctx->ldtr_sel,
ctx->ldtr_base);
+ if (ctx->ldtr_base)
+ print_symbol(ctx->ldtr_base, KERNEL_DATA_ADDR);
+ printf("\n /%08x(%x)\n", ctx->ldtr_limit,
ctx->ldtr_arbytes);
+ printf("\n");
+ printf("cr0: %08"PRIx64"\n", ctx->cr0);
+ printf("cr2: %08"PRIx64, ctx->cr2);
+ print_symbol(ctx->cr2, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("cr3: %08"PRIx64"\n", ctx->cr3);
+ printf("cr4: %08"PRIx64"\n", ctx->cr4);
+ printf("\n");
+ printf("dr0: %08"PRIx64, ctx->dr0);
+ print_symbol(ctx->dr0, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr1: %08"PRIx64, ctx->dr1);
+ print_symbol(ctx->dr1, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr2: %08"PRIx64, ctx->dr2);
+ print_symbol(ctx->dr2, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr3: %08"PRIx64, ctx->dr3);
+ print_symbol(ctx->dr3, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("dr6: %08"PRIx64"\n", ctx->dr6);
+ printf("dr7: %08"PRIx64"\n", ctx->dr7);
+ printf("\n");
+ printf("gdt: %08"PRIx64"/%lx", ctx->gdtr_base,
(long)(ctx->gdtr_limit));
+ print_symbol(ctx->gdtr_base, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("idt: %08"PRIx64"/%lx", ctx->idtr_base,
(long)(ctx->idtr_limit));
+ print_symbol(ctx->idtr_base, KERNEL_DATA_ADDR);
+ printf("\n");
+ printf("\n");
+ printf(" EFER: %08"PRIx64" flags:
%08"PRIx64"\n",
+ ctx->msr_efer, ctx->msr_flags);
+ printf("SYSENTER cs: %08"PRIx64" esp:
%08"PRIx64" eip: %08"PRIx64,
+ ctx->sysenter_cs, ctx->sysenter_esp,
ctx->sysenter_eip);
+ print_symbol(ctx->sysenter_eip, KERNEL_TEXT_ADDR);
+ printf("\n");
+ printf(" STAR: %08"PRIx64" SFMASK:
%08"PRIx64"\n",
+ ctx->msr_star, ctx->msr_syscall_mask);
+ printf("LSTAR: %08"PRIx64, ctx->msr_lstar);
+ print_symbol(ctx->msr_lstar, KERNEL_TEXT_ADDR);
+ printf("\n");
+ printf("CSTAR: %08"PRIx64, ctx->msr_cstar);
+ print_symbol(ctx->msr_cstar, KERNEL_TEXT_ADDR);
+ printf("\n");
+ printf("\n");
+ printf(" tsc: %016"PRIx64"\n", ctx->tsc);
+ printf("\n");
+ printf("\n");
+ printf("pending_event: %lx pending_vector: %lx\n",
+ (long)(ctx->pending_event), (long)(ctx->pending_vector));
+ printf("pending error_code: %lx\n\n",
(long)(ctx->error_code));
+ }
+}
+
+static void print_cpuctx(struct hvm_hw_cpu *ctx)
+{
+ if (guest_word_size == 8)
+ print_cpuctx_regs64(ctx);
+ else
+ print_cpuctx_regs32(ctx);
+
+}
+#endif
+
#define NONPROT_MODE_SEGMENT_SHIFT 4
static guest_word_t instr_pointer(vcpu_guest_context_any_t *ctx)
@@ -887,6 +1128,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr)
{
vcpu_guest_context_any_t ctx;
+#if defined(__i386__) || defined(__x86_64__)
+ struct hvm_hw_cpu cpuctx;
+#endif
if (xc_vcpu_getcontext(xenctx.xc_handle, xenctx.domid, vcpu, &ctx) <
0) {
perror("xc_vcpu_getcontext");
@@ -896,7 +1140,6 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr,
guest_word_t stk_addr)
#if defined(__i386__) || defined(__x86_64__)
{
if (xenctx.dominfo.hvm) {
- struct hvm_hw_cpu cpuctx;
xen_capabilities_info_t xen_caps = "";
if (xc_domain_hvm_getcontext_partial(
xenctx.xc_handle, xenctx.domid, HVM_SAVE_CODE(CPU),
@@ -931,7 +1174,44 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr,
guest_word_t stk_addr)
#endif
} else {
if (!stk_addr) {
- print_ctx(&ctx);
+#if defined(__i386__) || defined(__x86_64__)
+ if (xenctx.dominfo.hvm && ctxt_word_size == 8) {
+ if (guest_word_size == 4) {
+ if ((((uint32_t)ctx.x64.user_regs.eip) != cpuctx.rip) ||
+ (((uint32_t)ctx.x64.user_regs.esp) != cpuctx.rsp) ||
+ (((uint32_t)ctx.x64.ctrlreg[3]) != cpuctx.cr3)) {
+ fprintf(stderr, "Regs mismatch ip=%llx vs %llx
sp=%llx vs %llx cr3=%llx vs %llx\n",
+ (long long)((uint32_t)ctx.x64.user_regs.eip),
+ (long long)cpuctx.rip,
+ (long long)((uint32_t)ctx.x64.user_regs.esp),
+ (long long)cpuctx.rsp,
+ (long long)((uint32_t)ctx.x64.ctrlreg[3]),
+ (long long)cpuctx.cr3);
+ fprintf(stdout, "=============Regs mismatch
start=============\n");
+ print_ctx(&ctx);
+ fprintf(stdout, "=============Regs mismatch
end=============\n");
+ }
+ } else {
+ if ((ctx.x64.user_regs.eip != cpuctx.rip) ||
+ (ctx.x64.user_regs.esp != cpuctx.rsp) ||
+ (ctx.x64.ctrlreg[3] != cpuctx.cr3)) {
+ fprintf(stderr, "Regs mismatch ip=%llx vs %llx
sp=%llx vs %llx cr3=%llx vs %llx\n",
+ (long long)ctx.x64.user_regs.eip,
+ (long long)cpuctx.rip,
+ (long long)ctx.x64.user_regs.esp,
+ (long long)cpuctx.rsp,
+ (long long)ctx.x64.ctrlreg[3],
+ (long long)cpuctx.cr3);
+ fprintf(stdout, "=============Regs mismatch
start=============\n");
+ print_ctx(&ctx);
+ fprintf(stdout, "=============Regs mismatch
end=============\n");
+ }
+ }
+ print_cpuctx(&cpuctx);
+ }
+ else
+#endif
+ print_ctx(&ctx);
}
#ifndef NO_TRANSLATION
if (!stk_addr) {
--
1.7.1
From: Don Slutz <Don@CloudSwitch.com>
This can be used to dump the normal ctx for an HVM DomU by passing
-1 to fCPU.
If only some of the vCPUs in a HVM DomU, you need to ask for ask for
the on line CPU number, not the expected vCPU number; fCPU can be
used for this.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
tools/xentrace/xenctx.c | 39 ++++++++++++++++++++++++++++-----------
1 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index d3cbb22..a7a791a 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -1125,7 +1125,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int
vcpu, int width, guest
}
#endif
-static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr)
+static void dump_ctx(int vcpu, int fcpu, guest_word_t mem_addr, guest_word_t
stk_addr)
{
vcpu_guest_context_any_t ctx;
#if defined(__i386__) || defined(__x86_64__)
@@ -1139,11 +1139,11 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr,
guest_word_t stk_addr)
#if defined(__i386__) || defined(__x86_64__)
{
- if (xenctx.dominfo.hvm) {
+ if (fcpu >= 0 && xenctx.dominfo.hvm) {
xen_capabilities_info_t xen_caps = "";
if (xc_domain_hvm_getcontext_partial(
xenctx.xc_handle, xenctx.domid, HVM_SAVE_CODE(CPU),
- vcpu, &cpuctx, sizeof cpuctx) != 0) {
+ fcpu, &cpuctx, sizeof cpuctx) != 0) {
perror("xc_domain_hvm_getcontext_partial");
return;
}
@@ -1175,7 +1175,7 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr,
guest_word_t stk_addr)
} else {
if (!stk_addr) {
#if defined(__i386__) || defined(__x86_64__)
- if (xenctx.dominfo.hvm && ctxt_word_size == 8) {
+ if (fcpu >= 0 && xenctx.dominfo.hvm &&
ctxt_word_size == 8) {
if (guest_word_size == 4) {
if ((((uint32_t)ctx.x64.user_regs.eip) != cpuctx.rip) ||
(((uint32_t)ctx.x64.user_regs.esp) != cpuctx.rsp) ||
@@ -1229,12 +1229,22 @@ static void dump_all_vcpus(guest_word_t mem_addr,
guest_word_t stk_addr)
{
xc_vcpuinfo_t vinfo;
int vcpu;
+ int fcpu = 0;
+
for (vcpu = 0; vcpu <= xenctx.dominfo.max_vcpu_id; vcpu++)
{
if ( xc_vcpu_getinfo(xenctx.xc_handle, xenctx.domid, vcpu, &vinfo)
)
continue;
- if ( vinfo.online )
- dump_ctx(vcpu, mem_addr, stk_addr);
+ if ( vinfo.online ) {
+ printf("vcpu=%d(%d)\n", vcpu, fcpu);
+ dump_ctx(vcpu, fcpu, mem_addr, stk_addr);
+ fcpu++;
+ printf("\n");
+ } else {
+ printf("vcpu=%d offline\n", vcpu);
+ dump_ctx(vcpu, -1, mem_addr, stk_addr);
+ printf("\n");
+ }
}
}
@@ -1242,7 +1252,7 @@ static void usage(void)
{
printf("usage:\n\n");
- printf(" xenctx [options] <DOMAIN> [VCPU]\n\n");
+ printf(" xenctx [options] <DOMAIN> [VCPU] [fCPU]\n\n");
printf("options:\n");
printf(" -f, --frame-pointers\n");
@@ -1285,6 +1295,7 @@ int main(int argc, char **argv)
guest_word_t stk_addr = 0;
int vcpu = 0;
+ int fcpu = 0;
while ((ch = getopt_long(argc, argv, sopts, lopts, NULL)) != -1) {
switch(ch) {
@@ -1326,8 +1337,8 @@ int main(int argc, char **argv)
argv += optind; argc -= optind;
- if (argc < 1 || argc > 2) {
- printf("usage: xenctx [options] <domid> <optional
vcpu>\n");
+ if (argc < 1 || argc > 3) {
+ printf("usage: xenctx [options] <domid> <optional
vcpu> <optional fcpu>\n");
exit(-1);
}
@@ -1337,8 +1348,14 @@ int main(int argc, char **argv)
exit(-1);
}
- if (argc == 2)
+ if (argc == 2) {
vcpu = atoi(argv[1]);
+ fcpu = vcpu;
+ }
+
+ if (argc == 3) {
+ fcpu = atoi(argv[2]);
+ }
if (symbol_table)
read_symbol_table(symbol_table);
@@ -1367,7 +1384,7 @@ int main(int argc, char **argv)
if (xenctx.all_vcpus)
dump_all_vcpus(mem_addr, stk_addr);
else
- dump_ctx(vcpu, mem_addr, stk_addr);
+ dump_ctx(vcpu, fcpu, mem_addr, stk_addr);
if (xenctx.self_paused) {
ret = xc_domain_unpause(xenctx.xc_handle, xenctx.domid);
--
1.7.1
Andrew Cooper
2013-Nov-06 20:35 UTC
Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
On 06/11/13 20:08, Don Slutz wrote:> From: Don Slutz <Don@CloudSwitch.com> > > This allows the output of registers that are not available in the > not HVM view. > > Look up symbols for various registers as kernel data. > > Also a minor check that ip, sp, and cr3 are the same. > > Signed-off-by: Don Slutz <Don@CloudSwitch.com>What about tools/misc/xen-hvmctx ? This is a particularly awkward tool where we have two tools in different subtrees which do about 80% the same thing. I would certainly not object if someone were to merge these two tools into one. It is particularly awkward as xen-hvmctx ends up being installed into /usr/sbin/ and is therefore on the $PATH, while xenctx ends up off the $PATH in /usr/lib/xen/bin/ I am also fairly sure we have other examples of tools like this with sightly different variants living in different locations. ~Andrew> --- > tools/xentrace/xenctx.c | 284 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 282 insertions(+), 2 deletions(-) > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index 6ec9c74..d3cbb22 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -415,6 +415,247 @@ static void print_ctx(vcpu_guest_context_any_t *ctx) > print_ctx_64(&ctx->x64); > } > > +#if defined(__i386__) || defined(__x86_64__) > +static void print_cpuctx_regs64(struct hvm_hw_cpu *ctx) > +{ > + printf("rip: %016"PRIx64, ctx->rip); > + print_symbol(ctx->rip, KERNEL_TEXT_ADDR); > + print_flags(ctx->rflags); > + printf("rsp: %016"PRIx64"\n", ctx->rsp); > + > + printf("rax: %016"PRIx64"\t", ctx->rax); > + printf("rcx: %016"PRIx64"\t", ctx->rcx); > + printf("rdx: %016"PRIx64"\n", ctx->rdx); > + > + printf("rbx: %016"PRIx64"\t", ctx->rbx); > + printf("rsi: %016"PRIx64"\t", ctx->rsi); > + printf("rdi: %016"PRIx64"\n", ctx->rdi); > + > + printf("rbp: %016"PRIx64"\t", ctx->rbp); > + printf(" r8: %016"PRIx64"\t", ctx->r8); > + printf(" r9: %016"PRIx64"\n", ctx->r9); > + > + printf("r10: %016"PRIx64"\t", ctx->r10); > + printf("r11: %016"PRIx64"\t", ctx->r11); > + printf("r12: %016"PRIx64"\n", ctx->r12); > + > + printf("r13: %016"PRIx64"\t", ctx->r13); > + printf("r14: %016"PRIx64"\t", ctx->r14); > + printf("r15: %016"PRIx64"\n", ctx->r15); > + > + printf(" cs: %04x @ %016"PRIx64, ctx->cs_sel, ctx->cs_base); > + if (ctx->cs_base) > + print_symbol(ctx->cs_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->cs_limit, ctx->cs_arbytes); > + printf(" ss: %04x @ %016"PRIx64, ctx->ss_sel, ctx->ss_base); > + if (ctx->ss_base) > + print_symbol(ctx->ss_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->ss_limit, ctx->ss_arbytes); > + printf(" ds: %04x @ %016"PRIx64, ctx->ds_sel, ctx->ds_base); > + if (ctx->ds_base) > + print_symbol(ctx->ds_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->ds_limit, ctx->ds_arbytes); > + printf(" es: %04x @ %016"PRIx64, ctx->es_sel, ctx->es_base); > + if (ctx->es_base) > + print_symbol(ctx->es_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->es_limit, ctx->es_arbytes); > + printf(" fs: %04x @ %016"PRIx64, ctx->fs_sel, ctx->fs_base); > + if (ctx->fs_base) > + print_symbol(ctx->fs_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->fs_limit, ctx->fs_arbytes); > + printf(" gs: %04x @ %016"PRIx64"\\%016"PRIx64, ctx->gs_sel, > + ctx->gs_base, ctx->shadow_gs); > + if (ctx->gs_base) > + print_symbol(ctx->gs_base, KERNEL_DATA_ADDR); > + printf("\\"); > + if (ctx->shadow_gs) > + print_symbol(ctx->shadow_gs, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->gs_limit, ctx->gs_arbytes); > + > + if (xenctx.disp_all) { > + printf(" tr: %04x @ %016"PRIx64, ctx->tr_sel, ctx->tr_base); > + if (ctx->tr_base) > + print_symbol(ctx->tr_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->tr_limit, ctx->tr_arbytes); > + printf(" ldt: %04x @ %016"PRIx64, ctx->ldtr_sel, ctx->ldtr_base); > + if (ctx->ldtr_base) > + print_symbol(ctx->ldtr_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->ldtr_limit, ctx->ldtr_arbytes); > + printf("\n"); > + printf("cr0: %08"PRIx64"\n", ctx->cr0); > + printf("cr2: %08"PRIx64, ctx->cr2); > + print_symbol(ctx->cr2, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("cr3: %08"PRIx64"\n", ctx->cr3); > + printf("cr4: %08"PRIx64"\n", ctx->cr4); > + printf("\n"); > + printf("dr0: %08"PRIx64, ctx->dr0); > + print_symbol(ctx->dr0, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr1: %08"PRIx64, ctx->dr1); > + print_symbol(ctx->dr1, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr2: %08"PRIx64, ctx->dr2); > + print_symbol(ctx->dr2, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr3: %08"PRIx64, ctx->dr3); > + print_symbol(ctx->dr3, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr6: %08"PRIx64"\n", ctx->dr6); > + printf("dr7: %08"PRIx64"\n", ctx->dr7); > + printf("\n"); > + printf("gdt: %016"PRIx64"/%lx", ctx->gdtr_base, (long)(ctx->gdtr_limit)); > + print_symbol(ctx->gdtr_base, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("idt: %016"PRIx64"/%lx", ctx->idtr_base, (long)(ctx->idtr_limit)); > + print_symbol(ctx->idtr_base, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("\n"); > + printf(" EFER: %08"PRIx64" flags: %08"PRIx64"\n", > + ctx->msr_efer, ctx->msr_flags); > + printf("SYSENTER cs: %08"PRIx64" esp: %08"PRIx64" eip: %08"PRIx64, > + ctx->sysenter_cs, ctx->sysenter_esp, ctx->sysenter_eip); > + print_symbol(ctx->sysenter_eip, KERNEL_TEXT_ADDR); > + printf("\n"); > + printf(" STAR: %08"PRIx64" SFMASK: %08"PRIx64"\n", > + ctx->msr_star, ctx->msr_syscall_mask); > + printf("LSTAR: %08"PRIx64, ctx->msr_lstar); > + print_symbol(ctx->msr_lstar, KERNEL_TEXT_ADDR); > + printf("\n"); > + printf("CSTAR: %08"PRIx64, ctx->msr_cstar); > + print_symbol(ctx->msr_cstar, KERNEL_TEXT_ADDR); > + printf("\n"); > + printf("\n"); > + printf("\n"); > + printf(" tsc: %016"PRIx64"\n", ctx->tsc); > + printf("\n"); > + printf("\n"); > + printf("pending_event: %lx pending_vector: %lx\n", > + (long)(ctx->pending_event), (long)(ctx->pending_vector)); > + printf("pending error_code: %lx\n\n", (long)(ctx->error_code)); > + } > +} > + > +static void print_cpuctx_regs32(struct hvm_hw_cpu *ctx) > +{ > + printf("cs:eip: %04x:%08x", ctx->cs_sel, (uint32_t)ctx->rip); > + print_symbol((uint32_t)ctx->rip, KERNEL_TEXT_ADDR); > + print_flags((uint32_t)ctx->rflags); > + printf("ss:esp: %04x:%08x\n", ctx->ss_sel, (uint32_t)ctx->rsp); > + printf("\n"); > + > + printf("eax: %08x\t", (uint32_t)ctx->rax); > + printf("ebx: %08x\t", (uint32_t)ctx->rbx); > + printf("ecx: %08x\t", (uint32_t)ctx->rcx); > + printf("edx: %08x\n", (uint32_t)ctx->rdx); > + > + printf("esi: %08x\t", (uint32_t)ctx->rsi); > + printf("edi: %08x\t", (uint32_t)ctx->rdi); > + printf("ebp: %08x\n", (uint32_t)ctx->rbp); > + printf("\n"); > + > + printf(" cs: %04x @ %08"PRIx64, ctx->cs_sel, ctx->cs_base); > + if (ctx->cs_base) > + print_symbol(ctx->cs_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->cs_limit, ctx->cs_arbytes); > + printf(" ss: %04x @ %08"PRIx64, ctx->ss_sel, ctx->ss_base); > + if (ctx->ss_base) > + print_symbol(ctx->ss_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->ss_limit, ctx->ss_arbytes); > + printf(" ds: %04x @ %08"PRIx64, ctx->ds_sel, ctx->ds_base); > + if (ctx->ds_base) > + print_symbol(ctx->ds_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->ds_limit, ctx->ds_arbytes); > + printf(" es: %04x @ %08"PRIx64, ctx->es_sel, ctx->es_base); > + if (ctx->es_base) > + print_symbol(ctx->es_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->es_limit, ctx->es_arbytes); > + printf(" fs: %04x @ %08"PRIx64, ctx->fs_sel, ctx->fs_base); > + if (ctx->fs_base) > + print_symbol(ctx->fs_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->fs_limit, ctx->fs_arbytes); > + printf(" gs: %04x @ %08"PRIx64"\\%08"PRIx64, ctx->gs_sel, > + ctx->gs_base, ctx->shadow_gs); > + if (ctx->gs_base) > + print_symbol(ctx->gs_base, KERNEL_DATA_ADDR); > + printf("\\"); > + if (ctx->shadow_gs) > + print_symbol(ctx->shadow_gs, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->gs_limit, ctx->gs_arbytes); > + > + if (xenctx.disp_all) { > + printf(" tr: %04x @ %08"PRIx64, ctx->tr_sel, ctx->tr_base); > + if (ctx->tr_base) > + print_symbol(ctx->tr_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->tr_limit, ctx->tr_arbytes); > + printf(" ldt: %04x @ %08"PRIx64, ctx->ldtr_sel, ctx->ldtr_base); > + if (ctx->ldtr_base) > + print_symbol(ctx->ldtr_base, KERNEL_DATA_ADDR); > + printf("\n /%08x(%x)\n", ctx->ldtr_limit, ctx->ldtr_arbytes); > + printf("\n"); > + printf("cr0: %08"PRIx64"\n", ctx->cr0); > + printf("cr2: %08"PRIx64, ctx->cr2); > + print_symbol(ctx->cr2, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("cr3: %08"PRIx64"\n", ctx->cr3); > + printf("cr4: %08"PRIx64"\n", ctx->cr4); > + printf("\n"); > + printf("dr0: %08"PRIx64, ctx->dr0); > + print_symbol(ctx->dr0, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr1: %08"PRIx64, ctx->dr1); > + print_symbol(ctx->dr1, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr2: %08"PRIx64, ctx->dr2); > + print_symbol(ctx->dr2, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr3: %08"PRIx64, ctx->dr3); > + print_symbol(ctx->dr3, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("dr6: %08"PRIx64"\n", ctx->dr6); > + printf("dr7: %08"PRIx64"\n", ctx->dr7); > + printf("\n"); > + printf("gdt: %08"PRIx64"/%lx", ctx->gdtr_base, (long)(ctx->gdtr_limit)); > + print_symbol(ctx->gdtr_base, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("idt: %08"PRIx64"/%lx", ctx->idtr_base, (long)(ctx->idtr_limit)); > + print_symbol(ctx->idtr_base, KERNEL_DATA_ADDR); > + printf("\n"); > + printf("\n"); > + printf(" EFER: %08"PRIx64" flags: %08"PRIx64"\n", > + ctx->msr_efer, ctx->msr_flags); > + printf("SYSENTER cs: %08"PRIx64" esp: %08"PRIx64" eip: %08"PRIx64, > + ctx->sysenter_cs, ctx->sysenter_esp, ctx->sysenter_eip); > + print_symbol(ctx->sysenter_eip, KERNEL_TEXT_ADDR); > + printf("\n"); > + printf(" STAR: %08"PRIx64" SFMASK: %08"PRIx64"\n", > + ctx->msr_star, ctx->msr_syscall_mask); > + printf("LSTAR: %08"PRIx64, ctx->msr_lstar); > + print_symbol(ctx->msr_lstar, KERNEL_TEXT_ADDR); > + printf("\n"); > + printf("CSTAR: %08"PRIx64, ctx->msr_cstar); > + print_symbol(ctx->msr_cstar, KERNEL_TEXT_ADDR); > + printf("\n"); > + printf("\n"); > + printf(" tsc: %016"PRIx64"\n", ctx->tsc); > + printf("\n"); > + printf("\n"); > + printf("pending_event: %lx pending_vector: %lx\n", > + (long)(ctx->pending_event), (long)(ctx->pending_vector)); > + printf("pending error_code: %lx\n\n", (long)(ctx->error_code)); > + } > +} > + > +static void print_cpuctx(struct hvm_hw_cpu *ctx) > +{ > + if (guest_word_size == 8) > + print_cpuctx_regs64(ctx); > + else > + print_cpuctx_regs32(ctx); > + > +} > +#endif > + > #define NONPROT_MODE_SEGMENT_SHIFT 4 > > static guest_word_t instr_pointer(vcpu_guest_context_any_t *ctx) > @@ -887,6 +1128,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest > static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) > { > vcpu_guest_context_any_t ctx; > +#if defined(__i386__) || defined(__x86_64__) > + struct hvm_hw_cpu cpuctx; > +#endif > > if (xc_vcpu_getcontext(xenctx.xc_handle, xenctx.domid, vcpu, &ctx) < 0) { > perror("xc_vcpu_getcontext"); > @@ -896,7 +1140,6 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) > #if defined(__i386__) || defined(__x86_64__) > { > if (xenctx.dominfo.hvm) { > - struct hvm_hw_cpu cpuctx; > xen_capabilities_info_t xen_caps = ""; > if (xc_domain_hvm_getcontext_partial( > xenctx.xc_handle, xenctx.domid, HVM_SAVE_CODE(CPU), > @@ -931,7 +1174,44 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) > #endif > } else { > if (!stk_addr) { > - print_ctx(&ctx); > +#if defined(__i386__) || defined(__x86_64__) > + if (xenctx.dominfo.hvm && ctxt_word_size == 8) { > + if (guest_word_size == 4) { > + if ((((uint32_t)ctx.x64.user_regs.eip) != cpuctx.rip) || > + (((uint32_t)ctx.x64.user_regs.esp) != cpuctx.rsp) || > + (((uint32_t)ctx.x64.ctrlreg[3]) != cpuctx.cr3)) { > + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", > + (long long)((uint32_t)ctx.x64.user_regs.eip), > + (long long)cpuctx.rip, > + (long long)((uint32_t)ctx.x64.user_regs.esp), > + (long long)cpuctx.rsp, > + (long long)((uint32_t)ctx.x64.ctrlreg[3]), > + (long long)cpuctx.cr3); > + fprintf(stdout, "=============Regs mismatch start=============\n"); > + print_ctx(&ctx); > + fprintf(stdout, "=============Regs mismatch end=============\n"); > + } > + } else { > + if ((ctx.x64.user_regs.eip != cpuctx.rip) || > + (ctx.x64.user_regs.esp != cpuctx.rsp) || > + (ctx.x64.ctrlreg[3] != cpuctx.cr3)) { > + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", > + (long long)ctx.x64.user_regs.eip, > + (long long)cpuctx.rip, > + (long long)ctx.x64.user_regs.esp, > + (long long)cpuctx.rsp, > + (long long)ctx.x64.ctrlreg[3], > + (long long)cpuctx.cr3); > + fprintf(stdout, "=============Regs mismatch start=============\n"); > + print_ctx(&ctx); > + fprintf(stdout, "=============Regs mismatch end=============\n"); > + } > + } > + print_cpuctx(&cpuctx); > + } > + else > +#endif > + print_ctx(&ctx); > } > #ifndef NO_TRANSLATION > if (!stk_addr) {
Jan Beulich
2013-Nov-07 08:04 UTC
Re: [PATCH v2 02/12] xenctx: Add -2 (--two-pages) option to switch stack size to 8KiB
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > @@ -650,9 +651,11 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > > stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE) > & ~((guest_word_t) XC_PAGE_SIZE - 1)); > + if (xenctx.two_pages) > + stack_limit += XC_PAGE_SIZE; > printf("\n"); > printf("Stack:\n"); > - for (i=1; i<5 && stack < stack_limit; i++) { > + for (i=1; i<10 && stack < stack_limit; i++) {Touching a malformed line like this makes it almost mandatory imo to fix the coding style (missing blanks around = and <). Apart from that, the change isn''t really related to the subject of the patch afaict, and as you''re making thing more flexible anyway, it would seem reasonable to also have a command line option to control this limit. Jan
Jan Beulich
2013-Nov-07 08:07 UTC
Re: [PATCH v2 01/12] xenctx: Clean up stack trace when hypercall_page not in symbol table
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > From: Don Slutz <Don@CloudSwitch.com> > > Before: > > Call Trace: > [<ffffffff8006b2b0>] default_idle+0x29 <-- > [<ffffffff80048d19>] cpu_idle+0x95 > [<ffffffff803e7801>] start_kernel+0x220 > [<0000000000000000>] startup_64+0x80000000 > [<ffffffff803e722f>] x86_64_start_kernel+0x22f > [<0000000000000000>] startup_64+0x80000000 > [<0000000000000000>] startup_64+0x80000000 > [<0000000000000000>] startup_64+0x80000000 > [<0000000000000000>] startup_64+0x80000000 > > After: > > Call Trace: > [<ffffffff8006b2b0>] default_idle+0x29 <-- > [<ffffffff80048d19>] cpu_idle+0x95 > [<ffffffff803e7801>] start_kernel+0x220 > [<ffffffff803e722f>] x86_64_start_kernel+0x22f > > Signed-off-by: Don Slutz <Don@CloudSwitch.com>Reviewed-by: Jan Beulich <jbeulich@suse.com> And a note on patch submission: You send _to_ the list, and _cc_ maintainers and other relevant people (without stretching the meaning of "relevant" too much). Jan> --- > tools/xentrace/xenctx.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index 060e480..10292fa 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -83,8 +83,9 @@ static int is_kernel_text(guest_word_t addr) > if (addr >= kernel_stext && > addr <= kernel_etext) > return 1; > - if (addr >= kernel_hypercallpage && > - addr <= kernel_hypercallpage + 4096) > + if (kernel_hypercallpage && > + (addr >= kernel_hypercallpage && > + addr <= kernel_hypercallpage + 4096)) > return 1; > if (addr >= kernel_sinittext && > addr <= kernel_einittext) > -- > 1.7.1
Jan Beulich
2013-Nov-07 08:09 UTC
Re: [PATCH v2 03/12] xenctx: Output ascii version of stack also.
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote:Why? I personally don''t see a need - there''s rarely much useful ASCII data on the stack. So afaic: If at all, then only via command line option defaulting to off. Jan> From: Don Slutz <Don@CloudSwitch.com> > > Signed-off-by: Don Slutz <Don@CloudSwitch.com> > --- > tools/xentrace/xenctx.c | 27 ++++++++++++++++++++++++++- > 1 files changed, 26 insertions(+), 1 deletions(-) > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index 06a8850..dabce16 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -640,6 +640,8 @@ static int print_code(vcpu_guest_context_any_t *ctx, int > vcpu) > return 0; > } > > +#define BYTES_PER_LINE 32 > + > static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > { > guest_word_t stack = stack_pointer(ctx); > @@ -647,6 +649,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int > vcpu, int width) > guest_word_t frame; > guest_word_t word; > guest_word_t *p; > + guest_word_t ascii[BYTES_PER_LINE/4]; > + unsigned char *bytep; > int i; > > stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE) > @@ -656,15 +660,36 @@ static int print_stack(vcpu_guest_context_any_t *ctx, > int vcpu, int width) > printf("\n"); > printf("Stack:\n"); > for (i=1; i<10 && stack < stack_limit; i++) { > - while(stack < stack_limit && stack < stack_pointer(ctx) + i*32) { > + int j = 0; > + int k; > + > + while(stack < stack_limit && stack < stack_pointer(ctx) + > i*BYTES_PER_LINE) { > p = map_page(ctx, vcpu, stack); > if (!p) > return -1; > word = read_stack_word(p, width); > + ascii[j++] = word; > printf(" "); > print_stack_word(word, width); > stack += width; > } > + printf(" "); > + for (k = j; k < BYTES_PER_LINE/width; k++) > + printf("%s ", width == 8 > + ? " " > + : " "); > + for (k = 0; k < j; k++) { > + int l; > + > + bytep = (unsigned char*)&ascii[k]; > + for (l = 0; l < width; l++) { > + if ((*bytep < 127) && (*bytep >= 32)) > + printf("%c", *bytep); > + else > + printf("."); > + bytep++; > + } > + } > printf("\n"); > } > printf("\n"); > -- > 1.7.1
Jan Beulich
2013-Nov-07 08:12 UTC
Re: [PATCH v2 04/12] xenctx: Add stack address to dump, switch to 16 bytes per line.
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote:Again - why? This makes it just two entries per line for a 64-bit guest. Pretty little I would say. Jan> From: Don Slutz <Don@CloudSwitch.com> > > Signed-off-by: Don Slutz <Don@CloudSwitch.com> > --- > tools/xentrace/xenctx.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index dabce16..a20281e 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -640,7 +640,7 @@ static int print_code(vcpu_guest_context_any_t *ctx, int > vcpu) > return 0; > } > > -#define BYTES_PER_LINE 32 > +#define BYTES_PER_LINE 16 > > static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > { > @@ -663,6 +663,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int > vcpu, int width) > int j = 0; > int k; > > + print_stack_word(stack, width); > + printf(":"); > while(stack < stack_limit && stack < stack_pointer(ctx) + > i*BYTES_PER_LINE) { > p = map_page(ctx, vcpu, stack); > if (!p) > -- > 1.7.1
Jan Beulich
2013-Nov-07 08:13 UTC
Re: [PATCH v2 05/12] xenctx: Change print_symbol to do the space before.
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > From: Don Slutz <Don@CloudSwitch.com> > > This stops the output of an extra space at the end of the line. > > Signed-off-by: Don Slutz <Don@CloudSwitch.com>Reviewed-by: Jan Beulich <jbeulich@suse.com>> --- > tools/xentrace/xenctx.c | 20 ++++++++++---------- > 1 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index a20281e..5e2354c 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -158,9 +158,9 @@ static void print_symbol(guest_word_t addr) > return; > > if (addr==s->address) > - printf("%s ", s->name); > + printf(" %s", s->name); > else > - printf("%s+%#x ", s->name, (unsigned int)(addr - s->address)); > + printf(" %s+%#x", s->name, (unsigned int)(addr - s->address)); > } > > static void read_symbol_table(const char *symtab) > @@ -287,7 +287,7 @@ static void print_ctx_32(vcpu_guest_context_x86_32_t > *ctx) > { > struct cpu_user_regs_x86_32 *regs = &ctx->user_regs; > > - printf("cs:eip: %04x:%08x ", regs->cs, regs->eip); > + printf("cs:eip: %04x:%08x", regs->cs, regs->eip); > print_symbol(regs->eip); > print_flags(regs->eflags); > printf("ss:esp: %04x:%08x\n", regs->ss, regs->esp); > @@ -316,7 +316,7 @@ static void print_ctx_32on64(vcpu_guest_context_x86_64_t > *ctx) > { > struct cpu_user_regs_x86_64 *regs = &ctx->user_regs; > > - printf("cs:eip: %04x:%08x ", regs->cs, (uint32_t)regs->eip); > + printf("cs:eip: %04x:%08x", regs->cs, (uint32_t)regs->eip); > print_symbol((uint32_t)regs->eip); > print_flags((uint32_t)regs->eflags); > printf("ss:esp: %04x:%08x\n", regs->ss, (uint32_t)regs->esp); > @@ -345,7 +345,7 @@ static void print_ctx_64(vcpu_guest_context_x86_64_t > *ctx) > { > struct cpu_user_regs_x86_64 *regs = &ctx->user_regs; > > - printf("rip: %016"PRIx64" ", regs->rip); > + printf("rip: %016"PRIx64, regs->rip); > print_symbol(regs->rip); > print_flags(regs->rflags); > printf("rsp: %016"PRIx64"\n", regs->rsp); > @@ -443,7 +443,7 @@ static void print_ctx_32(vcpu_guest_context_t *ctx) > { > vcpu_guest_core_regs_t *regs = &ctx->user_regs; > > - printf("PC: %08"PRIx32" ", regs->pc32); > + printf("PC: %08"PRIx32, regs->pc32); > print_symbol(regs->pc32); > printf("\n"); > printf("CPSR: %08"PRIx32"\n", regs->cpsr); > @@ -495,7 +495,7 @@ static void print_ctx_64(vcpu_guest_context_t *ctx) > { > vcpu_guest_core_regs_t *regs = &ctx->user_regs; > > - printf("PC: %016"PRIx64" ", regs->pc64); > + printf("PC: %016"PRIx64, regs->pc64); > print_symbol(regs->pc64); > printf("\n"); > > @@ -702,7 +702,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int > vcpu, int width) > printf("Call Trace:\n"); > printf("%c [<", xenctx.stack_trace ? ''*'' : '' ''); > print_stack_word(instr_pointer(ctx), width); > - printf(">] "); > + printf(">]"); > > print_symbol(instr_pointer(ctx)); > printf(" <--\n"); > @@ -742,7 +742,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int > vcpu, int width) > word = read_stack_word(p, width); > printf("%c [<", xenctx.stack_trace ? ''|'' : '' ''); > print_stack_word(word, width); > - printf(">] "); > + printf(">]"); > print_symbol(word); > printf("\n"); > stack += width; > @@ -758,7 +758,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int > vcpu, int width) > if (is_kernel_text(word)) { > printf(" [<"); > print_stack_word(word, width); > - printf(">] "); > + printf(">]"); > print_symbol(word); > printf("\n"); > } else if (xenctx.stack_trace) { > -- > 1.7.1
Ian Campbell
2013-Nov-07 08:22 UTC
Re: [PATCH v2 01/12] xenctx: Clean up stack trace when hypercall_page not in symbol table
On Thu, 2013-11-07 at 08:07 +0000, Jan Beulich wrote:> And a note on patch submission: You send _to_ the list, and _cc_ > maintainers and other relevant people (without stretching the > meaning of "relevant" too much).Do we need such a rule? Why is it useful? I personally don''t mind which field causes the mail to arrive in my INBOX. I suppose you filter "To: xen-devel; Cc: Jan" differently to "To: Jan; Cc: xen-devel"? http://wiki.xenproject.org/wiki/Submitting_Xen_Patches#Cc_the_maintainer_of_the_code_you_are_modifying is confusingly worded and somewhat contradictory, in that it initially requests to send to the maintainer and then later to cc them. As I say, I don''t think it really matters which but if you care then it would be useful to clarify the wording there. Ian.
Jan Beulich
2013-Nov-07 08:22 UTC
Re: [PATCH v2 08/12] xenctx: Add -d <daddr> option to dump memory at daddr as a stack.
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > @@ -956,9 +973,9 @@ int main(int argc, char **argv) > } > > if (xenctx.all_vcpus) > - dump_all_vcpus(); > + dump_all_vcpus(stk_addr); > else > - dump_ctx(vcpu); > + dump_ctx(vcpu, stk_addr);For one, it hardly makes much sense to dump all vCPU-s with a single stack address. And assuming a stack can''t sit at address zero is wrong too - you shouldn''t make any assumptions in particular for HVM guests. So what you intend here should lead to just the requested "stack" to be printed, without touching other functionality (i.e. likely you''ll want the "if()" above preceded by another one, and itself converted to an "else if()"). Jan
Jan Beulich
2013-Nov-07 08:25 UTC
Re: [PATCH v2 09/12] xenctx: Add -m <maddr> option to dump memory at maddr.
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > @@ -973,9 +1065,9 @@ int main(int argc, char **argv) > } > > if (xenctx.all_vcpus) > - dump_all_vcpus(stk_addr); > + dump_all_vcpus(mem_addr, stk_addr); > else > - dump_ctx(vcpu, stk_addr); > + dump_ctx(vcpu, mem_addr, stk_addr);Same comment as on the previous patch - just dump the requested memory here, without modifying other behavior. Jan
Jan Beulich
2013-Nov-07 08:35 UTC
Re: [PATCH v2 10/12] xenctx: change is_kernel_text() into is_kernel_addr().
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > @@ -76,22 +86,28 @@ unsigned long long kernel_start = 0xc0000000; > unsigned long long kernel_start = 0xffffffff80000000UL; > #endif > > -static int is_kernel_text(guest_word_t addr) > +static type_of_addr is_kernel_addr(guest_word_t addr)The "is_" prefix is now bogus.> { > - if (symbol_table == NULL) > - return (addr > kernel_start); > + if (symbol_table == NULL) { > + if (addr > kernel_start) > + return KERNEL_TEXT_ADDR; > + else > + return NOT_KERNEL_ADDR; > + } > > if (addr >= kernel_stext && > addr <= kernel_etext) > - return 1; > - if (kernel_hypercallpage && > - (addr >= kernel_hypercallpage && > - addr <= kernel_hypercallpage + 4096)) > - return 1; > + return KERNEL_TEXT_ADDR; > + if (kernel_hypercallpage && (addr >= kernel_hypercallpage && > + addr <= kernel_hypercallpage + 4096))This reformatting is pointlessly bloating the patch - if you want the line reformatted, you should (a) do this in the patch that adds the first part of the condition and (b) obey to indentation rules.> + return KERNEL_TEXT_ADDR; > if (addr >= kernel_sinittext && > addr <= kernel_einittext) > - return 1; > - return 0; > + return KERNEL_TEXT_ADDR; > + if (addr >= kernel_text && > + addr <= kernel_end) > + return KERNEL_DATA_ADDR;As you supposedly filtered out all text ranges before, did you really mean to compare to kernel_text here (rather than kernel_start)?> @@ -749,7 +769,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest > void *p = map_page(ctx, vcpu, stack); > if (!p) > return -1; > - word = read_stack_word(p, width); > + word = read_mem_word(ctx, vcpu, stack, width);How is this change related to the purpose of the patch?> @@ -818,7 +838,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest > if (xenctx.stack_trace) { > print_stack_word(stack, width); > printf(": |-- "); > - print_stack_word(read_stack_word(p, width), width); > + print_stack_word(frame, width);And this one?> @@ -843,13 +863,13 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest > void *p = map_page(ctx, vcpu, stack); > if (!p) > return -1; > - word = read_stack_word(p, width); > - if (is_kernel_text(word)) { > + word = read_mem_word(ctx, vcpu, stack, width); > + if (is_kernel_addr(word) >= KERNEL_TEXT_ADDR) {And one more. Jan
Jan Beulich
2013-Nov-07 08:38 UTC
Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > @@ -931,7 +1174,44 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) > #endif > } else { > if (!stk_addr) { > - print_ctx(&ctx); > +#if defined(__i386__) || defined(__x86_64__) > + if (xenctx.dominfo.hvm && ctxt_word_size == 8) { > + if (guest_word_size == 4) { > + if ((((uint32_t)ctx.x64.user_regs.eip) != cpuctx.rip) || > + (((uint32_t)ctx.x64.user_regs.esp) != cpuctx.rsp) || > + (((uint32_t)ctx.x64.ctrlreg[3]) != cpuctx.cr3)) { > + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", > + (long long)((uint32_t)ctx.x64.user_regs.eip), > + (long long)cpuctx.rip, > + (long long)((uint32_t)ctx.x64.user_regs.esp), > + (long long)cpuctx.rsp, > + (long long)((uint32_t)ctx.x64.ctrlreg[3]), > + (long long)cpuctx.cr3); > + fprintf(stdout, "=============Regs mismatch start=============\n"); > + print_ctx(&ctx); > + fprintf(stdout, "=============Regs mismatch end=============\n"); > + } > + } else { > + if ((ctx.x64.user_regs.eip != cpuctx.rip) || > + (ctx.x64.user_regs.esp != cpuctx.rsp) || > + (ctx.x64.ctrlreg[3] != cpuctx.cr3)) { > + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", > + (long long)ctx.x64.user_regs.eip, > + (long long)cpuctx.rip, > + (long long)ctx.x64.user_regs.esp, > + (long long)cpuctx.rsp, > + (long long)ctx.x64.ctrlreg[3], > + (long long)cpuctx.cr3); > + fprintf(stdout, "=============Regs mismatch start=============\n"); > + print_ctx(&ctx); > + fprintf(stdout, "=============Regs mismatch end=============\n"); > + } > + } > + print_cpuctx(&cpuctx); > + } > + else > +#endif > + print_ctx(&ctx);Apart from Andrew''s comments, which I agree with - most of the additions above clearly don''t belong here: This is not a diagnostic utility. Jan
>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > This can be used to dump the normal ctx for an HVM DomU by passing > -1 to fCPU. > > If only some of the vCPUs in a HVM DomU, you need to ask for ask for > the on line CPU number, not the expected vCPU number; fCPU can be > used for this.I''m sorry, but I can''t make sense of this description, and hence can''t review the patch. Jan
Jan Beulich
2013-Nov-07 08:47 UTC
Re: [PATCH v2 01/12] xenctx: Clean up stack trace when hypercall_page not in symbol table
>>> On 07.11.13 at 09:22, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2013-11-07 at 08:07 +0000, Jan Beulich wrote: >> And a note on patch submission: You send _to_ the list, and _cc_ >> maintainers and other relevant people (without stretching the >> meaning of "relevant" too much). > > Do we need such a rule? Why is it useful? > > I personally don''t mind which field causes the mail to arrive in my > INBOX. I suppose you filter "To: xen-devel; Cc: Jan" differently to "To: > Jan; Cc: xen-devel"?Exactly. Mails Cc-ed to me get treated almost equally to all other xen-devel traffic, whereas mails directed at me mean to me that a response or other kind of action is expected. Basically the usual (common sense?) email rules... So maybe I was too strict about the maintainer part - I don''t mind being sent mails to me that fall under the areas I''m maintainer of, since there I''m obviously requested to take some sort of action. But in the case here I certainly feel it was wrong to send the whole series _to_ everyone.> http://wiki.xenproject.org/wiki/Submitting_Xen_Patches#Cc_the_maintainer_of_ > the_code_you_are_modifying is confusingly worded and somewhat contradictory, > in that it initially requests to send to the maintainer and then later to cc > them. As I say, I don''t think it really matters which but if you care then it > would be useful to clarify the wording there.Yes, I think we should. Jan
Ian Campbell
2013-Nov-07 12:36 UTC
Re: [PATCH v2 01/12] xenctx: Clean up stack trace when hypercall_page not in symbol table
On Thu, 2013-11-07 at 08:47 +0000, Jan Beulich wrote:> >>> On 07.11.13 at 09:22, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Thu, 2013-11-07 at 08:07 +0000, Jan Beulich wrote: > >> And a note on patch submission: You send _to_ the list, and _cc_ > >> maintainers and other relevant people (without stretching the > >> meaning of "relevant" too much). > > > > Do we need such a rule? Why is it useful? > > > > I personally don''t mind which field causes the mail to arrive in my > > INBOX. I suppose you filter "To: xen-devel; Cc: Jan" differently to "To: > > Jan; Cc: xen-devel"? > > Exactly. Mails Cc-ed to me get treated almost equally to all other > xen-devel traffic, whereas mails directed at me mean to me that > a response or other kind of action is expected. Basically the > usual (common sense?) email rules... > > So maybe I was too strict about the maintainer part - I don''t mind > being sent mails to me that fall under the areas I''m maintainer of, > since there I''m obviously requested to take some sort of action. > But in the case here I certainly feel it was wrong to send the > whole series _to_ everyone.Ah, yes that makes sense, I hadn''t noticed that bit.> > http://wiki.xenproject.org/wiki/Submitting_Xen_Patches#Cc_the_maintainer_of_ > > the_code_you_are_modifying is confusingly worded and somewhat contradictory, > > in that it initially requests to send to the maintainer and then later to cc > > them. As I say, I don''t think it really matters which but if you care then it > > would be useful to clarify the wording there. > > Yes, I think we should.
Ian Campbell
2013-Nov-07 12:38 UTC
Re: [PATCH v2 01/12] xenctx: Clean up stack trace when hypercall_page not in symbol table
On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote:> From: Don Slutz <Don@CloudSwitch.com> > > Before: > > Call Trace: > [<ffffffff8006b2b0>] default_idle+0x29 <-- > [<ffffffff80048d19>] cpu_idle+0x95 > [<ffffffff803e7801>] start_kernel+0x220 > [<0000000000000000>] startup_64+0x80000000 > [<ffffffff803e722f>] x86_64_start_kernel+0x22f > [<0000000000000000>] startup_64+0x80000000 > [<0000000000000000>] startup_64+0x80000000 > [<0000000000000000>] startup_64+0x80000000 > [<0000000000000000>] startup_64+0x80000000 > > After: > > Call Trace: > [<ffffffff8006b2b0>] default_idle+0x29 <-- > [<ffffffff80048d19>] cpu_idle+0x95 > [<ffffffff803e7801>] start_kernel+0x220 > [<ffffffff803e722f>] x86_64_start_kernel+0x22f > > Signed-off-by: Don Slutz <Don@CloudSwitch.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Campbell
2013-Nov-07 12:40 UTC
Re: [PATCH v2 02/12] xenctx: Add -2 (--two-pages) option to switch stack size to 8KiB
On Thu, 2013-11-07 at 08:04 +0000, Jan Beulich wrote:> >>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > > @@ -650,9 +651,11 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > > > > stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE) > > & ~((guest_word_t) XC_PAGE_SIZE - 1)); > > + if (xenctx.two_pages) > > + stack_limit += XC_PAGE_SIZE; > > printf("\n"); > > printf("Stack:\n"); > > - for (i=1; i<5 && stack < stack_limit; i++) { > > + for (i=1; i<10 && stack < stack_limit; i++) { > > Touching a malformed line like this makes it almost mandatory imo > to fix the coding style (missing blanks around = and <). > > Apart from that, the change isn''t really related to the subject of > the patch afaict, and as you''re making thing more flexible anyway, > it would seem reasonable to also have a command line option to > control this limit.Or derive it from stack_limit and/or the number of pages? What about kernels which use 4 pages for their stack ;-) Ian.
Ian Campbell
2013-Nov-07 12:43 UTC
Re: [PATCH v2 03/12] xenctx: Output ascii version of stack also.
On Thu, 2013-11-07 at 08:09 +0000, Jan Beulich wrote:> >>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > > Why? I personally don''t see a need - there''s rarely much useful > ASCII data on the stack.This is true IME> So afaic: If at all, then only via command > line option defaulting to off.Well, assuming it doesn''t make the output >80 columns (which I can''t tell from here) it seems harmless enough?> "); > > + for (k = 0; k < j; k++) { > > + int l; > > + > > + bytep = (unsigned char*)&ascii[k]; > > + for (l = 0; l < width; l++) { > > + if ((*bytep < 127) && (*bytep >= 32))is there not a ctype.h function which deals with this? isprint seems like a good candidate...> > + printf("%c", *bytep); > > + else > > + printf("."); > > + bytep++; > > + } > > + } > > printf("\n"); > > } > > printf("\n"); > > -- > > 1.7.1 > > >
Ian Campbell
2013-Nov-07 12:46 UTC
Re: [PATCH v2 04/12] xenctx: Add stack address to dump, switch to 16 bytes per line.
On Thu, 2013-11-07 at 08:12 +0000, Jan Beulich wrote:> >>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > > Again - why?Having the stack address there can make it easier to locate a word referenced by a register for example, without error prone counting. In general many of the changelogs in this series could do with a second sentence in them after the title giving some rationale so we don''t have to guess.> This makes it just two entries per line for a 64-bit > guest. Pretty little I would say.Agreed. I expect that along with the previous patch lines would now be over 80 columns for sure. If that''s the case then either the ascii or the address (or both) needs to be optional IMHO.> > Jan > > > From: Don Slutz <Don@CloudSwitch.com> > > > > Signed-off-by: Don Slutz <Don@CloudSwitch.com> > > --- > > tools/xentrace/xenctx.c | 4 +++- > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > > index dabce16..a20281e 100644 > > --- a/tools/xentrace/xenctx.c > > +++ b/tools/xentrace/xenctx.c > > @@ -640,7 +640,7 @@ static int print_code(vcpu_guest_context_any_t *ctx, int > > vcpu) > > return 0; > > } > > > > -#define BYTES_PER_LINE 32 > > +#define BYTES_PER_LINE 16 > > > > static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > > { > > @@ -663,6 +663,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int > > vcpu, int width) > > int j = 0; > > int k; > > > > + print_stack_word(stack, width); > > + printf(":"); > > while(stack < stack_limit && stack < stack_pointer(ctx) + > > i*BYTES_PER_LINE) { > > p = map_page(ctx, vcpu, stack); > > if (!p) > > -- > > 1.7.1 > > >
Ian Campbell
2013-Nov-07 12:47 UTC
Re: [PATCH v2 05/12] xenctx: Change print_symbol to do the space before.
On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote:> From: Don Slutz <Don@CloudSwitch.com> > > This stops the output of an extra space at the end of the line. > > Signed-off-by: Don Slutz <Don@CloudSwitch.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Campbell
2013-Nov-07 12:48 UTC
Re: [PATCH v2 06/12] xenctx: More info on failed to map page.
On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote:> From: Don Slutz <Don@CloudSwitch.com> > > Also output an extra new line since we may be in the middle of output. > > Signed-off-by: Don Slutz <Don@CloudSwitch.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Campbell
2013-Nov-07 12:50 UTC
Re: [PATCH v2 07/12] xenctx: Add stack addr to call trace.
On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote:> From: Don Slutz <Don@CloudSwitch.com>Can you give an example of the output please. What is "stack address" is it the base of the function''s stack frame perhaps? Or maybe the top? Or maybe the framepointer?> > Signed-off-by: Don Slutz <Don@CloudSwitch.com> > --- > tools/xentrace/xenctx.c | 20 ++++++++++++++------ > 1 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index dcf431c..4dc6574 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -700,6 +700,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > printf("Stack Trace:\n"); > else > printf("Call Trace:\n"); > + printf("%s ", width == 8 > + ? " " > + : " ");I think this can be done as ("%*s", width*2, "") or something roughly like that.> printf("%c [<", xenctx.stack_trace ? ''*'' : '' ''); > print_stack_word(instr_pointer(ctx), width); > printf(">]"); > @@ -715,9 +718,10 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > p = map_page(ctx, vcpu, stack); > if (!p) > return -1; > - printf("| "); > + print_stack_word(stack, width); > + printf(": | "); > print_stack_word(read_stack_word(p, width), width); > - printf(" \n"); > + printf("\n"); > stack += width; > } > } else { > @@ -729,7 +733,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > return -1; > frame = read_stack_word(p, width); > if (xenctx.stack_trace) { > - printf("|-- "); > + print_stack_word(stack, width); > + printf(": |-- "); > print_stack_word(read_stack_word(p, width), width); > printf("\n"); > } > @@ -740,7 +745,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > if (!p) > return -1; > word = read_stack_word(p, width); > - printf("%c [<", xenctx.stack_trace ? ''|'' : '' ''); > + print_stack_word(stack, width); > + printf(": %c [<", xenctx.stack_trace ? ''|'' : '' ''); > print_stack_word(word, width); > printf(">]"); > print_symbol(word); > @@ -756,13 +762,15 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) > return -1; > word = read_stack_word(p, width); > if (is_kernel_text(word)) { > - printf(" [<"); > + print_stack_word(stack, width); > + printf(": [<"); > print_stack_word(word, width); > printf(">]"); > print_symbol(word); > printf("\n"); > } else if (xenctx.stack_trace) { > - printf(" "); > + print_stack_word(stack, width); > + printf(": "); > print_stack_word(word, width); > printf("\n"); > }
On Thu, 2013-11-07 at 08:40 +0000, Jan Beulich wrote:> >>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > > This can be used to dump the normal ctx for an HVM DomU by passing > > -1 to fCPU. > > > > If only some of the vCPUs in a HVM DomU, you need to ask for ask for > > the on line CPU number, not the expected vCPU number; fCPU can be > > used for this. > > I''m sorry, but I can''t make sense of this description, and hence > can''t review the patch.Yeah, I''m wondering what the f stands for. Ian
Jan Beulich
2013-Nov-07 13:17 UTC
Re: [PATCH v2 03/12] xenctx: Output ascii version of stack also.
>>> On 07.11.13 at 13:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2013-11-07 at 08:09 +0000, Jan Beulich wrote: >> >>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >> >> Why? I personally don''t see a need - there''s rarely much useful >> ASCII data on the stack. > > This is true IME > >> So afaic: If at all, then only via command >> line option defaulting to off. > > Well, assuming it doesn''t make the output >80 columns (which I can''t > tell from here) it seems harmless enough?If you end up looking at the output in a wide enough window, it''ll make you scroll more. Jan
Jan Beulich
2013-Nov-07 13:21 UTC
Re: [PATCH v2 03/12] xenctx: Output ascii version of stack also.
>>> On 07.11.13 at 14:17, "Jan Beulich" <JBeulich@suse.com> wrote: >>>> On 07.11.13 at 13:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> On Thu, 2013-11-07 at 08:09 +0000, Jan Beulich wrote: >>> >>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >>> >>> Why? I personally don''t see a need - there''s rarely much useful >>> ASCII data on the stack. >> >> This is true IME >> >>> So afaic: If at all, then only via command >>> line option defaulting to off. >> >> Well, assuming it doesn''t make the output >80 columns (which I can''t >> tell from here) it seems harmless enough? > > If you end up looking at the output in a wide enough window, it''ll > make you scroll more.Oops, I was mentally in patch 04 here. So yes, it may be harmless, but clutters the data that you need to look at, and I personally prefer to look at _just_ the data that''s interesting, without having to (mentally or physically) discard other pieces. Jan
Don Slutz
2013-Nov-07 13:28 UTC
Re: [PATCH v2 01/12] xenctx: Clean up stack trace when hypercall_page not in symbol table
On 11/07/13 07:36, Ian Campbell wrote:> On Thu, 2013-11-07 at 08:47 +0000, Jan Beulich wrote: >>>>> On 07.11.13 at 09:22, Ian Campbell <Ian.Campbell@citrix.com> wrote: >>> On Thu, 2013-11-07 at 08:07 +0000, Jan Beulich wrote: >>>> And a note on patch submission: You send _to_ the list, and _cc_ >>>> maintainers and other relevant people (without stretching the >>>> meaning of "relevant" too much). >>> Do we need such a rule? Why is it useful? >>> >>> I personally don''t mind which field causes the mail to arrive in my >>> INBOX. I suppose you filter "To: xen-devel; Cc: Jan" differently to "To: >>> Jan; Cc: xen-devel"? >> Exactly. Mails Cc-ed to me get treated almost equally to all other >> xen-devel traffic, whereas mails directed at me mean to me that >> a response or other kind of action is expected. Basically the >> usual (common sense?) email rules... >> >> So maybe I was too strict about the maintainer part - I don''t mind >> being sent mails to me that fall under the areas I''m maintainer of, >> since there I''m obviously requested to take some sort of action. >> But in the case here I certainly feel it was wrong to send the >> whole series _to_ everyone. > Ah, yes that makes sense, I hadn''t noticed that bit.Well, I was confused. get_maintainer.pl reports that George Dunlap <george.dunlap@eu.citrix.com> (supporter:XENTRACE) Ian Jackson <ian.jackson@eu.citrix.com> (supporter:TOOLSTACK) Stefano Stabellini <stefano.stabellini@eu.citrix.com> (supporter:TOOLSTACK) Ian Campbell <ian.campbell@citrix.com> (supporter:TOOLSTACK) From MAINTAINERS: M: Mail patches to: FullName <address@domain> and I followed that. It was months ago that I read over the wiki page. Jan came from: Subject: Re: [Xen-devel] [PATCH] xenctx: Add an option to output more registers. Date: Fri, 18 Oct 2013 09:00:05 +0100 From: Jan Beulich <JBeulich@suse.com> To: Don Slutz <dslutz@verizon.com> CC: Ian Campbell <ian.campbell@citrix.com>, Don Slutz <Don@CloudSwitch.com>, George Dunlap <george.dunlap@eu.citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, xen-devel <xen-devel@lists.xenproject.org> >>> On 17.10.13 at 20:41, Don Slutz <dslutz@verizon.com> wrote: > From: Don Slutz <Don@CloudSwitch.com> > > Also fixup handling of symbol files, and output of 2 page stacks, and output > of non stack memory. Considering the size of the change this should be broken up, and the individual changes need to be described better in the commit message. [...] and my understanding was that re-worked patches need to have --to=<responder> added. One on the things that is not clear is do I change from --cc= to --to=after a response?>>> http://wiki.xenproject.org/wiki/Submitting_Xen_Patches#Cc_the_maintainer_of_ >>> the_code_you_are_modifying is confusingly worded and somewhat contradictory, >>> in that it initially requests to send to the maintainer and then later to cc >>> them. As I say, I don''t think it really matters which but if you care then it >>> would be useful to clarify the wording there. >> Yes, I think we should.I would agree. I would also change the wording in MAINTAINERS. -Don Slutz --------------050208010407090402030903 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 8bit <html> <head> <meta content="text/html; charset=UTF-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 11/07/13 07:36, Ian Campbell wrote:<br> </div> <blockquote cite="mid:1383827816.32399.20.camel@kazak.uk.xensource.com" type="cite"> <pre wrap="">On Thu, 2013-11-07 at 08:47 +0000, Jan Beulich wrote: </pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">On 07.11.13 at 09:22, Ian Campbell <a class="moz-txt-link-rfc2396E" href="mailto:Ian.Campbell@citrix.com"><Ian.Campbell@citrix.com></a> wrote: </pre> </blockquote> </blockquote> <pre wrap="">On Thu, 2013-11-07 at 08:07 +0000, Jan Beulich wrote: </pre> <blockquote type="cite"> <pre wrap="">And a note on patch submission: You send _to_ the list, and _cc_ maintainers and other relevant people (without stretching the meaning of "relevant" too much). </pre> </blockquote> <pre wrap=""> Do we need such a rule? Why is it useful? I personally don''t mind which field causes the mail to arrive in my INBOX. I suppose you filter "To: xen-devel; Cc: Jan" differently to "To: Jan; Cc: xen-devel"? </pre> </blockquote> <pre wrap=""> Exactly. Mails Cc-ed to me get treated almost equally to all other xen-devel traffic, whereas mails directed at me mean to me that a response or other kind of action is expected. Basically the usual (common sense?) email rules... So maybe I was too strict about the maintainer part - I don''t mind being sent mails to me that fall under the areas I''m maintainer of, since there I''m obviously requested to take some sort of action. But in the case here I certainly feel it was wrong to send the whole series _to_ everyone. </pre> </blockquote> <pre wrap=""> Ah, yes that makes sense, I hadn''t noticed that bit. </pre> </blockquote> Well, I was confused. get_maintainer.pl reports that<br> <br> George Dunlap <a class="moz-txt-link-rfc2396E" href="mailto:george.dunlap@eu.citrix.com"><george.dunlap@eu.citrix.com></a> (supporter:XENTRACE)<br> Ian Jackson <a class="moz-txt-link-rfc2396E" href="mailto:ian.jackson@eu.citrix.com"><ian.jackson@eu.citrix.com></a> (supporter:TOOLSTACK)<br> Stefano Stabellini <a class="moz-txt-link-rfc2396E" href="mailto:stefano.stabellini@eu.citrix.com"><stefano.stabellini@eu.citrix.com></a> (supporter:TOOLSTACK)<br> Ian Campbell <a class="moz-txt-link-rfc2396E" href="mailto:ian.campbell@citrix.com"><ian.campbell@citrix.com></a> (supporter:TOOLSTACK)<br> <br> From MAINTAINERS:<br> <br> M: Mail patches to: FullName <address@domain><br> <br> and I followed that. It was months ago that I read over the wiki page.<br> <br> Jan came from:<br> <blockquote> <table class="moz-email-headers-table" border="0" cellpadding="0" cellspacing="0"> <tbody> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Subject: </th> <td>Re: [Xen-devel] [PATCH] xenctx: Add an option to output more registers.</td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Date: </th> <td>Fri, 18 Oct 2013 09:00:05 +0100</td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">From: </th> <td>Jan Beulich <a class="moz-txt-link-rfc2396E" href="mailto:JBeulich@suse.com"><JBeulich@suse.com></a></td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">To: </th> <td>Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:dslutz@verizon.com"><dslutz@verizon.com></a></td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">CC: </th> <td>Ian Campbell <a class="moz-txt-link-rfc2396E" href="mailto:ian.campbell@citrix.com"><ian.campbell@citrix.com></a>, Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:Don@CloudSwitch.com"><Don@CloudSwitch.com></a>, George Dunlap <a class="moz-txt-link-rfc2396E" href="mailto:george.dunlap@eu.citrix.com"><george.dunlap@eu.citrix.com></a>, Ian Jackson <a class="moz-txt-link-rfc2396E" href="mailto:ian.jackson@eu.citrix.com"><ian.jackson@eu.citrix.com></a>, Stefano Stabellini <a class="moz-txt-link-rfc2396E" href="mailto:stefano.stabellini@eu.citrix.com"><stefano.stabellini@eu.citrix.com></a>, xen-devel <a class="moz-txt-link-rfc2396E" href="mailto:xen-devel@lists.xenproject.org"><xen-devel@lists.xenproject.org></a></td> </tr> </tbody> </table> <br> <br> <pre>>>> On 17.10.13 at 20:41, Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:dslutz@verizon.com"><dslutz@verizon.com></a> wrote: > From: Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:Don@CloudSwitch.com"><Don@CloudSwitch.com></a> > > Also fixup handling of symbol files, and output of 2 page stacks, and output > of non stack memory. Considering the size of the change this should be broken up, and the individual changes need to be described better in the commit message. [...] </pre> </blockquote> and my understanding was that re-worked patches need to have --to=<responder> added. One on the things that is not clear is do I change from --cc= to --to=after a response?<br> <blockquote> </blockquote> <blockquote cite="mid:1383827816.32399.20.camel@kazak.uk.xensource.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap=""><a class="moz-txt-link-freetext" href="http://wiki.xenproject.org/wiki/Submitting_Xen_Patches#Cc_the_maintainer_of_">http://wiki.xenproject.org/wiki/Submitting_Xen_Patches#Cc_the_maintainer_of_</a> the_code_you_are_modifying is confusingly worded and somewhat contradictory, in that it initially requests to send to the maintainer and then later to cc them. As I say, I don''t think it really matters which but if you care then it would be useful to clarify the wording there. </pre> </blockquote> <pre wrap=""> Yes, I think we should.</pre> </blockquote> </blockquote> I would agree. I would also change the wording in MAINTAINERS.<br> <br> -Don Slutz<br> </body> </html> --------------050208010407090402030903-- --===============3778525012181304035=Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3778525012181304035==--
Don Slutz
2013-Nov-07 14:34 UTC
Re: [PATCH v2 07/12] xenctx: Add stack addr to call trace.
On 11/07/13 07:50, Ian Campbell wrote:> On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote: >> From: Don Slutz <Don@CloudSwitch.com> > Can you give an example of the output please.Here it is (With all patches active): dcs-xen-54:~/xen>sudo /home/don/xen/dist/install/usr/lib/xen/bin/xenctx -s /boot/System.map-2.6.18-128.el5 2 rip: ffffffff8006b2b0 default_idle+0x29 flags: 00000246 i z p rsp: ffffffff803ddf90 rax: 0000000000000000 rcx: 0000000000000000 rdx: 0000000000000000 rbx: ffffffff8006b287 rsi: 0000000000000001 rdi: ffffffff802f0658 rbp: 0000000000086800 r8: ffffffff803dc000 r9: 000000000000003f r10: ffff81017d2437c0 r11: 0000000000000282 r12: 0000000000000000 r13: 0000000000000000 r14: 0000000000000000 r15: 0000000000000000 cs: 0010 @ 0000000000000000 /ffffffff(a9b) ss: 0018 @ 0000000000000000 /ffffffff(c93) ds: 0018 @ 0000000000000000 /ffffffff(c93) es: 0018 @ 0000000000000000 /ffffffff(c93) fs: 0000 @ 0000000000000000 /ffffffff(c00) gs: 0000 @ ffffffff803ac000\0000000000000000 boot_cpu_pda\ /ffffffff(c00) Code (instr addr ffffffff8006b2b0) 65 48 8b 04 25 10 00 00 00 8b 80 38 e0 ff ff a8 08 75 04 fb f4 <eb> 01 fb 65 48 8b 04 25 10 00 00 Stack: ffffffff803ddf90: ffffffff80048d19 0000000000200800 .......... ..... ffffffff803ddfa0: ffffffff803e7801 0000000000086800 .x>......h...... ffffffff803ddfb0: 0000000000000000 ffffffff80430720 ........ .C..... ffffffff803ddfc0: ffffffff803e722f 80008e000010019c /r>............. ffffffff803ddfd0: 00000000ffffffff 0000000000000000 ................ ffffffff803ddfe0: 0000000000000000 0000000000200000 .......... ..... ffffffff803ddff0: 0000000000000000 0000000000000000 ................ Call Trace: [<ffffffff8006b2b0>] default_idle+0x29 <-- ffffffff803ddf90: [<ffffffff80048d19>] cpu_idle+0x95 ffffffff803ddfa0: [<ffffffff803e7801>] start_kernel+0x220 ffffffff803ddfc0: [<ffffffff803e722f>] x86_64_start_kernel+0x22f> > What is "stack address" is it the base of the function''s stack frame > perhaps? Or maybe the top? Or maybe the framepointer?It is the address of the stack "word".>> Signed-off-by: Don Slutz <Don@CloudSwitch.com> >> --- >> tools/xentrace/xenctx.c | 20 ++++++++++++++------ >> 1 files changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c >> index dcf431c..4dc6574 100644 >> --- a/tools/xentrace/xenctx.c >> +++ b/tools/xentrace/xenctx.c >> @@ -700,6 +700,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) >> printf("Stack Trace:\n"); >> else >> printf("Call Trace:\n"); >> + printf("%s ", width == 8 >> + ? " " >> + : " "); > I think this can be done as ("%*s", width*2, "") or something roughly > like that.Will look into it. -Don Slutz>> printf("%c [<", xenctx.stack_trace ? ''*'' : '' ''); >> print_stack_word(instr_pointer(ctx), width); >> printf(">]"); >> @@ -715,9 +718,10 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) >> p = map_page(ctx, vcpu, stack); >> if (!p) >> return -1; >> - printf("| "); >> + print_stack_word(stack, width); >> + printf(": | "); >> print_stack_word(read_stack_word(p, width), width); >> - printf(" \n"); >> + printf("\n"); >> stack += width; >> } >> } else { >> @@ -729,7 +733,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) >> return -1; >> frame = read_stack_word(p, width); >> if (xenctx.stack_trace) { >> - printf("|-- "); >> + print_stack_word(stack, width); >> + printf(": |-- "); >> print_stack_word(read_stack_word(p, width), width); >> printf("\n"); >> } >> @@ -740,7 +745,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) >> if (!p) >> return -1; >> word = read_stack_word(p, width); >> - printf("%c [<", xenctx.stack_trace ? ''|'' : '' ''); >> + print_stack_word(stack, width); >> + printf(": %c [<", xenctx.stack_trace ? ''|'' : '' ''); >> print_stack_word(word, width); >> printf(">]"); >> print_symbol(word); >> @@ -756,13 +762,15 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) >> return -1; >> word = read_stack_word(p, width); >> if (is_kernel_text(word)) { >> - printf(" [<"); >> + print_stack_word(stack, width); >> + printf(": [<"); >> print_stack_word(word, width); >> printf(">]"); >> print_symbol(word); >> printf("\n"); >> } else if (xenctx.stack_trace) { >> - printf(" "); >> + print_stack_word(stack, width); >> + printf(": "); >> print_stack_word(word, width); >> printf("\n"); >> } >--------------000206070506010304020805 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 8bit <html> <head> <meta content="text/html; charset=UTF-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 11/07/13 07:50, Ian Campbell wrote:<br> </div> <blockquote cite="mid:1383828610.32399.32.camel@kazak.uk.xensource.com" type="cite"> <pre wrap="">On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote: </pre> <blockquote type="cite"> <pre wrap="">From: Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:Don@CloudSwitch.com"><Don@CloudSwitch.com></a> </pre> </blockquote> <pre wrap=""> Can you give an example of the output please.</pre> </blockquote> Here it is (With all patches active):<br> <blockquote><tt>dcs-xen-54:~/xen>sudo /home/don/xen/dist/install/usr/lib/xen/bin/xenctx -s /boot/System.map-2.6.18-128.el5 2</tt><tt><br> </tt><tt>rip: ffffffff8006b2b0 default_idle+0x29</tt><tt><br> </tt><tt>flags: 00000246 i z p</tt><tt><br> </tt><tt>rsp: ffffffff803ddf90</tt><tt><br> </tt><tt>rax: 0000000000000000 rcx: 0000000000000000 rdx: 0000000000000000</tt><tt><br> </tt><tt>rbx: ffffffff8006b287 rsi: 0000000000000001 rdi: ffffffff802f0658</tt><tt><br> </tt><tt>rbp: 0000000000086800 r8: ffffffff803dc000 r9: 000000000000003f</tt><tt><br> </tt><tt>r10: ffff81017d2437c0 r11: 0000000000000282 r12: 0000000000000000</tt><tt><br> </tt><tt>r13: 0000000000000000 r14: 0000000000000000 r15: 0000000000000000</tt><tt><br> </tt><tt> cs: 0010 @ 0000000000000000</tt><tt><br> </tt><tt> /ffffffff(a9b)</tt><tt><br> </tt><tt> ss: 0018 @ 0000000000000000</tt><tt><br> </tt><tt> /ffffffff(c93)</tt><tt><br> </tt><tt> ds: 0018 @ 0000000000000000</tt><tt><br> </tt><tt> /ffffffff(c93)</tt><tt><br> </tt><tt> es: 0018 @ 0000000000000000</tt><tt><br> </tt><tt> /ffffffff(c93)</tt><tt><br> </tt><tt> fs: 0000 @ 0000000000000000</tt><tt><br> </tt><tt> /ffffffff(c00)</tt><tt><br> </tt><tt> gs: 0000 @ ffffffff803ac000\0000000000000000 boot_cpu_pda\</tt><tt><br> </tt><tt> /ffffffff(c00)</tt><tt><br> </tt><tt>Code (instr addr ffffffff8006b2b0)</tt><tt><br> </tt><tt>65 48 8b 04 25 10 00 00 00 8b 80 38 e0 ff ff a8 08 75 04 fb f4 <eb> 01 fb 65 48 8b 04 25 10 00 00 </tt><tt><br> </tt><tt><br> </tt><tt><br> </tt><tt>Stack:</tt><tt><br> </tt><tt>ffffffff803ddf90: ffffffff80048d19 0000000000200800 .......... .....</tt><tt><br> </tt><tt>ffffffff803ddfa0: ffffffff803e7801 0000000000086800 .x>......h......</tt><tt><br> </tt><tt>ffffffff803ddfb0: 0000000000000000 ffffffff80430720 ........ .C.....</tt><tt><br> </tt><tt>ffffffff803ddfc0: ffffffff803e722f 80008e000010019c /r>.............</tt><tt><br> </tt><tt>ffffffff803ddfd0: 00000000ffffffff 0000000000000000 ................</tt><tt><br> </tt><tt>ffffffff803ddfe0: 0000000000000000 0000000000200000 .......... .....</tt><tt><br> </tt><tt>ffffffff803ddff0: 0000000000000000 0000000000000000 ................</tt><tt><br> </tt><tt><br> </tt><tt>Call Trace:</tt><tt><br> </tt><tt> [<ffffffff8006b2b0>] default_idle+0x29 <--</tt><tt><br> </tt><tt>ffffffff803ddf90: [<ffffffff80048d19>] cpu_idle+0x95</tt><tt><br> </tt><tt>ffffffff803ddfa0: [<ffffffff803e7801>] start_kernel+0x220</tt><tt><br> </tt><tt>ffffffff803ddfc0: [<ffffffff803e722f>] x86_64_start_kernel+0x22f</tt><tt><br> </tt><tt><br> </tt></blockquote> <blockquote cite="mid:1383828610.32399.32.camel@kazak.uk.xensource.com" type="cite"> <pre wrap=""> What is "stack address" is it the base of the function''s stack frame perhaps? Or maybe the top? Or maybe the framepointer? </pre> </blockquote> It is the address of the stack "word".<br> <blockquote cite="mid:1383828610.32399.32.camel@kazak.uk.xensource.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap=""> Signed-off-by: Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:Don@CloudSwitch.com"><Don@CloudSwitch.com></a> --- tools/xentrace/xenctx.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-) diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c index dcf431c..4dc6574 100644 --- a/tools/xentrace/xenctx.c +++ b/tools/xentrace/xenctx.c @@ -700,6 +700,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) printf("Stack Trace:\n"); else printf("Call Trace:\n"); + printf("%s ", width == 8 + ? " " + : " "); </pre> </blockquote> <pre wrap=""> I think this can be done as ("%*s", width*2, "") or something roughly like that. </pre> </blockquote> Will look into it.<br> -Don Slutz<br> <blockquote cite="mid:1383828610.32399.32.camel@kazak.uk.xensource.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap=""> printf("%c [<", xenctx.stack_trace ? ''*'' : '' ''); print_stack_word(instr_pointer(ctx), width); printf(">]"); @@ -715,9 +718,10 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) p = map_page(ctx, vcpu, stack); if (!p) return -1; - printf("| "); + print_stack_word(stack, width); + printf(": | "); print_stack_word(read_stack_word(p, width), width); - printf(" \n"); + printf("\n"); stack += width; } } else { @@ -729,7 +733,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) return -1; frame = read_stack_word(p, width); if (xenctx.stack_trace) { - printf("|-- "); + print_stack_word(stack, width); + printf(": |-- "); print_stack_word(read_stack_word(p, width), width); printf("\n"); } @@ -740,7 +745,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) if (!p) return -1; word = read_stack_word(p, width); - printf("%c [<", xenctx.stack_trace ? ''|'' : '' ''); + print_stack_word(stack, width); + printf(": %c [<", xenctx.stack_trace ? ''|'' : '' ''); print_stack_word(word, width); printf(">]"); print_symbol(word); @@ -756,13 +762,15 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) return -1; word = read_stack_word(p, width); if (is_kernel_text(word)) { - printf(" [<"); + print_stack_word(stack, width); + printf(": [<"); print_stack_word(word, width); printf(">]"); print_symbol(word); printf("\n"); } else if (xenctx.stack_trace) { - printf(" "); + print_stack_word(stack, width); + printf(": "); print_stack_word(word, width); printf("\n"); } </pre> </blockquote> <pre wrap=""> </pre> </blockquote> <br> </body> </html> --------------000206070506010304020805-- --===============7337615213434112852=Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7337615213434112852==--
Ian Campbell
2013-Nov-07 14:44 UTC
Re: [PATCH v2 07/12] xenctx: Add stack addr to call trace.
On Thu, 2013-11-07 at 09:34 -0500, Don Slutz wrote:> On 11/07/13 07:50, Ian Campbell wrote: > > > On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote: > > > From: Don Slutz <Don@CloudSwitch.com> > > Can you give an example of the output please. > Here it is (With all patches active): > cs: 0010 @ 0000000000000000 > /ffffffff(a9b)Is the ugly wrapping real or email whitespace damage?> ss: 0018 @ 0000000000000000 > /ffffffff(c93) > ds: 0018 @ 0000000000000000 > /ffffffff(c93) > es: 0018 @ 0000000000000000 > /ffffffff(c93) > fs: 0000 @ 0000000000000000 > /ffffffff(c00) > gs: 0000 @ ffffffff803ac000\0000000000000000 boot_cpu_pda\ > /ffffffff(c00)[...]> > > What is "stack address" is it the base of the function''s stack frame > > perhaps? Or maybe the top? Or maybe the framepointer?> It is the address of the stack "word".I''m afraid I am non the wiser. Ian.
Don Slutz
2013-Nov-07 15:06 UTC
Re: [PATCH v2 07/12] xenctx: Add stack addr to call trace.
On 11/07/13 09:44, Ian Campbell wrote:> On Thu, 2013-11-07 at 09:34 -0500, Don Slutz wrote: >> On 11/07/13 07:50, Ian Campbell wrote: >> >>> On Wed, 2013-11-06 at 15:08 -0500, Don Slutz wrote: >>>> From: Don Slutz <Don@CloudSwitch.com> >>> Can you give an example of the output please. >> Here it is (With all patches active): >> cs: 0010 @ 0000000000000000 >> /ffffffff(a9b) > Is the ugly wrapping real or email whitespace damage?Real. The way it currently looks is clearer with a symbol. Doing both the address and symbol and other data is not simple. I was trying to keep things aligned which is very hard with variable length output. xen-hvmctx''s output: cs 0x00000010 (0x0000000000000000 + 0xffffffff / 0x00a9b) Using + to indicate a limit is not so clear either. Things like idt are simpler. idt: ffffffff8042c000/fff idt_table>> ss: 0018 @ 0000000000000000 >> /ffffffff(c93) >> ds: 0018 @ 0000000000000000 >> /ffffffff(c93) >> es: 0018 @ 0000000000000000 >> /ffffffff(c93) >> fs: 0000 @ 0000000000000000 >> /ffffffff(c00) >> gs: 0000 @ ffffffff803ac000\0000000000000000 boot_cpu_pda\ >> /ffffffff(c00)Should this be split into "gs" and "shadow gs"?> [...] >>> What is "stack address" is it the base of the function''s stack frame >>> perhaps? Or maybe the top? Or maybe the framepointer? >> It is the address of the stack "word". > I''m afraid I am non the wiser. > > Ian.Does just: ffffffff803ddf90: ffffffff80048d19 0000000000200800 .......... ..... vs ffffffff803ddf90: [<ffffffff80048d19>] cpu_idle+0x95 help? The start of the line is the same. -Don
Don Slutz
2013-Nov-07 15:24 UTC
Re: [PATCH v2 02/12] xenctx: Add -2 (--two-pages) option to switch stack size to 8KiB
On 11/07/13 07:40, Ian Campbell wrote:> On Thu, 2013-11-07 at 08:04 +0000, Jan Beulich wrote: >>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >>> @@ -650,9 +651,11 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width) >>> >>> stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE) >>> & ~((guest_word_t) XC_PAGE_SIZE - 1)); >>> + if (xenctx.two_pages) >>> + stack_limit += XC_PAGE_SIZE; >>> printf("\n"); >>> printf("Stack:\n"); >>> - for (i=1; i<5 && stack < stack_limit; i++) { >>> + for (i=1; i<10 && stack < stack_limit; i++) { >> Touching a malformed line like this makes it almost mandatory imo >> to fix the coding style (missing blanks around = and <).Will do.>> >> Apart from that, the change isn''t really related to the subject of >> the patch afaict, and as you''re making thing more flexible anyway, >> it would seem reasonable to also have a command line option to >> control this limit.Part 1 is. Part 2 ("i<5" to "i<10") is not, it is part of patch #3 (and should have been there) to keep the amount output the same. A new option does make sense; will add it.> Or derive it from stack_limit and/or the number of pages?Not sure this is better then a new option. stack_limit currently reduces the amount of output.> What about kernels which use 4 pages for their stack ;-)Patch #8 would allow you to do this, 2 pages at a time. (or could also add -4...) -Don Slutz> Ian. >
Ian Campbell
2013-Nov-07 16:03 UTC
Re: [PATCH v2 07/12] xenctx: Add stack addr to call trace.
On Thu, 2013-11-07 at 10:06 -0500, Don Slutz wrote:> >>> What is "stack address" is it the base of the function''s stack frame > >>> perhaps? Or maybe the top? Or maybe the framepointer? > >> It is the address of the stack "word". > > I''m afraid I am non the wiser. > > > > Ian. > Does just: > > ffffffff803ddf90: ffffffff80048d19 0000000000200800 .......... ..... > vs > ffffffff803ddf90: [<ffffffff80048d19>] cpu_idle+0x95 > > help? The start of the line is the same.So it is the address of the word on the stack which contains the return address for the function call? Ian.
Don Slutz
2013-Nov-07 18:13 UTC
Re: [PATCH v2 07/12] xenctx: Add stack addr to call trace.
On 11/07/13 11:03, Ian Campbell wrote:> On Thu, 2013-11-07 at 10:06 -0500, Don Slutz wrote: > >>>>> What is "stack address" is it the base of the function''s stack frame >>>>> perhaps? Or maybe the top? Or maybe the framepointer? >>>> It is the address of the stack "word". >>> I''m afraid I am non the wiser. >>> >>> Ian. >> Does just: >> >> ffffffff803ddf90: ffffffff80048d19 0000000000200800 .......... ..... >> vs >> ffffffff803ddf90: [<ffffffff80048d19>] cpu_idle+0x95 >> >> help? The start of the line is the same. > So it is the address of the word on the stack which contains the return > address for the function call?Yes. -Don Slutz> Ian. >
Don Slutz
2013-Nov-07 18:56 UTC
Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
On 11/06/13 15:35, Andrew Cooper wrote:> On 06/11/13 20:08, Don Slutz wrote: >> From: Don Slutz <Don@CloudSwitch.com> >> >> This allows the output of registers that are not available in the >> not HVM view. >> >> Look up symbols for various registers as kernel data. >> >> Also a minor check that ip, sp, and cr3 are the same. >> >> Signed-off-by: Don Slutz <Don@CloudSwitch.com> > What about tools/misc/xen-hvmctx ?1st I have heard of it. Will look into this.> > This is a particularly awkward tool where we have two tools in different > subtrees which do about 80% the same thing.80% looks high to me (xenctx does do stack, code, and symbol table also), but I do agree that having 2 tools to do 1 thing (display cpu registers) is confusing.> I would certainly not object if someone were to merge these two tools > into one. It is particularly awkward as xen-hvmctx ends up being > installed into /usr/sbin/ and is therefore on the $PATH, while xenctx > ends up off the $PATH in /usr/lib/xen/bin/The way I would do this is to make 1 source file, delete the 2nd, and install the same code as both xenctx and xen-hvmctx. argv[0] can be decoded and the default action adjusted based on the name. In this case there would be command line options to get the current old output for each tool. -Don Slutz> I am also fairly sure we have other examples of tools like this with > sightly different variants living in different locations. > > ~Andrew > >> --- >> tools/xentrace/xenctx.c | 284 ++++++++++++++++++++++++++++++++++++++++++++++- >> 1 files changed, 282 insertions(+), 2 deletions(-) >> >> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c >> index 6ec9c74..d3cbb22 100644 >> --- a/tools/xentrace/xenctx.c >> +++ b/tools/xentrace/xenctx.c >> @@ -415,6 +415,247 @@ static void print_ctx(vcpu_guest_context_any_t *ctx) >> print_ctx_64(&ctx->x64); >> } >> >> +#if defined(__i386__) || defined(__x86_64__) >> +static void print_cpuctx_regs64(struct hvm_hw_cpu *ctx) >> +{ >> + printf("rip: %016"PRIx64, ctx->rip); >> + print_symbol(ctx->rip, KERNEL_TEXT_ADDR); >> + print_flags(ctx->rflags); >> + printf("rsp: %016"PRIx64"\n", ctx->rsp); >> + >> + printf("rax: %016"PRIx64"\t", ctx->rax); >> + printf("rcx: %016"PRIx64"\t", ctx->rcx); >> + printf("rdx: %016"PRIx64"\n", ctx->rdx); >> + >> + printf("rbx: %016"PRIx64"\t", ctx->rbx); >> + printf("rsi: %016"PRIx64"\t", ctx->rsi); >> + printf("rdi: %016"PRIx64"\n", ctx->rdi); >> + >> + printf("rbp: %016"PRIx64"\t", ctx->rbp); >> + printf(" r8: %016"PRIx64"\t", ctx->r8); >> + printf(" r9: %016"PRIx64"\n", ctx->r9); >> + >> + printf("r10: %016"PRIx64"\t", ctx->r10); >> + printf("r11: %016"PRIx64"\t", ctx->r11); >> + printf("r12: %016"PRIx64"\n", ctx->r12); >> + >> + printf("r13: %016"PRIx64"\t", ctx->r13); >> + printf("r14: %016"PRIx64"\t", ctx->r14); >> + printf("r15: %016"PRIx64"\n", ctx->r15); >> + >> + printf(" cs: %04x @ %016"PRIx64, ctx->cs_sel, ctx->cs_base); >> + if (ctx->cs_base) >> + print_symbol(ctx->cs_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->cs_limit, ctx->cs_arbytes); >> + printf(" ss: %04x @ %016"PRIx64, ctx->ss_sel, ctx->ss_base); >> + if (ctx->ss_base) >> + print_symbol(ctx->ss_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->ss_limit, ctx->ss_arbytes); >> + printf(" ds: %04x @ %016"PRIx64, ctx->ds_sel, ctx->ds_base); >> + if (ctx->ds_base) >> + print_symbol(ctx->ds_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->ds_limit, ctx->ds_arbytes); >> + printf(" es: %04x @ %016"PRIx64, ctx->es_sel, ctx->es_base); >> + if (ctx->es_base) >> + print_symbol(ctx->es_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->es_limit, ctx->es_arbytes); >> + printf(" fs: %04x @ %016"PRIx64, ctx->fs_sel, ctx->fs_base); >> + if (ctx->fs_base) >> + print_symbol(ctx->fs_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->fs_limit, ctx->fs_arbytes); >> + printf(" gs: %04x @ %016"PRIx64"\\%016"PRIx64, ctx->gs_sel, >> + ctx->gs_base, ctx->shadow_gs); >> + if (ctx->gs_base) >> + print_symbol(ctx->gs_base, KERNEL_DATA_ADDR); >> + printf("\\"); >> + if (ctx->shadow_gs) >> + print_symbol(ctx->shadow_gs, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->gs_limit, ctx->gs_arbytes); >> + >> + if (xenctx.disp_all) { >> + printf(" tr: %04x @ %016"PRIx64, ctx->tr_sel, ctx->tr_base); >> + if (ctx->tr_base) >> + print_symbol(ctx->tr_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->tr_limit, ctx->tr_arbytes); >> + printf(" ldt: %04x @ %016"PRIx64, ctx->ldtr_sel, ctx->ldtr_base); >> + if (ctx->ldtr_base) >> + print_symbol(ctx->ldtr_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->ldtr_limit, ctx->ldtr_arbytes); >> + printf("\n"); >> + printf("cr0: %08"PRIx64"\n", ctx->cr0); >> + printf("cr2: %08"PRIx64, ctx->cr2); >> + print_symbol(ctx->cr2, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("cr3: %08"PRIx64"\n", ctx->cr3); >> + printf("cr4: %08"PRIx64"\n", ctx->cr4); >> + printf("\n"); >> + printf("dr0: %08"PRIx64, ctx->dr0); >> + print_symbol(ctx->dr0, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr1: %08"PRIx64, ctx->dr1); >> + print_symbol(ctx->dr1, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr2: %08"PRIx64, ctx->dr2); >> + print_symbol(ctx->dr2, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr3: %08"PRIx64, ctx->dr3); >> + print_symbol(ctx->dr3, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr6: %08"PRIx64"\n", ctx->dr6); >> + printf("dr7: %08"PRIx64"\n", ctx->dr7); >> + printf("\n"); >> + printf("gdt: %016"PRIx64"/%lx", ctx->gdtr_base, (long)(ctx->gdtr_limit)); >> + print_symbol(ctx->gdtr_base, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("idt: %016"PRIx64"/%lx", ctx->idtr_base, (long)(ctx->idtr_limit)); >> + print_symbol(ctx->idtr_base, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("\n"); >> + printf(" EFER: %08"PRIx64" flags: %08"PRIx64"\n", >> + ctx->msr_efer, ctx->msr_flags); >> + printf("SYSENTER cs: %08"PRIx64" esp: %08"PRIx64" eip: %08"PRIx64, >> + ctx->sysenter_cs, ctx->sysenter_esp, ctx->sysenter_eip); >> + print_symbol(ctx->sysenter_eip, KERNEL_TEXT_ADDR); >> + printf("\n"); >> + printf(" STAR: %08"PRIx64" SFMASK: %08"PRIx64"\n", >> + ctx->msr_star, ctx->msr_syscall_mask); >> + printf("LSTAR: %08"PRIx64, ctx->msr_lstar); >> + print_symbol(ctx->msr_lstar, KERNEL_TEXT_ADDR); >> + printf("\n"); >> + printf("CSTAR: %08"PRIx64, ctx->msr_cstar); >> + print_symbol(ctx->msr_cstar, KERNEL_TEXT_ADDR); >> + printf("\n"); >> + printf("\n"); >> + printf("\n"); >> + printf(" tsc: %016"PRIx64"\n", ctx->tsc); >> + printf("\n"); >> + printf("\n"); >> + printf("pending_event: %lx pending_vector: %lx\n", >> + (long)(ctx->pending_event), (long)(ctx->pending_vector)); >> + printf("pending error_code: %lx\n\n", (long)(ctx->error_code)); >> + } >> +} >> + >> +static void print_cpuctx_regs32(struct hvm_hw_cpu *ctx) >> +{ >> + printf("cs:eip: %04x:%08x", ctx->cs_sel, (uint32_t)ctx->rip); >> + print_symbol((uint32_t)ctx->rip, KERNEL_TEXT_ADDR); >> + print_flags((uint32_t)ctx->rflags); >> + printf("ss:esp: %04x:%08x\n", ctx->ss_sel, (uint32_t)ctx->rsp); >> + printf("\n"); >> + >> + printf("eax: %08x\t", (uint32_t)ctx->rax); >> + printf("ebx: %08x\t", (uint32_t)ctx->rbx); >> + printf("ecx: %08x\t", (uint32_t)ctx->rcx); >> + printf("edx: %08x\n", (uint32_t)ctx->rdx); >> + >> + printf("esi: %08x\t", (uint32_t)ctx->rsi); >> + printf("edi: %08x\t", (uint32_t)ctx->rdi); >> + printf("ebp: %08x\n", (uint32_t)ctx->rbp); >> + printf("\n"); >> + >> + printf(" cs: %04x @ %08"PRIx64, ctx->cs_sel, ctx->cs_base); >> + if (ctx->cs_base) >> + print_symbol(ctx->cs_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->cs_limit, ctx->cs_arbytes); >> + printf(" ss: %04x @ %08"PRIx64, ctx->ss_sel, ctx->ss_base); >> + if (ctx->ss_base) >> + print_symbol(ctx->ss_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->ss_limit, ctx->ss_arbytes); >> + printf(" ds: %04x @ %08"PRIx64, ctx->ds_sel, ctx->ds_base); >> + if (ctx->ds_base) >> + print_symbol(ctx->ds_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->ds_limit, ctx->ds_arbytes); >> + printf(" es: %04x @ %08"PRIx64, ctx->es_sel, ctx->es_base); >> + if (ctx->es_base) >> + print_symbol(ctx->es_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->es_limit, ctx->es_arbytes); >> + printf(" fs: %04x @ %08"PRIx64, ctx->fs_sel, ctx->fs_base); >> + if (ctx->fs_base) >> + print_symbol(ctx->fs_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->fs_limit, ctx->fs_arbytes); >> + printf(" gs: %04x @ %08"PRIx64"\\%08"PRIx64, ctx->gs_sel, >> + ctx->gs_base, ctx->shadow_gs); >> + if (ctx->gs_base) >> + print_symbol(ctx->gs_base, KERNEL_DATA_ADDR); >> + printf("\\"); >> + if (ctx->shadow_gs) >> + print_symbol(ctx->shadow_gs, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->gs_limit, ctx->gs_arbytes); >> + >> + if (xenctx.disp_all) { >> + printf(" tr: %04x @ %08"PRIx64, ctx->tr_sel, ctx->tr_base); >> + if (ctx->tr_base) >> + print_symbol(ctx->tr_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->tr_limit, ctx->tr_arbytes); >> + printf(" ldt: %04x @ %08"PRIx64, ctx->ldtr_sel, ctx->ldtr_base); >> + if (ctx->ldtr_base) >> + print_symbol(ctx->ldtr_base, KERNEL_DATA_ADDR); >> + printf("\n /%08x(%x)\n", ctx->ldtr_limit, ctx->ldtr_arbytes); >> + printf("\n"); >> + printf("cr0: %08"PRIx64"\n", ctx->cr0); >> + printf("cr2: %08"PRIx64, ctx->cr2); >> + print_symbol(ctx->cr2, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("cr3: %08"PRIx64"\n", ctx->cr3); >> + printf("cr4: %08"PRIx64"\n", ctx->cr4); >> + printf("\n"); >> + printf("dr0: %08"PRIx64, ctx->dr0); >> + print_symbol(ctx->dr0, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr1: %08"PRIx64, ctx->dr1); >> + print_symbol(ctx->dr1, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr2: %08"PRIx64, ctx->dr2); >> + print_symbol(ctx->dr2, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr3: %08"PRIx64, ctx->dr3); >> + print_symbol(ctx->dr3, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("dr6: %08"PRIx64"\n", ctx->dr6); >> + printf("dr7: %08"PRIx64"\n", ctx->dr7); >> + printf("\n"); >> + printf("gdt: %08"PRIx64"/%lx", ctx->gdtr_base, (long)(ctx->gdtr_limit)); >> + print_symbol(ctx->gdtr_base, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("idt: %08"PRIx64"/%lx", ctx->idtr_base, (long)(ctx->idtr_limit)); >> + print_symbol(ctx->idtr_base, KERNEL_DATA_ADDR); >> + printf("\n"); >> + printf("\n"); >> + printf(" EFER: %08"PRIx64" flags: %08"PRIx64"\n", >> + ctx->msr_efer, ctx->msr_flags); >> + printf("SYSENTER cs: %08"PRIx64" esp: %08"PRIx64" eip: %08"PRIx64, >> + ctx->sysenter_cs, ctx->sysenter_esp, ctx->sysenter_eip); >> + print_symbol(ctx->sysenter_eip, KERNEL_TEXT_ADDR); >> + printf("\n"); >> + printf(" STAR: %08"PRIx64" SFMASK: %08"PRIx64"\n", >> + ctx->msr_star, ctx->msr_syscall_mask); >> + printf("LSTAR: %08"PRIx64, ctx->msr_lstar); >> + print_symbol(ctx->msr_lstar, KERNEL_TEXT_ADDR); >> + printf("\n"); >> + printf("CSTAR: %08"PRIx64, ctx->msr_cstar); >> + print_symbol(ctx->msr_cstar, KERNEL_TEXT_ADDR); >> + printf("\n"); >> + printf("\n"); >> + printf(" tsc: %016"PRIx64"\n", ctx->tsc); >> + printf("\n"); >> + printf("\n"); >> + printf("pending_event: %lx pending_vector: %lx\n", >> + (long)(ctx->pending_event), (long)(ctx->pending_vector)); >> + printf("pending error_code: %lx\n\n", (long)(ctx->error_code)); >> + } >> +} >> + >> +static void print_cpuctx(struct hvm_hw_cpu *ctx) >> +{ >> + if (guest_word_size == 8) >> + print_cpuctx_regs64(ctx); >> + else >> + print_cpuctx_regs32(ctx); >> + >> +} >> +#endif >> + >> #define NONPROT_MODE_SEGMENT_SHIFT 4 >> >> static guest_word_t instr_pointer(vcpu_guest_context_any_t *ctx) >> @@ -887,6 +1128,9 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest >> static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) >> { >> vcpu_guest_context_any_t ctx; >> +#if defined(__i386__) || defined(__x86_64__) >> + struct hvm_hw_cpu cpuctx; >> +#endif >> >> if (xc_vcpu_getcontext(xenctx.xc_handle, xenctx.domid, vcpu, &ctx) < 0) { >> perror("xc_vcpu_getcontext"); >> @@ -896,7 +1140,6 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) >> #if defined(__i386__) || defined(__x86_64__) >> { >> if (xenctx.dominfo.hvm) { >> - struct hvm_hw_cpu cpuctx; >> xen_capabilities_info_t xen_caps = ""; >> if (xc_domain_hvm_getcontext_partial( >> xenctx.xc_handle, xenctx.domid, HVM_SAVE_CODE(CPU), >> @@ -931,7 +1174,44 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) >> #endif >> } else { >> if (!stk_addr) { >> - print_ctx(&ctx); >> +#if defined(__i386__) || defined(__x86_64__) >> + if (xenctx.dominfo.hvm && ctxt_word_size == 8) { >> + if (guest_word_size == 4) { >> + if ((((uint32_t)ctx.x64.user_regs.eip) != cpuctx.rip) || >> + (((uint32_t)ctx.x64.user_regs.esp) != cpuctx.rsp) || >> + (((uint32_t)ctx.x64.ctrlreg[3]) != cpuctx.cr3)) { >> + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", >> + (long long)((uint32_t)ctx.x64.user_regs.eip), >> + (long long)cpuctx.rip, >> + (long long)((uint32_t)ctx.x64.user_regs.esp), >> + (long long)cpuctx.rsp, >> + (long long)((uint32_t)ctx.x64.ctrlreg[3]), >> + (long long)cpuctx.cr3); >> + fprintf(stdout, "=============Regs mismatch start=============\n"); >> + print_ctx(&ctx); >> + fprintf(stdout, "=============Regs mismatch end=============\n"); >> + } >> + } else { >> + if ((ctx.x64.user_regs.eip != cpuctx.rip) || >> + (ctx.x64.user_regs.esp != cpuctx.rsp) || >> + (ctx.x64.ctrlreg[3] != cpuctx.cr3)) { >> + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", >> + (long long)ctx.x64.user_regs.eip, >> + (long long)cpuctx.rip, >> + (long long)ctx.x64.user_regs.esp, >> + (long long)cpuctx.rsp, >> + (long long)ctx.x64.ctrlreg[3], >> + (long long)cpuctx.cr3); >> + fprintf(stdout, "=============Regs mismatch start=============\n"); >> + print_ctx(&ctx); >> + fprintf(stdout, "=============Regs mismatch end=============\n"); >> + } >> + } >> + print_cpuctx(&cpuctx); >> + } >> + else >> +#endif >> + print_ctx(&ctx); >> } >> #ifndef NO_TRANSLATION >> if (!stk_addr) {
Don Slutz
2013-Nov-07 21:19 UTC
Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
On 11/07/13 03:38, Jan Beulich wrote:>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >> @@ -931,7 +1174,44 @@ static void dump_ctx(int vcpu, guest_word_t mem_addr, guest_word_t stk_addr) >> #endif >> } else { >> if (!stk_addr) { >> - print_ctx(&ctx); >> +#if defined(__i386__) || defined(__x86_64__) >> + if (xenctx.dominfo.hvm && ctxt_word_size == 8) { >> + if (guest_word_size == 4) { >> + if ((((uint32_t)ctx.x64.user_regs.eip) != cpuctx.rip) || >> + (((uint32_t)ctx.x64.user_regs.esp) != cpuctx.rsp) || >> + (((uint32_t)ctx.x64.ctrlreg[3]) != cpuctx.cr3)) { >> + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", >> + (long long)((uint32_t)ctx.x64.user_regs.eip), >> + (long long)cpuctx.rip, >> + (long long)((uint32_t)ctx.x64.user_regs.esp), >> + (long long)cpuctx.rsp, >> + (long long)((uint32_t)ctx.x64.ctrlreg[3]), >> + (long long)cpuctx.cr3); >> + fprintf(stdout, "=============Regs mismatch start=============\n"); >> + print_ctx(&ctx); >> + fprintf(stdout, "=============Regs mismatch end=============\n"); >> + } >> + } else { >> + if ((ctx.x64.user_regs.eip != cpuctx.rip) || >> + (ctx.x64.user_regs.esp != cpuctx.rsp) || >> + (ctx.x64.ctrlreg[3] != cpuctx.cr3)) { >> + fprintf(stderr, "Regs mismatch ip=%llx vs %llx sp=%llx vs %llx cr3=%llx vs %llx\n", >> + (long long)ctx.x64.user_regs.eip, >> + (long long)cpuctx.rip, >> + (long long)ctx.x64.user_regs.esp, >> + (long long)cpuctx.rsp, >> + (long long)ctx.x64.ctrlreg[3], >> + (long long)cpuctx.cr3); >> + fprintf(stdout, "=============Regs mismatch start=============\n"); >> + print_ctx(&ctx); >> + fprintf(stdout, "=============Regs mismatch end=============\n"); >> + } >> + } >> + print_cpuctx(&cpuctx); >> + } >> + else >> +#endif >> + print_ctx(&ctx); > Apart from Andrew''s comments, which I agree with - most of the > additions above clearly don''t belong here: This is not a diagnostic > utility.Fine with me, I will drop this part. I added during the time (~2010-2011) when I was looking at a DomU that was crashed. What I remember about this DomU was that vCPU 1 was offline and vCPU 0 was the crash cause and vCPU 2 & 3 where in the code to go offline. The routine xc_domain_hvm_getcontext_partial() was returning vCPU 2''s data when asked for vCPU 1 (via instance == 1). This is the key reason I did the code in patch #12 (basically a way to call xc_domain_hvm_getcontext_partial() and xc_vcpu_getcontext() with different args). At the time I was not working on full xen code, just changing xenctx the help me out as needed. Looking at the code now, I do not understand how this could happen. -Don Slutz> Jan >
On 11/07/13 07:53, Ian Campbell wrote:> On Thu, 2013-11-07 at 08:40 +0000, Jan Beulich wrote: >>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >>> This can be used to dump the normal ctx for an HVM DomU by passing >>> -1 to fCPU. >>> >>> If only some of the vCPUs in a HVM DomU, you need to ask for ask for >>> the on line CPU number, not the expected vCPU number; fCPU can be >>> used for this. >> I''m sorry, but I can''t make sense of this description, and hence >> can''t review the patch. > Yeah, I''m wondering what the f stands for. > > Ian >As I have said in Subject: Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available. Date: Thu, 7 Nov 2013 16:19:34 -0500 From: Don Slutz <dslutz@terremark.com> To: Jan Beulich <JBeulich@suse.com>, Don Slutz <dslutz@verizon.com> CC: Ian Campbell <ian.campbell@citrix.com>, Don Slutz <Don@CloudSwitch.com>, George Dunlap <george.dunlap@eu.citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, xen-devel <xen-devel@lists.xenproject.org> This was code to work around a bug. I do not see how I got the result I did and since that DomU (and old xen version) is long gone, I will attempt to get a DomU in a similar state and check that this is not need. -Don Slutz --------------010705080601090000000803 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 8bit <html> <head> <meta content="text/html; charset=UTF-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 11/07/13 07:53, Ian Campbell wrote:<br> </div> <blockquote cite="mid:1383828793.32399.33.camel@kazak.uk.xensource.com" type="cite"> <pre wrap="">On Thu, 2013-11-07 at 08:40 +0000, Jan Beulich wrote: </pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">On 06.11.13 at 21:08, Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:dslutz@verizon.com"><dslutz@verizon.com></a> wrote: </pre> </blockquote> </blockquote> <pre wrap="">This can be used to dump the normal ctx for an HVM DomU by passing -1 to fCPU. If only some of the vCPUs in a HVM DomU, you need to ask for ask for the on line CPU number, not the expected vCPU number; fCPU can be used for this. </pre> </blockquote> <pre wrap=""> I''m sorry, but I can''t make sense of this description, and hence can''t review the patch. </pre> </blockquote> <pre wrap=""> Yeah, I''m wondering what the f stands for. Ian </pre> </blockquote> As I have said in<br> <br> <table class="moz-email-headers-table" border="0" cellpadding="0" cellspacing="0"> <tbody> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Subject: </th> <td>Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.</td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Date: </th> <td>Thu, 7 Nov 2013 16:19:34 -0500</td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">From: </th> <td>Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:dslutz@terremark.com"><dslutz@terremark.com></a></td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">To: </th> <td>Jan Beulich <a class="moz-txt-link-rfc2396E" href="mailto:JBeulich@suse.com"><JBeulich@suse.com></a>, Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:dslutz@verizon.com"><dslutz@verizon.com></a></td> </tr> <tr> <th align="RIGHT" nowrap="nowrap" valign="BASELINE">CC: </th> <td>Ian Campbell <a class="moz-txt-link-rfc2396E" href="mailto:ian.campbell@citrix.com"><ian.campbell@citrix.com></a>, Don Slutz <a class="moz-txt-link-rfc2396E" href="mailto:Don@CloudSwitch.com"><Don@CloudSwitch.com></a>, George Dunlap <a class="moz-txt-link-rfc2396E" href="mailto:george.dunlap@eu.citrix.com"><george.dunlap@eu.citrix.com></a>, Ian Jackson <a class="moz-txt-link-rfc2396E" href="mailto:ian.jackson@eu.citrix.com"><ian.jackson@eu.citrix.com></a>, Stefano Stabellini <a class="moz-txt-link-rfc2396E" href="mailto:stefano.stabellini@eu.citrix.com"><stefano.stabellini@eu.citrix.com></a>, xen-devel <a class="moz-txt-link-rfc2396E" href="mailto:xen-devel@lists.xenproject.org"><xen-devel@lists.xenproject.org></a></td> </tr> </tbody> </table> <br> This was code to work around a bug. I do not see how I got the result I did and since that DomU (and old xen version) is long gone, I will attempt to get a DomU in a similar state and check that this is not need.<br> -Don Slutz<br> <br> </body> </html> --------------010705080601090000000803-- --===============7158028254293697682=Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7158028254293697682==--
Don Slutz
2013-Nov-08 00:12 UTC
Re: [PATCH v2 03/12] xenctx: Output ascii version of stack also.
On 11/07/13 08:21, Jan Beulich wrote:>>>> On 07.11.13 at 14:17, "Jan Beulich" <JBeulich@suse.com> wrote: >>>>> On 07.11.13 at 13:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: >>> On Thu, 2013-11-07 at 08:09 +0000, Jan Beulich wrote: >>>>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >>>> Why? I personally don''t see a need - there''s rarely much useful >>>> ASCII data on the stack. >>> This is true IME >>> >>>> So afaic: If at all, then only via command >>>> line option defaulting to off. >>> Well, assuming it doesn''t make the output >80 columns (which I can''t >>> tell from here) it seems harmless enough? >> If you end up looking at the output in a wide enough window, it''ll >> make you scroll more. > Oops, I was mentally in patch 04 here. So yes, it may be harmless, > but clutters the data that you need to look at, and I personally > prefer to look at _just_ the data that''s interesting, without having > to (mentally or physically) discard other pieces. > > Jan >I am happy to make it a command line option. -Don Slutz
Don Slutz
2013-Nov-08 00:13 UTC
Re: [PATCH v2 04/12] xenctx: Add stack address to dump, switch to 16 bytes per line.
On 11/07/13 07:46, Ian Campbell wrote:> On Thu, 2013-11-07 at 08:12 +0000, Jan Beulich wrote: >>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >> Again - why? > Having the stack address there can make it easier to locate a word > referenced by a register for example, without error prone counting. In > general many of the changelogs in this series could do with a second > sentence in them after the title giving some rationale so we don''t have > to guess. > >> This makes it just two entries per line for a 64-bit >> guest. Pretty little I would say. > Agreed. > > I expect that along with the previous patch lines would now be over 80 > columns for sure. If that''s the case then either the ascii or the > address (or both) needs to be optional IMHO. > >> Jan >>I am happy to make it a command line option. -Don Slutz
Don Slutz
2013-Nov-08 00:21 UTC
Re: [PATCH v2 08/12] xenctx: Add -d <daddr> option to dump memory at daddr as a stack.
On 11/07/13 03:22, Jan Beulich wrote:>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >> @@ -956,9 +973,9 @@ int main(int argc, char **argv) >> } >> >> if (xenctx.all_vcpus) >> - dump_all_vcpus(); >> + dump_all_vcpus(stk_addr); >> else >> - dump_ctx(vcpu); >> + dump_ctx(vcpu, stk_addr); > For one, it hardly makes much sense to dump all vCPU-s with a > single stack address. And assuming a stack can''t sit at address > zero is wrong too - you shouldn''t make any assumptions in > particular for HVM guests. > > So what you intend here should lead to just the requested > "stack" to be printed, without touching other functionality (i.e. > likely you''ll want the "if()" above preceded by another one, and > itself converted to an "else if()"). > > Jan >It is clear that handling "-d" out side of dump_ctx() make sense. Not so sure on the simple else. I think that -C should still dump all cpus even if -d is provided. I guess I could complain if both -d and -C are specified, but I like to just do the requested actions. -Don Slutz
Don Slutz
2013-Nov-08 00:24 UTC
Re: [PATCH v2 09/12] xenctx: Add -m <maddr> option to dump memory at maddr.
On 11/07/13 03:25, Jan Beulich wrote:>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >> @@ -973,9 +1065,9 @@ int main(int argc, char **argv) >> } >> >> if (xenctx.all_vcpus) >> - dump_all_vcpus(stk_addr); >> + dump_all_vcpus(mem_addr, stk_addr); >> else >> - dump_ctx(vcpu, stk_addr); >> + dump_ctx(vcpu, mem_addr, stk_addr); > Same comment as on the previous patch - just dump the requested > memory here, without modifying other behavior. > > Jan >Yes, will re-work to do the same as the previous patch''s re-work will do. -Don
Don Slutz
2013-Nov-08 00:51 UTC
Re: [PATCH v2 10/12] xenctx: change is_kernel_text() into is_kernel_addr().
On 11/07/13 03:35, Jan Beulich wrote:>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >> @@ -76,22 +86,28 @@ unsigned long long kernel_start = 0xc0000000; >> unsigned long long kernel_start = 0xffffffff80000000UL; >> #endif >> >> -static int is_kernel_text(guest_word_t addr) >> +static type_of_addr is_kernel_addr(guest_word_t addr) > The "is_" prefix is now bogus.Ok, will remove.> >> { >> - if (symbol_table == NULL) >> - return (addr > kernel_start); >> + if (symbol_table == NULL) { >> + if (addr > kernel_start) >> + return KERNEL_TEXT_ADDR; >> + else >> + return NOT_KERNEL_ADDR; >> + } >> >> if (addr >= kernel_stext && >> addr <= kernel_etext) >> - return 1; >> - if (kernel_hypercallpage && >> - (addr >= kernel_hypercallpage && >> - addr <= kernel_hypercallpage + 4096)) >> - return 1; >> + return KERNEL_TEXT_ADDR; >> + if (kernel_hypercallpage && (addr >= kernel_hypercallpage && >> + addr <= kernel_hypercallpage + 4096)) > This reformatting is pointlessly bloating the patch - if you want > the line reformatted, you should (a) do this in the patch that > adds the first part of the condition and (b) obey to indentation > rules.Opps, I did not what this reformatting, will just drop it.>> + return KERNEL_TEXT_ADDR; >> if (addr >= kernel_sinittext && >> addr <= kernel_einittext) >> - return 1; >> - return 0; >> + return KERNEL_TEXT_ADDR; >> + if (addr >= kernel_text && >> + addr <= kernel_end) >> + return KERNEL_DATA_ADDR; > As you supposedly filtered out all text ranges before, did you really > mean to compare to kernel_text here (rather than kernel_start)?Yes, I did. I think it is better to use the value that is in the system map over the default. It has changed: dcs-xen-54:~/xen>grep " _text" /boot/System.map-* /boot/System.map-2.6.18-128.el5:ffffffff80000000 A _text /boot/System.map-3.6.11-5.fc17.x86_64:ffffffff81000000 T _text /boot/System.map-3.8.11-100.fc17.x86_64:ffffffff81000000 T _text But since it is a command line argument, if specified, should it be used instead?>> @@ -749,7 +769,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest >> void *p = map_page(ctx, vcpu, stack); >> if (!p) >> return -1; >> - word = read_stack_word(p, width); >> + word = read_mem_word(ctx, vcpu, stack, width); > How is this change related to the purpose of the patch?Opps, wrong split. This (and the one 2 below changing to read_mem_word) should be in their own patch (or back in patch 8, with patch 9 before 8. Without this change specifying an unaligned address can read across a page boundary which would not get he correct data (and can fault).>> @@ -818,7 +838,7 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest >> if (xenctx.stack_trace) { >> print_stack_word(stack, width); >> printf(": |-- "); >> - print_stack_word(read_stack_word(p, width), width); >> + print_stack_word(frame, width); > And this one? >This cleanup mostly likely should be just dropped (or made it''s own patch). The context is just one line short: frame = read_stack_word(p, width); if (xenctx.stack_trace) { ... print_stack_word(read_stack_word(p, width), width); And since read_stack_word(p, width) should always return the same data for the same address; the 2nd call is not needed.>> @@ -843,13 +863,13 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width, guest >> void *p = map_page(ctx, vcpu, stack); >> if (!p) >> return -1; >> - word = read_stack_word(p, width); >> - if (is_kernel_text(word)) { >> + word = read_mem_word(ctx, vcpu, stack, width); >> + if (is_kernel_addr(word) >= KERNEL_TEXT_ADDR) { > And one more.The 2nd line is needed is_kernel_text() changed to is_kernel_addr(). -Don Slutz> Jan >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Jan Beulich
2013-Nov-08 08:40 UTC
Re: [PATCH v2 10/12] xenctx: change is_kernel_text() into is_kernel_addr().
>>> On 08.11.13 at 01:51, Don Slutz <dslutz@verizon.com> wrote: > On 11/07/13 03:35, Jan Beulich wrote: >>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >>> + return KERNEL_TEXT_ADDR; >>> if (addr >= kernel_sinittext && >>> addr <= kernel_einittext) >>> - return 1; >>> - return 0; >>> + return KERNEL_TEXT_ADDR; >>> + if (addr >= kernel_text && >>> + addr <= kernel_end) >>> + return KERNEL_DATA_ADDR; >> As you supposedly filtered out all text ranges before, did you really >> mean to compare to kernel_text here (rather than kernel_start)? > Yes, I did. I think it is better to use the value that is in the system map > over the default. It has changed: > > dcs-xen-54:~/xen>grep " _text" /boot/System.map-* > /boot/System.map-2.6.18-128.el5:ffffffff80000000 A _text > /boot/System.map-3.6.11-5.fc17.x86_64:ffffffff81000000 T _text > /boot/System.map-3.8.11-100.fc17.x86_64:ffffffff81000000 T _text > > But since it is a command line argument, if specified, should it be used > instead?I guess so. In any event - after having split out all text pieces (hopefully), the check here should cover the whole kernel image, no matter how the ordering between text and data is. Jan
Ian Campbell
2013-Nov-08 09:50 UTC
Re: [PATCH v2 10/12] xenctx: change is_kernel_text() into is_kernel_addr().
On Fri, 2013-11-08 at 08:40 +0000, Jan Beulich wrote:> >>> On 08.11.13 at 01:51, Don Slutz <dslutz@verizon.com> wrote: > > On 11/07/13 03:35, Jan Beulich wrote: > >>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: > >>> + return KERNEL_TEXT_ADDR; > >>> if (addr >= kernel_sinittext && > >>> addr <= kernel_einittext) > >>> - return 1; > >>> - return 0; > >>> + return KERNEL_TEXT_ADDR; > >>> + if (addr >= kernel_text && > >>> + addr <= kernel_end) > >>> + return KERNEL_DATA_ADDR; > >> As you supposedly filtered out all text ranges before, did you really > >> mean to compare to kernel_text here (rather than kernel_start)? > > Yes, I did. I think it is better to use the value that is in the system map > > over the default. It has changed: > > > > dcs-xen-54:~/xen>grep " _text" /boot/System.map-* > > /boot/System.map-2.6.18-128.el5:ffffffff80000000 A _text > > /boot/System.map-3.6.11-5.fc17.x86_64:ffffffff81000000 T _text > > /boot/System.map-3.8.11-100.fc17.x86_64:ffffffff81000000 T _text > > > > But since it is a command line argument, if specified, should it be used > > instead? > > I guess so.Yes, since the user may not have a symbol table for the given kernel and it may not even be Linux. I''m sure a Windows kernel guru debugging a Windows guest would know the right number for that version of Windows. (IIRC the option was added by just such a guru...)> In any event - after having split out all text pieces > (hopefully), the check here should cover the whole kernel image, > no matter how the ordering between text and data is. > > Jan >
Ian Campbell
2013-Nov-08 10:13 UTC
Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
On Thu, 2013-11-07 at 13:56 -0500, Don Slutz wrote:> On 11/06/13 15:35, Andrew Cooper wrote:> > I would certainly not object if someone were to merge these two tools > > into one. It is particularly awkward as xen-hvmctx ends up being > > installed into /usr/sbin/ and is therefore on the $PATH, while xenctx > > ends up off the $PATH in /usr/lib/xen/bin/ > The way I would do this is to make 1 source file, delete the 2nd, and > install the same code as both xenctx and xen-hvmctx. argv[0] can be > decoded and the default action adjusted based on the name. In this > case there would be command line options to get the current old output > for each tool.Personally I don''t think we need to retain any kind of CLI compatibility for such developer focused tools. I''d be perfectly happy for the (sensible) functionality of hvmctx to be merged into xenctx and to then disappear. Ian.
Ian Campbell
2013-Nov-08 10:15 UTC
Re: [PATCH v2 07/12] xenctx: Add stack addr to call trace.
On Thu, 2013-11-07 at 13:13 -0500, Don Slutz wrote:> On 11/07/13 11:03, Ian Campbell wrote: > > On Thu, 2013-11-07 at 10:06 -0500, Don Slutz wrote: > > > >>>>> What is "stack address" is it the base of the function''s stack frame > >>>>> perhaps? Or maybe the top? Or maybe the framepointer? > >>>> It is the address of the stack "word". > >>> I''m afraid I am non the wiser. > >>> > >>> Ian. > >> Does just: > >> > >> ffffffff803ddf90: ffffffff80048d19 0000000000200800 .......... ..... > >> vs > >> ffffffff803ddf90: [<ffffffff80048d19>] cpu_idle+0x95 > >> > >> help? The start of the line is the same. > > So it is the address of the word on the stack which contains the return > > address for the function call? > Yes.Is that useful? The outermost word of the stack frame might arguably be useful, and I''m not even sure of that, but this one I''m even less convinced by (the two may correspond on some arches, but not all). Ian.
Andrew Cooper
2013-Nov-08 10:29 UTC
Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
On 08/11/13 10:13, Ian Campbell wrote:> On Thu, 2013-11-07 at 13:56 -0500, Don Slutz wrote: >> On 11/06/13 15:35, Andrew Cooper wrote: >>> I would certainly not object if someone were to merge these two tools >>> into one. It is particularly awkward as xen-hvmctx ends up being >>> installed into /usr/sbin/ and is therefore on the $PATH, while xenctx >>> ends up off the $PATH in /usr/lib/xen/bin/ >> The way I would do this is to make 1 source file, delete the 2nd, and >> install the same code as both xenctx and xen-hvmctx. argv[0] can be >> decoded and the default action adjusted based on the name. In this >> case there would be command line options to get the current old output >> for each tool. > Personally I don''t think we need to retain any kind of CLI compatibility > for such developer focused tools. I''d be perfectly happy for the > (sensible) functionality of hvmctx to be merged into xenctx and to then > disappear. > > Ian. >Agreed - I would much prefer one single tool with a useful set of options rather than keeping around something which vaguely looks like an older tool for the sake of it. This is strictly a dev tool rather than production tool. ~Andrew
Ian Campbell
2013-Nov-08 10:32 UTC
Re: [PATCH v2 11/12] xenctx: Dump registers via hvm info if available.
On Fri, 2013-11-08 at 10:29 +0000, Andrew Cooper wrote:> On 08/11/13 10:13, Ian Campbell wrote: > > On Thu, 2013-11-07 at 13:56 -0500, Don Slutz wrote: > >> On 11/06/13 15:35, Andrew Cooper wrote: > >>> I would certainly not object if someone were to merge these two tools > >>> into one. It is particularly awkward as xen-hvmctx ends up being > >>> installed into /usr/sbin/ and is therefore on the $PATH, while xenctx > >>> ends up off the $PATH in /usr/lib/xen/bin/ > >> The way I would do this is to make 1 source file, delete the 2nd, and > >> install the same code as both xenctx and xen-hvmctx. argv[0] can be > >> decoded and the default action adjusted based on the name. In this > >> case there would be command line options to get the current old output > >> for each tool. > > Personally I don''t think we need to retain any kind of CLI compatibility > > for such developer focused tools. I''d be perfectly happy for the > > (sensible) functionality of hvmctx to be merged into xenctx and to then > > disappear. > > > > Ian. > > > > Agreed - I would much prefer one single tool with a useful set of > options rather than keeping around something which vaguely looks like an > older tool for the sake of it. > > This is strictly a dev tool rather than production tool.Right. and on that basis In the short term while this is being sorted out I''d be happy with a patch to move hvmctx into LIBDIR instead of SBINDIR too, so it is less prominent. (the question of whether xenctx really belongs in $PATH or not is one we can consider separately later...) Ian.
On 06/11/13 20:08, Don Slutz wrote:> From: Don Slutz <dslutz@verizon.com> > > Change from v1 to v2: > title was: xenctx: Add an option to output more registers. > Processed review comments. > Jan Beulich: > Split 1 change into 12. > Switch to enum for is_kernel_text(), renamed to is_kernel_addr(). > Renamed vars like memAddr to mem_addr. > Ian Campbell: > More on is_kernel_text().I don''t think this series really needs my Ack -- xenctx is actually in the wrong place, AFAICT, as it doesn''t have anything really to do with xentrace. Its functionality is more similar to xen-hvmctx, which ATM lives in tools/misc. At some point it might make sense to reorg stuff a bit... -George
On Fri, 2013-11-08 at 11:29 +0000, George Dunlap wrote:> At some point it might make sense to reorg stuff a bit...Agreed.
Don Slutz
2013-Nov-08 13:50 UTC
Re: [PATCH v2 10/12] xenctx: change is_kernel_text() into is_kernel_addr().
On 11/08/13 04:50, Ian Campbell wrote:> On Fri, 2013-11-08 at 08:40 +0000, Jan Beulich wrote: >>>>> On 08.11.13 at 01:51, Don Slutz <dslutz@verizon.com> wrote: >>> On 11/07/13 03:35, Jan Beulich wrote: >>>>>>> On 06.11.13 at 21:08, Don Slutz <dslutz@verizon.com> wrote: >>>>> + return KERNEL_TEXT_ADDR; >>>>> if (addr >= kernel_sinittext && >>>>> addr <= kernel_einittext) >>>>> - return 1; >>>>> - return 0; >>>>> + return KERNEL_TEXT_ADDR; >>>>> + if (addr >= kernel_text && >>>>> + addr <= kernel_end) >>>>> + return KERNEL_DATA_ADDR; >>>> As you supposedly filtered out all text ranges before, did you really >>>> mean to compare to kernel_text here (rather than kernel_start)? >>> Yes, I did. I think it is better to use the value that is in the system map >>> over the default. It has changed: >>> >>> dcs-xen-54:~/xen>grep " _text" /boot/System.map-* >>> /boot/System.map-2.6.18-128.el5:ffffffff80000000 A _text >>> /boot/System.map-3.6.11-5.fc17.x86_64:ffffffff81000000 T _text >>> /boot/System.map-3.8.11-100.fc17.x86_64:ffffffff81000000 T _text >>> >>> But since it is a command line argument, if specified, should it be used >>> instead? >> I guess so. > Yes, since the user may not have a symbol table for the given kernel and > it may not even be Linux. I''m sure a Windows kernel guru debugging a > Windows guest would know the right number for that version of Windows. > (IIRC the option was added by just such a guru...)If you do not specify a symbol file, kernel_start will be used. This is the case when both are specified. If I am reading this right that is 2 votes to use kernel_start if specified otherwise kernel_text. -Don Slutz>> In any event - after having split out all text pieces >> (hopefully), the check here should cover the whole kernel image, >> no matter how the ordering between text and data is. >> >> Jan >> >