| Message ID | 20200729220638.344477-17-its@irrelevant.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/block/nvme: dma handling and address mapping cleanup | expand |
On 20-07-30 00:06:38, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > Since nvme_map_prp always operate on the request-scoped qsg/iovs, just > pass a single pointer to the NvmeRequest instead of two for each of the > qsg and iov. > > Suggested-by: Minwoo Im <minwoo.im.dev@gmail.com> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > --- > hw/block/nvme.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 55b1a68ced8c..aea8a8b6946c 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -284,8 +284,8 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, QEMUSGList *qsg, QEMUIOVector *iov, > return NVME_SUCCESS; > } > > -static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1, > - uint64_t prp2, uint32_t len, NvmeCtrl *n) > +static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t prp1, uint64_t prp2, > + uint32_t len, NvmeRequest *req) > { > hwaddr trans_len = n->page_size - (prp1 % n->page_size); > trans_len = MIN(len, trans_len); > @@ -293,6 +293,9 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1, > uint16_t status; > bool prp_list_in_cmb = false; > > + QEMUSGList *qsg = &req->qsg; > + QEMUIOVector *iov = &req->iov; > + > trace_pci_nvme_map_prp(trans_len, len, prp1, prp2, num_prps); > > if (unlikely(!prp1)) { > @@ -386,7 +389,7 @@ static uint16_t nvme_dma_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len, > { > uint16_t status = NVME_SUCCESS; > > - status = nvme_map_prp(&req->qsg, &req->iov, prp1, prp2, len, n); > + status = nvme_map_prp(n, prp1, prp2, len, req); > if (status) { > return status; > } > @@ -431,7 +434,7 @@ static uint16_t nvme_map_dptr(NvmeCtrl *n, size_t len, NvmeRequest *req) > uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1); > uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2); > > - return nvme_map_prp(&req->qsg, &req->iov, prp1, prp2, len, n); > + return nvme_map_prp(n, prp1, prp2, len, req); > } > > static void nvme_post_cqes(void *opaque) This looks better to have qsg and iov in the NvmeRequest. Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
On Thu, 2020-07-30 at 00:06 +0200, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > Since nvme_map_prp always operate on the request-scoped qsg/iovs, just > pass a single pointer to the NvmeRequest instead of two for each of the > qsg and iov. > > Suggested-by: Minwoo Im <minwoo.im.dev@gmail.com> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > --- > hw/block/nvme.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 55b1a68ced8c..aea8a8b6946c 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -284,8 +284,8 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, QEMUSGList *qsg, QEMUIOVector *iov, > return NVME_SUCCESS; > } > > -static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1, > - uint64_t prp2, uint32_t len, NvmeCtrl *n) > +static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t prp1, uint64_t prp2, > + uint32_t len, NvmeRequest *req) > { > hwaddr trans_len = n->page_size - (prp1 % n->page_size); > trans_len = MIN(len, trans_len); > @@ -293,6 +293,9 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1, > uint16_t status; > bool prp_list_in_cmb = false; > > + QEMUSGList *qsg = &req->qsg; > + QEMUIOVector *iov = &req->iov; > + > trace_pci_nvme_map_prp(trans_len, len, prp1, prp2, num_prps); > > if (unlikely(!prp1)) { > @@ -386,7 +389,7 @@ static uint16_t nvme_dma_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len, > { > uint16_t status = NVME_SUCCESS; > > - status = nvme_map_prp(&req->qsg, &req->iov, prp1, prp2, len, n); > + status = nvme_map_prp(n, prp1, prp2, len, req); > if (status) { > return status; > } > @@ -431,7 +434,7 @@ static uint16_t nvme_map_dptr(NvmeCtrl *n, size_t len, NvmeRequest *req) > uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1); > uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2); > > - return nvme_map_prp(&req->qsg, &req->iov, prp1, prp2, len, n); > + return nvme_map_prp(n, prp1, prp2, len, req); > } > > static void nvme_post_cqes(void *opaque) Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 55b1a68ced8c..aea8a8b6946c 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -284,8 +284,8 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, QEMUSGList *qsg, QEMUIOVector *iov, return NVME_SUCCESS; } -static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1, - uint64_t prp2, uint32_t len, NvmeCtrl *n) +static uint16_t nvme_map_prp(NvmeCtrl *n, uint64_t prp1, uint64_t prp2, + uint32_t len, NvmeRequest *req) { hwaddr trans_len = n->page_size - (prp1 % n->page_size); trans_len = MIN(len, trans_len); @@ -293,6 +293,9 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1, uint16_t status; bool prp_list_in_cmb = false; + QEMUSGList *qsg = &req->qsg; + QEMUIOVector *iov = &req->iov; + trace_pci_nvme_map_prp(trans_len, len, prp1, prp2, num_prps); if (unlikely(!prp1)) { @@ -386,7 +389,7 @@ static uint16_t nvme_dma_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len, { uint16_t status = NVME_SUCCESS; - status = nvme_map_prp(&req->qsg, &req->iov, prp1, prp2, len, n); + status = nvme_map_prp(n, prp1, prp2, len, req); if (status) { return status; } @@ -431,7 +434,7 @@ static uint16_t nvme_map_dptr(NvmeCtrl *n, size_t len, NvmeRequest *req) uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1); uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2); - return nvme_map_prp(&req->qsg, &req->iov, prp1, prp2, len, n); + return nvme_map_prp(n, prp1, prp2, len, req); } static void nvme_post_cqes(void *opaque)