From patchwork Fri Dec 2 21:23:54 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Christopherson X-Patchwork-Id: 9459255 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CDFB76074E for ; Fri, 2 Dec 2016 21:23:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BB75A2522B for ; Fri, 2 Dec 2016 21:23:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B0337285B2; Fri, 2 Dec 2016 21:23:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3F1362522B for ; Fri, 2 Dec 2016 21:23:57 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 1042D81F5E for ; Fri, 2 Dec 2016 13:23:57 -0800 (PST) X-Original-To: intel-sgx-kernel-dev@lists.01.org Delivered-To: intel-sgx-kernel-dev@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BE1A181F5E for ; Fri, 2 Dec 2016 13:23:55 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP; 02 Dec 2016 13:23:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.33,288,1477983600"; d="scan'208"; a="1076761043" Received: from orsmsx103.amr.corp.intel.com ([10.22.225.130]) by fmsmga001.fm.intel.com with ESMTP; 02 Dec 2016 13:23:55 -0800 Received: from orsmsx115.amr.corp.intel.com (10.22.240.11) by ORSMSX103.amr.corp.intel.com (10.22.225.130) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 2 Dec 2016 13:23:54 -0800 Received: from orsmsx108.amr.corp.intel.com ([169.254.2.107]) by ORSMSX115.amr.corp.intel.com ([10.22.240.11]) with mapi id 14.03.0248.002; Fri, 2 Dec 2016 13:23:54 -0800 From: "Christopherson, Sean J" To: 'Jarkko Sakkinen' Thread-Topic: [intel-sgx-kernel-dev] [PATCH v4 5/8] intel_sgx: freeze VMAs after EPC pages have been added Thread-Index: AQHSTBV4k7c8JNusgEGqiu3d3Q2P/6D0+ydwgACv2gD//35bwA== Date: Fri, 2 Dec 2016 21:23:54 +0000 Message-ID: <37306EFA9975BE469F115FDE982C075B9B695ABE@ORSMSX108.amr.corp.intel.com> References: <20161201205632.8593-1-jarkko.sakkinen@linux.intel.com> <20161201205632.8593-6-jarkko.sakkinen@linux.intel.com> <37306EFA9975BE469F115FDE982C075B9B695A31@ORSMSX108.amr.corp.intel.com> <20161202205620.jmlphjqgfzfhimja@intel.com> In-Reply-To: <20161202205620.jmlphjqgfzfhimja@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_PUBLIC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGU4YTZjNGUtZDBiYi00MGZhLTliNjMtMDE4YmNmOTUyZTM0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX1BVQkxJQyJ9XX1dfSwiU3ViamVjdExhYmVscyI6W10sIlRNQ1ZlcnNpb24iOiIxNS45LjYuNiIsIlRydXN0ZWRMYWJlbEhhc2giOiJuRWxoR1JsTzMzY3UyS21qbFVTUVwvd0RmaFNpOE10TERadnFFd3VQK0dUdz0ifQ== x-originating-ip: [10.22.254.139] MIME-Version: 1.0 Subject: Re: [intel-sgx-kernel-dev] [PATCH v4 5/8] intel_sgx: freeze VMAs after EPC pages have been added X-BeenThere: intel-sgx-kernel-dev@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Project: Intel® Software Guard Extensions for Linux*: https://01.org/intel-software-guard-extensions" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "intel-sgx-kernel-dev@lists.01.org" Errors-To: intel-sgx-kernel-dev-bounces@lists.01.org Sender: "intel-sgx-kernel-dev" X-Virus-Scanned: ClamAV using ClamSMTP Jarkko Sakkinen wrote: > > Below is my suggested version of sgx_vma_open/sgx_vma_close to > > eliminate the calls to sgx_invalidate. I think it makes sense to > > remove sgx_invalidate from sgx_vma_open as well as sgx_vma_close to > > keep "invalidate" behavior consistent between the two functions, and > > within sgx_vma_open itself, i.e. jump to out_fork like the other > > failing cases. This also saves us from grabbing an unnecessary > > reference to the encl and results in an early return in the subsequent > > sgx_vma_close since vma->vm_private_data is null. > > Is the difference to above is that you set the flag instead of calling > sgx_invalidate()? Are there other differences? The code also reinstates the zap_vma_ptes() calls in both functions and does not get a reference to the encl in the open case (because it jumps to out_fork). On top of your patch: diff --git a/intel_sgx_vma.c b/intel_sgx_vma.c index 1349d73..90c34b3 100644 --- a/intel_sgx_vma.c +++ b/intel_sgx_vma.c @@ -83,8 +83,9 @@ static void sgx_vma_open(struct vm_area_struct *vma) if (test_bit(SGX_ENCL_PAGES_ADDED, &encl->flags)) { mutex_lock(&encl->lock); - sgx_invalidate(encl); + encl->flags |= SGX_ENCL_INVALIDATED; mutex_unlock(&encl->lock); + goto out_fork; } kref_get(&encl->refcount); @@ -103,8 +104,10 @@ static void sgx_vma_close(struct vm_area_struct *vma) return; mutex_lock(&encl->lock); - sgx_invalidate(encl); + encl->flags |= SGX_ENCL_INVALIDATED; mutex_unlock(&encl->lock); + zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start); + vma->vm_private_data = NULL; kref_put(&encl->refcount, sgx_encl_release); } Or from the base: diff --git a/intel_sgx_vma.c b/intel_sgx_vma.c index 0649978..90c34b3 100644 --- a/intel_sgx_vma.c +++ b/intel_sgx_vma.c @@ -72,21 +72,21 @@ static void sgx_vma_open(struct vm_area_struct *vma) { struct sgx_encl *encl; - /* Was vm_private_data nullified as a result of the previous fork? */ + /* fork after fork */ encl = vma->vm_private_data; if (!encl) goto out_fork; - /* Was the process forked? mm_struct changes when the process is - * forked. - */ - mutex_lock(&encl->lock); - if (encl->mm != vma->vm_mm) { + /* mm_struct changes when the process is forked */ + if (encl->mm != vma->vm_mm) + goto out_fork; + + if (test_bit(SGX_ENCL_PAGES_ADDED, &encl->flags)) { + mutex_lock(&encl->lock); + encl->flags |= SGX_ENCL_INVALIDATED; mutex_unlock(&encl->lock); goto out_fork; } - encl->vma_cnt++; - mutex_unlock(&encl->lock); kref_get(&encl->refcount); return; @@ -99,21 +99,15 @@ static void sgx_vma_close(struct vm_area_struct *vma) { struct sgx_encl *encl = vma->vm_private_data; - /* If process was forked, VMA is still there but - * vm_private_data is set to NULL. - */ + /* fork */ if (!encl) return; mutex_lock(&encl->lock); - encl->vma_cnt--; - vma->vm_private_data = NULL; - - sgx_zap_tcs_ptes(encl, vma); - zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start); - + encl->flags |= SGX_ENCL_INVALIDATED; mutex_unlock(&encl->lock); - + zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start); + vma->vm_private_data = NULL; kref_put(&encl->refcount, sgx_encl_release); }