linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Properly take MMIO path
@ 2025-10-13 15:34 Leon Romanovsky
  2025-10-13 15:34 ` [PATCH 1/4] blk-mq-dma: migrate to dma_map_phys instead of map_page Leon Romanovsky
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-13 15:34 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Jason Gunthorpe, Keith Busch, linux-block, linux-kernel,
	linux-nvme, Sagi Grimberg, Marek Szyprowski

This is NVMe and block specific patches from my DMA series [1] which
improved DMA physical API to properly support MMIO memory.

Thanks

[1] https://lore.kernel.org/all/cover.1757423202.git.leonro@nvidia.com/

Leon Romanovsky (4):
  blk-mq-dma: migrate to dma_map_phys instead of map_page
  blk-mq-dma: unify DMA unmap routine
  block-dma: properly take MMIO path
  nvme-pci: unmap MMIO pages with appropriate interface

 block/blk-mq-dma.c            | 52 +++++++++++++++++++++++++++++++++--
 drivers/nvme/host/pci.c       | 18 ++++++++----
 include/linux/bio-integrity.h |  1 +
 include/linux/blk-integrity.h |  3 +-
 include/linux/blk-mq-dma.h    | 35 ++---------------------
 include/linux/blk_types.h     |  2 ++
 6 files changed, 68 insertions(+), 43 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/4] blk-mq-dma: migrate to dma_map_phys instead of map_page
  2025-10-13 15:34 [PATCH 0/4] Properly take MMIO path Leon Romanovsky
@ 2025-10-13 15:34 ` Leon Romanovsky
  2025-10-15  4:16   ` Christoph Hellwig
  2025-10-13 15:34 ` [PATCH 2/4] blk-mq-dma: unify DMA unmap routine Leon Romanovsky
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-13 15:34 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Leon Romanovsky, Jason Gunthorpe, Keith Busch, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

From: Leon Romanovsky <leonro@nvidia.com>

After introduction of dma_map_phys(), there is no need to convert
from physical address to struct page in order to map page. So let's
use it directly.

