1. Patchew
  2. Seabios
  3. View series

[SeaBIOS] [PATCH] nvme: Move PRP data to ZoneHigh

Alexander Graf via SeaBIOS posted 1 patch 2 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20220119142146.21370-1-graf@amazon.com
src/hw/nvme-int.h | 12 ++++++++----
src/hw/nvme.c     | 24 ++++++++++++++++--------
2 files changed, 24 insertions(+), 12 deletions(-)
[SeaBIOS] [PATCH] nvme: Move PRP data to ZoneHigh
Posted by Alexander Graf via SeaBIOS 2 years, 3 months ago 
Commit 01f2736cc905d ("nvme: Pass large I/O requests as PRP lists")
introduced multi-page requests using the NVMe PRP mechanism. To store the
list and "first page to write to" hints, it added fields to the NVMe
namespace struct.

Unfortunately, that struct resides in fseg which is read-only at runtime.
While KVM ignores the read-only part and allows writes, real hardware and
TCG adhere to the semantics and ignore writes to the fseg region. The net
effect of that is that reads and writes were always happening on address 0,
unless they went through the bounce buffer logic.

This patch splits moves all PRP maintenance data from the actual namespace
allocation and allocates them in ZoneHigh instead. That way, the PRP list
is fully read-write at runtime.

Fixes: 01f2736cc905d ("nvme: Pass large I/O requests as PRP lists")
Reported-by: Matt DeVillier <matt.devillier@gmail.com>
Signed-off-by: Alexander Graf <graf@amazon.com>
---
 src/hw/nvme-int.h | 12 ++++++++----
 src/hw/nvme.c     | 24 ++++++++++++++++--------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/hw/nvme-int.h b/src/hw/nvme-int.h
index a4c1555..ceb2c79 100644
--- a/src/hw/nvme-int.h
+++ b/src/hw/nvme-int.h
@@ -108,6 +108,13 @@ struct nvme_ctrl {
     struct nvme_cq io_cq;
 };
 
+/* Page List Maintenance Data */
+struct nvme_prp_info {
+    u32 prpl_len;
+    void *prp1;
+    u64 prpl[NVME_MAX_PRPL_ENTRIES];
+};
+
 struct nvme_namespace {
     struct drive_s drive;
     struct nvme_ctrl *ctrl;
@@ -123,10 +130,7 @@ struct nvme_namespace {
     /* Page aligned buffer of size NVME_PAGE_SIZE. */
     char *dma_buffer;
 
-    /* Page List */
-    u32 prpl_len;
-    void *prp1;
-    u64 prpl[NVME_MAX_PRPL_ENTRIES];
+    struct nvme_prp_info *prp;
 };
 
 /* Data structures for NVMe admin identify commands */
diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index f035fa2..963c31e 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -267,6 +267,14 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, u32 ns_idx, u8 mdts)
     ns->ns_id = ns_id;
     ns->lba_count = id->nsze;
 
+    ns->prp = malloc_high(sizeof(*ns->prp));
+    if (!ns->prp) {
+        warn_noalloc();
+        free(ns);
+        goto free_buffer;
+    }
+    memset(ns->prp, 0, sizeof(*ns->prp));
+
     struct nvme_lba_format *fmt = &id->lbaf[current_lba_format];
 
     ns->block_size    = 1U << fmt->lbads;
