All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] block/nvme: Minor tracing improvements
@ 2021-01-27 21:21 Philippe Mathieu-Daudé
  2021-01-27 21:21 ` [PATCH 1/2] block/nvme: Properly display doorbell stride length in trace event Philippe Mathieu-Daudé
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-27 21:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Stefan Hajnoczi,
	Philippe Mathieu-Daudé

Fix an incorrect trace event (sometimes it is better to
have no information rather than having incorrect info...)
and add another event to display the spec implemented by
the controller.

Philippe Mathieu-Daudé (2):
  block/nvme: Properly display doorbell stride length in trace event
  block/nvme: Trace NVMe spec version supported by the controller

 block/nvme.c       | 8 +++++++-
 block/trace-events | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.26.2




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] block/nvme: Properly display doorbell stride length in trace event
  2021-01-27 21:21 [PATCH 0/2] block/nvme: Minor tracing improvements Philippe Mathieu-Daudé
@ 2021-01-27 21:21 ` Philippe Mathieu-Daudé
  2021-01-27 21:30   ` Klaus Jensen
  2021-01-27 21:21 ` [PATCH 2/2] block/nvme: Trace NVMe spec version supported by the controller Philippe Mathieu-Daudé
  2021-02-02 14:44 ` [PATCH 0/2] block/nvme: Minor tracing improvements Kevin Wolf
  2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-27 21:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Stefan Hajnoczi,
	Philippe Mathieu-Daudé

Commit 15b2260bef3 ("block/nvme: Trace controller capabilities")
misunderstood the doorbell stride value from the datasheet, use
the correct one. The 'doorbell_scale' variable used few lines
later is correct.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nvme.c b/block/nvme.c
index 5a6fbacf4a5..80c4318d8fc 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -745,7 +745,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     trace_nvme_controller_capability("Contiguous Queues Required",
                                      NVME_CAP_CQR(cap));
     trace_nvme_controller_capability("Doorbell Stride",
-                                     2 << (2 + NVME_CAP_DSTRD(cap)));
+                                     1 << (2 + NVME_CAP_DSTRD(cap)));
     trace_nvme_controller_capability("Subsystem Reset Supported",
                                      NVME_CAP_NSSRS(cap));
     trace_nvme_controller_capability("Memory Page Size Minimum",
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] block/nvme: Trace NVMe spec version supported by the controller
  2021-01-27 21:21 [PATCH 0/2] block/nvme: Minor tracing improvements Philippe Mathieu-Daudé
  2021-01-27 21:21 ` [PATCH 1/2] block/nvme: Properly display doorbell stride length in trace event Philippe Mathieu-Daudé
@ 2021-01-27 21:21 ` Philippe Mathieu-Daudé
  2021-01-27 21:32   ` Klaus Jensen
  2021-02-02 14:44 ` [PATCH 0/2] block/nvme: Minor tracing improvements Kevin Wolf
  2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-27 21:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Stefan Hajnoczi,
	Philippe Mathieu-Daudé

NVMe controllers implement different versions of the spec,
and different features of it. It is useful to gather this
information when debugging.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/nvme.c       | 6 ++++++
 block/trace-events | 1 +
 2 files changed, 7 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c
index 80c4318d8fc..2b5421e7aa6 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -708,6 +708,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     AioContext *aio_context = bdrv_get_aio_context(bs);
     int ret;
     uint64_t cap;
+    uint32_t ver;
     uint64_t timeout_ms;
     uint64_t deadline, now;
     volatile NvmeBar *regs = NULL;
@@ -764,6 +765,11 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     bs->bl.request_alignment = s->page_size;
     timeout_ms = MIN(500 * NVME_CAP_TO(cap), 30000);
 
+    ver = le32_to_cpu(regs->vs);
+    trace_nvme_controller_spec_version(extract32(ver, 16, 16),
+                                       extract32(ver, 8, 8),
+                                       extract32(ver, 0, 8));
+
     /* Reset device to get a clean state. */
     regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE);
     /* Wait for CSTS.RDY = 0. */
diff --git a/block/trace-events b/block/trace-events
index 8368f4acb0b..ecbc32a80a9 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -136,6 +136,7 @@ qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s
 # nvme.c
 nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64
 nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64
+nvme_controller_spec_version(uint32_t mjr, uint32_t mnr, uint32_t ter) "Specification supported: %u.%u.%u"
 nvme_kick(void *s, unsigned q_index) "s %p q #%u"
 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"
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] block/nvme: Properly display doorbell stride length in trace event
  2021-01-27 21:21 ` [PATCH 1/2] block/nvme: Properly display doorbell stride length in trace event Philippe Mathieu-Daudé
