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

[Chris Wilson]

Quoting Zhenyu Wang (2018-10-11 10:01:26)
> On 2018.10.11 07:23:11 +0100, Chris Wilson wrote:
> > Quoting Zhenyu Wang (2018-10-11 03:45:07)
> > > 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..
> > 
> > We both thought this wasn't the approach we wanted to take. My position
> > is that this leaves the registers ostensibly programmed to garbage.
> 
> yeah, I agree that's not ideal, my point is that would bring more changes e.g
> gvt ctx create function need to be changed accordingly, etc. and I'm
> disappointed why CI didn't catch this in first place, as Terrence has
> contributed GVT guest boot test in igt, we need to add that for list patch
> checking!
> 
> Current gvt nightly test is using this one to kick, we'll see if a quick
> mitigation can be made.

Easiest is just to allocate the ctx->ppgtt. We waste a few pages, once.
Later on we can fine tune the integration.
-Chris