Re: [PATCH v4 30/32] drm/xe/gt: Report 'Fault response' pagefault error using SIGID

On Fri, Aug 14, 2026 at 12:30:35PM +0200, Michal Wajdeczko wrote:
> 
> 
> On 8/14/2026 12:12 AM, Daniele Ceraolo Spurio wrote:
> > 
> > 
> > On 8/13/2026 12:46 PM, Michal Wajdeczko wrote:
> >>
> >> On 8/13/2026 8:37 PM, Rodrigo Vivi wrote:
> >>> On Wed, Aug 12, 2026 at 09:14:46PM +0200, Michal Wajdeczko wrote:
> >>>> Report 'Unsuccessful response' error using xe_log_err() helper.
> >>>>
> >>>> Signed-off-by: Michal Wajdeczko <[email protected]>
> >>>> Cc: Rodrigo Vivi <[email protected]>
> >>>> Cc: Matthew Brost <[email protected]>
> >>>> Cc: Daniele Ceraolo Spurio <[email protected]>
> >>>> Cc: Aravind Iddamsetty <[email protected]>
> >>>> Cc: Mallesh Koujalagi <[email protected]>
> >>>> ---
> >>>> v2: rebased (Michal) reword message (Mallesh)
> >>>> ---
> >>>>   drivers/gpu/drm/xe/xe_pagefault.c | 4 ++--
> >>>>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/xe/xe_pagefault.c b/drivers/gpu/drm/xe/xe_pagefault.c
> >>>> index 7e829c61387f..eb600c466187 100644
> >>>> --- a/drivers/gpu/drm/xe/xe_pagefault.c
> >>>> +++ b/drivers/gpu/drm/xe/xe_pagefault.c
> >>>> @@ -14,6 +14,7 @@
> >>>>   #include "xe_gt_types.h"
> >>>>   #include "xe_gt_stats.h"
> >>>>   #include "xe_hw_engine.h"
> >>>> +#include "xe_log.h"
> >>>>   #include "xe_pagefault.h"
> >>>>   #include "xe_pagefault_types.h"
> >>>>   #include "xe_svm.h"
> >>>> @@ -625,8 +626,7 @@ static void xe_pagefault_queue_work(struct work_struct *w)
> >>>>                   xe_pagefault_save_to_vm(gt_to_xe(gt), pf);
> >>>>                   xe_pagefault_cache_start_invalidate(cache_start);
> >>>>                   xe_pagefault_print(pf);
> >>>> -                xe_gt_info(pf->gt, "Fault response: Unsuccessful %pe\n",
> >>>> -                       ERR_PTR(err));
> >>>> +                xe_log_err(pf->gt, PAGEFAULT, err, "Unsuccessful response\n");
> >>> I'm wondering if we should really go from info to err here...
> >> my question was different:
> >>     "why this err is only reported as info level?"
> >> as once we hit this condition we also print large dump
> >> (see xe_pagefault_print) likely for debug/diagnostics
> >> so it seems important
> > 
> > drive-by comment, but AFAIU this error can be intentionally triggered by UMD by simply accessing an invalid PPGTT address (see e.g. xe_exec_fault_mode@invalid-va), and we usually avoid using error-level logs for something that UMD can cause on demand.
> 
> thanks for explanation! 
> 
> so we have two choices here:
> 
> 1) drop this patch completely, or
> 2) switch to SIGID reporting with INFO severity
> 
> but currently xe_log_info() does not take an 'err' parameter
> so we will not print the actual error code as it is today
> 
> so we have two more choices:
> 
> a) update xe_log_info() macro to accept 'err', or
> b) add new xe_log_err_info() macro that takes 'err'
>    but it will still use CPER_SEV_INFO severity
> 
> I'm in favor for 2) + b)

I agree

> 
> > 
> > Daniele
> > 
> >>
> >> @Matthew, comments?
> >>
> >>> Why not staying with xe_log_info or now?
> >> I can change that for xe_log_info, but then (by design)
> >> we will not print an error
> >>
> >>> But also, why err and not ERR_PTR(err)?
> >> patch 10/32 [1] introduced helpers that take just errno
> >> value and convert that into data { ERR_PTR(err), 0 } pair
> >>
> >> [1] https://patchwork.freedesktop.org/patch/745993/?series=171022&rev=4
> >>
> >>>>               } else {
> >>>>                   xe_gt_stats_incr(pf->gt, XE_GT_STATS_ID_INVALID_PREFETCH_PAGEFAULT_COUNT, 1);
> >>>>                   xe_gt_dbg(pf->gt, "Prefetch Fault response: Unsuccessful %pe\n",
> >>>> -- 
> >>>> 2.47.1
> >>>>
> > 
>