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