[Mesa-dev] [PATCH] intel/aubinator: Properly handle batch buffer chaining

[Gandikota, Sirisha]

aubinator.c: In function ‘parse_commands’:
aubinator.c:768:19: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
          if (p[0] & (1 << 22) == 0)

-----Original Message-----
From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On Behalf Of Jason Ekstrand
Sent: Thursday, September 08, 2016 9:12 PM
To: mesa-dev at lists.freedesktop.org
Cc: Ekstrand, Jason <jason.ekstrand at intel.com>
Subject: [Mesa-dev] [PATCH] intel/aubinator: Properly handle batch buffer chaining

The original aubinator that Kristian wrote had a bug in the handling of MI_BATCH_BUFFER_START that propagated into the version in upstream mesa.
Say you have two batch buffers A and B where A calls MI_BATCH_BUFFER_START to jump to B.  Now suppose that A and B are placed consecutively in the address space with A before B.  What can happen is that aubinator will process A, and start processing B when it should.  When it gets done with B, it returns and continues to process A.  Because A doesn't have any more data after the MI_BATCH_BUFFER_START, it will just process a bunch of NOPs until it gets to the next buffer in memory which is B again.  In this scenario B gets processed twice which can be very confusing.  If you place things in memory just right, you can also end up with infinite loops which are all sorts of fun.

The root problem here is that it continues to process commands even after an MI_BATCH_BUFFER_START.  By simply checking the 2nd level we can detect whether or not the command buffer we are jumping to will return here and stop processing commands if it won't.

Signed-off-by: Jason Ekstrand <jason at jlekstrand.net<mailto:jason at jlekstrand.net>>
---
 src/intel/tools/aubinator.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c index fe1f369..73a7f21 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -766,6 +766,13 @@ parse_commands(struct gen_spec *spec, uint32_t *cmds, int size, int engine)
             start = p[1];

          parse_commands(spec, gtt + start, 1 << 20, engine);
+
+         /* MI_BATCH_BUFFER_START with "2nd Level Batch Buffer" unset acts
+          * like a goto.  No commands after such a MI_BATCH_BUFFER_START will
+          * get processed so we should bail as well.
+          */
+         if (p[0] & (1 << 22) == 0)

[SG]: The above line might need extra pair of parenthesis around comparison to get rid of the compile time warning I was seeing as below. Otherwise, Patch looks good to me.
aubinator.c: In function ‘parse_commands’:
aubinator.c:768:19: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
          if (p[0] & (1 << 22) == 0)

+            break;
       } else if ((p[0] & 0xffff0000) == AUB_MI_BATCH_BUFFER_END) {
          break;
       }
--
2.5.0.400.gff86faf

_______________________________________________
mesa-dev mailing list
mesa-dev at lists.freedesktop.org<mailto:mesa-dev at lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160914/02084848/attachment.html>