On 02/02/2017 11:33 PM, Chris Wilson wrote:
On Thu, Feb 02, 2017 at 07:27:45AM -0800, Oscar Mateo wrote:
From: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
The GuC descriptor is big in size. If we use local definition of
guc_desc we have a chance to overflow stack. Use allocated one.
v2: Rebased
v3: Split
v4: Handle ENOMEM, cover all uses of guc_context_desc, use kzalloc (Oscar)
Signed-off-by: Deepak S <deepak.s@xxxxxxxxx>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 94 ++++++++++++++++++------------
1 file changed, 57 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 8ced9e2..b4f14f3 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -102,9 +102,13 @@ static int guc_update_doorbell_id(struct intel_guc *guc,
struct sg_table *sg = guc->ctx_pool_vma->pages;
void *doorbell_bitmap = guc->doorbell_bitmap;
struct guc_doorbell_info *doorbell;
- struct guc_context_desc desc;
+ struct guc_context_desc *desc;
size_t len;
+ desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+ if (!desc)
+ return -ENOMEM;
+
doorbell = client->vaddr + client->doorbell_offset;
if (client->doorbell_id != GUC_INVALID_DOORBELL_ID &&
@@ -116,15 +120,22 @@ static int guc_update_doorbell_id(struct intel_guc *guc,
}
/* Update the GuC's idea of the doorbell ID */
- len = sg_pcopy_to_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
- sizeof(desc) * client->ctx_index);
- if (len != sizeof(desc))
+ len = sg_pcopy_to_buffer(sg->sgl, sg->nents, desc, sizeof(*desc),
+ sizeof(*desc) * client->ctx_index);
This is silly. You are creating a pointer using kmalloc to copy into a
pointer created using alloc_page. Just write directly into the backing
store.
-Chris
I guess I deserve this for not digging any deeper. From what I can see,
the backing store is an array of 1024 context descriptors. If the whole
context descriptor fell in one page, I could kmap_atomic only that. As
it is, I would need to vmap a couple of pages to make sure I always get
a complete pointer to guc_context_desc. Would you be happy with that?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
