Re: [PATCH 3/6] drm/xe/rtp: Do not break parsing when missing context

[Matt Roper <matthew.d.roper@intel.com>]

On Wed, Jan 14, 2026 at 07:49:53PM -0300, Gustavo Sousa wrote:
> With the current implementation, the RTP framework will cause parsing of
> the rule set to be interrupted if one rule requires a context item (gt
> or hwe) that is missing (i.e. when the value is NULL).
> 
> This is arguably a semantic error instead of a syntactic one, meaning
> that RTP should not interrupt parsing the rules.  With the current
> behavior, we would miss detecting other errors that could appear in the
> remaining rules and could also prevent valid rules joined by "OR" from
> being evaluated.
> 
> Make sure that we do not stop parsing the rule set when detecting
> missing context and let's add rtp_rules_test_cases to reflect that.
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/xe/tests/xe_rtp_test.c | 28 ++++++++++++++++
>  drivers/gpu/drm/xe/xe_rtp.c            | 60 ++++++++++++++++++++++------------
>  2 files changed, 68 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
> index b2286dd9d92a..19c7142b2fe4 100644
> --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c
> +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
> @@ -177,6 +177,34 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = {
>  		.expected_err = -EINVAL,
>  		XE_RTP_RULES(OR, FUNC(match_yes)),
>  	},
> +
> +	/* No match because hwe is NULL. */
> +	{
> +		.name = "engine-class",

Nitpick:  The names for these tests doesn't really make it clear what's
being tested.  Something like "missing-context-engine-class" might be
better?

I'm wondering if an even more useful test would be one that scans our
actual RTP tables and just looks for anything on gt_was[] or oob_was[]
that has an engine class rule, or anything on device_oob_was[] that has
either an IP version rule or an engine class rule.  If we catch those
mistakes during early kunit testing, then we shouldn't really need to
worry about what the behavior would be if one of them survived to be
parsed on a live device.

Anyway, the rest of the patch looks fine, so aside from improving the
names,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>


Matt

> +		.expected_match = false,
> +		XE_RTP_RULES(ENGINE_CLASS(RENDER)),
> +	},
> +
> +	/*
> +	 * Missing context (hwe==NULL) does not cause parsing to stop, hence we
> +	 * expect a match.
> +	 */
> +	{
> +		.name = "engine-class-or-yes",
> +		.expected_match = true,
> +		XE_RTP_RULES(ENGINE_CLASS(RENDER), OR, FUNC(match_yes)),
> +	},
> +
> +	/*
> +	 * Missing context (hwe==NULL) does not cause parsing to stop, hence we
> +	 * expect a syntax error.
> +	 */
> +	{
> +		.name = "engine-class-or-or-yes",
> +		.expected_match = false,
> +		.expected_err = -EINVAL,
> +		XE_RTP_RULES(ENGINE_CLASS(RENDER), OR, OR, FUNC(match_yes)),
> +	},
>  };
>  
>  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 e955df6c22ca..e0bed5ac1369 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.c
> +++ b/drivers/gpu/drm/xe/xe_rtp.c
> @@ -61,60 +61,76 @@ static bool rule_matches_with_err(const struct xe_device *xe,
>  				xe->info.subplatform == r->subplatform;
>  			break;
>  		case XE_RTP_MATCH_GRAPHICS_VERSION:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.graphics_verx100 == r->ver_start &&
>  				(!has_samedia(xe) || !xe_gt_is_media_type(gt));
>  			break;
>  		case XE_RTP_MATCH_GRAPHICS_VERSION_RANGE:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.graphics_verx100 >= r->ver_start &&
>  				xe->info.graphics_verx100 <= r->ver_end &&
>  				(!has_samedia(xe) || !xe_gt_is_media_type(gt));
>  			break;
>  		case XE_RTP_MATCH_GRAPHICS_VERSION_ANY_GT:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.graphics_verx100 == r->ver_start;
>  			break;
>  		case XE_RTP_MATCH_GRAPHICS_STEP:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.step.graphics >= r->step_start &&
>  				xe->info.step.graphics < r->step_end &&
>  				(!has_samedia(xe) || !xe_gt_is_media_type(gt));
>  			break;
>  		case XE_RTP_MATCH_MEDIA_VERSION:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.media_verx100 == r->ver_start &&
>  				(!has_samedia(xe) || xe_gt_is_media_type(gt));
>  			break;
>  		case XE_RTP_MATCH_MEDIA_VERSION_RANGE:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.media_verx100 >= r->ver_start &&
>  				xe->info.media_verx100 <= r->ver_end &&
>  				(!has_samedia(xe) || xe_gt_is_media_type(gt));
>  			break;
>  		case XE_RTP_MATCH_MEDIA_STEP:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.step.media >= r->step_start &&
>  				xe->info.step.media < r->step_end &&
>  				(!has_samedia(xe) || xe_gt_is_media_type(gt));
>  			break;
>  		case XE_RTP_MATCH_MEDIA_VERSION_ANY_GT:
> -			if (drm_WARN_ON(&xe->drm, !gt))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !gt)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = xe->info.media_verx100 == r->ver_start;
>  			break;
> @@ -125,14 +141,18 @@ static bool rule_matches_with_err(const struct xe_device *xe,
>  			match = xe->info.is_dgfx;
>  			break;
>  		case XE_RTP_MATCH_ENGINE_CLASS:
> -			if (drm_WARN_ON(&xe->drm, !hwe))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !hwe)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = hwe->class == r->engine_class;
>  			break;
>  		case XE_RTP_MATCH_NOT_ENGINE_CLASS:
> -			if (drm_WARN_ON(&xe->drm, !hwe))
> -				goto error;
> +			if (drm_WARN_ON(&xe->drm, !hwe)) {
> +				match = false;
> +				break;
> +			}
>  
>  			match = hwe->class != r->engine_class;
>  			break;
> 
> -- 
> 2.52.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation