AMD General Thinking about it more, would it be better to add error-checking logic in the debugfs parser?
When the address parameter is set, we should also check the sub_block parameter to ensure the injection method is applied correctly to the intended sub-blocks. Otherwise, we should reject the debugfs ctrl command. Any thoughts? Regards, Hawking -----Original Message----- From: amd-gfx <[email protected]> On Behalf Of Stanley.Yang Sent: Thursday, July 2, 2026 6:32 PM To: [email protected] Cc: Yang, Stanley <[email protected]> Subject: [PATCH V2 1/1] drm/amdgpu/ras: only check bad page for address-based UMC injection UMC error injection on MI300 series is dispatched by the RAS TA via the (sub-block, method) pair; only the "coherent" methods are address based, the single-shot/persistent/ac-parity ones ignore the address. The debugfs control path validated the injection address against the bad page list for every UMC injection. Restrict that check to address-based injections and warn when a non address-based one is given a non-zero address. Other ASICs keep injecting by address. Changed from V1: move address based checking to uniras layer Signed-off-by: Stanley.Yang <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 10 +++- .../gpu/drm/amd/ras/ras_mgr/amdgpu_ras_cmd.c | 58 ++++++++++++++++++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index af48dd2ebd16..f280a312b0a7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -606,8 +606,14 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f, ret = amdgpu_ras_feature_enable(adev, &data.head, 1); break; case 2: - /* umc ce/ue error injection for a bad page is not allowed */ - if (data.head.block == AMDGPU_RAS_BLOCK__UMC) + /* + * UMC ce/ue error injection for a bad page is not allowed. For + * uniras (SMU v13+) devices the injection address is validated by + * the ras_mgr inject handler, so only run the legacy bad page + * check for the legacy RAS path. + */ + if (data.head.block == AMDGPU_RAS_BLOCK__UMC && + !amdgpu_uniras_enabled(adev)) ret = amdgpu_ras_check_bad_page(adev, data.inject.address); if (ret == -EINVAL) { dev_warn(adev->dev, "RAS WARN: input address 0x%llx is invalid.", diff --git a/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_ras_cmd.c b/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_ras_cmd.c index bfbfdffbfbe6..063c7b0a7b00 100644 --- a/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_ras_cmd.c +++ b/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_ras_cmd.c @@ -82,6 +82,57 @@ static uint64_t local_addr_to_xgmi_global_addr(struct ras_core_context *ras_core return (addr + xgmi->physical_node_id * xgmi->node_segment_size); } +/* + * UMC error injection is dispatched by the RAS TA using the +(sub-block, method) + * pair carried in struct ras_cmd_inject_error_req as subblock_id (enum + * error_sub_block_umc) and method (enum inject_method_umc). Only the "coherent" + * methods program an explicit injection address and are therefore +address-based; + * the single-shot, persistent and ac-parity methods ignore the address. + * + * Keep the values below in sync with the RAS TA. + */ +enum umc_error_sub_block { + UMC_ERROR_CRC = 0, + UMC_ERROR_SRAM = 1, + UMC_ERROR_ODECC = 2, + UMC_ERROR_PARITY_DATA = 3, + UMC_ERROR_PARITY_CMD = 4, +}; + +enum umc_inject_method { + UMC_METH_COHERENT = 0, + UMC_METH_SINGLE_SHOT = 1, + UMC_METH_PERSISTENT = 2, + UMC_METH_PERSISTENT_DISABLE = 3, + UMC_METH_COHERENT_NO_DETECTION = 4, + UMC_METH_COHERENT_WR = 5, + UMC_METH_SINGLE_SHOT_WR = 6, + UMC_METH_PERSISTENT_WR = 7, + UMC_METH_SINGLE_SHOT_CLEAN = 8, +}; + +/* + * Return true if a UMC error injection using @sub_block and @method is + * address-based, i.e. it programs an explicit injection address that +must be + * validated. The non address-based methods ignore the address. + */ +static bool amdgpu_ras_umc_inject_is_address_based(u32 sub_block, u64 +method) { + switch (sub_block) { + case UMC_ERROR_CRC: + return method == UMC_METH_COHERENT || + method == UMC_METH_COHERENT_NO_DETECTION || + method == UMC_METH_COHERENT_WR; + case UMC_ERROR_ODECC: + return method == UMC_METH_COHERENT; + case UMC_ERROR_PARITY_DATA: + return method == UMC_METH_COHERENT || + method == UMC_METH_COHERENT_WR; + default: + return false; + } +} + static int amdgpu_ras_inject_error(struct ras_core_context *ras_core, struct ras_cmd_ctx *cmd, void *data) { @@ -90,7 +141,8 @@ static int amdgpu_ras_inject_error(struct ras_core_context *ras_core, (struct ras_cmd_inject_error_req *)cmd->input_buff_raw; int ret = RAS_CMD__ERROR_GENERIC; - if (req->block_id == RAS_BLOCK_ID__UMC) { + if (req->block_id == RAS_BLOCK_ID__UMC && + amdgpu_ras_umc_inject_is_address_based(req->subblock_id, +req->method)) { if (amdgpu_ras_mgr_check_retired_addr(adev, req->address)) { RAS_DEV_WARN(ras_core->dev, "RAS WARN: inject: 0x%llx has already been marked as bad!\n", @@ -111,6 +163,10 @@ static int amdgpu_ras_inject_error(struct ras_core_context *ras_core, req->block_id != RAS_BLOCK_ID__GFX) { req->address = local_addr_to_xgmi_global_addr(ras_core, req->address); } + } else if (req->block_id == RAS_BLOCK_ID__UMC && req->address) { + RAS_DEV_WARN(adev, + "RAS WARN: non address based injection, ignore the injection address 0x%llx\n", + req->address); } amdgpu_ras_trigger_error_prepare(ras_core, req); -- 2.43.0
