[v4,4/9] {hmp, hw/pvrdma}: Expose device internals via monitor interface

Commit Message

Yuval Shaia March 3, 2019, 8:33 p.m. UTC

Allow interrogating device internals through HMP interface.
The exposed indicators can be used for troubleshooting by developers or
sysadmin.
There is no need to expose these attributes to a management system (e.x.
libvirt) because (1) most of them are not "device-management' related
info and (2) there is no guarantee the interface is stable.

Signed-off-by: Yuval Shaia <[e-mail]>
---
 hmp-commands-info.hx       | 14 ++++++++
 hmp.c                      | 27 +++++++++++++++
 hmp.h                      |  1 +
 hw/rdma/Makefile.objs      |  2 +-
 hw/rdma/rdma_backend.c     | 70 +++++++++++++++++++++++++++++---------
 hw/rdma/rdma_hmp.c         | 30 ++++++++++++++++
 hw/rdma/rdma_rm.c          | 60 ++++++++++++++++++++++++++++++++
 hw/rdma/rdma_rm.h          |  1 +
 hw/rdma/rdma_rm_defs.h     | 27 ++++++++++++++-
 hw/rdma/vmw/pvrdma.h       |  5 +++
 hw/rdma/vmw/pvrdma_main.c  | 21 ++++++++++++
 include/hw/rdma/rdma_hmp.h | 40 ++++++++++++++++++++++
 12 files changed, 279 insertions(+), 19 deletions(-)
 create mode 100644 hw/rdma/rdma_hmp.c
 create mode 100644 include/hw/rdma/rdma_hmp.h

Comments

Yuval Shaia March 6, 2019, 10:22 a.m. UTC | #1

