Re: [Qemu-devel] [PATCH v2] qemu-log: add log category for MMU info

[Peter Maydell]

On 10 October 2014 11:59, Antony Pavlov <address@hidden> wrote:
> Running barebox on qemu-system-mips* with '-d unimp' overloads
> stderr by very very many mips_cpu_handle_mmu_fault() messages:
>
>   mips_cpu_handle_mmu_fault address=b80003fd ret 0 physical 00000000180003fd 
> prot 3
>   mips_cpu_handle_mmu_fault address=a0800884 ret 0 physical 0000000000800884 
> prot 3
>   mips_cpu_handle_mmu_fault pc a080cd80 ad b80003fd rw 0 mmu_idx 0
>
> So it's very difficult to find LOG_UNIMP message.
>
> The mips_cpu_handle_mmu_fault() messages appears on enabling ANY
> logging! It's not very handy.
>
> Adding separate log category for *_cpu_handle_mmu_fault()
> logging fixes the problem.
>
> Signed-off-by: Antony Pavlov <address@hidden>

This mostly looks good, thanks!

I should also note that I'm happy for us to just implement
the common-code (ie the log flag) and fix those targets
which are particularly obnoxious about logging and/or
easy to fix. We can always leave the other targets to
update their code later (or you could update other targets
in separate patches once the main one has gone in).

A minor tweak:

> --- a/target-cris/helper.c
> +++ b/target-cris/helper.c
> @@ -30,9 +30,11 @@
>  #ifdef CRIS_HELPER_DEBUG
>  #define D(x) x
>  #define D_LOG(...) qemu_log(__VA_ARGS__)
> +#define LOG_MMU(...) qemu_log_mask(CPU_LOG_MMU, __VA_ARGS__)
>  #else
>  #define D(x)
>  #define D_LOG(...) do { } while (0)
> +#define LOG_MMU(...) do { } while (0)
>  #endif

Now this logging is configurably enablable at runtime,
we should just call qemu_log_mask() directly, rather
than wrapping it in a LOG_MMU macro which might be compiled
out.

> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -27,6 +27,12 @@
>
>  //#define DEBUG_MMU
>
> +#ifdef DEBUG_MMU
> +#  define LOG_MMU(...) qemu_log_mask(CPU_LOG_MMU, __VA_ARGS__)
> +#else
> +#  define LOG_MMU(...) do { } while (0)
> +#endif

Similarly here.

> --- a/target-microblaze/helper.c
> +++ b/target-microblaze/helper.c
> @@ -22,7 +22,14 @@
>  #include "qemu/host-utils.h"
>
>  #define D(x)
> -#define DMMU(x)
> +
> +#undef DEBUG_MMU
> +
> +#ifdef DEBUG_MMU
> +#  define LOG_MMU(...) qemu_log_mask(CPU_LOG_MMU, __VA_ARGS__)
> +#else
> +#  define LOG_MMU(...) do { } while (0)
> +#endif

and here.

> --- a/target-ppc/mmu-hash32.c
> +++ b/target-ppc/mmu-hash32.c

There are other PPC files which also do MMU logging;
we should either fix them all or none of them.

thanks
-- PMM