From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@qumranet.com>
Subject: Re: [PATCH 8/9] Add VMRUN handler
Date: Mon, 01 Sep 2008 16:41:43 +0300
Message-ID: <48BBF117.30208@qumranet.com>
References: <1220270281-15720-1-git-send-email-agraf@suse.de> <1220270281-15720-2-git-send-email-agraf@suse.de> <1220270281-15720-3-git-send-email-agraf@suse.de> <1220270281-15720-4-git-send-email-agraf@suse.de> <1220270281-15720-5-git-send-email-agraf@suse.de> <1220270281-15720-6-git-send-email-agraf@suse.de> <1220270281-15720-7-git-send-email-agraf@suse.de> <1220270281-15720-8-git-send-email-agraf@suse.de> <1220270281-15720-9-git-send-email-agraf@suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: KVM list <kvm@vger.kernel.org>, Joerg Roedel <joro@8bytes.org>,
	Anthony Liguori <anthony@codemonkey.ws>
To: Alexander Graf <agraf@suse.de>
Return-path: <kvm-owner@vger.kernel.org>
Received: from il.qumranet.com ([212.179.150.194]:24786 "EHLO il.qumranet.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751303AbYIANlp (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 1 Sep 2008 09:41:45 -0400
In-Reply-To: <1220270281-15720-9-git-send-email-agraf@suse.de>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Alexander Graf wrote:
> This patch implements VMRUN. VMRUN enters a virtual CPU and runs that
> in the same context as the normal guest CPU would run.
> So basically it is implemented the same way, a normal CPU would do it.
>
> We also prepare all intercepts that get OR'ed with the original
> intercepts, as we do not allow a level 2 guest to be intercepted less
> than the first level guest.
>
>  
>  
> +/* Not needed until device passthrough */
> +/* #define NESTED_KVM_MERGE_IOPM */
> +
>   

I'd like to drop port 80 passthrough anyway.  Device assignment is 
unlikely to make heavy use of ioports.

> @@ -663,9 +674,21 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  	msrpm_pages = alloc_pages(GFP_KERNEL, MSRPM_ALLOC_ORDER);
>  	if (!msrpm_pages)
>  		goto uninit;
> +
> +	nested_msrpm_pages = alloc_pages(GFP_KERNEL, MSRPM_ALLOC_ORDER);
> +	if (!nested_msrpm_pages)
> +		goto uninit;
> +
> +	nested_iopm_pages = alloc_pages(GFP_KERNEL, IOPM_ALLOC_ORDER);
> +	if (!nested_iopm_pages)
> +		goto uninit;
> +
>   

Maybe we should do that on the first time the guest enters nested svm, 
to save a bit of memory.

We can do that in a later patch, though.

>  
> +
> +static int nested_svm_vmrun_msrpm(struct vcpu_svm *svm, void *arg1,
> +				  void *arg2, void *opaque)
> +{
> +	int i;
> +	u32 *nested_msrpm = (u32*)arg1;
> +	for (i=0; i< PAGE_SIZE * (1 << MSRPM_ALLOC_ORDER) / 4; i++)
> +		svm->nested_msrpm[i] = svm->msrpm[i] | nested_msrpm[i];
> +	svm->vmcb->control.msrpm_base_pa = __pa(svm->nested_msrpm);
> +
> +	return 0;
> +}
>   

Hm.  Have you verified that kvm actually has msr emulation for all the 
msrs it allows through msrpm?

I guess it has to, since the msrs can be set through save/restore.


(vmrun emulation)
> +
> +	force_new_asid(&svm->vcpu);
>   

I would be nice not to do this (can be left for later of course; it 
could be quite complex).

> +
> +static int vmrun_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
> +{
> +	nsvm_printk("VMrun\n");
> +
> +	svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
> +	skip_emulated_instruction(&svm->vcpu);
> +
> +	if (svm->vmcb->save.cpl) {
> +		printk(KERN_ERR "%s: invalid cpl 0x%x at ip 0x%lx\n",
> +		       __func__, svm->vmcb->save.cpl, kvm_rip_read(&svm->vcpu));
> +		kvm_queue_exception(&svm->vcpu, GP_VECTOR);
> +		return 1;
> +	}
>   

Skip after check.

I think you also need special treatment for the guest's eflags.if.  If 
interrupts are enabled for the guest when vmrun is executed, and kvm 
tries to inject a virtual interrupt, then it should result in a virtual 
#VMEXIT.

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