From 638352edf54d9300a5cc62d34b896113316b3cb1 Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Thu, 1 Feb 2024 14:58:09 +0100 Subject: [PATCH] nvme/rq: fix prp mapping Mapping PRPs are apparently not that easy. Prior to this patch, the logic was a little too clever. Dumb it down a bit and fix a bunch of edge cases. And add a bunch of tests for these cases. Reported-by: Karl Bonde Torp Signed-off-by: Klaus Jensen --- src/nvme/rq.c | 20 +++++++------- src/nvme/rq_test.c | 69 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/nvme/rq.c b/src/nvme/rq.c index dd88c6bb..9dbafac8 100644 --- a/src/nvme/rq.c +++ b/src/nvme/rq.c @@ -59,20 +59,20 @@ static void __attribute__((constructor)) init_max_prps(void) static inline int __map_first(leint64_t *prp1, leint64_t *prplist, uint64_t iova, size_t len) { /* number of prps required to map the buffer */ - int prpcount = (int)len >> __VFN_PAGESHIFT; + int prpcount = 1; *prp1 = cpu_to_le64(iova); - /* - * If prpcount is at least one and the iova is not aligned to the page - * size, we need one more prp than what we calculated above. - * Additionally, we align the iova down to a page size boundary, - * simplifying the following loop. - */ - if (prpcount && !ALIGNED(iova, __VFN_PAGESIZE)) { + /* account for what is covered with the first prp */ + len -= min_t(size_t, len, __VFN_PAGESIZE - (iova & (__VFN_PAGESIZE - 1))); + + /* any residual just adds more prps */ + if (len) + prpcount += (int)ALIGN_UP(len, __VFN_PAGESIZE) >> __VFN_PAGESHIFT; + + if (prpcount > 1 && !ALIGNED(iova, __VFN_PAGESIZE)) + /* align down to simplify loop below */ iova = ALIGN_DOWN(iova, __VFN_PAGESIZE); - prpcount++; - } if (prpcount > __rq_max_prps) { errno = EINVAL; diff --git a/src/nvme/rq_test.c b/src/nvme/rq_test.c index 21589712..ef94836a 100644 --- a/src/nvme/rq_test.c +++ b/src/nvme/rq_test.c @@ -27,7 +27,7 @@ int main(void) leint64_t *prplist; struct iovec iov[8]; - plan_tests(62); + plan_tests(89); assert(pgmap((void **)&prplist, __VFN_PAGESIZE) > 0); @@ -48,6 +48,13 @@ int main(void) ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000); ok1(le64_to_cpu(cmd.dptr.prp2) == 0x0); + /* test 4k + 8 aligned */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1000000, 0x1008); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000); + /* test 8k aligned */ memset((void *)prplist, 0x0, __VFN_PAGESIZE); nvme_rq_map_prp(&rq, &cmd, 0x1000000, 0x2000); @@ -55,6 +62,15 @@ int main(void) ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000); ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000); + /* test 8k + 16 aligned */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1000000, 0x2010); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000); + ok1(le64_to_cpu(prplist[0]) == 0x1001000); + ok1(le64_to_cpu(prplist[1]) == 0x1002000); + /* test 12k aligned */ memset((void *)prplist, 0x0, __VFN_PAGESIZE); nvme_rq_map_prp(&rq, &cmd, 0x1000000, 0x3000); @@ -64,6 +80,15 @@ int main(void) ok1(le64_to_cpu(prplist[0]) == 0x1001000); ok1(le64_to_cpu(prplist[1]) == 0x1002000); + /* test 12k + 24 aligned */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1000000, 0x3018); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000); + ok1(le64_to_cpu(prplist[0]) == 0x1001000); + ok1(le64_to_cpu(prplist[1]) == 0x1002000); + /* test 512b unaligned */ memset((void *)prplist, 0x0, __VFN_PAGESIZE); nvme_rq_map_prp(&rq, &cmd, 0x1000004, 0x200); @@ -71,6 +96,13 @@ int main(void) ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004); ok1(le64_to_cpu(cmd.dptr.prp2) == 0x0); + /* test 512 unaligned (nasty) */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1001000 - 4, 0x200); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1001000 - 4); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000); + /* test 4k unaligned */ memset((void *)prplist, 0x0, __VFN_PAGESIZE); nvme_rq_map_prp(&rq, &cmd, 0x1000004, 0x1000); @@ -78,6 +110,22 @@ int main(void) ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004); ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000); + /* test 4k + 8 unaligned */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1000004, 0x1008); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000); + + /* test 4k + 8 unaligned (nasty) */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1001000 - 4, 0x1008); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1001000 - 4); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000); + ok1(le64_to_cpu(prplist[0]) == 0x1001000); + ok1(le64_to_cpu(prplist[1]) == 0x1002000); + /* test 4k - 4 unaligned */ memset((void *)prplist, 0x0, __VFN_PAGESIZE); nvme_rq_map_prp(&rq, &cmd, 0x1000004, 0x1000 - 4); @@ -94,6 +142,15 @@ int main(void) ok1(le64_to_cpu(prplist[0]) == 0x1001000); ok1(le64_to_cpu(prplist[1]) == 0x1002000); + /* test 8k + 16 unaligned */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1000004, 0x2010); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000); + ok1(le64_to_cpu(prplist[0]) == 0x1001000); + ok1(le64_to_cpu(prplist[1]) == 0x1002000); + /* test 8k - 4 unaligned */ memset((void *)prplist, 0x0, __VFN_PAGESIZE); nvme_rq_map_prp(&rq, &cmd, 0x1000004, 0x2000 - 4); @@ -111,6 +168,16 @@ int main(void) ok1(le64_to_cpu(prplist[1]) == 0x1002000); ok1(le64_to_cpu(prplist[2]) == 0x1003000); + /* test 12k + 24 unaligned */ + memset((void *)prplist, 0x0, __VFN_PAGESIZE); + nvme_rq_map_prp(&rq, &cmd, 0x1000004, 0x3018); + + ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004); + ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000); + ok1(le64_to_cpu(prplist[0]) == 0x1001000); + ok1(le64_to_cpu(prplist[1]) == 0x1002000); + ok1(le64_to_cpu(prplist[2]) == 0x1003000); + /* test 512b aligned 1-iovec */ memset((void *)prplist, 0x0, __VFN_PAGESIZE); iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x200};