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

[Zhenyu Wang]

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.

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.

> 
> 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

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