@@ -431,10 +439,10 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
 
     if ((ns->block_size * count) > (NVME_PAGE_SIZE * 2)) {
         /* We need to describe more than 2 pages, rely on PRP List */
-        prp2 = ns->prpl;
+        prp2 = ns->prp->prpl;
     } else if ((ns->block_size * count) > NVME_PAGE_SIZE) {
         /* Directly embed the 2nd page if we only need 2 pages */
-        prp2 = (void *)(long)ns->prpl[0];
+        prp2 = (void *)(long)ns->prp->prpl[0];
     } else {
         /* One page is enough, don't expose anything else */
         prp2 = NULL;
@@ -465,15 +473,15 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
 
 static void nvme_reset_prpl(struct nvme_namespace *ns)
 {
-    ns->prpl_len = 0;
+    ns->prp->prpl_len = 0;
 }
 
 static int nvme_add_prpl(struct nvme_namespace *ns, u64 base)
 {
-    if (ns->prpl_len >= NVME_MAX_PRPL_ENTRIES)
+    if (ns->prp->prpl_len >= NVME_MAX_PRPL_ENTRIES)
         return -1;
 
-    ns->prpl[ns->prpl_len++] = base;
+    ns->prp->prpl[ns->prp->prpl_len++] = base;
 
     return 0;
 }
@@ -492,7 +500,7 @@ static int nvme_build_prpl(struct nvme_namespace *ns, void *op_buf, u16 count)
     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;
+        ns->prp->prp1 = op_buf;
         return count;
     }
 
@@ -507,7 +515,7 @@ static int nvme_build_prpl(struct nvme_namespace *ns, void *op_buf, u16 count)
     for (; size > 0; base += NVME_PAGE_SIZE, size -= NVME_PAGE_SIZE) {
         if (first_page) {
             /* First page is special */
-            ns->prp1 = (void*)base;
+            ns->prp->prp1 = (void*)base;
             first_page = 0;
             continue;
         }
@@ -726,7 +734,7 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
 
         blocks = nvme_build_prpl(ns, op_buf, blocks_remaining);
         if (blocks) {
-            res = nvme_io_readwrite(ns, op->lba + i, ns->prp1, blocks, write);
+            res = nvme_io_readwrite(ns, op->lba + i, ns->prp->prp1, blocks, write);
             dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
                                                                       : "read",
                     op->lba, blocks, res);
-- 
2.28.0.394.ge197136389




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] nvme: Move PRP data to ZoneHigh
Posted by Matt DeVillier 2 years, 3 months ago 
verified working on Purism Librem hardware :)

perhaps worth rolling into a 1.15.1 release, so that coreboot doesn't
have to roll back to 1.14.0 until 1.16.0 is released?

