[PATCH v3 12/15] drm/i915/gvt: Don't extend page_track to mpt layer

[Du, Changbin]

On Wed, Jan 10, 2018 at 09:11:04AM +0800, Zhao, Yan Y wrote:
> 
> instead, what about rename mpt interface to page tracker?
> 1. actually what we did in mpt.set_wp_page() is page tracker
> 2.  if you change
> 
> intel_gvt_hypervisor_enable_page_track() to intel_gvt_hypervisor_set_wp_page(), but actually the page is not write-protected, it will make future code writer confused.
> 
Yes, change the mpt protocol. And it requires xengt change accordingly. Before
that double confusing is worse: page_track -> write-protection -> page_track.
and how does xengt call it?

> On 1/9/2018 5:42 PM, Du, Changbin wrote:
> > On Tue, Jan 09, 2018 at 04:55:54PM +0800, Zhao, Yan Y wrote:
> > > hi changbin,
> > > 
> > > please see my comment inline.
> > > 
> > > On 1/8/2018 6:24 PM, changbin.du at intel.com wrote:
> > > > From: Changbin Du <changbin.du at intel.com>
> > > > 
> > > > Don't extend page_track to mpt layer. Keep MPT simple and clean.
> > > > 
> > > > Signed-off-by: Changbin Du <changbin.du at intel.com>
> > > > ---
> > > >    drivers/gpu/drm/i915/gvt/gtt.c | 19 +++++++++++++------
> > > >    drivers/gpu/drm/i915/gvt/mpt.h | 42 ++++++++++--------------------------------
> > > >    2 files changed, 23 insertions(+), 38 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> > > > index 642d9de..9df9c9bc 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > > > @@ -677,8 +677,8 @@ static void ppgtt_free_spt(struct intel_vgpu_ppgtt_spt *spt)
> > > >    		hash_del(&spt->guest_page.track.node);
> > > >    	if (spt->guest_page.track.tracked)
> > > > -		intel_gvt_hypervisor_disable_page_track(spt->vgpu,
> > > > -				&spt->guest_page.track);
> > > > +		intel_gvt_hypervisor_unset_wp_page(spt->vgpu,
> > > > +				spt->guest_page.track.gfn);
> > > unset_wp_page may confuse the users in kvmgt, since we actually use the page
> > > tracker mechanism in kvmgt, which is not write protection, because the guest
> > > page has been already changed before we got notification. we have no way to
> > > protect the write.
> > > so disable_page_track is better for kvmgt.
> > Then the change should be made in the mpt protocol if they have different
> > semantics. Since mpt should be synced with both xen and kvmgt, I'd leave this to
> > future. In this patch, we are trying not to change the mpt semantics by just
> > adding a simple wrapper layer.
> > 
> > Anyway this is not so terriable since the page track is based on write-protection.
> > 
> 

-- 
Thanks,
Changbin Du