@ 2021-01-27 21:30   ` Klaus Jensen
  0 siblings, 0 replies; 6+ messages in thread
From: Klaus Jensen @ 2021-01-27 21:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Fam Zheng, Kevin Wolf, qemu-block, qemu-devel, Max Reitz,
	Stefan Hajnoczi

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

On Jan 27 22:21, Philippe Mathieu-Daudé wrote:
> Commit 15b2260bef3 ("block/nvme: Trace controller capabilities")
> misunderstood the doorbell stride value from the datasheet, use
> the correct one. The 'doorbell_scale' variable used few lines
> later is correct.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

> ---
>  block/nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 5a6fbacf4a5..80c4318d8fc 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -745,7 +745,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
>      trace_nvme_controller_capability("Contiguous Queues Required",
>                                       NVME_CAP_CQR(cap));
>      trace_nvme_controller_capability("Doorbell Stride",
> -                                     2 << (2 + NVME_CAP_DSTRD(cap)));
> +                                     1 << (2 + NVME_CAP_DSTRD(cap)));
>      trace_nvme_controller_capability("Subsystem Reset Supported",
>                                       NVME_CAP_NSSRS(cap));
>      trace_nvme_controller_capability("Memory Page Size Minimum",
> -- 
> 2.26.2
> 
> 

-- 
One of us - No more doubt, silence or taboo about mental illness.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] block/nvme: Trace NVMe spec version supported by the controller
  2021-01-27 21:21 ` [PATCH 2/2] block/nvme: Trace NVMe spec version supported by the controller Philippe Mathieu-Daudé
@ 2021-01-27 21:32   ` Klaus Jensen
  0 siblings, 0 replies; 6+ messages in thread
From: Klaus Jensen @ 2021-01-27 21:32 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Fam Zheng, Kevin Wolf, qemu-block, qemu-devel, Max Reitz,
	Stefan Hajnoczi

[-- Attachment #1: Type: text/plain, Size: 2342 bytes --]

On Jan 27 22:21, Philippe Mathieu-Daudé wrote:
> NVMe controllers implement different versions of the spec,
> and different features of it. It is useful to gather this
> information when debugging.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

> ---
>  block/nvme.c       | 6 ++++++
>  block/trace-events | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 80c4318d8fc..2b5421e7aa6 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -708,6 +708,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
>      AioContext *aio_context = bdrv_get_aio_context(bs);
>      int ret;
>      uint64_t cap;
> +    uint32_t ver;
>      uint64_t timeout_ms;
>      uint64_t deadline, now;
>      volatile NvmeBar *regs = NULL;
> @@ -764,6 +765,11 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
>      bs->bl.request_alignment = s->page_size;
>      timeout_ms = MIN(500 * NVME_CAP_TO(cap), 30000);
>  
> +    ver = le32_to_cpu(regs->vs);
> +    trace_nvme_controller_spec_version(extract32(ver, 16, 16),
> +                                       extract32(ver, 8, 8),
> +                                       extract32(ver, 0, 8));
> +
>      /* Reset device to get a clean state. */
>      regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE);
>      /* Wait for CSTS.RDY = 0. */
> diff --git a/block/trace-events b/block/trace-events
> index 8368f4acb0b..ecbc32a80a9 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -136,6 +136,7 @@ qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s
>  # nvme.c
>  nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64
>  nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64
> +nvme_controller_spec_version(uint32_t mjr, uint32_t mnr, uint32_t ter) "Specification supported: %u.%u.%u"
>  nvme_kick(void *s, unsigned q_index) "s %p q #%u"
>  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"
> -- 
> 2.26.2
> 
> 

-- 
One of us - No more doubt, silence or taboo about mental illness.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] block/nvme: Minor tracing improvements
  2021-01-27 21:21 [PATCH 0/2] block/nvme: Minor tracing improvements Philippe Mathieu-Daudé
  2021-01-27 21:21 ` [PATCH 1/2] block/nvme: Properly display doorbell stride length in trace event Philippe Mathieu-Daudé
  2021-01-27 21:21 ` [PATCH 2/2] block/nvme: Trace NVMe spec version supported by the controller Philippe Mathieu-Daudé
@ 2021-02-02 14:44 ` Kevin Wolf
  2 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2021-02-02 14:44 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Fam Zheng, Stefan Hajnoczi, qemu-devel, qemu-block, Max Reitz

Am 27.01.2021 um 22:21 hat Philippe Mathieu-Daudé geschrieben:
> Fix an incorrect trace event (sometimes it is better to
> have no information rather than having incorrect info...)
> and add another event to display the spec implemented by
> the controller.

Thanks, applied to the block branch.

Kevin



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-02 15:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-27 21:21 [PATCH 0/2] block/nvme: Minor tracing improvements Philippe Mathieu-Daudé
2021-01-27 21:21 ` [PATCH 1/2] block/nvme: Properly display doorbell stride length in trace event Philippe Mathieu-Daudé
2021-01-27 21:30   ` Klaus Jensen
2021-01-27 21:21 ` [PATCH 2/2] block/nvme: Trace NVMe spec version supported by the controller Philippe Mathieu-Daudé
2021-01-27 21:32   ` Klaus Jensen
2021-02-02 14:44 ` [PATCH 0/2] block/nvme: Minor tracing improvements Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.