All of lore.kernel.org
 help / color / mirror / Atom feed
* Fwd: Discuss GVT context hacks in i915
       [not found] <56C19630.5010301@intel.com>
@ 2016-02-15  9:13 ` Zhi Wang
       [not found] ` <56C19B26.7020206@intel.com>
  1 sibling, 0 replies; 6+ messages in thread
From: Zhi Wang @ 2016-02-15  9:13 UTC (permalink / raw)
  To: intel-gfx@lists.freedesktop.org

+ mailing-list

-------- Original Message --------
Subject: Discuss GVT context hacks in i915
Date: Mon, 15 Feb 2016 17:11:12 +0800
From: Zhi Wang <zhi.a.wang@intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,  Chris Wilson 
<chris@chris-wilson.co.uk>, Daniel Vetter <daniel.vetter@intel.com>, 
Zhiyuan Lv <zhiyuan.lv@intel.com>, "Tian, Kevin" <kevin.tian@intel.com>

Hi Guys:
    Previously we have sent our GVT-g device model RFC code, and thanks
for the advices and comments! As you see GVT-g needs a special kind of
LRC context as shadow context so it could be able to use i915 GEM
submission subsystem, which requires some changes in i915.

Probably we can discuss them one by one and settle them down before the
next version of RFC code. :)

Difference between a i915 LRC context and a GVT LRC context
------

* The engine context initialization of a GVT LRC context can be
bypassed, as GVT-g will initialize it by guest LRC context.

http://www.spinics.net/lists/intel-gfx/msg86546.html

* The addressing mode bit in the context descriptor should be able to
configured at runtime, as some guest is still using 32bit PPGTT address
space.

http://www.spinics.net/lists/intel-gfx/msg86544.html

* As the GVT LRC context will go with the shadow PPGTT page table
populated by GVT-g, a GVT LRC context doesn't need a i915 PPGTT
instance, also its PDP root pointer doesn't need to be populated by i915.

http://www.spinics.net/lists/intel-gfx/msg86545.html

* The ring buffer size of a GVT LRC context ring buffer object should be
configurable or hard-coded to the max size, as some guest (e.g. windows)
has a large ring buffer and could submit a lot of commands at one time.

Hacking i915
-------

The above changes needs to hack i915, in the RFC code, you can see a lot
of codes like below:

if (!gvt_context) {
	/* do something i915 needed */
} else {
	/* do something GVT-g needed */
}

This kinds of code piece is GVT-g specific, another approach should be
refining some i915 APIs or say something generic, like:

if (context->need_i915_ppgtt) {
	/* allocating i915 ppgtt instance */
}

if (context->ppgtt) {
	/* update PDP root pointers in LRC context. */
}

if (context->need_initialization) {
	/* emit commands to initialize the engine context of LRC context */
}

I'm wondering which kinds do you prefer? Or maybe you can give me some
advices? :)

Thanks,
Zhi.


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Discuss GVT context hacks in i915
       [not found] ` <56C19B26.7020206@intel.com>
@ 2016-02-15 16:03   ` Wang, Zhi A
  2016-02-15 17:20     ` Daniel Vetter
  2016-02-16  3:11     ` Tian, Kevin
  0 siblings, 2 replies; 6+ messages in thread
From: Wang, Zhi A @ 2016-02-15 16:03 UTC (permalink / raw)
  To: Vetter, Daniel, Joonas Lahtinen, Chris Wilson, Lv, Zhiyuan,
	Tian, Kevin
  Cc: intel-gfx@lists.freedesktop.org

Hi  Daniel:
    Thanks for shedding the light! See my comments below. :) 

-----Original Message-----
From: Vetter, Daniel 
Sent: Monday, February 15, 2016 5:32 PM
To: Wang, Zhi A; Joonas Lahtinen; Chris Wilson; Lv, Zhiyuan; Tian, Kevin
Subject: Re: Discuss GVT context hacks in i915

