Skip to contentSkip to conversation

fix for regression in pat memory tracking in 4.9-rc1

RFC NAK Dave Airlie 5 participants 9 messages Oct 18, 2016 – Oct 22, 2016 4 messages not in the archive

Subsystems (6)
DRM drivers x86 DRM misc Radeon and amdgpu DRM drivers +2 more subsystems
lore
Share & tools
lore

Dave Airlie

author
#0 lore raw

Okay I spent some time looking into it, and this is the result.

We have to manually insert the VRAM BAR into the pat memory tracking table as WC. The only other way things get inserted are via ioremap, which we never do for the whole VRAM BAR. We could in theory map the VRAM BAR using the iomap stuff that i915 uses, but we don't and this seems easier for now.

We have to fix up at least nouveau and radeon I think as well.

Dave.

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

maintainer
#1 lore raw

PATCH 1/3 x86/pat: export io memory reserve/free api.

From: Dave Airlie <[email protected]>

These functions are needed for gpu/ttm drivers to reserve the VRAM area as write combined. In a lot of places we don't ioremap but still need to insert pfn from it into a VMA using vm_insert_mixed, but a recent change in mixed insertion means we need to reserve VRAM as WC upfront, so we need these APIs exported.

Signed-off-by Dave Airlie <[email protected]>
---
 arch/x86/mm/pat.c | 2 ++
 1 file changed, 2 insertions(+)
arch/x86/mm/pat.c +2 -0
index 170cc4f..5ce2fbb 100644
@@ -719,6 +719,7 @@ out_free:
 out_err:
 	return ret;
 }
