Re: [PATCH] PCI/P2PDMA: Root complex whitelist should not apply when an IOMMU is present

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux