linux-kernel - Re: [PATCH v4 09/16] dma-mapping: handle MMIO flow in dma_map|unmap

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: <20250831131250.GC10073@unreal>
Date: Sun, 31 Aug 2025 16:12:50 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: Marek Szyprowski <m.szyprowski@...sung.com>,
	Abdiel Janulgue <abdiel.janulgue@...il.com>,
	Alexander Potapenko <glider@...gle.com>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Hellwig <hch@....de>, Danilo Krummrich <dakr@...nel.org>,
	iommu@...ts.linux.dev, Jason Wang <jasowang@...hat.com>,
	Jens Axboe <axboe@...nel.dk>, Joerg Roedel <joro@...tes.org>,
	Jonathan Corbet <corbet@....net>, Juergen Gross <jgross@...e.com>,
	kasan-dev@...glegroups.com, Keith Busch <kbusch@...nel.org>,
	linux-block@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	linux-nvme@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org,
	linux-trace-kernel@...r.kernel.org,
	Madhavan Srinivasan <maddy@...ux.ibm.com>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Michael Ellerman <mpe@...erman.id.au>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Robin Murphy <robin.murphy@....com>, rust-for-linux@...r.kernel.org,
	Sagi Grimberg <sagi@...mberg.me>,
	Stefano Stabellini <sstabellini@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	virtualization@...ts.linux.dev, Will Deacon <will@...nel.org>,
	xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v4 09/16] dma-mapping: handle MMIO flow in
 dma_map|unmap_page

On Thu, Aug 28, 2025 at 12:17:30PM -0300, Jason Gunthorpe wrote:
> On Tue, Aug 19, 2025 at 08:36:53PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@...dia.com>
> > 
> > Extend base DMA page API to handle MMIO flow and follow
> > existing dma_map_resource() implementation to rely on dma_map_direct()
> > only to take DMA direct path.
> 
> I would reword this a little bit too
> 
> dma-mapping: implement DMA_ATTR_MMIO for dma_(un)map_page_attrs()
> 
> Make dma_map_page_attrs() and dma_map_page_attrs() respect
> DMA_ATTR_MMIO.
> 
> DMA_ATR_MMIO makes the functions behave the same as dma_(un)map_resource():
>  - No swiotlb is possible
>  - Legacy dma_ops arches use ops->map_resource()
>  - No kmsan
>  - No arch_dma_map_phys_direct()
> 
> The prior patches have made the internl funtions called here support
> DMA_ATTR_MMIO.
> 
> This is also preparation for turning dma_map_resource() into an inline
> calling dma_map_phys(DMA_ATTR_MMIO) to consolidate the flows.
> 
> > @@ -166,14 +167,25 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
> >  		return DMA_MAPPING_ERROR;
> >  
> >  	if (dma_map_direct(dev, ops) ||
> > -	    arch_dma_map_phys_direct(dev, phys + size))
> > +	    (!is_mmio && arch_dma_map_phys_direct(dev, phys + size)))
> >  		addr = dma_direct_map_phys(dev, phys, size, dir, attrs);
> 
> PPC is the only user of arch_dma_map_phys_direct() and it looks like
> it should be called on MMIO memory. Seems like another inconsistency
> with map_resource. I'd leave it like the above though for this series.
> 
> >  	else if (use_dma_iommu(dev))
> >  		addr = iommu_dma_map_phys(dev, phys, size, dir, attrs);
> > -	else
> > +	else if (is_mmio) {
> > +		if (!ops->map_resource)
> > +			return DMA_MAPPING_ERROR;
> 
> Probably written like:
> 
> 		if (ops->map_resource)
> 			addr = ops->map_resource(dev, phys, size, dir, attrs);
> 		else
> 			addr = DMA_MAPPING_ERROR;

I'm big fan of "if (!ops->map_resource)" coding style and prefer to keep it.

> 
> As I think some of the design here is to run the trace even on the
> failure path?

Yes, this is how it worked before.

> 
> Otherwise looks OK
> 
> Reviewed-by: Jason Gunthorpe <jgg@...dia.com>
> 
> Jason