diff mbox series

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

Message ID 20201027135547.374946-5-philmd@redhat.com
State Superseded
Headers show
Series block/nvme: Fix Aarch64 host | expand

Commit Message

Philippe Mathieu-Daudé Oct. 27, 2020, 1:55 p.m. UTC
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(+)

Comments

Eric Auger Oct. 28, 2020, 10:20 a.m. UTC | #1
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
Philippe Mathieu-Daudé Oct. 28, 2020, 10:25 a.m. UTC | #2
On 10/28/20 11:20 AM, Auger Eric wrote:
> 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)

This is a boolean:

  Contiguous Queues Required (CQR): This field is set to ‘1’ if
  the controller requires that I/O Submission Queues and I/O
  Completion Queues are required to be physically contiguous.
  This field is cleared to ‘0’ if the controller supports I/O
  Submission Queues and I/O Completion Queues that are not
  physically contiguous. If this field is set to ‘1’, then the
  Physically Contiguous bit (CDW11.PC) in the Create I/O Submission
  Queue and Create I/O Completion Queue commands shall be set to ‘1’.

>> +    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
>
Eric Auger Oct. 28, 2020, 10:36 a.m. UTC | #3
Hi Philippe,
On 10/28/20 11:25 AM, Philippe Mathieu-Daudé wrote:
> On 10/28/20 11:20 AM, Auger Eric wrote:
>> 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)
> 
> This is a boolean:
> 
>   Contiguous Queues Required (CQR): This field is set to ‘1’ if
>   the controller requires that I/O Submission Queues and I/O
>   Completion Queues are required to be physically contiguous.
>   This field is cleared to ‘0’ if the controller supports I/O
>   Submission Queues and I/O Completion Queues that are not
>   physically contiguous. If this field is set to ‘1’, then the
>   Physically Contiguous bit (CDW11.PC) in the Create I/O Submission
>   Queue and Create I/O Completion Queue commands shall be set to ‘1’.

Oh I mixed with NCQR :-(

sorry for the noise
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

> 
>>> +    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
>>
> 
>
diff mbox series

Patch

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));
+    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"