Booting with IOMMU disabled in the bios breaks this. I'll submit a fix in a minute. Should be easy. Tom On 19/09/17 11:25 AM, Tom St Denis wrote: > On 19/09/17 11:17 AM, Deucher, Alexander wrote: >>> -----Original Message----- >>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf >>> Of Tom St Denis >>> Sent: Tuesday, September 19, 2017 8:13 AM >>> To: amd-gfx at lists.freedesktop.org >>> Cc: StDenis, Tom >>> Subject: [PATCH] drm/amd/amdgpu: add support for iova_to_phys to >>> replace TTM trace (v5) >>> >>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com> >>> >>> (v2): Add domain to iova debugfs >>> (v3): Add true read/write methods to access system memory of pages >>>       mapped to the device >>> (v4): Move get_domain call out of loop and return on error >>> (v5): Just use kmap/kunmap >>> --- >>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 99 >>> +++++++++++++++++++++++++++++++++ >>>  1 file changed, 99 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> index 50d20903de4f..c7f8e081a772 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> @@ -43,6 +43,7 @@ >>>  #include <linux/swap.h> >>>  #include <linux/pagemap.h> >>>  #include <linux/debugfs.h> >>> +#include <linux/iommu.h> >> >> Just to double check, do you need any compile guards for the IOMMU >> disabled case? > > I honestly don't know. Is there an easy way to setup a cross build that > won't automagically enable the iommu in the configuration? > > (also, if the iommu team co-operated from the get go...). > > Looking at the cavium thunder ethernet source (I grepped for > iova_to_phys) it seems the return of dev_to_domain might be NULL if > IOMMU is disabled in which case these read/writes will return -EFAULT... > > ideally the iommu_iova_to_phys() would be an identity function if the > domain is NULL. Which it doesn't. It'll OOPS if domain==NULL. > > So the fix might be to skip the iova_to_phys call if domain==NULL and > act on the address as a physical address. > > On my carrizo with IOMMU enabled I can read the gfx ring buffer directly > with --vm-read for both the Carrizo and Polaris10 (the Carrizo doesn't > do mapping, the Polaris10 does). I can try a boot with iommu disabled > and see what happens. > > Tom > > >> >> Alex >> >>>  #include "amdgpu.h" >>>  #include "amdgpu_trace.h" >>>  #include "bif/bif_4_1_d.h" >>> @@ -1809,7 +1810,104 @@ static const struct file_operations >>> amdgpu_ttm_gtt_fops = { >>> >>>  #endif >>> >>> +static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user >>> *buf, >>> +                  size_t size, loff_t *pos) >>> +{ >>> +   struct amdgpu_device *adev = file_inode(f)->i_private; >>> +   ssize_t result, n; >>> +   int r; >>> +   uint64_t phys; >>> +   void *ptr; >>> +   struct iommu_domain *dom; >>> + >>> +   dom = iommu_get_domain_for_dev(adev->dev); >>> +   if (!dom) >>> +       return -EFAULT; >>> + >>> +   result = 0; >>> +   while (size) { >>> +       // get physical address and map >>> +       phys = iommu_iova_to_phys(dom, *pos); >>> + >>> +       // copy upto one page >>> +       if (size > PAGE_SIZE) >>> +           n = PAGE_SIZE; >>> +       else >>> +           n = size; >>> + >>> +       // to end of the page >>> +       if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE) >>> +           n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1)); >>> + >>> +       ptr = kmap(pfn_to_page(PFN_DOWN(phys))); >>> +       if (!ptr) >>> +           return -EFAULT; >>> + >>> +       r = copy_to_user(buf, ptr, n); >>> +       kunmap(pfn_to_page(PFN_DOWN(phys))); >>> +       if (r) >>> +           return -EFAULT; >>> + >>> +       *pos += n; >>> +       size -= n; >>> +       result += n; >>> +   } >>> + >>> +   return result; >>> +} >>> + >>> +static ssize_t amdgpu_iova_to_phys_write(struct file *f, const char >>> __user >>> *buf, >>> +                  size_t size, loff_t *pos) >>> +{ >>> +   struct amdgpu_device *adev = file_inode(f)->i_private; >>> +   ssize_t result, n; >>> +   int r; >>> +   uint64_t phys; >>> +   void *ptr; >>> +   struct iommu_domain *dom; >>> + >>> +   dom = iommu_get_domain_for_dev(adev->dev); >>> +   if (!dom) >>> +       return -EFAULT; >>> + >>> +   result = 0; >>> +   while (size) { >>> +       // get physical address and map >>> +       phys = iommu_iova_to_phys(dom, *pos); >>> >>> +       // copy upto one page >>> +       if (size > PAGE_SIZE) >>> +           n = PAGE_SIZE; >>> +       else >>> +           n = size; >>> + >>> +       // to end of the page >>> +       if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE) >>> +           n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1)); >>> + >>> +       ptr = kmap(pfn_to_page(PFN_DOWN(phys))); >>> +       if (!ptr) >>> +           return -EFAULT; >>> + >>> +       r = copy_from_user(ptr, buf, n); >>> +       kunmap(pfn_to_page(PFN_DOWN(phys))); >>> +       if (r) >>> +           return -EFAULT; >>> + >>> +       *pos += n; >>> +       size -= n; >>> +       result += n; >>> +   } >>> + >>> +   return result; >>> +} >>> + >>> +static const struct file_operations amdgpu_ttm_iova_fops = { >>> +   .owner = THIS_MODULE, >>> +   .read = amdgpu_iova_to_phys_read, >>> +   .write = amdgpu_iova_to_phys_write, >>> +   .llseek = default_llseek >>> +}; >>> >>>  static const struct { >>>      char *name; >>> @@ -1820,6 +1918,7 @@ static const struct { >>>  #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS >>>      { "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT }, >>>  #endif >>> +   { "amdgpu_iova", &amdgpu_ttm_iova_fops, TTM_PL_SYSTEM }, >>>  }; >>> >>>  #endif >>> -- >>> 2.12.0 >>> >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx at lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