+EXPORT_SYMBOL(io_reserve_memtype);
 
 /**
  * io_free_memtype - Release a memory type mapping for a region of memory

@@ -729,6 +730,7 @@ void io_free_memtype(resource_size_t start, resource_size_t end)
 {
 	free_memtype(start, end);
 }
+EXPORT_SYMBOL(io_free_memtype);
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 				unsigned long size, pgprot_t vma_prot)
Signature · 6 lines
2.5.5

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

A message that is not in the archive (sent off-list or not yet received): 1476771193-2759-2-git-send-email-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

NAK
#3 lore raw

Re: [PATCH 1/3] x86/pat: export io memory reserve/free api.

NACK,

I think you want to use 'iomap_create_wc()' instead to avoid aliasing.

Kind Regards,
Edward.

Quoted 27 lines · Dave Airlie

From: Dave Airlie <[email protected]>

These functions are needed for gpu/ttm drivers to reserve the VRAM area as write combined. In a lot of places we don't ioremap but still need to insert pfn from it into a VMA using vm_insert_mixed, but a recent change in mixed insertion means we need to reserve VRAM as WC upfront, so we need these APIs exported.

Signed-off-by Dave Airlie <[email protected]>
---
 arch/x86/mm/pat.c | 2 ++
 1 file changed, 2 insertions(+)
arch/x86/mm/pat.c +1 -0
index 170cc4f..5ce2fbb 100644
@@ -719,6 +719,7 @@ out_free:
 out_err:
 	return ret;
 }
+EXPORT_SYMBOL(io_reserve_memtype);
 
 /**
  * io_free_memtype - Release a memory type mapping for a region of memory

arch/x86/mm/pat.c void io_free_memtype(resource_size_t start, resource_size_t end) · line 736

Quoted 7 lines
arch/x86/mm/pat.c +1 -0
@@ -729,6 +730,7 @@ void io_free_memtype(resource_size_t start, resource_size_t end)
 {
 	free_memtype(start, end);
 }
+EXPORT_SYMBOL(io_free_memtype);
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 				unsigned long size, pgprot_t vma_prot)

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): d5dcf484-d760-8fe6-bc00-4d6db3ef9d05-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org

author
#5 lore raw

Re: [PATCH 1/3] x86/pat: export io memory reserve/free api.

Quoted 4 lines · Edward O'Callaghan

NACK,

I think you want to use 'iomap_create_wc()' instead to avoid aliasing.

Please explain what can alias here?

Dave.

Quoted 28 lines

Kind Regards,
Edward.

Quoted 23 lines · Dave Airlie

From: Dave Airlie <[email protected]>

These functions are needed for gpu/ttm drivers to reserve the VRAM area as write combined. In a lot of places we don't ioremap but still need to insert pfn from it into a VMA using vm_insert_mixed, but a recent change in mixed insertion means we need to reserve VRAM as WC upfront, so we need these APIs exported.

Signed-off-by Dave Airlie <[email protected]>
---
 arch/x86/mm/pat.c | 2 ++
 1 file changed, 2 insertions(+)
arch/x86/mm/pat.c +1 -0
index 170cc4f..5ce2fbb 100644
@@ -719,6 +719,7 @@ out_free:
 out_err:
      return ret;
 }
+EXPORT_SYMBOL(io_reserve_memtype);
Quoted 2 lines

arch/x86/mm/pat.c out_free: · line 725

Quoted 2 lines
arch/x86/mm/pat.c +0 -0
 /**
  * io_free_memtype - Release a memory type mapping for a region of

memory

Quoted 1 line

arch/x86/mm/pat.c void io_free_memtype(resource_size_t start,

Quoted 1 line
arch/x86/mm/pat.c +0 -0
@@ -729,6 +730,7 @@ void io_free_memtype(resource_size_t start,

resource_size_t end)

Quoted 9 lines

arch/x86/mm/pat.c void io_free_memtype(resource_size_t start, resource_size_t end) · line 736

Quoted 8 lines
arch/x86/mm/pat.c +1 -0
 {
      free_memtype(start, end);
 }
+EXPORT_SYMBOL(io_free_memtype);
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
                              unsigned long size, pgprot_t vma_prot)

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): CAPM=9tx2iBaHt-bg_WjUHE3wPprs2GvNt1LZNLngF-GLH2U_Zw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#7 lore raw

Re: [PATCH 1/3] x86/pat: export io memory reserve/free api.

Quoted 8 lines · Dave Airlie
Quoted 4 lines · On 18 Oct. 2016 17:23, "Edward O'Callaghan"…

NACK,

I think you want to use 'iomap_create_wc()' instead to avoid aliasing.

Please explain what can alias here?

Ah disregard the alias comment, I was remembering how it was introduced and that referred to 'io_mapping_create_wc()' not hard coding WC.

Any way, I think 'iomap_create_wc()' is what your looking for so no need to expose those symbols and hook them though.

Cheers,
Edward.

Quoted 32 lines

Dave.

Quoted 14 lines

Kind Regards,
Edward.

Quoted 9 lines · Dave Airlie

From: Dave Airlie <[email protected] <mailto:[email protected]>>

These functions are needed for gpu/ttm drivers to reserve the VRAM area as write combined. In a lot of places we don't ioremap but still need to insert pfn from it into a VMA using vm_insert_mixed, but a recent change in mixed insertion means we need to reserve VRAM as WC upfront, so we need these APIs exported.

Signed-off-by Dave Airlie <[email protected]

<mailto:[email protected]>>

Quoted 14 lines
Quoted 14 lines
---
 arch/x86/mm/pat.c | 2 ++
 1 file changed, 2 insertions(+)
arch/x86/mm/pat.c +1 -0
index 170cc4f..5ce2fbb 100644
@@ -719,6 +719,7 @@ out_free:
 out_err:
      return ret;
 }
+EXPORT_SYMBOL(io_reserve_memtype);
Quoted 15 lines
Quoted 2 lines

arch/x86/mm/pat.c out_free: · line 725

Quoted 2 lines
arch/x86/mm/pat.c +0 -0
 /**
  * io_free_memtype - Release a memory type mapping for a region of

memory

Quoted 1 line

arch/x86/mm/pat.c void io_free_memtype(resource_size_t start,

Quoted 1 line
arch/x86/mm/pat.c +0 -0
@@ -729,6 +730,7 @@ void io_free_memtype(resource_size_t start,

resource_size_t end)

Quoted 9 lines
Quoted 8 lines
arch/x86/mm/pat.c +1 -0
 {
      free_memtype(start, end);
 }
+EXPORT_SYMBOL(io_free_memtype);
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
                              unsigned long size, pgprot_t vma_prot)

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

#8 lore raw

Re: [PATCH 1/3] x86/pat: export io memory reserve/free api.

In reply to a message not in the archive

For the series, no regressions in my testing, no stalls, no issues noted.

Tested by: Shawn Starr <[email protected]>

Thanks,
Shawn

Quoted 26 lines · EDT Dave Airlie

From: Dave Airlie <[email protected]>

These functions are needed for gpu/ttm drivers to reserve the VRAM area as write combined. In a lot of places we don't ioremap but still need to insert pfn from it into a VMA using vm_insert_mixed, but a recent change in mixed insertion means we need to reserve VRAM as WC upfront, so we need these APIs exported.

Signed-off-by Dave Airlie <[email protected]>
---
 arch/x86/mm/pat.c | 2 ++
 1 file changed, 2 insertions(+)
arch/x86/mm/pat.c +1 -0
index 170cc4f..5ce2fbb 100644
@@ -719,6 +719,7 @@ out_free:
 out_err:
 	return ret;
 }
+EXPORT_SYMBOL(io_reserve_memtype);
 
 /**
  * io_free_memtype - Release a memory type mapping for a region of memory

arch/x86/mm/pat.c void io_free_memtype(resource_size_t start, · line 736

Quoted 7 lines
arch/x86/mm/pat.c +1 -0
@@ -729,6 +730,7 @@ void io_free_memtype(resource_size_t start,
 resource_size_t end) {
 	free_memtype(start, end);
 }
+EXPORT_SYMBOL(io_free_memtype);
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 				unsigned long size, pgprot_t vma_prot)

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 1476771193-2759-1-git-send-email-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

author
#10 lore raw

PATCH 2/3 drm/ttm: add API to reserve/free WC memory.

From: Dave Airlie <[email protected]>

These will be used by drivers to reserve/free WC memory in the PAT tracking tables, for VRAM BARs.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 24 ++++++++++++++++++++++++
 include/drm/ttm/ttm_bo_driver.h   |  2 ++
 2 files changed, 26 insertions(+)
drivers/gpu/drm/ttm/ttm_bo_util.c +24 -0
index bf6e216..192c003 100644
@@ -39,6 +39,13 @@
 #include <linux/module.h>
 #include <linux/reservation.h>
 
+#ifdef CONFIG_X86
+#include <asm/pat.h>
+#else
+#define io_reserve_memtype(start, end, pcm) (0)
+#define io_free_memtype(start, end)
+#endif
+
 void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
 {
 	ttm_bo_mem_put(bo, &bo->mem);

@@ -796,3 +803,20 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 	return 0;
 }
 EXPORT_SYMBOL(ttm_bo_pipeline_move);
+
+int ttm_io_reserve_memtype_wc(resource_size_t start, resource_size_t end)
+{
+	enum page_cache_mode pcm = _PAGE_CACHE_MODE_WC;
+	int ret;
+	ret = io_reserve_memtype(start, end, &pcm);
+	if (ret)
+		return ret;
+	return 0;
+}
+EXPORT_SYMBOL(ttm_io_reserve_memtype_wc);
+
+void ttm_io_free_memtype(resource_size_t start, resource_size_t end)
+{
+	io_free_memtype(start, end);
+}
+EXPORT_SYMBOL(ttm_io_free_memtype);
include/drm/ttm/ttm_bo_driver.h +2 -0
index 4f0a921..6b2d24e 100644
@@ -1054,6 +1054,8 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
  */
 extern pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp);
 
