diff mbox series

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

Message ID 20190308075238.1778-1-zhenyuw@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: relax debug BUG_ON() for closed context in hw_id pin | expand

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
diff mbox series

Patch

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));