[PATCH 2/3] drm/i915/gvt: Only assign ppgtt root at dispatch time

[Zhang, Xiong Y]

> This moves ppgtt root hook out of scan and shadow function, as it's only
> required at dispatch time. Also make sure this checks against shadow mm to
> be ready, otherwise bail earlier.
[Zhang, Xiong Y] Don't see the benefit for this.
I indeed found a potential issue in our code: intel_vgpu_create_workload() call intel_gvt_scan_and_shadow_workload() which operate shadow_ctx, but shadow_ctx may be used by other engine at this time, this will cause other engine trouble. I see before calling scan_and_shadow, it call mutex_lock(drm.struct_mutex), I don't think this could prevent the issue.

thanks
> 
> Cc: Xiong Zhang <xiong.y.zhang at intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 5b59cba9f93a..e28a3b3513a4 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -346,7 +346,7 @@ static int set_context_ppgtt_from_shadow(struct
> intel_vgpu_workload *workload,
>  	int i = 0;
> 
>  	if (mm->type != INTEL_GVT_MM_PPGTT
> || !mm->ppgtt_mm.shadowed)
> -		return -1;
> +		return -EINVAL;
> 
>  	if (mm->ppgtt_mm.root_entry_type ==
> GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {
>  		px_dma(&ppgtt->pml4) = mm->ppgtt_mm.shadow_pdps[0]; @@
> -410,12 +410,6 @@ int intel_gvt_scan_and_shadow_workload(struct
> intel_vgpu_workload *workload)
>  	if (workload->shadow)
>  		return 0;
> 
> -	ret = set_context_ppgtt_from_shadow(workload, shadow_ctx);
> -	if (ret < 0) {
> -		gvt_vgpu_err("workload shadow ppgtt isn't ready\n");
> -		return ret;
> -	}
> -
>  	/* pin shadow context by gvt even the shadow context will be pinned
>  	 * when i915 alloc request. That is because gvt will update the guest
>  	 * context from shadow context when workload is completed, and at
> that @@ -678,6 +672,8 @@ static int dispatch_workload(struct
> intel_vgpu_workload *workload)  {
>  	struct intel_vgpu *vgpu = workload->vgpu;
>  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> +	struct intel_vgpu_submission *s = &vgpu->submission;
> +	struct i915_gem_context *shadow_ctx = s->shadow_ctx;
>  	struct i915_request *rq;
>  	int ring_id = workload->ring_id;
>  	int ret;
> @@ -688,6 +684,12 @@ static int dispatch_workload(struct
> intel_vgpu_workload *workload)
>  	mutex_lock(&vgpu->vgpu_lock);
>  	mutex_lock(&dev_priv->drm.struct_mutex);
> 
> +	ret = set_context_ppgtt_from_shadow(workload, shadow_ctx);
> +	if (ret < 0) {
> +		gvt_vgpu_err("workload shadow ppgtt isn't ready\n");
> +		goto err_req;
> +	}
> +
>  	ret = intel_gvt_workload_req_alloc(workload);
>  	if (ret)
>  		goto err_req;
> --
> 2.20.1