Re: [PATCH v30 07/20] x86/sgx: Enumerate and track EPC sections

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,
	Serge Ayoun <serge.ayoun@intel.com>,
	Jethro Beekman <jethro@fortanix.com>
Subject: Re: [PATCH v30 07/20] x86/sgx: Enumerate and track EPC sections
Date: Thu, 28 May 2020 09:14:32 +0300	[thread overview]
Message-ID: <20200528061432.GA102386@linux.intel.com> (raw)
In-Reply-To: <20200528053515.GA64796@linux.intel.com>

On Thu, May 28, 2020 at 08:35:15AM +0300, Jarkko Sakkinen wrote:
> On Thu, May 28, 2020 at 08:25:43AM +0300, Jarkko Sakkinen wrote:
> > On Tue, May 26, 2020 at 08:56:14PM -0700, Sean Christopherson wrote:
> > > On Mon, May 25, 2020 at 11:23:04AM +0200, Borislav Petkov wrote:
> > > > On Fri, May 15, 2020 at 03:43:57AM +0300, Jarkko Sakkinen wrote:
> > > > > +struct sgx_epc_section sgx_epc_sections[SGX_MAX_EPC_SECTIONS];
> > > > > +int sgx_nr_epc_sections;
> > > > 
> > > > We have become very averse against global stuff. What is going to use
> > > > those, only sgx code I assume...?
> > > 
> > > Yes, only SGX code.  The reclaim/swap code needs access to the sections,
> > > and that code is in a different file, reclaim.c.  I don't have a super
> > > strong objection to sucking reclaim.c into main.c, but I'm somewhat
> > > indifferent on code organization as a whole.  Jarkko likely has a stronger
> > > opinion.
> > 
> > I'll change it.
> > 
> > It's not quite as easy as just "sucking the file in". All the commits
> > that touch the file need to be reworked:
> > 
> > $ git --no-pager log --format="%H %s" arch/x86/kernel/cpu/sgx/reclaim.c
> > 5aeca6dabf767e9350ee3188ba25ceb21f3162b4 x86/sgx: Add a page reclaimer
> > de9b1088959f36ffdaf43a49bfea1c7f9f81cac7 x86/sgx: Linux Enclave Driver
> > 08d8fcb74fe268059ee58fcc2a0833b244e1f22a x86/sgx: Enumerate and track EPC sections
> 
> Not that I haven't done this a lot last few years. A proven approach
> is to do it in two "git rebase -i mainline/master" sweeps:
> 
> 1. For each commit, remove reclaim.c entry from the Makefile and import
>    reclaim.c contents to main.c.
> 2. For each commit, delete reclaim.c.
> 
> I've tried quite a few different angles and this what I've converged
> into. Very hard to hit messy into messy merge conflicts.

Remembered why the things are the way they are. Also ioctl.c needs these
symbols and I'd keep that separate from the contents of main.c and
reclaim.c. There the separation obviously makes sense.

I'll anyway merge main.c and reclaim.c as one for v31 because they are
strongly connected.

/Jarkko