[SeaBIOS] [PATCH] nvme: Clean up nvme_cmd_readwrite()

David Woodhouse posted 1 patch 3 weeks, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/7b9788635cd81b782e9fc77f6a9c372ef2452ff2.camel@infradead.org
src/hw/nvme.c | 47 +++++++++++++++++++++--------------------------
1 file changed, 21 insertions(+), 26 deletions(-)

[SeaBIOS] [PATCH] nvme: Clean up nvme_cmd_readwrite()

Posted by David Woodhouse 3 weeks, 3 days ago
From: David woodhouse <dwmw@amazon.co.uk>

This ended up with an odd mix of recursion (albeit *mostly* tail-recursion)
and interation that could have been prettier. Make it prettier by making
nvme_build_prpl() return the number of blocks it consumes, and just
looping.

If nvme_build_prpl() doesn't want to play, just fall through to the
original loop.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
On Fri, 2020-10-30 at 19:09 -0400, Kevin O'Connor wrote:
> FWIW, I agree that a version without recursion (even tail recursion)
> would be nice.  If someone wants to test and confirm that, then that
> would be great.

Seems to work here... I just tidied it up a little more and added a
dprintf() to match the original loop. Didn't see it actually limiting
the requests in my testing so I made nvme_build_prpl() limit to 32
blocks and then it did... and still seems to put things in the right
place.

 src/hw/nvme.c | 47 +++++++++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index cc37bca..5363d3f 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -472,19 +472,20 @@ static int nvme_add_prpl(struct nvme_namespace *ns, u64 base)
 
 int nvme_build_prpl(struct nvme_namespace *ns, struct disk_op_s *op)
 {
-    int first_page = 1;
+    int first_page = 1, count = op->count;
     u32 base = (long)op->buf_fl;
-    s32 size = op->count * ns->block_size;
+    s32 size;
 
-    if (op->count > ns->max_req_size)
-        return -1;
+    if (count > ns->max_req_size)
+        count = ns->max_req_size;
 
     nvme_reset_prpl(ns);
 
+    size = count * ns->block_size;
     /* Special case for transfers that fit into PRP1, but are unaligned */
     if (((size + (base & ~NVME_PAGE_MASK)) <= NVME_PAGE_SIZE)) {
         ns->prp1 = op->buf_fl;
-        return 0;
+        return count;
     }
 
     /* Every request has to be page aligned */
@@ -506,7 +507,7 @@ int nvme_build_prpl(struct nvme_namespace *ns, struct disk_op_s *op)
             return -1;
     }
 
-    return 0;
+    return count;
 }
 
 static int
@@ -725,34 +726,28 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
 {
     int res = DISK_RET_SUCCESS;
     u16 const max_blocks = NVME_PAGE_SIZE / ns->block_size;
-    u16 i;
+    u16 i, blocks;
 
-    /* Split up requests that are larger than the device can handle */
-    if (op->count > ns->max_req_size) {
-        u16 count = op->count;
+    while (op->count && ((blocks = nvme_build_prpl(ns, op)) > 0)) {
+        res = nvme_io_readwrite(ns, op->lba, ns->prp1, blocks, write);
+        dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
+                                                                  : "read",
+                op->lba, blocks, res);
 
-        /* Handle the first max_req_size elements */
-        op->count = ns->max_req_size;
-        if (nvme_cmd_readwrite(ns, op, write))
+        if (res != DISK_RET_SUCCESS)
             return res;
 
-        /* Handle the remainder of the request */
-        op->count = count - ns->max_req_size;
-        op->lba += ns->max_req_size;
-        op->buf_fl += (ns->max_req_size * ns->block_size);
-        return nvme_cmd_readwrite(ns, op, write);
-    }
-
-    if (!nvme_build_prpl(ns, op)) {
-        /* Request goes via PRP List logic */
-        return nvme_io_readwrite(ns, op->lba, ns->prp1, op->count, write);
+        op->count -= blocks;
+        op->lba += blocks;
+        op->buf_fl += (blocks * ns->block_size);
     }
 
     for (i = 0; i < op->count && res == DISK_RET_SUCCESS;) {
-        u16 blocks_remaining = op->count - i;
-        u16 blocks = blocks_remaining < max_blocks ? blocks_remaining
-                                                   : max_blocks;
         char *op_buf = op->buf_fl + i * ns->block_size;
+        u16 blocks_remaining = op->count - i;
+
+        blocks = blocks_remaining < max_blocks ? blocks_remaining
+                                               : max_blocks;
 
         if (write) {
             memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
-- 
2.17.1



_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org