[Qemu-devel] [RFC PATCH 0/4] qemu-log: various fixes and enhancements

[Qemu-devel] [RFC PATCH 0/4] qemu-log: various fixes and enhancements

[alex.bennee]

From: Alex Bennée <alex.bennee@linaro.org>

Hi,

While working on aarch64 stuff I've made the following changes in my
local tree to help me debug. The first is a simple documentation fix.
The second is very helpful when your debugging missing instructions
while several shells deep in a build system. The final two I've
recently written as I'm doing some system mode debugging and found
that it's hard to have all the code gen logs turned on when your test
case is booting the kernel.

Comments and review please.

Alex Bennée (4):
  qemu-log: correct help text for -d cpu
  qemu-log: support simple pid substitution in logfile
  qemu-log: new option -dfilter to limit output
  qemu-log: make in_asm, out_asm and op_opt understand dfilter

 cpu-exec.c         |  2 +-
 include/qemu/log.h |  2 ++
 qemu-log.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 qemu-options.hx    |  9 +++++++
 tcg/tcg.c          | 24 ++++++++---------
 tcg/tcg.h          |  5 ++--
 translate-all.c    |  7 ++---
 vl.c               |  3 +++
 8 files changed, 105 insertions(+), 22 deletions(-)

-- 
1.9.1

[Qemu-devel] [RFC PATCH 1/4] qemu-log: correct help text for -d cpu

[alex.bennee]

From: Alex Bennée <alex.bennee@linaro.org>

This doesn't just dump CPU state on translation but on every block
entrance.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/qemu-log.c b/qemu-log.c
index 797f2af..35bbb56 100644
--- a/qemu-log.c
+++ b/qemu-log.c
@@ -105,7 +105,7 @@ const QEMULogItem qemu_log_items[] = {
     { CPU_LOG_EXEC, "exec",
       "show trace before each executed TB (lots of logs)" },
     { CPU_LOG_TB_CPU, "cpu",
-      "show CPU state before block translation" },
+      "show CPU registers before each executed TB (lots of logs)" },
     { CPU_LOG_PCALL, "pcall",
       "x86 only: show protected mode far calls/returns/exceptions" },
     { CPU_LOG_RESET, "cpu_reset",
-- 
1.9.1

[Qemu-devel] [RFC PATCH 2/4] qemu-log: support simple pid substitution in logfile

[alex.bennee]

From: Alex Bennée <alex.bennee@linaro.org>

When debugging stuff that occurs over several forks it would be useful
not to keep overwriting the one logfile you've set-up. This allows a
simple %d to be included once in the logfile parameter which is
substituted with getpid().

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/qemu-log.c b/qemu-log.c
index 35bbb56..2897596 100644
--- a/qemu-log.c
+++ b/qemu-log.c
@@ -81,11 +81,24 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
         qemu_log_close();
     }
 }
-
+/*
+ * Allow the user to include %d in their logfile which will be
+ * substituted with the current PID. This is useful for debugging many
+ * nested linux-user tasks but will result in lots of logs.
+ */
 void qemu_set_log_filename(const char *filename)
 {
     g_free(logfilename);
-    logfilename = g_strdup(filename);
+    if (g_strrstr(filename, "%d")) {
+        /* if we are going to format this we'd better validate first */
+        if (g_regex_match_simple("^[^%]+%d[^%]+$", filename, 0, 0)) {
+            logfilename = g_strdup_printf(filename, getpid());
+        } else {
+            g_error("Bad logfile format: %s", filename);
+        }
+    } else {
+        logfilename = g_strdup(filename);
+    }
     qemu_log_close();
     qemu_set_log(qemu_loglevel);
 }
-- 
1.9.1

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

[alex.bennee]

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>

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;
-- 
1.9.1

[Qemu-devel] [RFC PATCH 4/4] qemu-log: make in_asm, out_asm and op_opt understand dfilter

[alex.bennee]

From: Alex Bennée <alex.bennee@linaro.org>

This ensures the code generation debug code will honour -dfilter if set.

diff --git a/cpu-exec.c b/cpu-exec.c
index 0914d3c..9aa3f3f 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -610,7 +610,7 @@ int cpu_exec(CPUArchState *env)
                     next_tb = 0;
                     tcg_ctx.tb_ctx.tb_invalidated_flag = 0;
                 }
-                if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
+                if (qemu_loglevel_mask(CPU_LOG_EXEC) && qemu_log_in_addr_range(tb->pc)) {
                     qemu_log("Trace %p [" TARGET_FMT_lx "] %s\n",
                              tb->tc_ptr, tb->pc, lookup_symbol(tb->pc));
                 }
diff --git a/tcg/tcg.c b/tcg/tcg.c
index f1e0763..57d2b82 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2452,8 +2452,8 @@ static void dump_op_count(void)
 #endif
 
 
-static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
-                                      long search_pc)
+static inline int tcg_gen_code_common(TCGContext *s, uint64_t target_pc,
+                                      uint8_t *gen_code_buf, long search_pc)
 {
     TCGOpcode opc;
     int op_index;
@@ -2461,7 +2461,8 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
     const TCGArg *args;
 
 #ifdef DEBUG_DISAS
-    if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP))) {
+    if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP)
+                 && qemu_log_in_addr_range(target_pc))) {
         qemu_log("OP:\n");
         tcg_dump_ops(s);
         qemu_log("\n");
@@ -2489,7 +2490,8 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
 #endif
 
 #ifdef DEBUG_DISAS
-    if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT))) {
+    if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT)
+                 && qemu_log_in_addr_range(target_pc))) {
         qemu_log("OP after optimization and liveness analysis:\n");
         tcg_dump_ops(s);
         qemu_log("\n");
@@ -2512,11 +2514,6 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
         tcg_table_op_count[opc]++;
 #endif
         def = &tcg_op_defs[opc];
-#if 0
-        printf("%s: %d %d %d\n", def->name,
-               def->nb_oargs, def->nb_iargs, def->nb_cargs);
-        //        dump_regs(s);
-#endif
         switch(opc) {
         case INDEX_op_mov_i32:
         case INDEX_op_mov_i64:
@@ -2581,7 +2578,7 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
     return -1;
 }
 
-int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
+int tcg_gen_code(TCGContext *s, uint64_t target_pc, uint8_t *gen_code_buf)
 {
 #ifdef CONFIG_PROFILER
     {
@@ -2597,7 +2594,7 @@ int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
     }
 #endif
 
-    tcg_gen_code_common(s, gen_code_buf, -1);
+    tcg_gen_code_common(s, target_pc, gen_code_buf, -1);
 
     /* flush instruction cache */
     flush_icache_range((uintptr_t)gen_code_buf, (uintptr_t)s->code_ptr);
@@ -2609,9 +2606,10 @@ int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
    offset bytes from the start of the TB.  The contents of gen_code_buf must
    not be changed, though writing the same values is ok.
    Return -1 if not found. */
-int tcg_gen_code_search_pc(TCGContext *s, uint8_t *gen_code_buf, long offset)
+int tcg_gen_code_search_pc(TCGContext *s, uint64_t tpc,
+                           uint8_t *gen_code_buf, long offset)
 {
-    return tcg_gen_code_common(s, gen_code_buf, offset);
+    return tcg_gen_code_common(s, tpc, gen_code_buf, offset);
 }
 
 #ifdef CONFIG_PROFILER
diff --git a/tcg/tcg.h b/tcg/tcg.h
index f7efcb4..9200a25 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -559,8 +559,9 @@ void tcg_context_init(TCGContext *s);
 void tcg_prologue_init(TCGContext *s);
 void tcg_func_start(TCGContext *s);
 
-int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf);
-int tcg_gen_code_search_pc(TCGContext *s, uint8_t *gen_code_buf, long offset);
+int tcg_gen_code(TCGContext *s, uint64_t tpc, uint8_t *gen_code_buf);
+int tcg_gen_code_search_pc(TCGContext *s, uint64_t tpc,
+                           uint8_t *gen_code_buf, long offset);
 
 void tcg_set_frame(TCGContext *s, int reg, intptr_t start, intptr_t size);
 
diff --git a/translate-all.c b/translate-all.c
index f243c10..7596b9c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -176,7 +176,7 @@ int cpu_gen_code(CPUArchState *env, TranslationBlock *tb, int *gen_code_size_ptr
     s->interm_time += profile_getclock() - ti;
     s->code_time -= profile_getclock();
 #endif
-    gen_code_size = tcg_gen_code(s, gen_code_buf);
+    gen_code_size = tcg_gen_code(s, tb->pc, gen_code_buf);
     *gen_code_size_ptr = gen_code_size;
 #ifdef CONFIG_PROFILER
     s->code_time += profile_getclock();
@@ -185,7 +185,8 @@ int cpu_gen_code(CPUArchState *env, TranslationBlock *tb, int *gen_code_size_ptr
 #endif
 
 #ifdef DEBUG_DISAS
-    if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
+    if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)
+        && qemu_log_in_addr_range(tb->pc)) {
         qemu_log("OUT: [size=%d]\n", *gen_code_size_ptr);
         log_disas(tb->tc_ptr, *gen_code_size_ptr);
         qemu_log("\n");
@@ -235,7 +236,7 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
     s->tb_jmp_offset = NULL;
     s->tb_next = tb->tb_next;
 #endif
-    j = tcg_gen_code_search_pc(s, (uint8_t *)tc_ptr, searched_pc - tc_ptr);
+    j = tcg_gen_code_search_pc(s, tb->pc, (uint8_t *)tc_ptr, searched_pc - tc_ptr);
     if (j < 0)
         return -1;
     /* now find start of instruction before */
-- 
1.9.1

Re: [Qemu-devel] [RFC PATCH 3/4] qemu-log: new option -dfilter to limit output

[Christopher Covington]

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

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Re: [Qemu-devel] [RFC PATCH 3/4] qemu-log: new option -dfilter to limit output

[Alex Bennée]

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

Re: [Qemu-devel] [RFC PATCH 1/4] qemu-log: correct help text for -d cpu

[Leandro Dorileo]

On Wed, Mar 26, 2014 at 02:37:11PM +0000, alex.bennee@linaro.org wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> This doesn't just dump CPU state on translation but on every block
> entrance.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> diff --git a/qemu-log.c b/qemu-log.c
> index 797f2af..35bbb56 100644
> --- a/qemu-log.c
> +++ b/qemu-log.c
> @@ -105,7 +105,7 @@ const QEMULogItem qemu_log_items[] = {
>      { CPU_LOG_EXEC, "exec",
>        "show trace before each executed TB (lots of logs)" },
>      { CPU_LOG_TB_CPU, "cpu",
> -      "show CPU state before block translation" },
> +      "show CPU registers before each executed TB (lots of logs)" },

what about s/lots of logs/hight volume log/ or maybe s/lots of logs/hight volume/ ?

Regards

--
Leandro Dorileo

>      { CPU_LOG_PCALL, "pcall",
>        "x86 only: show protected mode far calls/returns/exceptions" },
>      { CPU_LOG_RESET, "cpu_reset",
> -- 
> 1.9.1
> 
> 

Re: [Qemu-devel] [RFC PATCH 2/4] qemu-log: support simple pid substitution in logfile

[Leandro Dorileo]

On Wed, Mar 26, 2014 at 02:37:12PM +0000, alex.bennee@linaro.org wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> When debugging stuff that occurs over several forks it would be useful
> not to keep overwriting the one logfile you've set-up. This allows a
> simple %d to be included once in the logfile parameter which is
> substituted with getpid().
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> diff --git a/qemu-log.c b/qemu-log.c
> index 35bbb56..2897596 100644
> --- a/qemu-log.c
> +++ b/qemu-log.c
> @@ -81,11 +81,24 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
>          qemu_log_close();
>      }
>  }
> -
> +/*
> + * Allow the user to include %d in their logfile which will be
> + * substituted with the current PID. This is useful for debugging many
> + * nested linux-user tasks but will result in lots of logs.
> + */
>  void qemu_set_log_filename(const char *filename)
>  {
>      g_free(logfilename);
> -    logfilename = g_strdup(filename);
> +    if (g_strrstr(filename, "%d")) {
> +        /* if we are going to format this we'd better validate first */
> +        if (g_regex_match_simple("^[^%]+%d[^%]+$", filename, 0, 0)) {
> +            logfilename = g_strdup_printf(filename, getpid());
> +        } else {
> +            g_error("Bad logfile format: %s", filename);
> +        }
> +    } else {
> +        logfilename = g_strdup(filename);
> +    }
>      qemu_log_close();
>      qemu_set_log(qemu_loglevel);
>  }

Looks good to me.

-- 
Leandro Dorileo

Re: [Qemu-devel] [RFC PATCH 1/4] qemu-log: correct help text for -d cpu

[Markus Armbruster]

Leandro Dorileo <l@dorileo.org> writes:

> On Wed, Mar 26, 2014 at 02:37:11PM +0000, alex.bennee@linaro.org wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
>> 
>> This doesn't just dump CPU state on translation but on every block
>> entrance.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> 
>> diff --git a/qemu-log.c b/qemu-log.c
>> index 797f2af..35bbb56 100644
>> --- a/qemu-log.c
>> +++ b/qemu-log.c
>> @@ -105,7 +105,7 @@ const QEMULogItem qemu_log_items[] = {
>>      { CPU_LOG_EXEC, "exec",
>>        "show trace before each executed TB (lots of logs)" },
>>      { CPU_LOG_TB_CPU, "cpu",
>> -      "show CPU state before block translation" },
>> +      "show CPU registers before each executed TB (lots of logs)" },
>
> what about s/lots of logs/hight volume log/ or maybe s/lots of
> logs/hight volume/ ?

Alex is sticking to the precedence in CPU_LOG_EXEC's help.

Feel free to improve, just keep them consistent.

[...]

Re: [Qemu-devel] [RFC PATCH 2/4] qemu-log: support simple pid substitution in logfile

[Alex Bennée]

Leandro Dorileo <l@dorileo.org> writes:

> On Wed, Mar 26, 2014 at 02:37:12PM +0000, alex.bennee@linaro.org wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
>> 
>> When debugging stuff that occurs over several forks it would be useful
>> not to keep overwriting the one logfile you've set-up. This allows a
>> simple %d to be included once in the logfile parameter which is
>> substituted with getpid().
<snip>
>
> Looks good to me.

Is that a Reviewed-by? If you include a Reviewed-by tag it makes it
easier to copy and paste into the commit for the next iteration.

-- 
Alex Bennée

Re: [Qemu-devel] [RFC PATCH 3/4] qemu-log: new option -dfilter to limit output

[Alex Bennée]

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?

-- 
Alex Bennée

Re: [Qemu-devel] [RFC PATCH 3/4] qemu-log: new option -dfilter to limit output

[Christopher Covington]

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

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Re: [Qemu-devel] [RFC PATCH 3/4] qemu-log: new option -dfilter to limit output

[Peter Maydell]

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

Re: [Qemu-devel] [RFC PATCH 2/4] qemu-log: support simple pid substitution in logfile

[Leandro Dorileo]

On Thu, Mar 27, 2014 at 09:59:21AM +0000, Alex Bennée wrote:
> 
> Leandro Dorileo <l@dorileo.org> writes:
> 
> > On Wed, Mar 26, 2014 at 02:37:12PM +0000, alex.bennee@linaro.org wrote:
> >> From: Alex Bennée <alex.bennee@linaro.org>
> >> 
> >> When debugging stuff that occurs over several forks it would be useful
> >> not to keep overwriting the one logfile you've set-up. This allows a
> >> simple %d to be included once in the logfile parameter which is
> >> substituted with getpid().
> <snip>
> >
> > Looks good to me.
> 
> Is that a Reviewed-by? If you include a Reviewed-by tag it makes it
> easier to copy and paste into the commit for the next iteration.

Sure, you can add

Reviewed-by: Leandro Dorileo <l@dorileo.org>

-- 
Leandro Dorileo