From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56350) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XdgzJ-0004zy-Qk for qemu-devel@nongnu.org; Mon, 13 Oct 2014 10:52:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XdgzC-0007wa-51 for qemu-devel@nongnu.org; Mon, 13 Oct 2014 10:52:41 -0400 Received: from cantor2.suse.de ([195.135.220.15]:38662 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XdgzB-0007wP-Uc for qemu-devel@nongnu.org; Mon, 13 Oct 2014 10:52:34 -0400 Message-ID: <543BE6C2.7060901@suse.de> Date: Mon, 13 Oct 2014 16:50:42 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1412938761-24283-1-git-send-email-antonynpavlov@gmail.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] qemu-log: add log category for MMU info List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , Antony Pavlov Cc: QEMU Developers , Blue Swirl , "Edgar E. Iglesias" , Guan Xuetao , Aurelien Jarno , Richard Henderson On 10.10.14 16:47, Peter Maydell wrote: > On 10 October 2014 11:59, Antony Pavlov 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 > > 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