Am 15/02/2016 um 10:11 schrieb Zhi Wang:
> Hi Guys:
>    Previously we have sent our GVT-g device model RFC code, and thanks 
> for the advices and comments! As you see GVT-g needs a special kind of 
> LRC context as shadow context so it could be able to use i915 GEM 
> submission subsystem, which requires some changes in i915.
>
> Probably we can discuss them one by one and settle them down before 
> the next version of RFC code. :)
>
> Difference between a i915 LRC context and a GVT LRC context
> ------
>
> * The engine context initialization of a GVT LRC context can be 
> bypassed, as GVT-g will initialize it by guest LRC context.
>
> http://www.spinics.net/lists/intel-gfx/msg86546.html
>
> * The addressing mode bit in the context descriptor should be able to 
> configured at runtime, as some guest is still using 32bit PPGTT 
> address space.
>
> http://www.spinics.net/lists/intel-gfx/msg86544.html
>
> * As the GVT LRC context will go with the shadow PPGTT page table 
> populated by GVT-g, a GVT LRC context doesn't need a i915 PPGTT 
> instance, also its PDP root pointer doesn't need to be populated by i915.
>
> http://www.spinics.net/lists/intel-gfx/msg86545.html
>
> * The ring buffer size of a GVT LRC context ring buffer object should 
> be configurable or hard-coded to the max size, as some guest (e.g. 
> windows) has a large ring buffer and could submit a lot of commands at 
> one time.
>
> Hacking i915
> -------
>
> The above changes needs to hack i915, in the RFC code, you can see a 
> lot of codes like below:
>
> if (!gvt_context) {
>     /* do something i915 needed */
> } else {
>     /* do something GVT-g needed */
> }
>
> This kinds of code piece is GVT-g specific, another approach should be 
> refining some i915 APIs or say something generic, like:
>
> if (context->need_i915_ppgtt) {
>     /* allocating i915 ppgtt instance */
> }
>
> if (context->ppgtt) {
>     /* update PDP root pointers in LRC context. */
> }
>
> if (context->need_initialization) {
>     /* emit commands to initialize the engine context of LRC context */
> }
>
> I'm wondering which kinds do you prefer? Or maybe you can give me some 
> advices? :)

Without looking at more than what's in your mail here and in the links, 
this style is the midlayer mistake: We have some abstraction (lrc 
context, ppgtt) and force everyone to go through the same code paths. 
Which means deep down in those paths we have lots of if (special_case) 
conditions, which make the code a maintainance nightmare. Yes execlist 
support has already a lot of such bad examples unfortunately.

The better design idea is to reuse the data structures and helper 
functions, but have new top-level entry functions for creating e.g. a 
xengt lrc context. So e.g. have a lrc init function for xengt which 
takes the setup stuff as parameters. Wrt ppgtt my idea was to reuse 
struct i915_hw_ppgtt for managing the shadow pagetable, with xengt using 
the i915_gem_gtt.c functions to write shadow pagetable entries. That way 
i915 still knows the virtual->physical mapping, which aids in e.g. crash 
dump recording. Of course you're not going to bind entire vma, but 
instead will use the lower-level functions that just bind pages.

[Zhi] Thanks! Just want to make sure that you prefer that GVT-g specific
modifications should be put into a fork of top-level i915 APIs? For example,
we prepare a new function to create the GVT context, which is a fork of
simplified i915_gem_create_context().

For i915_hw_ppgtt and GVT-g shadow page table, we tried to think about how
to merge these two similar things into one, but have some opens:

Most of the GTT/PPGTT page table entry routines in i915_gem_gtt.c, e.g. the
abstractions/ insert_entries() are aimed to generate the page table entry, but
GVT-g shadow page implementation also need the per-platform page table
entry bit field extraction routines. For example, extract the GFN from guest page
table, which means we have to add some new callbacks which native i915
will not use at all.  Is it OK for host i915 to add such kinds of callbacks?

b.  GVT-g shadow page table implementation should be the most complicated
part in GVT-g, maybe the first easy step should be putting the shadow page
table root pointer into i915_hw_ppgtt. E.g. GVT-g allocates a fake i915_hw_ppgtt
only use it to store root pointer and addressing mode bit?

