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

[Zhao, Yan Y]

In xengt, write-protect is write protect. But In kvmgt, write-protect is page-tracker, meaning the page is not write-protected, but we got notification on page write occurs.
So, I think we can change from "page_track -> write-protection -> page_track"
To " page_track -> page_track -> page_track"

After all, change xengt from wp to page track only requires 1 line of code.

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> Behalf Of Du, Changbin
> Sent: Wednesday, January 10, 2018 10:20 AM
> To: Zhao, Yan Y <yan.y.zhao at intel.com>
> Cc: Du, Changbin <changbin.du at intel.com>; intel-gvt-
> dev at lists.freedesktop.org
> Subject: Re: [PATCH v3 12/15] drm/i915/gvt: Don't extend page_track to mpt
> layer
> 
> 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
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev