linux-kernel - Re: [PATCH v7 00/17] Provide a new two step DMA mapping API

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: <1034b694-2b25-4649-a004-19e601061b90@samsung.com>
Date: Fri, 21 Mar 2025 00:52:30 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Leon Romanovsky <leon@...nel.org>, Robin Murphy <robin.murphy@....com>,
	Christoph Hellwig <hch@....de>, Jens Axboe <axboe@...nel.dk>, Joerg Roedel
	<joro@...tes.org>, Will Deacon <will@...nel.org>, Sagi Grimberg
	<sagi@...mberg.me>, Keith Busch <kbusch@...nel.org>, Bjorn Helgaas
	<bhelgaas@...gle.com>, Logan Gunthorpe <logang@...tatee.com>, Yishai Hadas
	<yishaih@...dia.com>, Shameer Kolothum
	<shameerali.kolothum.thodi@...wei.com>, Kevin Tian <kevin.tian@...el.com>,
	Alex Williamson <alex.williamson@...hat.com>,
	Jérôme Glisse <jglisse@...hat.com>, Andrew Morton
	<akpm@...ux-foundation.org>, Jonathan Corbet <corbet@....net>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-block@...r.kernel.org, linux-rdma@...r.kernel.org,
	iommu@...ts.linux.dev, linux-nvme@...ts.infradead.org,
	linux-pci@...r.kernel.org, kvm@...r.kernel.org, linux-mm@...ck.org, Randy
	Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH v7 00/17] Provide a new two step DMA mapping API

On 19.03.2025 18:58, Jason Gunthorpe wrote:
> On Fri, Mar 14, 2025 at 11:52:58AM +0100, Marek Szyprowski wrote:
>
>>> The only way to do so is to use dma_map_sg_attrs(), which relies on SG
>>> (the one that we want to remove) to map P2P pages.
>> That's something I don't get yet. How P2P pages can be used with
>> dma_map_sg_attrs(), but not with dma_map_page_attrs()? Both operate
>> internally on struct page pointer.
> It is a bit subtle, I ran in to this when exploring enabling proper
> P2P for dma_map_resource() too.
>
> The API signatures are:
>
> dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
> 		size_t offset, size_t size, enum dma_data_direction dir,
> 		unsigned long attrs);
> void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
> 		enum dma_data_direction dir, unsigned long attrs);
>
> The thing to notice immediately is that the unmap path does not get
> passed a struct page.

I see, thanks for pointing this out. It looks that I've encountered this 
issue too long time ago when I was implementing first 
iommu-to-dma-mapping glue for ARM arch. The lack of some kind of 
'object' returned from dma_map* (and dma_alloc*) and passed back to 
dma_unmap* (and dma_free_*) required some non-trivial workarounds there 
and made the unmap/free path a bit more complicated.

> So, lets think about the flow when the iommu is turned on.
>
> For normal struct page memory:
>
>   - dma_map_page_attrs() allocates some IOVA and returns it in the
>     dma_addr_t and then maps the struct page to the iommu page table
>
>   - dma_unmap_page_attrs() frees the IOVA from the given dma_addr_t
>   
> If we think about P2P now:
>
>   - dma_map_page_attrs() can inspect the struct page and determine it
>     is P2P. It computes a bus address which is not an IOVA, and does
>     not transit through the IOMMU. No IOVA allocation is performed. the
>     bus address is returned as the dma_addr_t
>
>   - dma_unmap_page_attrs() ... is impossible. We just get this
>     dma_addr_t that doesn't have enough information to tell anymore if
>     the address is a P2P bus address or not, so we can't tell if we
>     should unmap an iova from the dma_addr_t :\
>
> The sg path fixes this because it introduced a new flag in the
> scatterlist, SG_DMA_BUS_ADDRESS, that allows the sg map path to record
> the information for the unmap path so it can do the right thing.
>
> Leon's approach fixes this by putting an overarching transaction state
> around the DMA operation so that map and unmap operations can look in
> the state and determine if this is a P2P or non P2P map and then know
> how to unmap.
>
> For some background here, Christoph gave me this idea back at LSF/MM
> in Vancouver (two years ago now). At the time I was looking at
> replacing scatterlist and giving new DMA API ops to operate on a
> "scatterlist v2" structure.
>
> Christoph's vision was to make a performance DMA API path that could
> be used to implement any scatterlist-like data structure very
> efficiently without having to teach the DMA API about all sorts of
> scatterlist-like things.

Thanks for explaining one more motivation behind this patchset!

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland