From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v4 10/15] xenctx: Add -M option to dump memory at maddr. Date: Wed, 19 Mar 2014 17:08:01 +0000 Message-ID: <5329CEF1.9010603@eu.citrix.com> References: <1395180940-23901-1-git-send-email-dslutz@verizon.com> <1395180940-23901-11-git-send-email-dslutz@verizon.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1395180940-23901-11-git-send-email-dslutz@verizon.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Don Slutz , xen-devel@lists.xen.org Cc: Don Slutz , Ian Jackson , Ian Campbell , Jan Beulich , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 03/18/2014 10:15 PM, Don Slutz wrote: > Currently not supported on ARM. > > New routine read_mem_word() will correctly read a word that crosses > a page boundary. It will not fault if the 2nd page can not be > mapped. > > Here is an example: > > Memory (address ffffffff803ddf90): > ffffffff80048d19 0000000000200800 ffffffff803e7801 0000000000086800 > 0000000000000000 ffffffff80430720 ffffffff803e722f 80008e000010019c > 00000000ffffffff 0000000000000000 0000000000000000 0000000000200000 > 0000000000000000 0000000000000000 0000000000000000 00cf9b000000ffff > 00af9b000000ffff 00cf93000000ffff 00cffb000000ffff 00cff3000000ffff > > Signed-off-by: Don Slutz > --- > v4 Changed from -m to -M > > tools/xentrace/xenctx.c | 168 ++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 150 insertions(+), 18 deletions(-) > > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index e5c45df..e7d490e 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -29,23 +29,6 @@ > #include > #include > > -static struct xenctx { > - xc_interface *xc_handle; > - int domid; > - int frame_ptrs; > - int stack_trace; > - int disp_all; > - int multiple_pages; > - int bytes_per_line; > - int lines; > - int decode_as_ascii; > - int tag_stack_dump; > - int tag_call_trace; > - int all_vcpus; > - int self_paused; > - xc_dominfo_t dominfo; > -} xenctx; > - > #if defined (__i386__) || defined (__x86_64__) > typedef unsigned long long guest_word_t; > #define FMT_32B_WORD "%08llx" > @@ -69,6 +52,27 @@ typedef uint64_t guest_word_t; > > #define MAX_BYTES_PER_LINE 128 > > +static struct xenctx { > + xc_interface *xc_handle; > + int domid; > + int frame_ptrs; > + int stack_trace; > + int disp_all; > + int multiple_pages; > + int bytes_per_line; > + int lines; > + int decode_as_ascii; > + int tag_stack_dump; > + int tag_call_trace; > + int all_vcpus; > +#ifndef NO_TRANSLATION > + guest_word_t mem_addr; > + int do_memory; > +#endif > + int self_paused; > + xc_dominfo_t dominfo; > +} xenctx; > + > struct symbol { > guest_word_t address; > char *name; > @@ -630,6 +634,37 @@ static guest_word_t read_stack_word(guest_word_t *src, int width) > return word; > } > > +#ifndef NO_TRANSLATION You seem to be nesting "#ifndef NO_TRANSLATION" here -- there's already an #ifndef up above map_page. > +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; > + } > +} > +#endif > + > static void print_stack_word(guest_word_t word, int width) > { > if (width == 4) > @@ -638,6 +673,67 @@ static void print_stack_word(guest_word_t word, int width) > printf(FMT_64B_WORD, word); > } > > +#ifndef NO_TRANSLATION > +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[MAX_BYTES_PER_LINE/4]; > + int i; > + > + instr_start = mem_addr; > + instr = mem_addr; > + printf("Memory (address "); > + print_stack_word(instr, width); > + printf("):\n"); > + for (i = 1; i < xenctx.lines + 1; i++) > + { > + int j = 0; > + int k; > + > + if ( xenctx.tag_stack_dump ) > + { > + print_stack_word(instr, width); > + printf(":"); > + } > + while ( instr < instr_start + i * xenctx.bytes_per_line ) > + { > + void *p = map_page(ctx, vcpu, instr); > + if ( !p ) > + return; > + word = read_mem_word(ctx, vcpu, instr, width); > + if ( xenctx.decode_as_ascii ) > + ascii[j++] = word; > + printf(" "); > + print_stack_word(word, width); > + instr += width; > + } > + printf(" "); > + if ( xenctx.decode_as_ascii ) > + { > + for (k = j; k < xenctx.bytes_per_line / width; k++) > + printf(" %*s", width*2, ""); > + for (k = 0; k < j; k++) > + { > + int l; > + unsigned char *bytep = (unsigned char*)&ascii[k]; > + > + for (l = 0; l < width; l++) > + { > + if ( (*bytep < 127) && (*bytep >= 32) ) > + printf("%c", *bytep); > + else > + printf("."); > + bytep++; > + } > + } > + } > + printf("\n"); This inner loop seems to be an exact copy of the code in print_stack -- wouldn't it make sense to make a generic "dump memory area" function, and have both print_mem and print_stack call it? > + } > +} > +#endif > + > static int print_code(vcpu_guest_context_any_t *ctx, int vcpu) > { > guest_word_t instr; > @@ -874,6 +970,13 @@ static void dump_ctx(int vcpu) > } > #endif > > +#ifndef NO_TRANSLATION > + if ( xenctx.do_memory ) > + { > + print_mem(&ctx, vcpu, guest_word_size, xenctx.mem_addr); > + return; > + } > +#endif > print_ctx(&ctx); > #ifndef NO_TRANSLATION > if (print_code(&ctx, vcpu)) > @@ -928,13 +1031,21 @@ static void usage(void) > printf(" add address on each line of Stack dump.\n"); > printf(" -T, --tag-call-trace\n"); > printf(" add address on each line of Call trace.\n"); > +#ifndef NO_TRANSLATION > + printf(" -M maddr, --memory=maddr\n"); > + printf(" dump memory at maddr.\n"); > +#endif > } > > int main(int argc, char **argv) > { > int ch; > int ret; > +#ifndef NO_TRANSLATION > + static const char *sopts = "fs:hak:SCmb:l:DtTM:"; > +#else > static const char *sopts = "fs:hak:SCmb:l:DtT"; > +#endif > static const struct option lopts[] = { > {"stack-trace", 0, NULL, 'S'}, > {"symbol-table", 1, NULL, 's'}, > @@ -944,6 +1055,9 @@ int main(int argc, char **argv) > {"decode-as-ascii", 0, NULL, 'D'}, > {"tag-stack-dump", 0, NULL, 't'}, > {"tag-call-trace", 0, NULL, 'T'}, > +#ifndef NO_TRANSLATION > + {"memory", 1, NULL, 'M'}, > +#endif > {"bytes-per-line", 1, NULL, 'b'}, > {"lines", 1, NULL, 'l'}, > {"all", 0, NULL, 'a'}, > @@ -954,6 +1068,7 @@ int main(int argc, char **argv) > const char *symbol_table = NULL; > > int vcpu = 0; > + int do_default = 1; > > xenctx.bytes_per_line = 32; > xenctx.lines = 5; > @@ -1007,10 +1122,18 @@ int main(int argc, char **argv) > break; > case 'C': > xenctx.all_vcpus = 1; > + do_default = 0; > break; > case 'k': > kernel_start = strtoull(optarg, NULL, 0); > break; > +#ifndef NO_TRANSLATION > + case 'M': > + xenctx.mem_addr = strtoull(optarg, NULL, 0); > + xenctx.do_memory = 1; > + do_default = 0; > + break; > +#endif > case 'h': > usage(); > exit(-1); > @@ -1060,9 +1183,18 @@ int main(int argc, char **argv) > xenctx.self_paused = 1; > } > > +#ifndef NO_TRANSLATION > + if ( xenctx.do_memory ) > + { > + dump_ctx(vcpu); > + if (xenctx.all_vcpus) > + printf("\n"); > + } > + xenctx.do_memory = 0; > +#endif > if (xenctx.all_vcpus) > dump_all_vcpus(); > - else > + if ( do_default ) > dump_ctx(vcpu); So after this change, you can do "xenctx -M " and get memory, "xenctx -C" and get vcpu info, or "xenctx -M -C" and get both; but you can't get the ctx with both. Would it make sense to add an option to explicitly request dump_ctx(), so that you could have any combination? (And of course default to dump_ctx() if nothing is specified?) -George