From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: svm: save_svm_cpu_user_regs vs. svm_store_cpu_guest_regs Date: Wed, 09 May 2007 10:11:12 +0200 Message-ID: <46419E40.76E4.0078.0@novell.com> References: <4641938C.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org >>> Keir Fraser 09.05.07 09:51 >>> >On 9/5/07 08:25, "Jan Beulich" wrote: > >> What is the reason for save_svm_cpu_user_regs() explicitly saving rax? >> Without this, the function could be eliminated, with its single use = replaced >> by a call to svm_store_cpu_guest_regs(). > >It would be great if we can get away with just moving save/restore of rAX >into svm_{load,store}_cpu_guest_regs(), kill save_svm_cpu_user_regs() >completely, and get rid of the call at the top of the vmexit handler. >There's no equivalent call at the top of the VMX vmexit handler: all the >common HVM code will explicitly svm_store_cpu_guest_regs() before = depending >on GPR state. It's not really GPR state that matters here (GPRs are saved in the = respective exit.S files), which is why I wondered about the need for saving RAX. The = point here is that CS:RIP, RFLAGS, and SS:RSP may need to be stored, and the = fact that VMX doesn't do so doesn't mean it can be freely removed from SVM = code: If I'm seeing things right (I didn't check this on hardware, yet), hvm = hypercalls are currently having a security hole on VMX in that ring 3 code can = possibly invoke them and/or prevent ring 0 from invoking them - VMX code doesn't = seem to save the CS selector anywhere, but the ring_3() test depends on that = (so generally appears to operate on stale data, i.e. whatever was saved on the stack the last time vmx_store_cpu_guest_regs() was executed. If I wasn't buried in hunting bugs for SLE10 SP1, I would have confirmed this already = and sent a patch if necessary (along with quite a few more ones, more or less = all addressing 32on64/hvm weakness)... Jan