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

[Mihail Abakumov]

Alistair Francis писал 2017-09-26 20:13:
> On Tue, Sep 26, 2017 at 4:04 AM, Mihail Abakumov
> <address@hidden> wrote:
> > Added some helper features for windbgstub.
> >
> > Signed-off-by: Mihail Abakumov <address@hidden>
> > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> > Signed-off-by: Dmitriy Koltunov <address@hidden>
> > ---
> >  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