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

From: Auger Eric <eric.auger@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
Cc: Fam Zheng <fam@euphon.net>, Kevin Wolf <kwolf@redhat.com>,
	qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Keith Busch <kbusch@kernel.org>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Klaus Jensen <its@irrelevant.dk>
Subject: Re: [PATCH 04/25] block/nvme: Trace controller capabilities
Date: Wed, 28 Oct 2020 11:20:50 +0100	[thread overview]
Message-ID: <1d71f170-3681-e923-da32-581f90262be1@redhat.com> (raw)
In-Reply-To: <20201027135547.374946-5-philmd@redhat.com>

Hi Philippe,

On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote:
> Controllers have different capabilities and report them in the
> CAP register. We are particularly interested by the page size
> limits.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block/nvme.c       | 13 +++++++++++++
>  block/trace-events |  2 ++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 6f1d7f9b2a1..361b5772b7a 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -727,6 +727,19 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
>       * Initialization". */
>  
>      cap = le64_to_cpu(regs->cap);
> +    trace_nvme_controller_capability_raw(cap);
> +    trace_nvme_controller_capability("Maximum Queue Entries Supported",
> +                                     1 + NVME_CAP_MQES(cap));
> +    trace_nvme_controller_capability("Contiguous Queues Required",
> +                                     NVME_CAP_CQR(cap));
I think this should be +1 too (0's based value)
> +    trace_nvme_controller_capability("Doorbell Stride",
> +                                     2 << (2 + NVME_CAP_DSTRD(cap)));
> +    trace_nvme_controller_capability("Subsystem Reset Supported",
> +                                     NVME_CAP_NSSRS(cap));
> +    trace_nvme_controller_capability("Memory Page Size Minimum",
> +                                     1 << (12 + NVME_CAP_MPSMIN(cap)));
> +    trace_nvme_controller_capability("Memory Page Size Maximum",
> +                                     1 << (12 + NVME_CAP_MPSMAX(cap)));
>      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 0955c85c783..b90b07b15fa 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -134,6 +134,8 @@ 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_raw(uint64_t value) "0x%08"PRIx64
> +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"
> 
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric