Commit 0d92a6a4 authored by Dave Gordon's avatar Dave Gordon Committed by Tvrtko Ursulin

drm/i915/guc: keep GuC doorbell & process descriptor mapped in kernel

Don't use kmap_atomic() for doorbell & process descriptor access.
This patch fixes the BUG shown below, where the thread could sleep
while holding a kmap_atomic mapping. In order not to need to call
kmap_atomic() in this code path, we now set up a permanent kernel
mapping of the shared doorbell and process-descriptor page, and
use that in all doorbell and process-descriptor related code.

  BUG: scheduling while atomic: gem_close_race/1941/0x00000002
  Modules linked in: hid_generic usbhid i915 asix usbnet libphy mii
    i2c_algo_bit drm_kms_helper cfbfillrect syscopyarea cfbimgblt
    sysfillrect sysimgblt fb_sys_fops cfbcopyarea drm coretemp i2c_hid
    hid video pinctrl_sunrisepoint pinctrl_intel acpi_pad nls_iso8859_1
    e1000e ptp psmouse pps_core ahci libahci
  CPU: 0 PID: 1941 Comm: gem_close_race Tainted: G     U 4.4.0-160121+ #123
  Hardware name: Intel Corporation Skylake Client platform/Skylake AIO
    DDR3L RVP10, BIOS SKLSE2R1.R00.X100.B01.1509220551 09/22/2015
    0000000000013e40 ffff880166c27a78 ffffffff81280d02 ffff880172c13e40
    ffff880166c27a88 ffffffff810c203a ffff880166c27ac8 ffffffff814ec808
    ffff88016b7c6000 ffff880166c28000 00000000000f4240 0000000000000001
  Call Trace:
    [<ffffffff81280d02>] dump_stack+0x4b/0x79
    [<ffffffff810c203a>] __schedule_bug+0x41/0x4f
    [<ffffffff814ec808>] __schedule+0x5a8/0x690
    [<ffffffff814ec927>] schedule+0x37/0x80
    [<ffffffff814ef3fd>] schedule_hrtimeout_range_clock+0xad/0x130
    [<ffffffff81090be0>] ? hrtimer_init+0x10/0x10
    [<ffffffff814ef3f1>] ?  schedule_hrtimeout_range_clock+0xa1/0x130
    [<ffffffff814ef48e>] schedule_hrtimeout_range+0xe/0x10
    [<ffffffff814eef9b>] usleep_range+0x3b/0x40
    [<ffffffffa01ec109>] i915_guc_wq_check_space+0x119/0x210 [i915]
    [<ffffffffa01da47c>] intel_logical_ring_alloc_request_extras+0x5c/0x70 [i915]
    [<ffffffffa01cdbf1>] i915_gem_request_alloc+0x91/0x170 [i915]
    [<ffffffffa01c1c07>] i915_gem_do_execbuffer.isra.25+0xbc7/0x12a0 [i915]
    [<ffffffffa01cb785>] ?  i915_gem_object_get_pages_gtt+0x225/0x3c0 [i915]
    [<ffffffffa01d1fb6>] ? i915_gem_pwrite_ioctl+0xd6/0x9f0 [i915]
    [<ffffffffa01c2e68>] i915_gem_execbuffer2+0xa8/0x250 [i915]
    [<ffffffffa00f65d8>] drm_ioctl+0x258/0x4f0 [drm]
    [<ffffffffa01c2dc0>] ? i915_gem_execbuffer+0x340/0x340 [i915]
    [<ffffffff8111590d>] do_vfs_ioctl+0x2cd/0x4a0
    [<ffffffff8111eac2>] ? __fget+0x72/0xb0
    [<ffffffff81115b1c>] SyS_ioctl+0x3c/0x70
    [<ffffffff814effd7>] entry_SYSCALL_64_fastpath+0x12/0x6a
  ------------[ cut here ]------------

v4:
  Only tear down doorbell & kunmap() client object if we actually
  succeeded in allocating a client object (Tvrtko Ursulin)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93847Original-version-by: default avatarAlex Dai <yu.dai@intel.com>
Signed-off-by: default avatarDave Gordon <david.s.gordon@intel.com>
Cc: Tvtrko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
parent 1bec9b0b
...@@ -179,15 +179,11 @@ static void guc_init_doorbell(struct intel_guc *guc, ...@@ -179,15 +179,11 @@ static void guc_init_doorbell(struct intel_guc *guc,
struct i915_guc_client *client) struct i915_guc_client *client)
{ {
struct guc_doorbell_info *doorbell; struct guc_doorbell_info *doorbell;
void *base;
base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0)); doorbell = client->client_base + client->doorbell_offset;
doorbell = base + client->doorbell_offset;
doorbell->db_status = 1; doorbell->db_status = GUC_DOORBELL_ENABLED;
doorbell->cookie = 0; doorbell->cookie = 0;
kunmap_atomic(base);
} }
static int guc_ring_doorbell(struct i915_guc_client *gc) static int guc_ring_doorbell(struct i915_guc_client *gc)
...@@ -195,11 +191,9 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) ...@@ -195,11 +191,9 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
struct guc_process_desc *desc; struct guc_process_desc *desc;
union guc_doorbell_qw db_cmp, db_exc, db_ret; union guc_doorbell_qw db_cmp, db_exc, db_ret;
union guc_doorbell_qw *db; union guc_doorbell_qw *db;
void *base;
int attempt = 2, ret = -EAGAIN; int attempt = 2, ret = -EAGAIN;
base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0)); desc = gc->client_base + gc->proc_desc_offset;
desc = base + gc->proc_desc_offset;
/* Update the tail so it is visible to GuC */ /* Update the tail so it is visible to GuC */
desc->tail = gc->wq_tail; desc->tail = gc->wq_tail;
...@@ -215,7 +209,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) ...@@ -215,7 +209,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
db_exc.cookie = 1; db_exc.cookie = 1;
/* pointer of current doorbell cacheline */ /* pointer of current doorbell cacheline */
db = base + gc->doorbell_offset; db = gc->client_base + gc->doorbell_offset;
while (attempt--) { while (attempt--) {
/* lets ring the doorbell */ /* lets ring the doorbell */
...@@ -247,7 +241,6 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) ...@@ -247,7 +241,6 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
/* Finally, update the cached copy of the GuC's WQ head */ /* Finally, update the cached copy of the GuC's WQ head */
gc->wq_head = desc->head; gc->wq_head = desc->head;
kunmap_atomic(base);
return ret; return ret;
} }
...@@ -256,16 +249,12 @@ static void guc_disable_doorbell(struct intel_guc *guc, ...@@ -256,16 +249,12 @@ static void guc_disable_doorbell(struct intel_guc *guc,
{ {
struct drm_i915_private *dev_priv = guc_to_i915(guc); struct drm_i915_private *dev_priv = guc_to_i915(guc);
struct guc_doorbell_info *doorbell; struct guc_doorbell_info *doorbell;
void *base;
i915_reg_t drbreg = GEN8_DRBREGL(client->doorbell_id); i915_reg_t drbreg = GEN8_DRBREGL(client->doorbell_id);
int value; int value;
base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0)); doorbell = client->client_base + client->doorbell_offset;
doorbell = base + client->doorbell_offset;
doorbell->db_status = 0; doorbell->db_status = GUC_DOORBELL_DISABLED;
kunmap_atomic(base);
I915_WRITE(drbreg, I915_READ(drbreg) & ~GEN8_DRB_VALID); I915_WRITE(drbreg, I915_READ(drbreg) & ~GEN8_DRB_VALID);
...@@ -341,10 +330,8 @@ static void guc_init_proc_desc(struct intel_guc *guc, ...@@ -341,10 +330,8 @@ static void guc_init_proc_desc(struct intel_guc *guc,
struct i915_guc_client *client) struct i915_guc_client *client)
{ {
struct guc_process_desc *desc; struct guc_process_desc *desc;
void *base;
base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0)); desc = client->client_base + client->proc_desc_offset;
desc = base + client->proc_desc_offset;
memset(desc, 0, sizeof(*desc)); memset(desc, 0, sizeof(*desc));
...@@ -361,8 +348,6 @@ static void guc_init_proc_desc(struct intel_guc *guc, ...@@ -361,8 +348,6 @@ static void guc_init_proc_desc(struct intel_guc *guc,
desc->wq_size_bytes = client->wq_size; desc->wq_size_bytes = client->wq_size;
desc->wq_status = WQ_STATUS_ACTIVE; desc->wq_status = WQ_STATUS_ACTIVE;
desc->priority = client->priority; desc->priority = client->priority;
kunmap_atomic(base);
} }
/* /*
...@@ -474,7 +459,6 @@ static void guc_fini_ctx_desc(struct intel_guc *guc, ...@@ -474,7 +459,6 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
int i915_guc_wq_check_space(struct i915_guc_client *gc) int i915_guc_wq_check_space(struct i915_guc_client *gc)
{ {
struct guc_process_desc *desc; struct guc_process_desc *desc;
void *base;
u32 size = sizeof(struct guc_wq_item); u32 size = sizeof(struct guc_wq_item);
int ret = -ETIMEDOUT, timeout_counter = 200; int ret = -ETIMEDOUT, timeout_counter = 200;
...@@ -486,8 +470,7 @@ int i915_guc_wq_check_space(struct i915_guc_client *gc) ...@@ -486,8 +470,7 @@ int i915_guc_wq_check_space(struct i915_guc_client *gc)
if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size) if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size)
return 0; return 0;
base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0)); desc = gc->client_base + gc->proc_desc_offset;
desc = base + gc->proc_desc_offset;
while (timeout_counter-- > 0) { while (timeout_counter-- > 0) {
gc->wq_head = desc->head; gc->wq_head = desc->head;
...@@ -501,8 +484,6 @@ int i915_guc_wq_check_space(struct i915_guc_client *gc) ...@@ -501,8 +484,6 @@ int i915_guc_wq_check_space(struct i915_guc_client *gc)
usleep_range(1000, 2000); usleep_range(1000, 2000);
}; };
kunmap_atomic(base);
return ret; return ret;
} }
...@@ -661,21 +642,28 @@ static void guc_client_free(struct drm_device *dev, ...@@ -661,21 +642,28 @@ static void guc_client_free(struct drm_device *dev,
if (!client) if (!client)
return; return;
if (client->doorbell_id != GUC_INVALID_DOORBELL_ID) {
/*
* First disable the doorbell, then tell the GuC we've
* finished with it, finally deallocate it in our bitmap
*/
guc_disable_doorbell(guc, client);
host2guc_release_doorbell(guc, client);
release_doorbell(guc, client->doorbell_id);
}
/* /*
* XXX: wait for any outstanding submissions before freeing memory. * XXX: wait for any outstanding submissions before freeing memory.
* Be sure to drop any locks * Be sure to drop any locks
*/ */
if (client->client_base) {
/*
* If we got as far as setting up a doorbell, make sure
* we shut it down before unmapping & deallocating the
* memory. So first disable the doorbell, then tell the
* GuC that we've finished with it, finally deallocate
* it in our bitmap
*/
if (client->doorbell_id != GUC_INVALID_DOORBELL_ID) {
guc_disable_doorbell(guc, client);
host2guc_release_doorbell(guc, client);
release_doorbell(guc, client->doorbell_id);
}
kunmap(kmap_to_page(client->client_base));
}
gem_release_guc_obj(client->client_obj); gem_release_guc_obj(client->client_obj);
if (client->ctx_index != GUC_INVALID_CTX_ID) { if (client->ctx_index != GUC_INVALID_CTX_ID) {
...@@ -696,7 +684,7 @@ static void guc_client_free(struct drm_device *dev, ...@@ -696,7 +684,7 @@ static void guc_client_free(struct drm_device *dev,
* @ctx: the context that owns the client (we use the default render * @ctx: the context that owns the client (we use the default render
* context) * context)
* *
* Return: An i915_guc_client object if success. * Return: An i915_guc_client object if success, else NULL.
*/ */
static struct i915_guc_client *guc_client_alloc(struct drm_device *dev, static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
uint32_t priority, uint32_t priority,
...@@ -728,7 +716,9 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev, ...@@ -728,7 +716,9 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
if (!obj) if (!obj)
goto err; goto err;
/* We'll keep just the first (doorbell/proc) page permanently kmap'd. */
client->client_obj = obj; client->client_obj = obj;
client->client_base = kmap(i915_gem_object_get_page(obj, 0));
client->wq_offset = GUC_DB_SIZE; client->wq_offset = GUC_DB_SIZE;
client->wq_size = GUC_WQ_SIZE; client->wq_size = GUC_WQ_SIZE;
......
...@@ -31,6 +31,7 @@ struct drm_i915_gem_request; ...@@ -31,6 +31,7 @@ struct drm_i915_gem_request;
struct i915_guc_client { struct i915_guc_client {
struct drm_i915_gem_object *client_obj; struct drm_i915_gem_object *client_obj;
void *client_base; /* first page (only) of above */
struct intel_context *owner; struct intel_context *owner;
struct intel_guc *guc; struct intel_guc *guc;
uint32_t priority; uint32_t priority;
...@@ -52,6 +53,7 @@ struct i915_guc_client { ...@@ -52,6 +53,7 @@ struct i915_guc_client {
uint32_t q_fail; uint32_t q_fail;
uint32_t b_fail; uint32_t b_fail;
int retcode; int retcode;
int spare; /* pad to 32 DWords */
}; };
enum intel_guc_fw_status { enum intel_guc_fw_status {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment