Re: [PATCH v2 3/7] drm/xe/rtp: Don't short-circuit to false in or-yes case

[Violet Monti <violet.monti@intel.com>]

On Thu, Apr 30, 2026 at 05:20:04PM -0300, Gustavo Sousa wrote:
> 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>

Reviewed-by: Violet Monti <violet.monti@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
> 

-- 
--
Violet Monti