Skip to content

Commit

Permalink
nvme/rq: fix prp mapv
Browse files Browse the repository at this point in the history 
Fix two horrible bugs in nvme_rq_mapv_prp(). These would have been
caught by the tests. If the tests actually worked.

Fix the damn tests as well.

Reported-by: Minwoo Im <[email protected]>
Signed-off-by: Klaus Jensen <[email protected]>
  • Loading branch information
@birkelund
birkelund committed Nov 18, 2024
1 parent a0070fa commit a261331
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 36 deletions.
18 changes: 13 additions & 5 deletions src/nvme/rq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ static inline int __map_prp_first(leint64_t *prp1, leint64_t *prplist, uint64_t

if (prpcount > max_prps) {
errno = EINVAL;
return 0;
return -1;
}

/*
Expand Down Expand Up @@ -125,7 +125,7 @@ int nvme_rq_map_prp(struct nvme_ctrl *ctrl, struct nvme_rq *rq, union nvme_cmd *
int pageshift = __mps_to_pageshift(ctrl->config.mps);

prpcount = __map_prp_first(&cmd->dptr.prp1, prplist, iova, len, pageshift);
if (!prpcount) {
if (prpcount < 0) {
errno = EINVAL;
return -1;
}
Expand All @@ -145,7 +145,7 @@ int nvme_rq_mapv_prp(struct nvme_ctrl *ctrl, struct nvme_rq *rq, union nvme_cmd
int pageshift = __mps_to_pageshift(ctrl->config.mps);
size_t pagesize = 1 << pageshift;
int max_prps = 1 << (pageshift - 3);
int prpcount;
int ret, prpcount;
uint64_t iova;

if (!iommu_translate_vaddr(ctx, iov->iov_base, &iova)) {
Expand All @@ -155,6 +155,8 @@ int nvme_rq_mapv_prp(struct nvme_ctrl *ctrl, struct nvme_rq *rq, union nvme_cmd

/* map the first segment */
prpcount = __map_prp_first(&cmd->dptr.prp1, prplist, iova, len, pageshift);
if (prpcount < 0)
goto invalid;

/*
* At this point, one of three conditions must hold:
Expand Down Expand Up @@ -188,12 +190,18 @@ int nvme_rq_mapv_prp(struct nvme_ctrl *ctrl, struct nvme_rq *rq, union nvme_cmd
goto invalid;
}

prpcount += __map_prp_append(&prplist[prpcount - 1], iova, len, max_prps - prpcount,
pageshift);
ret = __map_prp_append(&prplist[prpcount - 1], iova, len, max_prps - prpcount,
pageshift);
if (ret < 0)
goto invalid;

prpcount += ret;
}

__set_prp2(&cmd->dptr.prp2, cpu_to_le64(rq->page.iova), prplist[0], prpcount);

return 0;

invalid:
errno = EINVAL;
return -1;
Expand Down
62 changes: 31 additions & 31 deletions src/nvme/rq_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ int main(void)
struct nvme_sgld *sglds;
struct iovec iov[8];

plan_tests(96);
plan_tests(126);

assert(pgmap((void **)&rq.page.vaddr, __VFN_PAGESIZE) > 0);

Expand All @@ -62,35 +62,35 @@ int main(void)

/* test 512b aligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x200);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x200) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x0);

/* test 4k aligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x1000);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x1000) == 0);

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(&ctrl, &rq, &cmd, 0x1000000, 0x1008);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x1008) == 0);

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(&ctrl, &rq, &cmd, 0x1000000, 0x2000);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x2000) == 0);

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(&ctrl, &rq, &cmd, 0x1000000, 0x2010);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x2010) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -99,7 +99,7 @@ int main(void)

/* test 12k aligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x3000);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x3000) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -108,7 +108,7 @@ int main(void)

/* test 12k + 24 aligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x3018);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000000, 0x3018) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -117,35 +117,35 @@ int main(void)

/* test 512b unaligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x200);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x200) == 0);

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(&ctrl, &rq, &cmd, 0x1001000 - 4, 0x200);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1001000 - 4, 0x200) == 0);

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(&ctrl, &rq, &cmd, 0x1000004, 0x1000);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x1000) == 0);

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(&ctrl, &rq, &cmd, 0x1000004, 0x1008);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x1008) == 0);

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(&ctrl, &rq, &cmd, 0x1001000 - 4, 0x1008);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1001000 - 4, 0x1008) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1001000 - 4);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -154,14 +154,14 @@ int main(void)

/* test 4k - 4 unaligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x1000 - 4);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x1000 - 4) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x0);

/* test 8k unaligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x2000);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x2000) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -170,7 +170,7 @@ int main(void)

/* test 8k + 16 unaligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x2010);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x2010) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -179,14 +179,14 @@ int main(void)

/* test 8k - 4 unaligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x2000 - 4);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x2000 - 4) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000);

/* test 12k unaligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x3000);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x3000) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -196,7 +196,7 @@ int main(void)

/* test 12k + 24 unaligned */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x3018);
ok1(nvme_rq_map_prp(&ctrl, &rq, &cmd, 0x1000004, 0x3018) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -207,31 +207,31 @@ int main(void)
/* test 512b aligned 1-iovec */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x200};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x0);

