Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: Don't try to create log runtime if there is no log

2018-02-01 Thread Chris Wilson 
Quoting Michal Wajdeczko (2018-01-31 17:32:39)
> In case of GuC initialization failure we may continue with driver
> load, but we wrongly assume that GuC is fully functional. This
> leads to the BUG as we attempt to access non-existing log vma.
> 
> [26386.121085] BUG: unable to handle kernel NULL pointer dereference at 
> 00a0
> [26386.121225] IP: guc_log_runtime_create+0x23/0xe0 [i915]
> [26386.121763] Call Trace:
> [26386.121870]  guc_log_late_setup+0xfd/0x140 [i915]
> [26386.121969]  i915_driver_load+0x7ab/0x1730 [i915]
> [26386.122069]  i915_pci_probe+0x2d/0x90 [i915]
> [26386.122089]  pci_device_probe+0x9c/0x120
> [26386.122107]  driver_probe_device+0x2a9/0x490
> [26386.122126]  __driver_attach+0xd9/0xe0
> [26386.122143]  ? driver_probe_device+0x490/0x490
> [26386.122158]  bus_for_each_dev+0x57/0x90
> [26386.122175]  bus_add_driver+0x1eb/0x260
> [26386.122190]  ? 0xa069a000
> [26386.122206]  driver_register+0x52/0xc0
> [26386.10]  ? 0xa069a000
> [26386.122234]  do_one_initcall+0x39/0x170
> [26386.122252]  ? kmem_cache_alloc_trace+0x1fd/0x2e0
> [26386.122273]  do_init_module+0x56/0x1ec
> [26386.122289]  load_module+0x219e/0x2550
> [26386.122309]  ? vfs_read+0x121/0x140
> [26386.122331]  ? SyS_finit_module+0xa5/0xe0
> [26386.122346]  SyS_finit_module+0xa5/0xe0
> [26386.122371]  entry_SYSCALL_64_fastpath+0x22/0x8f
> 
> Signed-off-by: Michal Wajdeczko 
> Cc: Chris Wilson 
> Cc: Michal Winiarski 
> Cc: Daniele Ceraolo Spurio 
> Cc: Sagar Arun Kamble 
> ---
>  drivers/gpu/drm/i915/intel_guc_log.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
> b/drivers/gpu/drm/i915/intel_guc_log.c
> index 3fbe93a..fdb1895 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -395,6 +395,9 @@ static int guc_log_runtime_create(struct intel_guc *guc)
> void *vaddr;
> int ret;
>  
> +   if (!guc->log.vma)
> +   return -ENODEV;
> +
> lockdep_assert_held(_priv->drm.struct_mutex);

We like to keep the caller precondition guards at the start of the
function, so I reversed this pair of lines.

I've pushed the two BUG fixes that Sagar reviewed. The fault injection
is a good idea, but I've left it alone while there is a question on a
couple of the patches. Thanks for the patches and review, looking
forward to the next series :)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: Don't try to create log runtime if there is no log

2018-02-01 Thread Sagar Arun Kamble 



On 1/31/2018 11:02 PM, Michal Wajdeczko wrote:

In case of GuC initialization failure we may continue with driver
load, but we wrongly assume that GuC is fully functional. This
leads to the BUG as we attempt to access non-existing log vma.