+int ttm_io_reserve_memtype_wc(resource_size_t start, resource_size_t end);
+void ttm_io_free_memtype(resource_size_t start, resource_size_t end);
 extern const struct ttm_mem_type_manager_func ttm_bo_manager_func;
 
 #if IS_ENABLED(CONFIG_AGP)
Signature · 6 lines
2.5.5

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
author
#11 lore raw

PATCH 3/3 amdgpu: reserve VRAM ranges in PAT memtype tables.

In reply to a message not in the archive

From: Dave Airlie <[email protected]>

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +3 -0
index 887483b..3142d70 100644
@@ -1111,6 +1111,8 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 		DRM_ERROR("Failed initializing VRAM heap.\n");
 		return r;
 	}
+
+	ttm_io_reserve_memtype_wc(adev->mc.aper_base, adev->mc.aper_base + adev->mc.aper_size);
 	/* Change the size here instead of the init above so only lpfn is affected */
 	amdgpu_ttm_set_active_vram_size(adev, adev->mc.visible_vram_size);
 

@@ -1203,6 +1205,7 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 	ttm_bo_clean_mm(&adev->mman.bdev, AMDGPU_PL_GWS);
 	ttm_bo_clean_mm(&adev->mman.bdev, AMDGPU_PL_OA);
 	ttm_bo_device_release(&adev->mman.bdev);
+	ttm_io_free_memtype(adev->mc.aper_base, adev->mc.aper_base + adev->mc.aper_size);
 	amdgpu_gart_fini(adev);
 	amdgpu_ttm_global_fini(adev);
 	adev->mman.initialized = false;
Signature · 6 lines
2.5.5

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
#12 lore raw
In reply to Dave Airlie
Quoted 9 lines · Dave Airlie

Okay I spent some time looking into it, and this is the result.

We have to manually insert the VRAM BAR into the pat memory tracking table as WC. The only other way things get inserted are via ioremap, which we never do for the whole VRAM BAR. We could in theory map the VRAM BAR using the iomap stuff that i915 uses, but we don't and this seems easier for now.

We have to fix up at least nouveau and radeon I think as well.

We have something pretty close to this in i915 already, in i915/i915_mm.c. Our idea was to reuse struct io_mapping, since that one already tracks the cache type for us (so that it could do the on-demand kmap stuff properly). Extending that to writing userspace ptes was pretty straightforward. And it has the upshot that it keeps the memtype tracking encapsulated (at least if we'd move the helper from i915_mm.c into core). -Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel