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

[Zhao, Yan Y]

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.

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.
>