From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 07/29] nVMX: Hold a vmcs02 for each vmcs12
Date: Sun, 30 Jan 2011 12:02:22 +0200
Message-ID: <4D45372E.2050605@redhat.com>
References: <1296116987-nyh@il.ibm.com> <201101270833.p0R8XQ4w002480@rice.haifa.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, gleb@redhat.com
To: "Nadav Har'El" <nyh@il.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:30621 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751286Ab1A3KC2 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 30 Jan 2011 05:02:28 -0500
In-Reply-To: <201101270833.p0R8XQ4w002480@rice.haifa.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 01/27/2011 10:33 AM, Nadav Har'El wrote:
> In this patch we add a list of L0 (hardware) VMCSs, which we'll use to hold a
> hardware VMCS for each active vmcs12 (i.e., for each L2 guest).
>
> We call each of these L0 VMCSs a "vmcs02", as it is the VMCS that L0 uses
> to run its nested guest L2.
>
>
> +/*
> + * Allocate an L0 VMCS (vmcs02) for the current L1 VMCS (vmcs12), if one
> + * does not already exist. The allocation is done in L0 memory, so to avoid
> + * denial-of-service attack by guests, we limit the number of concurrently-
> + * allocated vmcss. A well-behaving L1 will VMCLEAR unused vmcs12s and not
> + * trigger this limit.

No, it won't.  If you run N guests on a single-cpu kvm host, you'll have 
N active VMCSs.

> + */
> +static int nested_create_current_vmcs(struct kvm_vcpu *vcpu)
> +{
> +	struct vmcs_list *new_l2_guest;
> +	struct vmcs *vmcs02;
> +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> +
> +	if (nested_get_current_vmcs(vmx))
> +		return 0; /* nothing to do - we already have a VMCS */
> +
> +	if (vmx->nested.vmcs02_num>= NESTED_MAX_VMCS)
> +		return -ENOMEM;

I asked to replace this by dropping the entire vmcs02_list (or perhaps 
just its tail).

> +static void __nested_free_saved_vmcs(void *arg)
> +{
> +	struct saved_vmcs *saved_vmcs = arg;
> +	int cpu = raw_smp_processor_id();
> +
> +	if (saved_vmcs->cpu == cpu) /* TODO: how can this not be the case? */
> +		vmcs_clear(saved_vmcs->vmcs);

This check will always be true.

> +	if (per_cpu(current_vmcs, cpu) == saved_vmcs->vmcs)
> +		per_cpu(current_vmcs, cpu) = NULL;

And this will always be false, no?  Unless you free a vmcs02 while you 
use it?  Don't you always switch back to vmcs01 prior to freeing?

> +}
> +

>   	skip_emulated_instruction(vcpu);
> @@ -4050,6 +4190,8 @@ static void free_nested(struct vcpu_vmx
>   		nested_release_page(vmx->nested.current_vmcs12_page);
>   		vmx->nested.current_vmptr = -1ull;
>   	}
> +
> +	nested_free_all_vmcs(vmx);
>   }

Maybe this is the counterexample - we kill a vcpu while it is in nested 
mode.

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