[PATCH 0/3] Fix S3 entry crashes

[PATCH 0/3] Fix S3 entry crashes

[Pratik Vishwakarma]

This patch series resolves crashes observed during S3 entry.
First patch removes the cursor BO hack which causes crashes
mentioned in patches 2 and 3. Patches 2 and 3 add NULL
checks to prevent crashing the system.

Pratik Vishwakarma (3):
  drm/amd/display: Remove cursor hack for S3
  drm/amd/amdgpu: Fix crash in amdgpu_bo_reserve
  drm/amd/amdgpu: Fix NULL pointer OOPS during S3

 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h        |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c           | 17 +++++++++++++----
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 -----------
 3 files changed, 15 insertions(+), 15 deletions(-)

-- 
1.9.1

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

[PATCH 1/3] drm/amd/display: remove cursor hack for S3

[Pratik Vishwakarma]

cursor_bo operations cause crash during S3 entry.

On cursor movement between displays during S3 cycles
the system crashes on S3 entry.

These crashes are no more seen with this patch applied.

Also as per the comment just above the code that this
patch removes
"IN 4.10 kernel this code should be removed and
amdgpu_device_suspend code touching fram buffers
should be avoided for DC"

Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a2009d5..8c31abf 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3138,17 +3138,6 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
 		}
 	}
 
-	/* It's a hack for s3 since in 4.9 kernel filter out cursor buffer
-	 * prepare and cleanup in drm_atomic_helper_prepare_planes
-	 * and drm_atomic_helper_cleanup_planes because fb doens't in s3.
-	 * IN 4.10 kernel this code should be removed and amdgpu_device_suspend
-	 * code touching fram buffers should be avoided for DC.
-	 */
-	if (plane->type == DRM_PLANE_TYPE_CURSOR) {
-		struct amdgpu_crtc *acrtc = to_amdgpu_crtc(new_state->crtc);
-
-		acrtc->cursor_bo = obj;
-	}
 	return 0;
 }
 
-- 
1.9.1

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

[PATCH 2/3] drm/amd/amdgpu: Fix crash in amdgpu_bo_reserve

[Pratik Vishwakarma]

Fixes null pointer access in ww_mutex_lock
where lock->base is NULL

Crash dump is as follows:
Call Trace:
ww_mutex_lock+0x3a/0x8e
amdgpu_bo_reserve+0x40/0x87
amdgpu_device_suspend+0xf4/0x210
pci_pm_suspend+0x12a/0x1a5
? pci_dev_driver+0x36/0x36
dpm_run_callback+0x59/0xbf
__device_suspend+0x215/0x33f
async_suspend+0x1f/0x5c
async_run_entry_fn+0x3d/0xd2
process_one_work+0x1b0/0x314
worker_thread+0x1cb/0x2c1
? create_worker+0x1da/0x1da
kthread+0x156/0x15e
? kthread_flush_work+0xea/0xea
ret_from_fork+0x22/0x40

Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 7317480..c9df7ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -152,6 +152,8 @@ static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo, bool no_intr)
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 	int r;
 
+	if (&(bo->tbo.resv->lock) == NULL)
+		return -EINVAL;
 	r = ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
 	if (unlikely(r != 0)) {
 		if (r != -ERESTARTSYS)
-- 
1.9.1

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

[PATCH 3/3] drm/amd/amdgpu: Fix NULL pointer OOPS during S3

[Pratik Vishwakarma]

Fixes NULL pointer dereference in amdgpu_ttm_copy_mem_to_mem

BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
IP: amdgpu_ttm_copy_mem_to_mem+0x85/0x40c
Workqueue: events_unbound async_run_entry_fn
Call Trace:
? _raw_spin_unlock+0xe/0x20
? ttm_check_swapping+0x4e/0x72
? ttm_mem_global_reserve.constprop.4+0xb1/0xc0
amdgpu_move_blit+0x80/0xe2
amdgpu_bo_move+0x114/0x155
ttm_bo_handle_move_mem+0x1f7/0x34a
? ttm_bo_mem_space+0x162/0x38e
? dev_vprintk_emit+0x10a/0x1f2
ttm_bo_evict+0x13e/0x2e9
? do_wait_for_common+0x151/0x187
ttm_mem_evict_first+0x136/0x181
ttm_bo_force_list_clean+0x78/0x10f
amdgpu_device_suspend+0x167/0x210
pci_pm_suspend+0x12a/0x1a5
? pci_dev_driver+0x36/0x36
dpm_run_callback+0x59/0xbf
__device_suspend+0x215/0x33f
async_suspend+0x1f/0x5c
async_run_entry_fn+0x3d/0xd2
process_one_work+0x1b0/0x314
worker_thread+0x1cb/0x2c1
? create_worker+0x1da/0x1da
kthread+0x156/0x15e
? kthread_flush_work+0xea/0xea
ret_from_fork+0x22/0x40

Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 57d4da6..f9de429 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -414,12 +414,16 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 		return -EINVAL;
 	}
 
+	if (!src->mem->mm_node)
+		return -EINVAL;
 	src_mm = amdgpu_find_mm_node(src->mem, &src->offset);
 	src_node_start = amdgpu_mm_node_addr(src->bo, src_mm, src->mem) +
 					     src->offset;
 	src_node_size = (src_mm->size << PAGE_SHIFT) - src->offset;
 	src_page_offset = src_node_start & (PAGE_SIZE - 1);
 
+	if (!dst->mem->mm_node)
+		return -EINVAL;
 	dst_mm = amdgpu_find_mm_node(dst->mem, &dst->offset);
 	dst_node_start = amdgpu_mm_node_addr(dst->bo, dst_mm, dst->mem) +
 					     dst->offset;
-- 
1.9.1

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

Re: [PATCH 2/3] drm/amd/amdgpu: Fix crash in amdgpu_bo_reserve

[Christian König]

NAK, when bo->tbo.resv is NULL then the BO is corrupted (or already 
released).

Please find the root cause of that corruption or freed memory access 
instead of adding such crude workarounds.

Regards,
Christian.

Am 06.06.2018 um 11:25 schrieb Pratik Vishwakarma:
> Fixes null pointer access in ww_mutex_lock
> where lock->base is NULL
>
> Crash dump is as follows:
> Call Trace:
> ww_mutex_lock+0x3a/0x8e
> amdgpu_bo_reserve+0x40/0x87
> amdgpu_device_suspend+0xf4/0x210
> pci_pm_suspend+0x12a/0x1a5
> ? pci_dev_driver+0x36/0x36
> dpm_run_callback+0x59/0xbf
> __device_suspend+0x215/0x33f
> async_suspend+0x1f/0x5c
> async_run_entry_fn+0x3d/0xd2
> process_one_work+0x1b0/0x314
> worker_thread+0x1cb/0x2c1
> ? create_worker+0x1da/0x1da
> kthread+0x156/0x15e
> ? kthread_flush_work+0xea/0xea
> ret_from_fork+0x22/0x40
>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 7317480..c9df7ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -152,6 +152,8 @@ static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo, bool no_intr)
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>   	int r;
>   
> +	if (&(bo->tbo.resv->lock) == NULL)
> +		return -EINVAL;
>   	r = ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
>   	if (unlikely(r != 0)) {
>   		if (r != -ERESTARTSYS)

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

Re: [PATCH 3/3] drm/amd/amdgpu: Fix NULL pointer OOPS during S3

[Christian König]

NAK as well. mem->mm_node can't be NULL on a correctly allocated BO.

You are running into a BO corruption here and trying to work around by 
mitigating the effect and not fixing the root problem.

Regards,
Christian.

Am 06.06.2018 um 11:25 schrieb Pratik Vishwakarma:
> Fixes NULL pointer dereference in amdgpu_ttm_copy_mem_to_mem
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> IP: amdgpu_ttm_copy_mem_to_mem+0x85/0x40c
> Workqueue: events_unbound async_run_entry_fn
> Call Trace:
> ? _raw_spin_unlock+0xe/0x20
> ? ttm_check_swapping+0x4e/0x72
> ? ttm_mem_global_reserve.constprop.4+0xb1/0xc0
> amdgpu_move_blit+0x80/0xe2
> amdgpu_bo_move+0x114/0x155
> ttm_bo_handle_move_mem+0x1f7/0x34a
> ? ttm_bo_mem_space+0x162/0x38e
> ? dev_vprintk_emit+0x10a/0x1f2
> ttm_bo_evict+0x13e/0x2e9
> ? do_wait_for_common+0x151/0x187
> ttm_mem_evict_first+0x136/0x181
> ttm_bo_force_list_clean+0x78/0x10f
> amdgpu_device_suspend+0x167/0x210
> pci_pm_suspend+0x12a/0x1a5
> ? pci_dev_driver+0x36/0x36
> dpm_run_callback+0x59/0xbf
> __device_suspend+0x215/0x33f
> async_suspend+0x1f/0x5c
> async_run_entry_fn+0x3d/0xd2
> process_one_work+0x1b0/0x314
> worker_thread+0x1cb/0x2c1
> ? create_worker+0x1da/0x1da
> kthread+0x156/0x15e
> ? kthread_flush_work+0xea/0xea
> ret_from_fork+0x22/0x40
>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 57d4da6..f9de429 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -414,12 +414,16 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>   		return -EINVAL;
>   	}
>   
> +	if (!src->mem->mm_node)
> +		return -EINVAL;
>   	src_mm = amdgpu_find_mm_node(src->mem, &src->offset);
>   	src_node_start = amdgpu_mm_node_addr(src->bo, src_mm, src->mem) +
>   					     src->offset;
>   	src_node_size = (src_mm->size << PAGE_SHIFT) - src->offset;
>   	src_page_offset = src_node_start & (PAGE_SIZE - 1);
>   
> +	if (!dst->mem->mm_node)
> +		return -EINVAL;
>   	dst_mm = amdgpu_find_mm_node(dst->mem, &dst->offset);
>   	dst_node_start = amdgpu_mm_node_addr(dst->bo, dst_mm, dst->mem) +
>   					     dst->offset;

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

