Re: [PATCH v2] drm/xe/tests/rtp: Add kunit test for whitelist upper bounds

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

Matt Roper <matthew.d.roper@intel.com> writes:

> Xe must only add registers to the GT whitelist if they are listed in the
> "Software Allowlist" section of the bspec.  These registers have been
> carefully reviewed by the architecture/security teams to ensure that
> they are safe to whitelist from a security perspective.  The list of
> allowed registers changes from platform to platform, and it is not safe
> to assume that a register is safe to whitelist on a new platform/IP just
> because it was whitelisted on older ones.  This means that whitelist
> entries in the driver that used undefined upper bounds
> (XE_RTP_END_VERSION_UNDEFINED) for their version ranges should always be
> considered illegal since they could potentially open unexpected security
> holes on future platforms.  Add a kunit test to scan the whitelist RTP
> table and ensure that all entries have well-defined upper bounds on IP
> version ranges.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> Changes in v2:
> - Add missing EXPORT_SYMBOL_IF_KUNIT(register_whitelist)
> - Link to v1: https://lore.kernel.org/r/20260625-kunit_whitelist_bounds-v1-1-d8f5f055f1f0@intel.com
> ---
>  drivers/gpu/drm/xe/tests/xe_rtp_tables_test.c | 20 ++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_reg_whitelist.c         |  5 ++++-
>  drivers/gpu/drm/xe/xe_reg_whitelist.h         |  4 ++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_tables_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_tables_test.c
> index ef379cbb6a86..72ae1c13ad72 100644
> --- a/drivers/gpu/drm/xe/tests/xe_rtp_tables_test.c
> +++ b/drivers/gpu/drm/xe/tests/xe_rtp_tables_test.c
> @@ -5,6 +5,7 @@
>  
>  #include <kunit/test.h>
>  
> +#include "xe_reg_whitelist.h"
>  #include "xe_rtp_types.h"
>  #include "xe_tuning.h"
>  #include "xe_wa.h"
> @@ -75,11 +76,30 @@ static void xe_rtp_table_dev_oob_test(struct kunit *test)
>  
>  RTP_TABLE_PARAM(device_oob_was);
>  
> +static void xe_rtp_table_missing_upper_bound_test(struct kunit *test)
> +{
> +	const struct xe_rtp_entry_sr *entry = test->param_value;
> +
> +	for (int i = 0; i < entry->n_rules; i++) {
> +		u8 match_type = entry->rules[i].match_type;
> +
> +		if (match_type != XE_RTP_MATCH_GRAPHICS_VERSION_RANGE &&
> +		    match_type != XE_RTP_MATCH_MEDIA_VERSION_RANGE)
> +			continue;
> +
> +		KUNIT_EXPECT_NE(test, entry->rules[i].ver_end, XE_RTP_END_VERSION_UNDEFINED);

I wonder if it would be more informative to embed the relevant match
types in the expectation:

  KUNIT_EXPECT_FALSE(test, match_type == XE_RTP_MATCH_GRAPHICS_VERSION_RANGE &&
                     ver_end == XE_RTP_END_VERSION_UNDEFINED);
  KUNIT_EXPECT_FALSE(test, match_type == XE_RTP_MATCH_MEDIA_VERSION_RANGE &&
                     ver_end == XE_RTP_END_VERSION_UNDEFINED);

This would make it easy to see in the output of a failed test that
XE_RTP_END_VERSION_UNDEFINED should not be used for those match types.

Your call.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>


> +	}
> +}
> +
> +RTP_TABLE_PARAM(register_whitelist);
> +
>  static struct kunit_case xe_rtp_table_tests[] = {
>  	KUNIT_CASE_PARAM(xe_rtp_table_gt_test, gt_was_gen_params),
>  	KUNIT_CASE_PARAM(xe_rtp_table_gt_test, gt_tunings_gen_params),
>  	KUNIT_CASE_PARAM(xe_rtp_table_oob_test, oob_was_gen_params),
>  	KUNIT_CASE_PARAM(xe_rtp_table_dev_oob_test, device_oob_was_gen_params),
> +	KUNIT_CASE_PARAM(xe_rtp_table_missing_upper_bound_test,
> +			 register_whitelist_gen_params),
>  	{}
>  };
>  
> diff --git a/drivers/gpu/drm/xe/xe_reg_whitelist.c b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> index 3d9e3daab01a..fe996d23007b 100644
> --- a/drivers/gpu/drm/xe/xe_reg_whitelist.c
> +++ b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> @@ -5,6 +5,8 @@
>  
>  #include "xe_reg_whitelist.h"
>  
> +#include <kunit/visibility.h>
> +
>  #include "regs/xe_engine_regs.h"
>  #include "regs/xe_gt_regs.h"
>  #include "regs/xe_oa_regs.h"
> @@ -41,7 +43,7 @@ static bool match_multi_queue_class(const struct xe_device *xe,
>  	return xe_gt_supports_multi_queue(gt, hwe->class);
>  }
>  
> -static const struct xe_rtp_table_sr register_whitelist = XE_RTP_TABLE_SR(
> +VISIBLE_IF_KUNIT const struct xe_rtp_table_sr register_whitelist = XE_RTP_TABLE_SR(
>  	{ XE_RTP_NAME("WaAllowPMDepthAndInvocationCountAccessFromUMD, 1408556865"),
>  	  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(1200, 1210), ENGINE_CLASS(RENDER)),
>  	  XE_RTP_ACTIONS(WHITELIST(PS_INVOCATION_COUNT,
> @@ -104,6 +106,7 @@ static const struct xe_rtp_table_sr register_whitelist = XE_RTP_TABLE_SR(
>  				   RING_FORCE_TO_NONPRIV_ACCESS_RW))
>  	},
>  );
> +EXPORT_SYMBOL_IF_KUNIT(register_whitelist);
>  
>  static const struct xe_rtp_table_sr oa_whitelist = XE_RTP_TABLE_SR(
>  
> diff --git a/drivers/gpu/drm/xe/xe_reg_whitelist.h b/drivers/gpu/drm/xe/xe_reg_whitelist.h
> index e1eb1b7d5480..c0248063d515 100644
> --- a/drivers/gpu/drm/xe/xe_reg_whitelist.h
> +++ b/drivers/gpu/drm/xe/xe_reg_whitelist.h
> @@ -14,6 +14,10 @@ struct xe_hw_engine;
>  struct xe_reg_sr;
>  struct xe_reg_sr_entry;
>  
> +#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
> +extern const struct xe_rtp_table_sr register_whitelist;
> +#endif
> +
>  void xe_reg_whitelist_process_engine(struct xe_hw_engine *hwe);
>  
>  void xe_reg_whitelist_oa_regs(struct xe_gt *gt);
>
> ---
> base-commit: c04e038b8787434bf489ea6de4ab2e2d936083c7
> change-id: 20260625-kunit_whitelist_bounds-a88495e5cc7b
>
> Best regards,
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation