All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gustavo Sousa <gustavo.sousa@intel.com>
To: intel-xe@lists.freedesktop.org
Cc: Gustavo Sousa <gustavo.sousa@intel.com>,
	 Matt Roper <matthew.d.roper@intel.com>
Subject: [PATCH v2 3/7] drm/xe/rtp: Don't short-circuit to false in or-yes case
Date: Thu, 30 Apr 2026 17:20:04 -0300	[thread overview]
Message-ID: <20260430-rtp-rule-parser-v2-3-157e98b4ab51@intel.com> (raw)
In-Reply-To: <20260430-rtp-rule-parser-v2-0-157e98b4ab51@intel.com>

While RTP processing evaluates true on the "yes-or" case (i.e. a
conjunction of rules that evaluate to true followed by an "OR" without
the right hand operand), it does not on the "or-yes" one.

Both cases are considered malformed and could be a result of someone
dropping checks deemed not necessary anymore and forgetting to drop the
superfluous "OR".  Nevertheless, we should aim for consistency, and
having the "or-yes" case also evaluating to true while also causing a
warning seems reasonable.  So let's do that.

The "or-yes" pattern being evaluated to false comes from the fact that
that we unconditionally short-circuit upon finding XE_RTP_MATCH_OR on
the outer loop.  We should only do that if the preceding conjunction of
rules evaluated to true (meaning that rcount must be non-zero) and
continue the evaluation otherwise.

Do that and also add extra test cases to validate the short-circuiting
behavior.

Notice that some of the new test cases have a "FIXME" comment, which
comes from the fact that we are unable to detect syntax errors after the
short-circuit point.  That is going to be fixed in a follow-up change.

Link: https://lore.kernel.org/intel-xe/871pfw4lo9.fsf@intel.com/
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 | 42 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/xe/xe_rtp.c            | 15 ++++++++----
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
index 7dce699991a1..5e0afde9eab4 100644
--- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c
+++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
@@ -172,11 +172,49 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = {
 		XE_RTP_RULES(OR),
 	},
 	{
-		.name = "or-anything",
-		.expected_match = false,
+		.name = "or-yes",
+		.expected_match = true,
 		.expected_err = -EINVAL,
 		XE_RTP_RULES(OR, FUNC(match_yes)),
 	},
+	{
+		.name = "or-no",
+		.expected_match = false,
+		.expected_err = -EINVAL,
+		XE_RTP_RULES(OR, FUNC(match_no)),
+	},
+	{
+		.name = "yes-or",
+		.expected_match = true,
+		/* FIXME: The parser should raise an error here. */
+		.expected_err = 0,
+		XE_RTP_RULES(FUNC(match_yes), OR),
+	},
+	{
+		.name = "no-or",
+		.expected_match = false,
+		.expected_err = -EINVAL,
+		XE_RTP_RULES(FUNC(match_no), OR),
+	},
+	{
+		.name = "no-or-or-yes",
+		.expected_match = true,
+		.expected_err = -EINVAL,
+		XE_RTP_RULES(FUNC(match_no), OR, OR, FUNC(match_yes)),
+	},
+	{
+		.name = "yes-or-or-no",
+		.expected_match = true,
+		/* FIXME: The parser should raise an error here. */
+		.expected_err = 0,
+		XE_RTP_RULES(FUNC(match_yes), OR, OR, FUNC(match_no)),
+	},
+	{
+		.name = "no-or-or-no",
+		.expected_match = false,
+		.expected_err = -EINVAL,
+		XE_RTP_RULES(FUNC(match_no), OR, OR, FUNC(match_no)),
+	},
 };
 
 static void xe_rtp_rules_tests(struct kunit *test)
diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
index 0e1adf07e4e7..299fa4209a26 100644
--- a/drivers/gpu/drm/xe/xe_rtp.c
+++ b/drivers/gpu/drm/xe/xe_rtp.c
@@ -47,12 +47,18 @@ static bool rule_matches_with_err(const struct xe_device *xe,
 	for (r = rules, i = 0; i < n_rules; r = &rules[++i]) {
 		switch (r->match_type) {
 		case XE_RTP_MATCH_OR:
+			if (drm_WARN_ON(&xe->drm, !rcount)) {
+				if (err)
+					*err = -EINVAL;
+				continue;
+			}
+
 			/*
-			 * This is only reached if a complete set of
-			 * rules passed or none were evaluated. For both cases,
-			 * shortcut the other rules and return the proper value.
+			 * This is only reached if a complete conjunction of
+			 * rules passed, in which case we shortcut the other
+			 * rules and return true.
 			 */
-			goto done;
+			return true;
 		case XE_RTP_MATCH_PLATFORM:
 			match = xe->info.platform == r->platform;
 			break;
@@ -169,7 +175,6 @@ static bool rule_matches_with_err(const struct xe_device *xe,
 		}
 	}
 
-done:
 	if (drm_WARN_ON(&xe->drm, !rcount))
 		goto error;
 

-- 
2.53.0


  parent reply	other threads:[~2026-04-30 20:21 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-30 20:20 [PATCH v2 0/7] drm/xe/rtp: Miscellaneous improvements to rule matching Gustavo Sousa
2026-04-30 20:20 ` [PATCH v2 1/7] drm/xe/rtp: Write kunit test cases specific for " Gustavo Sousa
2026-04-30 20:20 ` [PATCH v2 2/7] drm/xe/rtp: Drop rule matching cases from rtp_to_sr_cases and rtp_cases Gustavo Sousa
2026-04-30 20:20 ` Gustavo Sousa [this message]
2026-05-13 21:52   ` [PATCH v2 3/7] drm/xe/rtp: Don't short-circuit to false in or-yes case Violet Monti
2026-05-13 23:04   ` Matt Roper
2026-04-30 20:20 ` [PATCH v2 4/7] drm/xe/rtp: Do not break parsing when missing context Gustavo Sousa
2026-04-30 20:20 ` [PATCH v2 5/7] drm/xe/rtp: Extract rule_match_item() Gustavo Sousa
2026-04-30 20:20 ` [PATCH v2 6/7] drm/xe/rtp: Fully parse the ruleset Gustavo Sousa
2026-05-13 22:07   ` Violet Monti
2026-05-13 23:14   ` Matt Roper
2026-04-30 20:20 ` [PATCH v2 7/7] drm/xe/rtp: Implement a structured parser for rule matching Gustavo Sousa
2026-05-13 22:09   ` Violet Monti
2026-05-13 23:34   ` Matt Roper
2026-05-14 20:19     ` Gustavo Sousa
2026-05-14 20:51       ` Matt Roper
2026-04-30 21:02 ` ✗ CI.checkpatch: warning for drm/xe/rtp: Miscellaneous improvements to rule matching (rev2) Patchwork
2026-04-30 21:03 ` ✓ CI.KUnit: success " Patchwork
2026-04-30 21:59 ` ✓ Xe.CI.BAT: " Patchwork
2026-05-01  9:25 ` ✗ Xe.CI.FULL: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260430-rtp-rule-parser-v2-3-157e98b4ab51@intel.com \
    --to=gustavo.sousa@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.d.roper@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.