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

[Andreas Färber]

Am 09.10.2010 um 17:10 schrieb Blue Swirl:

> 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.

While this may in theory be right, in practice the nop backend gives  
us nothing and the simple trace backend still has porting issues that  
need to be fixed.
I remember hacking around some time-related problems on Haiku (it was  
using an optional POSIX feature without check/wrapper), and there was  
a comment saying that similar changes need to be done for Win32  
support too.

Has anyone looked into a DTrace backend btw?

Andreas

> 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"),
> +                                           env->tl, mmu_idx,
> +                                           env- 
> >dmmu.mmu_primary_context,
> +                                           env- 
> >dmmu.mmu_secondary_context,
> +                                           address);
>     }
> -#endif
>
>     if (rw == 2)
>         return get_physical_address_code(env, physical, prot, address,
> @@ -618,16 +590,9 @@ int cpu_sparc_handle_mmu_fault (CPUState *env,
> target_ulong address, int rw,
>         vaddr = virt_addr + ((address & TARGET_PAGE_MASK) &
>                              (TARGET_PAGE_SIZE - 1));
>
> -        DPRINTF_MMU("Translate at %" PRIx64 " -> %" PRIx64 ","
> -                    " vaddr %" PRIx64
> -                    " mmu_idx=%d"
> -                    " tl=%d"
> -                    " primary context=%" PRIx64
> -                    " secondary context=%" PRIx64
> -                    "\n",
> -                    address, paddr, vaddr, mmu_idx, env->tl,
> -                    env->dmmu.mmu_primary_context,
> -                    env->dmmu.mmu_secondary_context);
> +        trace_sparc64_translate(address, paddr, vaddr, mmu_idx,
> +                                env->dmmu.mmu_primary_context,
> +                                env->dmmu.mmu_secondary_context);
>
>         tlb_set_page(env, vaddr, paddr, prot, mmu_idx, page_size);
>         return 0;
> diff --git a/trace-events b/trace-events
> index 4300178..da4c208 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -69,3 +69,13 @@ disable cpu_out(unsigned int addr, unsigned int
> val) "addr %#x value %u"
> # balloon.c
> # Since requests are raised via monitor, not many tracepoints are  
> needed.
> disable balloon_event(void *opaque, unsigned long addr) "opaque %p  
> addr %lu"
> +
> +# target-sparc/helper.c
> +disable sparc64_dfault(uint64_t address, uint64_t context, int
> mmu_idx, uint32_t tl) "DFAULT at %" PRIx64 " context %" PRIx64
> "mmu_idx=%d tl=%d"
> +disable sparc64_dprot(uint64_t address, uint64_t context, int
> mmu_idx, uint32_t tl) "DFAULT at %" PRIx64 " context %" PRIx64
> "mmu_idx=%d tl=%d"
> +disable sparc64_dmiss(uint64_t address, uint64_t context) "DMISS at
> %" PRIx64 " context %" PRIx64
> +disable sparc64_tfault(uint64_t address, uint64_t context) "TFAULT at
> %" PRIx64 " context %" PRIx64
> +disable sparc64_tmiss(uint64_t address, uint64_t context) "TMISS at
> %" PRIx64 " context %" PRIx64
> +disable sparc64_get_physical_address(const char *type, uint32_t tl,
> int mmu_idx, uint64_t primary_context, uint64_t secondary_context,
> uint64_t address) "%s tl=%d mmu_idx=%d primary context=%" PRIx64 "
> secondary context=%" PRIx64 " address=%" PRIx64
> +disable sparc64_translate(uint64_t address, uint64_t paddr, uint64_t
> vaddr, int mmu_idx, uint64_t primary_context, uint64_t
> secondary_context) "Translate at %" PRIx64 " -> %" PRIx64 ", vaddr %"
> PRIx64 " mmu_idx=%d primary context=%" PRIx64 " secondary context=%"
> PRIx64
> +disable sparc32_translate(uint32_t address, uint64_t paddr, uint32_t
> vaddr, int mmu_idx) "Translate at 0x%x -> %" PRIx64 ", vaddr 0x%x
> mmu_idx=%d"
> --
> 1.6.2.4