Re: [PATCH 07/29] nVMX: Hold a vmcs02 for each vmcs12

[Avi Kivity <avi@redhat.com>]

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