Re: [Intel-xe] [PATCH 1/2] drm/xe: Print devcoredump to drm_info

["Summers, Stuart" <stuart.summers@intel.com>]

On Wed, 2023-10-04 at 09:37 -0400, Rodrigo Vivi wrote:
> On Tue, Sep 26, 2023 at 09:20:55PM +0000, Stuart Summers wrote:
> > Currently the devcoredump is available in the file system
> > for a user. However 1) CI isn't currently set up to dump
> > this data and 2) if we try to dump this during the driver
> > load when a failure is observed, the driver will be torn
> > down before we have a chance to actually read this data.
> > 
> > Add a quick routine to print this out with drm_info as well
> > if CONFIG_DRM_XE_DEBUG is enabled.
> 
> I feel this is kind of a duplication of simple_error_capture()
> that I thought we could kill... at least that has a more specific

What was the reason to kill it?

> config: CONFIG_DRM_XE_SIMPLE_ERROR_CAPTURE
> 
> I mean, maybe the right thing to do is to kill that and have
> this one here, but with a dedicated config?!...

I feel like the ability to dump extra information when an error happens
during driver load is critical for debug. We could come up with a way
to dump this during teardown to some other file that is persistent -
even in /tmp or something?

Thanks,
Stuart

> 
> > 
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_devcoredump.c | 17 ++++++++++++++++-
> >  drivers/gpu/drm/xe/xe_devcoredump.h |  3 +++
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c
> > b/drivers/gpu/drm/xe/xe_devcoredump.c
> > index 68abc0b195be..aa41d8e9b602 100644
> > --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> > +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> > @@ -78,7 +78,11 @@ static ssize_t xe_devcoredump_read(char *buffer,
> > loff_t offset,
> >         iter.remain = count;
> >  
> >         ss = &coredump->snapshot;
> > -       p = drm_coredump_printer(&iter);
> > +
> > +       if (iter.data)
> > +               p = drm_coredump_printer(&iter);
> > +       else
> > +               p = drm_info_printer(coredump_to_xe(coredump)-
> > >drm.dev);
> >  
> >         drm_printf(&p, "**** Xe Device Coredump ****\n");
> >         drm_printf(&p, "kernel: " UTS_RELEASE "\n");
> > @@ -102,6 +106,15 @@ static ssize_t xe_devcoredump_read(char
> > *buffer, loff_t offset,
> >         return count - iter.remain;
> >  }
> >  
> > +/* Print the coredump locally also for debug purposes */
> > +void
> > +xe_devcoredump_print(struct xe_devcoredump *coredump)
> > +{
> > +#if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
> > +       xe_devcoredump_read(NULL, 0, 0, coredump, 0);
> > +#endif
> > +}
> > +
> >  static void xe_devcoredump_free(void *data)
> >  {
> >         struct xe_devcoredump *coredump = data;
> > @@ -192,5 +205,7 @@ void xe_devcoredump(struct xe_exec_queue *q)
> >  
> >         dev_coredumpm(xe->drm.dev, THIS_MODULE, coredump, 0,
> > GFP_KERNEL,
> >                       xe_devcoredump_read, xe_devcoredump_free);
> > +
> > +       xe_devcoredump_print(coredump);
> >  }
> >  #endif
> > diff --git a/drivers/gpu/drm/xe/xe_devcoredump.h
> > b/drivers/gpu/drm/xe/xe_devcoredump.h
> > index 6ac218a5c194..ba0c2a7b71b4 100644
> > --- a/drivers/gpu/drm/xe/xe_devcoredump.h
> > +++ b/drivers/gpu/drm/xe/xe_devcoredump.h
> > @@ -8,6 +8,9 @@
> >  
> >  struct xe_device;
> >  struct xe_exec_queue;
> > +struct xe_devcoredump;
> > +
> > +void xe_devcoredump_print(struct xe_devcoredump *coredump);
> >  
> >  #ifdef CONFIG_DEV_COREDUMP
> >  void xe_devcoredump(struct xe_exec_queue *q);
> > -- 
> > 2.34.1
> >