From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1A46C43381 for ; Wed, 10 Feb 2021 23:16:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 81D3A64EBB for ; Wed, 10 Feb 2021 23:16:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232565AbhBJXQF (ORCPT ); Wed, 10 Feb 2021 18:16:05 -0500 Received: from mga12.intel.com ([192.55.52.136]:59962 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234366AbhBJXNH (ORCPT ); Wed, 10 Feb 2021 18:13:07 -0500 IronPort-SDR: ose5+5MLuk1UhGX86n0jV4riMQ5sx4imqVPq4cgqumwdL13cQxrioYQLmmvFF2MfO2GOntrDXv qjNbbXIxp7UA== X-IronPort-AV: E=McAfee;i="6000,8403,9891"; a="161312464" X-IronPort-AV: E=Sophos;i="5.81,169,1610438400"; d="scan'208";a="161312464" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Feb 2021 15:12:22 -0800 IronPort-SDR: 2Nmz5lwLG+yiGinCih9E8NYBbCP534jpDXDyZJwKG6K0zPob5ypBU4Ph1vsV2wyhGF7WDTuDJX AOz7XvnWjmwA== X-IronPort-AV: E=Sophos;i="5.81,169,1610438400"; d="scan'208";a="489344248" Received: from gadalarx-mobl.amr.corp.intel.com (HELO khuang2-desk.gar.corp.intel.com) ([10.252.135.39]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Feb 2021 15:12:18 -0800 Date: Thu, 11 Feb 2021 12:12:16 +1300 From: Kai Huang To: Sean Christopherson Cc: Dave Hansen , Jarkko Sakkinen , linux-sgx@vger.kernel.org, kvm@vger.kernel.org, x86@kernel.org, luto@kernel.org, rick.p.edgecombe@intel.com, haitao.huang@intel.com, pbonzini@redhat.com, bp@alien8.de, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com Subject: Re: [RFC PATCH v4 05/26] x86/sgx: Introduce virtual EPC for use by KVM guests Message-Id: <20210211121216.bb9a9430eee1f2fd43702e93@intel.com> In-Reply-To: References: <11a923a314accf36a82aac4b676310a4802f5c75.1612777752.git.kai.huang@intel.com> <9aebc8e6-cff5-b2b4-04af-d3968a3586dc@intel.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-sgx@vger.kernel.org On Wed, 10 Feb 2021 08:52:25 -0800 Sean Christopherson wrote: > On Wed, Feb 10, 2021, Kai Huang wrote: > > On Tue, 2021-02-09 at 13:36 -0800, Dave Hansen wrote: > > > On 2/9/21 1:19 PM, Jarkko Sakkinen wrote: > > > > > Without that clearly documented, it would be unwise to merge this. > > > > E.g. > > > > > > > > - Have ioctl() to turn opened fd as vEPC. > > > > - If FLC is disabled, you could only use the fd for creating vEPC. > > > > > > > > Quite easy stuff to implement. > > ... > > > What's your opinion? Did I miss anything? > > Frankly, I think trying to smush them together would be a complete trainwreck. > > The vast majority of flows would need to go down completely different paths, so > you'd end up with code like this: > > diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c > index f2eac41bb4ff..5128043c7871 100644 > --- a/arch/x86/kernel/cpu/sgx/driver.c > +++ b/arch/x86/kernel/cpu/sgx/driver.c > @@ -46,6 +46,9 @@ static int sgx_release(struct inode *inode, struct file *file) > struct sgx_encl *encl = file->private_data; > struct sgx_encl_mm *encl_mm; > > + if (encl->not_an_enclave) > + return sgx_virt_epc_release(encl); > + > /* > * Drain the remaining mm_list entries. At this point the list contains > * entries for processes, which have closed the enclave file but have > @@ -83,6 +86,9 @@ static int sgx_mmap(struct file *file, struct vm_area_struct *vma) > struct sgx_encl *encl = file->private_data; > int ret; > > + if (encl->not_an_enclave) > + return sgx_virt_epc_mmap(encl, vma); > + > ret = sgx_encl_may_map(encl, vma->vm_start, vma->vm_end, vma->vm_flags); > if (ret) > return ret; > @@ -104,6 +110,11 @@ static unsigned long sgx_get_unmapped_area(struct file *file, > unsigned long pgoff, > unsigned long flags) > { > + struct sgx_encl *encl = file->private_data; > + > + if (encl->not_an_enclave) > + return sgx_virt_epc_mmap(encl, addr, len, pgoff, flags); > + > if ((flags & MAP_TYPE) == MAP_PRIVATE) > return -EINVAL; > > I suspect it would also be tricky to avoid introducing races, since anything that > is different for virtual EPC would have a dependency on the ioctl() being called. > > This would also prevent making /dev/sgx_enclave root-only while allowing users > access to /dev/sgx_vepc. Forcing admins to use LSMs to do the same is silly. Agreed. This is really a good point. Two different device nodes allows different permission control. Thanks. > > For the few flows that can share code, just split out the common bits to helpers.