On Sun, Mar 03, 2019 at 10:33:40PM +0200, Yuval Shaia wrote:
> Allow interrogating device internals through HMP interface.
> The exposed indicators can be used for troubleshooting by developers or
> sysadmin.
> There is no need to expose these attributes to a management system (e.x.
> libvirt) because (1) most of them are not "device-management' related
> info and (2) there is no guarantee the interface is stable.
> 
> Signed-off-by: Yuval Shaia <[e-mail]>
> ---
>  hmp-commands-info.hx       | 14 ++++++++
>  hmp.c                      | 27 +++++++++++++++
>  hmp.h                      |  1 +
>  hw/rdma/Makefile.objs      |  2 +-
>  hw/rdma/rdma_backend.c     | 70 +++++++++++++++++++++++++++++---------
>  hw/rdma/rdma_hmp.c         | 30 ++++++++++++++++
>  hw/rdma/rdma_rm.c          | 60 ++++++++++++++++++++++++++++++++
>  hw/rdma/rdma_rm.h          |  1 +
>  hw/rdma/rdma_rm_defs.h     | 27 ++++++++++++++-
>  hw/rdma/vmw/pvrdma.h       |  5 +++
>  hw/rdma/vmw/pvrdma_main.c  | 21 ++++++++++++
>  include/hw/rdma/rdma_hmp.h | 40 ++++++++++++++++++++++
>  12 files changed, 279 insertions(+), 19 deletions(-)
>  create mode 100644 hw/rdma/rdma_hmp.c
>  create mode 100644 include/hw/rdma/rdma_hmp.h
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b944d..c59444c461 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -202,6 +202,20 @@ STEXI
>  @item info pic
>  @findex info pic
>  Show PIC state.
> +ETEXI
> +
> +    {
> +        .name       = "rdma",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show RDMA state",
> +        .cmd        = hmp_info_rdma,
> +    },
> +
> +STEXI
> +@item info rdma
> +@findex info rdma
> +Show RDMA state.
>  ETEXI
>  
>      {
> diff --git a/hmp.c b/hmp.c
> index 1e006eeb49..68511b2441 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -51,6 +51,7 @@
>  #include "qemu/error-report.h"
>  #include "exec/ramlist.h"
>  #include "hw/intc/intc.h"
> +#include "hw/rdma/rdma_hmp.h"
>  #include "migration/snapshot.h"
>  #include "migration/misc.h"
>  
> @@ -968,6 +969,32 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
>                                     hmp_info_pic_foreach, mon);
>  }
>  
> +static int hmp_info_rdma_foreach(Object *obj, void *opaque)
> +{
> +    RdmaStatsProvider *rdma;
> +    RdmaStatsProviderClass *k;
> +    Monitor *mon = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_RDMA_STATS_PROVIDER)) {
> +        rdma = RDMA_STATS_PROVIDER(obj);
> +        k = RDMA_STATS_PROVIDER_GET_CLASS(obj);
> +        if (k->print_statistics) {
> +            k->print_statistics(mon, rdma);
> +        } else {
> +            monitor_printf(mon, "RDMA statistics not available for %s.\n",
> +                           object_get_typename(obj));
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +void hmp_info_rdma(Monitor *mon, const QDict *qdict)
> +{
> +    object_child_foreach_recursive(object_get_root(),
> +                                   hmp_info_rdma_foreach, mon);
> +}
> +

Hi Markus and David,
Is this implementation is acceptable by you?

Thanks,
Yuval


>  void hmp_info_pci(Monitor *mon, const QDict *qdict)
>  {
>      PciInfoList *info_list, *info;
> diff --git a/hmp.h b/hmp.h
> index 5f1addcca2..666949afc3 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
>  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
>  void hmp_info_irq(Monitor *mon, const QDict *qdict);
>  void hmp_info_pic(Monitor *mon, const QDict *qdict);
> +void hmp_info_rdma(Monitor *mon, const QDict *qdict);
>  void hmp_info_pci(Monitor *mon, const QDict *qdict);
>  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
>  void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
> index bd36cbf51c..dd59faff51 100644
> --- a/hw/rdma/Makefile.objs
> +++ b/hw/rdma/Makefile.objs
> @@ -1,5 +1,5 @@
>  ifeq ($(CONFIG_PVRDMA),y)
> -obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
> +obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma_hmp.o
>  obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
>                       vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
>  endif
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index 9679b842d1..bc2fefcf93 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -64,9 +64,9 @@ static inline void complete_work(enum ibv_wc_status status, uint32_t vendor_err,
>      comp_handler(ctx, &wc);
>  }
>  
> -static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> +static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
>  {
> -    int i, ne;
> +    int i, ne, total_ne = 0;
>      BackendCtx *bctx;
>      struct ibv_wc wc[2];
>  
> @@ -89,12 +89,18 @@ static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
>              rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
>              g_free(bctx);
>          }
> +        total_ne += ne;
>      } while (ne > 0);
> +    atomic_sub(&rdma_dev_res->stats.missing_cqe, total_ne);
>      qemu_mutex_unlock(&rdma_dev_res->lock);
>  
>      if (ne < 0) {
>          rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
>      }
> +
> +    rdma_dev_res->stats.completions += total_ne;
> +
> +    return total_ne;
>  }
>  
>  static void *comp_handler_thread(void *arg)
> @@ -122,6 +128,9 @@ static void *comp_handler_thread(void *arg)
>      while (backend_dev->comp_thread.run) {
>          do {
>              rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
> +            if (!rc) {
> +                backend_dev->rdma_dev_res->stats.poll_cq_ppoll_to++;
> +            }
>          } while (!rc && backend_dev->comp_thread.run);
>  
>          if (backend_dev->comp_thread.run) {
> @@ -138,6 +147,7 @@ static void *comp_handler_thread(void *arg)
>                                    errno);
>              }
>  
> +            backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
>              rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
>  
>              ibv_ack_cq_events(ev_cq, 1);
> @@ -271,7 +281,13 @@ int rdma_backend_query_port(RdmaBackendDev *backend_dev,
>  
>  void rdma_backend_poll_cq(RdmaDeviceResources *rdma_dev_res, RdmaBackendCQ *cq)
>  {
> -    rdma_poll_cq(rdma_dev_res, cq->ibcq);
> +    int polled;
> +
> +    rdma_dev_res->stats.poll_cq_from_guest++;
> +    polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
> +    if (!polled) {
> +        rdma_dev_res->stats.poll_cq_from_guest_empty++;
> +    }
>  }
>  
>  static GHashTable *ah_hash;
> @@ -333,7 +349,7 @@ static void ah_cache_init(void)
>  
>  static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
>                                  struct ibv_sge *dsge, struct ibv_sge *ssge,
> -                                uint8_t num_sge)
> +                                uint8_t num_sge, uint64_t *total_length)
>  {
>      RdmaRmMR *mr;
>      int ssge_idx;
> @@ -349,6 +365,8 @@ static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
>          dsge->length = ssge[ssge_idx].length;
>          dsge->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
>  
> +        *total_length += dsge->length;
> +
>          dsge++;
>      }
>  
> @@ -445,8 +463,10 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
>              rc = mad_send(backend_dev, sgid_idx, sgid, sge, num_sge);
>              if (rc) {
>                  complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
> +                backend_dev->rdma_dev_res->stats.mad_tx_err++;
>              } else {
>                  complete_work(IBV_WC_SUCCESS, 0, ctx);
> +                backend_dev->rdma_dev_res->stats.mad_tx++;
>              }
>          }
>          return;
> @@ -458,20 +478,21 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
>      rc = rdma_rm_alloc_cqe_ctx(backend_dev->rdma_dev_res, &bctx_id, bctx);
>      if (unlikely(rc)) {
>          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> -        goto out_free_bctx;
> +        goto err_free_bctx;
>      }
>  
> -    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge);
> +    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge,
> +                              &backend_dev->rdma_dev_res->stats.tx_len);
>      if (rc) {
>          complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>      }
>  
>      if (qp_type == IBV_QPT_UD) {
>          wr.wr.ud.ah = create_ah(backend_dev, qp->ibpd, sgid_idx, dgid);
>          if (!wr.wr.ud.ah) {
>              complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> -            goto out_dealloc_cqe_ctx;
> +            goto err_dealloc_cqe_ctx;
>          }
>          wr.wr.ud.remote_qpn = dqpn;
>          wr.wr.ud.remote_qkey = dqkey;
> @@ -488,15 +509,19 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
>          rdma_error_report("ibv_post_send fail, qpn=0x%x, rc=%d, errno=%d",
>                            qp->ibqp->qp_num, rc, errno);
>          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>      }
>  
> +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> +    backend_dev->rdma_dev_res->stats.tx++;
> +
>      return;
>  
> -out_dealloc_cqe_ctx:
> +err_dealloc_cqe_ctx:
> +    backend_dev->rdma_dev_res->stats.tx_err++;
>      rdma_rm_dealloc_cqe_ctx(backend_dev->rdma_dev_res, bctx_id);
>  
> -out_free_bctx:
> +err_free_bctx:
>      g_free(bctx);
>  }
>  
> @@ -554,6 +579,9 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
>              rc = save_mad_recv_buffer(backend_dev, sge, num_sge, ctx);
>              if (rc) {
>                  complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> +                rdma_dev_res->stats.mad_rx_bufs_err++;
> +            } else {
> +                rdma_dev_res->stats.mad_rx_bufs++;
>              }
>          }
>          return;
> @@ -565,13 +593,14 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
>      rc = rdma_rm_alloc_cqe_ctx(rdma_dev_res, &bctx_id, bctx);
>      if (unlikely(rc)) {
>          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> -        goto out_free_bctx;
> +        goto err_free_bctx;
>      }
>  
> -    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge);
> +    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge,
> +                              &backend_dev->rdma_dev_res->stats.rx_bufs_len);
>      if (rc) {
>          complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>      }
>  
>      wr.num_sge = num_sge;
> @@ -582,15 +611,19 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
>          rdma_error_report("ibv_post_recv fail, qpn=0x%x, rc=%d, errno=%d",
>                            qp->ibqp->qp_num, rc, errno);
>          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>      }
>  
> +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> +    rdma_dev_res->stats.rx_bufs++;
> +
>      return;
>  
> -out_dealloc_cqe_ctx:
> +err_dealloc_cqe_ctx:
> +    backend_dev->rdma_dev_res->stats.rx_bufs_err++;
>      rdma_rm_dealloc_cqe_ctx(rdma_dev_res, bctx_id);
>  
> -out_free_bctx:
> +err_free_bctx:
>      g_free(bctx);
>  }
>  
> @@ -929,12 +962,14 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
>      bctx = rdma_rm_get_cqe_ctx(backend_dev->rdma_dev_res, cqe_ctx_id);
>      if (unlikely(!bctx)) {
>          rdma_error_report("No matching ctx for req %ld", cqe_ctx_id);
> +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
>          return;
>      }
>  
>      mad = rdma_pci_dma_map(backend_dev->dev, bctx->sge.addr,
>                             bctx->sge.length);
>      if (!mad || bctx->sge.length < msg->umad_len + MAD_HDR_SIZE) {
> +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
>          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_MAD_BUFF,
>                        bctx->up_ctx);
>      } else {
> @@ -949,6 +984,7 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
>          wc.byte_len = msg->umad_len;
>          wc.status = IBV_WC_SUCCESS;
>          wc.wc_flags = IBV_WC_GRH;
> +        backend_dev->rdma_dev_res->stats.mad_rx++;
>          comp_handler(bctx->up_ctx, &wc);
>      }
>  
> diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
> new file mode 100644
> index 0000000000..c5814473c5
> --- /dev/null
> +++ b/hw/rdma/rdma_hmp.c
> @@ -0,0 +1,30 @@
> +/*
> + * RDMA device: Human Monitor interface
> + *
> + * Copyright (C) 2018 Oracle
> + * Copyright (C) 2018 Red Hat Inc
> + *
> + * Authors:
> + *     Yuval Shaia <[e-mail]>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/rdma/rdma_hmp.h"
> +#include "qemu/module.h"
> +
> +static const TypeInfo rdma_hmp_info = {
> +    .name = TYPE_RDMA_STATS_PROVIDER,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(RdmaStatsProviderClass),
> +};
> +
> +static void rdma_hmp_register_types(void)
> +{
> +    type_register_static(&rdma_hmp_info);
> +}
> +
> +type_init(rdma_hmp_register_types)
> diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> index 14580ca379..e019de1a14 100644
> --- a/hw/rdma/rdma_rm.c
> +++ b/hw/rdma/rdma_rm.c
> @@ -16,6 +16,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
> +#include "monitor/monitor.h"
>  
>  #include "trace.h"
>  #include "rdma_utils.h"
> @@ -26,6 +27,58 @@
>  #define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
>  #define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
>  
> +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
> +{
> +    monitor_printf(mon, "\ttx               : %" PRId64 "\n",
> +                   dev_res->stats.tx);
> +    monitor_printf(mon, "\ttx_len           : %" PRId64 "\n",
> +                   dev_res->stats.tx_len);
> +    monitor_printf(mon, "\ttx_err           : %" PRId64 "\n",
> +                   dev_res->stats.tx_err);
> +    monitor_printf(mon, "\trx_bufs          : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs);
> +    monitor_printf(mon, "\trx_bufs_len      : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs_len);
> +    monitor_printf(mon, "\trx_bufs_err      : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs_err);
> +    monitor_printf(mon, "\tcomps            : %" PRId64 "\n",
> +                   dev_res->stats.completions);
> +    monitor_printf(mon, "\tmissing_comps    : %" PRId32 "\n",
> +                   dev_res->stats.missing_cqe);
> +    monitor_printf(mon, "\tpoll_cq (bk)     : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_bk);
> +    monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_ppoll_to);
> +    monitor_printf(mon, "\tpoll_cq (fe)     : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_guest);
> +    monitor_printf(mon, "\tpoll_cq_empty    : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_guest_empty);
> +    monitor_printf(mon, "\tmad_tx           : %" PRId64 "\n",
> +                   dev_res->stats.mad_tx);
> +    monitor_printf(mon, "\tmad_tx_err       : %" PRId64 "\n",
> +                   dev_res->stats.mad_tx_err);
> +    monitor_printf(mon, "\tmad_rx           : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx);
> +    monitor_printf(mon, "\tmad_rx_err       : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_err);
> +    monitor_printf(mon, "\tmad_rx_bufs      : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_bufs);
> +    monitor_printf(mon, "\tmad_rx_bufs_err  : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_bufs_err);
> +    monitor_printf(mon, "\tPDs              : %" PRId32 "\n",
> +                   dev_res->pd_tbl.used);
> +    monitor_printf(mon, "\tMRs              : %" PRId32 "\n",
> +                   dev_res->mr_tbl.used);
> +    monitor_printf(mon, "\tUCs              : %" PRId32 "\n",
> +                   dev_res->uc_tbl.used);
> +    monitor_printf(mon, "\tQPs              : %" PRId32 "\n",
> +                   dev_res->qp_tbl.used);
> +    monitor_printf(mon, "\tCQs              : %" PRId32 "\n",
> +                   dev_res->cq_tbl.used);
> +    monitor_printf(mon, "\tCEQ_CTXs         : %" PRId32 "\n",
> +                   dev_res->cqe_ctx_tbl.used);
> +}
> +
>  static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
>                                  uint32_t tbl_sz, uint32_t res_sz)
>  {
> @@ -37,6 +90,7 @@ static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
>      tbl->bitmap = bitmap_new(tbl_sz);
>      tbl->tbl_sz = tbl_sz;
>      tbl->res_sz = res_sz;
> +    tbl->used = 0;
>      qemu_mutex_init(&tbl->lock);
>  }
>  
> @@ -76,6 +130,8 @@ static inline void *rdma_res_tbl_alloc(RdmaRmResTbl *tbl, uint32_t *handle)
>  
>      set_bit(*handle, tbl->bitmap);
>  
> +    tbl->used++;
> +
>      qemu_mutex_unlock(&tbl->lock);
>  
>      memset(tbl->tbl + *handle * tbl->res_sz, 0, tbl->res_sz);
> @@ -93,6 +149,7 @@ static inline void rdma_res_tbl_dealloc(RdmaRmResTbl *tbl, uint32_t handle)
>  
>      if (handle < tbl->tbl_sz) {
>          clear_bit(handle, tbl->bitmap);
> +        tbl->used--;
>      }
>  
>      qemu_mutex_unlock(&tbl->lock);
> @@ -620,6 +677,9 @@ int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
>  
>      qemu_mutex_init(&dev_res->lock);
>  
> +    memset(&dev_res->stats, 0, sizeof(dev_res->stats));
> +    atomic_set(&dev_res->stats.missing_cqe, 0);
> +
>      return 0;
>  }
>  
> diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> index 9ec87d2667..a527d306f6 100644
> --- a/hw/rdma/rdma_rm.h
> +++ b/hw/rdma/rdma_rm.h
> @@ -20,6 +20,7 @@
>  #include "rdma_backend_defs.h"
>  #include "rdma_rm_defs.h"
>  
> +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
>  int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
>                   Error **errp);
>  void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
> diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
> index f0ee1f3072..4b8d704cfe 100644
> --- a/hw/rdma/rdma_rm_defs.h
> +++ b/hw/rdma/rdma_rm_defs.h
> @@ -34,7 +34,9 @@
>  #define MAX_QP_INIT_RD_ATOM   16
>  #define MAX_AH                64
>  
> -#define MAX_RM_TBL_NAME 16
> +#define MAX_RM_TBL_NAME             16
> +#define MAX_CONSEQ_EMPTY_POLL_CQ    4096 /* considered as error above this */
> +
>  typedef struct RdmaRmResTbl {
>      char name[MAX_RM_TBL_NAME];
>      QemuMutex lock;
> @@ -42,6 +44,7 @@ typedef struct RdmaRmResTbl {
>      size_t tbl_sz;
>      size_t res_sz;
>      void *tbl;
> +    uint32_t used; /* number of used entries in the table */
>  } RdmaRmResTbl;
>  
>  typedef struct RdmaRmPD {
> @@ -96,6 +99,27 @@ typedef struct RdmaRmPort {
>      enum ibv_port_state state;
>  } RdmaRmPort;
>  
> +typedef struct RdmaRmStats {
> +    uint64_t tx;
> +    uint64_t tx_len;
> +    uint64_t tx_err;
> +    uint64_t rx_bufs;
> +    uint64_t rx_bufs_len;
> +    uint64_t rx_bufs_err;
> +    uint64_t completions;
> +    uint64_t mad_tx;
> +    uint64_t mad_tx_err;
> +    uint64_t mad_rx;
> +    uint64_t mad_rx_err;
> +    uint64_t mad_rx_bufs;
> +    uint64_t mad_rx_bufs_err;
> +    uint64_t poll_cq_from_bk;
> +    uint64_t poll_cq_from_guest;
> +    uint64_t poll_cq_from_guest_empty;
> +    uint64_t poll_cq_ppoll_to;
> +    uint32_t missing_cqe;
> +} RdmaRmStats;
> +
>  typedef struct RdmaDeviceResources {
>      RdmaRmPort port;
>      RdmaRmResTbl pd_tbl;
> @@ -106,6 +130,7 @@ typedef struct RdmaDeviceResources {
>      RdmaRmResTbl cqe_ctx_tbl;
>      GHashTable *qp_hash; /* Keeps mapping between real and emulated */
>      QemuMutex lock;
> +    RdmaRmStats stats;
>  } RdmaDeviceResources;
>  
>  #endif
> diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> index 0879224957..167706ec2c 100644
> --- a/hw/rdma/vmw/pvrdma.h
> +++ b/hw/rdma/vmw/pvrdma.h
> @@ -70,6 +70,10 @@ typedef struct DSRInfo {
>      PvrdmaRing cq;
>  } DSRInfo;
>  
> +typedef struct PVRDMADevStats {
> +    uint64_t commands;
> +} PVRDMADevStats;
> +
>  typedef struct PVRDMADev {
>      PCIDevice parent_obj;
>      MemoryRegion msix;
> @@ -89,6 +93,7 @@ typedef struct PVRDMADev {
>      CharBackend mad_chr;
>      VMXNET3State *func0;
>      Notifier shutdown_notifier;
> +    PVRDMADevStats stats;
>  } PVRDMADev;
>  #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
>  
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index b6061f4b6e..659331ac93 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -25,6 +25,8 @@
>  #include "cpu.h"
>  #include "trace.h"
>  #include "sysemu/sysemu.h"
> +#include "monitor/monitor.h"
> +#include "hw/rdma/rdma_hmp.h"
>  
>  #include "../rdma_rm.h"
>  #include "../rdma_backend.h"
> @@ -55,6 +57,18 @@ static Property pvrdma_dev_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static void pvrdma_print_statistics(Monitor *mon, RdmaStatsProvider *obj)
> +{
> +    PVRDMADev *dev = PVRDMA_DEV(obj);
> +    PCIDevice *pdev = PCI_DEVICE(dev);
> +
> +    monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
> +                   PCI_FUNC(pdev->devfn));
> +    monitor_printf(mon, "\tcommands         : %" PRId64 "\n",
> +                   dev->stats.commands);
> +    rdma_dump_device_counters(mon, &dev->rdma_dev_res);
> +}
> +
>  static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
>                            void *ring_state)
>  {
> @@ -394,6 +408,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
>          if (val == 0) {
>              trace_pvrdma_regs_write(addr, val, "REQUEST", "");
>              pvrdma_exec_cmd(dev);
> +            dev->stats.commands++;
>          }
>          break;
>      default:
> @@ -612,6 +627,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
>          goto out;
>      }
>  
> +    memset(&dev->stats, 0, sizeof(dev->stats));
> +
>      dev->shutdown_notifier.notify = pvrdma_shutdown_notifier;
>      qemu_register_shutdown_notifier(&dev->shutdown_notifier);
>  
> @@ -631,6 +648,7 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    RdmaStatsProviderClass *ir = RDMA_STATS_PROVIDER_CLASS(klass);
>  
>      k->realize = pvrdma_realize;
>      k->exit = pvrdma_exit;
> @@ -642,6 +660,8 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
>      dc->desc = "RDMA Device";
>      dc->props = pvrdma_dev_properties;
>      set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> +
> +    ir->print_statistics = pvrdma_print_statistics;
>  }
>  
>  static const TypeInfo pvrdma_info = {
> @@ -651,6 +671,7 @@ static const TypeInfo pvrdma_info = {
>      .class_init = pvrdma_class_init,
>      .interfaces = (InterfaceInfo[]) {
>          { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +        { TYPE_RDMA_STATS_PROVIDER },
>          { }
>      }
>  };
> diff --git a/include/hw/rdma/rdma_hmp.h b/include/hw/rdma/rdma_hmp.h
> new file mode 100644
> index 0000000000..dd23f2bc84
> --- /dev/null
> +++ b/include/hw/rdma/rdma_hmp.h
> @@ -0,0 +1,40 @@
> +/*
> + * RDMA device: Human Monitor interface
> + *
> + * Copyright (C) 2019 Oracle
> + * Copyright (C) 2019 Red Hat Inc
> + *
> + * Authors:
> + *     Yuval Shaia <[e-mail]>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef RDMA_HMP_H
> +#define RDMA_HMP_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_RDMA_STATS_PROVIDER "rdma"
> +
> +#define RDMA_STATS_PROVIDER_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(RdmaStatsProviderClass, (klass), \
> +                       TYPE_RDMA_STATS_PROVIDER)
> +#define RDMA_STATS_PROVIDER_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(RdmaStatsProviderClass, (obj), \
> +                     TYPE_RDMA_STATS_PROVIDER)
> +#define RDMA_STATS_PROVIDER(obj) \
> +    INTERFACE_CHECK(RdmaStatsProvider, (obj), \
> +                    TYPE_RDMA_STATS_PROVIDER)
> +
> +typedef struct RdmaStatsProvider RdmaStatsProvider;
> +
> +typedef struct RdmaStatsProviderClass {
> +    InterfaceClass parent;
> +
> +    void (*print_statistics)(Monitor *mon, RdmaStatsProvider *obj);
> +} RdmaStatsProviderClass;
> +
> +#endif
> -- 
> 2.17.2
> 
>

Dr. David Alan Gilbert March 6, 2019, 12:20 p.m. UTC | #2

* Yuval Shaia ([e-mail]) wrote:
> On Sun, Mar 03, 2019 at 10:33:40PM +0200, Yuval Shaia wrote:
> > Allow interrogating device internals through HMP interface.
> > The exposed indicators can be used for troubleshooting by developers or
> > sysadmin.
> > There is no need to expose these attributes to a management system (e.x.
> > libvirt) because (1) most of them are not "device-management' related
> > info and (2) there is no guarantee the interface is stable.
> > 
> > Signed-off-by: Yuval Shaia <[e-mail]>
> > ---
> >  hmp-commands-info.hx       | 14 ++++++++
> >  hmp.c                      | 27 +++++++++++++++
> >  hmp.h                      |  1 +
> >  hw/rdma/Makefile.objs      |  2 +-
> >  hw/rdma/rdma_backend.c     | 70 +++++++++++++++++++++++++++++---------
> >  hw/rdma/rdma_hmp.c         | 30 ++++++++++++++++
> >  hw/rdma/rdma_rm.c          | 60 ++++++++++++++++++++++++++++++++
> >  hw/rdma/rdma_rm.h          |  1 +
> >  hw/rdma/rdma_rm_defs.h     | 27 ++++++++++++++-
> >  hw/rdma/vmw/pvrdma.h       |  5 +++
> >  hw/rdma/vmw/pvrdma_main.c  | 21 ++++++++++++
> >  include/hw/rdma/rdma_hmp.h | 40 ++++++++++++++++++++++
> >  12 files changed, 279 insertions(+), 19 deletions(-)
> >  create mode 100644 hw/rdma/rdma_hmp.c
> >  create mode 100644 include/hw/rdma/rdma_hmp.h
> > 
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index cbee8b944d..c59444c461 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -202,6 +202,20 @@ STEXI
> >  @item info pic
> >  @findex info pic
> >  Show PIC state.
> > +ETEXI
> > +
> > +    {
> > +        .name       = "rdma",
> > +        .args_type  = "",
> > +        .params     = "",
> > +        .help       = "show RDMA state",
> > +        .cmd        = hmp_info_rdma,
> > +    },
> > +
> > +STEXI
> > +@item info rdma
> > +@findex info rdma
> > +Show RDMA state.
> >  ETEXI
> >  
> >      {
> > diff --git a/hmp.c b/hmp.c
> > index 1e006eeb49..68511b2441 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -51,6 +51,7 @@
> >  #include "qemu/error-report.h"
> >  #include "exec/ramlist.h"
> >  #include "hw/intc/intc.h"
> > +#include "hw/rdma/rdma_hmp.h"
> >  #include "migration/snapshot.h"
> >  #include "migration/misc.h"
> >  
> > @@ -968,6 +969,32 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
> >                                     hmp_info_pic_foreach, mon);
> >  }
> >  
> > +static int hmp_info_rdma_foreach(Object *obj, void *opaque)
> > +{
> > +    RdmaStatsProvider *rdma;
> > +    RdmaStatsProviderClass *k;
> > +    Monitor *mon = opaque;
> > +
> > +    if (object_dynamic_cast(obj, TYPE_RDMA_STATS_PROVIDER)) {
> > +        rdma = RDMA_STATS_PROVIDER(obj);
> > +        k = RDMA_STATS_PROVIDER_GET_CLASS(obj);
> > +        if (k->print_statistics) {
> > +            k->print_statistics(mon, rdma);
> > +        } else {
> > +            monitor_printf(mon, "RDMA statistics not available for %s.\n",
> > +                           object_get_typename(obj));
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +void hmp_info_rdma(Monitor *mon, const QDict *qdict)
> > +{
> > +    object_child_foreach_recursive(object_get_root(),
> > +                                   hmp_info_rdma_foreach, mon);
> > +}
> > +
> 
> Hi Markus and David,
> Is this implementation is acceptable by you?

I think from the HMP side I'm fine, so:

Acked-by: Dr. David Alan Gilbert <[e-mail]>

I would say that there's really two parts to this patch, all the places
you change your code to gather the stats, and the separate pieces that
wire it into HMP;  if you had to repost for another reason I'd
split the patch like that.

Dave

> Thanks,
> Yuval
> 
> 
> >  void hmp_info_pci(Monitor *mon, const QDict *qdict)
> >  {
> >      PciInfoList *info_list, *info;
> > diff --git a/hmp.h b/hmp.h
> > index 5f1addcca2..666949afc3 100644
> > --- a/hmp.h
> > +++ b/hmp.h
> > @@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
> >  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
> >  void hmp_info_irq(Monitor *mon, const QDict *qdict);
> >  void hmp_info_pic(Monitor *mon, const QDict *qdict);
> > +void hmp_info_rdma(Monitor *mon, const QDict *qdict);
> >  void hmp_info_pci(Monitor *mon, const QDict *qdict);
> >  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
> >  void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> > diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
> > index bd36cbf51c..dd59faff51 100644
> > --- a/hw/rdma/Makefile.objs
> > +++ b/hw/rdma/Makefile.objs
> > @@ -1,5 +1,5 @@
> >  ifeq ($(CONFIG_PVRDMA),y)
> > -obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
> > +obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma_hmp.o
> >  obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
> >                       vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
> >  endif
> > diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> > index 9679b842d1..bc2fefcf93 100644
> > --- a/hw/rdma/rdma_backend.c
> > +++ b/hw/rdma/rdma_backend.c
> > @@ -64,9 +64,9 @@ static inline void complete_work(enum ibv_wc_status status, uint32_t vendor_err,
> >      comp_handler(ctx, &wc);
> >  }
> >  
> > -static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> > +static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> >  {
> > -    int i, ne;
> > +    int i, ne, total_ne = 0;
> >      BackendCtx *bctx;
> >      struct ibv_wc wc[2];
> >  
> > @@ -89,12 +89,18 @@ static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> >              rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
> >              g_free(bctx);
> >          }
> > +        total_ne += ne;
> >      } while (ne > 0);
> > +    atomic_sub(&rdma_dev_res->stats.missing_cqe, total_ne);
> >      qemu_mutex_unlock(&rdma_dev_res->lock);
> >  
> >      if (ne < 0) {
> >          rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
> >      }
> > +
> > +    rdma_dev_res->stats.completions += total_ne;
> > +
> > +    return total_ne;
> >  }
> >  
> >  static void *comp_handler_thread(void *arg)
> > @@ -122,6 +128,9 @@ static void *comp_handler_thread(void *arg)
> >      while (backend_dev->comp_thread.run) {
> >          do {
> >              rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
> > +            if (!rc) {
> > +                backend_dev->rdma_dev_res->stats.poll_cq_ppoll_to++;
> > +            }
> >          } while (!rc && backend_dev->comp_thread.run);
> >  
> >          if (backend_dev->comp_thread.run) {
> > @@ -138,6 +147,7 @@ static void *comp_handler_thread(void *arg)
> >                                    errno);
> >              }
> >  
> > +            backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
> >              rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
> >  
> >              ibv_ack_cq_events(ev_cq, 1);
> > @@ -271,7 +281,13 @@ int rdma_backend_query_port(RdmaBackendDev *backend_dev,
> >  
> >  void rdma_backend_poll_cq(RdmaDeviceResources *rdma_dev_res, RdmaBackendCQ *cq)
> >  {
> > -    rdma_poll_cq(rdma_dev_res, cq->ibcq);
> > +    int polled;
> > +
> > +    rdma_dev_res->stats.poll_cq_from_guest++;
> > +    polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
> > +    if (!polled) {
> > +        rdma_dev_res->stats.poll_cq_from_guest_empty++;
> > +    }
> >  }
> >  
> >  static GHashTable *ah_hash;
> > @@ -333,7 +349,7 @@ static void ah_cache_init(void)
> >  
> >  static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
> >                                  struct ibv_sge *dsge, struct ibv_sge *ssge,
> > -                                uint8_t num_sge)
> > +                                uint8_t num_sge, uint64_t *total_length)
> >  {
> >      RdmaRmMR *mr;
> >      int ssge_idx;
> > @@ -349,6 +365,8 @@ static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
> >          dsge->length = ssge[ssge_idx].length;
> >          dsge->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
> >  
> > +        *total_length += dsge->length;
> > +
> >          dsge++;
> >      }
> >  
> > @@ -445,8 +463,10 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> >              rc = mad_send(backend_dev, sgid_idx, sgid, sge, num_sge);
> >              if (rc) {
> >                  complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
> > +                backend_dev->rdma_dev_res->stats.mad_tx_err++;
> >              } else {
> >                  complete_work(IBV_WC_SUCCESS, 0, ctx);
> > +                backend_dev->rdma_dev_res->stats.mad_tx++;
> >              }
> >          }
> >          return;
> > @@ -458,20 +478,21 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> >      rc = rdma_rm_alloc_cqe_ctx(backend_dev->rdma_dev_res, &bctx_id, bctx);
> >      if (unlikely(rc)) {
> >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> > -        goto out_free_bctx;
> > +        goto err_free_bctx;
> >      }
> >  
> > -    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge);
> > +    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge,
> > +                              &backend_dev->rdma_dev_res->stats.tx_len);
> >      if (rc) {
> >          complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> > -        goto out_dealloc_cqe_ctx;
> > +        goto err_dealloc_cqe_ctx;
> >      }
> >  
> >      if (qp_type == IBV_QPT_UD) {
> >          wr.wr.ud.ah = create_ah(backend_dev, qp->ibpd, sgid_idx, dgid);
> >          if (!wr.wr.ud.ah) {
> >              complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> > -            goto out_dealloc_cqe_ctx;
> > +            goto err_dealloc_cqe_ctx;
> >          }
> >          wr.wr.ud.remote_qpn = dqpn;
> >          wr.wr.ud.remote_qkey = dqkey;
> > @@ -488,15 +509,19 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> >          rdma_error_report("ibv_post_send fail, qpn=0x%x, rc=%d, errno=%d",
> >                            qp->ibqp->qp_num, rc, errno);
> >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> > -        goto out_dealloc_cqe_ctx;
> > +        goto err_dealloc_cqe_ctx;
> >      }
> >  
> > +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> > +    backend_dev->rdma_dev_res->stats.tx++;
> > +
> >      return;
> >  
> > -out_dealloc_cqe_ctx:
> > +err_dealloc_cqe_ctx:
> > +    backend_dev->rdma_dev_res->stats.tx_err++;
> >      rdma_rm_dealloc_cqe_ctx(backend_dev->rdma_dev_res, bctx_id);
> >  
> > -out_free_bctx:
> > +err_free_bctx:
> >      g_free(bctx);
> >  }
> >  
> > @@ -554,6 +579,9 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
> >              rc = save_mad_recv_buffer(backend_dev, sge, num_sge, ctx);
> >              if (rc) {
> >                  complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> > +                rdma_dev_res->stats.mad_rx_bufs_err++;
> > +            } else {
> > +                rdma_dev_res->stats.mad_rx_bufs++;
> >              }
> >          }
> >          return;
> > @@ -565,13 +593,14 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
> >      rc = rdma_rm_alloc_cqe_ctx(rdma_dev_res, &bctx_id, bctx);
> >      if (unlikely(rc)) {
> >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> > -        goto out_free_bctx;
> > +        goto err_free_bctx;
> >      }
> >  
> > -    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge);
> > +    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge,
> > +                              &backend_dev->rdma_dev_res->stats.rx_bufs_len);
> >      if (rc) {
> >          complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> > -        goto out_dealloc_cqe_ctx;
> > +        goto err_dealloc_cqe_ctx;
> >      }
> >  
> >      wr.num_sge = num_sge;
> > @@ -582,15 +611,19 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
> >          rdma_error_report("ibv_post_recv fail, qpn=0x%x, rc=%d, errno=%d",
> >                            qp->ibqp->qp_num, rc, errno);
> >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> > -        goto out_dealloc_cqe_ctx;
> > +        goto err_dealloc_cqe_ctx;
> >      }
> >  
> > +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> > +    rdma_dev_res->stats.rx_bufs++;
> > +
> >      return;
> >  
> > -out_dealloc_cqe_ctx:
> > +err_dealloc_cqe_ctx:
> > +    backend_dev->rdma_dev_res->stats.rx_bufs_err++;
> >      rdma_rm_dealloc_cqe_ctx(rdma_dev_res, bctx_id);
> >  
> > -out_free_bctx:
> > +err_free_bctx:
> >      g_free(bctx);
> >  }
> >  
> > @@ -929,12 +962,14 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
> >      bctx = rdma_rm_get_cqe_ctx(backend_dev->rdma_dev_res, cqe_ctx_id);
> >      if (unlikely(!bctx)) {
> >          rdma_error_report("No matching ctx for req %ld", cqe_ctx_id);
> > +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
> >          return;
> >      }
> >  
> >      mad = rdma_pci_dma_map(backend_dev->dev, bctx->sge.addr,
> >                             bctx->sge.length);
> >      if (!mad || bctx->sge.length < msg->umad_len + MAD_HDR_SIZE) {
> > +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
> >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_MAD_BUFF,
> >                        bctx->up_ctx);
> >      } else {
> > @@ -949,6 +984,7 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
> >          wc.byte_len = msg->umad_len;
> >          wc.status = IBV_WC_SUCCESS;
> >          wc.wc_flags = IBV_WC_GRH;
> > +        backend_dev->rdma_dev_res->stats.mad_rx++;
> >          comp_handler(bctx->up_ctx, &wc);
> >      }
> >  
> > diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
> > new file mode 100644
> > index 0000000000..c5814473c5
> > --- /dev/null
> > +++ b/hw/rdma/rdma_hmp.c
> > @@ -0,0 +1,30 @@
> > +/*
> > + * RDMA device: Human Monitor interface
> > + *
> > + * Copyright (C) 2018 Oracle
> > + * Copyright (C) 2018 Red Hat Inc
> > + *
> > + * Authors:
> > + *     Yuval Shaia <[e-mail]>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/rdma/rdma_hmp.h"
> > +#include "qemu/module.h"
> > +
> > +static const TypeInfo rdma_hmp_info = {
> > +    .name = TYPE_RDMA_STATS_PROVIDER,
> > +    .parent = TYPE_INTERFACE,
> > +    .class_size = sizeof(RdmaStatsProviderClass),
> > +};
> > +
> > +static void rdma_hmp_register_types(void)
> > +{
> > +    type_register_static(&rdma_hmp_info);
> > +}
> > +
> > +type_init(rdma_hmp_register_types)
> > diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> > index 14580ca379..e019de1a14 100644
> > --- a/hw/rdma/rdma_rm.c
> > +++ b/hw/rdma/rdma_rm.c
> > @@ -16,6 +16,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qapi/error.h"
> >  #include "cpu.h"
> > +#include "monitor/monitor.h"
> >  
> >  #include "trace.h"
> >  #include "rdma_utils.h"
> > @@ -26,6 +27,58 @@
> >  #define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
> >  #define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
> >  
> > +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
> > +{
> > +    monitor_printf(mon, "\ttx               : %" PRId64 "\n",
> > +                   dev_res->stats.tx);
> > +    monitor_printf(mon, "\ttx_len           : %" PRId64 "\n",
> > +                   dev_res->stats.tx_len);
> > +    monitor_printf(mon, "\ttx_err           : %" PRId64 "\n",
> > +                   dev_res->stats.tx_err);
> > +    monitor_printf(mon, "\trx_bufs          : %" PRId64 "\n",
> > +                   dev_res->stats.rx_bufs);
> > +    monitor_printf(mon, "\trx_bufs_len      : %" PRId64 "\n",
> > +                   dev_res->stats.rx_bufs_len);
> > +    monitor_printf(mon, "\trx_bufs_err      : %" PRId64 "\n",
> > +                   dev_res->stats.rx_bufs_err);
> > +    monitor_printf(mon, "\tcomps            : %" PRId64 "\n",
> > +                   dev_res->stats.completions);
> > +    monitor_printf(mon, "\tmissing_comps    : %" PRId32 "\n",
> > +                   dev_res->stats.missing_cqe);
> > +    monitor_printf(mon, "\tpoll_cq (bk)     : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_from_bk);
> > +    monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_ppoll_to);
> > +    monitor_printf(mon, "\tpoll_cq (fe)     : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_from_guest);
> > +    monitor_printf(mon, "\tpoll_cq_empty    : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_from_guest_empty);
> > +    monitor_printf(mon, "\tmad_tx           : %" PRId64 "\n",
> > +                   dev_res->stats.mad_tx);
> > +    monitor_printf(mon, "\tmad_tx_err       : %" PRId64 "\n",
> > +                   dev_res->stats.mad_tx_err);
> > +    monitor_printf(mon, "\tmad_rx           : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx);
> > +    monitor_printf(mon, "\tmad_rx_err       : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx_err);
> > +    monitor_printf(mon, "\tmad_rx_bufs      : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx_bufs);
> > +    monitor_printf(mon, "\tmad_rx_bufs_err  : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx_bufs_err);
> > +    monitor_printf(mon, "\tPDs              : %" PRId32 "\n",
> > +                   dev_res->pd_tbl.used);
> > +    monitor_printf(mon, "\tMRs              : %" PRId32 "\n",
> > +                   dev_res->mr_tbl.used);
> > +    monitor_printf(mon, "\tUCs              : %" PRId32 "\n",
> > +                   dev_res->uc_tbl.used);
> > +    monitor_printf(mon, "\tQPs              : %" PRId32 "\n",
> > +                   dev_res->qp_tbl.used);
> > +    monitor_printf(mon, "\tCQs              : %" PRId32 "\n",
> > +                   dev_res->cq_tbl.used);
> > +    monitor_printf(mon, "\tCEQ_CTXs         : %" PRId32 "\n",
> > +                   dev_res->cqe_ctx_tbl.used);
> > +}
> > +
> >  static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
> >                                  uint32_t tbl_sz, uint32_t res_sz)
> >  {
> > @@ -37,6 +90,7 @@ static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
> >      tbl->bitmap = bitmap_new(tbl_sz);
> >      tbl->tbl_sz = tbl_sz;
> >      tbl->res_sz = res_sz;
> > +    tbl->used = 0;
> >      qemu_mutex_init(&tbl->lock);
> >  }
> >  
> > @@ -76,6 +130,8 @@ static inline void *rdma_res_tbl_alloc(RdmaRmResTbl *tbl, uint32_t *handle)
> >  
> >      set_bit(*handle, tbl->bitmap);
> >  
> > +    tbl->used++;
> > +
> >      qemu_mutex_unlock(&tbl->lock);
> >  
> >      memset(tbl->tbl + *handle * tbl->res_sz, 0, tbl->res_sz);
> > @@ -93,6 +149,7 @@ static inline void rdma_res_tbl_dealloc(RdmaRmResTbl *tbl, uint32_t handle)
> >  
> >      if (handle < tbl->tbl_sz) {
> >          clear_bit(handle, tbl->bitmap);
> > +        tbl->used--;
> >      }
> >  
> >      qemu_mutex_unlock(&tbl->lock);
> > @@ -620,6 +677,9 @@ int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
> >  
> >      qemu_mutex_init(&dev_res->lock);
> >  
> > +    memset(&dev_res->stats, 0, sizeof(dev_res->stats));
> > +    atomic_set(&dev_res->stats.missing_cqe, 0);
> > +
> >      return 0;
> >  }
> >  
> > diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> > index 9ec87d2667..a527d306f6 100644
> > --- a/hw/rdma/rdma_rm.h
> > +++ b/hw/rdma/rdma_rm.h
> > @@ -20,6 +20,7 @@
> >  #include "rdma_backend_defs.h"
> >  #include "rdma_rm_defs.h"
> >  
> > +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
> >  int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
> >                   Error **errp);
> >  void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
> > diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
> > index f0ee1f3072..4b8d704cfe 100644
> > --- a/hw/rdma/rdma_rm_defs.h
> > +++ b/hw/rdma/rdma_rm_defs.h
> > @@ -34,7 +34,9 @@
> >  #define MAX_QP_INIT_RD_ATOM   16
> >  #define MAX_AH                64
> >  
> > -#define MAX_RM_TBL_NAME 16
> > +#define MAX_RM_TBL_NAME             16
> > +#define MAX_CONSEQ_EMPTY_POLL_CQ    4096 /* considered as error above this */
> > +
> >  typedef struct RdmaRmResTbl {
> >      char name[MAX_RM_TBL_NAME];
> >      QemuMutex lock;
> > @@ -42,6 +44,7 @@ typedef struct RdmaRmResTbl {
> >      size_t tbl_sz;
> >      size_t res_sz;
> >      void *tbl;
> > +    uint32_t used; /* number of used entries in the table */
> >  } RdmaRmResTbl;
> >  
> >  typedef struct RdmaRmPD {
> > @@ -96,6 +99,27 @@ typedef struct RdmaRmPort {
> >      enum ibv_port_state state;
> >  } RdmaRmPort;
> >  
> > +typedef struct RdmaRmStats {
> > +    uint64_t tx;
> > +    uint64_t tx_len;
> > +    uint64_t tx_err;
> > +    uint64_t rx_bufs;
> > +    uint64_t rx_bufs_len;
> > +    uint64_t rx_bufs_err;
> > +    uint64_t completions;
> > +    uint64_t mad_tx;
> > +    uint64_t mad_tx_err;
> > +    uint64_t mad_rx;
> > +    uint64_t mad_rx_err;
> > +    uint64_t mad_rx_bufs;
> > +    uint64_t mad_rx_bufs_err;
> > +    uint64_t poll_cq_from_bk;
> > +    uint64_t poll_cq_from_guest;
> > +    uint64_t poll_cq_from_guest_empty;
> > +    uint64_t poll_cq_ppoll_to;
> > +    uint32_t missing_cqe;
> > +} RdmaRmStats;
> > +
> >  typedef struct RdmaDeviceResources {
> >      RdmaRmPort port;
> >      RdmaRmResTbl pd_tbl;
> > @@ -106,6 +130,7 @@ typedef struct RdmaDeviceResources {
> >      RdmaRmResTbl cqe_ctx_tbl;
> >      GHashTable *qp_hash; /* Keeps mapping between real and emulated */
> >      QemuMutex lock;
> > +    RdmaRmStats stats;
> >  } RdmaDeviceResources;
> >  
> >  #endif
> > diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> > index 0879224957..167706ec2c 100644
> > --- a/hw/rdma/vmw/pvrdma.h
> > +++ b/hw/rdma/vmw/pvrdma.h
> > @@ -70,6 +70,10 @@ typedef struct DSRInfo {
> >      PvrdmaRing cq;
> >  } DSRInfo;
> >  
> > +typedef struct PVRDMADevStats {
> > +    uint64_t commands;
> > +} PVRDMADevStats;
> > +
> >  typedef struct PVRDMADev {
> >      PCIDevice parent_obj;
> >      MemoryRegion msix;
> > @@ -89,6 +93,7 @@ typedef struct PVRDMADev {
> >      CharBackend mad_chr;
> >      VMXNET3State *func0;
> >      Notifier shutdown_notifier;
> > +    PVRDMADevStats stats;
> >  } PVRDMADev;
> >  #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
> >  
> > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > index b6061f4b6e..659331ac93 100644
> > --- a/hw/rdma/vmw/pvrdma_main.c
> > +++ b/hw/rdma/vmw/pvrdma_main.c
> > @@ -25,6 +25,8 @@
> >  #include "cpu.h"
> >  #include "trace.h"
> >  #include "sysemu/sysemu.h"
> > +#include "monitor/monitor.h"
> > +#include "hw/rdma/rdma_hmp.h"
> >  
> >  #include "../rdma_rm.h"
> >  #include "../rdma_backend.h"
> > @@ -55,6 +57,18 @@ static Property pvrdma_dev_properties[] = {
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >  
> > +static void pvrdma_print_statistics(Monitor *mon, RdmaStatsProvider *obj)
> > +{
> > +    PVRDMADev *dev = PVRDMA_DEV(obj);
> > +    PCIDevice *pdev = PCI_DEVICE(dev);
> > +
> > +    monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
> > +                   PCI_FUNC(pdev->devfn));
> > +    monitor_printf(mon, "\tcommands         : %" PRId64 "\n",
> > +                   dev->stats.commands);
> > +    rdma_dump_device_counters(mon, &dev->rdma_dev_res);
> > +}
> > +
> >  static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
> >                            void *ring_state)
> >  {
> > @@ -394,6 +408,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
> >          if (val == 0) {
> >              trace_pvrdma_regs_write(addr, val, "REQUEST", "");
> >              pvrdma_exec_cmd(dev);
> > +            dev->stats.commands++;
> >          }
> >          break;
> >      default:
> > @@ -612,6 +627,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
> >          goto out;
> >      }
> >  
> > +    memset(&dev->stats, 0, sizeof(dev->stats));
> > +
> >      dev->shutdown_notifier.notify = pvrdma_shutdown_notifier;
> >      qemu_register_shutdown_notifier(&dev->shutdown_notifier);
> >  
> > @@ -631,6 +648,7 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > +    RdmaStatsProviderClass *ir = RDMA_STATS_PROVIDER_CLASS(klass);
> >  
> >      k->realize = pvrdma_realize;
> >      k->exit = pvrdma_exit;
> > @@ -642,6 +660,8 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
> >      dc->desc = "RDMA Device";
> >      dc->props = pvrdma_dev_properties;
> >      set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> > +
> > +    ir->print_statistics = pvrdma_print_statistics;
> >  }
> >  
> >  static const TypeInfo pvrdma_info = {
> > @@ -651,6 +671,7 @@ static const TypeInfo pvrdma_info = {
> >      .class_init = pvrdma_class_init,
> >      .interfaces = (InterfaceInfo[]) {
> >          { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> > +        { TYPE_RDMA_STATS_PROVIDER },
> >          { }
> >      }
> >  };
> > diff --git a/include/hw/rdma/rdma_hmp.h b/include/hw/rdma/rdma_hmp.h
> > new file mode 100644
> > index 0000000000..dd23f2bc84
> > --- /dev/null
> > +++ b/include/hw/rdma/rdma_hmp.h
> > @@ -0,0 +1,40 @@
> > +/*
> > + * RDMA device: Human Monitor interface
> > + *
> > + * Copyright (C) 2019 Oracle
> > + * Copyright (C) 2019 Red Hat Inc
> > + *
> > + * Authors:
> > + *     Yuval Shaia <[e-mail]>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef RDMA_HMP_H
> > +#define RDMA_HMP_H
> > +
> > +#include "qom/object.h"
> > +
> > +#define TYPE_RDMA_STATS_PROVIDER "rdma"
> > +
> > +#define RDMA_STATS_PROVIDER_CLASS(klass) \
> > +    OBJECT_CLASS_CHECK(RdmaStatsProviderClass, (klass), \
> > +                       TYPE_RDMA_STATS_PROVIDER)
> > +#define RDMA_STATS_PROVIDER_GET_CLASS(obj) \
> > +    OBJECT_GET_CLASS(RdmaStatsProviderClass, (obj), \
> > +                     TYPE_RDMA_STATS_PROVIDER)
> > +#define RDMA_STATS_PROVIDER(obj) \
> > +    INTERFACE_CHECK(RdmaStatsProvider, (obj), \
> > +                    TYPE_RDMA_STATS_PROVIDER)
> > +
> > +typedef struct RdmaStatsProvider RdmaStatsProvider;
> > +
> > +typedef struct RdmaStatsProviderClass {
> > +    InterfaceClass parent;
> > +
> > +    void (*print_statistics)(Monitor *mon, RdmaStatsProvider *obj);
> > +} RdmaStatsProviderClass;
> > +
> > +#endif
> > -- 
> > 2.17.2
> > 
> > 
--
Dr. David Alan Gilbert / [e-mail] / Manchester, UK

Yuval Shaia March 6, 2019, 12:46 p.m. UTC | #3

On Wed, Mar 06, 2019 at 12:20:49PM +0000, Dr. David Alan Gilbert wrote:
> * Yuval Shaia ([e-mail]) wrote:
> > On Sun, Mar 03, 2019 at 10:33:40PM +0200, Yuval Shaia wrote:
> > > Allow interrogating device internals through HMP interface.
> > > The exposed indicators can be used for troubleshooting by developers or
> > > sysadmin.
> > > There is no need to expose these attributes to a management system (e.x.
> > > libvirt) because (1) most of them are not "device-management' related
> > > info and (2) there is no guarantee the interface is stable.
> > > 
> > > Signed-off-by: Yuval Shaia <[e-mail]>
> > > ---
> > >  hmp-commands-info.hx       | 14 ++++++++
> > >  hmp.c                      | 27 +++++++++++++++
> > >  hmp.h                      |  1 +
> > >  hw/rdma/Makefile.objs      |  2 +-
> > >  hw/rdma/rdma_backend.c     | 70 +++++++++++++++++++++++++++++---------
> > >  hw/rdma/rdma_hmp.c         | 30 ++++++++++++++++
> > >  hw/rdma/rdma_rm.c          | 60 ++++++++++++++++++++++++++++++++
> > >  hw/rdma/rdma_rm.h          |  1 +
> > >  hw/rdma/rdma_rm_defs.h     | 27 ++++++++++++++-
> > >  hw/rdma/vmw/pvrdma.h       |  5 +++
> > >  hw/rdma/vmw/pvrdma_main.c  | 21 ++++++++++++
> > >  include/hw/rdma/rdma_hmp.h | 40 ++++++++++++++++++++++
> > >  12 files changed, 279 insertions(+), 19 deletions(-)
> > >  create mode 100644 hw/rdma/rdma_hmp.c
> > >  create mode 100644 include/hw/rdma/rdma_hmp.h
> > > 
> > > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > > index cbee8b944d..c59444c461 100644
> > > --- a/hmp-commands-info.hx
> > > +++ b/hmp-commands-info.hx
> > > @@ -202,6 +202,20 @@ STEXI
> > >  @item info pic
> > >  @findex info pic
> > >  Show PIC state.
> > > +ETEXI
> > > +
> > > +    {
> > > +        .name       = "rdma",
> > > +        .args_type  = "",
> > > +        .params     = "",
> > > +        .help       = "show RDMA state",
> > > +        .cmd        = hmp_info_rdma,
> > > +    },
> > > +
> > > +STEXI
> > > +@item info rdma
> > > +@findex info rdma
> > > +Show RDMA state.
> > >  ETEXI
> > >  
> > >      {
> > > diff --git a/hmp.c b/hmp.c
> > > index 1e006eeb49..68511b2441 100644
> > > --- a/hmp.c
> > > +++ b/hmp.c
> > > @@ -51,6 +51,7 @@
> > >  #include "qemu/error-report.h"
> > >  #include "exec/ramlist.h"
> > >  #include "hw/intc/intc.h"
> > > +#include "hw/rdma/rdma_hmp.h"
> > >  #include "migration/snapshot.h"
> > >  #include "migration/misc.h"
> > >  
> > > @@ -968,6 +969,32 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
> > >                                     hmp_info_pic_foreach, mon);
> > >  }
> > >  
> > > +static int hmp_info_rdma_foreach(Object *obj, void *opaque)
> > > +{
> > > +    RdmaStatsProvider *rdma;
> > > +    RdmaStatsProviderClass *k;
> > > +    Monitor *mon = opaque;
> > > +
> > > +    if (object_dynamic_cast(obj, TYPE_RDMA_STATS_PROVIDER)) {
> > > +        rdma = RDMA_STATS_PROVIDER(obj);
> > > +        k = RDMA_STATS_PROVIDER_GET_CLASS(obj);
> > > +        if (k->print_statistics) {
> > > +            k->print_statistics(mon, rdma);
> > > +        } else {
> > > +            monitor_printf(mon, "RDMA statistics not available for %s.\n",
> > > +                           object_get_typename(obj));
> > > +        }
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +void hmp_info_rdma(Monitor *mon, const QDict *qdict)
> > > +{
> > > +    object_child_foreach_recursive(object_get_root(),
> > > +                                   hmp_info_rdma_foreach, mon);
> > > +}
> > > +
> > 
> > Hi Markus and David,
> > Is this implementation is acceptable by you?
> 
> I think from the HMP side I'm fine, so:
> 
> Acked-by: Dr. David Alan Gilbert <[e-mail]>

Thanks for that.

Hope that some one can review the 'counters' part of the patch.

> 
> I would say that there's really two parts to this patch, all the places
> you change your code to gather the stats, and the separate pieces that
> wire it into HMP;  if you had to repost for another reason I'd
> split the patch like that.

Actually that how it was in v0 but then Markus (i think) comes up with the
idea to squash them as the *only* consumer of these counters is HMP.

But in general, i do agree to this but also can understand the other side.

> 
> Dave
> 
> > Thanks,
> > Yuval
> > 
> > 
> > >  void hmp_info_pci(Monitor *mon, const QDict *qdict)
> > >  {
> > >      PciInfoList *info_list, *info;
> > > diff --git a/hmp.h b/hmp.h
> > > index 5f1addcca2..666949afc3 100644
> > > --- a/hmp.h
> > > +++ b/hmp.h
> > > @@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
> > >  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
> > >  void hmp_info_irq(Monitor *mon, const QDict *qdict);
> > >  void hmp_info_pic(Monitor *mon, const QDict *qdict);
> > > +void hmp_info_rdma(Monitor *mon, const QDict *qdict);
> > >  void hmp_info_pci(Monitor *mon, const QDict *qdict);
> > >  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
> > >  void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> > > diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
> > > index bd36cbf51c..dd59faff51 100644
> > > --- a/hw/rdma/Makefile.objs
> > > +++ b/hw/rdma/Makefile.objs
> > > @@ -1,5 +1,5 @@
> > >  ifeq ($(CONFIG_PVRDMA),y)
> > > -obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
> > > +obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma_hmp.o
> > >  obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
> > >                       vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
> > >  endif
> > > diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> > > index 9679b842d1..bc2fefcf93 100644
> > > --- a/hw/rdma/rdma_backend.c
> > > +++ b/hw/rdma/rdma_backend.c
> > > @@ -64,9 +64,9 @@ static inline void complete_work(enum ibv_wc_status status, uint32_t vendor_err,
> > >      comp_handler(ctx, &wc);
> > >  }
> > >  
> > > -static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> > > +static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> > >  {
> > > -    int i, ne;
> > > +    int i, ne, total_ne = 0;
> > >      BackendCtx *bctx;
> > >      struct ibv_wc wc[2];
> > >  
> > > @@ -89,12 +89,18 @@ static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> > >              rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
> > >              g_free(bctx);
> > >          }
> > > +        total_ne += ne;
> > >      } while (ne > 0);
> > > +    atomic_sub(&rdma_dev_res->stats.missing_cqe, total_ne);
> > >      qemu_mutex_unlock(&rdma_dev_res->lock);
> > >  
> > >      if (ne < 0) {
> > >          rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
> > >      }
> > > +
> > > +    rdma_dev_res->stats.completions += total_ne;
> > > +
> > > +    return total_ne;
> > >  }
> > >  
> > >  static void *comp_handler_thread(void *arg)
> > > @@ -122,6 +128,9 @@ static void *comp_handler_thread(void *arg)
> > >      while (backend_dev->comp_thread.run) {
> > >          do {
> > >              rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
> > > +            if (!rc) {
> > > +                backend_dev->rdma_dev_res->stats.poll_cq_ppoll_to++;
> > > +            }
> > >          } while (!rc && backend_dev->comp_thread.run);
> > >  
> > >          if (backend_dev->comp_thread.run) {
> > > @@ -138,6 +147,7 @@ static void *comp_handler_thread(void *arg)
> > >                                    errno);
> > >              }
> > >  
> > > +            backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
> > >              rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
> > >  
> > >              ibv_ack_cq_events(ev_cq, 1);
> > > @@ -271,7 +281,13 @@ int rdma_backend_query_port(RdmaBackendDev *backend_dev,
> > >  
> > >  void rdma_backend_poll_cq(RdmaDeviceResources *rdma_dev_res, RdmaBackendCQ *cq)
> > >  {
> > > -    rdma_poll_cq(rdma_dev_res, cq->ibcq);
> > > +    int polled;
> > > +
> > > +    rdma_dev_res->stats.poll_cq_from_guest++;
> > > +    polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
> > > +    if (!polled) {
> > > +        rdma_dev_res->stats.poll_cq_from_guest_empty++;
> > > +    }
> > >  }
> > >  
> > >  static GHashTable *ah_hash;
> > > @@ -333,7 +349,7 @@ static void ah_cache_init(void)
> > >  
> > >  static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
> > >                                  struct ibv_sge *dsge, struct ibv_sge *ssge,
> > > -                                uint8_t num_sge)
> > > +                                uint8_t num_sge, uint64_t *total_length)
> > >  {
> > >      RdmaRmMR *mr;
> > >      int ssge_idx;
> > > @@ -349,6 +365,8 @@ static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
> > >          dsge->length = ssge[ssge_idx].length;
> > >          dsge->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
> > >  
> > > +        *total_length += dsge->length;
> > > +
> > >          dsge++;
> > >      }
> > >  
> > > @@ -445,8 +463,10 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> > >              rc = mad_send(backend_dev, sgid_idx, sgid, sge, num_sge);
> > >              if (rc) {
> > >                  complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
> > > +                backend_dev->rdma_dev_res->stats.mad_tx_err++;
> > >              } else {
> > >                  complete_work(IBV_WC_SUCCESS, 0, ctx);
> > > +                backend_dev->rdma_dev_res->stats.mad_tx++;
> > >              }
> > >          }
> > >          return;
> > > @@ -458,20 +478,21 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> > >      rc = rdma_rm_alloc_cqe_ctx(backend_dev->rdma_dev_res, &bctx_id, bctx);
> > >      if (unlikely(rc)) {
> > >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> > > -        goto out_free_bctx;
> > > +        goto err_free_bctx;
> > >      }
> > >  
> > > -    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge);
> > > +    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge,
> > > +                              &backend_dev->rdma_dev_res->stats.tx_len);
> > >      if (rc) {
> > >          complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> > > -        goto out_dealloc_cqe_ctx;
> > > +        goto err_dealloc_cqe_ctx;
> > >      }
> > >  
> > >      if (qp_type == IBV_QPT_UD) {
> > >          wr.wr.ud.ah = create_ah(backend_dev, qp->ibpd, sgid_idx, dgid);
> > >          if (!wr.wr.ud.ah) {
> > >              complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> > > -            goto out_dealloc_cqe_ctx;
> > > +            goto err_dealloc_cqe_ctx;
> > >          }
> > >          wr.wr.ud.remote_qpn = dqpn;
> > >          wr.wr.ud.remote_qkey = dqkey;
> > > @@ -488,15 +509,19 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> > >          rdma_error_report("ibv_post_send fail, qpn=0x%x, rc=%d, errno=%d",
> > >                            qp->ibqp->qp_num, rc, errno);
> > >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> > > -        goto out_dealloc_cqe_ctx;
> > > +        goto err_dealloc_cqe_ctx;
> > >      }
> > >  
> > > +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> > > +    backend_dev->rdma_dev_res->stats.tx++;
> > > +
> > >      return;
> > >  
> > > -out_dealloc_cqe_ctx:
> > > +err_dealloc_cqe_ctx:
> > > +    backend_dev->rdma_dev_res->stats.tx_err++;
> > >      rdma_rm_dealloc_cqe_ctx(backend_dev->rdma_dev_res, bctx_id);
> > >  
> > > -out_free_bctx:
> > > +err_free_bctx:
> > >      g_free(bctx);
> > >  }
> > >  
> > > @@ -554,6 +579,9 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
> > >              rc = save_mad_recv_buffer(backend_dev, sge, num_sge, ctx);
> > >              if (rc) {
> > >                  complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> > > +                rdma_dev_res->stats.mad_rx_bufs_err++;
> > > +            } else {
> > > +                rdma_dev_res->stats.mad_rx_bufs++;
> > >              }
> > >          }
> > >          return;
> > > @@ -565,13 +593,14 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
> > >      rc = rdma_rm_alloc_cqe_ctx(rdma_dev_res, &bctx_id, bctx);
> > >      if (unlikely(rc)) {
> > >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> > > -        goto out_free_bctx;
> > > +        goto err_free_bctx;
> > >      }
> > >  
> > > -    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge);
> > > +    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge,
> > > +                              &backend_dev->rdma_dev_res->stats.rx_bufs_len);
> > >      if (rc) {
> > >          complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> > > -        goto out_dealloc_cqe_ctx;
> > > +        goto err_dealloc_cqe_ctx;
> > >      }
> > >  
> > >      wr.num_sge = num_sge;
> > > @@ -582,15 +611,19 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
> > >          rdma_error_report("ibv_post_recv fail, qpn=0x%x, rc=%d, errno=%d",
> > >                            qp->ibqp->qp_num, rc, errno);
> > >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> > > -        goto out_dealloc_cqe_ctx;
> > > +        goto err_dealloc_cqe_ctx;
> > >      }
> > >  
> > > +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> > > +    rdma_dev_res->stats.rx_bufs++;
> > > +
> > >      return;
> > >  
> > > -out_dealloc_cqe_ctx:
> > > +err_dealloc_cqe_ctx:
> > > +    backend_dev->rdma_dev_res->stats.rx_bufs_err++;
> > >      rdma_rm_dealloc_cqe_ctx(rdma_dev_res, bctx_id);
> > >  
> > > -out_free_bctx:
> > > +err_free_bctx:
> > >      g_free(bctx);
> > >  }
> > >  
> > > @@ -929,12 +962,14 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
> > >      bctx = rdma_rm_get_cqe_ctx(backend_dev->rdma_dev_res, cqe_ctx_id);
> > >      if (unlikely(!bctx)) {
> > >          rdma_error_report("No matching ctx for req %ld", cqe_ctx_id);
> > > +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
> > >          return;
> > >      }
> > >  
> > >      mad = rdma_pci_dma_map(backend_dev->dev, bctx->sge.addr,
> > >                             bctx->sge.length);
> > >      if (!mad || bctx->sge.length < msg->umad_len + MAD_HDR_SIZE) {
> > > +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
> > >          complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_MAD_BUFF,
> > >                        bctx->up_ctx);
> > >      } else {
> > > @@ -949,6 +984,7 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
> > >          wc.byte_len = msg->umad_len;
> > >          wc.status = IBV_WC_SUCCESS;
> > >          wc.wc_flags = IBV_WC_GRH;
> > > +        backend_dev->rdma_dev_res->stats.mad_rx++;
> > >          comp_handler(bctx->up_ctx, &wc);
> > >      }
> > >  
> > > diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
> > > new file mode 100644
> > > index 0000000000..c5814473c5
> > > --- /dev/null
> > > +++ b/hw/rdma/rdma_hmp.c
> > > @@ -0,0 +1,30 @@
> > > +/*
> > > + * RDMA device: Human Monitor interface
> > > + *
> > > + * Copyright (C) 2018 Oracle
> > > + * Copyright (C) 2018 Red Hat Inc
> > > + *
> > > + * Authors:
> > > + *     Yuval Shaia <[e-mail]>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > > + * See the COPYING file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "hw/rdma/rdma_hmp.h"
> > > +#include "qemu/module.h"
> > > +
> > > +static const TypeInfo rdma_hmp_info = {
> > > +    .name = TYPE_RDMA_STATS_PROVIDER,
> > > +    .parent = TYPE_INTERFACE,
> > > +    .class_size = sizeof(RdmaStatsProviderClass),
> > > +};
> > > +
> > > +static void rdma_hmp_register_types(void)
> > > +{
> > > +    type_register_static(&rdma_hmp_info);
> > > +}
> > > +
> > > +type_init(rdma_hmp_register_types)
> > > diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> > > index 14580ca379..e019de1a14 100644
> > > --- a/hw/rdma/rdma_rm.c
> > > +++ b/hw/rdma/rdma_rm.c
> > > @@ -16,6 +16,7 @@
> > >  #include "qemu/osdep.h"
> > >  #include "qapi/error.h"
> > >  #include "cpu.h"
> > > +#include "monitor/monitor.h"
> > >  
> > >  #include "trace.h"
> > >  #include "rdma_utils.h"
> > > @@ -26,6 +27,58 @@
> > >  #define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
> > >  #define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
> > >  
> > > +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
> > > +{
> > > +    monitor_printf(mon, "\ttx               : %" PRId64 "\n",
> > > +                   dev_res->stats.tx);
> > > +    monitor_printf(mon, "\ttx_len           : %" PRId64 "\n",
> > > +                   dev_res->stats.tx_len);
> > > +    monitor_printf(mon, "\ttx_err           : %" PRId64 "\n",
> > > +                   dev_res->stats.tx_err);
> > > +    monitor_printf(mon, "\trx_bufs          : %" PRId64 "\n",
> > > +                   dev_res->stats.rx_bufs);
> > > +    monitor_printf(mon, "\trx_bufs_len      : %" PRId64 "\n",
> > > +                   dev_res->stats.rx_bufs_len);
> > > +    monitor_printf(mon, "\trx_bufs_err      : %" PRId64 "\n",
> > > +                   dev_res->stats.rx_bufs_err);
> > > +    monitor_printf(mon, "\tcomps            : %" PRId64 "\n",
> > > +                   dev_res->stats.completions);
> > > +    monitor_printf(mon, "\tmissing_comps    : %" PRId32 "\n",
> > > +                   dev_res->stats.missing_cqe);
> > > +    monitor_printf(mon, "\tpoll_cq (bk)     : %" PRId64 "\n",
> > > +                   dev_res->stats.poll_cq_from_bk);
> > > +    monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
> > > +                   dev_res->stats.poll_cq_ppoll_to);
> > > +    monitor_printf(mon, "\tpoll_cq (fe)     : %" PRId64 "\n",
> > > +                   dev_res->stats.poll_cq_from_guest);
> > > +    monitor_printf(mon, "\tpoll_cq_empty    : %" PRId64 "\n",
> > > +                   dev_res->stats.poll_cq_from_guest_empty);
> > > +    monitor_printf(mon, "\tmad_tx           : %" PRId64 "\n",
> > > +                   dev_res->stats.mad_tx);
> > > +    monitor_printf(mon, "\tmad_tx_err       : %" PRId64 "\n",
> > > +                   dev_res->stats.mad_tx_err);
> > > +    monitor_printf(mon, "\tmad_rx           : %" PRId64 "\n",
> > > +                   dev_res->stats.mad_rx);
> > > +    monitor_printf(mon, "\tmad_rx_err       : %" PRId64 "\n",
> > > +                   dev_res->stats.mad_rx_err);
> > > +    monitor_printf(mon, "\tmad_rx_bufs      : %" PRId64 "\n",
> > > +                   dev_res->stats.mad_rx_bufs);
> > > +    monitor_printf(mon, "\tmad_rx_bufs_err  : %" PRId64 "\n",
> > > +                   dev_res->stats.mad_rx_bufs_err);
> > > +    monitor_printf(mon, "\tPDs              : %" PRId32 "\n",
> > > +                   dev_res->pd_tbl.used);
> > > +    monitor_printf(mon, "\tMRs              : %" PRId32 "\n",
> > > +                   dev_res->mr_tbl.used);
> > > +    monitor_printf(mon, "\tUCs              : %" PRId32 "\n",
> > > +                   dev_res->uc_tbl.used);
> > > +    monitor_printf(mon, "\tQPs              : %" PRId32 "\n",
> > > +                   dev_res->qp_tbl.used);
> > > +    monitor_printf(mon, "\tCQs              : %" PRId32 "\n",
> > > +                   dev_res->cq_tbl.used);
> > > +    monitor_printf(mon, "\tCEQ_CTXs         : %" PRId32 "\n",
> > > +                   dev_res->cqe_ctx_tbl.used);
> > > +}
> > > +
> > >  static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
> > >                                  uint32_t tbl_sz, uint32_t res_sz)
> > >  {
> > > @@ -37,6 +90,7 @@ static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
> > >      tbl->bitmap = bitmap_new(tbl_sz);
> > >      tbl->tbl_sz = tbl_sz;
> > >      tbl->res_sz = res_sz;
> > > +    tbl->used = 0;
> > >      qemu_mutex_init(&tbl->lock);
> > >  }
> > >  
> > > @@ -76,6 +130,8 @@ static inline void *rdma_res_tbl_alloc(RdmaRmResTbl *tbl, uint32_t *handle)
> > >  
> > >      set_bit(*handle, tbl->bitmap);
> > >  
> > > +    tbl->used++;
> > > +
> > >      qemu_mutex_unlock(&tbl->lock);
> > >  
> > >      memset(tbl->tbl + *handle * tbl->res_sz, 0, tbl->res_sz);
> > > @@ -93,6 +149,7 @@ static inline void rdma_res_tbl_dealloc(RdmaRmResTbl *tbl, uint32_t handle)
> > >  
> > >      if (handle < tbl->tbl_sz) {
> > >          clear_bit(handle, tbl->bitmap);
> > > +        tbl->used--;
> > >      }
> > >  
> > >      qemu_mutex_unlock(&tbl->lock);
> > > @@ -620,6 +677,9 @@ int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
> > >  
> > >      qemu_mutex_init(&dev_res->lock);
> > >  
> > > +    memset(&dev_res->stats, 0, sizeof(dev_res->stats));
> > > +    atomic_set(&dev_res->stats.missing_cqe, 0);
> > > +
> > >      return 0;
> > >  }
> > >  
> > > diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> > > index 9ec87d2667..a527d306f6 100644
> > > --- a/hw/rdma/rdma_rm.h
> > > +++ b/hw/rdma/rdma_rm.h
> > > @@ -20,6 +20,7 @@
> > >  #include "rdma_backend_defs.h"
> > >  #include "rdma_rm_defs.h"
> > >  
> > > +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
> > >  int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
> > >                   Error **errp);
> > >  void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
> > > diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
> > > index f0ee1f3072..4b8d704cfe 100644
> > > --- a/hw/rdma/rdma_rm_defs.h
> > > +++ b/hw/rdma/rdma_rm_defs.h
> > > @@ -34,7 +34,9 @@
> > >  #define MAX_QP_INIT_RD_ATOM   16
> > >  #define MAX_AH                64
> > >  
> > > -#define MAX_RM_TBL_NAME 16
> > > +#define MAX_RM_TBL_NAME             16
> > > +#define MAX_CONSEQ_EMPTY_POLL_CQ    4096 /* considered as error above this */
> > > +
> > >  typedef struct RdmaRmResTbl {
> > >      char name[MAX_RM_TBL_NAME];
> > >      QemuMutex lock;
> > > @@ -42,6 +44,7 @@ typedef struct RdmaRmResTbl {
> > >      size_t tbl_sz;
> > >      size_t res_sz;
> > >      void *tbl;
> > > +    uint32_t used; /* number of used entries in the table */
> > >  } RdmaRmResTbl;
> > >  
> > >  typedef struct RdmaRmPD {
> > > @@ -96,6 +99,27 @@ typedef struct RdmaRmPort {
> > >      enum ibv_port_state state;
> > >  } RdmaRmPort;
> > >  
> > > +typedef struct RdmaRmStats {
> > > +    uint64_t tx;
> > > +    uint64_t tx_len;
> > > +    uint64_t tx_err;
> > > +    uint64_t rx_bufs;
> > > +    uint64_t rx_bufs_len;
> > > +    uint64_t rx_bufs_err;
> > > +    uint64_t completions;
> > > +    uint64_t mad_tx;
> > > +    uint64_t mad_tx_err;
> > > +    uint64_t mad_rx;
> > > +    uint64_t mad_rx_err;
> > > +    uint64_t mad_rx_bufs;
> > > +    uint64_t mad_rx_bufs_err;
> > > +    uint64_t poll_cq_from_bk;
> > > +    uint64_t poll_cq_from_guest;
> > > +    uint64_t poll_cq_from_guest_empty;
> > > +    uint64_t poll_cq_ppoll_to;
> > > +    uint32_t missing_cqe;
> > > +} RdmaRmStats;
> > > +
> > >  typedef struct RdmaDeviceResources {
> > >      RdmaRmPort port;
> > >      RdmaRmResTbl pd_tbl;
> > > @@ -106,6 +130,7 @@ typedef struct RdmaDeviceResources {
> > >      RdmaRmResTbl cqe_ctx_tbl;
> > >      GHashTable *qp_hash; /* Keeps mapping between real and emulated */
> > >      QemuMutex lock;
> > > +    RdmaRmStats stats;
> > >  } RdmaDeviceResources;
> > >  
> > >  #endif
> > > diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> > > index 0879224957..167706ec2c 100644
> > > --- a/hw/rdma/vmw/pvrdma.h
> > > +++ b/hw/rdma/vmw/pvrdma.h
> > > @@ -70,6 +70,10 @@ typedef struct DSRInfo {
> > >      PvrdmaRing cq;
> > >  } DSRInfo;
> > >  
> > > +typedef struct PVRDMADevStats {
> > > +    uint64_t commands;
> > > +} PVRDMADevStats;
> > > +
> > >  typedef struct PVRDMADev {
> > >      PCIDevice parent_obj;
> > >      MemoryRegion msix;
> > > @@ -89,6 +93,7 @@ typedef struct PVRDMADev {
> > >      CharBackend mad_chr;
> > >      VMXNET3State *func0;
> > >      Notifier shutdown_notifier;
> > > +    PVRDMADevStats stats;
> > >  } PVRDMADev;
> > >  #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
> > >  
> > > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > > index b6061f4b6e..659331ac93 100644
> > > --- a/hw/rdma/vmw/pvrdma_main.c
> > > +++ b/hw/rdma/vmw/pvrdma_main.c
> > > @@ -25,6 +25,8 @@
> > >  #include "cpu.h"
> > >  #include "trace.h"
> > >  #include "sysemu/sysemu.h"
> > > +#include "monitor/monitor.h"
> > > +#include "hw/rdma/rdma_hmp.h"
> > >  
> > >  #include "../rdma_rm.h"
> > >  #include "../rdma_backend.h"
> > > @@ -55,6 +57,18 @@ static Property pvrdma_dev_properties[] = {
> > >      DEFINE_PROP_END_OF_LIST(),
> > >  };
> > >  
> > > +static void pvrdma_print_statistics(Monitor *mon, RdmaStatsProvider *obj)
> > > +{
> > > +    PVRDMADev *dev = PVRDMA_DEV(obj);
> > > +    PCIDevice *pdev = PCI_DEVICE(dev);
> > > +
> > > +    monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
> > > +                   PCI_FUNC(pdev->devfn));
> > > +    monitor_printf(mon, "\tcommands         : %" PRId64 "\n",
> > > +                   dev->stats.commands);
> > > +    rdma_dump_device_counters(mon, &dev->rdma_dev_res);
> > > +}
> > > +
> > >  static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
> > >                            void *ring_state)
> > >  {
> > > @@ -394,6 +408,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
> > >          if (val == 0) {
> > >              trace_pvrdma_regs_write(addr, val, "REQUEST", "");
> > >              pvrdma_exec_cmd(dev);
> > > +            dev->stats.commands++;
> > >          }
> > >          break;
> > >      default:
> > > @@ -612,6 +627,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
> > >          goto out;
> > >      }
> > >  
> > > +    memset(&dev->stats, 0, sizeof(dev->stats));
> > > +
> > >      dev->shutdown_notifier.notify = pvrdma_shutdown_notifier;
> > >      qemu_register_shutdown_notifier(&dev->shutdown_notifier);
> > >  
> > > @@ -631,6 +648,7 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
> > >  {
> > >      DeviceClass *dc = DEVICE_CLASS(klass);
> > >      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > > +    RdmaStatsProviderClass *ir = RDMA_STATS_PROVIDER_CLASS(klass);
> > >  
> > >      k->realize = pvrdma_realize;
> > >      k->exit = pvrdma_exit;
> > > @@ -642,6 +660,8 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
> > >      dc->desc = "RDMA Device";
> > >      dc->props = pvrdma_dev_properties;
> > >      set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> > > +
> > > +    ir->print_statistics = pvrdma_print_statistics;
> > >  }
> > >  
> > >  static const TypeInfo pvrdma_info = {
> > > @@ -651,6 +671,7 @@ static const TypeInfo pvrdma_info = {
> > >      .class_init = pvrdma_class_init,
> > >      .interfaces = (InterfaceInfo[]) {
> > >          { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> > > +        { TYPE_RDMA_STATS_PROVIDER },
> > >          { }
> > >      }
> > >  };
> > > diff --git a/include/hw/rdma/rdma_hmp.h b/include/hw/rdma/rdma_hmp.h
> > > new file mode 100644
> > > index 0000000000..dd23f2bc84
> > > --- /dev/null
> > > +++ b/include/hw/rdma/rdma_hmp.h
> > > @@ -0,0 +1,40 @@
> > > +/*
> > > + * RDMA device: Human Monitor interface
> > > + *
> > > + * Copyright (C) 2019 Oracle
> > > + * Copyright (C) 2019 Red Hat Inc
> > > + *
> > > + * Authors:
> > > + *     Yuval Shaia <[e-mail]>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > > + * See the COPYING file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#ifndef RDMA_HMP_H
> > > +#define RDMA_HMP_H
> > > +
> > > +#include "qom/object.h"
> > > +
> > > +#define TYPE_RDMA_STATS_PROVIDER "rdma"
> > > +
> > > +#define RDMA_STATS_PROVIDER_CLASS(klass) \
> > > +    OBJECT_CLASS_CHECK(RdmaStatsProviderClass, (klass), \
> > > +                       TYPE_RDMA_STATS_PROVIDER)
> > > +#define RDMA_STATS_PROVIDER_GET_CLASS(obj) \
> > > +    OBJECT_GET_CLASS(RdmaStatsProviderClass, (obj), \
> > > +                     TYPE_RDMA_STATS_PROVIDER)
> > > +#define RDMA_STATS_PROVIDER(obj) \
> > > +    INTERFACE_CHECK(RdmaStatsProvider, (obj), \
> > > +                    TYPE_RDMA_STATS_PROVIDER)
> > > +
> > > +typedef struct RdmaStatsProvider RdmaStatsProvider;
> > > +
> > > +typedef struct RdmaStatsProviderClass {
> > > +    InterfaceClass parent;
> > > +
> > > +    void (*print_statistics)(Monitor *mon, RdmaStatsProvider *obj);
> > > +} RdmaStatsProviderClass;
> > > +
> > > +#endif
> > > -- 
> > > 2.17.2
> > > 
> > > 
> --
> Dr. David Alan Gilbert / [e-mail] / Manchester, UK

Marcel Apfelbaum March 7, 2019, 9:50 a.m. UTC | #4

Hi Yuval,

On 3/3/19 10:33 PM, Yuval Shaia wrote:
> Allow interrogating device internals through HMP interface.
> The exposed indicators can be used for troubleshooting by developers or
> sysadmin.
> There is no need to expose these attributes to a management system (e.x.
> libvirt) because (1) most of them are not "device-management' related
> info and (2) there is no guarantee the interface is stable.
>
> Signed-off-by: Yuval Shaia <[e-mail]>
> ---
>   hmp-commands-info.hx       | 14 ++++++++
>   hmp.c                      | 27 +++++++++++++++
>   hmp.h                      |  1 +
>   hw/rdma/Makefile.objs      |  2 +-
>   hw/rdma/rdma_backend.c     | 70 +++++++++++++++++++++++++++++---------
>   hw/rdma/rdma_hmp.c         | 30 ++++++++++++++++
>   hw/rdma/rdma_rm.c          | 60 ++++++++++++++++++++++++++++++++
>   hw/rdma/rdma_rm.h          |  1 +
>   hw/rdma/rdma_rm_defs.h     | 27 ++++++++++++++-
>   hw/rdma/vmw/pvrdma.h       |  5 +++
>   hw/rdma/vmw/pvrdma_main.c  | 21 ++++++++++++
>   include/hw/rdma/rdma_hmp.h | 40 ++++++++++++++++++++++
>   12 files changed, 279 insertions(+), 19 deletions(-)
>   create mode 100644 hw/rdma/rdma_hmp.c
>   create mode 100644 include/hw/rdma/rdma_hmp.h
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b944d..c59444c461 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -202,6 +202,20 @@ STEXI
>   @item info pic
>   @findex info pic
>   Show PIC state.
> +ETEXI
> +
> +    {
> +        .name       = "rdma",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show RDMA state",
> +        .cmd        = hmp_info_rdma,
> +    },
> +
> +STEXI
> +@item info rdma
> +@findex info rdma
> +Show RDMA state.
>   ETEXI
>   
>       {
> diff --git a/hmp.c b/hmp.c
> index 1e006eeb49..68511b2441 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -51,6 +51,7 @@
>   #include "qemu/error-report.h"
>   #include "exec/ramlist.h"
>   #include "hw/intc/intc.h"
> +#include "hw/rdma/rdma_hmp.h"
>   #include "migration/snapshot.h"
>   #include "migration/misc.h"
>   
> @@ -968,6 +969,32 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
>                                      hmp_info_pic_foreach, mon);
>   }
>   
> +static int hmp_info_rdma_foreach(Object *obj, void *opaque)
> +{
> +    RdmaStatsProvider *rdma;
> +    RdmaStatsProviderClass *k;
> +    Monitor *mon = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_RDMA_STATS_PROVIDER)) {
> +        rdma = RDMA_STATS_PROVIDER(obj);
> +        k = RDMA_STATS_PROVIDER_GET_CLASS(obj);
> +        if (k->print_statistics) {
> +            k->print_statistics(mon, rdma);
> +        } else {
> +            monitor_printf(mon, "RDMA statistics not available for %s.\n",
> +                           object_get_typename(obj));
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +void hmp_info_rdma(Monitor *mon, const QDict *qdict)
> +{
> +    object_child_foreach_recursive(object_get_root(),
> +                                   hmp_info_rdma_foreach, mon);
> +}
> +
>   void hmp_info_pci(Monitor *mon, const QDict *qdict)
>   {
>       PciInfoList *info_list, *info;
> diff --git a/hmp.h b/hmp.h
> index 5f1addcca2..666949afc3 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
>   void hmp_info_balloon(Monitor *mon, const QDict *qdict);
>   void hmp_info_irq(Monitor *mon, const QDict *qdict);
>   void hmp_info_pic(Monitor *mon, const QDict *qdict);
> +void hmp_info_rdma(Monitor *mon, const QDict *qdict);
>   void hmp_info_pci(Monitor *mon, const QDict *qdict);
>   void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
>   void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
> index bd36cbf51c..dd59faff51 100644
> --- a/hw/rdma/Makefile.objs
> +++ b/hw/rdma/Makefile.objs
> @@ -1,5 +1,5 @@
>   ifeq ($(CONFIG_PVRDMA),y)
> -obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
> +obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma_hmp.o
>   obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
>                        vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
>   endif
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index 9679b842d1..bc2fefcf93 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -64,9 +64,9 @@ static inline void complete_work(enum ibv_wc_status status, uint32_t vendor_err,
>       comp_handler(ctx, &wc);
>   }
>   
> -static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> +static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
>   {
> -    int i, ne;
> +    int i, ne, total_ne = 0;
>       BackendCtx *bctx;
>       struct ibv_wc wc[2];
>   
> @@ -89,12 +89,18 @@ static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
>               rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
>               g_free(bctx);
>           }
> +        total_ne += ne;
>       } while (ne > 0);
> +    atomic_sub(&rdma_dev_res->stats.missing_cqe, total_ne);
>       qemu_mutex_unlock(&rdma_dev_res->lock);
>   
>       if (ne < 0) {
>           rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
>       }
> +
> +    rdma_dev_res->stats.completions += total_ne;
> +
> +    return total_ne;
>   }
>   
>   static void *comp_handler_thread(void *arg)
> @@ -122,6 +128,9 @@ static void *comp_handler_thread(void *arg)
>       while (backend_dev->comp_thread.run) {
>           do {
>               rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
> +            if (!rc) {
> +                backend_dev->rdma_dev_res->stats.poll_cq_ppoll_to++;
> +            }
>           } while (!rc && backend_dev->comp_thread.run);
>   
>           if (backend_dev->comp_thread.run) {
> @@ -138,6 +147,7 @@ static void *comp_handler_thread(void *arg)
>                                     errno);
>               }
>   
> +            backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
>               rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
>   
>               ibv_ack_cq_events(ev_cq, 1);
> @@ -271,7 +281,13 @@ int rdma_backend_query_port(RdmaBackendDev *backend_dev,
>   
>   void rdma_backend_poll_cq(RdmaDeviceResources *rdma_dev_res, RdmaBackendCQ *cq)
>   {
> -    rdma_poll_cq(rdma_dev_res, cq->ibcq);
> +    int polled;
> +
> +    rdma_dev_res->stats.poll_cq_from_guest++;
> +    polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
> +    if (!polled) {
> +        rdma_dev_res->stats.poll_cq_from_guest_empty++;
> +    }
>   }
>   
>   static GHashTable *ah_hash;
> @@ -333,7 +349,7 @@ static void ah_cache_init(void)
>   
>   static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
>                                   struct ibv_sge *dsge, struct ibv_sge *ssge,
> -                                uint8_t num_sge)
> +                                uint8_t num_sge, uint64_t *total_length)
>   {
>       RdmaRmMR *mr;
>       int ssge_idx;
> @@ -349,6 +365,8 @@ static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
>           dsge->length = ssge[ssge_idx].length;
>           dsge->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
>   
> +        *total_length += dsge->length;
> +
>           dsge++;
>       }
>   
> @@ -445,8 +463,10 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
>               rc = mad_send(backend_dev, sgid_idx, sgid, sge, num_sge);
>               if (rc) {
>                   complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
> +                backend_dev->rdma_dev_res->stats.mad_tx_err++;
>               } else {
>                   complete_work(IBV_WC_SUCCESS, 0, ctx);
> +                backend_dev->rdma_dev_res->stats.mad_tx++;
>               }
>           }
>           return;
> @@ -458,20 +478,21 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
>       rc = rdma_rm_alloc_cqe_ctx(backend_dev->rdma_dev_res, &bctx_id, bctx);
>       if (unlikely(rc)) {
>           complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> -        goto out_free_bctx;
> +        goto err_free_bctx;
>       }
>   
> -    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge);
> +    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge,
> +                              &backend_dev->rdma_dev_res->stats.tx_len);
>       if (rc) {
>           complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>       }
>   
>       if (qp_type == IBV_QPT_UD) {
>           wr.wr.ud.ah = create_ah(backend_dev, qp->ibpd, sgid_idx, dgid);
>           if (!wr.wr.ud.ah) {
>               complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> -            goto out_dealloc_cqe_ctx;
> +            goto err_dealloc_cqe_ctx;
>           }
>           wr.wr.ud.remote_qpn = dqpn;
>           wr.wr.ud.remote_qkey = dqkey;
> @@ -488,15 +509,19 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
>           rdma_error_report("ibv_post_send fail, qpn=0x%x, rc=%d, errno=%d",
>                             qp->ibqp->qp_num, rc, errno);
>           complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>       }
>   
> +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> +    backend_dev->rdma_dev_res->stats.tx++;
> +
>       return;
>   
> -out_dealloc_cqe_ctx:
> +err_dealloc_cqe_ctx:
> +    backend_dev->rdma_dev_res->stats.tx_err++;
>       rdma_rm_dealloc_cqe_ctx(backend_dev->rdma_dev_res, bctx_id);
>   
> -out_free_bctx:
> +err_free_bctx:
>       g_free(bctx);
>   }
>   
> @@ -554,6 +579,9 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
>               rc = save_mad_recv_buffer(backend_dev, sge, num_sge, ctx);
>               if (rc) {
>                   complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> +                rdma_dev_res->stats.mad_rx_bufs_err++;
> +            } else {
> +                rdma_dev_res->stats.mad_rx_bufs++;
>               }
>           }
>           return;
> @@ -565,13 +593,14 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
>       rc = rdma_rm_alloc_cqe_ctx(rdma_dev_res, &bctx_id, bctx);
>       if (unlikely(rc)) {
>           complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
> -        goto out_free_bctx;
> +        goto err_free_bctx;
>       }
>   
> -    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge);
> +    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge,
> +                              &backend_dev->rdma_dev_res->stats.rx_bufs_len);
>       if (rc) {
>           complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>       }
>   
>       wr.num_sge = num_sge;
> @@ -582,15 +611,19 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
>           rdma_error_report("ibv_post_recv fail, qpn=0x%x, rc=%d, errno=%d",
>                             qp->ibqp->qp_num, rc, errno);
>           complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
> -        goto out_dealloc_cqe_ctx;
> +        goto err_dealloc_cqe_ctx;
>       }
>   
> +    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
> +    rdma_dev_res->stats.rx_bufs++;
> +
>       return;
>   
> -out_dealloc_cqe_ctx:
> +err_dealloc_cqe_ctx:
> +    backend_dev->rdma_dev_res->stats.rx_bufs_err++;
>       rdma_rm_dealloc_cqe_ctx(rdma_dev_res, bctx_id);
>   
> -out_free_bctx:
> +err_free_bctx:
>       g_free(bctx);
>   }
>   
> @@ -929,12 +962,14 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
>       bctx = rdma_rm_get_cqe_ctx(backend_dev->rdma_dev_res, cqe_ctx_id);
>       if (unlikely(!bctx)) {
>           rdma_error_report("No matching ctx for req %ld", cqe_ctx_id);
> +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
>           return;
>       }
>   
>       mad = rdma_pci_dma_map(backend_dev->dev, bctx->sge.addr,
>                              bctx->sge.length);
>       if (!mad || bctx->sge.length < msg->umad_len + MAD_HDR_SIZE) {
> +        backend_dev->rdma_dev_res->stats.mad_rx_err++;
>           complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_MAD_BUFF,
>                         bctx->up_ctx);
>       } else {
> @@ -949,6 +984,7 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
>           wc.byte_len = msg->umad_len;
>           wc.status = IBV_WC_SUCCESS;
>           wc.wc_flags = IBV_WC_GRH;
> +        backend_dev->rdma_dev_res->stats.mad_rx++;
>           comp_handler(bctx->up_ctx, &wc);
>       }
>   
> diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
> new file mode 100644
> index 0000000000..c5814473c5
> --- /dev/null
> +++ b/hw/rdma/rdma_hmp.c
> @@ -0,0 +1,30 @@
> +/*
> + * RDMA device: Human Monitor interface
> + *
> + * Copyright (C) 2018 Oracle
> + * Copyright (C) 2018 Red Hat Inc
> + *
> + * Authors:
> + *     Yuval Shaia <[e-mail]>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/rdma/rdma_hmp.h"
> +#include "qemu/module.h"
> +
> +static const TypeInfo rdma_hmp_info = {
> +    .name = TYPE_RDMA_STATS_PROVIDER,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(RdmaStatsProviderClass),
> +};
> +
> +static void rdma_hmp_register_types(void)
> +{
> +    type_register_static(&rdma_hmp_info);
> +}
> +
> +type_init(rdma_hmp_register_types)
> diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> index 14580ca379..e019de1a14 100644
> --- a/hw/rdma/rdma_rm.c
> +++ b/hw/rdma/rdma_rm.c
> @@ -16,6 +16,7 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "cpu.h"
> +#include "monitor/monitor.h"
>   
>   #include "trace.h"
>   #include "rdma_utils.h"
> @@ -26,6 +27,58 @@
>   #define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
>   #define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
>   
> +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
> +{
> +    monitor_printf(mon, "\ttx               : %" PRId64 "\n",
> +                   dev_res->stats.tx);
> +    monitor_printf(mon, "\ttx_len           : %" PRId64 "\n",
> +                   dev_res->stats.tx_len);
> +    monitor_printf(mon, "\ttx_err           : %" PRId64 "\n",
> +                   dev_res->stats.tx_err);
> +    monitor_printf(mon, "\trx_bufs          : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs);
> +    monitor_printf(mon, "\trx_bufs_len      : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs_len);
> +    monitor_printf(mon, "\trx_bufs_err      : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs_err);
> +    monitor_printf(mon, "\tcomps            : %" PRId64 "\n",
> +                   dev_res->stats.completions);
> +    monitor_printf(mon, "\tmissing_comps    : %" PRId32 "\n",
> +                   dev_res->stats.missing_cqe);
> +    monitor_printf(mon, "\tpoll_cq (bk)     : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_bk);
> +    monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_ppoll_to);
> +    monitor_printf(mon, "\tpoll_cq (fe)     : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_guest);
> +    monitor_printf(mon, "\tpoll_cq_empty    : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_guest_empty);
> +    monitor_printf(mon, "\tmad_tx           : %" PRId64 "\n",
> +                   dev_res->stats.mad_tx);
> +    monitor_printf(mon, "\tmad_tx_err       : %" PRId64 "\n",
> +                   dev_res->stats.mad_tx_err);
> +    monitor_printf(mon, "\tmad_rx           : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx);
> +    monitor_printf(mon, "\tmad_rx_err       : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_err);
> +    monitor_printf(mon, "\tmad_rx_bufs      : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_bufs);
> +    monitor_printf(mon, "\tmad_rx_bufs_err  : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_bufs_err);
> +    monitor_printf(mon, "\tPDs              : %" PRId32 "\n",
> +                   dev_res->pd_tbl.used);
> +    monitor_printf(mon, "\tMRs              : %" PRId32 "\n",
> +                   dev_res->mr_tbl.used);
> +    monitor_printf(mon, "\tUCs              : %" PRId32 "\n",
> +                   dev_res->uc_tbl.used);
> +    monitor_printf(mon, "\tQPs              : %" PRId32 "\n",
> +                   dev_res->qp_tbl.used);
> +    monitor_printf(mon, "\tCQs              : %" PRId32 "\n",
> +                   dev_res->cq_tbl.used);
> +    monitor_printf(mon, "\tCEQ_CTXs         : %" PRId32 "\n",
> +                   dev_res->cqe_ctx_tbl.used);
> +}
> +
>   static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
>                                   uint32_t tbl_sz, uint32_t res_sz)
>   {
> @@ -37,6 +90,7 @@ static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
>       tbl->bitmap = bitmap_new(tbl_sz);
>       tbl->tbl_sz = tbl_sz;
>       tbl->res_sz = res_sz;
> +    tbl->used = 0;
>       qemu_mutex_init(&tbl->lock);
>   }
>   
> @@ -76,6 +130,8 @@ static inline void *rdma_res_tbl_alloc(RdmaRmResTbl *tbl, uint32_t *handle)
>   
>       set_bit(*handle, tbl->bitmap);
>   
> +    tbl->used++;
> +
>       qemu_mutex_unlock(&tbl->lock);
>   
>       memset(tbl->tbl + *handle * tbl->res_sz, 0, tbl->res_sz);
> @@ -93,6 +149,7 @@ static inline void rdma_res_tbl_dealloc(RdmaRmResTbl *tbl, uint32_t handle)
>   
>       if (handle < tbl->tbl_sz) {
>           clear_bit(handle, tbl->bitmap);
> +        tbl->used--;
>       }
>   
>       qemu_mutex_unlock(&tbl->lock);
> @@ -620,6 +677,9 @@ int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
>   
>       qemu_mutex_init(&dev_res->lock);
>   
> +    memset(&dev_res->stats, 0, sizeof(dev_res->stats));
> +    atomic_set(&dev_res->stats.missing_cqe, 0);
> +
>       return 0;
>   }
>   
> diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> index 9ec87d2667..a527d306f6 100644
> --- a/hw/rdma/rdma_rm.h
> +++ b/hw/rdma/rdma_rm.h
> @@ -20,6 +20,7 @@
>   #include "rdma_backend_defs.h"
>   #include "rdma_rm_defs.h"
>   
> +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
>   int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
>                    Error **errp);
>   void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
> diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
> index f0ee1f3072..4b8d704cfe 100644
> --- a/hw/rdma/rdma_rm_defs.h
> +++ b/hw/rdma/rdma_rm_defs.h
> @@ -34,7 +34,9 @@
>   #define MAX_QP_INIT_RD_ATOM   16
>   #define MAX_AH                64
>   
> -#define MAX_RM_TBL_NAME 16
> +#define MAX_RM_TBL_NAME             16
> +#define MAX_CONSEQ_EMPTY_POLL_CQ    4096 /* considered as error above this */
> +
>   typedef struct RdmaRmResTbl {
>       char name[MAX_RM_TBL_NAME];
>       QemuMutex lock;
> @@ -42,6 +44,7 @@ typedef struct RdmaRmResTbl {
>       size_t tbl_sz;
>       size_t res_sz;
>       void *tbl;
> +    uint32_t used; /* number of used entries in the table */
>   } RdmaRmResTbl;
>   
>   typedef struct RdmaRmPD {
> @@ -96,6 +99,27 @@ typedef struct RdmaRmPort {
>       enum ibv_port_state state;
>   } RdmaRmPort;
>   
> +typedef struct RdmaRmStats {
> +    uint64_t tx;
> +    uint64_t tx_len;
> +    uint64_t tx_err;
> +    uint64_t rx_bufs;
> +    uint64_t rx_bufs_len;
> +    uint64_t rx_bufs_err;
> +    uint64_t completions;
> +    uint64_t mad_tx;
> +    uint64_t mad_tx_err;
> +    uint64_t mad_rx;
> +    uint64_t mad_rx_err;
> +    uint64_t mad_rx_bufs;
> +    uint64_t mad_rx_bufs_err;
> +    uint64_t poll_cq_from_bk;
> +    uint64_t poll_cq_from_guest;
> +    uint64_t poll_cq_from_guest_empty;
> +    uint64_t poll_cq_ppoll_to;
> +    uint32_t missing_cqe;
> +} RdmaRmStats;
> +
>   typedef struct RdmaDeviceResources {
>       RdmaRmPort port;
>       RdmaRmResTbl pd_tbl;
> @@ -106,6 +130,7 @@ typedef struct RdmaDeviceResources {
>       RdmaRmResTbl cqe_ctx_tbl;
>       GHashTable *qp_hash; /* Keeps mapping between real and emulated */
>       QemuMutex lock;
> +    RdmaRmStats stats;
>   } RdmaDeviceResources;
>   
>   #endif
> diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> index 0879224957..167706ec2c 100644
> --- a/hw/rdma/vmw/pvrdma.h
> +++ b/hw/rdma/vmw/pvrdma.h
> @@ -70,6 +70,10 @@ typedef struct DSRInfo {
>       PvrdmaRing cq;
>   } DSRInfo;
>   
> +typedef struct PVRDMADevStats {
> +    uint64_t commands;
> +} PVRDMADevStats;
> +
>   typedef struct PVRDMADev {
>       PCIDevice parent_obj;
>       MemoryRegion msix;
> @@ -89,6 +93,7 @@ typedef struct PVRDMADev {
>       CharBackend mad_chr;
>       VMXNET3State *func0;
>       Notifier shutdown_notifier;
> +    PVRDMADevStats stats;
>   } PVRDMADev;
>   #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
>   
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index b6061f4b6e..659331ac93 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -25,6 +25,8 @@
>   #include "cpu.h"
>   #include "trace.h"
>   #include "sysemu/sysemu.h"
> +#include "monitor/monitor.h"
> +#include "hw/rdma/rdma_hmp.h"
>   
>   #include "../rdma_rm.h"
>   #include "../rdma_backend.h"
> @@ -55,6 +57,18 @@ static Property pvrdma_dev_properties[] = {
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> +static void pvrdma_print_statistics(Monitor *mon, RdmaStatsProvider *obj)
> +{
> +    PVRDMADev *dev = PVRDMA_DEV(obj);
> +    PCIDevice *pdev = PCI_DEVICE(dev);
> +
> +    monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
> +                   PCI_FUNC(pdev->devfn));
> +    monitor_printf(mon, "\tcommands         : %" PRId64 "\n",
> +                   dev->stats.commands);
> +    rdma_dump_device_counters(mon, &dev->rdma_dev_res);
> +}
> +
>   static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
>                             void *ring_state)
>   {
> @@ -394,6 +408,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
>           if (val == 0) {
>               trace_pvrdma_regs_write(addr, val, "REQUEST", "");
>               pvrdma_exec_cmd(dev);
> +            dev->stats.commands++;
>           }
>           break;
>       default:
> @@ -612,6 +627,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
>           goto out;
>       }
>   
> +    memset(&dev->stats, 0, sizeof(dev->stats));
> +
>       dev->shutdown_notifier.notify = pvrdma_shutdown_notifier;
>       qemu_register_shutdown_notifier(&dev->shutdown_notifier);
>   
> @@ -631,6 +648,7 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    RdmaStatsProviderClass *ir = RDMA_STATS_PROVIDER_CLASS(klass);
>   
>       k->realize = pvrdma_realize;
>       k->exit = pvrdma_exit;
> @@ -642,6 +660,8 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
>       dc->desc = "RDMA Device";
>       dc->props = pvrdma_dev_properties;
>       set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> +
> +    ir->print_statistics = pvrdma_print_statistics;
>   }
>   
>   static const TypeInfo pvrdma_info = {
> @@ -651,6 +671,7 @@ static const TypeInfo pvrdma_info = {
>       .class_init = pvrdma_class_init,
>       .interfaces = (InterfaceInfo[]) {
>           { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +        { TYPE_RDMA_STATS_PROVIDER },
>           { }
>       }
>   };
> diff --git a/include/hw/rdma/rdma_hmp.h b/include/hw/rdma/rdma_hmp.h
> new file mode 100644
> index 0000000000..dd23f2bc84
> --- /dev/null
> +++ b/include/hw/rdma/rdma_hmp.h
> @@ -0,0 +1,40 @@
> +/*
> + * RDMA device: Human Monitor interface
> + *
> + * Copyright (C) 2019 Oracle
> + * Copyright (C) 2019 Red Hat Inc
> + *
> + * Authors:
> + *     Yuval Shaia <[e-mail]>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef RDMA_HMP_H
> +#define RDMA_HMP_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_RDMA_STATS_PROVIDER "rdma"
> +
> +#define RDMA_STATS_PROVIDER_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(RdmaStatsProviderClass, (klass), \
> +                       TYPE_RDMA_STATS_PROVIDER)
> +#define RDMA_STATS_PROVIDER_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(RdmaStatsProviderClass, (obj), \
> +                     TYPE_RDMA_STATS_PROVIDER)
> +#define RDMA_STATS_PROVIDER(obj) \
> +    INTERFACE_CHECK(RdmaStatsProvider, (obj), \
> +                    TYPE_RDMA_STATS_PROVIDER)
> +
> +typedef struct RdmaStatsProvider RdmaStatsProvider;
> +
> +typedef struct RdmaStatsProviderClass {
> +    InterfaceClass parent;
> +
> +    void (*print_statistics)(Monitor *mon, RdmaStatsProvider *obj);
> +} RdmaStatsProviderClass;
> +
> +#endif


RDMAStats implementation looks ok to me.

Reviewed-by: Marcel Apfelbaum<[e-mail]>

Thanks,
Marcel

Markus Armbruster March 8, 2019, 4:37 p.m. UTC | #5

Yuval Shaia <[e-mail]> writes:

> Allow interrogating device internals through HMP interface.
> The exposed indicators can be used for troubleshooting by developers or
> sysadmin.
> There is no need to expose these attributes to a management system (e.x.
> libvirt) because (1) most of them are not "device-management' related
> info and (2) there is no guarantee the interface is stable.
>
> Signed-off-by: Yuval Shaia <[e-mail]>
> ---
>  hmp-commands-info.hx       | 14 ++++++++
>  hmp.c                      | 27 +++++++++++++++
>  hmp.h                      |  1 +
>  hw/rdma/Makefile.objs      |  2 +-
>  hw/rdma/rdma_backend.c     | 70 +++++++++++++++++++++++++++++---------
>  hw/rdma/rdma_hmp.c         | 30 ++++++++++++++++
>  hw/rdma/rdma_rm.c          | 60 ++++++++++++++++++++++++++++++++
>  hw/rdma/rdma_rm.h          |  1 +
>  hw/rdma/rdma_rm_defs.h     | 27 ++++++++++++++-
>  hw/rdma/vmw/pvrdma.h       |  5 +++
>  hw/rdma/vmw/pvrdma_main.c  | 21 ++++++++++++
>  include/hw/rdma/rdma_hmp.h | 40 ++++++++++++++++++++++
>  12 files changed, 279 insertions(+), 19 deletions(-)
>  create mode 100644 hw/rdma/rdma_hmp.c
>  create mode 100644 include/hw/rdma/rdma_hmp.h
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b944d..c59444c461 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -202,6 +202,20 @@ STEXI
>  @item info pic
>  @findex info pic
>  Show PIC state.
> +ETEXI
> +
> +    {
> +        .name       = "rdma",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show RDMA state",
> +        .cmd        = hmp_info_rdma,
> +    },
> +
> +STEXI
> +@item info rdma
> +@findex info rdma
> +Show RDMA state.
>  ETEXI
>  
>      {
> diff --git a/hmp.c b/hmp.c
> index 1e006eeb49..68511b2441 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -51,6 +51,7 @@
>  #include "qemu/error-report.h"
>  #include "exec/ramlist.h"
>  #include "hw/intc/intc.h"
> +#include "hw/rdma/rdma_hmp.h"
>  #include "migration/snapshot.h"
>  #include "migration/misc.h"
>  
> @@ -968,6 +969,32 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
>                                     hmp_info_pic_foreach, mon);
>  }
>  
> +static int hmp_info_rdma_foreach(Object *obj, void *opaque)
> +{
> +    RdmaStatsProvider *rdma;
> +    RdmaStatsProviderClass *k;
> +    Monitor *mon = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_RDMA_STATS_PROVIDER)) {
> +        rdma = RDMA_STATS_PROVIDER(obj);
> +        k = RDMA_STATS_PROVIDER_GET_CLASS(obj);
> +        if (k->print_statistics) {
> +            k->print_statistics(mon, rdma);
> +        } else {
> +            monitor_printf(mon, "RDMA statistics not available for %s.\n",
> +                           object_get_typename(obj));
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +void hmp_info_rdma(Monitor *mon, const QDict *qdict)
> +{
> +    object_child_foreach_recursive(object_get_root(),
> +                                   hmp_info_rdma_foreach, mon);
> +}
> +
>  void hmp_info_pci(Monitor *mon, const QDict *qdict)
>  {
>      PciInfoList *info_list, *info;
> diff --git a/hmp.h b/hmp.h
> index 5f1addcca2..666949afc3 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
>  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
>  void hmp_info_irq(Monitor *mon, const QDict *qdict);
>  void hmp_info_pic(Monitor *mon, const QDict *qdict);
> +void hmp_info_rdma(Monitor *mon, const QDict *qdict);
>  void hmp_info_pci(Monitor *mon, const QDict *qdict);
>  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
>  void hmp_info_tpm(Monitor *mon, const QDict *qdict);

HMP stuff looks good to me.

> diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
> index bd36cbf51c..dd59faff51 100644
> --- a/hw/rdma/Makefile.objs
> +++ b/hw/rdma/Makefile.objs
> @@ -1,5 +1,5 @@
>  ifeq ($(CONFIG_PVRDMA),y)
> -obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
> +obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma_hmp.o
>  obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
>                       vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
>  endif
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index 9679b842d1..bc2fefcf93 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c

The changes to this file look like they're for gathering stats.  The
patch would be much simpler to review had you kept the stats gathering
separate.  I can only review the HMP / QOM part, and to do that, I have
to identify what to ignore.

[...]
> diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
> new file mode 100644
> index 0000000000..c5814473c5
> --- /dev/null
> +++ b/hw/rdma/rdma_hmp.c
> @@ -0,0 +1,30 @@
> +/*
> + * RDMA device: Human Monitor interface

The file name and this comment are a bit akward.  Yes, you create
TYPE_RDMA_STATS_PROVIDER for use in HMP info rdma, but there's
absolutely nothing HMP-related in this file.  Same for rdma_hmp.h below.

Call them rdma_stats.c and rdma_stats.h?

> + *
> + * Copyright (C) 2018 Oracle
> + * Copyright (C) 2018 Red Hat Inc
> + *
> + * Authors:
> + *     Yuval Shaia <[e-mail]>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/rdma/rdma_hmp.h"
> +#include "qemu/module.h"
> +
> +static const TypeInfo rdma_hmp_info = {
> +    .name = TYPE_RDMA_STATS_PROVIDER,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(RdmaStatsProviderClass),
> +};
> +
> +static void rdma_hmp_register_types(void)
> +{
> +    type_register_static(&rdma_hmp_info);
> +}
> +
> +type_init(rdma_hmp_register_types)

Also rename _hmp_ to _stats_.

> diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> index 14580ca379..e019de1a14 100644
> --- a/hw/rdma/rdma_rm.c
> +++ b/hw/rdma/rdma_rm.c
> @@ -16,6 +16,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
> +#include "monitor/monitor.h"
>  
>  #include "trace.h"
>  #include "rdma_utils.h"
> @@ -26,6 +27,58 @@
>  #define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
>  #define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
>  
> +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
> +{
> +    monitor_printf(mon, "\ttx               : %" PRId64 "\n",
> +                   dev_res->stats.tx);
> +    monitor_printf(mon, "\ttx_len           : %" PRId64 "\n",
> +                   dev_res->stats.tx_len);
> +    monitor_printf(mon, "\ttx_err           : %" PRId64 "\n",
> +                   dev_res->stats.tx_err);
> +    monitor_printf(mon, "\trx_bufs          : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs);
> +    monitor_printf(mon, "\trx_bufs_len      : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs_len);
> +    monitor_printf(mon, "\trx_bufs_err      : %" PRId64 "\n",
> +                   dev_res->stats.rx_bufs_err);
> +    monitor_printf(mon, "\tcomps            : %" PRId64 "\n",
> +                   dev_res->stats.completions);
> +    monitor_printf(mon, "\tmissing_comps    : %" PRId32 "\n",
> +                   dev_res->stats.missing_cqe);
> +    monitor_printf(mon, "\tpoll_cq (bk)     : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_bk);
> +    monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_ppoll_to);
> +    monitor_printf(mon, "\tpoll_cq (fe)     : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_guest);
> +    monitor_printf(mon, "\tpoll_cq_empty    : %" PRId64 "\n",
> +                   dev_res->stats.poll_cq_from_guest_empty);
> +    monitor_printf(mon, "\tmad_tx           : %" PRId64 "\n",
> +                   dev_res->stats.mad_tx);
> +    monitor_printf(mon, "\tmad_tx_err       : %" PRId64 "\n",
> +                   dev_res->stats.mad_tx_err);
> +    monitor_printf(mon, "\tmad_rx           : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx);
> +    monitor_printf(mon, "\tmad_rx_err       : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_err);
> +    monitor_printf(mon, "\tmad_rx_bufs      : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_bufs);
> +    monitor_printf(mon, "\tmad_rx_bufs_err  : %" PRId64 "\n",
> +                   dev_res->stats.mad_rx_bufs_err);
> +    monitor_printf(mon, "\tPDs              : %" PRId32 "\n",
> +                   dev_res->pd_tbl.used);
> +    monitor_printf(mon, "\tMRs              : %" PRId32 "\n",
> +                   dev_res->mr_tbl.used);
> +    monitor_printf(mon, "\tUCs              : %" PRId32 "\n",
> +                   dev_res->uc_tbl.used);
> +    monitor_printf(mon, "\tQPs              : %" PRId32 "\n",
> +                   dev_res->qp_tbl.used);
> +    monitor_printf(mon, "\tCQs              : %" PRId32 "\n",
> +                   dev_res->cq_tbl.used);
> +    monitor_printf(mon, "\tCEQ_CTXs         : %" PRId32 "\n",
> +                   dev_res->cqe_ctx_tbl.used);
> +}
> +
>  static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
>                                  uint32_t tbl_sz, uint32_t res_sz)
>  {
> @@ -37,6 +90,7 @@ static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,

More stats gathering, I think.

[...]
> diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> index 9ec87d2667..a527d306f6 100644
> --- a/hw/rdma/rdma_rm.h
> +++ b/hw/rdma/rdma_rm.h
> @@ -20,6 +20,7 @@
>  #include "rdma_backend_defs.h"
>  #include "rdma_rm_defs.h"
>  
> +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
>  int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
>                   Error **errp);
>  void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
> diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
> index f0ee1f3072..4b8d704cfe 100644
> --- a/hw/rdma/rdma_rm_defs.h
> +++ b/hw/rdma/rdma_rm_defs.h

Still more stats gathering, I think.

[...]
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index b6061f4b6e..659331ac93 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -25,6 +25,8 @@
>  #include "cpu.h"
>  #include "trace.h"
>  #include "sysemu/sysemu.h"
> +#include "monitor/monitor.h"
> +#include "hw/rdma/rdma_hmp.h"
>  
>  #include "../rdma_rm.h"
>  #include "../rdma_backend.h"
> @@ -55,6 +57,18 @@ static Property pvrdma_dev_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static void pvrdma_print_statistics(Monitor *mon, RdmaStatsProvider *obj)
> +{
> +    PVRDMADev *dev = PVRDMA_DEV(obj);
> +    PCIDevice *pdev = PCI_DEVICE(dev);
> +
> +    monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
> +                   PCI_FUNC(pdev->devfn));
> +    monitor_printf(mon, "\tcommands         : %" PRId64 "\n",
> +                   dev->stats.commands);
> +    rdma_dump_device_counters(mon, &dev->rdma_dev_res);
> +}
> +
>  static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
>                            void *ring_state)
>  {
> @@ -394,6 +408,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,

Still more stats gathering, I think.

[...]
> @@ -631,6 +648,7 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    RdmaStatsProviderClass *ir = RDMA_STATS_PROVIDER_CLASS(klass);
>  
>      k->realize = pvrdma_realize;
>      k->exit = pvrdma_exit;
> @@ -642,6 +660,8 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
>      dc->desc = "RDMA Device";
>      dc->props = pvrdma_dev_properties;
>      set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> +
> +    ir->print_statistics = pvrdma_print_statistics;
>  }
>  
>  static const TypeInfo pvrdma_info = {
> @@ -651,6 +671,7 @@ static const TypeInfo pvrdma_info = {
>      .class_init = pvrdma_class_init,
>      .interfaces = (InterfaceInfo[]) {
>          { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +        { TYPE_RDMA_STATS_PROVIDER },

We use INTERFACE_ rather than TYPE_ for some, but not all interfaces.
You follow TYPE_INTERRUPT_STATS_PROVIDER precedence.  We suck.  Not your
problem to solve.

>          { }
>      }
>  };
> diff --git a/include/hw/rdma/rdma_hmp.h b/include/hw/rdma/rdma_hmp.h
> new file mode 100644
> index 0000000000..dd23f2bc84
> --- /dev/null
> +++ b/include/hw/rdma/rdma_hmp.h
> @@ -0,0 +1,40 @@
> +/*
> + * RDMA device: Human Monitor interface
> + *
> + * Copyright (C) 2019 Oracle
> + * Copyright (C) 2019 Red Hat Inc
> + *
> + * Authors:
> + *     Yuval Shaia <[e-mail]>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef RDMA_HMP_H
> +#define RDMA_HMP_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_RDMA_STATS_PROVIDER "rdma"
> +
> +#define RDMA_STATS_PROVIDER_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(RdmaStatsProviderClass, (klass), \
> +                       TYPE_RDMA_STATS_PROVIDER)
> +#define RDMA_STATS_PROVIDER_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(RdmaStatsProviderClass, (obj), \
> +                     TYPE_RDMA_STATS_PROVIDER)
> +#define RDMA_STATS_PROVIDER(obj) \
> +    INTERFACE_CHECK(RdmaStatsProvider, (obj), \
> +                    TYPE_RDMA_STATS_PROVIDER)
> +
> +typedef struct RdmaStatsProvider RdmaStatsProvider;
> +
> +typedef struct RdmaStatsProviderClass {
> +    InterfaceClass parent;
> +
> +    void (*print_statistics)(Monitor *mon, RdmaStatsProvider *obj);
> +} RdmaStatsProviderClass;
> +
> +#endif

I think using QOM to find the RDMA devices came out rather nicely.
Thanks for giving it a try.

Please consider the renames I recommended.

Acked-by: Markus Armbruster <[e-mail]>

Yuval Shaia March 8, 2019, 6:57 p.m. UTC | #6

On Fri, Mar 08, 2019 at 05:37:45PM +0100, Markus Armbruster wrote:
> Yuval Shaia <[e-mail]> writes:
> 
> > Allow interrogating device internals through HMP interface.
> > The exposed indicators can be used for troubleshooting by developers or
> > sysadmin.
> > There is no need to expose these attributes to a management system (e.x.
> > libvirt) because (1) most of them are not "device-management' related
> > info and (2) there is no guarantee the interface is stable.
> >
> > Signed-off-by: Yuval Shaia <[e-mail]>
> > ---
> >  hmp-commands-info.hx       | 14 ++++++++
> >  hmp.c                      | 27 +++++++++++++++
> >  hmp.h                      |  1 +
> >  hw/rdma/Makefile.objs      |  2 +-
> >  hw/rdma/rdma_backend.c     | 70 +++++++++++++++++++++++++++++---------
> >  hw/rdma/rdma_hmp.c         | 30 ++++++++++++++++
> >  hw/rdma/rdma_rm.c          | 60 ++++++++++++++++++++++++++++++++
> >  hw/rdma/rdma_rm.h          |  1 +
> >  hw/rdma/rdma_rm_defs.h     | 27 ++++++++++++++-
> >  hw/rdma/vmw/pvrdma.h       |  5 +++
> >  hw/rdma/vmw/pvrdma_main.c  | 21 ++++++++++++
> >  include/hw/rdma/rdma_hmp.h | 40 ++++++++++++++++++++++
> >  12 files changed, 279 insertions(+), 19 deletions(-)
> >  create mode 100644 hw/rdma/rdma_hmp.c
> >  create mode 100644 include/hw/rdma/rdma_hmp.h
> >
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index cbee8b944d..c59444c461 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -202,6 +202,20 @@ STEXI
> >  @item info pic
> >  @findex info pic
> >  Show PIC state.
> > +ETEXI
> > +
> > +    {
> > +        .name       = "rdma",
> > +        .args_type  = "",
> > +        .params     = "",
> > +        .help       = "show RDMA state",
> > +        .cmd        = hmp_info_rdma,
> > +    },
> > +
> > +STEXI
> > +@item info rdma
> > +@findex info rdma
> > +Show RDMA state.
> >  ETEXI
> >  
> >      {
> > diff --git a/hmp.c b/hmp.c
> > index 1e006eeb49..68511b2441 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -51,6 +51,7 @@
> >  #include "qemu/error-report.h"
> >  #include "exec/ramlist.h"
> >  #include "hw/intc/intc.h"
> > +#include "hw/rdma/rdma_hmp.h"
> >  #include "migration/snapshot.h"
> >  #include "migration/misc.h"
> >  
> > @@ -968,6 +969,32 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
> >                                     hmp_info_pic_foreach, mon);
> >  }
> >  
> > +static int hmp_info_rdma_foreach(Object *obj, void *opaque)
> > +{
> > +    RdmaStatsProvider *rdma;
> > +    RdmaStatsProviderClass *k;
> > +    Monitor *mon = opaque;
> > +
> > +    if (object_dynamic_cast(obj, TYPE_RDMA_STATS_PROVIDER)) {
> > +        rdma = RDMA_STATS_PROVIDER(obj);
> > +        k = RDMA_STATS_PROVIDER_GET_CLASS(obj);
> > +        if (k->print_statistics) {
> > +            k->print_statistics(mon, rdma);
> > +        } else {
> > +            monitor_printf(mon, "RDMA statistics not available for %s.\n",
> > +                           object_get_typename(obj));
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +void hmp_info_rdma(Monitor *mon, const QDict *qdict)
> > +{
> > +    object_child_foreach_recursive(object_get_root(),
> > +                                   hmp_info_rdma_foreach, mon);
> > +}
> > +
> >  void hmp_info_pci(Monitor *mon, const QDict *qdict)
> >  {
> >      PciInfoList *info_list, *info;
> > diff --git a/hmp.h b/hmp.h
> > index 5f1addcca2..666949afc3 100644
> > --- a/hmp.h
> > +++ b/hmp.h
> > @@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
> >  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
> >  void hmp_info_irq(Monitor *mon, const QDict *qdict);
> >  void hmp_info_pic(Monitor *mon, const QDict *qdict);
> > +void hmp_info_rdma(Monitor *mon, const QDict *qdict);
> >  void hmp_info_pci(Monitor *mon, const QDict *qdict);
> >  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
> >  void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> 
> HMP stuff looks good to me.
> 
> > diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
> > index bd36cbf51c..dd59faff51 100644
> > --- a/hw/rdma/Makefile.objs
> > +++ b/hw/rdma/Makefile.objs
> > @@ -1,5 +1,5 @@
> >  ifeq ($(CONFIG_PVRDMA),y)
> > -obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
> > +obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma_hmp.o
> >  obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
> >                       vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
> >  endif
> > diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> > index 9679b842d1..bc2fefcf93 100644
> > --- a/hw/rdma/rdma_backend.c
> > +++ b/hw/rdma/rdma_backend.c
> 
> The changes to this file look like they're for gathering stats.  The
> patch would be much simpler to review had you kept the stats gathering
> separate.  I can only review the HMP / QOM part, and to do that, I have
> to identify what to ignore.
> 
> [...]
> > diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
> > new file mode 100644
> > index 0000000000..c5814473c5
> > --- /dev/null
> > +++ b/hw/rdma/rdma_hmp.c
> > @@ -0,0 +1,30 @@
> > +/*
> > + * RDMA device: Human Monitor interface
> 
> The file name and this comment are a bit akward.  Yes, you create
> TYPE_RDMA_STATS_PROVIDER for use in HMP info rdma, but there's
> absolutely nothing HMP-related in this file.  Same for rdma_hmp.h below.
> 
> Call them rdma_stats.c and rdma_stats.h?
> 
> > + *
> > + * Copyright (C) 2018 Oracle
> > + * Copyright (C) 2018 Red Hat Inc
> > + *
> > + * Authors:
> > + *     Yuval Shaia <[e-mail]>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/rdma/rdma_hmp.h"
> > +#include "qemu/module.h"
> > +
> > +static const TypeInfo rdma_hmp_info = {
> > +    .name = TYPE_RDMA_STATS_PROVIDER,
> > +    .parent = TYPE_INTERFACE,
> > +    .class_size = sizeof(RdmaStatsProviderClass),
> > +};
> > +
> > +static void rdma_hmp_register_types(void)
> > +{
> > +    type_register_static(&rdma_hmp_info);
> > +}
> > +
> > +type_init(rdma_hmp_register_types)
> 
> Also rename _hmp_ to _stats_.
> 
> > diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> > index 14580ca379..e019de1a14 100644
> > --- a/hw/rdma/rdma_rm.c
> > +++ b/hw/rdma/rdma_rm.c
> > @@ -16,6 +16,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qapi/error.h"
> >  #include "cpu.h"
> > +#include "monitor/monitor.h"
> >  
> >  #include "trace.h"
> >  #include "rdma_utils.h"
> > @@ -26,6 +27,58 @@
> >  #define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
> >  #define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
> >  
> > +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
> > +{
> > +    monitor_printf(mon, "\ttx               : %" PRId64 "\n",
> > +                   dev_res->stats.tx);
> > +    monitor_printf(mon, "\ttx_len           : %" PRId64 "\n",
> > +                   dev_res->stats.tx_len);
> > +    monitor_printf(mon, "\ttx_err           : %" PRId64 "\n",
> > +                   dev_res->stats.tx_err);
> > +    monitor_printf(mon, "\trx_bufs          : %" PRId64 "\n",
> > +                   dev_res->stats.rx_bufs);
> > +    monitor_printf(mon, "\trx_bufs_len      : %" PRId64 "\n",
> > +                   dev_res->stats.rx_bufs_len);
> > +    monitor_printf(mon, "\trx_bufs_err      : %" PRId64 "\n",
> > +                   dev_res->stats.rx_bufs_err);
> > +    monitor_printf(mon, "\tcomps            : %" PRId64 "\n",
> > +                   dev_res->stats.completions);
> > +    monitor_printf(mon, "\tmissing_comps    : %" PRId32 "\n",
> > +                   dev_res->stats.missing_cqe);
> > +    monitor_printf(mon, "\tpoll_cq (bk)     : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_from_bk);
> > +    monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_ppoll_to);
> > +    monitor_printf(mon, "\tpoll_cq (fe)     : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_from_guest);
> > +    monitor_printf(mon, "\tpoll_cq_empty    : %" PRId64 "\n",
> > +                   dev_res->stats.poll_cq_from_guest_empty);
> > +    monitor_printf(mon, "\tmad_tx           : %" PRId64 "\n",
> > +                   dev_res->stats.mad_tx);
> > +    monitor_printf(mon, "\tmad_tx_err       : %" PRId64 "\n",
> > +                   dev_res->stats.mad_tx_err);
> > +    monitor_printf(mon, "\tmad_rx           : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx);
> > +    monitor_printf(mon, "\tmad_rx_err       : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx_err);
> > +    monitor_printf(mon, "\tmad_rx_bufs      : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx_bufs);
> > +    monitor_printf(mon, "\tmad_rx_bufs_err  : %" PRId64 "\n",
> > +                   dev_res->stats.mad_rx_bufs_err);
> > +    monitor_printf(mon, "\tPDs              : %" PRId32 "\n",
> > +                   dev_res->pd_tbl.used);
> > +    monitor_printf(mon, "\tMRs              : %" PRId32 "\n",
> > +                   dev_res->mr_tbl.used);
> > +    monitor_printf(mon, "\tUCs              : %" PRId32 "\n",
> > +                   dev_res->uc_tbl.used);
> > +    monitor_printf(mon, "\tQPs              : %" PRId32 "\n",
> > +                   dev_res->qp_tbl.used);
> > +    monitor_printf(mon, "\tCQs              : %" PRId32 "\n",
> > +                   dev_res->cq_tbl.used);
> > +    monitor_printf(mon, "\tCEQ_CTXs         : %" PRId32 "\n",
> > +                   dev_res->cqe_ctx_tbl.used);
> > +}
> > +
> >  static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
> >                                  uint32_t tbl_sz, uint32_t res_sz)
> >  {
> > @@ -37,6 +90,7 @@ static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
> 
> More stats gathering, I think.
> 
> [...]
> > diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> > index 9ec87d2667..a527d306f6 100644
> > --- a/hw/rdma/rdma_rm.h
> > +++ b/hw/rdma/rdma_rm.h
> > @@ -20,6 +20,7 @@
> >  #include "rdma_backend_defs.h"
> >  #include "rdma_rm_defs.h"
> >  
> > +void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
> >  int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
> >                   Error **errp);
> >  void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
> > diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
> > index f0ee1f3072..4b8d704cfe 100644
> > --- a/hw/rdma/rdma_rm_defs.h
> > +++ b/hw/rdma/rdma_rm_defs.h
> 
> Still more stats gathering, I think.
> 
> [...]
> > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > index b6061f4b6e..659331ac93 100644
> > --- a/hw/rdma/vmw/pvrdma_main.c
> > +++ b/hw/rdma/vmw/pvrdma_main.c
> > @@ -25,6 +25,8 @@
> >  #include "cpu.h"
> >  #include "trace.h"
> >  #include "sysemu/sysemu.h"
> > +#include "monitor/monitor.h"
> > +#include "hw/rdma/rdma_hmp.h"
> >  
> >  #include "../rdma_rm.h"
> >  #include "../rdma_backend.h"
> > @@ -55,6 +57,18 @@ static Property pvrdma_dev_properties[] = {
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >  
> > +static void pvrdma_print_statistics(Monitor *mon, RdmaStatsProvider *obj)
> > +{
> > +    PVRDMADev *dev = PVRDMA_DEV(obj);
> > +    PCIDevice *pdev = PCI_DEVICE(dev);
> > +
> > +    monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
> > +                   PCI_FUNC(pdev->devfn));
> > +    monitor_printf(mon, "\tcommands         : %" PRId64 "\n",
> > +                   dev->stats.commands);
> > +    rdma_dump_device_counters(mon, &dev->rdma_dev_res);
> > +}
> > +
> >  static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
> >                            void *ring_state)
> >  {
> > @@ -394,6 +408,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
> 
> Still more stats gathering, I think.
> 
> [...]
> > @@ -631,6 +648,7 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > +    RdmaStatsProviderClass *ir = RDMA_STATS_PROVIDER_CLASS(klass);
> >  
> >      k->realize = pvrdma_realize;
> >      k->exit = pvrdma_exit;
> > @@ -642,6 +660,8 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
> >      dc->desc = "RDMA Device";
> >      dc->props = pvrdma_dev_properties;
> >      set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> > +
> > +    ir->print_statistics = pvrdma_print_statistics;
> >  }
> >  
> >  static const TypeInfo pvrdma_info = {
> > @@ -651,6 +671,7 @@ static const TypeInfo pvrdma_info = {
> >      .class_init = pvrdma_class_init,
> >      .interfaces = (InterfaceInfo[]) {
> >          { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> > +        { TYPE_RDMA_STATS_PROVIDER },
> 
> We use INTERFACE_ rather than TYPE_ for some, but not all interfaces.
> You follow TYPE_INTERRUPT_STATS_PROVIDER precedence.  We suck.  Not your
> problem to solve.
> 
> >          { }
> >      }
> >  };
> > diff --git a/include/hw/rdma/rdma_hmp.h b/include/hw/rdma/rdma_hmp.h
> > new file mode 100644
> > index 0000000000..dd23f2bc84
> > --- /dev/null
> > +++ b/include/hw/rdma/rdma_hmp.h
> > @@ -0,0 +1,40 @@
> > +/*
> > + * RDMA device: Human Monitor interface
> > + *
> > + * Copyright (C) 2019 Oracle
> > + * Copyright (C) 2019 Red Hat Inc
> > + *
> > + * Authors:
> > + *     Yuval Shaia <[e-mail]>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef RDMA_HMP_H
> > +#define RDMA_HMP_H
> > +
> > +#include "qom/object.h"
> > +
> > +#define TYPE_RDMA_STATS_PROVIDER "rdma"
> > +
> > +#define RDMA_STATS_PROVIDER_CLASS(klass) \
> > +    OBJECT_CLASS_CHECK(RdmaStatsProviderClass, (klass), \
> > +                       TYPE_RDMA_STATS_PROVIDER)
> > +#define RDMA_STATS_PROVIDER_GET_CLASS(obj) \
> > +    OBJECT_GET_CLASS(RdmaStatsProviderClass, (obj), \
> > +                     TYPE_RDMA_STATS_PROVIDER)
> > +#define RDMA_STATS_PROVIDER(obj) \
> > +    INTERFACE_CHECK(RdmaStatsProvider, (obj), \
> > +                    TYPE_RDMA_STATS_PROVIDER)
> > +
> > +typedef struct RdmaStatsProvider RdmaStatsProvider;
> > +
> > +typedef struct RdmaStatsProviderClass {
> > +    InterfaceClass parent;
> > +
> > +    void (*print_statistics)(Monitor *mon, RdmaStatsProvider *obj);
> > +} RdmaStatsProviderClass;
> > +
> > +#endif
> 
> I think using QOM to find the RDMA devices came out rather nicely.
> Thanks for giving it a try.
> 
> Please consider the renames I recommended.

Will do, it perfectly make sense.

> 
> Acked-by: Markus Armbruster <[e-mail]>

Thanks.

Yuval Shaia March 10, 2019, 8:06 a.m. UTC | #7

> 
> [...]
> > diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
> > new file mode 100644
> > index 0000000000..c5814473c5
> > --- /dev/null
> > +++ b/hw/rdma/rdma_hmp.c
> > @@ -0,0 +1,30 @@
> > +/*
> > + * RDMA device: Human Monitor interface
> 
> The file name and this comment are a bit akward.  Yes, you create
> TYPE_RDMA_STATS_PROVIDER for use in HMP info rdma, but there's
> absolutely nothing HMP-related in this file.  Same for rdma_hmp.h below.
> 
> Call them rdma_stats.c and rdma_stats.h?

Renamed to rdma.h and rdma.c, wo knows what other things might be added in
the future.

> 
> > + *
> > + * Copyright (C) 2018 Oracle
> > + * Copyright (C) 2018 Red Hat Inc
> > + *
> > + * Authors:
> > + *     Yuval Shaia <[e-mail]>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/rdma/rdma_hmp.h"
> > +#include "qemu/module.h"
> > +
> > +static const TypeInfo rdma_hmp_info = {
> > +    .name = TYPE_RDMA_STATS_PROVIDER,
> > +    .parent = TYPE_INTERFACE,
> > +    .class_size = sizeof(RdmaStatsProviderClass),
> > +};
> > +
> > +static void rdma_hmp_register_types(void)
> > +{
> > +    type_register_static(&rdma_hmp_info);
> > +}
> > +
> > +type_init(rdma_hmp_register_types)
> 
> Also rename _hmp_ to _stats_.

Ditto.

>

Patch

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index cbee8b944d..c59444c461 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -202,6 +202,20 @@  STEXI
 @item info pic
 @findex info pic
 Show PIC state.
+ETEXI
+
+    {
+        .name       = "rdma",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show RDMA state",
+        .cmd        = hmp_info_rdma,
+    },
+
+STEXI
+@item info rdma
+@findex info rdma
+Show RDMA state.
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index 1e006eeb49..68511b2441 100644
--- a/hmp.c
+++ b/hmp.c
@@ -51,6 +51,7 @@ 
 #include "qemu/error-report.h"
 #include "exec/ramlist.h"
 #include "hw/intc/intc.h"
+#include "hw/rdma/rdma_hmp.h"
 #include "migration/snapshot.h"
 #include "migration/misc.h"
 
@@ -968,6 +969,32 @@  void hmp_info_pic(Monitor *mon, const QDict *qdict)
                                    hmp_info_pic_foreach, mon);
 }
 
+static int hmp_info_rdma_foreach(Object *obj, void *opaque)
+{
+    RdmaStatsProvider *rdma;
+    RdmaStatsProviderClass *k;
+    Monitor *mon = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_RDMA_STATS_PROVIDER)) {
+        rdma = RDMA_STATS_PROVIDER(obj);
+        k = RDMA_STATS_PROVIDER_GET_CLASS(obj);
+        if (k->print_statistics) {
+            k->print_statistics(mon, rdma);
+        } else {
+            monitor_printf(mon, "RDMA statistics not available for %s.\n",
+                           object_get_typename(obj));
+        }
+    }
+
+    return 0;
+}
+
+void hmp_info_rdma(Monitor *mon, const QDict *qdict)
+{
+    object_child_foreach_recursive(object_get_root(),
+                                   hmp_info_rdma_foreach, mon);
+}
+
 void hmp_info_pci(Monitor *mon, const QDict *qdict)
 {
     PciInfoList *info_list, *info;
diff --git a/hmp.h b/hmp.h
index 5f1addcca2..666949afc3 100644
--- a/hmp.h
+++ b/hmp.h
@@ -36,6 +36,7 @@  void hmp_info_spice(Monitor *mon, const QDict *qdict);
 void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_irq(Monitor *mon, const QDict *qdict);
 void hmp_info_pic(Monitor *mon, const QDict *qdict);
+void hmp_info_rdma(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
index bd36cbf51c..dd59faff51 100644
--- a/hw/rdma/Makefile.objs
+++ b/hw/rdma/Makefile.objs
@@ -1,5 +1,5 @@ 
 ifeq ($(CONFIG_PVRDMA),y)
-obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
+obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma_hmp.o
 obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
                      vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
 endif
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 9679b842d1..bc2fefcf93 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -64,9 +64,9 @@  static inline void complete_work(enum ibv_wc_status status, uint32_t vendor_err,
     comp_handler(ctx, &wc);
 }
 
-static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
+static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
 {
-    int i, ne;
+    int i, ne, total_ne = 0;
     BackendCtx *bctx;
     struct ibv_wc wc[2];
 
@@ -89,12 +89,18 @@  static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
             rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
             g_free(bctx);
         }
+        total_ne += ne;
     } while (ne > 0);
+    atomic_sub(&rdma_dev_res->stats.missing_cqe, total_ne);
     qemu_mutex_unlock(&rdma_dev_res->lock);
 
     if (ne < 0) {
         rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
     }
+
+    rdma_dev_res->stats.completions += total_ne;
+
+    return total_ne;
 }
 
 static void *comp_handler_thread(void *arg)
@@ -122,6 +128,9 @@  static void *comp_handler_thread(void *arg)
     while (backend_dev->comp_thread.run) {
         do {
             rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
+            if (!rc) {
+                backend_dev->rdma_dev_res->stats.poll_cq_ppoll_to++;
+            }
         } while (!rc && backend_dev->comp_thread.run);
 
         if (backend_dev->comp_thread.run) {
@@ -138,6 +147,7 @@  static void *comp_handler_thread(void *arg)
                                   errno);
             }
 
+            backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
             rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
 
             ibv_ack_cq_events(ev_cq, 1);
@@ -271,7 +281,13 @@  int rdma_backend_query_port(RdmaBackendDev *backend_dev,
 
 void rdma_backend_poll_cq(RdmaDeviceResources *rdma_dev_res, RdmaBackendCQ *cq)
 {
-    rdma_poll_cq(rdma_dev_res, cq->ibcq);
+    int polled;
+
+    rdma_dev_res->stats.poll_cq_from_guest++;
+    polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
+    if (!polled) {
+        rdma_dev_res->stats.poll_cq_from_guest_empty++;
+    }
 }
 
 static GHashTable *ah_hash;
@@ -333,7 +349,7 @@  static void ah_cache_init(void)
 
 static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
                                 struct ibv_sge *dsge, struct ibv_sge *ssge,
-                                uint8_t num_sge)
+                                uint8_t num_sge, uint64_t *total_length)
 {
     RdmaRmMR *mr;
     int ssge_idx;
@@ -349,6 +365,8 @@  static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
         dsge->length = ssge[ssge_idx].length;
         dsge->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
 
+        *total_length += dsge->length;
+
         dsge++;
     }
 
@@ -445,8 +463,10 @@  void rdma_backend_post_send(RdmaBackendDev *backend_dev,
             rc = mad_send(backend_dev, sgid_idx, sgid, sge, num_sge);
             if (rc) {
                 complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
+                backend_dev->rdma_dev_res->stats.mad_tx_err++;
             } else {
                 complete_work(IBV_WC_SUCCESS, 0, ctx);
+                backend_dev->rdma_dev_res->stats.mad_tx++;
             }
         }
         return;
@@ -458,20 +478,21 @@  void rdma_backend_post_send(RdmaBackendDev *backend_dev,
     rc = rdma_rm_alloc_cqe_ctx(backend_dev->rdma_dev_res, &bctx_id, bctx);
     if (unlikely(rc)) {
         complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
-        goto out_free_bctx;
+        goto err_free_bctx;
     }
 
-    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge);
+    rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge,
+                              &backend_dev->rdma_dev_res->stats.tx_len);
     if (rc) {
         complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
-        goto out_dealloc_cqe_ctx;
+        goto err_dealloc_cqe_ctx;
     }
 
     if (qp_type == IBV_QPT_UD) {
         wr.wr.ud.ah = create_ah(backend_dev, qp->ibpd, sgid_idx, dgid);
         if (!wr.wr.ud.ah) {
             complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
-            goto out_dealloc_cqe_ctx;
+            goto err_dealloc_cqe_ctx;
         }
         wr.wr.ud.remote_qpn = dqpn;
         wr.wr.ud.remote_qkey = dqkey;
@@ -488,15 +509,19 @@  void rdma_backend_post_send(RdmaBackendDev *backend_dev,
         rdma_error_report("ibv_post_send fail, qpn=0x%x, rc=%d, errno=%d",
                           qp->ibqp->qp_num, rc, errno);
         complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
-        goto out_dealloc_cqe_ctx;
+        goto err_dealloc_cqe_ctx;
     }
 
+    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
+    backend_dev->rdma_dev_res->stats.tx++;
+
     return;
 
-out_dealloc_cqe_ctx:
+err_dealloc_cqe_ctx:
+    backend_dev->rdma_dev_res->stats.tx_err++;
     rdma_rm_dealloc_cqe_ctx(backend_dev->rdma_dev_res, bctx_id);
 
-out_free_bctx:
+err_free_bctx:
     g_free(bctx);
 }
 
@@ -554,6 +579,9 @@  void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
             rc = save_mad_recv_buffer(backend_dev, sge, num_sge, ctx);
             if (rc) {
                 complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
+                rdma_dev_res->stats.mad_rx_bufs_err++;
+            } else {
+                rdma_dev_res->stats.mad_rx_bufs++;
             }
         }
         return;
@@ -565,13 +593,14 @@  void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
     rc = rdma_rm_alloc_cqe_ctx(rdma_dev_res, &bctx_id, bctx);
     if (unlikely(rc)) {
         complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
-        goto out_free_bctx;
+        goto err_free_bctx;
     }
 
-    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge);
+    rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge,
+                              &backend_dev->rdma_dev_res->stats.rx_bufs_len);
     if (rc) {
         complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
-        goto out_dealloc_cqe_ctx;
+        goto err_dealloc_cqe_ctx;
     }
 
     wr.num_sge = num_sge;
@@ -582,15 +611,19 @@  void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
         rdma_error_report("ibv_post_recv fail, qpn=0x%x, rc=%d, errno=%d",
                           qp->ibqp->qp_num, rc, errno);
         complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_FAIL_BACKEND, ctx);
-        goto out_dealloc_cqe_ctx;
+        goto err_dealloc_cqe_ctx;
     }
 
+    atomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
+    rdma_dev_res->stats.rx_bufs++;
+
     return;
 
-out_dealloc_cqe_ctx:
+err_dealloc_cqe_ctx:
+    backend_dev->rdma_dev_res->stats.rx_bufs_err++;
     rdma_rm_dealloc_cqe_ctx(rdma_dev_res, bctx_id);
 
-out_free_bctx:
+err_free_bctx:
     g_free(bctx);
 }
 
@@ -929,12 +962,14 @@  static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
     bctx = rdma_rm_get_cqe_ctx(backend_dev->rdma_dev_res, cqe_ctx_id);
     if (unlikely(!bctx)) {
         rdma_error_report("No matching ctx for req %ld", cqe_ctx_id);
+        backend_dev->rdma_dev_res->stats.mad_rx_err++;
         return;
     }
 
     mad = rdma_pci_dma_map(backend_dev->dev, bctx->sge.addr,
                            bctx->sge.length);
     if (!mad || bctx->sge.length < msg->umad_len + MAD_HDR_SIZE) {
+        backend_dev->rdma_dev_res->stats.mad_rx_err++;
         complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_MAD_BUFF,
                       bctx->up_ctx);
     } else {
@@ -949,6 +984,7 @@  static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
         wc.byte_len = msg->umad_len;
         wc.status = IBV_WC_SUCCESS;
         wc.wc_flags = IBV_WC_GRH;
+        backend_dev->rdma_dev_res->stats.mad_rx++;
         comp_handler(bctx->up_ctx, &wc);
     }
 
diff --git a/hw/rdma/rdma_hmp.c b/hw/rdma/rdma_hmp.c
new file mode 100644
index 0000000000..c5814473c5
--- /dev/null
+++ b/hw/rdma/rdma_hmp.c
@@ -0,0 +1,30 @@ 
+/*
+ * RDMA device: Human Monitor interface
+ *
+ * Copyright (C) 2018 Oracle
+ * Copyright (C) 2018 Red Hat Inc
+ *
+ * Authors:
+ *     Yuval Shaia <[e-mail]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/rdma/rdma_hmp.h"
+#include "qemu/module.h"
+
+static const TypeInfo rdma_hmp_info = {
+    .name = TYPE_RDMA_STATS_PROVIDER,
+    .parent = TYPE_INTERFACE,
+    .class_size = sizeof(RdmaStatsProviderClass),
+};
+
+static void rdma_hmp_register_types(void)
+{
+    type_register_static(&rdma_hmp_info);
+}
+
+type_init(rdma_hmp_register_types)
diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 14580ca379..e019de1a14 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -16,6 +16,7 @@ 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "cpu.h"
+#include "monitor/monitor.h"
 
 #include "trace.h"
 #include "rdma_utils.h"
@@ -26,6 +27,58 @@ 
 #define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
 #define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
 
+void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
+{
+    monitor_printf(mon, "\ttx               : %" PRId64 "\n",
+                   dev_res->stats.tx);
+    monitor_printf(mon, "\ttx_len           : %" PRId64 "\n",
+                   dev_res->stats.tx_len);
+    monitor_printf(mon, "\ttx_err           : %" PRId64 "\n",
+                   dev_res->stats.tx_err);
+    monitor_printf(mon, "\trx_bufs          : %" PRId64 "\n",
+                   dev_res->stats.rx_bufs);
+    monitor_printf(mon, "\trx_bufs_len      : %" PRId64 "\n",
+                   dev_res->stats.rx_bufs_len);
+    monitor_printf(mon, "\trx_bufs_err      : %" PRId64 "\n",
+                   dev_res->stats.rx_bufs_err);
+    monitor_printf(mon, "\tcomps            : %" PRId64 "\n",
+                   dev_res->stats.completions);
+    monitor_printf(mon, "\tmissing_comps    : %" PRId32 "\n",
+                   dev_res->stats.missing_cqe);
+    monitor_printf(mon, "\tpoll_cq (bk)     : %" PRId64 "\n",
+                   dev_res->stats.poll_cq_from_bk);
+    monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
+                   dev_res->stats.poll_cq_ppoll_to);
+    monitor_printf(mon, "\tpoll_cq (fe)     : %" PRId64 "\n",
+                   dev_res->stats.poll_cq_from_guest);
+    monitor_printf(mon, "\tpoll_cq_empty    : %" PRId64 "\n",
+                   dev_res->stats.poll_cq_from_guest_empty);
+    monitor_printf(mon, "\tmad_tx           : %" PRId64 "\n",
+                   dev_res->stats.mad_tx);
+    monitor_printf(mon, "\tmad_tx_err       : %" PRId64 "\n",
+                   dev_res->stats.mad_tx_err);
+    monitor_printf(mon, "\tmad_rx           : %" PRId64 "\n",
+                   dev_res->stats.mad_rx);
+    monitor_printf(mon, "\tmad_rx_err       : %" PRId64 "\n",
+                   dev_res->stats.mad_rx_err);
+    monitor_printf(mon, "\tmad_rx_bufs      : %" PRId64 "\n",
+                   dev_res->stats.mad_rx_bufs);
+    monitor_printf(mon, "\tmad_rx_bufs_err  : %" PRId64 "\n",
+                   dev_res->stats.mad_rx_bufs_err);
+    monitor_printf(mon, "\tPDs              : %" PRId32 "\n",
+                   dev_res->pd_tbl.used);
+    monitor_printf(mon, "\tMRs              : %" PRId32 "\n",
+                   dev_res->mr_tbl.used);
+    monitor_printf(mon, "\tUCs              : %" PRId32 "\n",
+                   dev_res->uc_tbl.used);
+    monitor_printf(mon, "\tQPs              : %" PRId32 "\n",
+                   dev_res->qp_tbl.used);
+    monitor_printf(mon, "\tCQs              : %" PRId32 "\n",
+                   dev_res->cq_tbl.used);
+    monitor_printf(mon, "\tCEQ_CTXs         : %" PRId32 "\n",
+                   dev_res->cqe_ctx_tbl.used);
+}
+
 static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
                                 uint32_t tbl_sz, uint32_t res_sz)
 {
@@ -37,6 +90,7 @@  static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
     tbl->bitmap = bitmap_new(tbl_sz);
     tbl->tbl_sz = tbl_sz;
     tbl->res_sz = res_sz;
+    tbl->used = 0;
     qemu_mutex_init(&tbl->lock);
 }
 
@@ -76,6 +130,8 @@  static inline void *rdma_res_tbl_alloc(RdmaRmResTbl *tbl, uint32_t *handle)
 
     set_bit(*handle, tbl->bitmap);
 
+    tbl->used++;
+
     qemu_mutex_unlock(&tbl->lock);
 
     memset(tbl->tbl + *handle * tbl->res_sz, 0, tbl->res_sz);
@@ -93,6 +149,7 @@  static inline void rdma_res_tbl_dealloc(RdmaRmResTbl *tbl, uint32_t handle)
 
     if (handle < tbl->tbl_sz) {
         clear_bit(handle, tbl->bitmap);
+        tbl->used--;
     }
 
     qemu_mutex_unlock(&tbl->lock);
@@ -620,6 +677,9 @@  int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
 
     qemu_mutex_init(&dev_res->lock);
 
+    memset(&dev_res->stats, 0, sizeof(dev_res->stats));
+    atomic_set(&dev_res->stats.missing_cqe, 0);
+
     return 0;
 }
 
diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
index 9ec87d2667..a527d306f6 100644
--- a/hw/rdma/rdma_rm.h
+++ b/hw/rdma/rdma_rm.h
@@ -20,6 +20,7 @@ 
 #include "rdma_backend_defs.h"
 #include "rdma_rm_defs.h"
 
+void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
 int rdma_rm_init(RdmaDeviceResources *dev_res, struct ibv_device_attr *dev_attr,
                  Error **errp);
 void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
index f0ee1f3072..4b8d704cfe 100644
--- a/hw/rdma/rdma_rm_defs.h
+++ b/hw/rdma/rdma_rm_defs.h
@@ -34,7 +34,9 @@ 
 #define MAX_QP_INIT_RD_ATOM   16
 #define MAX_AH                64
 
-#define MAX_RM_TBL_NAME 16
+#define MAX_RM_TBL_NAME             16
+#define MAX_CONSEQ_EMPTY_POLL_CQ    4096 /* considered as error above this */
+
 typedef struct RdmaRmResTbl {
     char name[MAX_RM_TBL_NAME];
     QemuMutex lock;
@@ -42,6 +44,7 @@  typedef struct RdmaRmResTbl {
     size_t tbl_sz;
     size_t res_sz;
     void *tbl;
+    uint32_t used; /* number of used entries in the table */
 } RdmaRmResTbl;
 
 typedef struct RdmaRmPD {
@@ -96,6 +99,27 @@  typedef struct RdmaRmPort {
     enum ibv_port_state state;
 } RdmaRmPort;
 
+typedef struct RdmaRmStats {
+    uint64_t tx;
+    uint64_t tx_len;
+    uint64_t tx_err;
+    uint64_t rx_bufs;
+    uint64_t rx_bufs_len;
+    uint64_t rx_bufs_err;
+    uint64_t completions;
+    uint64_t mad_tx;
+    uint64_t mad_tx_err;
+    uint64_t mad_rx;
+    uint64_t mad_rx_err;
+    uint64_t mad_rx_bufs;
+    uint64_t mad_rx_bufs_err;
+    uint64_t poll_cq_from_bk;
+    uint64_t poll_cq_from_guest;
+    uint64_t poll_cq_from_guest_empty;
+    uint64_t poll_cq_ppoll_to;
+    uint32_t missing_cqe;
+} RdmaRmStats;
+
 typedef struct RdmaDeviceResources {
     RdmaRmPort port;
     RdmaRmResTbl pd_tbl;
@@ -106,6 +130,7 @@  typedef struct RdmaDeviceResources {
     RdmaRmResTbl cqe_ctx_tbl;
     GHashTable *qp_hash; /* Keeps mapping between real and emulated */
     QemuMutex lock;
+    RdmaRmStats stats;
 } RdmaDeviceResources;
 
 #endif
diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
index 0879224957..167706ec2c 100644
--- a/hw/rdma/vmw/pvrdma.h
+++ b/hw/rdma/vmw/pvrdma.h
@@ -70,6 +70,10 @@  typedef struct DSRInfo {
     PvrdmaRing cq;
 } DSRInfo;
 
+typedef struct PVRDMADevStats {
+    uint64_t commands;
+} PVRDMADevStats;
+
 typedef struct PVRDMADev {
     PCIDevice parent_obj;
     MemoryRegion msix;
@@ -89,6 +93,7 @@  typedef struct PVRDMADev {
     CharBackend mad_chr;
     VMXNET3State *func0;
     Notifier shutdown_notifier;
+    PVRDMADevStats stats;
 } PVRDMADev;
 #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
 
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index b6061f4b6e..659331ac93 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -25,6 +25,8 @@ 
 #include "cpu.h"
 #include "trace.h"
 #include "sysemu/sysemu.h"
+#include "monitor/monitor.h"
+#include "hw/rdma/rdma_hmp.h"
 
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
@@ -55,6 +57,18 @@  static Property pvrdma_dev_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void pvrdma_print_statistics(Monitor *mon, RdmaStatsProvider *obj)
+{
+    PVRDMADev *dev = PVRDMA_DEV(obj);
+    PCIDevice *pdev = PCI_DEVICE(dev);
+
+    monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
+                   PCI_FUNC(pdev->devfn));
+    monitor_printf(mon, "\tcommands         : %" PRId64 "\n",
+                   dev->stats.commands);
+    rdma_dump_device_counters(mon, &dev->rdma_dev_res);
+}
+
 static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
                           void *ring_state)
 {
@@ -394,6 +408,7 @@  static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
         if (val == 0) {
             trace_pvrdma_regs_write(addr, val, "REQUEST", "");
             pvrdma_exec_cmd(dev);
+            dev->stats.commands++;
         }
         break;
     default:
@@ -612,6 +627,8 @@  static void pvrdma_realize(PCIDevice *pdev, Error **errp)
         goto out;
     }
 
+    memset(&dev->stats, 0, sizeof(dev->stats));
+
     dev->shutdown_notifier.notify = pvrdma_shutdown_notifier;
     qemu_register_shutdown_notifier(&dev->shutdown_notifier);
 
@@ -631,6 +648,7 @@  static void pvrdma_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    RdmaStatsProviderClass *ir = RDMA_STATS_PROVIDER_CLASS(klass);
 
     k->realize = pvrdma_realize;
     k->exit = pvrdma_exit;
@@ -642,6 +660,8 @@  static void pvrdma_class_init(ObjectClass *klass, void *data)
     dc->desc = "RDMA Device";
     dc->props = pvrdma_dev_properties;
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+
+    ir->print_statistics = pvrdma_print_statistics;
 }
 
 static const TypeInfo pvrdma_info = {
@@ -651,6 +671,7 @@  static const TypeInfo pvrdma_info = {
     .class_init = pvrdma_class_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { TYPE_RDMA_STATS_PROVIDER },
         { }
     }
 };
diff --git a/include/hw/rdma/rdma_hmp.h b/include/hw/rdma/rdma_hmp.h
new file mode 100644
index 0000000000..dd23f2bc84
--- /dev/null
+++ b/include/hw/rdma/rdma_hmp.h
@@ -0,0 +1,40 @@ 
+/*
+ * RDMA device: Human Monitor interface
+ *
+ * Copyright (C) 2019 Oracle
+ * Copyright (C) 2019 Red Hat Inc
+ *
+ * Authors:
+ *     Yuval Shaia <[e-mail]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef RDMA_HMP_H
+#define RDMA_HMP_H
+
+#include "qom/object.h"
+
+#define TYPE_RDMA_STATS_PROVIDER "rdma"
+
+#define RDMA_STATS_PROVIDER_CLASS(klass) \
+    OBJECT_CLASS_CHECK(RdmaStatsProviderClass, (klass), \
+                       TYPE_RDMA_STATS_PROVIDER)
+#define RDMA_STATS_PROVIDER_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(RdmaStatsProviderClass, (obj), \
+                     TYPE_RDMA_STATS_PROVIDER)
+#define RDMA_STATS_PROVIDER(obj) \
+    INTERFACE_CHECK(RdmaStatsProvider, (obj), \
+                    TYPE_RDMA_STATS_PROVIDER)
+
+typedef struct RdmaStatsProvider RdmaStatsProvider;
+
+typedef struct RdmaStatsProviderClass {
+    InterfaceClass parent;
+
+    void (*print_statistics)(Monitor *mon, RdmaStatsProvider *obj);
+} RdmaStatsProviderClass;
+
+#endif