diff mbox

[v1] drm/i915: Enhanced for initialize partially filled pagetables

Message ID 20170928020904.5030-1-xiaolin.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Xiaolin Zhang Sept. 28, 2017, 2:09 a.m. UTC
if vgpu active, the page table entry should be initialized after
allocation and then the hypersivor can ping pages succesuffly,
otherwise hypervisor will ping pages failed and the host will print
a lot of annoying errors such as “ERROR gvt: guest page write error -22,
gfn 0x7ada8, pa 0x7ada89a8, var 0x6, len 1” when create linux guest.

Signed-off-by: Xiaolin Zhang <xiaolin.zhang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Joonas Lahtinen Sept. 28, 2017, 2:25 p.m. UTC | #1
On Thu, 2017-09-28 at 10:09 +0800, Xiaolin Zhang wrote:
> if vgpu active, the page table entry should be initialized after
> allocation and then the hypersivor can ping pages succesuffly,
> otherwise hypervisor will ping pages failed and the host will print
> a lot of annoying errors such as “ERROR gvt: guest page write error -22,
> gfn 0x7ada8, pa 0x7ada89a8, var 0x6, len 1” when create linux guest.
> 
> Signed-off-by: Xiaolin Zhang <xiaolin.zhang@intel.com>

Why does the hypervisor try to access the entries prior to them being
made valid for hardware?

Regards, Joonas
Xiaolin Zhang Sept. 30, 2017, 2:58 a.m. UTC | #2
On 09/28/2017 10:25 PM, Joonas Lahtinen wrote:

On Thu, 2017-09-28 at 10:09 +0800, Xiaolin Zhang wrote:


if vgpu active, the page table entry should be initialized after
allocation and then the hypersivor can ping pages succesuffly,
otherwise hypervisor will ping pages failed and the host will print
a lot of annoying errors such as “ERROR gvt: guest page write error -22,
gfn 0x7ada8, pa 0x7ada89a8, var 0x6, len 1” when create linux guest.

Signed-off-by: Xiaolin Zhang <xiaolin.zhang@intel.com><mailto:xiaolin.zhang@intel.com>


Why does the hypervisor try to access the entries prior to them being
made valid for hardware?

Regards, Joonas


Hi Joonas,

thanks your comment.

I think what you ask is the point we got the error message in gvt since the current gvt

implementation is that page under write protection and trapped should be valid

with correct shadow page setup and p2m translation. Actually, to work with

“initialize partially filled pagetables" , there is a certain refine work to do in gvt side

(maybe less or maybe large). but before refine work done, this patch is trying to bring back

gvt behavior as before “initialize partially filled pagetables"patch.

BRs, Xiaolin
Joonas Lahtinen Oct. 2, 2017, 9:03 a.m. UTC | #3
+ Zhenyu and Zhi

On Sat, 2017-09-30 at 02:58 +0000, Zhang, Xiaolin wrote:
> On 09/28/2017 10:25 PM, Joonas Lahtinen wrote:
> > On Thu, 2017-09-28 at 10:09 +0800, Xiaolin Zhang wrote:
> > > if vgpu active, the page table entry should be initialized after
> > > allocation and then the hypersivor can ping pages succesuffly,
> > > otherwise hypervisor will ping pages failed and the host will
> > > print
> > > a lot of annoying errors such as “ERROR gvt: guest page write
> > > error -22,
> > > gfn 0x7ada8, pa 0x7ada89a8, var 0x6, len 1” when create linux
> > > guest.
> > > 
> > > Signed-off-by: Xiaolin Zhang <xiaolin.zhang@intel.com>
> > 
> > Why does the hypervisor try to access the entries prior to them
> > being
> > made valid for hardware?
> > 
> > Regards, Joonas
> 
> Hi Joonas,
> thanks your comment. 
> I think what you ask is the point we got the error message in gvt since the current gvt
> implementation is that page under write protection and trapped should be valid 
> with correct shadow page setup and p2m translation.

My question is that if the hardware doesn't care about them being
uninitialized at this point, how can GVT? If the GVT implementation
relies heavily on how the i915 driver currently happens to behave, not
on what contracts it has with the hardware, these breakages are bound
to happen repeatedly. The code is being transformed and optimized on
daily basis, how it currently behaves is not a solid foundation for
implementing the host side virtualization.

> Actually, to work with 
> “initialize partially filled pagetables" , there is a certain refine work to do in gvt side
> (maybe less or maybe large). but before refine work done, this patch is trying to bring back 
> gvt behavior as before “initialize partially filled pagetables"patch. 

We should first minimize and then keep the vgpu specific checks to
minimum. So this would need to be fixed on the GVT side of code.

Regards, Joonas
Wang, Zhi A Oct. 3, 2017, 7:50 a.m. UTC | #4
It's because there is an optimization of pre-shadow in guest i915. When an i915 PPGTT is created, guest i915 will notify the hypervisor about this. Hypervisor will start to track the page table at this time. It's like the page table has already been valid on HW after being tracked by hypervisor. So when a PTE with invalid content is linked into the PDE, HV will report some errors.

We tried to remove this optimization and shadow the whole PPGTT page table before a vGPU workload was submitted. It ends up with huge performance drop since the PPGTT shadow has to be created and destroyed for every submit. It cost so much CPU% time and wastes a lot of GPU bandwidth.

A better solution from HV side is proactively stopping track a page with invalid content when it's linked into the page table and performing the shadow for this page before HV dispatches a vGPU workload (of course, if it's still invalid, HV will report error at this time). The lazy shadow feature stills needs some efforts to land into GVT-g. So we needs a short term solution.

From my opinion, we can check:

a) If the workaround can stay in GVT-g.
b) Gaps if the workaround has to stay in i915.

If we can move to a), then this patch is not necessary.

For long term solution, I'm working on that. It still needs quite some re-factors of code structure since once HV bites an invalid entry, it has to unwind the shadow has been done in that page recursively.

Thanks,
Zhi.

-----Original Message-----
From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com] 

Sent: Monday, October 2, 2017 12:03 PM
To: Zhang, Xiaolin <xiaolin.zhang@intel.com>; intel-gvt-dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
Subject: Re: [Intel-gfx] [PATCH v1] drm/i915: Enhanced for initialize partially filled pagetables

+ Zhenyu and Zhi

On Sat, 2017-09-30 at 02:58 +0000, Zhang, Xiaolin wrote:
> On 09/28/2017 10:25 PM, Joonas Lahtinen wrote:

> > On Thu, 2017-09-28 at 10:09 +0800, Xiaolin Zhang wrote:

> > > if vgpu active, the page table entry should be initialized after 

> > > allocation and then the hypersivor can ping pages succesuffly, 

> > > otherwise hypervisor will ping pages failed and the host will 

> > > print a lot of annoying errors such as “ERROR gvt: guest page 

> > > write error -22, gfn 0x7ada8, pa 0x7ada89a8, var 0x6, len 1” when 

> > > create linux guest.

> > > 

> > > Signed-off-by: Xiaolin Zhang <xiaolin.zhang@intel.com>

> > 

> > Why does the hypervisor try to access the entries prior to them 

> > being made valid for hardware?

> > 

> > Regards, Joonas

> 

> Hi Joonas,

> thanks your comment. 

> I think what you ask is the point we got the error message in gvt 

> since the current gvt implementation is that page under write 

> protection and trapped should be valid with correct shadow page setup and p2m translation.


My question is that if the hardware doesn't care about them being uninitialized at this point, how can GVT? If the GVT implementation relies heavily on how the i915 driver currently happens to behave, not on what contracts it has with the hardware, these breakages are bound to happen repeatedly. The code is being transformed and optimized on daily basis, how it currently behaves is not a solid foundation for implementing the host side virtualization.

> Actually, to work with

> “initialize partially filled pagetables" , there is a certain refine 

> work to do in gvt side (maybe less or maybe large). but before refine 

> work done, this patch is trying to bring back gvt behavior as before “initialize partially filled pagetables"patch.


We should first minimize and then keep the vgpu specific checks to minimum. So this would need to be fixed on the GVT side of code.

Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 731ce229a923..be52b139817d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1175,7 +1175,7 @@  static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 			if (IS_ERR(pt))
 				goto unwind;
 
-			if (count < GEN8_PTES)
+			if (count < GEN8_PTES || intel_vgpu_active(vm->i915))
 				gen8_initialize_pt(vm, pt);
 
 			gen8_ppgtt_set_pde(vm, pd, pt, pde);