[PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Zhiyuan Lv]

Broadwell hardware supports both ring buffer mode and execlist mode.
When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
only. The reason is that GVT-g does not support the dynamic mode
switch between ring buffer mode and execlist mode when running
multiple virtual machines. Consider that ring buffer mode is legacy
mode, it makes sense to drop it inside virtual machines.

Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2dc8709..39df304 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -239,6 +239,9 @@ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists
 	if (INTEL_INFO(dev)->gen >= 9)
 		return 1;
 
+	if (HAS_LOGICAL_RING_CONTEXTS(dev) && intel_vgpu_active(dev))
+		return 1;
+
 	if (enable_execlists == 0)
 		return 0;
 
-- 
1.9.1

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

[PATCH 0/7] drm/intel: guest i915 changes for Broadwell to run inside VM with Intel GVT-g

[Zhiyuan Lv]

I915 kernel driver can now work inside a virtual machine on Haswell
with Intel GVT-g. In order to do the same thing on Broadwell, there
are some extra changes needed. The two main things are to support the
more complicated PPGTT page table structure and EXECLIST contexts.
GVT-g will perform shadow PPGTT and shadow context, which requires
guest driver to explicitly notify host device model the life cycle of
PPGTT and EXECLIST contexts.

The first and the forth patches added some restrictions to drivers in
virtualization scenario to make the shadow work easier. The first
patch is based on Mika's earlier one, but we use it for vgpu only.
The sixth patch is the implementation of the notification for
shadowing.

Zhiyuan Lv (7):
  drm/i915: preallocate pdps for 32 bit vgpu
  drm/i915: Enable full ppgtt for vgpu
  drm/i915: Always enable execlists on BDW for vgpu
  drm/i915: always pin lrc context for vgpu with Intel GVT-g
  drm/i915: Update PV INFO page definition for Intel GVT-g
  drm/i915: guest i915 notification for Intel-GVTg
  drm/i915: Allow Broadwell guest with Intel GVT-g

 drivers/gpu/drm/i915/i915_gem_gtt.c | 77 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_vgpu.c    |  2 +-
 drivers/gpu/drm/i915/i915_vgpu.h    | 34 +++++++++++++++-
 drivers/gpu/drm/i915/intel_lrc.c    | 44 ++++++++++++++++++---
 4 files changed, 145 insertions(+), 12 deletions(-)

-- 
1.9.1

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

[PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Zhiyuan Lv]

Broadwell hardware supports both ring buffer mode and execlist mode.
When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
only. The reason is that GVT-g does not support the dynamic mode
switch between ring buffer mode and execlist mode when running
multiple virtual machines. Consider that ring buffer mode is legacy
mode, it makes sense to drop it inside virtual machines.

Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2dc8709..39df304 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -239,6 +239,9 @@ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists
 	if (INTEL_INFO(dev)->gen >= 9)
 		return 1;
 
+	if (HAS_LOGICAL_RING_CONTEXTS(dev) && intel_vgpu_active(dev))
+		return 1;
+
 	if (enable_execlists == 0)
 		return 0;
 
-- 
1.9.1

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

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Chris Wilson]

On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> Broadwell hardware supports both ring buffer mode and execlist mode.
> When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> only. The reason is that GVT-g does not support the dynamic mode
> switch between ring buffer mode and execlist mode when running
> multiple virtual machines. Consider that ring buffer mode is legacy
> mode, it makes sense to drop it inside virtual machines.

If that is the case, you should query the host as to what mode it is
running.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Zhiyuan Lv]

Hi Chris,

On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > Broadwell hardware supports both ring buffer mode and execlist mode.
> > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> > only. The reason is that GVT-g does not support the dynamic mode
> > switch between ring buffer mode and execlist mode when running
> > multiple virtual machines. Consider that ring buffer mode is legacy
> > mode, it makes sense to drop it inside virtual machines.
> 
> If that is the case, you should query the host as to what mode it is
> running.

Thanks for the reply! You mean we query the host mode, then tell the
guest driver inside VM, so that it could use the same mode as host
right? That might be a little complicated, and the only benefit is to
support legacy ring buffer mode ...

