[Intel-gfx] [PATCH igt] tools/error_decode: Print ASCII user buffers

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-15 13:35:43)
> 
> On 14/11/2017 13:19, Chris Wilson wrote:
> > Look at the first 16 characters of the user buffer and if they are all
> > printable, assume that user buffer contains ASCII data (e.g. a debug log)
> > that we want to simply print out. Otherwise, we treat it as a binary data
> > and hexdump as before.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   tools/intel_error_decode.c | 20 ++++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> > 
> > diff --git a/tools/intel_error_decode.c b/tools/intel_error_decode.c
> > index cdef3b18..84ef72b6 100644
> > --- a/tools/intel_error_decode.c
> > +++ b/tools/intel_error_decode.c
> > @@ -51,6 +51,7 @@
> >   #include <assert.h>
> >   #include <intel_bufmgr.h>
> >   #include <zlib.h>
> > +#include <ctype.h>
> >   
> >   #include "intel_chipset.h"
> >   #include "intel_io.h"
> > @@ -434,6 +435,16 @@ print_fault_data(unsigned devid, uint32_t data1, uint32_t data0)
> >   
> >   #define MAX_RINGS 10 /* I really hope this never... */
> >   
> > +static bool maybe_ascii(const void *data, int check)
> > +{
> > +     const char *c = data;
> > +     while (check--) {
> > +             if (!isprint(*c++))
> > +                     return false;
> > +     }
> > +     return true;
> > +}
> > +
> >   static void decode(struct drm_intel_decode *ctx,
> >                  const char *buffer_name,
> >                  const char *ring_name,
> > @@ -458,6 +469,15 @@ static void decode(struct drm_intel_decode *ctx,
> >               drm_intel_decode_set_batch_pointer(ctx, data, gtt_offset,
> >                                                  *count);
> >               drm_intel_decode(ctx);
> > +     } else if (maybe_ascii(data, 16)) {
> 
> Is there a matching i915 patch for this? I am wondering if we could put 
> a stronger signature header if we are going to save some logs in our bos.

No, this is content from (entirely under the control of) userspace.

> > +             char *c = (char *)data;
> > +             int max = 4 * *count;
> > +             for (int i = 0; i < max; i++) {
> > +                     if (!c[i])
> > +                             break;
> > +                     putchar(c[i]);
> 
> Why char by char if the only termination is nul byte and not just puts?

Is there a putsn? There's no guarantee of a nul terminator either.
-Chris