[PATCH v2 6/7] drm/xe/rtp: Fully parse the ruleset

[Gustavo Sousa <gustavo.sousa@intel.com>]

The function rule_matches() short-circuits evaluation of the implicit
conjunctions (each substring of rules not containing OR) and the
explicit disjunctions (implicit conjunctions joined by OR). In other
words:

  - in a conjunction, once a rule evaluate to false, we skip to the next
    OR (if any) to evaluate the next conjunction;
  - in a disjunction, once a conjunction evaluates to true, we return
    true and skip evaluating all the remaining rules.

While this behavior results in a correct logical value, due to how the
"OR" short-circuiting is implemented, it has the side-effect that rule
set does not get fully "parsed", allowing incomplete constructs like
(rule1, OR) to evaluate to true when rule1 is true.  We should treat
such constructs as invalid and treat them the same way we do for stuff
like (OR, rule1).

As such, update rule_matches() to "parse" the whole rule set, and that
while keeping the short-circuit aspect of evaluation.  With that, we can
fix the FIXME test cases that cover that behavior.

v2:
  - Do not change short-circuit *evaluation* behavior. (Matt)

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/xe/tests/xe_rtp_test.c |  6 ++----
 drivers/gpu/drm/xe/xe_rtp.c            | 26 +++++++++++++-------------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
index e009c1243b7c..3429c5d1b5b6 100644
--- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c
+++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
@@ -186,8 +186,7 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = {
 	{
 		.name = "yes-or",
 		.expected_match = true,
-		/* FIXME: The parser should raise an error here. */
-		.expected_err = 0,
+		.expected_err = -EINVAL,
 		XE_RTP_RULES(FUNC(match_yes), OR),
 	},
 	{
@@ -205,8 +204,7 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = {
 	{
 		.name = "yes-or-or-no",
 		.expected_match = true,
-		/* FIXME: The parser should raise an error here. */
-		.expected_err = 0,
+		.expected_err = -EINVAL,
 		XE_RTP_RULES(FUNC(match_yes), OR, OR, FUNC(match_no)),
 	},
 	{
diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
index c49f80f398af..976a2e1f5592 100644
--- a/drivers/gpu/drm/xe/xe_rtp.c
+++ b/drivers/gpu/drm/xe/xe_rtp.c
@@ -129,6 +129,7 @@ static bool rule_matches_with_err(const struct xe_device *xe,
 {
 	const struct xe_rtp_rule *r;
 	unsigned int i, rcount = 0;
+	bool short_circuit_or = false;
 
 	if (err)
 		*err = 0;
@@ -143,13 +144,16 @@ static bool rule_matches_with_err(const struct xe_device *xe,
 
 			/*
 			 * This is only reached if a complete conjunction of
-			 * rules passed, in which case we shortcut the other
-			 * rules and return true.
+			 * rules passed, in which case we short-circuit rule
+			 * evaluation, but still keep parsing to find any syntax
+			 * errors.
 			 */
-			return true;
+			short_circuit_or = true;
+			rcount = 0;
+			continue;
 		}
 
-		if (rule_match_item(xe, gt, hwe, r)) {
+		if (short_circuit_or || rule_match_item(xe, gt, hwe, r)) {
 			rcount++;
 		} else {
 			/*
@@ -166,16 +170,12 @@ static bool rule_matches_with_err(const struct xe_device *xe,
 		}
 	}
 
-	if (drm_WARN_ON(&xe->drm, !rcount))
-		goto error;
-
-	return true;
-
-error:
-	if (err)
-		*err = -EINVAL;
+	if (drm_WARN_ON(&xe->drm, !rcount)) {
+		if (err)
+			*err = -EINVAL;
+	}
 
-	return false;
+	return short_circuit_or || rcount;
 }
 
 static bool rule_matches(const struct xe_device *xe,

-- 
2.53.0