Reviewed-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 block/blk-mq-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index 449950029872..4ba7b0323da4 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -93,8 +93,8 @@ static bool blk_dma_map_bus(struct blk_dma_iter *iter, struct phys_vec *vec)
 static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
 		struct blk_dma_iter *iter, struct phys_vec *vec)
 {
-	iter->addr = dma_map_page(dma_dev, phys_to_page(vec->paddr),
-			offset_in_page(vec->paddr), vec->len, rq_dma_dir(req));
+	iter->addr = dma_map_phys(dma_dev, vec->paddr, vec->len,
+			rq_dma_dir(req), 0);
 	if (dma_mapping_error(dma_dev, iter->addr)) {
 		iter->status = BLK_STS_RESOURCE;
 		return false;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/4] blk-mq-dma: unify DMA unmap routine
  2025-10-13 15:34 [PATCH 0/4] Properly take MMIO path Leon Romanovsky
  2025-10-13 15:34 ` [PATCH 1/4] blk-mq-dma: migrate to dma_map_phys instead of map_page Leon Romanovsky
@ 2025-10-13 15:34 ` Leon Romanovsky
  2025-10-13 18:53   ` Keith Busch
  2025-10-13 15:34 ` [PATCH 3/4] block-dma: properly take MMIO path Leon Romanovsky
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-13 15:34 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Leon Romanovsky, Jason Gunthorpe, Keith Busch, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

From: Leon Romanovsky <leonro@nvidia.com>

Combine regular and meta-intgerity DMA unmapping flow to one
blk_rq_dma_unmap(). This allows us to handle addition of new
DMA_ATTR_MMIO flow without adding extra function parameters.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 block/blk-mq-dma.c            | 29 +++++++++++++++++++++++++++++
 include/linux/blk-integrity.h |  3 +--
 include/linux/blk-mq-dma.h    | 35 ++---------------------------------
 3 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index 4ba7b0323da4..0648bcb705ff 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -260,6 +260,35 @@ bool blk_rq_dma_map_iter_next(struct request *req, struct device *dma_dev,
 }
 EXPORT_SYMBOL_GPL(blk_rq_dma_map_iter_next);
 
+/**
+ * blk_rq_dma_unmap - try to DMA unmap a request
+ * @req:	request to unmap
+ * @dma_dev:	device to unmap from
+ * @state:	DMA IOVA state
+ * @mapped_len: number of bytes to unmap
+ *
+ * Returns %false if the callers need to manually unmap every DMA segment
+ * mapped using @iter or %true if no work is left to be done.
+ */
+bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
+		struct dma_iova_state *state, size_t mapped_len)
+{
+	struct bio_integrity_payload *bip = bio_integrity(req->bio);
+
+	if ((!bip && req->cmd_flags & REQ_P2PDMA) ||
+	    bio_integrity_flagged(req->bio, BIP_P2P_DMA))
+		return true;
+
+	if (dma_use_iova(state)) {
+		dma_iova_destroy(dma_dev, state, mapped_len, rq_dma_dir(req),
+				 0);
+		return true;
+	}
+
+	return !dma_need_unmap(dma_dev);
+}
+EXPORT_SYMBOL(blk_rq_dma_unmap);
+
 static inline struct scatterlist *
 blk_next_sg(struct scatterlist **sg, struct scatterlist *sglist)
 {
diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
index b659373788f6..4a0e65f00bd6 100644
--- a/include/linux/blk-integrity.h
+++ b/include/linux/blk-integrity.h
@@ -32,8 +32,7 @@ static inline bool blk_rq_integrity_dma_unmap(struct request *req,
 		struct device *dma_dev, struct dma_iova_state *state,
 		size_t mapped_len)
 {
-	return blk_dma_unmap(req, dma_dev, state, mapped_len,
-			bio_integrity(req->bio)->bip_flags & BIP_P2P_DMA);
+	return blk_rq_dma_unmap(req, dma_dev, state, mapped_len);
 }
 
 int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
diff --git a/include/linux/blk-mq-dma.h b/include/linux/blk-mq-dma.h
index 51829958d872..e93e167ec551 100644
--- a/include/linux/blk-mq-dma.h
+++ b/include/linux/blk-mq-dma.h
@@ -29,6 +29,8 @@ bool blk_rq_dma_map_iter_start(struct request *req, struct device *dma_dev,
 		struct dma_iova_state *state, struct blk_dma_iter *iter);
 bool blk_rq_dma_map_iter_next(struct request *req, struct device *dma_dev,
 		struct dma_iova_state *state, struct blk_dma_iter *iter);
+bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
+		struct dma_iova_state *state, size_t mapped_len);
 
 /**
  * blk_rq_dma_map_coalesce - were all segments coalesced?
@@ -42,37 +44,4 @@ static inline bool blk_rq_dma_map_coalesce(struct dma_iova_state *state)
 	return dma_use_iova(state);
 }
 
-/**
- * blk_dma_unmap - try to DMA unmap a request
- * @req:	request to unmap
- * @dma_dev:	device to unmap from
- * @state:	DMA IOVA state
- * @mapped_len: number of bytes to unmap
- * @is_p2p:	true if mapped with PCI_P2PDMA_MAP_BUS_ADDR
- *
- * Returns %false if the callers need to manually unmap every DMA segment
- * mapped using @iter or %true if no work is left to be done.
- */
-static inline bool blk_dma_unmap(struct request *req, struct device *dma_dev,
-		struct dma_iova_state *state, size_t mapped_len, bool is_p2p)
-{
-	if (is_p2p)
-		return true;
-
-	if (dma_use_iova(state)) {
-		dma_iova_destroy(dma_dev, state, mapped_len, rq_dma_dir(req),
-				 0);
-		return true;
-	}
-
-	return !dma_need_unmap(dma_dev);
-}
-
-static inline bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
-		struct dma_iova_state *state, size_t mapped_len)
-{
-	return blk_dma_unmap(req, dma_dev, state, mapped_len,
-				req->cmd_flags & REQ_P2PDMA);
-}
-
 #endif /* BLK_MQ_DMA_H */
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/4] block-dma: properly take MMIO path
  2025-10-13 15:34 [PATCH 0/4] Properly take MMIO path Leon Romanovsky
  2025-10-13 15:34 ` [PATCH 1/4] blk-mq-dma: migrate to dma_map_phys instead of map_page Leon Romanovsky
  2025-10-13 15:34 ` [PATCH 2/4] blk-mq-dma: unify DMA unmap routine Leon Romanovsky
