Re: [SeaBIOS] [PATCH v2] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues

[Kevin O'Connor]

On Thu, Oct 12, 2017 at 12:42:34AM +0200, Filippo Sironi wrote:
> Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
> depth rather than picking a fixed number (256) which might not be
> supported by some NVMe controllers (the NVMe specification says that an
> NVMe controller may support any number between 2 to 4096).
>
> Still cap the I/O queues depth to 256 since, during my testing, SeaBIOS
> was running out of memory when using something higher than 256 (4096 on
> the NVMe controller that I've had a chance to try).

Okay, thanks.  So, it sounds like it is a bug fix (as it could prevent
a failure if the hardware queue depth is less than 256).

I'd prefer not to introduce min()/max() macros to individual driver
files.  (I'm fine with a patch that globally introduces min/max, but I
don't think we should be adding them piecemeal.)  Are you okay with
the patch below instead?

-Kevin

Author: Filippo Sironi 
Date:   Thu Oct 12 00:42:34 2017 +0200

nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues

Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
    depth rather than picking a fixed number (256) which might not be
    supported by some NVMe controllers (the NVMe specification says that an
    NVMe controller may support any number between 2 to 4096).

Still cap the I/O queues depth to 256 since, during my testing, SeaBIOS
    was running out of memory when using something higher than 256 (4096 on
    the NVMe controller that I've had a chance to try).

Signed-off-by: Filippo Sironi

diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index 946487f..e6d739d 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -318,8 +318,11 @@ nvme_create_io_cq(struct nvme_ctrl *ctrl, struct nvme_cq *cq, u16 q_idx)
 {
     int rc;
     struct nvme_sqe *cmd_create_cq;
+    u16 length = 1 + (ctrl->reg->cap & 0xffff);
+    if (length > NVME_PAGE_SIZE / sizeof(struct nvme_cqe))
+        length = NVME_PAGE_SIZE / sizeof(struct nvme_cqe);

-    rc = nvme_init_cq(ctrl, cq, q_idx, NVME_PAGE_SIZE / sizeof(struct nvme_cqe));
+    rc = nvme_init_cq(ctrl, cq, q_idx, length);
     if (rc) {
         goto err;
     }
@@ -359,8 +362,11 @@ nvme_create_io_sq(struct nvme_ctrl *ctrl, struct nvme_sq *sq, u16 q_idx, struct
 {
     int rc;
     struct nvme_sqe *cmd_create_sq;
+    u16 length = 1 + (ctrl->reg->cap & 0xffff);
+    if (length > NVME_PAGE_SIZE / sizeof(struct nvme_cqe))
+        length = NVME_PAGE_SIZE / sizeof(struct nvme_cqe);

-    rc = nvme_init_sq(ctrl, sq, q_idx, NVME_PAGE_SIZE / sizeof(struct nvme_cqe), cq);
+    rc = nvme_init_sq(ctrl, sq, q_idx, length, cq);
     if (rc) {
         goto err;
     }