Re: [Qemu-devel] [PATCH v4 07/11] qemu-log: new option -dfilter to limit output

[Alex Bennée]

Christopher Covington <address@hidden> writes:

> Hi Alex,
>
> On 08/03/2015 05:14 AM, Alex Bennée wrote:
>> When debugging big programs or system emulation sometimes you want both
>> the verbosity of cpu,exec et all but don't want to generate lots of logs
>> for unneeded stuff. This patch adds a new option -dfilter which allows
>> you to specify interesting address ranges in the form:
>> 
>>   -dfilter 0x8000-0x9000,0xffffffc000080000+0x200,...
>> 
>> Then logging code can use the new qemu_log_in_addr_range() function to
>> decide if it will output logging information for the given range.
>> 
>> Signed-off-by: Alex Bennée <address@hidden>
>
> My usual flow is to filter based on mode (CurrentEL on AArch64) and PID
> (CONTEXTIDR on AArch64). Do you foresee any problems with adding such
> filters?

I have indeed added such filters on for my own debugging efforts.
However the problem is they are very target focused and I don't want to
pollute the generic code with target specific hacks. I suspect we need
two things:

  - a way to add target specific options to the UI
  - a target agnostic hook which allows additional refinement of the filter

>
> Thanks,
> Christopher Covington
>
>> v2
>>   - More clean-ups to the documentation
>> 
>> v3
>>   - re-base
>>   - use GArray instead of GList to avoid cache bouncing
>>   - checkpatch fixes
>> ---
>>  include/qemu/log.h |  2 ++
>>  qemu-log.c         | 57 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  qemu-options.hx    | 16 +++++++++++++++
>>  vl.c               |  3 +++
>>  4 files changed, 78 insertions(+)
>> 
>> diff --git a/include/qemu/log.h b/include/qemu/log.h
>> index b80f8f5..ade1f76 100644
>> --- a/include/qemu/log.h
>> +++ b/include/qemu/log.h
>> @@ -182,6 +182,8 @@ static inline void qemu_set_log(int log_flags)
>>  }
>>  
>>  void qemu_set_log_filename(const char *filename);
>> +void qemu_set_dfilter_ranges(const char *ranges);
>> +bool qemu_log_in_addr_range(uint64_t addr);
>>  int qemu_str_to_log_mask(const char *str);
>>  
>>  /* Print a usage message listing all the valid logging categories
>> diff --git a/qemu-log.c b/qemu-log.c
>> index 77ed7bc..b3ebd3c 100644
>> --- a/qemu-log.c
>> +++ b/qemu-log.c
>> @@ -19,11 +19,13 @@
>>  
>>  #include "qemu-common.h"
>>  #include "qemu/log.h"
>> +#include "qemu/range.h"
>>  
>>  static char *logfilename;
>>  FILE *qemu_logfile;
>>  int qemu_loglevel;
>>  static int log_append = 0;
>> +static GArray *debug_regions;
>>  
>>  void qemu_log(const char *fmt, ...)
>>  {
>> @@ -92,6 +94,61 @@ void qemu_set_log_filename(const char *filename)
>>      qemu_set_log(qemu_loglevel);
>>  }
>>  
>> +/* Returns true if addr is in our debug filter or no filter defined
>> + */
>> +bool qemu_log_in_addr_range(uint64_t addr)
>> +{
>> +    if (debug_regions) {
>> +        int i = 0;
>> +        for (i = 0; i < debug_regions->len; i++) {
>> +            struct Range *range = &g_array_index(debug_regions, Range, i);
>> +            if (addr >= range->begin && addr <= range->end) {
>> +                return true;
>> +            }
>> +        }
>> +        return false;
>> +    } else {
>> +        return true;
>> +    }
>> +}
>> +
>> +
>> +void qemu_set_dfilter_ranges(const char *filter_spec)
>> +{
>> +    gchar **ranges = g_strsplit(filter_spec, ",", 0);
>> +    if (ranges) {
>> +        gchar **next = ranges;
>> +        gchar *r = *next++;
>> +        debug_regions = g_array_sized_new(FALSE, FALSE,
>> +                                          sizeof(Range), 
>> g_strv_length(ranges));
>> +        while (r) {
>> +            gchar *delim = g_strrstr(r, "-");
>> +            if (!delim) {
>> +                delim = g_strrstr(r, "+");
>> +            }
>> +            if (delim) {
>> +                struct Range range;
>> +                range.begin = strtoul(r, NULL, 0);
>> +                switch (*delim) {
>> +                case '+':
>> +                    range.end = range.begin + strtoul(delim+1, NULL, 0);
>> +                    break;
>> +                case '-':
>> +                    range.end = strtoul(delim+1, NULL, 0);
>> +                    break;
>> +                default:
>> +                    g_assert_not_reached();
>> +                }
>> +                g_array_append_val(debug_regions, range);
>> +            } else {
>> +                g_error("Bad range specifier in: %s", r);
>> +            }
>> +            r = *next++;
>> +        }
>> +        g_strfreev(ranges);
>> +    }
>> +}
>> +
>>  const QEMULogItem qemu_log_items[] = {
>>      { CPU_LOG_TB_OUT_ASM, "out_asm",
>>        "show generated host assembly code for each compiled TB" },
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ae53346..90f0df9 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2987,6 +2987,22 @@ STEXI
>>  Output log in @var{logfile} instead of to stderr
>>  ETEXI
>>  
>> +DEF("dfilter", HAS_ARG, QEMU_OPTION_DFILTER, \
>> +    "-dfilter range,..  filter debug output to range of addresses (useful 
>> for -d cpu,exec,etc..)\n",
>> +    QEMU_ARCH_ALL)
>> +STEXI
>> address@hidden -dfilter @var{range1}[,...]
>> address@hidden -dfilter
>> +Filter debug output to that relevant to a range of target addresses. The 
>> filter
>> +spec can be either @address@hidden or @address@hidden where @var{start}
>> address@hidden and @var{size} are the addresses and sizes required. For 
>> example:
>> address@hidden
>> +    -dfilter 0x8000-0x9000,0xffffffc000080000+0x200
>> address@hidden example
>> +Will dump output for any code in the 0x1000 sized block starting at 0x8000 
>> and
>> +the 0x200 sized block starting at 0xffffffc000080000.
>> +ETEXI
>> +
>>  DEF("L", HAS_ARG, QEMU_OPTION_L, \
>>      "-L path         set the directory for the BIOS, VGA BIOS and 
>> keymaps\n",
>>      QEMU_ARCH_ALL)
>> diff --git a/vl.c b/vl.c
>> index 1d2de4f..05211cf 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3349,6 +3349,9 @@ int main(int argc, char **argv, char **envp)
>>              case QEMU_OPTION_D:
>>                  log_file = optarg;
>>                  break;
>> +            case QEMU_OPTION_DFILTER:
>> +                qemu_set_dfilter_ranges(optarg);
>> +                break;
>>              case QEMU_OPTION_PERFMAP:
>>                  tb_enable_perfmap();
>>                  break;
>> 

-- 
Alex Bennée