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

[Alexander Graf <agraf@suse.de>]

On 10.10.14 16:47, Peter Maydell wrote:
> On 10 October 2014 11:59, Antony Pavlov <antonynpavlov@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 <antonynpavlov@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