AMD General

> -----Original Message-----
> From: Zhang, Hawking <[email protected]>
> Sent: Friday, July 3, 2026 9:57 PM
> To: Yang, Stanley <[email protected]>; [email protected]
> Cc: Yang, Stanley <[email protected]>
> Subject: RE: [PATCH V2 1/1] drm/amdgpu/ras: only check bad page for
> address-based UMC injection
>
> AMD General
>
> Thinking about it more, would it be better to add error-checking logic in the
> debugfs parser?

[Stanley]: We should consider the introduction of ioctl interface in future, so 
the debugfs parser will not be called.

>
> 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.

[Stanley]: How to handle the injection address 0x0 if user wants to inject 
address 0x0, currently, driver cannot distinguish
whether the user injects address 0x0 or non-address-based injection.

>
> 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
>

Reply via email to