@ 2025-10-13 15:34 ` Leon Romanovsky
  2025-10-13 19:01   ` Keith Busch
  2025-10-13 15:34 ` [PATCH 4/4] nvme-pci: unmap MMIO pages with appropriate interface Leon Romanovsky
  2025-10-14 11:26 ` [PATCH 0/4] Properly take MMIO path shinichiro.kawasaki
  4 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-13 15:34 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Leon Romanovsky, Jason Gunthorpe, Keith Busch, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

From: Leon Romanovsky <leonro@nvidia.com>

Make sure that CPU is not synced and IOMMU is configured to take
MMIO path by providing newly introduced DMA_ATTR_MMIO attribute.

Reviewed-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 block/blk-mq-dma.c            | 23 ++++++++++++++++++++---
 include/linux/bio-integrity.h |  1 +
 include/linux/blk_types.h     |  2 ++
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index 0648bcb705ff..9495b78b6fd3 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -93,8 +93,13 @@ static bool blk_dma_map_bus(struct blk_dma_iter *iter, struct phys_vec *vec)
 static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
 		struct blk_dma_iter *iter, struct phys_vec *vec)
 {
+	unsigned int attrs = 0;
+
+	if (req->cmd_flags & REQ_MMIO)
+		attrs |= DMA_ATTR_MMIO;
+
 	iter->addr = dma_map_phys(dma_dev, vec->paddr, vec->len,
-			rq_dma_dir(req), 0);
+			rq_dma_dir(req), attrs);
 	if (dma_mapping_error(dma_dev, iter->addr)) {
 		iter->status = BLK_STS_RESOURCE;
 		return false;
@@ -109,14 +114,17 @@ static bool blk_rq_dma_map_iova(struct request *req, struct device *dma_dev,
 {
 	enum dma_data_direction dir = rq_dma_dir(req);
 	unsigned int mapped = 0;
+	unsigned int attrs = 0;
 	int error;
 
 	iter->addr = state->addr;
 	iter->len = dma_iova_size(state);
+	if (req->cmd_flags & REQ_MMIO)
+		attrs |= DMA_ATTR_MMIO;
 
 	do {
 		error = dma_iova_link(dma_dev, state, vec->paddr, mapped,
-				vec->len, dir, 0);
+				vec->len, dir, attrs);
 		if (error)
 			break;
 		mapped += vec->len;
@@ -184,6 +192,11 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
 		 * P2P transfers through the host bridge are treated the
 		 * same as non-P2P transfers below and during unmap.
 		 */
+		if (iter->iter.is_integrity)
+			bio_integrity(req->bio)->bip_flags |= BIP_MMIO;
+		else
+			req->cmd_flags |= REQ_MMIO;
+		fallthrough;
 	case PCI_P2PDMA_MAP_NONE:
 		break;
 	default:
@@ -274,14 +287,18 @@ bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
 		struct dma_iova_state *state, size_t mapped_len)
 {
 	struct bio_integrity_payload *bip = bio_integrity(req->bio);
+	unsigned int attrs = 0;
 
 	if ((!bip && req->cmd_flags & REQ_P2PDMA) ||
 	    bio_integrity_flagged(req->bio, BIP_P2P_DMA))
 		return true;
 
 	if (dma_use_iova(state)) {
+		if ((!bip && req->cmd_flags & REQ_MMIO) ||
+		    bio_integrity_flagged(req->bio, BIP_MMIO))
+			attrs |= DMA_ATTR_MMIO;
 		dma_iova_destroy(dma_dev, state, mapped_len, rq_dma_dir(req),
-				 0);
+				 attrs);
 		return true;
 	}
 
diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index 851254f36eb3..b77b2cfb7b0f 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -14,6 +14,7 @@ enum bip_flags {
 	BIP_CHECK_REFTAG	= 1 << 6, /* reftag check */
 	BIP_CHECK_APPTAG	= 1 << 7, /* apptag check */
 	BIP_P2P_DMA		= 1 << 8, /* using P2P address */
+	BIP_MMIO		= 1 << 9, /* contains MMIO memory */
 };
 
 struct bio_integrity_payload {
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 8e8d1cc8b06c..9affa3b2d047 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -382,6 +382,7 @@ enum req_flag_bits {
 	__REQ_FS_PRIVATE,	/* for file system (submitter) use */
 	__REQ_ATOMIC,		/* for atomic write operations */
 	__REQ_P2PDMA,		/* contains P2P DMA pages */
+	__REQ_MMIO,		/* contains MMIO memory */
 	/*
 	 * Command specific flags, keep last:
 	 */
@@ -415,6 +416,7 @@ enum req_flag_bits {
 #define REQ_FS_PRIVATE	(__force blk_opf_t)(1ULL << __REQ_FS_PRIVATE)
 #define REQ_ATOMIC	(__force blk_opf_t)(1ULL << __REQ_ATOMIC)
 #define REQ_P2PDMA	(__force blk_opf_t)(1ULL << __REQ_P2PDMA)
+#define REQ_MMIO	(__force blk_opf_t)(1ULL << __REQ_MMIO)
 
 #define REQ_NOUNMAP	(__force blk_opf_t)(1ULL << __REQ_NOUNMAP)
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/4] nvme-pci: unmap MMIO pages with appropriate interface
  2025-10-13 15:34 [PATCH 0/4] Properly take MMIO path Leon Romanovsky
                   ` (2 preceding siblings ...)
  2025-10-13 15:34 ` [PATCH 3/4] block-dma: properly take MMIO path Leon Romanovsky
@ 2025-10-13 15:34 ` Leon Romanovsky
  2025-10-15  4:20   ` Christoph Hellwig
  2025-10-14 11:26 ` [PATCH 0/4] Properly take MMIO path shinichiro.kawasaki
  4 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-13 15:34 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Leon Romanovsky, Jason Gunthorpe, Keith Busch, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

From: Leon Romanovsky <leonro@nvidia.com>

Block layer maps MMIO memory through dma_map_phys() interface
with help of DMA_ATTR_MMIO attribute. There is a need to unmap
that memory with the appropriate unmap function, something which
wasn't possible before adding new REQ attribute to block layer in
previous patch.

Reviewed-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/nvme/host/pci.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c916176bd9f0..2e9fb3c7bc09 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -689,11 +689,15 @@ static void nvme_free_prps(struct request *req)
 {
 	struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
 	struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
+	unsigned int attrs = 0;
 	unsigned int i;
 
+	if (req->cmd_flags & REQ_MMIO)
+		attrs |= DMA_ATTR_MMIO;
+
 	for (i = 0; i < iod->nr_dma_vecs; i++)
-		dma_unmap_page(nvmeq->dev->dev, iod->dma_vecs[i].addr,
-				iod->dma_vecs[i].len, rq_dma_dir(req));
+		dma_unmap_phys(nvmeq->dev->dev, iod->dma_vecs[i].addr,
+				iod->dma_vecs[i].len, rq_dma_dir(req), attrs);
 	mempool_free(iod->dma_vecs, nvmeq->dev->dmavec_mempool);
 }
 
@@ -704,16 +708,20 @@ static void nvme_free_sgls(struct request *req, struct nvme_sgl_desc *sge,
 	enum dma_data_direction dir = rq_dma_dir(req);
 	unsigned int len = le32_to_cpu(sge->length);
 	struct device *dma_dev = nvmeq->dev->dev;
+	unsigned int attrs = 0;
 	unsigned int i;
 
+	if (req->cmd_flags & REQ_MMIO)
+		attrs |= DMA_ATTR_MMIO;
+
 	if (sge->type == (NVME_SGL_FMT_DATA_DESC << 4)) {
-		dma_unmap_page(dma_dev, le64_to_cpu(sge->addr), len, dir);
+		dma_unmap_phys(dma_dev, le64_to_cpu(sge->addr), len, dir, attrs);
 		return;
 	}
 
 	for (i = 0; i < len / sizeof(*sg_list); i++)
-		dma_unmap_page(dma_dev, le64_to_cpu(sg_list[i].addr),
-			le32_to_cpu(sg_list[i].length), dir);
+		dma_unmap_phys(dma_dev, le64_to_cpu(sg_list[i].addr),
+			le32_to_cpu(sg_list[i].length), dir, attrs);
 }
 
 static void nvme_unmap_metadata(struct request *req)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] blk-mq-dma: unify DMA unmap routine
  2025-10-13 15:34 ` [PATCH 2/4] blk-mq-dma: unify DMA unmap routine Leon Romanovsky
@ 2025-10-13 18:53   ` Keith Busch
  2025-10-13 19:35     ` Leon Romanovsky
  0 siblings, 1 reply; 15+ messages in thread
From: Keith Busch @ 2025-10-13 18:53 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Christoph Hellwig, Jens Axboe, Leon Romanovsky, Jason Gunthorpe,
	linux-block, linux-kernel, linux-nvme, Sagi Grimberg

On Mon, Oct 13, 2025 at 06:34:10PM +0300, Leon Romanovsky wrote:
> +bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
> +		struct dma_iova_state *state, size_t mapped_len)
> +{
> +	struct bio_integrity_payload *bip = bio_integrity(req->bio);
> +
> +	if ((!bip && req->cmd_flags & REQ_P2PDMA) ||
> +	    bio_integrity_flagged(req->bio, BIP_P2P_DMA))
> +		return true;

I don't think you can unify it at this part here because the data
payload might not be P2P but the integrity payload could be. The data
payload needs to proceed to the next unmapping step in that case, but
this change would have it return true early.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] block-dma: properly take MMIO path
  2025-10-13 15:34 ` [PATCH 3/4] block-dma: properly take MMIO path Leon Romanovsky
@ 2025-10-13 19:01   ` Keith Busch
  2025-10-13 19:29     ` Leon Romanovsky
  0 siblings, 1 reply; 15+ messages in thread
From: Keith Busch @ 2025-10-13 19:01 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Christoph Hellwig, Jens Axboe, Leon Romanovsky, Jason Gunthorpe,
	linux-block, linux-kernel, linux-nvme, Sagi Grimberg

On Mon, Oct 13, 2025 at 06:34:11PM +0300, Leon Romanovsky wrote:
>  static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
>  		struct blk_dma_iter *iter, struct phys_vec *vec)
>  {
> +	unsigned int attrs = 0;
> +
> +	if (req->cmd_flags & REQ_MMIO)
> +		attrs |= DMA_ATTR_MMIO;

Since data and integrity paylods use these same functions and may point
to different kinds of memory, I think you'd have to pass the 'attrs'
from the caller since it knows which flags to check for MMIO dma.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] block-dma: properly take MMIO path
  2025-10-13 19:01   ` Keith Busch
@ 2025-10-13 19:29     ` Leon Romanovsky
  2025-10-13 19:34       ` Keith Busch
  0 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-13 19:29 UTC (permalink / raw)
  To: Keith Busch
  Cc: Christoph Hellwig, Jens Axboe, Jason Gunthorpe, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

On Mon, Oct 13, 2025 at 01:01:18PM -0600, Keith Busch wrote:
> On Mon, Oct 13, 2025 at 06:34:11PM +0300, Leon Romanovsky wrote:
> >  static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
> >  		struct blk_dma_iter *iter, struct phys_vec *vec)
> >  {
> > +	unsigned int attrs = 0;
> > +
> > +	if (req->cmd_flags & REQ_MMIO)
> > +		attrs |= DMA_ATTR_MMIO;
> 
> Since data and integrity paylods use these same functions and may point
> to different kinds of memory, I think you'd have to pass the 'attrs'
> from the caller since it knows which flags to check for MMIO dma.

I think that hunk will fix it.

diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index 38f5c34ca223..8af88ba97c7a 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -88,9 +88,11 @@ static bool blk_dma_map_bus(struct blk_dma_iter *iter, struct phys_vec *vec)
 static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
                struct blk_dma_iter *iter, struct phys_vec *vec)
 {
+       struct bio_integrity_payload *bip = bio_integrity(req->bio);
        unsigned int attrs = 0;

-       if (req->cmd_flags & REQ_MMIO)
+       if ((!bip && req->cmd_flags & REQ_MMIO) ||
+           bio_integrity_flagged(req->bio, BIP_MMIO))
                attrs |= DMA_ATTR_MMIO;

        iter->addr = dma_map_phys(dma_dev, vec->paddr, vec->len,
~

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] block-dma: properly take MMIO path
  2025-10-13 19:29     ` Leon Romanovsky
@ 2025-10-13 19:34       ` Keith Busch
  2025-10-15  4:18         ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Keith Busch @ 2025-10-13 19:34 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Christoph Hellwig, Jens Axboe, Jason Gunthorpe, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

On Mon, Oct 13, 2025 at 10:29:40PM +0300, Leon Romanovsky wrote:
> On Mon, Oct 13, 2025 at 01:01:18PM -0600, Keith Busch wrote:
> > On Mon, Oct 13, 2025 at 06:34:11PM +0300, Leon Romanovsky wrote:
> > >  static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
> > >  		struct blk_dma_iter *iter, struct phys_vec *vec)
> > >  {
> > > +	unsigned int attrs = 0;
> > > +
> > > +	if (req->cmd_flags & REQ_MMIO)
> > > +		attrs |= DMA_ATTR_MMIO;
> > 
> > Since data and integrity paylods use these same functions and may point
> > to different kinds of memory, I think you'd have to pass the 'attrs'
> > from the caller since it knows which flags to check for MMIO dma.
> 
> I think that hunk will fix it.
> 
> diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
> index 38f5c34ca223..8af88ba97c7a 100644
> --- a/block/blk-mq-dma.c
> +++ b/block/blk-mq-dma.c
> @@ -88,9 +88,11 @@ static bool blk_dma_map_bus(struct blk_dma_iter *iter, struct phys_vec *vec)
>  static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
>                 struct blk_dma_iter *iter, struct phys_vec *vec)
>  {
> +       struct bio_integrity_payload *bip = bio_integrity(req->bio);
>         unsigned int attrs = 0;
> 
> -       if (req->cmd_flags & REQ_MMIO)
> +       if ((!bip && req->cmd_flags & REQ_MMIO) ||
> +           bio_integrity_flagged(req->bio, BIP_MMIO))
>                 attrs |= DMA_ATTR_MMIO;

If cmd_flags has REQ_MMIO set, but the integrity flag doesn't have
BIP_MMIO set, you will skip setting DMA_ATTR_MMIO, but I think we need
that set when this is called for the data payload.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] blk-mq-dma: unify DMA unmap routine
  2025-10-13 18:53   ` Keith Busch
@ 2025-10-13 19:35     ` Leon Romanovsky
  0 siblings, 0 replies; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-13 19:35 UTC (permalink / raw)
  To: Keith Busch
  Cc: Christoph Hellwig, Jens Axboe, Jason Gunthorpe, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

On Mon, Oct 13, 2025 at 12:53:47PM -0600, Keith Busch wrote:
> On Mon, Oct 13, 2025 at 06:34:10PM +0300, Leon Romanovsky wrote:
> > +bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
> > +		struct dma_iova_state *state, size_t mapped_len)
> > +{
> > +	struct bio_integrity_payload *bip = bio_integrity(req->bio);
> > +
> > +	if ((!bip && req->cmd_flags & REQ_P2PDMA) ||
> > +	    bio_integrity_flagged(req->bio, BIP_P2P_DMA))
> > +		return true;
> 
> I don't think you can unify it at this part here because the data
> payload might not be P2P but the integrity payload could be. The data
> payload needs to proceed to the next unmapping step in that case, but
> this change would have it return true early.

I was under wrong impression that request has or data payload or
integrity, but never both :(.

Thanks

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/4] Properly take MMIO path
  2025-10-13 15:34 [PATCH 0/4] Properly take MMIO path Leon Romanovsky
                   ` (3 preceding siblings ...)
  2025-10-13 15:34 ` [PATCH 4/4] nvme-pci: unmap MMIO pages with appropriate interface Leon Romanovsky
@ 2025-10-14 11:26 ` shinichiro.kawasaki
  4 siblings, 0 replies; 15+ messages in thread
From: shinichiro.kawasaki @ 2025-10-14 11:26 UTC (permalink / raw)
  To: linux-block; +Cc: shinichiro.kawasaki, dennis.maisenbacher

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

Dear patch submitter,

Blktests CI has tested the following submission:
Status:     FAILURE
Name:       [0/4] Properly take MMIO path
Patchwork:  https://patchwork.kernel.org/project/linux-block/list/?series=1010816&state=*
Run record: https://github.com/linux-blktests/linux-block/actions/runs/18488572174


Failed test cases: nvme/005



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] blk-mq-dma: migrate to dma_map_phys instead of map_page
  2025-10-13 15:34 ` [PATCH 1/4] blk-mq-dma: migrate to dma_map_phys instead of map_page Leon Romanovsky
@ 2025-10-15  4:16   ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2025-10-15  4:16 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Christoph Hellwig, Jens Axboe, Leon Romanovsky, Jason Gunthorpe,
	Keith Busch, linux-block, linux-kernel, linux-nvme, Sagi Grimberg

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] block-dma: properly take MMIO path
  2025-10-13 19:34       ` Keith Busch
@ 2025-10-15  4:18         ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2025-10-15  4:18 UTC (permalink / raw)
  To: Keith Busch
  Cc: Leon Romanovsky, Christoph Hellwig, Jens Axboe, Jason Gunthorpe,
	linux-block, linux-kernel, linux-nvme, Sagi Grimberg

On Mon, Oct 13, 2025 at 01:34:30PM -0600, Keith Busch wrote:
> On Mon, Oct 13, 2025 at 10:29:40PM +0300, Leon Romanovsky wrote:
> > On Mon, Oct 13, 2025 at 01:01:18PM -0600, Keith Busch wrote:
> > > On Mon, Oct 13, 2025 at 06:34:11PM +0300, Leon Romanovsky wrote:
> > > >  static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
> > > >  		struct blk_dma_iter *iter, struct phys_vec *vec)
> > > >  {
> > > > +	unsigned int attrs = 0;
> > > > +
> > > > +	if (req->cmd_flags & REQ_MMIO)
> > > > +		attrs |= DMA_ATTR_MMIO;
> > > 
> > > Since data and integrity paylods use these same functions and may point
> > > to different kinds of memory, I think you'd have to pass the 'attrs'
> > > from the caller since it knows which flags to check for MMIO dma.
> > 
> > I think that hunk will fix it.
> > 
> > diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
> > index 38f5c34ca223..8af88ba97c7a 100644
> > --- a/block/blk-mq-dma.c
> > +++ b/block/blk-mq-dma.c
> > @@ -88,9 +88,11 @@ static bool blk_dma_map_bus(struct blk_dma_iter *iter, struct phys_vec *vec)
> >  static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
> >                 struct blk_dma_iter *iter, struct phys_vec *vec)
> >  {
> > +       struct bio_integrity_payload *bip = bio_integrity(req->bio);
> >         unsigned int attrs = 0;
> > 
> > -       if (req->cmd_flags & REQ_MMIO)
> > +       if ((!bip && req->cmd_flags & REQ_MMIO) ||
> > +           bio_integrity_flagged(req->bio, BIP_MMIO))
> >                 attrs |= DMA_ATTR_MMIO;
> 
> If cmd_flags has REQ_MMIO set, but the integrity flag doesn't have
> BIP_MMIO set, you will skip setting DMA_ATTR_MMIO, but I think we need
> that set when this is called for the data payload.

Yes, we just need to check the relevant flag in the caller
and pass it in.  Or maybe just stash it in  struct blk_dma_iter.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] nvme-pci: unmap MMIO pages with appropriate interface
  2025-10-13 15:34 ` [PATCH 4/4] nvme-pci: unmap MMIO pages with appropriate interface Leon Romanovsky
@ 2025-10-15  4:20   ` Christoph Hellwig
  2025-10-15  6:44     ` Leon Romanovsky
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2025-10-15  4:20 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Christoph Hellwig, Jens Axboe, Leon Romanovsky, Jason Gunthorpe,
	Keith Busch, linux-block, linux-kernel, linux-nvme, Sagi Grimberg

On Mon, Oct 13, 2025 at 06:34:12PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> Block layer maps MMIO memory through dma_map_phys() interface
> with help of DMA_ATTR_MMIO attribute. There is a need to unmap
> that memory with the appropriate unmap function, something which
> wasn't possible before adding new REQ attribute to block layer in
> previous patch.

This should go into the same patch that switches to dma_map_phys.
Unless I'm missing something it also misses passing the flag for
the metadata mapping.

Btw, where is all this going?  Are you trying to remove the auto
detection of P2P in the low-level dma mapping routines?  If so that
should probably go into at very least the cover lttter, but probably also
the commit logs.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] nvme-pci: unmap MMIO pages with appropriate interface
  2025-10-15  4:20   ` Christoph Hellwig
@ 2025-10-15  6:44     ` Leon Romanovsky
  0 siblings, 0 replies; 15+ messages in thread
