From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Huang, Kai" Subject: Re: [intel-sgx-kernel-dev] [PATCH 08/10] kvm: vmx: add guest's IA32_SGXLEPUBKEYHASHn runtime switch support Date: Thu, 18 May 2017 20:14:24 +1200 Message-ID: References: <20170508052434.3627-1-kai.huang@linux.intel.com> <20170508052434.3627-9-kai.huang@linux.intel.com> <58dcdb2d-6894-b0a3-8d6f-2ab752fd6d22@linux.intel.com> <20170515124622.piupyk57vjdoppl5@intel.com> <478d9303-00b7-4f29-6124-0c1433851952@linux.intel.com> <1495030889.23465.13.camel@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Paolo Bonzini , haim.cohen@intel.com, "intel-sgx-kernel-dev@lists.01.org" , kvm list , Radim Krcmar To: Sean Christopherson , Jarkko Sakkinen , Andy Lutomirski Return-path: Received: from mga05.intel.com ([192.55.52.43]:39204 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752741AbdERIOb (ORCPT ); Thu, 18 May 2017 04:14:31 -0400 In-Reply-To: <1495030889.23465.13.camel@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 5/18/2017 2:21 AM, Sean Christopherson wrote: > On Tue, 2017-05-16 at 11:56 +1200, Huang, Kai wrote: >> >> On 5/16/2017 12:46 AM, Jarkko Sakkinen wrote: >>> >>> On Thu, May 11, 2017 at 08:28:37PM -0700, Andy Lutomirski wrote: >>>> >>>> [resending due to some kind of kernel.org glitch -- sorry if anyone >>>> gets duplicates] >>>> >>>> On Thu, May 11, 2017 at 5:32 PM, Huang, Kai >>>> wrote: >>>>> >>>>> My current patch is based on this assumption. For KVM guest, naturally, >>>>> we >>>>> will write the cached value to real MSRs when vcpu is scheduled in. For >>>>> host, SGX driver should write its own value to MSRs when it performs >>>>> EINIT >>>>> for LE. >>>> This seems unnecessarily slow (perhaps *extremely* slow) to me. I >>>> would propose a totally different solution: >>>> >>>> Have a percpu variable that stores the current SGXLEPUBKEYHASH along >>>> with whatever lock is needed (probably just a mutex). Users of EINIT >>>> will take the mutex, compare the percpu variable to the desired value, >>>> and, if it's different, do WRMSR and update the percpu variable. >>> This is exactly what I've been suggesting internally: trap EINIT and >>> check the value and write conditionally. >>> >>> I think this would be the best starting point. >> OK. Assuming we are going to have this percpu variable for >> IA32_SGXLEPUBKEYHASHn, I suppose KVM also will update guest's value to >> this percpu variable after KVM writes guest's value to hardware MSR? And >> host (SGX driver) need to do the same thing (check the value and write >> conditionally), correct? >> >> Thanks, >> -Kai > > Yes, the percpu variable is simply a cache so that the kernel doesn't have to do > four RDMSRs every time it wants to do EINIT. KVM would still maintain shadow > copies of the MSRs for each vcpu for emulating RDMSR, WRMSR and EINIT. I don't > think KVM would even need to be aware of the percpu variable, i.e. the entire > lock->(rd/wr)msr->EINIT->unlock sequence can probably be encapsulated in a > single function that is called from both the primary SGX driver and from KVM. > You are making assumption that KVM will run ENCLS on behalf of guest. :) If we don't need to look into guest's SIGSTRUCT, EINITTOKEN, etc, then I actually prefer to using MTF, as with MTF we don't have to do all the remapping guest's virtual address to KVM's virtual address thing, if we don't need to look into guest's ENCLS parameter. But if we need to look into guest's ENCLS parameters, for example, to locate physical SECS page, or to update physical EPC page's info (that KVM needs to maintain), maybe we can choose running ENCLS on behalf of guest. But if we are going to run ENCLS on behalf of guest, I think providing a single function which does write MSR and EINIT for KVM should be a good idea. Thanks, -Kai