Am 22.05.19 um 22:41 schrieb Logan Gunthorpe: > [CAUTION: External Email] > > On 2019-05-22 2:30 p.m., Koenig, Christian wrote: >> Hi Logan, >> >> Am 22.05.2019 22:12 schrieb Logan Gunthorpe <logang@xxxxxxxxxxxx>: >> >> [CAUTION: External Email] >> >> Presently, there is no path to DMA map P2PDMA memory, so if a TLP >> targeting this memory hits the root complex and an IOMMU is present, >> the IOMMU will reject the transaction, even if the RC would support >> P2PDMA. >> >> So until the kernel knows to map these DMA addresses in the IOMMU, >> we should not enable the whitelist when an IOMMU is present. >> >> >> Well NAK, cause that is exactly what we are doing. > > Are you DMA-mapping the addresses outside the P2PDMA code? If so there's > a huge mismatch with the existing users of P2PDMA (nvme-fabrics). If > you're not dma-mapping then I can't see how it could work because the > IOMMU should reject any requests to access those addresses. Well, we are using the DMA API (dma_map_resource) for this. If the P2P code is not using this then I would rather say that the P2P code is actually broken. Adding Christoph as well, cause he is usually the one discussion stuff like that with me. > By adding the whitelist in this way you will break any user that > attempts to use P2P in nvme-fabrics on whitelisted RCs with an IOMMU > enabled. > > Currently, the users of P2PDMA use pci_p2pdma_map_sg() which only > returns the PCI bus address. If P2PDMA transactions can now go through > an IOMMU, then this is wrong and broken. > > We need to ensure that all users of P2PDMA map this memory in the same > way. Which means, if the TLPs will go through an IOMMU they get > dma-map'd and, if they don't, they use the PCI Bus address (as the > current code does). Well that is exactly what dma_map_resource() already does, so we should probably just make using the DMA API mandatory. > Without the change proposed in this patch, I have to retract my review > and NAK your patch until we can sort out the mapping issues. Yeah, completely agree we can't do that right now. Regards, Christian. > > > Logan