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

[Zhenyu Wang]

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.

-- 
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/2d2b714e/attachment.sig>