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

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Rusty Russell wrote:
> Insist on a reason when injecting a #GP into a guest
>
> All places but one already do a printk before injecting a #GP: just
> formalize it a little.
>
> Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
>
> diff -r f75b0a5fc387 drivers/kvm/kvm_main.c
> --- a/drivers/kvm/kvm_main.c	Mon Jul 23 11:16:12 2007 +1000
> +++ b/drivers/kvm/kvm_main.c	Mon Jul 23 16:59:56 2007 +1000
> @@ -419,8 +419,15 @@ static int kvm_vm_release(struct inode *
>  	return 0;
>  }
>  
> -static void inject_gp(struct kvm_vcpu *vcpu)
> -{
> +static void __attribute__((format(printf, 2, 3)))
> +inject_gp(struct kvm_vcpu *vcpu, const char *why_fmt, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, why_fmt);
> +	printk(KERN_DEBUG "kvm: #GP ");
> +	vprintk(why_fmt, args);
> +	va_end(args);
>  	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().

>  
> @@ -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?

>  		return;
>  	}
>  
>  	if ((cr0 & X86_CR0_NW) && !(cr0 & X86_CR0_CD)) {
> -		printk(KERN_DEBUG "set_cr0: #GP, CD == 0 && NW == 1\n");
> -		inject_gp(vcpu);
> +		inject_gp(vcpu,
> +			  "set_cr0: CD == 0 && NW == 1\n");
>  		return;
>   

Are you deliberately ignoring an opportunity to reduce our line count?!

> @@ -1843,8 +1834,7 @@ int kvm_setup_pio(struct kvm_vcpu *vcpu,
>  		/*
>  		 * String I/O in reverse.  Yuck.  Kill the guest, fix later.
>  		 */
> -		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().

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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/