[Qemu-devel] Re: [PATCH, RFC] Sparc: convert some debug printf statements to tracepoints

[Blue Swirl]

On Sat, Oct 9, 2010 at 3:10 PM, Blue Swirl <address@hidden> wrote:
> Replace some debug printf statements with tracepoints.
>
> Signed-off-by: Blue Swirl <address@hidden>
> ---
> I think tracing way is more flexible than current conditional code. I
> remember wading through hundreds of megs of DPRINTF output looking for
> a clue about a specific event, so being able to control the trace
> dynamically is invaluable.
>
> Here's example simpletrace.py output:
> sparc64_translate 0.000 address=0x1fff0000040 paddr=0x1fff0000040
> vaddr=0x1fff0000000 mmu_idx=0x4 primary_context=0x0
> secondary_context=0x0
> sparc64_translate 24.017 address=0x1fff000bc50 paddr=0x1fff000bc50
> vaddr=0x1fff000a000 mmu_idx=0x4 primary_context=0x0
> secondary_context=0x0
> sparc64_translate 3.921 address=0x1fff000bc5c paddr=0x1fff000bc5c
> vaddr=0x1fff000a000 mmu_idx=0x2 primary_context=0x0
> secondary_context=0x0
> sparc64_translate 337.702 address=0x1fff000bf38 paddr=0x1fff000bf38
> vaddr=0x1fff000a000 mmu_idx=0x2 primary_context=0x0
> secondary_context=0x0
>
> There are problems #including "trace.h" from op_helper.c
> (qemu-common.h conflicts with dyngen-exec.h), otherwise this looks
> promising.
> ---
>  target-sparc/helper.c |   65 +++++++++++-------------------------------------
>  trace-events          |   10 +++++++
>  2 files changed, 25 insertions(+), 50 deletions(-)
>
> diff --git a/target-sparc/helper.c b/target-sparc/helper.c
> index aa1fd63..8078d92 100644
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -26,17 +26,11 @@
>  #include "cpu.h"
>  #include "exec-all.h"
>  #include "qemu-common.h"
> +#include "trace.h"
>
>  //#define DEBUG_MMU
>  //#define DEBUG_FEATURES
>
> -#ifdef DEBUG_MMU
> -#define DPRINTF_MMU(fmt, ...) \
> -    do { printf("MMU: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF_MMU(fmt, ...) do {} while (0)
> -#endif
> -
>  static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char 
> *cpu_model);
>
>  /* Sparc MMU emulation */
> @@ -240,10 +234,7 @@ int cpu_sparc_handle_mmu_fault (CPUState *env,
> target_ulong address, int rw,
>     if (error_code == 0) {
>         vaddr = address & TARGET_PAGE_MASK;
>         paddr &= TARGET_PAGE_MASK;
> -#ifdef DEBUG_MMU
> -        printf("Translate at " TARGET_FMT_lx " -> " TARGET_FMT_plx ", vaddr "
> -               TARGET_FMT_lx "\n", address, paddr, vaddr);
> -#endif
> +        trace_sparc32_translate(address, paddr, vaddr, mmu_idx);
>         tlb_set_page(env, vaddr, paddr, prot, mmu_idx, page_size);
>         return 0;
>     }
> @@ -466,16 +457,10 @@ static int get_physical_address_data(CPUState *env,
>             if ((env->dtlb[i].tte & 0x4) && is_user) {
>                 fault_type |= 1; /* privilege violation */
>                 env->exception_index = TT_DFAULT;
> -
> -                DPRINTF_MMU("DFAULT at %" PRIx64 " context %" PRIx64
> -                            " mmu_idx=%d tl=%d\n",
> -                            address, context, mmu_idx, env->tl);
> +                trace_sparc64_dfault(address, context, mmu_idx, env->tl);
>             } else if (!(env->dtlb[i].tte & 0x2) && (rw == 1)) {
>                 env->exception_index = TT_DPROT;
> -
> -                DPRINTF_MMU("DPROT at %" PRIx64 " context %" PRIx64
> -                            " mmu_idx=%d tl=%d\n",
> -                            address, context, mmu_idx, env->tl);
> +                trace_sparc64_dprot(address, context, mmu_idx, env->tl);
>             } else {
>                 *prot = PAGE_READ;
>                 if (env->dtlb[i].tte & 0x2)
> @@ -502,9 +487,7 @@ static int get_physical_address_data(CPUState *env,
>         }
>     }
>
> -    DPRINTF_MMU("DMISS at %" PRIx64 " context %" PRIx64 "\n",
> -                address, context);
> -
> +    trace_sparc64_dmiss(address, context);
>     env->dmmu.tag_access = (address & ~0x1fffULL) | context;
>     env->exception_index = TT_DMISS;
>     return 1;
> @@ -549,9 +532,7 @@ static int get_physical_address_code(CPUState *env,
>
>                 env->immu.tag_access = (address & ~0x1fffULL) | context;
>
> -                DPRINTF_MMU("TFAULT at %" PRIx64 " context %" PRIx64 "\n",
> -                            address, context);
> -
> +                trace_sparc64_tfault(address, context);
>                 return 1;
>             }
>             *prot = PAGE_EXEC;
> @@ -560,9 +541,7 @@ static int get_physical_address_code(CPUState *env,
>         }
>     }
>
> -    DPRINTF_MMU("TMISS at %" PRIx64 " context %" PRIx64 "\n",
> -                address, context);
> -
> +    trace_sparc64_tmiss(address, context);
>     /* Context is stored in DMMU (dmmuregs[1]) also for IMMU */
>     env->immu.tag_access = (address & ~0x1fffULL) | context;
>     env->exception_index = TT_TMISS;
> @@ -578,21 +557,14 @@ static int get_physical_address(CPUState *env,
> target_phys_addr_t *physical,
>        everything when an entry is evicted.  */
>     *page_size = TARGET_PAGE_SIZE;
>
> -#if defined (DEBUG_MMU)
>     /* safety net to catch wrong softmmu index use from dynamic code */
>     if (env->tl > 0 && mmu_idx != MMU_NUCLEUS_IDX) {
> -        DPRINTF_MMU("get_physical_address %s tl=%d mmu_idx=%d"
> -                    " primary context=%" PRIx64
> -                    " secondary context=%" PRIx64
> -                " address=%" PRIx64
> -                "\n",
> -                (rw == 2 ? "CODE" : "DATA"),
> -                env->tl, mmu_idx,
> -                env->dmmu.mmu_primary_context,
> -                env->dmmu.mmu_secondary_context,
> -                address);
> +        trace_sparc64_get_physical_address((rw == 2 ? "CODE" : "DATA"),

Passing a pointer obviously won't work, the string will not be
accessible when processing the trace data.