Re: [PATCH v2 04/19] block/nvme: Trace controller capabilities

[Auger Eric <eric.auger@redhat.com>]

Hi,

On 10/26/20 11:54 AM, Philippe Mathieu-Daudé wrote:
> Controllers have different capabilities and report them in the
> CAP register. We are particularly interested by the page size
> limits.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block/nvme.c       | 10 ++++++++++
>  block/trace-events |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 5abd7257cac..3b6d3972ec2 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -720,6 +720,16 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
>       * Initialization". */
>  
>      cap = le64_to_cpu(regs->cap);
> +    trace_nvme_controller_capability("Maximum Queue Entries Supported",
> +                                     NVME_CAP_MQES(cap));
> +    trace_nvme_controller_capability("Contiguous Queues Required",
> +                                     NVME_CAP_CQR(cap));
> +    trace_nvme_controller_capability("Subsystem  Reset Supported",
nit extra space
> +                                     NVME_CAP_NSSRS(cap));
> +    trace_nvme_controller_capability("Memory Page Size Minimum",
> +                                     NVME_CAP_MPSMIN(cap));
> +    trace_nvme_controller_capability("Memory Page Size Maximum",
> +                                     NVME_CAP_MPSMAX(cap));
Don't you want to trace the true MPSMIN/MAX (I mean with the shift)?

I would personally group those traces into a single one and avoid this
generic trace format but well ...

Thanks

Eric
>      if (!NVME_CAP_CSS(cap)) {
>          error_setg(errp, "Device doesn't support NVMe command set");
>          ret = -EINVAL;
> diff --git a/block/trace-events b/block/trace-events
> index 0e351c3fa3d..3f141dc6801 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -134,6 +134,7 @@ qed_aio_write_postfill(void *s, void *acb, uint64_t start, size_t len, uint64_t
>  qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s %p acb %p ret %d offset %"PRIu64" len %zu"
>  
>  # nvme.c
> +nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64
>  nvme_kick(void *s, int queue) "s %p queue %d"
>  nvme_dma_flush_queue_wait(void *s) "s %p"
>  nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"
>