For ppgtt size selection I think we should have a field for that in 
i915_hw_ppgtt, that the lrc setup code reads.

For the ringbuffer size you could just create a fake ringbuffer object I 
think.

[Zhi] You mean we should add another ring buffer object allocation function?  In GVT
context we reuse the i915 ring buffer submission interface like intel_ring_{begin,
advance}. If we follow this direction above, probably we should abstract the ringbuffer
object allocation functions, and put different ring buffer allocation callbacks when
initialize the intel ring buffer object?

-Daniel
>
> Thanks,
> Zhi.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Discuss GVT context hacks in i915
  2016-02-15 16:03   ` Wang, Zhi A
@ 2016-02-15 17:20     ` Daniel Vetter
  2016-02-16  1:09       ` Wang, Zhi A
  2016-02-16  3:11     ` Tian, Kevin
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2016-02-15 17:20 UTC (permalink / raw)
  To: Wang, Zhi A; +Cc: intel-gfx@lists.freedesktop.org, Vetter, Daniel

On Mon, Feb 15, 2016 at 04:03:49PM +0000, Wang, Zhi A wrote:
> Hi  Daniel:
>     Thanks for shedding the light! See my comments below. :) 
> 
> -----Original Message-----
> From: Vetter, Daniel 
> Sent: Monday, February 15, 2016 5:32 PM
> To: Wang, Zhi A; Joonas Lahtinen; Chris Wilson; Lv, Zhiyuan; Tian, Kevin
> Subject: Re: Discuss GVT context hacks in i915
> 
> Am 15/02/2016 um 10:11 schrieb Zhi Wang:
> > Hi Guys:
> >    Previously we have sent our GVT-g device model RFC code, and thanks 
> > for the advices and comments! As you see GVT-g needs a special kind of 
> > LRC context as shadow context so it could be able to use i915 GEM 
> > submission subsystem, which requires some changes in i915.
> >
> > Probably we can discuss them one by one and settle them down before 
> > the next version of RFC code. :)
> >
> > Difference between a i915 LRC context and a GVT LRC context
> > ------
> >
> > * The engine context initialization of a GVT LRC context can be 
> > bypassed, as GVT-g will initialize it by guest LRC context.
> >
> > http://www.spinics.net/lists/intel-gfx/msg86546.html
> >
> > * The addressing mode bit in the context descriptor should be able to 
> > configured at runtime, as some guest is still using 32bit PPGTT 
> > address space.
> >
> > http://www.spinics.net/lists/intel-gfx/msg86544.html
> >
> > * As the GVT LRC context will go with the shadow PPGTT page table 
> > populated by GVT-g, a GVT LRC context doesn't need a i915 PPGTT 
> > instance, also its PDP root pointer doesn't need to be populated by i915.
> >
> > http://www.spinics.net/lists/intel-gfx/msg86545.html
> >
> > * The ring buffer size of a GVT LRC context ring buffer object should 
> > be configurable or hard-coded to the max size, as some guest (e.g. 
> > windows) has a large ring buffer and could submit a lot of commands at 
> > one time.
> >
> > Hacking i915
> > -------
> >
> > The above changes needs to hack i915, in the RFC code, you can see a 
> > lot of codes like below:
> >
> > if (!gvt_context) {
> >     /* do something i915 needed */
> > } else {
> >     /* do something GVT-g needed */
> > }
> >
> > This kinds of code piece is GVT-g specific, another approach should be 
> > refining some i915 APIs or say something generic, like:
> >
> > if (context->need_i915_ppgtt) {
> >     /* allocating i915 ppgtt instance */
> > }
> >
> > if (context->ppgtt) {
> >     /* update PDP root pointers in LRC context. */
> > }
> >
> > if (context->need_initialization) {
> >     /* emit commands to initialize the engine context of LRC context */
> > }
> >
> > I'm wondering which kinds do you prefer? Or maybe you can give me some 
> > advices? :)
> 
> Without looking at more than what's in your mail here and in the links, 
> this style is the midlayer mistake: We have some abstraction (lrc 
> context, ppgtt) and force everyone to go through the same code paths. 
> Which means deep down in those paths we have lots of if (special_case) 
> conditions, which make the code a maintainance nightmare. Yes execlist 
> support has already a lot of such bad examples unfortunately.
> 
> The better design idea is to reuse the data structures and helper 
> functions, but have new top-level entry functions for creating e.g. a 
> xengt lrc context. So e.g. have a lrc init function for xengt which 
> takes the setup stuff as parameters. Wrt ppgtt my idea was to reuse 
> struct i915_hw_ppgtt for managing the shadow pagetable, with xengt using 
> the i915_gem_gtt.c functions to write shadow pagetable entries. That way 
> i915 still knows the virtual->physical mapping, which aids in e.g. crash 
> dump recording. Of course you're not going to bind entire vma, but 
> instead will use the lower-level functions that just bind pages.
> 
> [Zhi] Thanks! Just want to make sure that you prefer that GVT-g specific
> modifications should be put into a fork of top-level i915 APIs? For example,
> we prepare a new function to create the GVT context, which is a fork of
> simplified i915_gem_create_context().
> 
> For i915_hw_ppgtt and GVT-g shadow page table, we tried to think about how
> to merge these two similar things into one, but have some opens:
> 
> Most of the GTT/PPGTT page table entry routines in i915_gem_gtt.c, e.g. the
> abstractions/ insert_entries() are aimed to generate the page table entry, but
> GVT-g shadow page implementation also need the per-platform page table
> entry bit field extraction routines. For example, extract the GFN from guest page
> table, which means we have to add some new callbacks which native i915
> will not use at all.  Is it OK for host i915 to add such kinds of callbacks?
> 
> b.  GVT-g shadow page table implementation should be the most complicated
> part in GVT-g, maybe the first easy step should be putting the shadow page
> table root pointer into i915_hw_ppgtt. E.g. GVT-g allocates a fake i915_hw_ppgtt
> only use it to store root pointer and addressing mode bit?
> 
> For ppgtt size selection I think we should have a field for that in 
> i915_hw_ppgtt, that the lrc setup code reads.
> 
> For the ringbuffer size you could just create a fake ringbuffer object I 
> think.
> 
> [Zhi] You mean we should add another ring buffer object allocation function?  In GVT
> context we reuse the i915 ring buffer submission interface like intel_ring_{begin,
> advance}. If we follow this direction above, probably we should abstract the ringbuffer
> object allocation functions, and put different ring buffer allocation callbacks when
> initialize the intel ring buffer object?