From: Leon Romanovsky @ 2025-10-15  6:44 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Jason Gunthorpe, Keith Busch, linux-block,
	linux-kernel, linux-nvme, Sagi Grimberg

On Wed, Oct 15, 2025 at 06:20:53AM +0200, Christoph Hellwig wrote:
> On Mon, Oct 13, 2025 at 06:34:12PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > Block layer maps MMIO memory through dma_map_phys() interface
> > with help of DMA_ATTR_MMIO attribute. There is a need to unmap
> > that memory with the appropriate unmap function, something which
> > wasn't possible before adding new REQ attribute to block layer in
> > previous patch.
> 
> This should go into the same patch that switches to dma_map_phys.

I don't think so, dma_map_phys() patch [1] doesn't change any behavior
and dma_map_page() is equal to dma_map_phys(... , attr = 0),

> Unless I'm missing something it also misses passing the flag for
> the metadata mapping.

Yes, I didn't realize that same request can have both metadata and data
payloads.

> 
> Btw, where is all this going?  Are you trying to remove the auto
> detection of P2P in the low-level dma mapping routines?  If so that
> should probably go into at very least the cover lttter, but probably also
> the commit logs.

It is an outcome of multiple things:
1. We missed setting of IOMMU_MMIO flag in dma-iommu.c flow for p2p pages
and for that we need some external indication as memory type is already
known to the callers.
2. Robin expressed concerns about overloading DMA_ATTR_SKIP_CPU_SYNC.

[1] https://lore.kernel.org/all/a40705f38a9f3c757f30228b9b848ce0a87cbcdd.1760369219.git.leon@kernel.org/
[2] https://lore.kernel.org/all/751e7ece-8640-4653-b308-96da6731b8e7@arm.com/

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-10-15  6:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-13 15:34 [PATCH 0/4] Properly take MMIO path Leon Romanovsky
2025-10-13 15:34 ` [PATCH 1/4] blk-mq-dma: migrate to dma_map_phys instead of map_page Leon Romanovsky
2025-10-15  4:16   ` Christoph Hellwig
2025-10-13 15:34 ` [PATCH 2/4] blk-mq-dma: unify DMA unmap routine Leon Romanovsky
2025-10-13 18:53   ` Keith Busch
2025-10-13 19:35     ` Leon Romanovsky
2025-10-13 15:34 ` [PATCH 3/4] block-dma: properly take MMIO path Leon Romanovsky
2025-10-13 19:01   ` Keith Busch
2025-10-13 19:29     ` Leon Romanovsky
2025-10-13 19:34       ` Keith Busch
2025-10-15  4:18         ` Christoph Hellwig
2025-10-13 15:34 ` [PATCH 4/4] nvme-pci: unmap MMIO pages with appropriate interface Leon Romanovsky
2025-10-15  4:20   ` Christoph Hellwig
2025-10-15  6:44     ` Leon Romanovsky
2025-10-14 11:26 ` [PATCH 0/4] Properly take MMIO path shinichiro.kawasaki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).