Re: [PATCH v2 3/5] dbg_rw_guest_mem: Conditionally enable debug log output

[Don Slutz <dslutz@verizon.com>]

On 01/08/14 05:38, Ian Campbell wrote:
> On Tue, 2014-01-07 at 19:25 -0500, Don Slutz wrote:
>> If dbg_debug is non-zero, output debug.
>>
>> Include put_gfn debug logging.
>>
>> Here is a smaple output at dbg_debug == 2:
>>
>> (XEN) [2014-01-07 03:20:09] gmem:addr:8f56 buf:00000000006e2020 len:$1 domid:1 toaddr:0 dp:ffff83083e5fe000
>> (XEN) [2014-01-07 03:20:09] vaddr:8f56 domid:1
>> (XEN) [2014-01-07 03:20:09] X: vaddr:8f56 domid:1 mfn:64331a
>> (XEN) [2014-01-07 03:20:09] R: addr:8f56 pagecnt=1 domid:1 gfn:8
>> (XEN) [2014-01-07 03:20:09] gmem:exit:len:$0
>> (XEN) [2014-01-07 03:20:09] gmem:addr:8f57 buf:00000000006e2020 len:$1 domid:1 toaddr:0 dp:ffff83083e5fe000
>> (XEN) [2014-01-07 03:20:09] vaddr:8f57 domid:1
>> (XEN) [2014-01-07 03:20:09] X: vaddr:8f57 domid:1 mfn:64331a
>> (XEN) [2014-01-07 03:20:09] R: addr:8f57 pagecnt=1 domid:1 gfn:8
>> (XEN) [2014-01-07 03:20:09] gmem:exit:len:$0
>> (XEN) [2014-01-07 03:20:09] gmem:addr:6ae9168b buf:00000000006e2020 len:$2 domid:1 toaddr:0 dp:ffff83083e5fe000
>> (XEN) [2014-01-07 03:20:09] vaddr:6ae9168b domid:1
>> (XEN) [2014-01-07 03:20:09] X: vaddr:6ae9168b domid:1 mfn:ffffffffffffffff
>> (XEN) [2014-01-07 03:20:09] R: domid:1 gfn:6ae91
>> (XEN) [2014-01-07 03:20:09] gmem:exit:len:$2
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> ---
>>   xen/arch/x86/debug.c | 54 +++++++++++++++++++++++++---------------------------
>>   1 file changed, 26 insertions(+), 28 deletions(-)
>>
>> diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
>> index ba6a64d..777e5ba 100644
>> --- a/xen/arch/x86/debug.c
>> +++ b/xen/arch/x86/debug.c
>> @@ -30,16 +30,9 @@
>>    * gdbsx, etc..
>>    */
>>   
>> -#ifdef XEN_KDB_CONFIG
>> -#include "../kdb/include/kdbdefs.h"
>> -#include "../kdb/include/kdbproto.h"
>> -#define DBGP(...) {(kdbdbg) ? kdbp(__VA_ARGS__):0;}
>> -#define DBGP1(...) {(kdbdbg>1) ? kdbp(__VA_ARGS__):0;}
>> -#define DBGP2(...) {(kdbdbg>2) ? kdbp(__VA_ARGS__):0;}
>> -#else
>> -#define DBGP1(...) ((void)0)
>> -#define DBGP2(...) ((void)0)
>> -#endif
>> +static volatile int dbg_debug;
> Using volatile is almost always wrong. Why do you think it is needed
> here?

This was from Mukesh Rathor:

http://lists.xen.org/archives/html/xen-devel/2014-01/msg00426.html

I saw no reason to make it volatile, but maybe "kdb" needs this? Happy to change any way you want.

> If anything this variable is exactly the opposite, i..e __read_mostly or
> even const (given that I can't see anything which writes it I suppose
> this is a compile time setting?)

That has been how I have been testing it so far (changing the source to set values).  Mukesh claims to be able to change it at will.  Not sure how const may effect this.

       -Don Slutz

> Ian.
>