On Wed, Jan 19, 2022 at 8:22 AM Alexander Graf <graf@amazon.com> wrote:
>
> Commit 01f2736cc905d ("nvme: Pass large I/O requests as PRP lists")
> introduced multi-page requests using the NVMe PRP mechanism. To store the
> list and "first page to write to" hints, it added fields to the NVMe
> namespace struct.
>
> Unfortunately, that struct resides in fseg which is read-only at runtime.
> While KVM ignores the read-only part and allows writes, real hardware and
> TCG adhere to the semantics and ignore writes to the fseg region. The net
> effect of that is that reads and writes were always happening on address 0,
> unless they went through the bounce buffer logic.
>
> This patch splits moves all PRP maintenance data from the actual namespace
> allocation and allocates them in ZoneHigh instead. That way, the PRP list
> is fully read-write at runtime.
>
> Fixes: 01f2736cc905d ("nvme: Pass large I/O requests as PRP lists")
> Reported-by: Matt DeVillier <matt.devillier@gmail.com>
> Signed-off-by: Alexander Graf <graf@amazon.com>
> ---
>  src/hw/nvme-int.h | 12 ++++++++----
>  src/hw/nvme.c     | 24 ++++++++++++++++--------
>  2 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/src/hw/nvme-int.h b/src/hw/nvme-int.h
> index a4c1555..ceb2c79 100644
> --- a/src/hw/nvme-int.h
> +++ b/src/hw/nvme-int.h
> @@ -108,6 +108,13 @@ struct nvme_ctrl {
>      struct nvme_cq io_cq;
>  };
>
> +/* Page List Maintenance Data */
> +struct nvme_prp_info {
> +    u32 prpl_len;
> +    void *prp1;
> +    u64 prpl[NVME_MAX_PRPL_ENTRIES];
> +};
> +
>  struct nvme_namespace {
>      struct drive_s drive;
>      struct nvme_ctrl *ctrl;
> @@ -123,10 +130,7 @@ struct nvme_namespace {
>      /* Page aligned buffer of size NVME_PAGE_SIZE. */
>      char *dma_buffer;
>
> -    /* Page List */
> -    u32 prpl_len;
> -    void *prp1;
> -    u64 prpl[NVME_MAX_PRPL_ENTRIES];
> +    struct nvme_prp_info *prp;
>  };
>
>  /* Data structures for NVMe admin identify commands */
> diff --git a/src/hw/nvme.c b/src/hw/nvme.c
> index f035fa2..963c31e 100644
> --- a/src/hw/nvme.c
> +++ b/src/hw/nvme.c
> @@ -267,6 +267,14 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, u32 ns_idx, u8 mdts)
>      ns->ns_id = ns_id;
>      ns->lba_count = id->nsze;
>
> +    ns->prp = malloc_high(sizeof(*ns->prp));
> +    if (!ns->prp) {
> +        warn_noalloc();
> +        free(ns);
> +        goto free_buffer;
> +    }
> +    memset(ns->prp, 0, sizeof(*ns->prp));
> +
>      struct nvme_lba_format *fmt = &id->lbaf[current_lba_format];
>
>      ns->block_size    = 1U << fmt->lbads;
> @@ -431,10 +439,10 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
>
>      if ((ns->block_size * count) > (NVME_PAGE_SIZE * 2)) {
>          /* We need to describe more than 2 pages, rely on PRP List */
> -        prp2 = ns->prpl;
> +        prp2 = ns->prp->prpl;
>      } else if ((ns->block_size * count) > NVME_PAGE_SIZE) {
>          /* Directly embed the 2nd page if we only need 2 pages */
> -        prp2 = (void *)(long)ns->prpl[0];
> +        prp2 = (void *)(long)ns->prp->prpl[0];
>      } else {
>          /* One page is enough, don't expose anything else */
>          prp2 = NULL;
> @@ -465,15 +473,15 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count,
>
>  static void nvme_reset_prpl(struct nvme_namespace *ns)
>  {
> -    ns->prpl_len = 0;
> +    ns->prp->prpl_len = 0;
>  }
>
>  static int nvme_add_prpl(struct nvme_namespace *ns, u64 base)
>  {
> -    if (ns->prpl_len >= NVME_MAX_PRPL_ENTRIES)
> +    if (ns->prp->prpl_len >= NVME_MAX_PRPL_ENTRIES)
>          return -1;
>
> -    ns->prpl[ns->prpl_len++] = base;
> +    ns->prp->prpl[ns->prp->prpl_len++] = base;
>
>      return 0;
>  }
> @@ -492,7 +500,7 @@ static int nvme_build_prpl(struct nvme_namespace *ns, void *op_buf, u16 count)
>      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;
> +        ns->prp->prp1 = op_buf;
>          return count;
>      }
>
> @@ -507,7 +515,7 @@ static int nvme_build_prpl(struct nvme_namespace *ns, void *op_buf, u16 count)
>      for (; size > 0; base += NVME_PAGE_SIZE, size -= NVME_PAGE_SIZE) {
>          if (first_page) {
>              /* First page is special */
> -            ns->prp1 = (void*)base;
> +            ns->prp->prp1 = (void*)base;
>              first_page = 0;
>              continue;
>          }
> @@ -726,7 +734,7 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write)
>
>          blocks = nvme_build_prpl(ns, op_buf, blocks_remaining);
>          if (blocks) {
> -            res = nvme_io_readwrite(ns, op->lba + i, ns->prp1, blocks, write);
> +            res = nvme_io_readwrite(ns, op->lba + i, ns->prp->prp1, blocks, write);
>              dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
>                                                                        : "read",
>                      op->lba, blocks, res);
> --
> 2.28.0.394.ge197136389
>
>
>
>
> 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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.