On Fri, 2 Oct 2020, Christoph Hellwig wrote: > Hi Stefano, > > I've looked over xen-swiotlb in linux-next, that is with your recent > changes to take dma offsets into account. One thing that puzzles me > is that xen_swiotlb_map_page passes virt_to_phys(xen_io_tlb_start) as > the tbl_dma_addr argument to swiotlb_tbl_map_single, despite the fact > that the argument is a dma_addr_t and both other callers translate > from a physical to the dma address. Was this an oversight?
Hi Christoph, It was not an oversight, it was done on purpose, although maybe I could have been wrong. There was a brief discussion on this topic here: https://marc.info/?l=linux-kernel&m=159011972107683&w=2 https://marc.info/?l=linux-kernel&m=159018047129198&w=2 I'll repeat and summarize here for convenience. swiotlb_init_with_tbl is called by xen_swiotlb_init, passing a virtual address (xen_io_tlb_start), which gets converted to phys and stored in io_tlb_start as a physical address at the beginning of swiotlb_init_with_tbl. Afterwards, xen_swiotlb_map_page calls swiotlb_tbl_map_single. The second parameter, dma_addr_t tbl_dma_addr, is used to calculate the right slot in the swiotlb buffer to use, comparing it against io_tlb_start. Thus, I think it makes sense for xen_swiotlb_map_page to call swiotlb_tbl_map_single passing an address meant to be compared with io_tlb_start, which is __pa(xen_io_tlb_start), so virt_to_phys(xen_io_tlb_start) seems to be what we want. However, you are right that it is strange that tbl_dma_addr is a dma_addr_t, and maybe it shouldn't be? Maybe the tbl_dma_addr parameter to swiotlb_tbl_map_single should be a phys address instead? Or it could be swiotlb_init_with_tbl to be wrong and it should take a dma address to initialize the swiotlb buffer. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
