[PATCH v2] drm/i915/gvt: Add runtime_pm get/put to proctect MMIO accessing

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> Behalf Of Dong, Chuanxiao
> Sent: Wednesday, May 31, 2017 1:13 PM
> To: Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: intel-gvt-dev at lists.freedesktop.org
> Subject: RE: [PATCH v2] drm/i915/gvt: Add runtime_pm get/put to proctect
> MMIO accessing
> 
> 
> 
> > -----Original Message-----
> > From: intel-gvt-dev
> > [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On Behalf Of
> > Zhenyu Wang
> > Sent: Wednesday, May 31, 2017 12:27 PM
> > To: Dong, Chuanxiao <chuanxiao.dong at intel.com>
> > Cc: intel-gvt-dev at lists.freedesktop.org
> > Subject: Re: [PATCH v2] drm/i915/gvt: Add runtime_pm get/put to
> > proctect MMIO accessing
> >
> > On 2017.05.27 13:27:33 +0800, Chuanxiao Dong wrote:
> > > In some cases, GVT-g is accessing MMIO without holding runtime_pm
> > > and this patch can add the runtime_pm get/put to make sure when
> > > accessing MMIO the i915 HW is really powered on.
> > >
> > > v2:
> > > - make the runtime pm protection smaller granularity. (Zhenyu)
> > >
> > > Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/gtt.c      |  2 ++
> > >  drivers/gpu/drm/i915/gvt/handlers.c | 11 +++++++++++
> > >  2 files changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c
> > > b/drivers/gpu/drm/i915/gvt/gtt.c index c6f0077..13bfe09 100644
> > > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > > @@ -251,8 +251,10 @@ static void write_pte64(struct drm_i915_private
> > > *dev_priv,
> > >
> > >  	writeq(pte, addr);
> > >
> > > +	intel_runtime_pm_get(dev_priv);
> > >  	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> > >  	POSTING_READ(GFX_FLSH_CNTL_GEN6);
> > > +	intel_runtime_pm_put(dev_priv);
> > >  }
> >
> > Could we handle this invalidate explicitly instead of hiding in write pte?
> > And invalidate/flush only after one set of ggtt operations end?
> > otherwise I'm afraid this is overkill. And we can remove the posting read as
> well.
> 
> Good suggestion. Would like to send separate patch to add invalidate/flush
> API to handle pte writing case.

Just realized that, the gtt write operations may come from the emulated_gtt_mmio_write from guest thus gvt cannot know when the set of ggtt operations are done.

And adding runtime_pm_get/put just around the I915_WRITE won't cause the overkill because i915 are using the autosuspend runtime PM which has a latency (probably 10s) to suspend after called runtime_pm_put, the i915 HW can only to suspend if there is no more runtime_pm_get to be called in these 10s. So actually even we just call runtime_pm_get/runtime_pm_put in emulate_mmio_write/read function, there won't be overkill.

Thanks
Chuanxiao