[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 02/16] hw/block/nvme: add mapping helpers
From: |
Minwoo Im |
Subject: |
Re: [PATCH 02/16] hw/block/nvme: add mapping helpers |
Date: |
Thu, 30 Jul 2020 00:19:28 +0900 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
Hi Klaus,
On 20-07-20 13:37:34, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Add nvme_map_addr, nvme_map_addr_cmb and nvme_addr_to_cmb helpers and
> use them in nvme_map_prp.
>
> This fixes a bug where in the case of a CMB transfer, the device would
> map to the buffer with a wrong length.
>
> Fixes: b2b2b67a00574 ("nvme: Add support for Read Data and Write Data in
> CMBs.")
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t
> prp1,
> uint64_t prp2, uint32_t len, NvmeCtrl *n)
> {
> hwaddr trans_len = n->page_size - (prp1 % n->page_size);
> trans_len = MIN(len, trans_len);
> int num_prps = (len >> n->page_bits) + 1;
> + uint16_t status;
>
> if (unlikely(!prp1)) {
> trace_pci_nvme_err_invalid_prp();
> return NVME_INVALID_FIELD | NVME_DNR;
> - } else if (n->bar.cmbsz && prp1 >= n->ctrl_mem.addr &&
> - prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
> - qsg->nsg = 0;
> + }
> +
> + if (nvme_addr_is_cmb(n, prp1)) {
> qemu_iovec_init(iov, num_prps);
> - qemu_iovec_add(iov, (void *)&n->cmbuf[prp1 - n->ctrl_mem.addr],
> trans_len);
> } else {
> pci_dma_sglist_init(qsg, &n->parent_obj, num_prps);
> - qemu_sglist_add(qsg, prp1, trans_len);
> }
> +
> + status = nvme_map_addr(n, qsg, iov, prp1, trans_len);
> + if (status) {
> + goto unmap;
> + }
> +
> len -= trans_len;
> if (len) {
> if (unlikely(!prp2)) {
> trace_pci_nvme_err_invalid_prp2_missing();
> + status = NVME_INVALID_FIELD | NVME_DNR;
> goto unmap;
> }
> if (len > n->page_size) {
> @@ -242,6 +309,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg,
> QEMUIOVector *iov, uint64_t prp1,
> if (i == n->max_prp_ents - 1 && len > n->page_size) {
> if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
> trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
> + status = NVME_INVALID_FIELD | NVME_DNR;
> goto unmap;
> }
>
> @@ -255,14 +323,14 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg,
> QEMUIOVector *iov, uint64_t prp1,
>
> if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
> trace_pci_nvme_err_invalid_prplist_ent(prp_ent);
> + status = NVME_INVALID_FIELD | NVME_DNR;
> goto unmap;
> }
>
> trans_len = MIN(len, n->page_size);
> - if (qsg->nsg){
> - qemu_sglist_add(qsg, prp_ent, trans_len);
> - } else {
> - qemu_iovec_add(iov, (void *)&n->cmbuf[prp_ent -
> n->ctrl_mem.addr], trans_len);
> + status = nvme_map_addr(n, qsg, iov, prp_ent, trans_len);
> + if (status) {
> + goto unmap;
> }
> len -= trans_len;
> i++;
> @@ -270,20 +338,27 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg,
> QEMUIOVector *iov, uint64_t prp1,
> } else {
> if (unlikely(prp2 & (n->page_size - 1))) {
> trace_pci_nvme_err_invalid_prp2_align(prp2);
> + status = NVME_INVALID_FIELD | NVME_DNR;
> goto unmap;
> }
> - if (qsg->nsg) {
> - qemu_sglist_add(qsg, prp2, len);
> - } else {
> - qemu_iovec_add(iov, (void *)&n->cmbuf[prp2 -
> n->ctrl_mem.addr], trans_len);
> + status = nvme_map_addr(n, qsg, iov, prp2, len);
> + if (status) {
> + goto unmap;
I like these parts which is much better to read the codes.
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
Thanks,
[PATCH 04/16] hw/block/nvme: remove redundant has_sg member, Klaus Jensen, 2020/07/20
[PATCH 06/16] hw/block/nvme: pass request along for tracing, Klaus Jensen, 2020/07/20