[rfc] fix for regression in pat memory tracking in 4.9-rc1

[rfc] fix for regression in pat memory tracking in 4.9-rc1

[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.

Dave.

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

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 <airlied@redhat.com>
---
 arch/x86/mm/pat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index 170cc4f..5ce2fbb 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -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)
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Edward O'Callaghan]

[-- Attachment #1.1.1: Type: text/plain, Size: 1354 bytes --]

NACK,

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

Kind Regards,
Edward.

On 10/18/2016 05:13 PM, Dave Airlie wrote:
> From: Dave Airlie <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> 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 <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/x86/mm/pat.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index 170cc4f..5ce2fbb 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -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)
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Dave Airlie]

[-- Attachment #1.1: Type: text/plain, Size: 1569 bytes --]

On 18 Oct. 2016 17:23, "Edward O'Callaghan" <funfunctor-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>
wrote:
>
> NACK,
>
> I think you want to use 'iomap_create_wc()' instead to avoid aliasing.

Please explain what can alias here?

Dave.

>
> Kind Regards,
> Edward.
>
> On 10/18/2016 05:13 PM, Dave Airlie wrote:
> > From: Dave Airlie <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >
> > 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 <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  arch/x86/mm/pat.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> > index 170cc4f..5ce2fbb 100644
> > --- a/arch/x86/mm/pat.c
> > +++ b/arch/x86/mm/pat.c
> > @@ -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)
> >
>

[-- Attachment #1.2: Type: text/html, Size: 2366 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Edward O'Callaghan]

[-- Attachment #1.1.1: Type: text/plain, Size: 2203 bytes --]



On 10/18/2016 07:29 PM, Dave Airlie wrote:
> On 18 Oct. 2016 17:23, "Edward O'Callaghan" <funfunctor-dczkZgxz+BNOTv0TxJ7zrQ@public.gmane.orgt
> <mailto:funfunctor-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>> wrote:
>>
>> 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.

> 
> Dave.
> 
>>
>> Kind Regards,
>> Edward.
>>
>> On 10/18/2016 05:13 PM, Dave Airlie wrote:
>> > From: Dave Airlie <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org <mailto:airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>>
>> >
>> > 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 <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> <mailto:airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>>
>> > ---
>> >  arch/x86/mm/pat.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
>> > index 170cc4f..5ce2fbb 100644
>> > --- a/arch/x86/mm/pat.c
>> > +++ b/arch/x86/mm/pat.c
>> > @@ -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)
>> >
>>
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Shawn Starr]

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

Tested by: Shawn Starr <shawn.starr@rogers.com>

Thanks,
Shawn

On Tuesday, October 18, 2016 4:13:11 PM EDT Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> 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 <airlied@redhat.com>
> ---
>  arch/x86/mm/pat.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index 170cc4f..5ce2fbb 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -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)


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

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

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 24 ++++++++++++++++++++++++
 include/drm/ttm/ttm_bo_driver.h   |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index bf6e216..192c003 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -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);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4f0a921..6b2d24e 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -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)
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 887483b..3142d70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -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;
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [rfc] fix for regression in pat memory tracking in 4.9-rc1

[Daniel Vetter]

On Tue, Oct 18, 2016 at 04:13:10PM +1000, Dave Airlie wrote:
> 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel