drm/i915: relax debug BUG_ON() for closed context in hw_id pin
diff mbox series

Message ID 20190308075238.1778-1-zhenyuw@linux.intel.com
State New
Headers show
Series
  • drm/i915: relax debug BUG_ON() for closed context in hw_id pin
Related show

Commit Message

Zhenyu Wang March 8, 2019, 7:52 a.m. UTC
Current GVT created context is marked closed as not to be used for
host user. But its hw_id should still be used. So this is to relax
debug BUG_ON() in __i915_gem_context_pin_hw_id() for GVT contexts
which can use force single submission flag to identify.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Chris Wilson March 8, 2019, 8:31 a.m. UTC | #1
Quoting Zhenyu Wang (2019-03-08 07:52:37)
> Current GVT created context is marked closed as not to be used for
> host user. But its hw_id should still be used. So this is to relax
> debug BUG_ON() in __i915_gem_context_pin_hw_id() for GVT contexts
> which can use force single submission flag to identify.

The alternative strategy would be to always pin the id for GVT. How many
gvt contexts? One per host or one per client? Or we don't mark them as
closed (not so keen on that as it does provide some protection).

I think I'd rather delete the GEM_BUG_ON() if it's not invariant -- we
only escape it firing for kernel contexts because they pin their id.
-Chris
Zhenyu Wang March 9, 2019, 3:39 a.m. UTC | #2
On 2019.03.08 08:31:51 +0000, Chris Wilson wrote:
> Quoting Zhenyu Wang (2019-03-08 07:52:37)
> > Current GVT created context is marked closed as not to be used for
> > host user. But its hw_id should still be used. So this is to relax
> > debug BUG_ON() in __i915_gem_context_pin_hw_id() for GVT contexts
> > which can use force single submission flag to identify.
> 
> The alternative strategy would be to always pin the id for GVT. How many
> gvt contexts? One per host or one per client? Or we don't mark them as
> closed (not so keen on that as it does provide some protection).
>

Currently one per VM guest, always pin the id would also be good.

> I think I'd rather delete the GEM_BUG_ON() if it's not invariant -- we
> only escape it firing for kernel contexts because they pin their id.

I think anyway we'd better pin id for gvt context, as it's good to keep
it for one VM cycle.

thanks
Chris Wilson March 9, 2019, 5:41 p.m. UTC | #3
Quoting Zhenyu Wang (2019-03-09 03:39:36)
> On 2019.03.08 08:31:51 +0000, Chris Wilson wrote:
> > Quoting Zhenyu Wang (2019-03-08 07:52:37)
> > > Current GVT created context is marked closed as not to be used for
> > > host user. But its hw_id should still be used. So this is to relax
> > > debug BUG_ON() in __i915_gem_context_pin_hw_id() for GVT contexts
> > > which can use force single submission flag to identify.
> > 
> > The alternative strategy would be to always pin the id for GVT. How many
> > gvt contexts? One per host or one per client? Or we don't mark them as
> > closed (not so keen on that as it does provide some protection).
> >
> 
> Currently one per VM guest, always pin the id would also be good.
> 
> > I think I'd rather delete the GEM_BUG_ON() if it's not invariant -- we
> > only escape it firing for kernel contexts because they pin their id.
> 
> I think anyway we'd better pin id for gvt context, as it's good to keep
> it for one VM cycle.

Pin away, I have a plan to remove the hw-id limit...
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b9f321947982..0cbc5293da1c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1400,7 +1400,8 @@  int __i915_gem_context_pin_hw_id(struct i915_gem_context *ctx)
 
 	mutex_lock(&i915->contexts.mutex);
 
-	GEM_BUG_ON(i915_gem_context_is_closed(ctx));
+	GEM_BUG_ON(i915_gem_context_is_closed(ctx) &&
+		   !i915_gem_context_force_single_submission(ctx));
 
 	if (list_empty(&ctx->hw_id_link)) {
 		GEM_BUG_ON(atomic_read(&ctx->hw_id_pin_count));