And GVT-g host implementation was not well tested with ring buffer mode
since it cannot start windows VM. Probably I should change the commit
message to say explicitly that ring buffer mode is not supported with
GVT-g? Thanks!

Regards,
-Zhiyuan

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Chris Wilson]

On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> Hi Chris,
> 
> On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > Broadwell hardware supports both ring buffer mode and execlist mode.
> > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> > > only. The reason is that GVT-g does not support the dynamic mode
> > > switch between ring buffer mode and execlist mode when running
> > > multiple virtual machines. Consider that ring buffer mode is legacy
> > > mode, it makes sense to drop it inside virtual machines.
> > 
> > If that is the case, you should query the host as to what mode it is
> > running.
> 
> Thanks for the reply! You mean we query the host mode, then tell the
> guest driver inside VM, so that it could use the same mode as host
> right? That might be a little complicated, and the only benefit is to
> support legacy ring buffer mode ...

The only benefit being that the guest works no matter what the host
does?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Zhiyuan Lv]

On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > Hi Chris,
> > 
> > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > Broadwell hardware supports both ring buffer mode and execlist mode.
> > > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> > > > only. The reason is that GVT-g does not support the dynamic mode
> > > > switch between ring buffer mode and execlist mode when running
> > > > multiple virtual machines. Consider that ring buffer mode is legacy
> > > > mode, it makes sense to drop it inside virtual machines.
> > > 
> > > If that is the case, you should query the host as to what mode it is
> > > running.
> > 
> > Thanks for the reply! You mean we query the host mode, then tell the
> > guest driver inside VM, so that it could use the same mode as host
> > right? That might be a little complicated, and the only benefit is to
> > support legacy ring buffer mode ...
> 
> The only benefit being that the guest works no matter what the host
> does?

Supporting ring buffer mode may need more work in GVT-g. When we started to
enable BDW support, ring buffer mode used to work but was not well tested. And
the inter-VM switch (all in ringbuffer mode) may be tricker comparing with
driver context switch with "MI_SET_CONTEXT", because we need to switch ring
buffer whereas driver does not. Regarding this, the EXECLIST mode looks
cleaner. In order to support that, we may have to: 1, change more LRI commands
to MMIO in current driver; 2, more testing/debugging of inter-VM context
switch flow.

Based on that, I think we should really make statement that "ring buffer mode"
is not supported by GVT-g on BDW :-)

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Joonas Lahtinen]

Hi,

On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > Hi Chris,
> > > 
> > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > execlist mode.
> > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > execlist mode
> > > > > only. The reason is that GVT-g does not support the dynamic 
> > > > > mode
> > > > > switch between ring buffer mode and execlist mode when 
> > > > > running
> > > > > multiple virtual machines. Consider that ring buffer mode is 
> > > > > legacy
> > > > > mode, it makes sense to drop it inside virtual machines.
> > > > 
> > > > If that is the case, you should query the host as to what mode 
> > > > it is
> > > > running.
> > > 
> > > Thanks for the reply! You mean we query the host mode, then tell 
> > > the
> > > guest driver inside VM, so that it could use the same mode as 
> > > host
> > > right? That might be a little complicated, and the only benefit 
> > > is to
> > > support legacy ring buffer mode ...
> > 
> > The only benefit being that the guest works no matter what the host
> > does?
> 
> Supporting ring buffer mode may need more work in GVT-g. When we 
> started to
> enable BDW support, ring buffer mode used to work but was not well 
> tested. And
> the inter-VM switch (all in ringbuffer mode) may be tricker comparing 
> with
> driver context switch with "MI_SET_CONTEXT", because we need to 
> switch ring
> buffer whereas driver does not. Regarding this, the EXECLIST mode 
> looks
> cleaner. In order to support that, we may have to: 1, change more LRI 
> commands
> to MMIO in current driver; 2, more testing/debugging of inter-VM 
> context
> switch flow.
> 
> Based on that, I think we should really make statement that "ring 
> buffer mode"
> is not supported by GVT-g on BDW :-)
> 

I think just move the vpgu test even before test for GEN9 just making
it return true on intel_vgpu_active(dev) and add a comment that
currently vGPU only supports execlist command submission, and then add
an error early in the init in some appropriate spot if
intel_vgpu_active is true but logical ring context are not available
(which would practically mean GEN < 8).

Does that sound OK?

Regards, Joonas

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

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Zhiyuan Lv]

Hi Joonas,

Thanks for the review! And my reply inline.

Regards,
-Zhiyuan

On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote:
> Hi,
> 
> On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > > Hi Chris,
> > > > 
> > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > > execlist mode.
> > > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > > execlist mode
> > > > > > only. The reason is that GVT-g does not support the dynamic 
> > > > > > mode
> > > > > > switch between ring buffer mode and execlist mode when 
> > > > > > running
> > > > > > multiple virtual machines. Consider that ring buffer mode is 
> > > > > > legacy
> > > > > > mode, it makes sense to drop it inside virtual machines.
> > > > > 
> > > > > If that is the case, you should query the host as to what mode 
> > > > > it is
> > > > > running.
> > > > 
> > > > Thanks for the reply! You mean we query the host mode, then tell 
> > > > the
> > > > guest driver inside VM, so that it could use the same mode as 
> > > > host
> > > > right? That might be a little complicated, and the only benefit 
> > > > is to
> > > > support legacy ring buffer mode ...
> > > 
> > > The only benefit being that the guest works no matter what the host
> > > does?
> > 
> > Supporting ring buffer mode may need more work in GVT-g. When we 
> > started to
> > enable BDW support, ring buffer mode used to work but was not well 
> > tested. And
> > the inter-VM switch (all in ringbuffer mode) may be tricker comparing 
> > with
> > driver context switch with "MI_SET_CONTEXT", because we need to 
> > switch ring
> > buffer whereas driver does not. Regarding this, the EXECLIST mode 
> > looks
> > cleaner. In order to support that, we may have to: 1, change more LRI 
> > commands
> > to MMIO in current driver; 2, more testing/debugging of inter-VM 
> > context
> > switch flow.
> > 
> > Based on that, I think we should really make statement that "ring 
> > buffer mode"
> > is not supported by GVT-g on BDW :-)
> > 
> 
> I think just move the vpgu test even before test for GEN9 just making
> it return true on intel_vgpu_active(dev) and add a comment that
> currently vGPU only supports execlist command submission, and then add

Will do. Thanks!

> an error early in the init in some appropriate spot if
> intel_vgpu_active is true but logical ring context are not available
> (which would practically mean GEN < 8).
> 
> Does that sound OK?

Ring buffer mode for Haswell with vgpu is still supported. So probably I have
the check like below? Thanks!

@@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device *dev)
        if (WARN_ON(dev_priv->ring[RCS].default_context))
                return 0;
 
+       if (intel_vgpu_active(dev)) {
+               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
+                           !i915.enable_execlist))
+                       return 0;
+       }
+
        if (i915.enable_execlists) {
                /* NB: intentionally left blank. We will allocate our own
                 * backing objects as we need them, thank you very much */

> 
> Regards, Joonas
> 
> > > -Chris
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Joonas Lahtinen]

