From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752654AbdLGQFz (ORCPT ); Thu, 7 Dec 2017 11:05:55 -0500 Received: from mga05.intel.com ([192.55.52.43]:61860 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752609AbdLGQFx (ORCPT ); Thu, 7 Dec 2017 11:05:53 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,373,1508828400"; d="scan'208";a="184984852" Date: Thu, 7 Dec 2017 18:05:48 +0200 From: Jarkko Sakkinen To: "Christopherson, Sean J" Cc: "intel-sgx-kernel-dev@lists.01.org" , "platform-driver-x86@vger.kernel.org" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , Ingo Molnar , "H. Peter Anvin" , Darren Hart , Thomas Gleixner , Andy Shevchenko Subject: Re: [intel-sgx-kernel-dev] [PATCH v7 4/8] intel_sgx: driver for Intel Software Guard Extensions Message-ID: <20171207160548.doovfdh2lqg5brm3@linux.intel.com> References: <20171207015614.7914-1-jarkko.sakkinen@linux.intel.com> <20171207015614.7914-5-jarkko.sakkinen@linux.intel.com> <37306EFA9975BE469F115FDE982C075BC6B39193@ORSMSX108.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <37306EFA9975BE469F115FDE982C075BC6B39193@ORSMSX108.amr.corp.intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 07, 2017 at 02:46:39PM +0000, Christopherson, Sean J wrote: > > + for (i = 0; i < 2; i++) { > > + va_page = list_first_entry(&encl->va_pages, > > + struct sgx_va_page, list); > > + va_offset = sgx_alloc_va_slot(va_page); > > + if (va_offset < PAGE_SIZE) > > + break; > > + > > + list_move_tail(&va_page->list, &encl->va_pages); > > + } > > This is broken, there is no guarantee that the next VA page will have > a free slot. You have to walk over all VA pages to guarantee a slot > is found, e.g. this caused EWB and ELDU errors. I did run some extensive stress tests on this and did not experience any issues. Full VA pages are always put to the end. Please point me to the test where this breaks so that I can fix the issue if it persists. > Querying list.next to determine if an encl_page is resident in the EPC > is ugly and unintuitive, and depending on list's internal state seems > dangerous. Why not use a flag in the encl_page, e.g. as in the patch > I submitted almost 8 months ago for combining epc_page and va_page into > a union? And, the encl's SGX_ENCL_SECS_EVICTED flag can be dropped if > a flag is added to indicate whether or not any encl_page is resident in > the EPC. > > https://lists.01.org/pipermail/intel-sgx-kernel-dev/2017-April/000570.html I think it is better to just zero list entry and do list_empty test. You correct that checking that with poison is ugly. Last flag bit wll be needed for the SGX_ENCL_PAGE_TRIM. It is useful to have the flag in the enclave in order to be able to pack struct sgx_encl_page. /Jarkko