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

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, May 22, 2019 at 02:12:52PM -0600, Logan Gunthorpe wrote:
> 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.
> 
> While we are at it, remove the comment mentioning future work
> to add a white list.

There was a lot of discussion about this.  Did everybody come to a
consensus about what should be done?  Can you post a patch with
reviewed-by if appropriate?

> Fixes: 0f97da831026 ("PCI/P2PDMA: Allow P2P DMA between any devices under AMD ZEN Root Complex")
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/p2pdma.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> Hey,
> 
> I realized recently that I missed this issue between the IOMMU and
> the whitelist when reviewing Christian's patch.
> 
> Unless there are any objections, I think this should be squashed
> with the commit marked in the Fixes tag (from pci-v5.2-changes).
> 
> Thanks,
> 
> Logan
> 
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 742928d0053e..4d2f6a44cba3 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -18,6 +18,7 @@
>  #include <linux/percpu-refcount.h>
>  #include <linux/random.h>
>  #include <linux/seq_buf.h>
> +#include <linux/iommu.h>
> 
>  struct pci_p2pdma {
>  	struct percpu_ref devmap_ref;
> @@ -284,6 +285,9 @@ static bool root_complex_whitelist(struct pci_dev *dev)
>  	struct pci_dev *root = pci_get_slot(host->bus, PCI_DEVFN(0, 0));
>  	unsigned short vendor, device;
> 
> +	if (iommu_present(dev->dev.bus))
> +		return false;
> +
>  	if (!root)
>  		return false;
> 
> @@ -453,8 +457,7 @@ static int upstream_bridge_distance_warn(struct pci_dev *provider,
>   *
>   * For now, "compatible" means the provider and the clients are all behind
>   * the same PCI root port. This cuts out cases that may work but is safest
> - * for the user. Future work can expand this to white-list root complexes that
> - * can safely forward between each ports.
> + * for the user.
>   */
>  int pci_p2pdma_distance_many(struct pci_dev *provider, struct device **clients,
>  			     int num_clients, bool verbose)
> --
> 2.20.1