Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: Yf/wrz06MIODuTzxQlsrdd7eEGI4rM98fnCL15l+6FCbc3mugp38njZUq4YD0fszJ/GTBJhCJ5
 EUE1r1s+uIXw==
IronPort-SDR: nzLoNI+bhUndUVmfUQVSmr6S/Nufin6J4EAut0UP3tpxQTZ+D4kuCH22koxCHN+FxYSTnaE5FF
 J7Ah1wkgzeKw==
Date:   Thu, 28 May 2020 09:52:23 +0300
From:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To:     Sean Christopherson <sean.j.christopherson@intel.com>
Cc:     Borislav Petkov <bp@alien8.de>, linux-kernel@vger.kernel.org,
        x86@kernel.org, linux-sgx@vger.kernel.org,
        akpm@linux-foundation.org, dave.hansen@intel.com,
        nhorman@redhat.com, npmccallum@redhat.com, haitao.huang@intel.com,
        andriy.shevchenko@linux.intel.com, tglx@linutronix.de,
        kai.svahn@intel.com, josh@joshtriplett.org, luto@kernel.org,
        kai.huang@intel.com, rientjes@google.com, cedric.xing@intel.com,
        puiterwijk@redhat.com, Jethro Beekman <jethro@fortanix.com>
Subject: Re: [PATCH v30 08/20] x86/sgx: Add functions to allocate and free
 EPC pages
Message-ID: <20200528065223.GB188849@linux.intel.com>
References: <20200515004410.723949-1-jarkko.sakkinen@linux.intel.com>
 <20200515004410.723949-9-jarkko.sakkinen@linux.intel.com>
 <20200526125207.GE28228@zn.tnic>
 <20200527042111.GI31696@linux.intel.com>
 <20200527204638.GG1721@zn.tnic>
 <20200528012319.GA7577@linux.intel.com>
 <20200528013617.GD25962@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200528013617.GD25962@linux.intel.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, May 27, 2020 at 06:36:18PM -0700, Sean Christopherson wrote:
> On Thu, May 28, 2020 at 04:23:19AM +0300, Jarkko Sakkinen wrote:
> > On Wed, May 27, 2020 at 10:46:38PM +0200, Borislav Petkov wrote:
> > > On Tue, May 26, 2020 at 09:21:11PM -0700, Sean Christopherson wrote:
> > > > In other words, sgx_alloc_epc_section() is poorly named.  It doesn't
> > > > actually allocate EPC, it allocates kernel structures to map and track EPC.
> > > > sgx_(un)map_epc_section() would be more accurate and would hopefully
> > > > alleviate some of the confusion.
> 
> ...
> 
> > I'm not sure I follow fully Sean's reasoning but the way alloc is used
> > mostly in Linux is to ask through some API the used kernel memory
> > allocator to give memory for some kernel data structures.
> 
> Function names are usually some form of
> 
>   <namespace>_<verb>_<object>
> 
> where 'object' is the target of the 'verb'.  So sgx_alloc_epc_section()
> is most likely going to be read as "SGX, allocate an EPC section".  But
> that code doesn't allocate an EPC section, it maps an EPC section, and on
> success, adds the section's pages to the unsanitized list, i.e. what
> effectively becomes the pool of EPC pages.  The allocation part is a side
> effect of how we track EPC pages, it's not the primary purpose of the
> function.
> 
> Maybe sgx_add_epc_section() and sgx_remove_epc_section() would be better
> than map/unmap?
> 
> Eliminating the misnamed sgx_alloc_epc_section() frees up the "alloc" verb
> for use in the actual EPC page allocation paths, i.e. avoids having to
> rename those to "grab".  IMO, "alloc" is the best name as it most closely
> aligns with the nomenclature for regular pages, e.g. "grab" is most often
> used to elevate refcounts.

I'm thinking that you are over-engineering something this :-) Naming is
never perfect.

But I do get the original comment about sgx_alloc_page().

/Jarkko