drm/xe/rtp: Fully parse the ruleset

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)

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Violet Monti <violet.monti@intel.com>
Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-6-0c51039899f4@intel.com
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
index dfb15d81d799e80d459ee57f87adbd7f10723b53..642f6e090ad0fd2736eabec141357bd197c401fc 100644 (file)
--- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c
+++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
@@ -209,8 +209,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),
        },
        {
@@ -228,8 +227,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 c49f80f398afab3d405e0bc8688d086f63d5b47f..976a2e1f559257d8348e0fe306a89b8e0953b214 100644 (file)
--- 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,