From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper Date: Wed, 10 Jun 2009 18:24:10 +0300 Message-ID: <4A2FD01A.5090208@redhat.com> References: <20090609213009.436123773@amt.cnet> <20090609213312.838419569@amt.cnet> <4A2F7A15.2080302@redhat.com> <20090610121448.GB6672@amt.cnet> <4A2FA5D3.9020604@redhat.com> <20090610131731.GG6672@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, sheng.yang@intel.com To: Marcelo Tosatti Return-path: Received: from mx2.redhat.com ([66.187.237.31]:35236 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750978AbZFJPYL (ORCPT ); Wed, 10 Jun 2009 11:24:11 -0400 In-Reply-To: <20090610131731.GG6672@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: >> Ah, you're right. >> >> I don't think it's worthwhile to add all this just for debugging. You >> can add a function that dumps the spte chain as well as the features >> MSR, and we can decode it by hand when we see it. Disadvantage is more >> work for us when we hit the bug, but at least that function is reusable >> in other contexts. >> > > The problem is if someone hits the bug who has no possibility of > reporting the printks. Nice thing about the WARN_ON's there is you can > look up kerneloops.org, match the line number to the kernel version, > and narrow down what bits are wrong (you still need reporter to send > contents of dmesg for full spte value). > Well, we can KERN_WARNING instead of KERN_DEBUG in that printout. > Also the bit-by-bit validity checks (the inspect_spte function in vmx.c) > can be used in the mmu audit code (thats the reason for print=0 in the > callback parameters), so it is reusable in other contexes. > > What you dislike is hardcoding the bits checked in C code? Don't worry > about the level stuff, will be handled next. > I just don't want to introduce yet another function-callback pair for such a small thing. No objection to the code itself. Printing out the spte hierarchy seems a good idea in other bug contexts, so at least that function is reusable. -- error compiling committee.c: too many arguments to function