Re: [PATCH 1/2] kvm: Insist on a reason when injecting a #GP into a guest

[Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>]

On Tue, 2007-07-24 at 13:15 +0300, Avi Kivity wrote:
> Rusty Russell wrote:
> > +static void __attribute__((format(printf, 2, 3)))
> > +inject_gp(struct kvm_vcpu *vcpu, const char *why_fmt, ...)
...
> >  	kvm_arch_ops->inject_gp(vcpu, 0);
> >  }
> >   
> 
> Some guests (non hardware accelerated hypervisors, mostly) will generate 
> #gp.  We'll want to inject exceptions without noise then.  I don't think 
> we have such an exception today, but it's an excuse to have a clean 
> __inject_gp() and a varargs-infested inject_gp().

I agree, except I never saw the point of a this wrapper anyway.  Is
there a good reason not to call kvm_arch_ops->inject_gp directly?

> > @@ -464,22 +471,21 @@ void set_cr0(struct kvm_vcpu *vcpu, unsi
> >  void set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
> >  {
> >  	if (cr0 & CR0_RESERVED_BITS) {
> > -		printk(KERN_DEBUG "set_cr0: 0x%lx #GP, reserved bits 0x%lx\n",
> > -		       cr0, vcpu->cr0);
> > -		inject_gp(vcpu);
> > +		inject_gp(vcpu,
> > +			  "set_cr0: 0x%lx reserved bits 0x%lx\n",
> > +			  cr0, vcpu->cr0);
> >   
> 
> Why three lines?

Erk, I deleted the #GP from these messages later.  Will fix.

> > -		printk(KERN_ERR "kvm: guest string pio down\n");
> > -		inject_gp(vcpu);
> > +		inject_gp(vcpu, "guest string pio down\n");
> >  		return 1;
> >  	}
> >  	vcpu->run->io.count = now;
> 
> This one really wants to be a KERN_ERR.  It's not a guest exception, 
> it's a kvm unimplemented feature for which the guest pays the price.  I 
> guess a good place for __inject_gp().

Good point, by itself this patch is bad.  See 2/2.

Thanks,
Rusty.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/