RE: [PATCH 2/3] drm/amd/amdgpu: Fix crash in amdgpu_bo_reserve

[Vishwakarma, Pratik]

Ok. Abandoning the series.
I am working on identifying the root cause and will post the new patch soon.

Regards
Pratik

-----Original Message-----
From: Koenig, Christian 
Sent: Wednesday, June 6, 2018 3:17 PM
To: Vishwakarma, Pratik <Pratik.Vishwakarma@amd.com>; amd-gfx@lists.freedesktop.org; alexdeucher@gmail.com; Wentland, Harry <Harry.Wentland@amd.com>
Subject: Re: [PATCH 2/3] drm/amd/amdgpu: Fix crash in amdgpu_bo_reserve

NAK, when bo->tbo.resv is NULL then the BO is corrupted (or already released).

Please find the root cause of that corruption or freed memory access instead of adding such crude workarounds.

Regards,
Christian.

Am 06.06.2018 um 11:25 schrieb Pratik Vishwakarma:
> Fixes null pointer access in ww_mutex_lock where lock->base is NULL
>
> Crash dump is as follows:
> Call Trace:
> ww_mutex_lock+0x3a/0x8e
> amdgpu_bo_reserve+0x40/0x87
> amdgpu_device_suspend+0xf4/0x210
> pci_pm_suspend+0x12a/0x1a5
> ? pci_dev_driver+0x36/0x36
> dpm_run_callback+0x59/0xbf
> __device_suspend+0x215/0x33f
> async_suspend+0x1f/0x5c
> async_run_entry_fn+0x3d/0xd2
> process_one_work+0x1b0/0x314
> worker_thread+0x1cb/0x2c1
> ? create_worker+0x1da/0x1da
> kthread+0x156/0x15e
> ? kthread_flush_work+0xea/0xea
> ret_from_fork+0x22/0x40
>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 7317480..c9df7ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -152,6 +152,8 @@ static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo, bool no_intr)
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>   	int r;
>   
> +	if (&(bo->tbo.resv->lock) == NULL)
> +		return -EINVAL;
>   	r = ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
>   	if (unlikely(r != 0)) {
>   		if (r != -ERESTARTSYS)

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