nvme: add low-level APIs to map prp/sgl without @rq #28
Conversation
src/nvme/core.c
Outdated
| cba = ctrl->cmb.iova; | ||
| ctrl->flags |= NVME_CTRL_F_CMB_IOVA; | ||
| } else | ||
| cba = ctrl->cmb.hwaddr; |
There was a problem hiding this comment.
I don't see any reason not to just always map the CMB into the iommu. By using the IOVA address space for the CBA, we ensure that
- it will work if we want to do p2p
- the cba is guaranteed not to overlap with addresses allocated in the iova space. I am not 100% sure if we are guaranteed that the hwaddr that we use for cba cannot end up being allocated by the iova allocator?
src/nvme/util.c
Outdated
| } | ||
|
|
||
| int nvme_map_prp(struct nvme_ctrl *ctrl, leint64_t *prplist, uint64_t prplist_iova, | ||
| union nvme_cmd *cmd, uint64_t iova, size_t len) |
There was a problem hiding this comment.
If always map the CMB into the iova address space, then we can get rid of prplist_iova and translate prplist here directly. Then it would always work, regardless of if or not the prplist ist located in the cmb or in host memory, no?
There was a problem hiding this comment.
Ah okay, then even if application does not use iova mapped address for CMB, we can use IOMMU routine for simplicity. But, just like the following commits, vaddr translation should consider and choose whether to use iova or hwaddr based on the CMB configurations.
Agreed on using IOMMU IOVA for the simplicity for both cases.
| return -1; | ||
|
|
||
| if (nvme_translate_vaddr(ctrl, prplist, &prplist_iova)) | ||
| return -1; |
There was a problem hiding this comment.
following on what i mentioned above, here you've added a helper that first checks the iommu, then the cmb. But, we can simplify and just always have it in the iommu.
minwooim
force-pushed
the
for-prpsgl-map
branch
from
037eb15 to
4d60a8c
Compare
birkelund
force-pushed
the
for-prpsgl-map
branch
from
4d60a8c to
7914ce5
Compare
birkelund
force-pushed
the
for-prpsgl-map
branch
from
7914ce5 to
bce0f6c
Compare
Add low-level helpers to map PRP and SGL data and data list which contains the actual prp and sgl entries within a memory page. The existing nvme_rq mapping helpers always use the preallocated struct nvme_rq page.vaddr member. Some applications might want to locate the prplist or sgl segments in another location such as the CMB (Controller Memory Buffer) rather than the host memory. In that case, the application could update the struct nvme_rq page member, but it's not a good idea since it is a library-specific data structure and has its own policy. To give more flexible options to upper layer applications, this patch adds a low-level API which are more generic using parameters for the prplist or segment list. The newly added public helpers are: * nvme_map_prp * nvme_mapv_prp * nvme_mapv_sgl This does not have functional change on the existing API. Signed-off-by: Minwoo Im <[email protected]> [k.jensen: update commit message and fixed tests] Signed-off-by: Klaus Jensen <[email protected]>
birkelund
force-pushed
the
for-prpsgl-map
branch
from
bce0f6c to
c744bb1
Compare
Add low-level helpers to map PRP and SGL data and data list which contains the actual prp and sgl entries within a memory page. nvme_rq_map* helpers use datalist as @rq->page.vaddr which is allocated at the initial time. Some applications might want to locate the datalist memory page in another location such as CMB (Controller Memory Buffer) rather than the host memory. In that case, application should update @rq->page, but it's not a good idea since it's a too much libvfn-specific data structure and it has its own policy. To give more flexible options to upper layer application, this patch adds low-level APIs which are much more generic than nvme_rq_map* helpers by receving a @prplist and @seg for PRP and SGL list from the caller.
The newly added public helpers are:
This commit does not have functional changes of the existing APIs.