[PATCH 2/2] drm/i915/gvt: A workaround for an regression causedby one i915 PPGTT optimization

[zhi.wang.linux at gmail.com]

Because we have the requirement to stop trapping it immediately.

1) Invalid PTE entries in PTE pages cost more efforts in GVT-g. Malicious guest can submit a lot of this kinds of page table with workload. It’s like a DDOS
2) If it’s not an valid PTE page and we trapped it, there will be risk of instruction which is not supported by instruction emulator in hypervisor.

That’s the reason why I call lazy shadow as root fix.

Sent from Mail for Windows 10

From: Du, Changbin
Sent: Thursday, October 19, 2017 9:14 AM
To: Wang, Zhi A
Cc: Du, Changbin; Zhi Wang; intel-gvt-dev at lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/i915/gvt: A workaround for an regression causedby one i915 PPGTT optimization

On Wed, Oct 18, 2017 at 11:07:24PM -0700, Wang, Zhi A wrote:
> Practically yes. But we cannot assume guest is always behaving good. That's the reason why I call it a workaround and the root cause of fix should be the lazy shadow, which I hope I can make it work this week.
>
I'd think that's an optimization. Lazy shadowing doesn't mean invalid PTE case
is gone, right? (dont aussme guest's behaviour.)

> Thanks,
> Zhi.
> 
> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On Behalf Of Du, Changbin
> Sent: Thursday, October 19, 2017 5:55 AM
> To: Zhi Wang <zhi.wang.linux at gmail.com>
> Cc: Du, Changbin <changbin.du at intel.com>; intel-gvt-dev at lists.freedesktop.org; Wang, Zhi A <zhi.a.wang at intel.com>
> Subject: Re: [PATCH 2/2] drm/i915/gvt: A workaround for an regression caused by one i915 PPGTT optimization
> 
> I think this is not a workaround but a fix. GTE always has valid value is not a true assumption. Shadowing code should cover this case, and it is not an error (the error msg should remove).
> 
> On Thu, Oct 19, 2017 at 02:30:09AM +0800, Zhi Wang wrote:
> > In the commit:
> > 
> > commit 14826673247eaf36b16fd821fac27efa663f3fa6
> > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > Date:   Fri Sep 8 19:16:22 2017 +0100
> > 
> >     drm/i915: Only initialize partially filled pagetables
> > 
> >     If we know that we will completely fill a pagetable (i.e. we are
> >     inserting a complete set of 512 pages), we can skip prefilling that PT
> >     with scratch entries. If we have to abort the insertion prior to writing
> >     the real entries, we will teardown the pagetable and remove it from the
> >     page directory (so that we will restart the allocation next time).
> > 
> >     We could do similar tricks for the PD and PDP, but the likelihood of a
> >     single insertion covering the entire 512 entries diminishes, as do the
> >     cycle savings. The saving are even greater (relatively) when we are
> >     preallocating page tables for huge pages, as then we never need to fill
> >     the page table.
> > 
> > It will link an un-initialized PTE page into a PPGTT page table 
> > tracked by GVT-g, which leads to linux guest failing to boot. Since 
> > the fix of root casue still needs some time to be ready, a temporary 
> > workaround is introduced first.
> > 
> > Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/gtt.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c 
> > b/drivers/gpu/drm/i915/gvt/gtt.c index 6fa9271..d24d52d 100644
> > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > @@ -767,7 +767,6 @@ static int ppgtt_write_protection_handler(void 
> > *data, u64 pa,  {
> >  	struct intel_vgpu_page_track *t = data;
> >  	struct intel_vgpu_guest_page *p = page_track_to_guest_page(t);
> > -	int ret;
> >  
> >  	if (bytes != 4 && bytes != 8)
> >  		return -EINVAL;
> > @@ -775,11 +774,9 @@ static int ppgtt_write_protection_handler(void *data, u64 pa,
> >  	if (!t->tracked)
> >  		return -EINVAL;
> >  
> > -	ret = ppgtt_handle_guest_write_page_table_bytes(p,
> > +	ppgtt_handle_guest_write_page_table_bytes(p,
> >  		pa, p_data, bytes);
> > -	if (ret)
> > -		return ret;
> > -	return ret;
> > +	return 0;
> >  }
> >  
> >  static int reclaim_one_mm(struct intel_gvt *gvt);
> > --
> > 2.7.4
> > 
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> 
> --
> Thanks,
> Changbin Du

-- 
Thanks,
Changbin Du

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20171027/23e8d76f/attachment-0001.html>