From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 07/29] nVMX: Hold a vmcs02 for each vmcs12 Date: Mon, 31 Jan 2011 11:41:19 +0200 Message-ID: <4D4683BF.8050702@redhat.com> References: <1296116987-nyh@il.ibm.com> <201101270833.p0R8XQ4w002480@rice.haifa.ibm.com> <4D45372E.2050605@redhat.com> <20110131092629.GB23022@fermat.math.technion.ac.il> 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" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:62197 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755310Ab1AaJl1 (ORCPT ); Mon, 31 Jan 2011 04:41:27 -0500 In-Reply-To: <20110131092629.GB23022@fermat.math.technion.ac.il> Sender: kvm-owner@vger.kernel.org List-ID: On 01/31/2011 11:26 AM, Nadav Har'El wrote: > Hi, > > On Sun, Jan 30, 2011, Avi Kivity wrote about "Re: [PATCH 07/29] nVMX: Hold a vmcs02 for each vmcs12": > > >+/* > > >+ * 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. > > Of course. What I said was that *unused* vmcs12s (in the sense that they > don't describe any active guest) will normally be unloaded (VMCLEARed) by L1 > and so will not take up space. Only VMCSs actually being used to run guests > will take up space. If you have N running guests, then right, you'll have > N VMCSs. I put the limit at 256, which on one hand allows L1 to run 256 L2s > (which I think is well above what people will normally run on one CPU), and > on the other hand limits the amount of damage that a malicious L1 can do: > At worst it can cause the host to allocate (and pin) 256 extra VMCSs, which > sum up to 1 MB. > > I thought that this compromise was good enough, and you didn't, and although > I still don't understand why, I promise I will change it. I'll make this > change my top priority now. VM crashes on legal guest behaviour are bad; and since it's easy to avoid, why do it? > > >+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. > > This is what I thought too... I call this function on the saved_vmcs->cpu > cpu, so there's no reason why it would find itself being called on a different > cpu. > > The only reason I added this sanity check was that __vcpu_clear has the same > one, and there too it seemed redundant, and I thought maybe I was missing > something. Do you know why __vcpu_clear needs this test? __vcpu_clear() can race with itself (called from the cpu migration path vs. cpu offline path) -- error compiling committee.c: too many arguments to function