Like I said I didn't look at the code. If you already use the existing
ringbuffer alloc functions just add a low-levl one that has an additional
size parameter instead of the deafult. Assuming that works for you.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Discuss GVT context hacks in i915
  2016-02-15 17:20     ` Daniel Vetter
@ 2016-02-16  1:09       ` Wang, Zhi A
  0 siblings, 0 replies; 6+ messages in thread
From: Wang, Zhi A @ 2016-02-16  1:09 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org, Vetter, Daniel

Pretty clear. Thanks for the ideas! They really inspire me. :)

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, February 16, 2016 1:20 AM
To: Wang, Zhi A
Cc: Vetter, Daniel; Joonas Lahtinen; Chris Wilson; Lv, Zhiyuan; Tian, Kevin; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] Discuss GVT context hacks in i915

On Mon, Feb 15, 2016 at 04:03:49PM +0000, Wang, Zhi A wrote:
> Hi  Daniel:
>     Thanks for shedding the light! See my comments below. :) 
> 
> -----Original Message-----
> From: Vetter, Daniel 
> Sent: Monday, February 15, 2016 5:32 PM
> To: Wang, Zhi A; Joonas Lahtinen; Chris Wilson; Lv, Zhiyuan; Tian, Kevin
> Subject: Re: Discuss GVT context hacks in i915
> 
> Am 15/02/2016 um 10:11 schrieb Zhi Wang:
> > Hi Guys:
> >    Previously we have sent our GVT-g device model RFC code, and thanks 
> > for the advices and comments! As you see GVT-g needs a special kind of 
> > LRC context as shadow context so it could be able to use i915 GEM 
> > submission subsystem, which requires some changes in i915.
> >
> > Probably we can discuss them one by one and settle them down before 
> > the next version of RFC code. :)
> >
> > Difference between a i915 LRC context and a GVT LRC context
> > ------
> >
> > * The engine context initialization of a GVT LRC context can be 
> > bypassed, as GVT-g will initialize it by guest LRC context.
> >
> > http://www.spinics.net/lists/intel-gfx/msg86546.html
> >
> > * The addressing mode bit in the context descriptor should be able to 
> > configured at runtime, as some guest is still using 32bit PPGTT 
> > address space.
> >
> > http://www.spinics.net/lists/intel-gfx/msg86544.html
> >
> > * As the GVT LRC context will go with the shadow PPGTT page table 
> > populated by GVT-g, a GVT LRC context doesn't need a i915 PPGTT 
> > instance, also its PDP root pointer doesn't need to be populated by i915.
> >
> > http://www.spinics.net/lists/intel-gfx/msg86545.html
> >
> > * The ring buffer size of a GVT LRC context ring buffer object should 
> > be configurable or hard-coded to the max size, as some guest (e.g. 
> > windows) has a large ring buffer and could submit a lot of commands at 
> > one time.
> >
> > Hacking i915
> > -------
> >
> > The above changes needs to hack i915, in the RFC code, you can see a 
> > lot of codes like below:
> >
> > if (!gvt_context) {
> >     /* do something i915 needed */
> > } else {
> >     /* do something GVT-g needed */
> > }
> >
> > This kinds of code piece is GVT-g specific, another approach should be 
> > refining some i915 APIs or say something generic, like:
> >
> > if (context->need_i915_ppgtt) {
> >     /* allocating i915 ppgtt instance */
> > }
> >
> > if (context->ppgtt) {
> >     /* update PDP root pointers in LRC context. */
> > }
> >
> > if (context->need_initialization) {
> >     /* emit commands to initialize the engine context of LRC context */
> > }
> >
> > I'm wondering which kinds do you prefer? Or maybe you can give me some 
> > advices? :)
> 
> Without looking at more than what's in your mail here and in the links, 
> this style is the midlayer mistake: We have some abstraction (lrc 
> context, ppgtt) and force everyone to go through the same code paths. 
> Which means deep down in those paths we have lots of if (special_case) 
> conditions, which make the code a maintainance nightmare. Yes execlist 
> support has already a lot of such bad examples unfortunately.
> 
> The better design idea is to reuse the data structures and helper 
> functions, but have new top-level entry functions for creating e.g. a 
> xengt lrc context. So e.g. have a lrc init function for xengt which 
> takes the setup stuff as parameters. Wrt ppgtt my idea was to reuse 
> struct i915_hw_ppgtt for managing the shadow pagetable, with xengt using 
> the i915_gem_gtt.c functions to write shadow pagetable entries. That way 
> i915 still knows the virtual->physical mapping, which aids in e.g. crash 
> dump recording. Of course you're not going to bind entire vma, but 
> instead will use the lower-level functions that just bind pages.
> 
> [Zhi] Thanks! Just want to make sure that you prefer that GVT-g specific
> modifications should be put into a fork of top-level i915 APIs? For example,
> we prepare a new function to create the GVT context, which is a fork of
> simplified i915_gem_create_context().
> 
> For i915_hw_ppgtt and GVT-g shadow page table, we tried to think about how
> to merge these two similar things into one, but have some opens:
> 
> Most of the GTT/PPGTT page table entry routines in i915_gem_gtt.c, e.g. the
> abstractions/ insert_entries() are aimed to generate the page table entry, but
> GVT-g shadow page implementation also need the per-platform page table
> entry bit field extraction routines. For example, extract the GFN from guest page
> table, which means we have to add some new callbacks which native i915
> will not use at all.  Is it OK for host i915 to add such kinds of callbacks?
> 
> b.  GVT-g shadow page table implementation should be the most complicated
> part in GVT-g, maybe the first easy step should be putting the shadow page
> table root pointer into i915_hw_ppgtt. E.g. GVT-g allocates a fake i915_hw_ppgtt
> only use it to store root pointer and addressing mode bit?
> 
> For ppgtt size selection I think we should have a field for that in 
> i915_hw_ppgtt, that the lrc setup code reads.
> 
> For the ringbuffer size you could just create a fake ringbuffer object I 
> think.
> 
> [Zhi] You mean we should add another ring buffer object allocation function?  In GVT
> context we reuse the i915 ring buffer submission interface like intel_ring_{begin,
> advance}. If we follow this direction above, probably we should abstract the ringbuffer
> object allocation functions, and put different ring buffer allocation callbacks when
> initialize the intel ring buffer object?

