Re: [igt-dev] [i-g-t] tests/i915/exec_balancer: Added Skip Guc Submission

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Nov 29, 2021 at 11:15:22AM +0000, Tvrtko Ursulin wrote:
> 
> On 29/11/2021 10:58, Katragadda, MastanX wrote:
> > Hi   Tvrtko Ursulin,
> >               Based on following backend information added skip on guc enabled platforms.
> > 
> > basically there is a quirk in GuC scheduling where if a context reaches the head of the queue and can't be scheduled it blocks the rest of the queue. A queue is an engine class. So in this case if the user submits to VCS0, VCS0, then VCS1 and the first submission to VCS0 is spinner the VCS1 submission is blocked. This more or less is exactly what this test is doing, thus it hangs. We have a request with the GuC firmware team to be able to tweak this head of queue blocking behaviour but don't expect to land anytime soon. Also in the real world this isn't an issue as the user should always be using VEs which should never block the head of the queue unless all the engines within the class are busy.
> > 
> > Test is expected to fail with GuC submission, skip it in CI.
> 
> Does "blocks the rest of the queue" mean unrelated contexts submitted
> against the same engine class?
> 

Yes.

> If so then it would be a DoS vector and the "user _should_ always" would not
> be sufficient.
>

This is a DoS vector but there are about a million others and this no
worse than most. 

> Or if the blockage is localised to a single context then it might be fine
> (but unfortunate) if on top we chose to disallow submission to non-virtual
> indices in the engine map (in case of GuC)? If the firmware bug is not

We discussed this we basically land on if the UMDs want to do something
stupid, let them as with all the other DoS in the i915. We likely can't
disable non-virtual submission as some UMDs want to explictly place some
contexts on certain engines (compute). In that case the UMD has to be
smart enough to not submit contexts in a way to expose this scheduling
quirk.

> getting fixed that is. I may be on the wrong track here since I am not 100%

Just because the GuC scheduling doesn't work like execlists doesn't mean
that it is bug. Checking for the HoQ only is done for a good reason -
performance a uC can do everything and anything like execlists as it is
single low power CPU controling the scheduling of an entire GT. We can't
protect against everything a user can do that is stupid.

FWIW we do have an open task assigned to the GuC team to allow a KMD
configurable search depth when the queue can't be scheduled. No idea
when this is going to get implemented.

> certain I figured out why it exactly gets stuck.
> 
> Because, looking at the bonded-pair to start with, if the test is emitting a
> pair of request on the same context, spinner first, then a another one with
> a semaphore dependency I am not sure why it hangs. When the spinner switches
> out after time slice expires the second request should run, cancel the
> spinner and exit. At which point they are both complete.
>

I think only the MT version of bonded-pair hangs but it has been a while
since I looked at this.

IMO this fix is 100% correct as this is a known, tracked issue. It was
agreed upon (arch, i915, GuC team) that we just skip these tests with
GuC submission.

Matt

> Regards,
> 
> Tvrtko
> 
> > Regards,
> > Mastan
> > 
> > -----Original Message-----
> > From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > Sent: 29 November 2021 16:01
> > To: Katragadda, MastanX <mastanx.katragadda@intel.com>; igt-dev@lists.freedesktop.org; Surendrakumar Upadhyay, TejaskumarX <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > Subject: Re: [igt-dev] [i-g-t] tests/i915/exec_balancer: Added Skip Guc Submission
> > 
> > 
> > On 26/11/2021 01:57, Mastan Katragadda wrote:
> > > This a known failure when running
> > > igt@gem_exec_balancer@bonded-(dual|pair|sync)
> > > tests with GuC submission.The hang is expected with GuC submission
> > > since the test was written to expect execlist scheduling hence added
> > > skip if Guc submission enabled.
> > 
> > Looking at the test I don't see anything backend specific. Can you provide an explanation of how and why it hangs?
> > 
> > Regards,
> > 
> > Tvrtko
> > 
> > > Signed-off-by: Mastan Katragadda <mastanx.katragadda@intel.com>
> > > ---
> > >    tests/i915/gem_exec_balancer.c | 1 +
> > >    1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/tests/i915/gem_exec_balancer.c
> > > b/tests/i915/gem_exec_balancer.c index cc07a5a9..d58734ab 100644
> > > --- a/tests/i915/gem_exec_balancer.c
> > > +++ b/tests/i915/gem_exec_balancer.c
> > > @@ -3320,6 +3320,7 @@ igt_main
> > >    	igt_subtest_group {
> > >    		igt_fixture {
> > > +			igt_require(!gem_using_guc_submission(i915));
> > >    			intel_allocator_multiprocess_start();
> > >    		}
> > >