Re: [Qemu-devel] [PATCH 05/43] windbg: added helper features

[Mihail Abakumov <mikhail.abakumov@ispras.ru>]

Alistair Francis писал 2017-09-26 20:13:
> On Tue, Sep 26, 2017 at 4:04 AM, Mihail Abakumov
> <mikhail.abakumov@ispras.ru> wrote:
>> Added some helper features for windbgstub.
>> 
>> Signed-off-by: Mihail Abakumov <mikhail.abakumov@ispras.ru>
>> Signed-off-by: Pavel Dovgalyuk <dovgaluk@ispras.ru>
>> Signed-off-by: Dmitriy Koltunov <koltunov@ispras.ru>
>> ---
>>  include/exec/windbgstub-utils.h |   46 
>> +++++++++++++++++++++++++++++++++++++++
>>  include/exec/windbgstub.h       |    3 +++
>>  windbgstub.c                    |    1 +
>>  3 files changed, 50 insertions(+)
>> 
>> diff --git a/include/exec/windbgstub-utils.h 
>> b/include/exec/windbgstub-utils.h
>> index 2390597f1f..65f336e4bf 100755
>> --- a/include/exec/windbgstub-utils.h
>> +++ b/include/exec/windbgstub-utils.h
>> @@ -13,7 +13,53 @@
>>  #define WINDBGSTUB_UTILS_H
>> 
>>  #include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>> +#include "cpu.h"
>>  #include "exec/windbgstub.h"
>>  #include "exec/windbgkd.h"
>> 
>> +#ifndef TARGET_I386
>> +#error Unsupported Architecture
>> +#endif
>> +#ifdef TARGET_X86_64 /* Unimplemented yet */
>> +#error Unsupported Architecture
>> +#endif
>> +
>> +#if (WINDBG_DEBUG_ON)
>> +
>> +# define WINDBG_DEBUG(...) do {    \
>> +    printf("Debug: " __VA_ARGS__); \
>> +    printf("\n");                  \
>> +} while (false)
>> +
>> +# define WINDBG_ERROR(...) do {    \
>> +    printf("Error: " __VA_ARGS__); \
>> +    printf("\n");                  \
>> +} while (false)
> 
> Use qemu_log() instead of printf().
> 
> Have a look as some other files for the usual way we handle debug 
> printing.
> 

Thanks for your feedback. I'm replaced printf() with qemu_log().

You can find new version here:
     
https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03912.html

>> +
>> +#else
>> +
>> +# define WINDBG_DEBUG(...)
>> +# define WINDBG_ERROR(...) error_report(WINDBG ": " __VA_ARGS__)
>> +
>> +#endif
>> +
>> +#define FMT_ADDR "addr:0x" TARGET_FMT_lx
>> +#define FMT_ERR  "Error:%d"
>> +
>> +#define UINT8_P(ptr) ((uint8_t *) (ptr))
>> +#define UINT32_P(ptr) ((uint32_t *) (ptr))
>> +#define FIELD_P(type, field, ptr) ((typeof_field(type, field) *) 
>> (ptr))
>> +#define PTR(var) UINT8_P(&var)
>> +
>> +#define M64_SIZE sizeof(DBGKD_MANIPULATE_STATE64)
>> +
>> +#define sizeof_field(type, field) sizeof(((type *) NULL)->field)
>> +
>> +#define READ_VMEM(cpu, addr, type) ({                         \
>> +    type _t;                                                  \
>> +    cpu_memory_rw_debug(cpu, addr, PTR(_t), sizeof(type), 0); \
>> +    _t;                                                       \
>> +})
>> +
>>  #endif
>> diff --git a/include/exec/windbgstub.h b/include/exec/windbgstub.h
>> index 1a6e1cc6e5..703fc26b8f 100755
>> --- a/include/exec/windbgstub.h
>> +++ b/include/exec/windbgstub.h
>> @@ -12,6 +12,9 @@
>>  #ifndef WINDBGSTUB_H
>>  #define WINDBGSTUB_H
>> 
>> +#define WINDBG "windbg"
>> +#define WINDBG_DEBUG_ON false
> 
> You should have a check here to see if the user has already set
> WINDBG_DEBUG_ON to allow people to set it during build time.
> 
> Thanks,
> Alistair
> 

Yes, you're right. I done it.

>> +
>>  int windbg_server_start(const char *device);
>> 
>>  #endif
>> diff --git a/windbgstub.c b/windbgstub.c
>> index 4951f59203..3830446988 100755
>> --- a/windbgstub.c
>> +++ b/windbgstub.c
>> @@ -11,6 +11,7 @@
>> 
>>  #include "qemu/osdep.h"
>>  #include "exec/windbgstub.h"
>> +#include "exec/windbgstub-utils.h"
>> 
>>  int windbg_server_start(const char *device)
>>  {
>> 
>> 

--
Thanks,
Mihail Abakumov