Re: [Qemu-devel] [PATCH v3] hw/virtio/virtio-mmio: Convert DPRINTF to trace and log
Gotcha, thanks for the tip! I'll resend updated patch soon. On Fri, May 3, 2019 at 5:12 PM Philippe Mathieu-Daudé wrote: > Hi Li, > > On 5/3/19 10:46 AM, Boxuan Li wrote: > > Use traces for debug message and qemu_log_mask for errors. > > > > Signed-off-by: Boxuan Li > > --- > > v1: > https://patchew.org/QEMU/20190428110258.86681-1-libox...@connect.hku.hk/ > > v2: > https://patchew.org/QEMU/20190501081039.58938-1-libox...@connect.hku.hk/ > > v3: Use qemu_log_mask for errors > > --- > > hw/virtio/trace-events | 7 +++ > > hw/virtio/virtio-mmio.c | 44 > +--- > > 2 files changed, 28 insertions(+), 23 deletions(-) > > > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > > index 60c649c4bc..81cbe1c4b2 100644 > > --- a/hw/virtio/trace-events > > +++ b/hw/virtio/trace-events > > @@ -46,3 +46,10 @@ virtio_balloon_handle_output(const char *name, > uint64_t gpa) "section name: %s g > > virtio_balloon_get_config(uint32_t num_pages, uint32_t actual) > "num_pages: %d actual: %d" > > virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual: > %d oldactual: %d" > > virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon > target: 0x%"PRIx64" num_pages: %d" > > + > > +# virtio-mmio.c > > +virtio_mmio_read(int offset) "virtio_mmio_read offset 0x%x" > > +virtio_mmio_write_offset(int offset, uint64_t value) "virtio_mmio_write > offset 0x%x value 0x%" PRIx64 > > If possible, use uint64_t/PRIx64 for offset (to avoid casting to int). > > > +virtio_mmio_guest_page(uint64_t size, int shift) "guest page size 0x%" > PRIx64 " shift %d" > > +virtio_mmio_queue_write(int value, int max_size) "mmio_queue write %d > max %d" > > +virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d" > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > > index 5807aa87fe..c8a9d974b3 100644 > > --- a/hw/virtio/virtio-mmio.c > > +++ b/hw/virtio/virtio-mmio.c > > @@ -27,16 +27,8 @@ > > #include "sysemu/kvm.h" > > #include "hw/virtio/virtio-bus.h" > > #include "qemu/error-report.h" > > - > > -/* #define DEBUG_VIRTIO_MMIO */ > > - > > -#ifdef DEBUG_VIRTIO_MMIO > > - > > -#define DPRINTF(fmt, ...) \ > > -do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0) > > -#else > > -#define DPRINTF(fmt, ...) do {} while (0) > > -#endif > > +#include "qemu/log.h" > > +#include "trace.h" > > > > /* QOM macros */ > > /* virtio-mmio-bus */ > > @@ -107,7 +99,7 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr > offset, unsigned size) > > VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; > > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > > > -DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset); > > +trace_virtio_mmio_read((int)offset); > > > > if (!vdev) { > > /* If no backend is present, we treat most registers as > > @@ -144,7 +136,9 @@ static uint64_t virtio_mmio_read(void *opaque, > hwaddr offset, unsigned size) > > } > > } > > if (size != 4) { > > -DPRINTF("wrong size access to register!\n"); > > +qemu_log_mask(LOG_GUEST_ERROR, > > +"%s: wrong size access to register!\n", > > +__func__); > > QEMU codying style is to indent the arguments aligned, after the caller > parenthesis: > > qemu_log_mask(LOG_GUEST_ERROR, > "%s: wrong size access to register!\n", > __func__); > > That said: > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé > > Thanks for the cleanup! > > > return 0; > > } > > switch (offset) { > > @@ -182,10 +176,12 @@ static uint64_t virtio_mmio_read(void *opaque, > hwaddr offset, unsigned size) > > case VIRTIO_MMIO_QUEUE_ALIGN: > > case VIRTIO_MMIO_QUEUE_NOTIFY: > > case VIRTIO_MMIO_INTERRUPT_ACK: > > -DPRINTF("read of write-only register\n"); > > +qemu_log_mask(LOG_GUEST_ERROR, > > +"%s: read of write-only register\n", > > +__func__); > > return 0; > > default: > > -DPRINTF("bad register offset\n"); > > +qemu_log_mask(LOG_GUEST_ERROR, "%s: bad register offset\n", > __func__); > > return 0; > > } > > return 0; > > @@ -197,8 +193,7 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > > VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; > > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > > > -DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n", > > -(int)offset, value); > > +trace_virtio_mmio_write_offset((int)offset, value); > > > > if (!vdev) { > > /* If no backend is present, we just make all registers > > @@ -226,7 +221,9 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > > return; > > } > > if (size != 4) { > > -DPRINTF("wrong size access to regist
Re: [Qemu-devel] [PATCH v3] hw/virtio/virtio-mmio: Convert DPRINTF to trace and log
Hi Li, On 5/3/19 10:46 AM, Boxuan Li wrote: > Use traces for debug message and qemu_log_mask for errors. > > Signed-off-by: Boxuan Li > --- > v1: https://patchew.org/QEMU/20190428110258.86681-1-libox...@connect.hku.hk/ > v2: https://patchew.org/QEMU/20190501081039.58938-1-libox...@connect.hku.hk/ > v3: Use qemu_log_mask for errors > --- > hw/virtio/trace-events | 7 +++ > hw/virtio/virtio-mmio.c | 44 +--- > 2 files changed, 28 insertions(+), 23 deletions(-) > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > index 60c649c4bc..81cbe1c4b2 100644 > --- a/hw/virtio/trace-events > +++ b/hw/virtio/trace-events > @@ -46,3 +46,10 @@ virtio_balloon_handle_output(const char *name, uint64_t > gpa) "section name: %s g > virtio_balloon_get_config(uint32_t num_pages, uint32_t actual) "num_pages: > %d actual: %d" > virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual: %d > oldactual: %d" > virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon > target: 0x%"PRIx64" num_pages: %d" > + > +# virtio-mmio.c > +virtio_mmio_read(int offset) "virtio_mmio_read offset 0x%x" > +virtio_mmio_write_offset(int offset, uint64_t value) "virtio_mmio_write > offset 0x%x value 0x%" PRIx64 If possible, use uint64_t/PRIx64 for offset (to avoid casting to int). > +virtio_mmio_guest_page(uint64_t size, int shift) "guest page size 0x%" > PRIx64 " shift %d" > +virtio_mmio_queue_write(int value, int max_size) "mmio_queue write %d max %d" > +virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d" > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 5807aa87fe..c8a9d974b3 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -27,16 +27,8 @@ > #include "sysemu/kvm.h" > #include "hw/virtio/virtio-bus.h" > #include "qemu/error-report.h" > - > -/* #define DEBUG_VIRTIO_MMIO */ > - > -#ifdef DEBUG_VIRTIO_MMIO > - > -#define DPRINTF(fmt, ...) \ > -do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0) > -#else > -#define DPRINTF(fmt, ...) do {} while (0) > -#endif > +#include "qemu/log.h" > +#include "trace.h" > > /* QOM macros */ > /* virtio-mmio-bus */ > @@ -107,7 +99,7 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr > offset, unsigned size) > VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > -DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset); > +trace_virtio_mmio_read((int)offset); > > if (!vdev) { > /* If no backend is present, we treat most registers as > @@ -144,7 +136,9 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr > offset, unsigned size) > } > } > if (size != 4) { > -DPRINTF("wrong size access to register!\n"); > +qemu_log_mask(LOG_GUEST_ERROR, > +"%s: wrong size access to register!\n", > +__func__); QEMU codying style is to indent the arguments aligned, after the caller parenthesis: qemu_log_mask(LOG_GUEST_ERROR, "%s: wrong size access to register!\n", __func__); That said: Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Thanks for the cleanup! > return 0; > } > switch (offset) { > @@ -182,10 +176,12 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr > offset, unsigned size) > case VIRTIO_MMIO_QUEUE_ALIGN: > case VIRTIO_MMIO_QUEUE_NOTIFY: > case VIRTIO_MMIO_INTERRUPT_ACK: > -DPRINTF("read of write-only register\n"); > +qemu_log_mask(LOG_GUEST_ERROR, > +"%s: read of write-only register\n", > +__func__); > return 0; > default: > -DPRINTF("bad register offset\n"); > +qemu_log_mask(LOG_GUEST_ERROR, "%s: bad register offset\n", > __func__); > return 0; > } > return 0; > @@ -197,8 +193,7 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > -DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n", > -(int)offset, value); > +trace_virtio_mmio_write_offset((int)offset, value); > > if (!vdev) { > /* If no backend is present, we just make all registers > @@ -226,7 +221,9 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > return; > } > if (size != 4) { > -DPRINTF("wrong size access to register!\n"); > +qemu_log_mask(LOG_GUEST_ERROR, > +"%s: wrong size access to register!\n", > +__func__); > return; > } > switch (offset) { > @@ -246,8 +243,7 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > if (proxy->guest_page_shift > 31) {
Re: [Qemu-devel] [PATCH v3] hw/virtio/virtio-mmio: Convert DPRINTF to trace and log
Boxuan Li writes: > Use traces for debug message and qemu_log_mask for errors. > > Signed-off-by: Boxuan Li Reviewed-by: Alex Bennée > --- > v1: https://patchew.org/QEMU/20190428110258.86681-1-libox...@connect.hku.hk/ > v2: https://patchew.org/QEMU/20190501081039.58938-1-libox...@connect.hku.hk/ > v3: Use qemu_log_mask for errors > --- > hw/virtio/trace-events | 7 +++ > hw/virtio/virtio-mmio.c | 44 +--- > 2 files changed, 28 insertions(+), 23 deletions(-) > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events > index 60c649c4bc..81cbe1c4b2 100644 > --- a/hw/virtio/trace-events > +++ b/hw/virtio/trace-events > @@ -46,3 +46,10 @@ virtio_balloon_handle_output(const char *name, uint64_t > gpa) "section name: %s g > virtio_balloon_get_config(uint32_t num_pages, uint32_t actual) "num_pages: > %d actual: %d" > virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual: %d > oldactual: %d" > virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon > target: 0x%"PRIx64" num_pages: %d" > + > +# virtio-mmio.c > +virtio_mmio_read(int offset) "virtio_mmio_read offset 0x%x" > +virtio_mmio_write_offset(int offset, uint64_t value) "virtio_mmio_write > offset 0x%x value 0x%" PRIx64 > +virtio_mmio_guest_page(uint64_t size, int shift) "guest page size 0x%" > PRIx64 " shift %d" > +virtio_mmio_queue_write(int value, int max_size) "mmio_queue write %d max %d" > +virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d" > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 5807aa87fe..c8a9d974b3 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -27,16 +27,8 @@ > #include "sysemu/kvm.h" > #include "hw/virtio/virtio-bus.h" > #include "qemu/error-report.h" > - > -/* #define DEBUG_VIRTIO_MMIO */ > - > -#ifdef DEBUG_VIRTIO_MMIO > - > -#define DPRINTF(fmt, ...) \ > -do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0) > -#else > -#define DPRINTF(fmt, ...) do {} while (0) > -#endif > +#include "qemu/log.h" > +#include "trace.h" > > /* QOM macros */ > /* virtio-mmio-bus */ > @@ -107,7 +99,7 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr > offset, unsigned size) > VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > -DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset); > +trace_virtio_mmio_read((int)offset); > > if (!vdev) { > /* If no backend is present, we treat most registers as > @@ -144,7 +136,9 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr > offset, unsigned size) > } > } > if (size != 4) { > -DPRINTF("wrong size access to register!\n"); > +qemu_log_mask(LOG_GUEST_ERROR, > +"%s: wrong size access to register!\n", > +__func__); > return 0; > } > switch (offset) { > @@ -182,10 +176,12 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr > offset, unsigned size) > case VIRTIO_MMIO_QUEUE_ALIGN: > case VIRTIO_MMIO_QUEUE_NOTIFY: > case VIRTIO_MMIO_INTERRUPT_ACK: > -DPRINTF("read of write-only register\n"); > +qemu_log_mask(LOG_GUEST_ERROR, > +"%s: read of write-only register\n", > +__func__); > return 0; > default: > -DPRINTF("bad register offset\n"); > +qemu_log_mask(LOG_GUEST_ERROR, "%s: bad register offset\n", > __func__); > return 0; > } > return 0; > @@ -197,8 +193,7 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > > -DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n", > -(int)offset, value); > +trace_virtio_mmio_write_offset((int)offset, value); > > if (!vdev) { > /* If no backend is present, we just make all registers > @@ -226,7 +221,9 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > return; > } > if (size != 4) { > -DPRINTF("wrong size access to register!\n"); > +qemu_log_mask(LOG_GUEST_ERROR, > +"%s: wrong size access to register!\n", > +__func__); > return; > } > switch (offset) { > @@ -246,8 +243,7 @@ static void virtio_mmio_write(void *opaque, hwaddr > offset, uint64_t value, > if (proxy->guest_page_shift > 31) { > proxy->guest_page_shift = 0; > } > -DPRINTF("guest page size %" PRIx64 " shift %d\n", value, > -proxy->guest_page_shift); > +trace_virtio_mmio_guest_page(value, proxy->guest_page_shift); > break; > case VIRTIO_MMIO_QUEUE_SEL: > if (value < VIRTIO_QUEUE_MAX) { > @@ -255,7 +251,7 @@ static void virtio_mmio_write(void *opaque, hwaddr