From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 2/4] KVM: PPC: Only WARN on invalid emulation Date: Tue, 18 Dec 2012 17:05:58 -0600 Message-ID: <1355871958.5138.4@snotra> References: <1355871255.5138.2@snotra> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: , KVM list , Marcelo Tosatti , Gleb Natapov To: Alexander Graf Return-path: In-Reply-To: (from agraf@suse.de on Tue Dec 18 17:01:19 2012) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 12/18/2012 05:01:19 PM, Alexander Graf wrote: > > On 18.12.2012, at 23:54, Scott Wood wrote: > > > On 12/18/2012 06:38:41 AM, Alexander Graf wrote: > >> When we hit an emulation result that we didn't expect, that is an > error, > >> but it's nothing that warrants a BUG(), because it can be guest > triggered. > >> So instead, let's only WARN() the user that this happened. > >> Signed-off-by: Alexander Graf > >> --- > >> arch/powerpc/kvm/powerpc.c | 3 ++- > >> 1 files changed, 2 insertions(+), 1 deletions(-) > >> diff --git a/arch/powerpc/kvm/powerpc.c > b/arch/powerpc/kvm/powerpc.c > >> index be83fca..e2225e5 100644 > >> --- a/arch/powerpc/kvm/powerpc.c > >> +++ b/arch/powerpc/kvm/powerpc.c > >> @@ -237,7 +237,8 @@ int kvmppc_emulate_mmio(struct kvm_run *run, > struct kvm_vcpu *vcpu) > >> r = RESUME_HOST; > >> break; > >> default: > >> - BUG(); > >> + WARN_ON(1); > >> + r = RESUME_GUEST; > > > > Do you have a specific way of a guest triggering this in mind, or > is it just being cautious? The guest probably shouldn't be allowed > to spam the kernel log with WARNs either. Is a traceback even useful > here? > > For debugging, yes. I figured the interesting bits would be in the stack frames you've just returned from. > But maybe we would be better off with a trace point. Or a ratelimited error message, and maybe we should return to host instead of guest? > Anyway, a WARN is better than a BUG either way for now. Yes... > I was able to provoke this by live patching an instruction without > flushing the icache, so that the last_inst instruction fetch gets a > different instruction from the instruction that resulted in the trap > we're currently in. Which EMULATE code did you get in that case? -Scott