lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251113195008.GA111768@unreal>
Date: Thu, 13 Nov 2025 21:50:08 +0200
From: Leon Romanovsky <leon@...nel.org>
To: Jens Axboe <axboe@...nel.dk>
Cc: Keith Busch <kbusch@...nel.org>, Christoph Hellwig <hch@....de>,
	Sagi Grimberg <sagi@...mberg.me>, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-nvme@...ts.infradead.org
Subject: Re: [PATCH v4 0/2] block: Enable proper MMIO memory handling for P2P
 DMA

On Thu, Nov 13, 2025 at 10:45:53AM -0700, Jens Axboe wrote:
> On 11/13/25 10:12 AM, Jens Axboe wrote:
> > On 11/13/25 9:39 AM, Jens Axboe wrote:
> >>
> >> On Wed, 12 Nov 2025 21:48:03 +0200, Leon Romanovsky wrote:
> >>> Changelog:
> >>> v4:
> >>>  * Changed double "if" to be "else if".
> >>>  * Added missed PCI_P2PDMA_MAP_NONE case.
> >>> v3: https://patch.msgid.link/20251027-block-with-mmio-v3-0-ac3370e1f7b7@nvidia.com
> >>>  * Encoded p2p map type in IOD flags instead of DMA attributes.
> >>>  * Removed REQ_P2PDMA flag from block layer.
> >>>  * Simplified map_phys conversion patch.
> >>> v2: https://lore.kernel.org/all/20251020-block-with-mmio-v2-0-147e9f93d8d4@nvidia.com/
> >>>  * Added Chirstoph's Reviewed-by tag for first patch.
> >>>  * Squashed patches
> >>>  * Stored DMA MMIO attribute in NVMe IOD flags variable instead of block layer.
> >>> v1: https://patch.msgid.link/20251017-block-with-mmio-v1-0-3f486904db5e@nvidia.com
> >>>  * Reordered patches.
> >>>  * Dropped patch which tried to unify unmap flow.
> >>>  * Set MMIO flag separately for data and integrity payloads.
> >>> v0: https://lore.kernel.org/all/cover.1760369219.git.leon@kernel.org/
> >>>
> >>> [...]
> >>
> >> Applied, thanks!
> >>
> >> [1/2] nvme-pci: migrate to dma_map_phys instead of map_page
> >>       commit: f10000db2f7cf29d8c2ade69266bed7b51c772cb
> >> [2/2] block-dma: properly take MMIO path
> >>       commit: 8df2745e8b23fdbe34c5b0a24607f5aaf10ed7eb
> > 
> > And now dropped again - this doesn't boot on neither my big test box
> > with 33 nvme drives, nor even on my local test vm. Two different archs,
> > and very different setups. Which begs the question, how on earth was
> > this tested, if it doesn't boot on anything I have here?!
> 
> I took a look, and what happens here is that iter.p2pdma.map is 0 as it
> never got set to anything. That is the same as PCI_P2PDMA_MAP_UNKNOWN,
> and hence we just end up in a BLK_STS_RESOURCE. First of all, returning
> BLK_STS_RESOURCE for that seems... highly suspicious. That should surely
> be a fatal error. And secondly, this just further backs up that there's
> ZERO testing done on this patchset at all. WTF?
> 
> FWIW, the below makes it boot just fine, as expected, as a default zero
> filled iter then matches the UNKNOWN case.
> 
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index e5ca8301bb8b..4cce69226773 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1087,6 +1087,7 @@ static blk_status_t nvme_map_data(struct request *req)
>  	case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
>  		iod->flags |= IOD_DATA_MMIO;
>  		break;
> +	case PCI_P2PDMA_MAP_UNKNOWN:
>  	case PCI_P2PDMA_MAP_NONE:
>  		break;
>  	default:
> @@ -1122,6 +1123,7 @@ static blk_status_t nvme_pci_setup_meta_iter(struct request *req)
>  	case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
>  		iod->flags |= IOD_META_MMIO;
>  		break;
> +	case PCI_P2PDMA_MAP_UNKNOWN:
>  	case PCI_P2PDMA_MAP_NONE:
>  		break;
>  	default:

Sorry for troubles.

Can you please squash this fixup instead?
diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index 98554929507a..807048644f2e 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -172,6 +172,7 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
 
        memset(&iter->p2pdma, 0, sizeof(iter->p2pdma));
        iter->status = BLK_STS_OK;
+       iter->p2pdma.map = PCI_P2PDMA_MAP_NONE;
 
        /*
         * Grab the first segment ASAP because we'll need it to check for P2P




> 
> -- 
> Jens Axboe

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