1. Patchew
  2. Seabios
  3. View series

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

Filippo Sironi posted 1 patch 7 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/1507668123-19960-1-git-send-email-sironi@amazon.de
There is a newer version of this series
src/hw/nvme.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[SeaBIOS] [PATCH] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
Posted by Filippo Sironi 7 years, 7 months ago 
... and cap the length to 256 to avoid oom-ing.

Signed-off-by: Filippo Sironi <sironi@amazon.de>
---
 src/hw/nvme.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index 946487f4fd43..19a4e7a48d3c 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -20,6 +20,8 @@
 #include "nvme.h"
 #include "nvme-int.h"
 
+#define min(a, b) ({ typeof(a) _a = a; typeof(b) _b = b; _a < _b ? _a : _b; })
+
 static void *
 zalloc_page_aligned(struct zone_s *zone, u32 size)
 {
@@ -318,8 +320,10 @@ 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;
 
-    rc = nvme_init_cq(ctrl, cq, q_idx, NVME_PAGE_SIZE / sizeof(struct nvme_cqe));
+    length = min(1 + (ctrl->reg->cap & 0xffff), NVME_PAGE_SIZE / sizeof(struct nvme_cqe));
+    rc = nvme_init_cq(ctrl, cq, q_idx, length);
     if (rc) {
         goto err;
     }
@@ -359,8 +363,10 @@ 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;
 
-    rc = nvme_init_sq(ctrl, sq, q_idx, NVME_PAGE_SIZE / sizeof(struct nvme_cqe), cq);
+    length = min(1 + (ctrl->reg->cap & 0xffff), NVME_PAGE_SIZE / sizeof(struct nvme_cqe));
+    rc = nvme_init_sq(ctrl, sq, q_idx, length, cq);
     if (rc) {
         goto err;
     }
-- 
2.7.4


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
Posted by Kevin O'Connor 7 years, 7 months ago 
On Tue, Oct 10, 2017 at 10:42:03PM +0200, Filippo Sironi wrote:
> ... and cap the length to 256 to avoid oom-ing.

Please provide more information on what this patch does.  What
additional capabilities does it provide?  What could break if this
patch is not applied?

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
Posted by Sironi, Filippo 7 years, 7 months ago 
> On 12. Oct 2017, at 00:54, Kevin O'Connor <kevin@koconnor.net> wrote:
> 
> On Tue, Oct 10, 2017 at 10:42:03PM +0200, Filippo Sironi wrote:
>> ... and cap the length to 256 to avoid oom-ing.
> 
> Please provide more information on what this patch does.  What
> additional capabilities does it provide?  What could break if this
> patch is not applied?
> 
> -Kevin

Kevin,

as the commit title says, the patch is mostly about using the advertized maximum
number of entries in the I/O queues rather than using a fixed number (256).  In
addition, the patch caps the number of entries in the I/O queues to 256 since
from  my testing, SeaBIOS was oom-ing when attempting to allocate more.

I'm going to repost with a more verbose commit message.

Filippo
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH v2] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
Posted by Filippo Sironi 7 years, 7 months ago 
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 <sironi@amazon.de>
---
 src/hw/nvme.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index 946487f4fd43..19a4e7a48d3c 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -20,6 +20,8 @@
 #include "nvme.h"
 #include "nvme-int.h"
 
+#define min(a, b) ({ typeof(a) _a = a; typeof(b) _b = b; _a < _b ? _a : _b; })
+
 static void *
 zalloc_page_aligned(struct zone_s *zone, u32 size)
 {
@@ -318,8 +320,10 @@ 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;
 
-    rc = nvme_init_cq(ctrl, cq, q_idx, NVME_PAGE_SIZE / sizeof(struct nvme_cqe));
+    length = min(1 + (ctrl->reg->cap & 0xffff), NVME_PAGE_SIZE / sizeof(struct nvme_cqe));
+    rc = nvme_init_cq(ctrl, cq, q_idx, length);
     if (rc) {
         goto err;
     }
@@ -359,8 +363,10 @@ 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;
 
-    rc = nvme_init_sq(ctrl, sq, q_idx, NVME_PAGE_SIZE / sizeof(struct nvme_cqe), cq);
+    length = min(1 + (ctrl->reg->cap & 0xffff), NVME_PAGE_SIZE / sizeof(struct nvme_cqe));
+    rc = nvme_init_sq(ctrl, sq, q_idx, length, cq);
     if (rc) {
         goto err;
     }
-- 
2.7.4


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
Posted by Kevin O'Connor 7 years, 6 months ago 
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 <sironi@amazon.de>
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 <sironi@amazon.de>

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;
     }

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
Posted by Sironi, Filippo 7 years, 6 months ago 
> On 14. Oct 2017, at 17:15, Kevin O'Connor <kevin@koconnor.net> wrote:
> 
> 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

Looks good to me.

Thanks,
Filippo


> Author: Filippo Sironi <sironi@amazon.de>
> 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 <sironi@amazon.de>
> 
> 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;
>     }
> 

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] nvme: Use the Maximum Queue Entries Supported (MQES) to initialize I/O queues
Posted by Kevin O'Connor 7 years, 6 months ago 
On Sun, Oct 15, 2017 at 10:30:29AM +0000, Sironi, Filippo wrote:
> 
> > On 14. Oct 2017, at 17:15, Kevin O'Connor <kevin@koconnor.net> wrote:
> > 
> > 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
> 
> Looks good to me.

Thanks.  I committed this change.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.