[Intel-gfx] [PATCH] drm/i915: Check ppgtt validity for GVT GEM context

[Zhenyu Wang]

On 2018.10.09 14:08:20 +0800, Xiong Zhang wrote:
> 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.
> 
> GVT maintain a shadow ppgtt for each guest ppgtt, and attach this shadow
> ppgtt to gpu when the corresponding guest ppgtt will be scheduled onto gpu.
> The structure of shadow ppgtt is different from i915_hw_ppgtt, a lot of gvt
> refactor work should be done in order to use i915_hw_ppgtt structure.
> So let's fix the regression first.
> 
> Fixes:4a3d3f6785be("drm/i915: Match code to comment and enforce ppgtt for execlists")
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
> ---

Chris, any comment? Without this, current gvt breaks on drm tip with kernel oops.
Could we fix the regression first then check gvt ctx stub ppgtt handling later?
Which would require more change on gvt side, not sure if need any more helper from
i915 ppgtt code with a clean fix..

Thanks

>  drivers/gpu/drm/i915/intel_lrc.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index ff0e2b3..d604d8a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -385,7 +385,7 @@ static u64 execlists_update_context(struct i915_request *rq)
>  	 * PML4 is allocated during ppgtt init, so this is not needed
>  	 * in 48-bit mode.
>  	 */
> -	if (!i915_vm_is_48bit(&ppgtt->vm))
> +	if (ppgtt && !i915_vm_is_48bit(&ppgtt->vm))
>  		execlists_update_context_pdps(ppgtt, reg_state);
>  
>  	return ce->lrc_desc;
> @@ -1210,7 +1210,6 @@ execlists_context_pin(struct intel_engine_cs *engine,
>  	struct intel_context *ce = to_intel_context(ctx, engine);
>  
>  	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
> -	GEM_BUG_ON(!ctx->ppgtt);
>  
>  	if (likely(ce->pin_count++))
>  		return ce;
> @@ -2538,7 +2537,7 @@ static void execlists_init_reg_state(u32 *regs,
>  	CTX_REG(regs, CTX_PDP0_UDW, GEN8_RING_PDP_UDW(engine, 0), 0);
>  	CTX_REG(regs, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(engine, 0), 0);
>  
> -	if (i915_vm_is_48bit(&ctx->ppgtt->vm)) {
> +	if (ctx->ppgtt && i915_vm_is_48bit(&ctx->ppgtt->vm)) {
>  		/* 64b PPGTT (48bit canonical)
>  		 * PDP0_DESCRIPTOR contains the base address to PML4 and
>  		 * other PDP Descriptors are ignored.
> -- 
> 2.7.4
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20181011/e7a4c7f1/attachment-0001.sig>