[Qemu-devel] [PULL for-2.9 0/3] Tracing patches

[Qemu-devel] [PULL for-2.9 0/3] Tracing patches

[Stefan Hajnoczi]

The following changes since commit 08329701199449bde497570dcfdb9c86062baf20:

  qom: Fix regression with 'qom-type' (2017-03-23 17:59:40 +0000)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/tracing-pull-request

for you to fetch changes up to 0d3ef78829332f2fdc323d1b625b60fe9c89119c:

  trace: Avoid abuse of amdvi_mmio_read (2017-03-24 09:21:42 +0000)

----------------------------------------------------------------

----------------------------------------------------------------

Eric Blake (3):
  trace: Fix backwards mirror_yield parameters
  trace: Fix incorrect megasas trace parameters
  trace: Avoid abuse of amdvi_mmio_read

 block/mirror.c       | 5 +++--
 hw/i386/amd_iommu.c  | 3 +--
 hw/scsi/megasas.c    | 6 +++---
 hw/i386/trace-events | 1 +
 4 files changed, 8 insertions(+), 7 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL for-2.9 1/3] trace: Fix backwards mirror_yield parameters

[Stefan Hajnoczi]

From: Eric Blake <eblake@redhat.com>

block/trace-events lists the parameters for mirror_yield
consistently with other mirror events (cnt just after s, like in
mirror_before_sleep; in_flight last, like in mirror_yield_in_flight).
But the callers were passing parameters in the wrong order, leading
to poor trace messages, including type truncation when there are
more than 4G dirty sectors involved.  Broken since its introduction
in commit bd48bde.

While touching this, ensure that all callers use the same type
(uint64_t) for cnt, as a later patch will enable the compiler to do
stricter type-checking.

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index ca4baa5..9e2fecc 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -634,7 +634,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
             }
 
             if (s->in_flight >= MAX_IN_FLIGHT) {
-                trace_mirror_yield(s, s->in_flight, s->buf_free_count, -1);
+                trace_mirror_yield(s, UINT64_MAX, s->buf_free_count,
+                                   s->in_flight);
                 mirror_wait_for_io(s);
                 continue;
             }
@@ -809,7 +810,7 @@ static void coroutine_fn mirror_run(void *opaque)
             s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
             if (s->in_flight >= MAX_IN_FLIGHT || s->buf_free_count == 0 ||
                 (cnt == 0 && s->in_flight > 0)) {
-                trace_mirror_yield(s, s->in_flight, s->buf_free_count, cnt);
+                trace_mirror_yield(s, cnt, s->buf_free_count, s->in_flight);
                 mirror_wait_for_io(s);
                 continue;
             } else if (cnt != 0) {
-- 
2.9.3

[Qemu-devel] [PULL for-2.9 2/3] trace: Fix incorrect megasas trace parameters

[Stefan Hajnoczi]

From: Eric Blake <eblake@redhat.com>

hw/scsi/trace-events lists cmd as the first parameter for both
megasas_iovec_overflow and megasas_iovec_underflow, but the caller
was mistakenly passing cmd->iov_size twice instead of the command
index.  Also, trace_megasas_abort_invalid is called with parameters
in the wrong order.  Broken since its introduction in commit
e8f943c3.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/megasas.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index e3d59b7..84b8caf 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -291,7 +291,7 @@ static int megasas_map_sgl(MegasasState *s, MegasasCmd *cmd, union mfi_sgl *sgl)
     if (cmd->iov_size > iov_size) {
         trace_megasas_iovec_overflow(cmd->index, iov_size, cmd->iov_size);
     } else if (cmd->iov_size < iov_size) {
-        trace_megasas_iovec_underflow(cmd->iov_size, iov_size, cmd->iov_size);
+        trace_megasas_iovec_underflow(cmd->index, iov_size, cmd->iov_size);
     }
     cmd->iov_offset = 0;
     return 0;
@@ -1924,8 +1924,8 @@ static int megasas_handle_abort(MegasasState *s, MegasasCmd *cmd)
         abort_ctx &= (uint64_t)0xFFFFFFFF;
     }
     if (abort_cmd->context != abort_ctx) {
-        trace_megasas_abort_invalid_context(cmd->index, abort_cmd->index,
-                                            abort_cmd->context);
+        trace_megasas_abort_invalid_context(cmd->index, abort_cmd->context,
+                                            abort_cmd->index);
         s->event_count++;
         return MFI_STAT_ABORT_NOT_POSSIBLE;
     }
