Re: [PATCH 2/4] KVM: PPC: Only WARN on invalid emulation

[Scott Wood <scottwood@freescale.com>]

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 <agraf@suse.de>
> >> ---
> >> 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