<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<div><br>
</div>
<div id="mail-editor-reference-message-container" dir="auto"><br>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" style="font-size: 11pt;"><strong>From:</strong> Welty, Brian <brian.welty@intel.com><br>
<strong>Sent:</strong> Saturday, October 21, 2023 12:17:01 am<br>
<strong>To:</strong> Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com>; intel-xe@lists.freedesktop.org <intel-xe@lists.freedesktop.org><br>
<strong>Cc:</strong> Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com><br>
<strong>Subject:</strong> Re: [PATCH v1 06/12] drm/xe: Rename XE_RESET_FAILED_UEVENT to XE_RESET_REQUIRED_UEVENT.<br>
</div>
<br>
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><font size="2"><span style="font-size:11pt;">
<div class="PlainText" dir="auto"><br>
<br>
On 10/19/2023 9:55 PM, Himal Prasad Ghimiray wrote:<br>
> DEVICE_STATUS=NEEDS_RESET will be used for other reasons apart from<br>
> gt reset failure. Hence use more generic uevent name and provide reason<br>
> for reset along with the uevent.<br>
<br>
Looks good to me.<br>
Reviewed-by: Brian Welty <brian.welty@intel.com><br>
<br>
But as I mention in other email, this looks unsafe if 2 GT resets are<br>
happening (and fail) concurrently. uevent will be overwritten/corrupted.<br>
But can fix separate from this patch.</div>
<div class="PlainText" dir="auto"><br>
</div>
<div class="PlainText" dir="auto">Hi Brian,</div>
<div class="PlainText" dir="auto"><br>
</div>
<div class="PlainText" dir="auto">Thanks for the review and input. </div>
<div class="PlainText" dir="auto">Will work on the input in separate patch.</div>
<div class="PlainText" dir="auto"><br>
</div>
<div class="PlainText" dir="auto">BR</div>
<div class="PlainText" dir="auto">Himal Ghimiray <br>
<br>
> <br>
> Cc: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com><br>
> Cc: Brian Welty <brian.welty@intel.com><br>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com><br>
> ---<br>
> drivers/gpu/drm/xe/xe_gt.c | 13 +++++++------<br>
> include/uapi/drm/xe_drm.h | 17 ++++++++++++-----<br>
> 2 files changed, 19 insertions(+), 11 deletions(-)<br>
> <br>
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c<br>
> index 74e1f47bd401..91e0a9a7f1cd 100644<br>
> --- a/drivers/gpu/drm/xe/xe_gt.c<br>
> +++ b/drivers/gpu/drm/xe/xe_gt.c<br>
> @@ -545,16 +545,17 @@ static int do_gt_restart(struct xe_gt *gt)<br>
> <br>
> static void xe_uevent_gt_reset_failure(struct pci_dev *pdev, u8 tile_id, u8 gt_id)<br>
> {<br>
> - char *reset_event[4];<br>
> + char *reset_event[5];<br>
> <br>
> - reset_event[0] = XE_RESET_FAILED_UEVENT "=NEEDS_RESET";<br>
> - reset_event[1] = kasprintf(GFP_KERNEL, "TILE_ID=%d", tile_id);<br>
> - reset_event[2] = kasprintf(GFP_KERNEL, "GT_ID=%d", gt_id);<br>
> - reset_event[3] = NULL;<br>
> + reset_event[0] = XE_RESET_REQUIRED_UEVENT;<br>
> + reset_event[1] = XE_RESET_REQUIRED_UEVENT_REASON_GT;<br>
> + reset_event[2] = kasprintf(GFP_KERNEL, "TILE_ID=%d", tile_id);<br>
> + reset_event[3] = kasprintf(GFP_KERNEL, "GT_ID=%d", gt_id);<br>
> + reset_event[4] = NULL;<br>
> kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, reset_event);<br>
> <br>
> - kfree(reset_event[1]);<br>
> kfree(reset_event[2]);<br>
> + kfree(reset_event[3]);<br>
> }<br>
> <br>
> static int gt_reset(struct xe_gt *gt)<br>
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h<br>
> index 24bf8f0f52e8..ae1b1c7528d5 100644<br>
> --- a/include/uapi/drm/xe_drm.h<br>
> +++ b/include/uapi/drm/xe_drm.h<br>
> @@ -19,12 +19,19 @@ extern "C" {<br>
> /**<br>
> * DOC: uevent generated by xe on it's pci node.<br>
> *<br>
> - * XE_RESET_FAILED_UEVENT - Event is generated when attempt to reset gt<br>
> - * fails. The value supplied with the event is always "NEEDS_RESET".<br>
> - * Additional information supplied is tile id and gt id of the gt unit for<br>
> - * which reset has failed.<br>
> + * XE_RESET_REQUIRED_UEVENT - Event is generated when device needs reset.<br>
> + * The REASON is provided along with the event for which reset is required.<br>
> + * On the basis of REASONS, additional information might be supplied.<br>
> */<br>
> -#define XE_RESET_FAILED_UEVENT "DEVICE_STATUS"<br>
> +#define XE_RESET_REQUIRED_UEVENT "DEVICE_STATUS=NEEDS_RESET"<br>
> +<br>
> +/**<br>
> + * XE_RESET_REQUIRED_UEVENT_REASON_GT - Reason provided to XE_RESET_REQUIRED_UEVENT<br>
> + * incase of gt reset failure. The additional information supplied is tile id and<br>
> + * gt id of the gt unit for which reset has failed.<br>
> + */<br>
> +#define XE_RESET_REQUIRED_UEVENT_REASON_GT "REASON=GT_RESET_FAILED"<br>
> +<br>
> <br>
> /**<br>
> * struct xe_user_extension - Base class for defining a chain of extensions<br>
</div>
</span></font><br>
</div>
</body>
</html>