[PATCH v6 1/4] drm/xe: Validate wedged_mode parameter and define enum for modes
Michal Wajdeczko
michal.wajdeczko at intel.com
Tue May 27 17:18:56 UTC 2025
On 21.05.2025 14:25, Lukasz Laguna wrote:
> Check correctness of the wedged_mode parameter input to ensure only
> supported values are accepted. Additionally, replace magic numbers with
> a clearly defined enum.
>
> Signed-off-by: Lukasz Laguna <lukasz.laguna at intel.com>
few nits below, but LGTM
Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> ---
> drivers/gpu/drm/xe/xe_debugfs.c | 5 +--
> drivers/gpu/drm/xe/xe_device.c | 46 ++++++++++++++++++++++++++--
> drivers/gpu/drm/xe/xe_device.h | 2 ++
> drivers/gpu/drm/xe/xe_device_types.h | 7 ++++-
> drivers/gpu/drm/xe/xe_guc_ads.c | 4 +--
> drivers/gpu/drm/xe/xe_guc_capture.c | 3 +-
> drivers/gpu/drm/xe/xe_guc_submit.c | 7 +++--
> drivers/gpu/drm/xe/xe_module.c | 5 +--
> drivers/gpu/drm/xe/xe_module.h | 2 +-
> 9 files changed, 67 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index d83cd6ed3fa8..5dd0ad3f9bae 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -163,8 +163,9 @@ static ssize_t wedged_mode_set(struct file *f, const char __user *ubuf,
> if (ret)
> return ret;
>
> - if (wedged_mode > 2)
> - return -EINVAL;
> + ret = xe_device_validate_wedged_mode(xe, wedged_mode);
> + if (ret)
> + return ret;
>
> if (xe->wedged.mode == wedged_mode)
> return size;
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 660b0c5126dc..fa4f1a3ac1f2 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -726,7 +726,10 @@ int xe_device_probe_early(struct xe_device *xe)
> if (err)
> return err;
>
> - xe->wedged.mode = xe_modparam.wedged_mode;
> + xe->wedged.mode = xe_device_validate_wedged_mode(xe, xe_modparam.wedged_mode) ?
> + XE_WEDGED_MODE_DEFAULT : xe_modparam.wedged_mode;
> + drm_dbg(&xe->drm, "wedged_mode: setting mode (%u) %s\n",
> + xe->wedged.mode, xe_device_wedged_mode_to_string(xe->wedged.mode));
>
> return 0;
> }
> @@ -1140,7 +1143,7 @@ void xe_device_declare_wedged(struct xe_device *xe)
> struct xe_gt *gt;
> u8 id;
>
> - if (xe->wedged.mode == 0) {
> + if (xe->wedged.mode == XE_WEDGED_MODE_NEVER) {
> drm_dbg(&xe->drm, "Wedged mode is forcibly disabled\n");
> return;
> }
> @@ -1168,3 +1171,42 @@ void xe_device_declare_wedged(struct xe_device *xe)
> for_each_gt(gt, xe, id)
> xe_gt_declare_wedged(gt);
> }
> +
> +/**
> + * xe_device_validate_wedged_mode - Check if given mode is supported
> + * @xe: the &xe_device
> + * @mode: requested mode to validate
> + *
> + * Check whether the provided wedged mode is supported.
> + *
> + * Return: 0 if mode is supported, error code otherwise.
> + */
> +int xe_device_validate_wedged_mode(struct xe_device *xe, unsigned int mode)
> +{
> + if (mode > XE_WEDGED_MODE_UPON_ANY_HANG) {
> + drm_dbg(&xe->drm, "wedged_mode: invalid value (%u)\n", mode);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * xe_device_wedged_mode_to_string - Convert enum value to string.
> + * @mode: the &xe_wedged_mode to convert
> + *
> + * Returns: wedged mode as a user friendly string.
> + */
> +const char *xe_device_wedged_mode_to_string(enum xe_wedged_mode mode)
nit: xe_wedged_mode_to_string()
> +{
> + switch (mode) {
> + case XE_WEDGED_MODE_NEVER:
> + return "never";
> + case XE_WEDGED_MODE_UPON_CRITICAL_ERROR:
> + return "upon-critical-error";
> + case XE_WEDGED_MODE_UPON_ANY_HANG:
> + return "upon-any-hang";
> + default:
> + return "<invalid>";
> + }
> +}
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 0bc3bc8e6803..d793faa09f89 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -191,6 +191,8 @@ static inline bool xe_device_wedged(struct xe_device *xe)
> }
>
> void xe_device_declare_wedged(struct xe_device *xe);
> +int xe_device_validate_wedged_mode(struct xe_device *xe, unsigned int mode);
> +const char *xe_device_wedged_mode_to_string(enum xe_wedged_mode mode);
>
> struct xe_file *xe_file_get(struct xe_file *xef);
> void xe_file_put(struct xe_file *xef);
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 902b2e30b3b9..a8dfacb951c1 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -558,7 +558,12 @@ struct xe_device {
> /** @wedged.flag: Xe device faced a critical error and is now blocked. */
> atomic_t flag;
> /** @wedged.mode: Mode controlled by kernel parameter and debugfs */
> - int mode;
> + enum xe_wedged_mode {
> + XE_WEDGED_MODE_NEVER = 0,
> + XE_WEDGED_MODE_UPON_CRITICAL_ERROR = 1,
> + XE_WEDGED_MODE_UPON_ANY_HANG = 2,
> + XE_WEDGED_MODE_DEFAULT = XE_WEDGED_MODE_UPON_CRITICAL_ERROR,
> + } mode;
> } wedged;
>
> /** @bo_device: Struct to control async free of BOs */
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index 07a027755627..ca8bdb0ec08d 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -470,7 +470,7 @@ static void guc_policies_init(struct xe_guc_ads *ads)
> ads_blob_write(ads, policies.max_num_work_items,
> GLOBAL_POLICY_MAX_NUM_WI);
>
> - if (xe->wedged.mode == 2)
> + if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG)
> global_flags |= GLOBAL_POLICY_DISABLE_ENGINE_RESET;
>
> ads_blob_write(ads, policies.global_flags, global_flags);
> @@ -1026,7 +1026,7 @@ int xe_guc_ads_scheduler_policy_toggle_reset(struct xe_guc_ads *ads)
> policies->dpc_promote_time = ads_blob_read(ads, policies.dpc_promote_time);
> policies->max_num_work_items = ads_blob_read(ads, policies.max_num_work_items);
> policies->is_valid = 1;
> - if (xe->wedged.mode == 2)
> + if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG)
> policies->global_flags |= GLOBAL_POLICY_DISABLE_ENGINE_RESET;
> else
> policies->global_flags &= ~GLOBAL_POLICY_DISABLE_ENGINE_RESET;
> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c
> index 859a3ba91be5..4f203bd0a1b1 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture.c
> +++ b/drivers/gpu/drm/xe/xe_guc_capture.c
> @@ -1856,7 +1856,8 @@ xe_guc_capture_get_matching_and_lock(struct xe_exec_queue *q)
> return NULL;
>
> xe = gt_to_xe(q->gt);
> - if (xe->wedged.mode >= 2 || !xe_device_uc_enabled(xe) || IS_SRIOV_VF(xe))
> + if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG || !xe_device_uc_enabled(xe) ||
> + IS_SRIOV_VF(xe))
nit: personally I would split this into 3 statements since each of them
is totally different reasoning, and let the compiler do any optimization
> return NULL;
>
> ss = &xe->devcoredump.snapshot;
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 80f748baad3f..54020b26c772 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -863,8 +863,9 @@ void xe_guc_submit_wedge(struct xe_guc *guc)
> err = devm_add_action_or_reset(guc_to_xe(guc)->drm.dev,
> guc_submit_wedged_fini, guc);
> if (err) {
> - xe_gt_err(gt, "Failed to register clean-up on wedged.mode=2; "
> - "Although device is wedged.\n");
> + xe_gt_err(gt, "Failed to register clean-up in wedged.mode=%s; "
> + "Although device is wedged.\n",
> + xe_device_wedged_mode_to_string(XE_WEDGED_MODE_UPON_ANY_HANG));
> return;
> }
>
> @@ -879,7 +880,7 @@ static bool guc_submit_hint_wedged(struct xe_guc *guc)
> {
> struct xe_device *xe = guc_to_xe(guc);
>
> - if (xe->wedged.mode != 2)
> + if (xe->wedged.mode != XE_WEDGED_MODE_UPON_ANY_HANG)
> return false;
>
> if (xe_device_wedged(xe))
> diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
> index 1c4dfafbcd0b..6848abf23acf 100644
> --- a/drivers/gpu/drm/xe/xe_module.c
> +++ b/drivers/gpu/drm/xe/xe_module.c
> @@ -10,6 +10,7 @@
>
> #include <drm/drm_module.h>
>
> +#include "xe_device.h"
> #include "xe_drv.h"
> #include "xe_configfs.h"
> #include "xe_hw_fence.h"
> @@ -25,7 +26,7 @@ struct xe_modparam xe_modparam = {
> #ifdef CONFIG_PCI_IOV
> .max_vfs = IS_ENABLED(CONFIG_DRM_XE_DEBUG) ? ~0 : 0,
> #endif
> - .wedged_mode = 1,
> + .wedged_mode = XE_WEDGED_MODE_DEFAULT,
> .svm_notifier_size = 512,
> /* the rest are 0 by default */
> };
> @@ -68,7 +69,7 @@ MODULE_PARM_DESC(max_vfs,
> "(0 = no VFs [default]; N = allow up to N VFs)");
> #endif
>
> -module_param_named_unsafe(wedged_mode, xe_modparam.wedged_mode, int, 0600);
> +module_param_named_unsafe(wedged_mode, xe_modparam.wedged_mode, uint, 0600);
> MODULE_PARM_DESC(wedged_mode,
> "Module's default policy for the wedged mode - 0=never, 1=upon-critical-errors[default], 2=upon-any-hang");
>
> diff --git a/drivers/gpu/drm/xe/xe_module.h b/drivers/gpu/drm/xe/xe_module.h
> index 5a3bfea8b7b4..1c75f38ca393 100644
> --- a/drivers/gpu/drm/xe/xe_module.h
> +++ b/drivers/gpu/drm/xe/xe_module.h
> @@ -21,7 +21,7 @@ struct xe_modparam {
> #ifdef CONFIG_PCI_IOV
> unsigned int max_vfs;
> #endif
> - int wedged_mode;
> + unsigned int wedged_mode;
> u32 svm_notifier_size;
> };
>
More information about the Intel-xe
mailing list