linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: 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, bp@alien8.de,
	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>
Subject: Re: [PATCH v25 07/21] x86/sgx: Enumerate and track EPC sections
Date: Thu, 6 Feb 2020 17:34:06 +0200	[thread overview]
Message-ID: <20200206153406.GA9694@linux.intel.com> (raw)
In-Reply-To: <20200205231147.GD28111@linux.intel.com>

On Thu, Feb 06, 2020 at 01:11:47AM +0200, Jarkko Sakkinen wrote:
> On Wed, Feb 05, 2020 at 11:57:00AM -0800, Sean Christopherson wrote:
> > > +		ret = __eremove(sgx_epc_addr(page));
> > > +		if (!WARN_ON_ONCE(ret)) {
> > 
> > Sadly, this WARN can fire after kexec() on systems with multiple EPC
> > sections if the SECS has child pages in another section.
> 
> What causes this?

Ah obviously this can happen given that the final loop is done per
section before other sections are processed.

Lets fix the code first rather than change the approach based on code
that has an underlying regression. Performance can be fine tuned even
after upstreaming. Especially if the performance increases complexity it
is better to work that after there is a mainline code base.

Given that the loop is done in separate thread anyway, I'm not sure how
bad performance issue there is anyway. Performance based changes should
be always done based on a workloads and statistics.

I think you'd fix this issue by first changing the functions as:

static void sgx_sanitize_section(struct sgx_epc_section *section)
{
	struct sgx_epc_page *page, *tmp;
	LIST_HEAD(secs_list);
	int ret;

	while (!list_empty(&section->unsanitized_page_list)) {
		if (kthread_should_stop())
			return;

		spin_lock(&section->lock);

		page = list_first_entry(&section->unsanitized_page_list,
					struct sgx_epc_page, list);

		ret = __eremove(sgx_epc_addr(page));
		if (!ret)
			list_move(&page->list, &section->page_list);
		else
			list_move_tail(&page->list, &secs_list);

		spin_unlock(&section->lock);

		cond_resched();
	}

	list_move_tail(&secs_list, &section->unsanitized_list);
}

Then in ksgxswapd() you'd

for (i = 0; i < sgx_nr_epc_sections; i++)
	sgx_sanitize_section(&sgx_epc_sections[i]);

/* 2nd round for SECS */
for (i = 0; i < sgx_nr_epc_sections; i++)
	sgx_sanitize_section(&sgx_epc_sections[i]);

Finally you'd:

for (i = 0; i < sgx_nr_epc_sections; i++)
	WARN_ONCE(!list_empty(&sgx_epc_sections[i]->unsanitized_list));

/Jarkko

  reply	other threads:[~2020-02-06 15:34 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-04  6:05 [PATCH v25 00/21] Intel SGX foundations Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 01/21] x86/cpufeatures: x86/msr: Add Intel SGX hardware bits Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 02/21] x86/cpufeatures: x86/msr: Intel SGX Launch Control " Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 03/21] x86/mm: x86/sgx: Signal SIGSEGV with PF_SGX Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 04/21] x86/sgx: Add SGX microarchitectural data structures Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 05/21] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 06/21] x86/cpu/intel: Detect SGX supprt Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 07/21] x86/sgx: Enumerate and track EPC sections Jarkko Sakkinen
2020-02-05 19:57   ` Sean Christopherson
2020-02-05 23:11     ` Jarkko Sakkinen
2020-02-06 15:34       ` Jarkko Sakkinen [this message]
2020-02-06 15:35     ` Jarkko Sakkinen
2020-02-06 15:55       ` Sean Christopherson
2020-02-04  6:05 ` [PATCH v25 08/21] x86/sgx: Add functions to allocate and free EPC pages Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 09/21] mm: Introduce vm_ops->may_mprotect() Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 10/21] x86/sgx: Linux Enclave Driver Jarkko Sakkinen
2020-02-04  6:18   ` Jarkko Sakkinen
2020-02-05 15:58   ` Haitao Huang
2020-02-07 17:03   ` Haitao Huang
2020-02-04  6:05 ` [PATCH v25 11/21] selftests/x86: Recurse into subdirectories Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 12/21] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 13/21] x86/sgx: Add provisioning Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 14/21] x86/sgx: Add a page reclaimer Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 15/21] x86/sgx: ptrace() support for the SGX driver Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 16/21] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 17/21] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 18/21] x86/traps: Attempt to fixup exceptions in vDSO before signaling Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 19/21] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 20/21] selftests/x86: Add vDSO selftest for SGX Jarkko Sakkinen
2020-02-04  6:05 ` [PATCH v25 21/21] docs: x86/sgx: Document SGX micro architecture and kernel internals Jarkko Sakkinen
2020-02-05 17:54   ` Randy Dunlap
2020-02-05 23:07     ` Jarkko Sakkinen
2020-02-05 23:10       ` Randy Dunlap
2020-02-06 14:50         ` Jarkko Sakkinen
2020-02-04 15:11 ` [PATCH v25 00/21] Intel SGX foundations Sean Christopherson
2020-02-05 21:59   ` Jarkko Sakkinen
2020-02-05 23:09     ` Jarkko Sakkinen
2020-02-05 16:01 ` Haitao Huang
2020-02-05 22:01   ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200206153406.GA9694@linux.intel.com \
    --to=jarkko.sakkinen@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=cedric.xing@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=haitao.huang@intel.com \
    --cc=josh@joshtriplett.org \
    --cc=kai.huang@intel.com \
    --cc=kai.svahn@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=nhorman@redhat.com \
    --cc=npmccallum@redhat.com \
    --cc=puiterwijk@redhat.com \
    --cc=rientjes@google.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=serge.ayoun@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).