From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 05B0DCD3445 for ; Fri, 8 May 2026 21:43:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BAEC310F60E; Fri, 8 May 2026 21:43:02 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="ekFuyXiw"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6540610F60E for ; Fri, 8 May 2026 21:43:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1778276581; x=1809812581; h=from:date:subject:mime-version:content-transfer-encoding: message-id:references:in-reply-to:to:cc; bh=rw+Ip/gSD880IFSM6T155d1G2LeWVYEYhpRg8FGWUmE=; b=ekFuyXiwG+HpucYZrmWGp1GMfOw1l9esLmPLEbUW+tV9LFqtqseG+K+I IYGjqzSsSv/lDhwJofISssZxstRRgLPc3PMSqUryh0xTob+lEUlNF4fTz iqsKKNTE55WsmVuSh3YT6E8XRvhJjFgLhaZRXFgsrj5pnV7nZ5NBfdlLS Ah1xPHRGDhwhamNI0tajrlyFvsOjwg2Q+FvSQugJwi57KLtPE8T8SdLnv Yt34f3b6xISqsVvgALvSBh06MqZSLSa5V7o1U0n6b9bOVsbip5SYGkG+F bgEmbaUIsBtimB+eudr/FEgZrk8RGbYtaf7nhqVFDYnKoTo+ktFTYTZjY w==; X-CSE-ConnectionGUID: ZX9PQOdlTqeshDJxZn+Jcg== X-CSE-MsgGUID: fzgy46e2TDGkmY1wEjWjtQ== X-IronPort-AV: E=McAfee;i="6800,10657,11780"; a="90635999" X-IronPort-AV: E=Sophos;i="6.23,224,1770624000"; d="scan'208";a="90635999" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 May 2026 14:43:00 -0700 X-CSE-ConnectionGUID: SAZWa+RDTG+MMhpGL5F02w== X-CSE-MsgGUID: xTlzjOwlQ12zyF+cRopX/A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,224,1770624000"; d="scan'208";a="267244262" Received: from jjgreens-desk20.amr.corp.intel.com (HELO [192.168.1.16]) ([10.124.220.81]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 May 2026 14:42:59 -0700 From: Gustavo Sousa Date: Fri, 08 May 2026 18:42:38 -0300 Subject: [PATCH v2 8/8] drm/xe/reg_sr: Do sanity check for MCR vs non-MCR MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Message-Id: <20260508-rtp-mcr-check-v2-8-9897b147a5d2@intel.com> References: <20260508-rtp-mcr-check-v2-0-9897b147a5d2@intel.com> In-Reply-To: <20260508-rtp-mcr-check-v2-0-9897b147a5d2@intel.com> To: intel-xe@lists.freedesktop.org Cc: Gustavo Sousa , Matt Roper X-Mailer: b4 0.15-dev X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" 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) Cc: Matt Roper Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/tests/xe_rtp_test.c | 71 ++++++++++++++++++++++++++++++---- drivers/gpu/drm/xe/tests/xe_wa_test.c | 12 +++++- drivers/gpu/drm/xe/xe_gt.c | 3 ++ drivers/gpu/drm/xe/xe_gt_mcr.c | 24 ++++++++++++ drivers/gpu/drm/xe/xe_gt_mcr.h | 1 + drivers/gpu/drm/xe/xe_reg_sr.c | 36 +++++++++++++++++ 6 files changed, 139 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c index e5a0f985a700..5d78f2283df9 100644 --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c @@ -9,24 +9,30 @@ #include #include +#include #include #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; } diff --git a/drivers/gpu/drm/xe/tests/xe_wa_test.c b/drivers/gpu/drm/xe/tests/xe_wa_test.c index 2bf6fab015cd..ff0e2502b39f 100644 --- a/drivers/gpu/drm/xe/tests/xe_wa_test.c +++ b/drivers/gpu/drm/xe/tests/xe_wa_test.c @@ -9,6 +9,8 @@ #include #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; diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index c4b25daad542..783eb6d631b5 100644 --- a/drivers/gpu/drm/xe/xe_gt.c +++ b/drivers/gpu/drm/xe/xe_gt.c @@ -7,6 +7,8 @@ #include +#include + #include #include @@ -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) { diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c index 2b2a4d9c3749..04f0098070a4 100644 --- a/drivers/gpu/drm/xe/xe_gt_mcr.c +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c @@ -3,6 +3,9 @@ * Copyright © 2022 Intel Corporation */ +#include +#include + #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 diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.h b/drivers/gpu/drm/xe/xe_gt_mcr.h index 2be9419b8acc..75374662f10d 100644 --- a/drivers/gpu/drm/xe/xe_gt_mcr.h +++ b/drivers/gpu/drm/xe/xe_gt_mcr.h @@ -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); diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c index 2df0277efb2f..e328f5072557 100644 --- a/drivers/gpu/drm/xe/xe_reg_sr.c +++ b/drivers/gpu/drm/xe/xe_reg_sr.c @@ -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; -- 2.53.0