diff mbox

[RFC,3/4] qemu-log: new option -dfilter to limit output

Message ID 1395844634-11729-4-git-send-email-alex.bennee@linaro.org
State Superseded
Headers show

Commit Message

Alex Bennée March 26, 2014, 2:37 p.m. UTC
From: Alex Bennée <alex.bennee@linaro.org>

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 <alex.bennee@linaro.org>

Comments

Christopher Covington March 26, 2014, 3:49 p.m. UTC | #1
Hi Alex,

Neat series.

On 03/26/2014 10:37 AM, alex.bennee@linaro.org wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> 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.
> 

> diff --git a/qemu-options.hx b/qemu-options.hx
> index ee5437b..a5cd095 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2667,6 +2667,15 @@ 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)

It might be helpful to include a quick note in the documentation about the
format of range (based on your commit message and the code, I take it that
"a-b" means <a to b> while "a+b" means <a to a+b>).

Thanks,
Christopher
Alex Bennée March 26, 2014, 5:32 p.m. UTC | #2
Christopher Covington <cov@codeaurora.org> writes:

> Hi Alex,
>
> Neat series.
>
> On 03/26/2014 10:37 AM, alex.bennee@linaro.org wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
>> 
>> 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.
>> 
>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ee5437b..a5cd095 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2667,6 +2667,15 @@ 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)
>
> It might be helpful to include a quick note in the documentation about the
> format of range (based on your commit message and the code, I take it that
> "a-b" means <a to b> while "a+b" means <a to a+b>).

Indeed. I shall try and update the relevant bits to make nice docs
useful ;-)

>
> Thanks,
> Christopher
Alex Bennée March 27, 2014, 11:44 a.m. UTC | #3
Christopher Covington <cov@codeaurora.org> writes:

> Hi Alex,
>
<snip>

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
@item -dfilter @var{range1}[,...]
@findex -dfilter
Filter debug output to that relevant to a range of target addresses. The filter
spec can be either 0x${START}-0x${END} or 0x${START}+0x${OFF} where ${START}
${END} and ${OFF} are the addresses and sizes required. For example:

    -dfilter 0x8000-0x9000,0xffffffc000080000+0x200

Will dump output for any code in the 0x1000 sized block starting at 0x8000 and
the 0x200 sized block starting at 0xffffffc000080000.
ETEXI

Does that read clearly enough? I assume that should magically make it's
way to the man page somehow?
Christopher Covington March 27, 2014, 2:14 p.m. UTC | #4
Hi Alex,

On 03/27/2014 07:44 AM, Alex Bennée wrote:
> 
> Christopher Covington <cov@codeaurora.org> writes:
> 
>> Hi Alex,
>>
> <snip>
> 
> 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
> @item -dfilter @var{range1}[,...]
> @findex -dfilter
> Filter debug output to that relevant to a range of target addresses. The filter
> spec can be either 0x${START}-0x${END} or 0x${START}+0x${OFF} where ${START}
> ${END} and ${OFF} are the addresses and sizes required. For example:
> 
>     -dfilter 0x8000-0x9000,0xffffffc000080000+0x200
> 
> Will dump output for any code in the 0x1000 sized block starting at 0x8000 and
> the 0x200 sized block starting at 0xffffffc000080000.
> ETEXI
> 
> Does that read clearly enough? I assume that should magically make it's
> way to the man page somehow?

This level of detail is helpful. I think "range1" and "filter spec" should use
matching terms. Unless space is at a premium, spelling out ${OFFSET} might aid
some readers.

In the C portion, I wonder if renaming qemu_log_in_addr_range to something
like qemu_addr_in_dfilter_ranges might add clarity?

Christopher
Peter Maydell March 27, 2014, 2:17 p.m. UTC | #5
On 27 March 2014 11:44, Alex Bennée <alex.bennee@linaro.org> wrote:
> 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
> @item -dfilter @var{range1}[,...]
> @findex -dfilter
> Filter debug output to that relevant to a range of target addresses. The filter
> spec can be either 0x${START}-0x${END} or 0x${START}+0x${OFF} where ${START}
> ${END} and ${OFF} are the addresses and sizes required. For example:
>
>     -dfilter 0x8000-0x9000,0xffffffc000080000+0x200
>
> Will dump output for any code in the 0x1000 sized block starting at 0x8000 and
> the 0x200 sized block starting at 0xffffffc000080000.
> ETEXI

I take it that for start-end ranges the 'start' address is
included and the 'end' address is not, then? That might
be worth mentioning explicitly.

thanks
-- PMM
diff mbox

Patch

diff --git a/include/qemu/log.h b/include/qemu/log.h
index d515424..25710ad 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -174,6 +174,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 2897596..e2fa3fd 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 GSList *debug_regions = NULL;
 
 void qemu_log(const char *fmt, ...)
 {
@@ -103,6 +105,60 @@  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) {
+        GSList *region = debug_regions;
+        do {
+            struct Range *range = region->data;
+            if (addr >= range->begin && addr <= range->end) {
+                return true;
+            }
+            region = g_slist_next(region);
+        } while (region);
+        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++;
+        while (r) {
+            gchar *delim = g_strrstr(r, "-");
+            if (!delim) {
+                delim = g_strrstr(r, "+");
+            }
+            if (delim) {
+                struct Range *range = g_malloc(sizeof(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();
+                }
+                debug_regions = g_slist_append(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 ee5437b..a5cd095 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2667,6 +2667,15 @@  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
+@item -dfilter @var{range1}[,...]
+@findex -dfilter
+Filter debug output to that relevant to a range of target addresses
+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 02bf8ec..c036367 100644
--- a/vl.c
+++ b/vl.c
@@ -3342,6 +3342,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_s:
                 add_device_config(DEV_GDB, "tcp::" DEFAULT_GDBSTUB_PORT);
                 break;