-- 
2.9.3

[Qemu-devel] [PULL for-2.9 3/3] trace: Avoid abuse of amdvi_mmio_read

[Stefan Hajnoczi]

From: Eric Blake <eblake@redhat.com>

hw/i386/trace-events has an amdvi_mmio_read trace that is used for
both normal reads (listing the register name, address, size, and
offset) and for an error case (abusing the register name to show
an error message, the address to show the maximum value supported,
then shoehorning address and size into the size and offset
parameters).  The change from a wide address to a narrower size
parameter could truncate a (rather-large) bogus read attempt, so
it's better to create a separate dedicated trace with correct types,
rather than abusing the trace mechanism.  Broken since its
introduction in commit d29a09c.

[Change trace event argument type from hwaddr to uint64_t since
user-defined types should not be used for trace events.  This fixes a
build failure with LTTng UST.
--Stefan]

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/i386/amd_iommu.c  | 3 +--
 hw/i386/trace-events | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index e0732cc..f86a40a 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -572,8 +572,7 @@ static uint64_t amdvi_mmio_read(void *opaque, hwaddr addr, unsigned size)
 
     uint64_t val = -1;
     if (addr + size > AMDVI_MMIO_SIZE) {
-        trace_amdvi_mmio_read("error: addr outside region: max ",
-                (uint64_t)AMDVI_MMIO_SIZE, addr, size);
+        trace_amdvi_mmio_read_invalid(AMDVI_MMIO_SIZE, addr, size);
         return (uint64_t)-1;
     }
 
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 88ad5e4..baed874 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -37,6 +37,7 @@ amdvi_cache_update(uint16_t domid, uint8_t bus, uint8_t slot, uint8_t func, uint
 amdvi_completion_wait_fail(uint64_t addr) "error: fail to write at address 0x%"PRIx64
 amdvi_mmio_write(const char *reg, uint64_t addr, unsigned size, uint64_t val, uint64_t offset) "%s write addr 0x%"PRIx64", size %u, val 0x%"PRIx64", offset 0x%"PRIx64
 amdvi_mmio_read(const char *reg, uint64_t addr, unsigned size, uint64_t offset) "%s read addr 0x%"PRIx64", size %u offset 0x%"PRIx64
+amdvi_mmio_read_invalid(int max, uint64_t addr, unsigned size) "error: addr outside region (max 0x%x): read addr 0x%" PRIx64 ", size %u"
 amdvi_command_error(uint64_t status) "error: Executing commands with command buffer disabled 0x%"PRIx64
 amdvi_command_read_fail(uint64_t addr, uint32_t head) "error: fail to access memory at 0x%"PRIx64" + 0x%"PRIx32
 amdvi_command_exec(uint32_t head, uint32_t tail, uint64_t buf) "command buffer head at 0x%"PRIx32" command buffer tail at 0x%"PRIx32" command buffer base at 0x%"PRIx64
-- 
2.9.3

Re: [Qemu-devel] [PULL for-2.9 0/3] Tracing patches

[Peter Maydell]

On 24 March 2017 at 14:08, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 08329701199449bde497570dcfdb9c86062baf20:
>
>   qom: Fix regression with 'qom-type' (2017-03-23 17:59:40 +0000)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/tracing-pull-request
>
> for you to fetch changes up to 0d3ef78829332f2fdc323d1b625b60fe9c89119c:
>
>   trace: Avoid abuse of amdvi_mmio_read (2017-03-24 09:21:42 +0000)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
>
> Eric Blake (3):
>   trace: Fix backwards mirror_yield parameters
>   trace: Fix incorrect megasas trace parameters
>   trace: Avoid abuse of amdvi_mmio_read
>
>  block/mirror.c       | 5 +++--
>  hw/i386/amd_iommu.c  | 3 +--
>  hw/scsi/megasas.c    | 6 +++---
>  hw/i386/trace-events | 1 +
>  4 files changed, 8 insertions(+), 7 deletions(-)
>

Applied, thanks.

-- PMM