drm/i915/gvt: force to set all context control bits from guest

Submitted by Zhenyu Wang on March 19, 2018, 9:09 a.m.

Details

Message ID 20180319090905.17133-1-zhenyuw@linux.intel.com
State New
Headers show
Series "drm/i915/gvt: force to set all context control bits from guest" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Zhenyu Wang March 19, 2018, 9:09 a.m.
Our shadow context content is from guest but with masked control reg like
CTX_CONTEXT_CONTROL, we need to make sure all settings from guest would be set
when this context is on hw, this trys to force mask enable bits for all to
ensure every bits setting would be effective on hw.

One regression found related to once inhibit bit is set, gpu engine are working
on inhibit state until MI_LOAD_REG_IMM command or context image clear inhibit
bit with mask bit set to 1, and val bit set to 0. In gvt-g currently workload
has the highest priority, so gvt-g workload could trigger preempt context
easily, preempt context set inhibit bit, then gvt-g workload is scheduled in,
but gvt-g workload shadow context image usually doesn't set inhibit mask bit,
so gpu is still in inhibit state when gvt workload is running. This caused gpu
hang.

Suggested-by: Zhang, Xiong <xiong.y.zhang@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/gvt/scheduler.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 87baeb69aaa5..638abe84857c 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -173,8 +173,14 @@  static int populate_shadow_context(struct intel_vgpu_workload *workload)
 #define COPY_REG(name) \
 	intel_gvt_hypervisor_read_gpa(vgpu, workload->ring_context_gpa \
 		+ RING_CTX_OFF(name.val), &shadow_ring_context->name.val, 4)
+#define COPY_REG_MASKED(name) {\
+		intel_gvt_hypervisor_read_gpa(vgpu, workload->ring_context_gpa \
+					      + RING_CTX_OFF(name.val),\
+					      &shadow_ring_context->name.val, 4);\
+		shadow_ring_context->name.val |= 0xffff << 16;\
+	}
 
-	COPY_REG(ctx_ctrl);
+	COPY_REG_MASKED(ctx_ctrl);
 	COPY_REG(ctx_timestamp);
 
 	if (ring_id == RCS) {
@@ -183,6 +189,7 @@  static int populate_shadow_context(struct intel_vgpu_workload *workload)
 		COPY_REG(rcs_indirect_ctx_offset);
 	}
 #undef COPY_REG
+#undef COPY_REG_MASKED
 
 	intel_gvt_hypervisor_read_gpa(vgpu,
 			workload->ring_context_gpa +

Comments

> Our shadow context content is from guest but with masked control reg like

> CTX_CONTEXT_CONTROL, we need to make sure all settings from guest

> would be set when this context is on hw, this trys to force mask enable bits

> for all to ensure every bits setting would be effective on hw.

> 

> One regression found related to once inhibit bit is set, gpu engine are

> working on inhibit state until MI_LOAD_REG_IMM command or context

> image clear inhibit bit with mask bit set to 1, and val bit set to 0. In gvt-g

> currently workload has the highest priority, so gvt-g workload could trigger

> preempt context easily, preempt context set inhibit bit, then gvt-g workload

> is scheduled in, but gvt-g workload shadow context image usually doesn't set

> inhibit mask bit, so gpu is still in inhibit state when gvt workload is running.

> This caused gpu hang.

> 

> Suggested-by: Zhang, Xiong <xiong.y.zhang@intel.com>

> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>

> ---

>  drivers/gpu/drm/i915/gvt/scheduler.c | 9 ++++++++-

>  1 file changed, 8 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c

> b/drivers/gpu/drm/i915/gvt/scheduler.c

> index 87baeb69aaa5..638abe84857c 100644

> --- a/drivers/gpu/drm/i915/gvt/scheduler.c

> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c

> @@ -173,8 +173,14 @@ static int populate_shadow_context(struct

> intel_vgpu_workload *workload)  #define COPY_REG(name) \

>  	intel_gvt_hypervisor_read_gpa(vgpu, workload->ring_context_gpa \

>  		+ RING_CTX_OFF(name.val), &shadow_ring_context->name.val, 4)

> +#define COPY_REG_MASKED(name) {\

> +		intel_gvt_hypervisor_read_gpa(vgpu, workload->ring_context_gpa

> \

> +					      + RING_CTX_OFF(name.val),\

> +					      &shadow_ring_context->name.val, 4);\

> +		shadow_ring_context->name.val |= 0xffff << 16;\

> +	}

> 

> -	COPY_REG(ctx_ctrl);

> +	COPY_REG_MASKED(ctx_ctrl);

>  	COPY_REG(ctx_timestamp);

> 

>  	if (ring_id == RCS) {

> @@ -183,6 +189,7 @@ static int populate_shadow_context(struct

> intel_vgpu_workload *workload)

>  		COPY_REG(rcs_indirect_ctx_offset);

>  	}

>  #undef COPY_REG

> +#undef COPY_REG_MASKED

> 

>  	intel_gvt_hypervisor_read_gpa(vgpu,

>  			workload->ring_context_gpa +

> --

[Zhang, Xiong Y] Reviewed-by: Zhang, Xiong <xiong.y.zhang@intel.com>
> 2.16.2

> 

> _______________________________________________

> intel-gvt-dev mailing list

> intel-gvt-dev@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
On 2018.03.19 09:30:23 +0000, Zhang, Xiong Y wrote:
> > Our shadow context content is from guest but with masked control reg like
> > CTX_CONTEXT_CONTROL, we need to make sure all settings from guest
> > would be set when this context is on hw, this trys to force mask enable bits
> > for all to ensure every bits setting would be effective on hw.
> > 
> > One regression found related to once inhibit bit is set, gpu engine are
> > working on inhibit state until MI_LOAD_REG_IMM command or context
> > image clear inhibit bit with mask bit set to 1, and val bit set to 0. In gvt-g
> > currently workload has the highest priority, so gvt-g workload could trigger
> > preempt context easily, preempt context set inhibit bit, then gvt-g workload
> > is scheduled in, but gvt-g workload shadow context image usually doesn't set
> > inhibit mask bit, so gpu is still in inhibit state when gvt workload is running.
> > This caused gpu hang.
> > 
> > Suggested-by: Zhang, Xiong <xiong.y.zhang@intel.com>
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/scheduler.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index 87baeb69aaa5..638abe84857c 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -173,8 +173,14 @@ static int populate_shadow_context(struct
> > intel_vgpu_workload *workload)  #define COPY_REG(name) \
> >  	intel_gvt_hypervisor_read_gpa(vgpu, workload->ring_context_gpa \
> >  		+ RING_CTX_OFF(name.val), &shadow_ring_context->name.val, 4)
> > +#define COPY_REG_MASKED(name) {\
> > +		intel_gvt_hypervisor_read_gpa(vgpu, workload->ring_context_gpa
> > \
> > +					      + RING_CTX_OFF(name.val),\
> > +					      &shadow_ring_context->name.val, 4);\
> > +		shadow_ring_context->name.val |= 0xffff << 16;\
> > +	}
> > 
> > -	COPY_REG(ctx_ctrl);
> > +	COPY_REG_MASKED(ctx_ctrl);
> >  	COPY_REG(ctx_timestamp);
> > 
> >  	if (ring_id == RCS) {
> > @@ -183,6 +189,7 @@ static int populate_shadow_context(struct
> > intel_vgpu_workload *workload)
> >  		COPY_REG(rcs_indirect_ctx_offset);
> >  	}
> >  #undef COPY_REG
> > +#undef COPY_REG_MASKED
> > 
> >  	intel_gvt_hypervisor_read_gpa(vgpu,
> >  			workload->ring_context_gpa +
> > --
> [Zhang, Xiong Y] Reviewed-by: Zhang, Xiong <xiong.y.zhang@intel.com>

Applied, thanks for review!