On 10.10.14 16:47, Peter Maydell wrote:
> On 10 October 2014 11:59, Antony Pavlov <antonynpav...@gmail.com> 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 <antonynpav...@gmail.com>
>
> 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.
The MMU lookups are in a pretty hot path. Are you sure we always want to
have the log enabled checks and register pollution in there?
Alex