On pe, 2015-08-21 at 10:24 +0800, Zhiyuan Lv wrote:
> Hi Joonas,
> 
> Thanks for the review! And my reply inline.
> 
> Regards,
> -Zhiyuan
> 
> On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote:
> > Hi,
> > 
> > On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> > > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > > > Hi Chris,
> > > > > 
> > > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > > > execlist mode.
> > > > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > > > execlist mode
> > > > > > > only. The reason is that GVT-g does not support the 
> > > > > > > dynamic 
> > > > > > > mode
> > > > > > > switch between ring buffer mode and execlist mode when 
> > > > > > > running
> > > > > > > multiple virtual machines. Consider that ring buffer mode 
> > > > > > > is 
> > > > > > > legacy
> > > > > > > mode, it makes sense to drop it inside virtual machines.
> > > > > > 
> > > > > > If that is the case, you should query the host as to what 
> > > > > > mode 
> > > > > > it is
> > > > > > running.
> > > > > 
> > > > > Thanks for the reply! You mean we query the host mode, then 
> > > > > tell 
> > > > > the
> > > > > guest driver inside VM, so that it could use the same mode as 
> > > > > host
> > > > > right? That might be a little complicated, and the only 
> > > > > benefit 
> > > > > is to
> > > > > support legacy ring buffer mode ...
> > > > 
> > > > The only benefit being that the guest works no matter what the 
> > > > host
> > > > does?
> > > 
> > > Supporting ring buffer mode may need more work in GVT-g. When we 
> > > started to
> > > enable BDW support, ring buffer mode used to work but was not 
> > > well 
> > > tested. And
> > > the inter-VM switch (all in ringbuffer mode) may be tricker 
> > > comparing 
> > > with
> > > driver context switch with "MI_SET_CONTEXT", because we need to 
> > > switch ring
> > > buffer whereas driver does not. Regarding this, the EXECLIST mode 
> > > looks
> > > cleaner. In order to support that, we may have to: 1, change more 
> > > LRI 
> > > commands
> > > to MMIO in current driver; 2, more testing/debugging of inter-VM 
> > > context
> > > switch flow.
> > > 
> > > Based on that, I think we should really make statement that "ring 
> > > buffer mode"
> > > is not supported by GVT-g on BDW :-)
> > > 
> > 
> > I think just move the vpgu test even before test for GEN9 just 
> > making
> > it return true on intel_vgpu_active(dev) and add a comment that
> > currently vGPU only supports execlist command submission, and then 
> > add
> 
> Will do. Thanks!
> 
> > an error early in the init in some appropriate spot if
> > intel_vgpu_active is true but logical ring context are not 
> > available
> > (which would practically mean GEN < 8).
> > 
> > Does that sound OK?
> 
> Ring buffer mode for Haswell with vgpu is still supported. So 
> probably I have
> the check like below? Thanks!
> 

Ah sorry, I missed that.

> @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> *dev)
>         if (WARN_ON(dev_priv->ring[RCS].default_context))
>                 return 0;
>  
> +       if (intel_vgpu_active(dev)) {
> +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> +                           !i915.enable_execlist))
> +                       return 0;
> +       }
> +

This looks fine to me. Maybe comment might be in place stating that
support is not yet implemented, but could be.

Regards, Joonas

>         if (i915.enable_execlists) {
>                 /* NB: intentionally left blank. We will allocate our 
> own
>                  * backing objects as we need them, thank you very 
> much */
> 
> > 
> > Regards, Joonas
> > 
> > > > -Chris
> > > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Daniel Vetter]

On Mon, Aug 24, 2015 at 03:36:46PM +0300, Joonas Lahtinen wrote:
> On pe, 2015-08-21 at 10:24 +0800, Zhiyuan Lv wrote:
> > Hi Joonas,
> > 
> > Thanks for the review! And my reply inline.
> > 
> > Regards,
> > -Zhiyuan
> > 
> > On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote:
> > > Hi,
> > > 
> > > On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> > > > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > > > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > > > > Hi Chris,
> > > > > > 
> > > > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > > > > execlist mode.
> > > > > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > > > > execlist mode
> > > > > > > > only. The reason is that GVT-g does not support the 
> > > > > > > > dynamic 
> > > > > > > > mode
> > > > > > > > switch between ring buffer mode and execlist mode when 
> > > > > > > > running
> > > > > > > > multiple virtual machines. Consider that ring buffer mode 
> > > > > > > > is 
> > > > > > > > legacy
> > > > > > > > mode, it makes sense to drop it inside virtual machines.
> > > > > > > 
> > > > > > > If that is the case, you should query the host as to what 
> > > > > > > mode 
> > > > > > > it is
> > > > > > > running.
> > > > > > 
> > > > > > Thanks for the reply! You mean we query the host mode, then 
> > > > > > tell 
> > > > > > the
> > > > > > guest driver inside VM, so that it could use the same mode as 
> > > > > > host
> > > > > > right? That might be a little complicated, and the only 
> > > > > > benefit 
> > > > > > is to
> > > > > > support legacy ring buffer mode ...
> > > > > 
> > > > > The only benefit being that the guest works no matter what the 
> > > > > host
> > > > > does?
> > > > 
> > > > Supporting ring buffer mode may need more work in GVT-g. When we 
> > > > started to
> > > > enable BDW support, ring buffer mode used to work but was not 
> > > > well 
> > > > tested. And
> > > > the inter-VM switch (all in ringbuffer mode) may be tricker 
> > > > comparing 
> > > > with
> > > > driver context switch with "MI_SET_CONTEXT", because we need to 
> > > > switch ring
> > > > buffer whereas driver does not. Regarding this, the EXECLIST mode 
> > > > looks
> > > > cleaner. In order to support that, we may have to: 1, change more 
> > > > LRI 
> > > > commands
> > > > to MMIO in current driver; 2, more testing/debugging of inter-VM 
> > > > context
> > > > switch flow.
> > > > 
> > > > Based on that, I think we should really make statement that "ring 
> > > > buffer mode"
> > > > is not supported by GVT-g on BDW :-)
> > > > 
> > > 
> > > I think just move the vpgu test even before test for GEN9 just 
> > > making
> > > it return true on intel_vgpu_active(dev) and add a comment that
> > > currently vGPU only supports execlist command submission, and then 
> > > add
> > 
> > Will do. Thanks!
> > 
> > > an error early in the init in some appropriate spot if
> > > intel_vgpu_active is true but logical ring context are not 
> > > available
> > > (which would practically mean GEN < 8).
> > > 
> > > Does that sound OK?
> > 
> > Ring buffer mode for Haswell with vgpu is still supported. So 
> > probably I have
> > the check like below? Thanks!
> > 
> 
> Ah sorry, I missed that.
> 
> > @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> > *dev)
> >         if (WARN_ON(dev_priv->ring[RCS].default_context))
> >                 return 0;
> >  
> > +       if (intel_vgpu_active(dev)) {
> > +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> > +                           !i915.enable_execlist))
> > +                       return 0;
> > +       }
> > +
> 
> This looks fine to me. Maybe comment might be in place stating that
> support is not yet implemented, but could be.

