Re: [PATCH] drm/i915: Add ppgtt to GVT GEM context

[Zhenyu Wang <zhenyuw@linux.intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4340 bytes --]

On 2018.10.18 13:40:31 +0800, Xiong Zhang wrote:
> Currently the guest couldn't boot up under GVT-g environment as the
> following call trace exists:
> [  272.504762] BUG: unable to handle kernel NULL pointer dereference at 0000000000000100
> [  272.504834] Call Trace:
> [  272.504852]  execlists_context_pin+0x2b2/0x520 [i915]
> [  272.504869]  intel_gvt_scan_and_shadow_workload+0x50/0x4d0 [i915]
> [  272.504887]  intel_vgpu_create_workload+0x3e2/0x570 [i915]
> [  272.504901]  intel_vgpu_submit_execlist+0xc0/0x2a0 [i915]
> [  272.504916]  elsp_mmio_write+0xc7/0x130 [i915]
> [  272.504930]  intel_vgpu_mmio_reg_rw+0x24a/0x4c0 [i915]
> [  272.504944]  intel_vgpu_emulate_mmio_write+0xac/0x240 [i915]
> [  272.504947]  intel_vgpu_rw+0x22d/0x270 [kvmgt]
> [  272.504949]  intel_vgpu_write+0x164/0x1f0 [kvmgt]
> 
> GVT GEM context is created by i915_gem_context_create_gvt() which
> doesn't allocate ppgtt. So GVT GEM context structure doesn't have
> a valid i915_hw_ppgtt.
> 
> This patch create ppgtt table at GVT GEM context creation, then assign
> shadow ppgtt's root table address to this ppgtt when shadow ppgtt will
> be used on GPU. So GVT GEM context has valid ppgtt address. But note
> that this ppgtt only contain valid ppgtt root table address, the table
> entry in this ppgtt structure are invalid.
> 
> Fixes:4a3d3f6785be("drm/i915: Match code to comment and enforce ppgtt for execlists")
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>

Any more comment for this? We need it for current gvt broken on drm-tip,
and it requires to change i915 for gvt ppgtt allocation, so I assume it's
better to be merged by i915 directly, or do you like a gvt pull instead?

Thanks.

> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c    | 29 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index ea34003..b7e0529 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -334,6 +334,29 @@ static void release_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx)
>  	i915_gem_object_put(wa_ctx->indirect_ctx.obj);
>  }
>  
> +static int set_context_ppgtt_from_shadow(struct intel_vgpu_workload *workload,
> +					 struct i915_gem_context *ctx)
> +{
> +	struct intel_vgpu_mm *mm = workload->shadow_mm;
> +	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
> +	int i = 0;
> +
> +	if (mm->type != INTEL_GVT_MM_PPGTT ||
> +	    !mm->ppgtt_mm.shadowed)
> +		return -1;
> +
> +	if (mm->ppgtt_mm.root_entry_type == GTT_TYPE_PPGTT_ROOT_L4_ENTRY)
> +		px_dma(&ppgtt->pml4) = mm->ppgtt_mm.shadow_pdps[0];
> +	else {
> +		for (i = 0; i < GVT_RING_CTX_NR_PDPS; i++) {
> +			px_dma(ppgtt->pdp.page_directory[i]) =
> +				mm->ppgtt_mm.shadow_pdps[i];
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * intel_gvt_scan_and_shadow_workload - audit the workload by scanning and
>   * shadow it as well, include ringbuffer,wa_ctx and ctx.
> @@ -358,6 +381,12 @@ int intel_gvt_scan_and_shadow_workload(struct intel_vgpu_workload *workload)
>  	if (workload->req)
>  		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
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 8cbe580..b97963d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -457,7 +457,7 @@ i915_gem_context_create_gvt(struct drm_device *dev)
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> -	ctx = __create_hw_context(to_i915(dev), NULL);
> +	ctx = i915_gem_create_context(to_i915(dev), NULL);
>  	if (IS_ERR(ctx))
>  		goto out;
>  
> -- 
> 2.7.4
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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