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

[Marek Szyprowski <m.szyprowski@samsung.com>]

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