You should fail this instead so that i915.ko knows that the render side of
the gpu doesn't work. And maybe just DRM_INFO with a useful informational
notice?

Also same comment here: Don't we need to coordinate this a bit better with
the host?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Zhiyuan Lv]

Hi Daniel,

On Wed, Aug 26, 2015 at 10:50:23AM +0200, Daniel Vetter wrote:
> > > @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> > > *dev)
> > >         if (WARN_ON(dev_priv->ring[RCS].default_context))
> > >                 return 0;
> > >  
> > > +       if (intel_vgpu_active(dev)) {
> > > +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> > > +                           !i915.enable_execlist))
> > > +                       return 0;
> > > +       }
> > > +
> > 
> > This looks fine to me. Maybe comment might be in place stating that
> > support is not yet implemented, but could be.
> 
> You should fail this instead so that i915.ko knows that the render side of
> the gpu doesn't work. And maybe just DRM_INFO with a useful informational
> notice?

Thanks for the comments! Will change.

> 
> Also same comment here: Don't we need to coordinate this a bit better with
> the host?

In host side, if driver is running in ring buffer mode, we will let GVT-g
initialization fail, so that guest cannot set vgpu active. Would that be good
enough? Thanks!

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu

[Daniel Vetter]

On Thu, Aug 27, 2015 at 10:49:28AM +0800, Zhiyuan Lv wrote:
> Hi Daniel,
> 
> On Wed, Aug 26, 2015 at 10:50:23AM +0200, Daniel Vetter wrote:
> > > > @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> > > > *dev)
> > > >         if (WARN_ON(dev_priv->ring[RCS].default_context))
> > > >                 return 0;
> > > >  
> > > > +       if (intel_vgpu_active(dev)) {
> > > > +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> > > > +                           !i915.enable_execlist))
> > > > +                       return 0;
> > > > +       }
> > > > +
> > > 
> > > This looks fine to me. Maybe comment might be in place stating that
> > > support is not yet implemented, but could be.
> > 
> > You should fail this instead so that i915.ko knows that the render side of
> > the gpu doesn't work. And maybe just DRM_INFO with a useful informational
> > notice?
> 
> Thanks for the comments! Will change.
> 
> > 
> > Also same comment here: Don't we need to coordinate this a bit better with
> > the host?
> 
> In host side, if driver is running in ring buffer mode, we will let GVT-g
> initialization fail, so that guest cannot set vgpu active. Would that be good
> enough? Thanks!

Yeah that seems sensible.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx