[SeaBIOS] [PATCH 1/5] nvme: Rework nvme_io_readwrite() to return -1 on error

Kevin O'Connor posted 5 patches 3 years, 10 months ago
There is a newer version of this series
[SeaBIOS] [PATCH 1/5] nvme: Rework nvme_io_readwrite() to return -1 on error
Posted by Kevin O'Connor 3 years, 10 months ago
Rename nvme_io_readwrite() to nvme_io_xfer() and change it so it
implements the debugging dprintf() and it returns -1 on an error.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 src/hw/nvme.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index f035fa2..a97501b 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -417,8 +417,8 @@ err:
 /* Reads count sectors into buf. Returns DISK_RET_*. The buffer cannot cross
    page boundaries. */
 static int
-nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
-                  int write)
+nvme_io_xfer(struct nvme_namespace *ns, u64 lba, void *buf, u16 count,
+             int write)
 {
     u32 buf_addr = (u32)buf;
     void *prp2;
@@ -426,7 +426,7 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
     if (buf_addr & 0x3) {
         /* Buffer is misaligned */
         warn_internalerror();
-        return DISK_RET_EBADTRACK;
+        return -1;
     }
 
     if ((ns->block_size * count) > (NVME_PAGE_SIZE * 2)) {
@@ -457,10 +457,12 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
         dprintf(2, "read io: %08x %08x %08x %08x\n",
                 cqe.dword[0], cqe.dword[1], cqe.dword[2], cqe.dword[3]);
 
-        return DISK_RET_EBADTRACK;
+        return -1;
     }
 
-    return DISK_RET_SUCCESS;
+    dprintf(5, "ns %u %s lba %llu+%u\n", ns->ns_id, write ? "write" : "read",
+            lba, count);
+    return count;
 }
 
 static void nvme_reset_prpl(struct nvme_namespace *ns)
@@ -716,20 +718,18 @@ nvme_scan(void)
 static int
 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, blocks;
 
-    for (i = 0; i < op->count && res == DISK_RET_SUCCESS;) {
+    for (i = 0; i < op->count;) {
         u16 blocks_remaining = op->count - i;
         char *op_buf = op->buf_fl + i * ns->block_size;
 
         blocks = nvme_build_prpl(ns, op_buf, blocks_remaining);
         if (blocks) {
-            res = nvme_io_readwrite(ns, op->lba + i, ns->prp1, blocks, write);
-            dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
-                                                                      : "read",
-                    op->lba, blocks, res);
+            int res = nvme_io_xfer(ns, op->lba + i, ns->prp1, blocks, write);
+            if (res < 0)
+                return DISK_RET_EBADTRACK;
         } else {
             blocks = blocks_remaining < max_blocks ? blocks_remaining
                                                    : max_blocks;
@@ -738,12 +738,12 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
                 memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
             }
 
-            res = nvme_io_readwrite(ns, op->lba + i, ns->dma_buffer, blocks, write);
-            dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
-                                                                      : "read",
-                    op->lba + i, blocks, res);
+            int res = nvme_io_xfer(ns, op->lba + i, ns->dma_buffer,
+                                   blocks, write);
+            if (res < 0)
+                return DISK_RET_EBADTRACK;
 
-            if (!write && res == DISK_RET_SUCCESS) {
+            if (!write) {
                 memcpy(op_buf, ns->dma_buffer, blocks * ns->block_size);
             }
         }
@@ -751,7 +751,7 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
         i += blocks;
     }
 
-    return res;
+    return DISK_RET_SUCCESS;
 }
 
 int
-- 
2.31.1

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH 1/5] nvme: Rework nvme_io_readwrite() to return -1 on error
Posted by Alexander Graf via SeaBIOS 3 years, 10 months ago
On 19.01.22 19:45, Kevin O'Connor wrote:
> Rename nvme_io_readwrite() to nvme_io_xfer() and change it so it
> implements the debugging dprintf() and it returns -1 on an error.
>
> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>


Reviewed-by: Alexander Graf <graf@amazon.com>

Alex