Like I said I didn't look at the code. If you already use the existing
ringbuffer alloc functions just add a low-levl one that has an additional
size parameter instead of the deafult. Assuming that works for you.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Discuss GVT context hacks in i915
  2016-02-15 16:03   ` Wang, Zhi A
  2016-02-15 17:20     ` Daniel Vetter
@ 2016-02-16  3:11     ` Tian, Kevin
  2016-02-16  4:08       ` Zhi Wang
  1 sibling, 1 reply; 6+ messages in thread
From: Tian, Kevin @ 2016-02-16  3:11 UTC (permalink / raw)
  To: Wang, Zhi A, Vetter, Daniel, Joonas Lahtinen, Chris Wilson,
	Lv, Zhiyuan
  Cc: intel-gfx@lists.freedesktop.org

> From: Wang, Zhi A
> Sent: Tuesday, February 16, 2016 12:04 AM
> 
> The better design idea is to reuse the data structures and helper
> functions, but have new top-level entry functions for creating e.g. a
> xengt lrc context. So e.g. have a lrc init function for xengt which
> takes the setup stuff as parameters. Wrt ppgtt my idea was to reuse
> struct i915_hw_ppgtt for managing the shadow pagetable, with xengt using
> the i915_gem_gtt.c functions to write shadow pagetable entries. That way
> i915 still knows the virtual->physical mapping, which aids in e.g. crash
> dump recording. Of course you're not going to bind entire vma, but
> instead will use the lower-level functions that just bind pages.
> 
> [Zhi] Thanks! Just want to make sure that you prefer that GVT-g specific
> modifications should be put into a fork of top-level i915 APIs? For example,
> we prepare a new function to create the GVT context, which is a fork of
> simplified i915_gem_create_context().

Not specific for GVT. You need to make it generic to accept any lrc init
function where GVT is just one user.

> 
> For i915_hw_ppgtt and GVT-g shadow page table, we tried to think about how
> to merge these two similar things into one, but have some opens:
> 
> Most of the GTT/PPGTT page table entry routines in i915_gem_gtt.c, e.g. the
> abstractions/ insert_entries() are aimed to generate the page table entry, but
> GVT-g shadow page implementation also need the per-platform page table
> entry bit field extraction routines. For example, extract the GFN from guest page
> table, which means we have to add some new callbacks which native i915
> will not use at all.  Is it OK for host i915 to add such kinds of callbacks?

What Daniel suggested is to reuse low level functions to write shadow
PTE entries. It's not about how we sync shadow PTE content from guest
PTE content. So how to extract GFN from guest page table will be still
kept within GVT shadow code. Only when GVT shadow wants to operate
shadow PTE entries, it goes to i915_gem_gtt.c.

> 
> b.  GVT-g shadow page table implementation should be the most complicated
> part in GVT-g, maybe the first easy step should be putting the shadow page
> table root pointer into i915_hw_ppgtt. E.g. GVT-g allocates a fake i915_hw_ppgtt
> only use it to store root pointer and addressing mode bit?

Not a 'fake' one. It a real i915_hw_ppgtt but in a special mode that the 
actual mgmt. logic comes from another place (GVT shadow) but the low 
level interface can be reused (possibly some slight changes still required)