/* test 4k aligned 1-iovec */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x1000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x0);

/* test 8k aligned 1-iovec */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x2000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000);

/* test 12k aligned 1-iovec */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x3000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -242,7 +242,7 @@ int main(void)
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x1000};
iov[1] = (struct iovec) {.iov_base = (void *)0x1001000, .iov_len = 0x1000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 2);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 2) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000);
Expand All @@ -252,7 +252,7 @@ int main(void)
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x1000};
iov[1] = (struct iovec) {.iov_base = (void *)0x1001000, .iov_len = 0x1000};
iov[2] = (struct iovec) {.iov_base = (void *)0x1002000, .iov_len = 0x1000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 3);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 3) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -263,7 +263,7 @@ int main(void)
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x1000};
iov[1] = (struct iovec) {.iov_base = (void *)0x1001000, .iov_len = 0x2000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 3);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 3) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -273,15 +273,15 @@ int main(void)
/* test 512b unaligned 1-iovec */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000004, .iov_len = 0x200};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x0);

/* test 4k unaligned 1-iovec */
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000004, .iov_len = 0x1000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 1) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000);
Expand All @@ -290,7 +290,7 @@ int main(void)
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000004, .iov_len = 0x1000 - 4};
iov[1] = (struct iovec) {.iov_base = (void *)0x1001000, .iov_len = 0x1000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 2);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 2) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000);
Expand All @@ -300,7 +300,7 @@ int main(void)
iov[0] = (struct iovec) {.iov_base = (void *)0x1000004, .iov_len = 0x1000 - 4};
iov[1] = (struct iovec) {.iov_base = (void *)0x1001000, .iov_len = 0x1000};
iov[2] = (struct iovec) {.iov_base = (void *)0x1002000, .iov_len = 0x1000};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 3);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 3) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000004);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x8000000);
Expand All @@ -311,7 +311,7 @@ int main(void)
memset((void *)prplist, 0x0, __VFN_PAGESIZE);
iov[0] = (struct iovec) {.iov_base = (void *)0x1000000, .iov_len = 0x1000};
iov[1] = (struct iovec) {.iov_base = (void *)0x1001000, .iov_len = 0x1000 - 4};
nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 2);
ok1(nvme_rq_mapv_prp(&ctrl, &rq, &cmd, iov, 2) == 0);

ok1(le64_to_cpu(cmd.dptr.prp1) == 0x1000000);
ok1(le64_to_cpu(cmd.dptr.prp2) == 0x1001000);
Expand Down

0 comments on commit a261331

Please sign in to comment.