drm/xe/reg_sr: Do sanity check for MCR vs non-MCR

The type struct xe_reg_mcr exists to ensure that the correct API is used
when handling MCR registers.  However, for the register save/restore
functionality, the RTP processing always cast the register to a struct
xe_reg and then apply_one_mmio() selects the MMIO API based on the "mcr"
field of the register instance.

This allows the developer to commit mistakes like passing a MCR register
for an RTP action for a GT where the respective register is not MCR; and
vice-versa.

To capture such scenarios, do a sanity check in xe_reg_sr_add() that,
upon an inconsistency:

- "fixes" the register type by favoring what we have in our MCR range
  tables instead of what the developer selected for the save/restore
  entry;
- raises a notice-level message to inform about the inconsistency.

Note: As a collateral of this change, we need to include MCR
initialization in xe_wa_test.c, otherwise a bunch of test cases end up
failing because xe_gt_mcr_check_reg() will always return false, meaning
that will incorrectly say that a MCR register is not MCR.

v2:
  - Downgrade messages to notice level so as not to block CI execution
    when inconsistencies are found. (Matt)
  - Add missing EXPORT_SYMBOL_IF_KUNIT() calls. (Gustavo)

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patch.msgid.link/20260514-rtp-mcr-check-v3-7-30dd47855fee@intel.com
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
This commit is contained in:
Gustavo Sousa 2026-05-14 18:44:50 -03:00
parent cdf8489533
commit 058da4a2b1
6 changed files with 139 additions and 8 deletions

View File

@ -9,24 +9,30 @@
#include <drm/drm_drv.h>
#include <drm/drm_kunit_helpers.h>
#include <kunit/static_stub.h>
#include <kunit/test.h>
#include "regs/xe_gt_regs.h"
#include "regs/xe_reg_defs.h"
#include "xe_device.h"
#include "xe_device_types.h"
#include "xe_gt_mcr.h"
#include "xe_kunit_helpers.h"
#include "xe_pci_test.h"
#include "xe_reg_sr.h"
#include "xe_rtp.h"
#define REGULAR_REG1 XE_REG(1)
#define REGULAR_REG2 XE_REG(2)
#define REGULAR_REG3 XE_REG(3)
#define MCR_REG1 XE_REG_MCR(1)
#define MCR_REG2 XE_REG_MCR(2)
#define MCR_REG3 XE_REG_MCR(3)
#define MASKED_REG1 XE_REG(1, XE_REG_OPTION_MASKED)
#define REGULAR_REG1 XE_REG(1)
#define REGULAR_REG2 XE_REG(2)
#define REGULAR_REG3 XE_REG(3)
#define REGULAR_REG4 XE_REG(4)
#define BAD_REGULAR_REG5 XE_REG(5)
#define MCR_REG1 XE_REG_MCR(1)
#define MCR_REG2 XE_REG_MCR(2)
#define MCR_REG3 XE_REG_MCR(3)
#define BAD_MCR_REG4 XE_REG_MCR(4)
#define MCR_REG5 XE_REG_MCR(5)
#define MASKED_REG1 XE_REG(1, XE_REG_OPTION_MASKED)
#undef XE_REG_MCR
#define XE_REG_MCR(...) XE_REG(__VA_ARGS__, .mcr = 1)
@ -48,6 +54,23 @@ struct rtp_test_case {
const struct xe_rtp_entry *entries;
};
static bool fake_xe_gt_mcr_check_reg(struct xe_gt *gt, struct xe_reg reg)
{
/*
* All supported platforms in this imaginary setup will always have REG4
* as a non-MCR register and REG5 as MCR, meaning that BAD_MCR_REG4 and
* BAD_REGULAR_REG5 represent programming errors to be captured by our
* tests.
*/
if (reg.raw == BAD_REGULAR_REG5.raw)
return true;
if (reg.raw == BAD_MCR_REG4.raw)
return false;
return reg.mcr;
}
static bool match_yes(const struct xe_device *xe, const struct xe_gt *gt,
const struct xe_hw_engine *hwe)
{
@ -304,6 +327,38 @@ static const struct rtp_to_sr_test_case rtp_to_sr_cases[] = {
{}
},
},
{
.name = "bad-mcr-reg-forced-to-regular",
.expected_reg = REGULAR_REG4,
.expected_set_bits = REG_BIT(0),
.expected_clr_bits = REG_BIT(0),
.expected_active = BIT(0),
.expected_count_sr_entries = 1,
.expected_sr_errors = 1,
.entries = (const struct xe_rtp_entry_sr[]) {
{ XE_RTP_NAME("bad-mcr-regular-reg"),
XE_RTP_RULES(FUNC(match_yes)),
XE_RTP_ACTIONS(SET(BAD_MCR_REG4, REG_BIT(0)))
},
{}
},
},
{
.name = "bad-regular-reg-forced-to-mcr",
.expected_reg = MCR_REG5,
.expected_set_bits = REG_BIT(0),
.expected_clr_bits = REG_BIT(0),
.expected_active = BIT(0),
.expected_count_sr_entries = 1,
.expected_sr_errors = 1,
.entries = (const struct xe_rtp_entry_sr[]) {
{ XE_RTP_NAME("bad-regular-reg"),
XE_RTP_RULES(FUNC(match_yes)),
XE_RTP_ACTIONS(SET(BAD_REGULAR_REG5, REG_BIT(0)))
},
{}
},
},
};
static void xe_rtp_process_to_sr_tests(struct kunit *test)
@ -523,6 +578,8 @@ static int xe_rtp_test_init(struct kunit *test)
xe->drm.dev = dev;
test->priv = xe;
kunit_activate_static_stub(test, xe_gt_mcr_check_reg, fake_xe_gt_mcr_check_reg);
return 0;
}

