[PATCH] drm/i915/gvt: set shadow entry to scratch page when guest driver only update partial of the GGTT entry

[Tian, Kevin]

> From: Xiaoguang Chen
> Sent: Tuesday, March 21, 2017 5:59 PM
> 
> Sometimes guest driver only update partial of GGTT entry then access it. In
> this situation the gfn we got from the GGTT table is invalid and can not be

Is partial update caused by splitting 64bit entry update into two 32bit
entry updates, or just because only need to change partial fields when
the PTE is already present?

"is invalid" and "can not" are too strong assertive here. Even in such 
case it should be "may be".

> translate to a valid mfn.
> p2m translation should happen only when the whole parts of a GGTT entry
> got updated.

Based on this description how do you judge there is a full update? I
don't say the patch actually records any state to help later judgement.

Or do you really mean that "Invalid P2M translation can be a valid
scenario, as guest may do split update (e.g. splitting one 64bit
entry update to two 32bit entry update). In such situation, we should
update shadow PTE to scratch page against any P2M translation error. 
Suppose guest driver shouldn't use this partial entry for real purpose.
Later when guest driver completes the other part update, the success 
of new P2M translation will lead to a real shadow PTE sync."?


> 
> Setting the shadow entry pointing to a scratch page instead if only partial of
> GGTT entry updated.


> 
> Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> SIgned-off-by: Xiaoguang Chen <xiaoguang.chen at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gtt.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index da73127..119cd30 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1837,11 +1837,21 @@ static int emulate_gtt_mmio_write(struct
> intel_vgpu *vgpu, unsigned int off,
>  		ret = gtt_entry_p2m(vgpu, &e, &m);
>  		if (ret) {
>  			gvt_vgpu_err("fail to translate guest gtt entry\n");
> -			return ret;
> +			/* guest driver may read/write the entry when only
> +			 * partial of the guest entry got updated.
> +			 * In this situation the gfn we got from GGTT table is
> +			 * invalid and can not be translated to a valid mfn.
> +			 * p2m translation should happen only after the
> whole
> +			 * parts of a GGTT entry got updated.
> +			 *
> +			 * Setting the shadow entry pointing to a scratch page
> +			 * instead while GGTT entry only got partial update.
> +			 */
> +			ops->set_pfn(&m, gvt->gtt.scratch_ggtt_mfn);
>  		}
>  	} else {
>  		m = e;
> -		m.val64 = 0;
> +		ops->set_pfn(&m, gvt->gtt.scratch_ggtt_mfn);

This looks more than what commit message described. It's about
using scratch page when guest PTE is non-present. Need include
such info in the description.

>  	}
> 
>  	ggtt_set_shadow_entry(ggtt_mm, &m, g_gtt_index);
> --
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev