[PATCH] nvme: fix dma direction macro in dma_unmap_page

[PATCH] nvme: fix dma direction macro in dma_unmap_page

[zhichuang]

From: Zhichuang Sun <zhichuang@google.com>

This patch fix a DMA related bug in the nvme driver,
which caused a surge of DMA errors. The DMA debug
subsystem complains:

"nvme device driver frees DMA memory with different direction
[mapped with DMA_FROM_DEVICE] [unmapped with DMA_BIDIRECTIONAL]"

It's caused by the wrong macro used in "dma_unmap_page()" to
specify the dma direction. It should use "rq_dma_dir(req)"
instead of "rq_data_dir(req)".

For map:
drivers/nvme/host/pci.c
nvme_queue_rq
  --> nvme_map_data
    --> nvme_map_metadata
      --> dma_map_bvec(dev->dev, rq_integrity_vec(req), rq_dma_dir(req), 0);
         --> dma_map_page_attrs(dev, (bv)->bv_page, (bv)->bv_offset, \
				 (bv)->bv_len, (dir), (attrs))

For unmap:
drivers/nvme/host/pci.c
nvme_pci_complete_rq
  --> dma_unmap_page(dev->dev, iod->meta_dma, \
		rq_integrity_vec(req)->bv_len, rq_data_dir(req));
    --> dma_unmap_page_attrs(dev, addr, len, dir, attrs)

For the same req, during map, the mapping direction is passed with
rq_dma_dir(req); during unmap, the mapping direction is passed with
rq_data_dir(req). rq_data_dir and rq_dma_dir interprate direction
differently, see definition below:

include/linux/blkdev.h
 #define rq_data_dir(rq)         (op_is_write(req_op(rq)) ? WRITE : READ)

 #define rq_dma_dir(rq) \
         (op_is_write(req_op(rq)) ? DMA_TO_DEVICE : DMA_FROM_DEVICE)

include/linux/kernel.h
 /* generic data direction definitions */
 #define READ                    0
 #define WRITE                   1

include/linux/dma-direction.h
 enum dma_data_direction {
         DMA_BIDIRECTIONAL = 0,
         DMA_TO_DEVICE = 1,
         DMA_FROM_DEVICE = 2,
         DMA_NONE = 3,
 };

As a result, for a read DMA req, during map, it is recorded as
DMA_FROM_DEVICE(2) with rq_dma_dir(rq), and during unmap, it is
unmapped as READ(0) with rq_data_dir, which will be interprated
as DMA_DMA_BIDIRECTIONAL(0), thus causing a mismatch during DMA
unmap, triggering false alarms of DMA bugs.

Signed-off-by: Zhichuang Sun <zhichuang@google.com>
---
 drivers/nvme/host/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 72725729cb6c..7778f384293c 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -967,7 +967,7 @@ static __always_inline void nvme_pci_unmap_rq(struct request *req)
 	        struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
 
 		dma_unmap_page(dev->dev, iod->meta_dma,
-			       rq_integrity_vec(req)->bv_len, rq_data_dir(req));
+			       rq_integrity_vec(req)->bv_len, rq_dma_dir(req));
 	}
 
 	if (blk_rq_nr_phys_segments(req))
-- 
2.41.0.255.g8b1d071c50-goog

Re: [PATCH] nvme: fix dma direction macro in dma_unmap_page

[Jens Axboe]

On 7/14/23 1:14?PM, zhichuang@google.com wrote:
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 72725729cb6c..7778f384293c 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -967,7 +967,7 @@ static __always_inline void nvme_pci_unmap_rq(struct request *req)
>  	        struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
>  
>  		dma_unmap_page(dev->dev, iod->meta_dma,
> -			       rq_integrity_vec(req)->bv_len, rq_data_dir(req));
> +			       rq_integrity_vec(req)->bv_len, rq_dma_dir(req));
>  	}
>  
>  	if (blk_rq_nr_phys_segments(req))

This already got fixed and applied by Keith, pulled by me, and heading
upstream today:

https://git.kernel.dk/cgit/linux/commit/?h=block-6.5&id=b8f6446b6853768cb99e7c201bddce69ca60c15e

-- 
Jens Axboe