[PATCH] drm/i915/gvt: Do not check shadowed when adding ppgtt to GVT GEM context

[Zhenyu Wang]

On 2019.07.04 12:27:35 +0800, Colin Xu wrote:
> 
> On 2019-07-04 10:55, Zhenyu Wang wrote:
> > On 2019.07.04 08:09:29 +0800, Colin Xu wrote:
> > > Windows guest can't run after force-TDR with host log:
> > > ...
> > > gvt: vgpu 1: workload shadow ppgtt isn't ready
> > > gvt: vgpu 1: fail to dispatch workload, skip
> > > ...
> > > 
> > > The error is raised by set_context_ppgtt_from_shadow(), when it checks
> > > and found the shadow_mm isn't marked as shadowed.
> > > 
> > > In work thread before each submission, a shadow_mm is set to shadowed in:
> > > shadow_ppgtt_mm()
> > > <-intel_vgpu_pin_mm()
> > > <-prepare_workload()
> > > <-dispatch_workload()
> > > <-workload_thread()
> > > However checking whether or not shadow_mm is shadowed is prior to it:
> > > set_context_ppgtt_from_shadow()
> > > <-dispatch_workload()
> > > <-workload_thread()
> > > 
> > > In normal case, create workload will check the existence of shadow_mm,
> > > if not it will create a new one and marked as shadowed. If already exist
> > > it will reuse the old one. Since shadow_mm is reused, checking of shadowed
> > > in set_context_ppgtt_from_shadow() actually always see the state set in
> > > creation, but not the state set in intel_vgpu_pin_mm().
> > > 
> > > When force-TDR, all engines are reset, since it's not dmlr level, all
> > > ppgtt_mm are invalidated but not destroyed. Invalidation will mark all
> > > reused shadow_mm as not shadowed but still keeps in ppgtt_mm_list_head.
> > > If workload submission phase those shadow_mm are reused with shadowed
> > > not set, then set_context_ppgtt_from_shadow() will report error.
> > > 
> > > No need to check shadowed state in set_context_ppgtt_from_shadow().
> > Thanks for tracing this! I think this is a side effect from 1e18d5e6731d674fee0bb4b66f5ea61e504452a3,
> > which I moved this setting out of scan function.
> 
> I think even before 1e18d5e6731d674fee0bb4b66f5ea61e504452a it still has problem
> since intel_gvt_scan_and_shadow_workload() is before prepare_workload().
>

Yep.

> > 
> > One potential issue with this is shadow_pdps at this time might not have valid value yet,
> > how about we move shadow ctx ppgtt setting after we really prepared mm for workload and
> > before i915 submission? As this setting now is mostly to please i915, we have already set
> > context's ppgtt by ourself.
> > 
> If prepare_workload() failed, should we still set context ppgtt for the shadow?
>

As we always populate our shadow ppgtt by context state, the shadow i915 ctx ppgtt
is not actually used but only for context pin. As long as we ensure ctx ppgtt is set
for dispatch workload, then it's ok.

For newer code which we always pin gvt context, maybe just setup it once is enough,
that could be refined later.

> > > Fixes: 4f15665ccbba (drm/i915: Add ppgtt to GVT GEM context)
> > > 
> > > Signed-off-by: Colin Xu <colin.xu at intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gvt/scheduler.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > index 196b4155a309..a5db57926962 100644
> > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > @@ -371,7 +371,7 @@ static int set_context_ppgtt_from_shadow(struct intel_vgpu_workload *workload,
> > >   	struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(ctx->vm);
> > >   	int i = 0;
> > > -	if (mm->type != INTEL_GVT_MM_PPGTT || !mm->ppgtt_mm.shadowed)
> > > +	if (mm->type != INTEL_GVT_MM_PPGTT)
> > >   		return -EINVAL;
> > >   	if (mm->ppgtt_mm.root_entry_type == GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {
> > > -- 
> > > 2.22.0
> > > 
> > > _______________________________________________
> > > intel-gvt-dev mailing list
> > > intel-gvt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> 
> -- 
> Best Regards,
> Colin Xu
> 

-- 
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-gvt-dev/attachments/20190704/063a7c12/attachment.sig>