> ---
>   src/hw/nvme.c | 34 +++++++++++++++++-----------------
>   1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/hw/nvme.c b/src/hw/nvme.c
> index f035fa2..a97501b 100644
> --- a/src/hw/nvme.c
> +++ b/src/hw/nvme.c
> @@ -417,8 +417,8 @@ err:
>   /* Reads count sectors into buf. Returns DISK_RET_*. The buffer cannot cross
>      page boundaries. */
>   static int
> -nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
> -                  int write)
> +nvme_io_xfer(struct nvme_namespace *ns, u64 lba, void *buf, u16 count,
> +             int write)
>   {
>       u32 buf_addr = (u32)buf;
>       void *prp2;
> @@ -426,7 +426,7 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
>       if (buf_addr & 0x3) {
>           /* Buffer is misaligned */
>           warn_internalerror();
> -        return DISK_RET_EBADTRACK;
> +        return -1;
>       }
>
>       if ((ns->block_size * count) > (NVME_PAGE_SIZE * 2)) {
> @@ -457,10 +457,12 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
>           dprintf(2, "read io: %08x %08x %08x %08x\n",
>                   cqe.dword[0], cqe.dword[1], cqe.dword[2], cqe.dword[3]);
>
> -        return DISK_RET_EBADTRACK;
> +        return -1;
>       }
>
> -    return DISK_RET_SUCCESS;
> +    dprintf(5, "ns %u %s lba %llu+%u\n", ns->ns_id, write ? "write" : "read",
> +            lba, count);
> +    return count;
>   }
>
>   static void nvme_reset_prpl(struct nvme_namespace *ns)
> @@ -716,20 +718,18 @@ nvme_scan(void)
>   static int
>   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, blocks;
>
> -    for (i = 0; i < op->count && res == DISK_RET_SUCCESS;) {
> +    for (i = 0; i < op->count;) {
>           u16 blocks_remaining = op->count - i;
>           char *op_buf = op->buf_fl + i * ns->block_size;
>
>           blocks = nvme_build_prpl(ns, op_buf, blocks_remaining);
>           if (blocks) {
> -            res = nvme_io_readwrite(ns, op->lba + i, ns->prp1, blocks, write);
> -            dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
> -                                                                      : "read",
> -                    op->lba, blocks, res);
> +            int res = nvme_io_xfer(ns, op->lba + i, ns->prp1, blocks, write);
> +            if (res < 0)
> +                return DISK_RET_EBADTRACK;
>           } else {
>               blocks = blocks_remaining < max_blocks ? blocks_remaining
>                                                      : max_blocks;
> @@ -738,12 +738,12 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
>                   memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
>               }
>
> -            res = nvme_io_readwrite(ns, op->lba + i, ns->dma_buffer, blocks, write);
> -            dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
> -                                                                      : "read",
> -                    op->lba + i, blocks, res);
> +            int res = nvme_io_xfer(ns, op->lba + i, ns->dma_buffer,
> +                                   blocks, write);
> +            if (res < 0)
> +                return DISK_RET_EBADTRACK;
>
> -            if (!write && res == DISK_RET_SUCCESS) {
> +            if (!write) {
>                   memcpy(op_buf, ns->dma_buffer, blocks * ns->block_size);
>               }
>           }
> @@ -751,7 +751,7 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
>           i += blocks;
>       }
>
> -    return res;
> +    return DISK_RET_SUCCESS;
>   }
>
>   int
> --
> 2.31.1
>
> _______________________________________________
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-leave@seabios.org



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH 1/5] nvme: Rework nvme_io_readwrite() to return -1 on error
Posted by Alexander Graf via SeaBIOS 3 years, 10 months ago
On 19.01.22 19:45, Kevin O'Connor wrote:
> Rename nvme_io_readwrite() to nvme_io_xfer() and change it so it
> implements the debugging dprintf() and it returns -1 on an error.
>
> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> ---
>   src/hw/nvme.c | 34 +++++++++++++++++-----------------
>   1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/hw/nvme.c b/src/hw/nvme.c
> index f035fa2..a97501b 100644
> --- a/src/hw/nvme.c
> +++ b/src/hw/nvme.c
> @@ -417,8 +417,8 @@ err:
>   /* Reads count sectors into buf. Returns DISK_RET_*. The buffer cannot cross


This comment no longer is accurate after the patch. I guess it was half 
stale before already, but now it's completely wrong :).


Alex


>      page boundaries. */
>   static int
> -nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
> -                  int write)
> +nvme_io_xfer(struct nvme_namespace *ns, u64 lba, void *buf, u16 count,
> +             int write)
>   {
>       u32 buf_addr = (u32)buf;
>       void *prp2;
> @@ -426,7 +426,7 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
>       if (buf_addr & 0x3) {
>           /* Buffer is misaligned */
>           warn_internalerror();
> -        return DISK_RET_EBADTRACK;
> +        return -1;
>       }
>
>       if ((ns->block_size * count) > (NVME_PAGE_SIZE * 2)) {
> @@ -457,10 +457,12 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
>           dprintf(2, "read io: %08x %08x %08x %08x\n",
>                   cqe.dword[0], cqe.dword[1], cqe.dword[2], cqe.dword[3]);
>
> -        return DISK_RET_EBADTRACK;
> +        return -1;
>       }
>
> -    return DISK_RET_SUCCESS;
> +    dprintf(5, "ns %u %s lba %llu+%u\n", ns->ns_id, write ? "write" : "read",
> +            lba, count);
> +    return count;
>   }
>
>   static void nvme_reset_prpl(struct nvme_namespace *ns)
> @@ -716,20 +718,18 @@ nvme_scan(void)
>   static int
>   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, blocks;
>
> -    for (i = 0; i < op->count && res == DISK_RET_SUCCESS;) {
> +    for (i = 0; i < op->count;) {
>           u16 blocks_remaining = op->count - i;
>           char *op_buf = op->buf_fl + i * ns->block_size;
>
>           blocks = nvme_build_prpl(ns, op_buf, blocks_remaining);
>           if (blocks) {
> -            res = nvme_io_readwrite(ns, op->lba + i, ns->prp1, blocks, write);
> -            dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
> -                                                                      : "read",
> -                    op->lba, blocks, res);
> +            int res = nvme_io_xfer(ns, op->lba + i, ns->prp1, blocks, write);
> +            if (res < 0)
> +                return DISK_RET_EBADTRACK;
>           } else {
>               blocks = blocks_remaining < max_blocks ? blocks_remaining
>                                                      : max_blocks;
> @@ -738,12 +738,12 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
>                   memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
>               }
>
> -            res = nvme_io_readwrite(ns, op->lba + i, ns->dma_buffer, blocks, write);
> -            dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
> -                                                                      : "read",
> -                    op->lba + i, blocks, res);
> +            int res = nvme_io_xfer(ns, op->lba + i, ns->dma_buffer,
> +                                   blocks, write);
> +            if (res < 0)
> +                return DISK_RET_EBADTRACK;
>
> -            if (!write && res == DISK_RET_SUCCESS) {
> +            if (!write) {
>                   memcpy(op_buf, ns->dma_buffer, blocks * ns->block_size);
>               }
>           }
> @@ -751,7 +751,7 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
>           i += blocks;
>       }
>
> -    return res;
> +    return DISK_RET_SUCCESS;
>   }
>
>   int
> --
> 2.31.1
>
> _______________________________________________
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-leave@seabios.org



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


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