Thanks
Kevin
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Discuss GVT context hacks in i915
  2016-02-16  3:11     ` Tian, Kevin
@ 2016-02-16  4:08       ` Zhi Wang
  0 siblings, 0 replies; 6+ messages in thread
From: Zhi Wang @ 2016-02-16  4:08 UTC (permalink / raw)
  To: Tian, Kevin, Vetter, Daniel, Joonas Lahtinen, Chris Wilson,
	Lv, Zhiyuan
  Cc: intel-gfx@lists.freedesktop.org

Hi:
     Thanks Kevin! See my comments below.

On 02/16/16 11:11, Tian, Kevin wrote:
>> From: Wang, Zhi A
>> Sent: Tuesday, February 16, 2016 12:04 AM
>>
>> The better design idea is to reuse the data structures and helper
>> functions, but have new top-level entry functions for creating e.g. a
>> xengt lrc context. So e.g. have a lrc init function for xengt which
>> takes the setup stuff as parameters. Wrt ppgtt my idea was to reuse
>> struct i915_hw_ppgtt for managing the shadow pagetable, with xengt using
>> the i915_gem_gtt.c functions to write shadow pagetable entries. That way
>> i915 still knows the virtual->physical mapping, which aids in e.g. crash
>> dump recording. Of course you're not going to bind entire vma, but
>> instead will use the lower-level functions that just bind pages.
>>
>> [Zhi] Thanks! Just want to make sure that you prefer that GVT-g specific
>> modifications should be put into a fork of top-level i915 APIs? For example,
>> we prepare a new function to create the GVT context, which is a fork of
>> simplified i915_gem_create_context().
>
> Not specific for GVT. You need to make it generic to accept any lrc init
> function where GVT is just one user.
>

@Daniel, Just want to clarify the "new top-level entry functions "here, 
should I refine the related i915 APIs as below:

int xengt_i915_api()
{
	call i915_common_low_level_api();
}

or:

native_callbacks();
xengt_callbacks();

int i915_api(*callbacks);

>>
>> For i915_hw_ppgtt and GVT-g shadow page table, we tried to think about how
>> to merge these two similar things into one, but have some opens:
>>
>> Most of the GTT/PPGTT page table entry routines in i915_gem_gtt.c, e.g. the
>> abstractions/ insert_entries() are aimed to generate the page table entry, but
>> GVT-g shadow page implementation also need the per-platform page table
>> entry bit field extraction routines. For example, extract the GFN from guest page
>> table, which means we have to add some new callbacks which native i915
>> will not use at all.  Is it OK for host i915 to add such kinds of callbacks?
>
> What Daniel suggested is to reuse low level functions to write shadow
> PTE entries. It's not about how we sync shadow PTE content from guest
> PTE content. So how to extract GFN from guest page table will be still
> kept within GVT shadow code. Only when GVT shadow wants to operate
> shadow PTE entries, it goes to i915_gem_gtt.c.
>
>>
>> b.  GVT-g shadow page table implementation should be the most complicated
>> part in GVT-g, maybe the first easy step should be putting the shadow page
>> table root pointer into i915_hw_ppgtt. E.g. GVT-g allocates a fake i915_hw_ppgtt
>> only use it to store root pointer and addressing mode bit?
>
> Not a 'fake' one. It a real i915_hw_ppgtt but in a special mode that the
> actual mgmt. logic comes from another place (GVT shadow) but the low
> level interface can be reused (possibly some slight changes still required)
>

OK. That's much better! we could still keep some bitfield extraction 
routines inside GVT-g.

> Thanks
> Kevin
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-02-16  4:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <56C19630.5010301@intel.com>
2016-02-15  9:13 ` Fwd: Discuss GVT context hacks in i915 Zhi Wang
     [not found] ` <56C19B26.7020206@intel.com>
2016-02-15 16:03   ` Wang, Zhi A
2016-02-15 17:20     ` Daniel Vetter
2016-02-16  1:09       ` Wang, Zhi A
2016-02-16  3:11     ` Tian, Kevin
2016-02-16  4:08       ` Zhi Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.