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

[Zhenyu Wang]

On 2018.01.10 02:52:32 +0000, Du, Changbin wrote:
> The problem is changing mpt protocol will beak xengt on next rebase. 
> 
> + Xiong for xengt.
>

I think "page track" might be more general name for this either
by WP or other page track utility, that we might change MPT hook as well.

btw, is this one bisect-able? It looks you removed counter for tracked page
which is still used in WP handler in this? Even I like your later change, but
we still need to make each one bisect-able.

> -----Original Message-----
> From: Zhao, Yan Y 
> Sent: Wednesday, January 10, 2018 10:46 AM
> To: Du, Changbin <changbin.du 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
> 
> 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
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20180123/8a7647f1/attachment.sig>