[Intel-gfx] [PATCH] drm/i915/guc: Dynamically alloc GuC descriptor

[Chris Wilson]

On Thu, Feb 02, 2017 at 07:27:45AM -0800, Oscar Mateo wrote:
> From: Michal Wajdeczko <michal.wajdeczko at intel.com>
> 
> 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 at intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> ---
>  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

-- 
Chris Wilson, Intel Open Source Technology Centre