Re: [PATCH v5 10/17] xenctx: Add -m (--memory) <maddr> option to dump memory at maddr.

[Don Slutz <dslutz@verizon.com>]

On 03/21/14 10:53, Ian Campbell wrote:
> On Thu, 2014-03-20 at 15:06 -0400, 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 <Don@CloudSwitch.com>
>> ---
>> v5: Remove extra #ifndef.  Make common routine print_lines().
>>
>>   tools/xentrace/xenctx.c | 232 +++++++++++++++++++++++++++++++++---------------
>>   1 file changed, 162 insertions(+), 70 deletions(-)
>>
>> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
>> index 5732665..13f56ed 100644
>> --- a/tools/xentrace/xenctx.c
>> +++ b/tools/xentrace/xenctx.c
>> @@ -33,23 +33,6 @@
>>   #define DEFAULT_BYTES_PER_LINE 32
>>   #define DEFAULT_LINES 5
>>   
>> -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"
>> @@ -73,6 +56,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;
>> +
> This motion seems to be unmotivated by the commit message.

I will add the reason to the commit message:

Moved xenctx because guest_word_t is not defined where it was.



> (I've no idea why there is a struct for this stuff anyway, even though
> there is a good chance I added it, it could just as easily be globals in
> a program of this sort)

I just went with what was there...

>>   int main(int argc, char **argv)
>>   {
>>       int ch;
>>       int ret;
>> +#ifndef NO_TRANSLATION
>> +    static const char *sopts = "fs:hak:SCn:b:l:DtTm:";
>> +#else
>>       static const char *sopts = "fs:hak:SCn:b:l:DtT";
>> +#endif
>       static const char *sopts = "fs:hak:SCn:b:l:DtT"
> #ifdef ....
> 				"m:"
> #endif
> 	;

This is the way it use to be:

#ifdef __ia64__
         "r:"
#endif


>
> Or something to avoid that massive repetition. Another possibility:
>
> #ifndef NO_TRANSLATION
> #define TRANSLATION_OPTS "m:"
> #else
> #define TRANSLATION_OPTS
> #endif
> 		sopts = "fs:....." TRANS...._OPTS

I am planning on the following the old way (i.e. no new TRANSLATION_OPTS define).

    -Don Slutz

> Ian.
>
>