View File

@ -9,6 +9,8 @@
#include <kunit/test.h>
#include "xe_device.h"
#include "xe_gt.h"
#include "xe_gt_mcr.h"
#include "xe_kunit_helpers.h"
#include "xe_pci_test.h"
#include "xe_reg_sr.h"
@ -19,8 +21,10 @@ static int xe_wa_test_init(struct kunit *test)
{
const struct xe_pci_fake_data *param = test->param_value;
struct xe_pci_fake_data data = *param;
struct xe_device *xe;
struct device *dev;
struct xe_device *xe;
struct xe_gt *gt;
int id;
int ret;
dev = drm_kunit_helper_alloc_device(test);
@ -33,6 +37,12 @@ static int xe_wa_test_init(struct kunit *test)
ret = xe_pci_fake_device_init(xe);
KUNIT_ASSERT_EQ(test, ret, 0);
/* Needed for sanitize_mcr(). */
for_each_gt(gt, xe, id) {
xe_gt_mcr_init_early(gt);
xe_gt_mmio_init(gt);
}
if (!param->graphics_verx100)
xe->info.step = param->step;

View File

@ -7,6 +7,8 @@
#include <linux/minmax.h>
#include <kunit/visibility.h>
#include <drm/drm_managed.h>
#include <uapi/drm/xe_drm.h>
@ -785,6 +787,7 @@ void xe_gt_mmio_init(struct xe_gt *gt)
if (IS_SRIOV_VF(xe))
gt->mmio.sriov_vf_gt = gt;
}
EXPORT_SYMBOL_IF_KUNIT(xe_gt_mmio_init);
void xe_gt_record_user_engines(struct xe_gt *gt)
{

View File

@ -3,6 +3,9 @@
* Copyright © 2022 Intel Corporation
*/
#include <kunit/static_stub.h>
#include <kunit/visibility.h>
#include "xe_gt_mcr.h"
#include "regs/xe_gt_regs.h"
@ -553,6 +556,7 @@ void xe_gt_mcr_init_early(struct xe_gt *gt)
/* Mark instance 0 as initialized, we need this early for VRAM and CCS probe. */
gt->steering[INSTANCE0].initialized = true;
}
EXPORT_SYMBOL_IF_KUNIT(xe_gt_mcr_init_early);
/**
* xe_gt_mcr_init - Normal initialization of the MCR support
@ -614,6 +618,26 @@ static bool reg_in_steering_type_ranges(struct xe_gt *gt,
return false;
}
/*
* xe_gt_mcr_check_reg - check if a register is recognized by this GT as MCR
* @gt: GT structure
* @reg: The register to check
*
* Returns true if the register offset falls within one of the MMIO ranges
* classified as MCR for the GT.
*/
bool xe_gt_mcr_check_reg(struct xe_gt *gt, struct xe_reg reg)
{
KUNIT_STATIC_STUB_REDIRECT(xe_gt_mcr_check_reg, gt, reg);
for (int type = 0; type <= IMPLICIT_STEERING; type++)
if (reg_in_steering_type_ranges(gt, reg, type))
return true;
return false;
}
EXPORT_SYMBOL_IF_KUNIT(xe_gt_mcr_check_reg);
/*
* xe_gt_mcr_get_nonterminated_steering - find group/instance values that
* will steer a register to a non-terminated instance

View File

@ -26,6 +26,7 @@ void xe_gt_mcr_unicast_write(struct xe_gt *gt, struct xe_reg_mcr mcr_reg,
void xe_gt_mcr_multicast_write(struct xe_gt *gt, struct xe_reg_mcr mcr_reg,
u32 value);
bool xe_gt_mcr_check_reg(struct xe_gt *gt, struct xe_reg reg);
bool xe_gt_mcr_get_nonterminated_steering(struct xe_gt *gt,
struct xe_reg_mcr reg_mcr,
u8 *group, u8 *instance);

View File

@ -70,14 +70,49 @@ static void reg_sr_inc_error(struct xe_reg_sr *sr)
#endif
}
static struct xe_reg sanitize_mcr(struct xe_reg_sr *sr,
const struct xe_reg_sr_entry *e,
struct xe_gt *gt)
{
struct xe_reg reg = e->reg;
bool is_mcr;
/*
* We need the gt structure to check MCR ranges.
*/
if (!gt)
return reg;
is_mcr = xe_gt_mcr_check_reg(gt, reg);
if (is_mcr && !reg.mcr) {
reg.mcr = 1;
xe_gt_notice(gt, "xe_reg_sr_entry using non-MCR register for address 0x%x, forcing MCR\n",
reg.addr);
reg_sr_inc_error(sr);
}
if (!is_mcr && reg.mcr) {
reg.mcr = 0;
xe_gt_notice(gt, "xe_reg_sr_entry using MCR register for address 0x%x, forcing non-MCR\n",
reg.addr);
reg_sr_inc_error(sr);
}
return reg;
}
int xe_reg_sr_add(struct xe_reg_sr *sr,
const struct xe_reg_sr_entry *e,
struct xe_gt *gt)
{
unsigned long idx = e->reg.addr;
struct xe_reg_sr_entry *pentry = xa_load(&sr->xa, idx);
struct xe_reg reg;
int ret;
reg = sanitize_mcr(sr, e, gt);
if (pentry) {
if (!compatible_entries(pentry, e)) {
ret = -EINVAL;
@ -98,6 +133,7 @@ int xe_reg_sr_add(struct xe_reg_sr *sr,
}
*pentry = *e;
pentry->reg = reg;
ret = xa_err(xa_store(&sr->xa, idx, pentry, GFP_KERNEL));
if (ret)
goto fail_free;