[PATCH v3] drm/amdgpu: Take IOMMU remapping into account for p2p checks

[Alex Deucher]

On Wed, Aug 14, 2024 at 5:15 AM Rahul Jain <Rahul.Jain at amd.com> wrote:
>
> when trying to enable p2p the amdgpu_device_is_peer_accessible()
> checks the condition where address_mask overlaps the aper_base
> and hence returns 0, due to which the p2p disables for this platform
>
> IOMMU should remap the BAR addresses so the device can access
> them. Hence check if peer_adev is remapping DMA
>
> v3:
> - remove iommu_remap variable
>
> v2: (Alex)
> - Fix as per review comments
> - add new function amdgpu_device_check_iommu_remap to check if iommu
>   remap
>
> Signed-off-by: Rahul Jain <Rahul.Jain at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 46 +++++++++++++++++++---
>  1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index a6b8d0ba4758..040c75c491cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3952,6 +3952,25 @@ static void amdgpu_device_check_iommu_direct_map(struct amdgpu_device *adev)
>                 adev->ram_is_direct_mapped = true;
>  }
>
> +/**
> + * amdgpu_device_check_iommu_remap - check if iommu remaped BAR

change this to:
Check if DMA remapping is enabled.

since it's not just the BAR, all system address space accesses will be remapped.

> + *
> + * @adev: amdgpu_device pointer
> + *
> + * return if IOMMU remapping bar address
> + */
> +static bool amdgpu_device_check_iommu_remap(struct amdgpu_device *adev)
> +{
> +       struct iommu_domain *domain;
> +
> +       domain = iommu_get_domain_for_dev(adev->dev);
> +       if (domain && (domain->type == IOMMU_DOMAIN_DMA ||
> +               domain->type == IOMMU_DOMAIN_DMA_FQ))
> +               return true;
> +
> +       return false;
> +}
> +
>  static const struct attribute *amdgpu_dev_attributes[] = {
>         &dev_attr_pcie_replay_count.attr,
>         NULL
> @@ -6127,6 +6146,8 @@ bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
>                                       struct amdgpu_device *peer_adev)
>  {
>  #ifdef CONFIG_HSA_AMD_P2P
> +       bool peer_remap = amdgpu_device_check_iommu_remap(peer_adev);
> +
>         uint64_t address_mask = peer_adev->dev->dma_mask ?
>                 ~*peer_adev->dev->dma_mask : ~((1ULL << 32) - 1);
>         resource_size_t aper_limit =
> @@ -6135,13 +6156,26 @@ bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
>                 !adev->gmc.xgmi.connected_to_cpu &&
>                 !(pci_p2pdma_distance(adev->pdev, peer_adev->dev, false) < 0);
>
> -       return pcie_p2p && p2p_access && (adev->gmc.visible_vram_size &&
> -               adev->gmc.real_vram_size == adev->gmc.visible_vram_size &&
> -               !(adev->gmc.aper_base & address_mask ||
> -                 aper_limit & address_mask));
> -#else
> -       return false;
> +       if (peer_remap)
> +               /**

These don't need to be kerneldoc comments.  Replace /** with /*

> +                * IOMMU is remapping DMA for peer_adev so all accesses
> +                * should be within peer_adev's DMA mask
> +                */
> +               return pcie_p2p && p2p_access &&
> +                       (adev->gmc.visible_vram_size &&
> +                        adev->gmc.real_vram_size == adev->gmc.visible_vram_size);
> +       else
> +               /**

Same here.

With those fixed, it looks good to me, but it would be good if Ramesh
took a look as well as he wrote this code originally.

Alex


> +                * No IOMMU remapping so make sure the adev's aperture
> +                * fits into peer_adev's dma mask
> +                */
> +               return pcie_p2p && p2p_access &&
> +                       (adev->gmc.visible_vram_size &&
> +                       adev->gmc.real_vram_size == adev->gmc.visible_vram_size &&
> +                       !(adev->gmc.aper_base & address_mask ||
> +                       aper_limit & address_mask));
>  #endif
> +       return false;
>  }
>
>  int amdgpu_device_baco_enter(struct drm_device *dev)
> --
> 2.34.1
>