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

[David Woodhouse]

From: David woodhouse

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