[26386.121085] BUG: unable to handle kernel NULL pointer dereference at 
00a0
[26386.121225] IP: guc_log_runtime_create+0x23/0xe0 [i915]
[26386.121763] Call Trace:
[26386.121870]  guc_log_late_setup+0xfd/0x140 [i915]
[26386.121969]  i915_driver_load+0x7ab/0x1730 [i915]
[26386.122069]  i915_pci_probe+0x2d/0x90 [i915]
[26386.122089]  pci_device_probe+0x9c/0x120
[26386.122107]  driver_probe_device+0x2a9/0x490
[26386.122126]  __driver_attach+0xd9/0xe0
[26386.122143]  ? driver_probe_device+0x490/0x490
[26386.122158]  bus_for_each_dev+0x57/0x90
[26386.122175]  bus_add_driver+0x1eb/0x260
[26386.122190]  ? 0xa069a000
[26386.122206]  driver_register+0x52/0xc0
[26386.10]  ? 0xa069a000
[26386.122234]  do_one_initcall+0x39/0x170
[26386.122252]  ? kmem_cache_alloc_trace+0x1fd/0x2e0
[26386.122273]  do_init_module+0x56/0x1ec
[26386.122289]  load_module+0x219e/0x2550
[26386.122309]  ? vfs_read+0x121/0x140
[26386.122331]  ? SyS_finit_module+0xa5/0xe0
[26386.122346]  SyS_finit_module+0xa5/0xe0
[26386.122371]  entry_SYSCALL_64_fastpath+0x22/0x8f

Signed-off-by: Michal Wajdeczko 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 

Reviewed-by: Sagar Arun Kamble 

---
  drivers/gpu/drm/i915/intel_guc_log.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
b/drivers/gpu/drm/i915/intel_guc_log.c
index 3fbe93a..fdb1895 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -395,6 +395,9 @@ static int guc_log_runtime_create(struct intel_guc *guc)
void *vaddr;
int ret;
  
+	if (!guc->log.vma)

+   return -ENODEV;
+
lockdep_assert_held(_priv->drm.struct_mutex);
  
  	GEM_BUG_ON(guc_log_has_runtime(guc));


--
Thanks,
Sagar

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/5] drm/i915/guc: Don't try to create log runtime if there is no log

2018-01-31 Thread Michal Wajdeczko 
In case of GuC initialization failure we may continue with driver
load, but we wrongly assume that GuC is fully functional. This
leads to the BUG as we attempt to access non-existing log vma.

[26386.121085] BUG: unable to handle kernel NULL pointer dereference at 
00a0
[26386.121225] IP: guc_log_runtime_create+0x23/0xe0 [i915]
[26386.121763] Call Trace:
[26386.121870]  guc_log_late_setup+0xfd/0x140 [i915]
[26386.121969]  i915_driver_load+0x7ab/0x1730 [i915]
[26386.122069]  i915_pci_probe+0x2d/0x90 [i915]
[26386.122089]  pci_device_probe+0x9c/0x120
[26386.122107]  driver_probe_device+0x2a9/0x490
[26386.122126]  __driver_attach+0xd9/0xe0
[26386.122143]  ? driver_probe_device+0x490/0x490
[26386.122158]  bus_for_each_dev+0x57/0x90
[26386.122175]  bus_add_driver+0x1eb/0x260
[26386.122190]  ? 0xa069a000
[26386.122206]  driver_register+0x52/0xc0
[26386.10]  ? 0xa069a000
[26386.122234]  do_one_initcall+0x39/0x170
[26386.122252]  ? kmem_cache_alloc_trace+0x1fd/0x2e0
[26386.122273]  do_init_module+0x56/0x1ec
[26386.122289]  load_module+0x219e/0x2550
[26386.122309]  ? vfs_read+0x121/0x140
[26386.122331]  ? SyS_finit_module+0xa5/0xe0
[26386.122346]  SyS_finit_module+0xa5/0xe0
[26386.122371]  entry_SYSCALL_64_fastpath+0x22/0x8f

Signed-off-by: Michal Wajdeczko 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/intel_guc_log.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
b/drivers/gpu/drm/i915/intel_guc_log.c
index 3fbe93a..fdb1895 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -395,6 +395,9 @@ static int guc_log_runtime_create(struct intel_guc *guc)
void *vaddr;
int ret;
 
+   if (!guc->log.vma)
+   return -ENODEV;
+
lockdep_assert_held(_priv->drm.struct_mutex);
 
GEM_BUG_ON(guc_log_has_runtime(guc));
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx