On 2022-06-29 23:41, Logan Gunthorpe wrote:
On 2022-06-29 13:15, Robin Murphy wrote:
On 2022-06-29 16:57, Logan Gunthorpe wrote:
On 2022-06-29 06:07, Robin Murphy wrote:
On 2022-06-15 17:12, Logan Gunthorpe wrote:
When a PCI P2PDMA page is seen, set the IOVA length of the segment
to zero so that it is not mapped into the IOVA. Then, in finalise_sg(),
apply the appropriate bus address to the segment. The IOVA is not
created if the scatterlist only consists of P2PDMA pages.
A P2PDMA page may have three possible outcomes when being mapped:
1) If the data path between the two devices doesn't go through
the root port, then it should be mapped with a PCI bus address
2) If the data path goes through the host bridge, it should be
mapped
normally with an IOMMU IOVA.
3) It is not possible for the two devices to communicate and thus
the mapping operation should fail (and it will return
-EREMOTEIO).
Similar to dma-direct, the sg_dma_mark_pci_p2pdma() flag is used to
indicate bus address segments. On unmap, P2PDMA segments are skipped
over when determining the start and end IOVA addresses.
With this change, the flags variable in the dma_map_ops is set to
DMA_F_PCI_P2PDMA_SUPPORTED to indicate support for P2PDMA pages.
Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
---
drivers/iommu/dma-iommu.c | 68
+++++++++++++++++++++++++++++++++++----
1 file changed, 61 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f90251572a5d..b01ca0c6a7ab 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -21,6 +21,7 @@
#include <linux/iova.h>
#include <linux/irq.h>
#include <linux/list_sort.h>
+#include <linux/memremap.h>
#include <linux/mm.h>
#include <linux/mutex.h>
#include <linux/pci.h>
@@ -1062,6 +1063,16 @@ static int __finalise_sg(struct device *dev,
struct scatterlist *sg, int nents,
sg_dma_address(s) = DMA_MAPPING_ERROR;
sg_dma_len(s) = 0;
+ if (is_pci_p2pdma_page(sg_page(s)) && !s_iova_len) {
Logically, should we not be able to use sg_is_dma_bus_address() here? I
think it should be feasible, and simpler, to prepare the p2p segments
up-front, such that at this point all we need to do is restore the
original length (if even that, see below).
Per my previous email, no, because sg_is_dma_bus_address() is not set
yet and not meant to tell you something about the page. That flag will
be set below by pci_p2pdma_map_bus_segment() and then checkd in
iommu_dma_unmap_sg() to determine if the dma_address in the segment
needs to be unmapped.
I know it's not set yet as-is; I'm suggesting things should be
restructured so that it *would be*. In the logical design of this code,
the DMA addresses are effectively determined in iommu_dma_map_sg(), and
__finalise_sg() merely converts them from a relative to an absolute form
(along with undoing the other trickery). Thus the call to
pci_p2pdma_map_bus_segment() absolutely belongs in the main
iommu_map_sg() loop.
I don't see how that can work: __finalise_sg() does more than convert
them from relative to absolute, it also figures out which SG entry will
contain which dma_address segment. Which segment a P2PDMA address needs
to be programmed into depends on the how 'cur' is calculated which in
turn depends on things like seg_mask and max_len. This calculation is
not done in iommu_dma_map_sg() so I don't see how there's any hope of
assigning the bus address for the P2P segments in that function.
If there's a way to restructure things so that's possible that I'm not
seeing, I'm open to it but it's certainly not immediately obvious.
Huh? It's still virtually the same thing; iommu_dma_map_sg() calls
pci_p2pdma_map_bus_segment(s) and sets s->length to 0 if
PCI_P2PDMA_MAP_BUS_ADDR, then __finalise_sg() can use
sg_is_dma_bus_address(s) in place of is_pci_p2pdma_page(sg_page(s)), and
just propagate the DMA address and original length from s to cur.
Here you've written a patch which looks to correctly interrupt any
ongoing concatenation state and convey some data from the given input
segment to the appropriate output segment, so I'm baffled by why you'd
think you couldn't do what you've already done.
Thanks,
Robin.
